[ovs-dev] [PATCH 2/2] netdev-dpdk: Refactor vhost custom stats for extensibility.

Ilya Maximets i.maximets at samsung.com
Mon Aug 19 09:51:03 UTC 2019


On 19.08.2019 12:39, Sriram Vatala wrote:
> 
> -----Original Message-----
> From: Ilya Maximets <i.maximets at samsung.com>
> Sent: 09 August 2019 17:44
> To: ovs-dev at openvswitch.org
> Cc: Ian Stokes <ian.stokes at intel.com>; Kevin Traynor <ktraynor at redhat.com>; 
> David Marchand <david.marchand at redhat.com>; Sriram Vatala 
> <sriram.v at altencalsoftlabs.com>; Ilya Maximets <i.maximets at samsung.com>
> Subject: [PATCH 2/2] netdev-dpdk: Refactor vhost custom stats for 
> extensibility.
> 
> vHost interfaces currently has only one custom statistic, but there might be 
> others in the near future. This refactoring makes the code work in the same 
> way as it done for dpdk and afxdp stats to keep the common style over the 
> different code places and makes it easily extensible for the new stats 
> addition.
> 
> Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
> ---
>  lib/netdev-dpdk.c | 33 ++++++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 52ecf7576..90aabe3fb 
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -110,12 +110,6 @@ BUILD_ASSERT_DECL(MAX_NB_MBUF % 
> ROUND_DOWN_POW2(MAX_NB_MBUF / MIN_NB_MBUF)  BUILD_ASSERT_DECL((MAX_NB_MBUF / 
> ROUND_DOWN_POW2(MAX_NB_MBUF / MIN_NB_MBUF))
>                    % MP_CACHE_SZ == 0);
> 
> -/* Size of vHost custom stats. */
> -#define VHOST_CUSTOM_STATS_SIZE          1
> -
> -/* Names of vHost custom stats. */
> -#define VHOST_STAT_TX_RETRIES            "tx_retries"
> -
>  #define SOCKET0              0
> 
>  /* Default size of Physical NIC RXQ */
> @@ -2827,17 +2821,34 @@ netdev_dpdk_vhost_get_custom_stats(const struct netdev 
> *netdev,
>                                     struct netdev_custom_stats *custom_stats) 
> {
>      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> +    int i;
> 
> -    ovs_mutex_lock(&dev->mutex);
> +#define VHOST_CSTATS \
> +    VHOST_CSTAT(tx_retries)
> 
> -    custom_stats->size = VHOST_CUSTOM_STATS_SIZE;
> +#define VHOST_CSTAT(NAME) + 1
> +    custom_stats->size = VHOST_CSTATS;
> +#undef VHOST_CSTAT
>      custom_stats->counters = xcalloc(custom_stats->size,
>                                       sizeof *custom_stats->counters);
> -    ovs_strlcpy(custom_stats->counters[0].name, VHOST_STAT_TX_RETRIES,
> -                NETDEV_CUSTOM_STATS_NAME_SIZE);
> +
> +    for (i = 0; i < custom_stats->size; i++) { #define
> +VHOST_CSTAT(NAME) \
> +        ovs_strlcpy(custom_stats->counters[i].name, #NAME, \
> +                    NETDEV_CUSTOM_STATS_NAME_SIZE);
> +        VHOST_CSTATS;
> +#undef VHOST_CSTAT
> +    }
> +
> +    ovs_mutex_lock(&dev->mutex);
> 
>>>> In case of multiple VHOST_CSTAT entries declared under VHOST_CTATS, inside 
>>>> the for loop, VHOST_CSTATS would expand to string copy  for each entry with 
>>>> same index resulting in only updating  the last entry under   VHOST_CSTATS. 
>>>> So for all entries in custom_stats, counters field  will be same.

Oh, sorry. Thanks for spotting!
Will fix in v2.

> 
>      rte_spinlock_lock(&dev->stats_lock);
> -    custom_stats->counters[0].value = dev->tx_retries;
> +    for (i = 0; i < custom_stats->size; i++) { #define
> +VHOST_CSTAT(NAME) \
> +        custom_stats->counters[i].value = dev->NAME;
> +        VHOST_CSTATS;
> +#undef VHOST_CSTAT
> +    }
>      rte_spinlock_unlock(&dev->stats_lock);
> 
>>>> Here also "value" member for all  custom_stats  entries will be updated to 
>>>> same count in case of multiple VHOST_CSTAT entries.
>      ovs_mutex_unlock(&dev->mutex);
> --
> 2.17.1
> 
> Thanks for working on generic implementation for updating custom stats.
> 
> Thanks & Regards,
> Sriram.
> 


More information about the dev mailing list