[ovs-dev] [dpdk-latest PATCH] netdev-dpdk: add custom vhost statistics to count IRQs

Ilya Maximets i.maximets at ovn.org
Wed Oct 16 13:03:35 UTC 2019


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 <echaudro at redhat.com>
> ---
>   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.


More information about the dev mailing list