On 15.10.2019 13:20, Eelco Chaudron wrote:
When the dpdk vhost library executes an eventfd_write() call,
i.e. waking up the guest, a new callback will be called.

This patch adds the callback to count the number of
interrupts sent to the VM to track the number of times
interrupts where generated.

This might be of interest to find out system-calls were
called in the DPDK fast path.

The custom statistics can be seen with the following command:

   ovs-ofctl -O OpenFlow14 dump-ports <bridge> {<port>}

Signed-off-by: Eelco Chaudron <[email protected]>
---
  lib/netdev-dpdk.c |   27 ++++++++++++++++++++++++++-
  1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ba92e89..7ebbcdc 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -165,6 +165,8 @@ static int new_device(int vid);
  static void destroy_device(int vid);
  static int vring_state_changed(int vid, uint16_t queue_id, int enable);
  static void destroy_connection(int vid);
+static void vhost_guest_notified(int vid);
+
  static const struct vhost_device_ops virtio_net_device_ops =
  {
      .new_device =  new_device,
@@ -173,6 +175,7 @@ static const struct vhost_device_ops virtio_net_device_ops =
      .features_changed = NULL,
      .new_connection = NULL,
      .destroy_connection = destroy_connection,
+    .guest_notified = vhost_guest_notified,
  };
enum { DPDK_RING_SIZE = 256 };
@@ -416,6 +419,8 @@ struct netdev_dpdk {
          struct netdev_stats stats;
          /* Custom stat for retries when unable to transmit. */
          uint64_t tx_retries;
+        /* Custom stat counting number of times a vhost remote was woken up */
+        uint64_t vhost_irqs;
          /* Protects stats */
          rte_spinlock_t stats_lock;
          /* 4 pad bytes here. */
@@ -2826,7 +2831,8 @@ netdev_dpdk_vhost_get_custom_stats(const struct netdev 
*netdev,
      int i;
#define VHOST_CSTATS \
-    VHOST_CSTAT(tx_retries)
+    VHOST_CSTAT(tx_retries) \
+    VHOST_CSTAT(vhost_irqs)
#define VHOST_CSTAT(NAME) + 1
      custom_stats->size = VHOST_CSTATS;
@@ -3746,6 +3752,25 @@ destroy_connection(int vid)
      }
  }
+static
+void vhost_guest_notified(int vid)
+{
+    struct netdev_dpdk *dev;
+
+    ovs_mutex_lock(&dpdk_mutex);
+    LIST_FOR_EACH (dev, list_node, &dpdk_list) {
+        if (netdev_dpdk_get_vid(dev) == vid) {
+            ovs_mutex_lock(&dev->mutex);
+            rte_spinlock_lock(&dev->stats_lock);
+            dev->vhost_irqs++;
+            rte_spinlock_unlock(&dev->stats_lock);
+            ovs_mutex_unlock(&dev->mutex);
+            break;
+        }
+    }
+    ovs_mutex_unlock(&dpdk_mutex);

So, for every single eventfd_write() we're taking the global mutex,
traversing list of all the devices, taking one more mutex and finally
taking the spinlock just to increment a single counter.
I think, it's too much.

Wouldn't it significantly affect interrupt based virtio drivers in
terms of performance?  How frequently 2 vhost-user ports will lock
each other on the global device mutex?  How frequently 2 PMD threads
will lock each other on rx/tx to different vrings of the same vhost
port?

I see that 'guest_notified' patch is already in dpdk master, but
wouldn't it be better to accumulate this statistics inside DPDK
and return it with some new API like rte_vhost_get_vring_xstats()
if required?  I don't see any usecase for this callback beside
counting some stats.

For the current patch I could only suggest to convert it to simple
coverage counter like it done in 'vhost_tx_contention' patch here:
https://patchwork.ozlabs.org/patch/1175849/

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to