[ovs-dev] [PATCH v11 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats.

Sriram Vatala sriram.v at altencalsoftlabs.com
Tue Nov 5 12:54:47 UTC 2019


Thanks Kevin for your response.

@i.maximets at ovn.org  : Can you please review the patch set.

Thanks & Regards,
Sriram.

-----Original Message-----
From: Kevin Traynor <ktraynor at redhat.com> 
Sent: 05 November 2019 16:07
To: Sriram Vatala <sriram.v at altencalsoftlabs.com>; ovs-dev at openvswitch.org;
i.maximets at ovn.org
Cc: 'Ilya Maximets' <i.maximets at samsung.com>
Subject: Re: [PATCH v11 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH
custom stats.

On 05/11/2019 04:52, Sriram Vatala wrote:
> Hi All,
> Please consider this as a gentle remainder.
> 
> Thanks & Regards,
> Sriram.
> 
> -----Original Message-----
> From: Sriram Vatala <sriram.v at altencalsoftlabs.com>
> Sent: 30 October 2019 11:57
> To: 'ovs-dev at openvswitch.org' <ovs-dev at openvswitch.org>; 
> 'ktraynor at redhat.com' <ktraynor at redhat.com>; 'i.maximets at ovn.org'
> <i.maximets at ovn.org>
> Cc: 'Ilya Maximets' <i.maximets at samsung.com>
> Subject: RE: [PATCH v11 1/3] netdev-dpdk: Reuse vhost function for 
> dpdk ETH custom stats.
> 
> Hi All,
> Please review the update patch.
> 
> Changes since v10 :
> Corrected the spelling mistake in the commit message.
> 

Hi Sriram,

I've acked v10, so not planning to re-review unless there are some other
changes.

thanks,
Kevin.

p.s. In general if it is only minor changes it is usually ok to keep acks
from a previous version. If it is doubtful you can check with the reviewer.

> Thanks & Regards,
> Sriram.
> 
> -----Original Message-----
> From: Sriram Vatala <sriram.v at altencalsoftlabs.com>
> Sent: 29 October 2019 20:20
> To: ovs-dev at openvswitch.org; ktraynor at redhat.com; i.maximets at ovn.org
> Cc: Ilya Maximets <i.maximets at samsung.com>; Sriram Vatala 
> <sriram.v at altencalsoftlabs.com>
> Subject: [PATCH v11 1/3] netdev-dpdk: Reuse vhost function for dpdk 
> ETH custom stats.
> 
> From: Ilya Maximets <i.maximets at samsung.com>
> 
> This is yet another refactoring for upcoming detailed drop stats.
> It allows to use single function for all the software calculated 
> statistics in netdev-dpdk for both vhost and ETH ports.
> 
> UINT64_MAX used as a marker for non-supported statistics in a same way 
> as it's done in bridge.c for common netdev stats.
> 
> Co-authored-by: Sriram Vatala <sriram.v at altencalsoftlabs.com>
> Cc: Sriram Vatala <sriram.v at altencalsoftlabs.com>
> Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
> Signed-off-by: Sriram Vatala <sriram.v at altencalsoftlabs.com>
> ---
>  lib/netdev-dpdk.c | 69 
> +++++++++++++++++++++++++++--------------------
>  1 file changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> 04e1a2d1b..2cc2516a9 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -474,6 +474,8 @@ struct netdev_rxq_dpdk {  static void 
> netdev_dpdk_destruct(struct netdev *netdev);  static void 
> netdev_dpdk_vhost_destruct(struct netdev *netdev);
>  
> +static int netdev_dpdk_get_sw_custom_stats(const struct netdev *,
> +                                           struct netdev_custom_stats 
> +*);
>  static void netdev_dpdk_clear_xstats(struct netdev_dpdk *dev);
>  
>  int netdev_dpdk_get_vid(const struct netdev_dpdk *dev); @@ -1174,7 
> +1176,7 @@ common_construct(struct netdev *netdev, dpdk_port_t port_no,
>      dev->rte_xstats_ids = NULL;
>      dev->rte_xstats_ids_size = 0;
>  
> -    dev->tx_retries = 0;
> +    dev->tx_retries = (dev->type == DPDK_DEV_VHOST) ? 0 : UINT64_MAX;
>  
>      return 0;
>  }
> @@ -2775,7 +2777,9 @@ netdev_dpdk_get_custom_stats(const struct netdev 
> *netdev,
>  
>      uint32_t i;
>      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> -    int rte_xstats_ret;
> +    int rte_xstats_ret, sw_stats_size;
> +
> +    netdev_dpdk_get_sw_custom_stats(netdev, custom_stats);
>  
>      ovs_mutex_lock(&dev->mutex);
>  
> @@ -2790,23 +2794,22 @@ netdev_dpdk_get_custom_stats(const struct 
> netdev *netdev,
>          if (rte_xstats_ret > 0 &&
>              rte_xstats_ret <= dev->rte_xstats_ids_size) {
>  
> -            custom_stats->size = rte_xstats_ret;
> -            custom_stats->counters =
> -                    (struct netdev_custom_counter *)
> xcalloc(rte_xstats_ret,
> -                            sizeof(struct netdev_custom_counter));
> +            sw_stats_size = custom_stats->size;
> +            custom_stats->size += rte_xstats_ret;
> +            custom_stats->counters = xrealloc(custom_stats->counters,
> +                                              custom_stats->size *
> +                                              sizeof 
> + *custom_stats->counters);
>  
>              for (i = 0; i < rte_xstats_ret; i++) {
> -                ovs_strlcpy(custom_stats->counters[i].name,
> +                ovs_strlcpy(custom_stats->counters[sw_stats_size + 
> + i].name,
>                              netdev_dpdk_get_xstat_name(dev,
>  
> dev->rte_xstats_ids[i]),
>                              NETDEV_CUSTOM_STATS_NAME_SIZE);
> -                custom_stats->counters[i].value = values[i];
> +                custom_stats->counters[sw_stats_size + i].value = 
> + values[i];
>              }
>          } else {
>              VLOG_WARN("Cannot get XSTATS values for port:
> "DPDK_PORT_ID_FMT,
>                        dev->port_id);
> -            custom_stats->counters = NULL;
> -            custom_stats->size = 0;
>              /* Let's clear statistics cache, so it will be
>               * reconfigured */
>              netdev_dpdk_clear_xstats(dev); @@ -2821,39 +2824,47 @@ 
> netdev_dpdk_get_custom_stats(const struct netdev *netdev,  }
>  
>  static int
> -netdev_dpdk_vhost_get_custom_stats(const struct netdev *netdev,
> -                                   struct netdev_custom_stats
> *custom_stats)
> +netdev_dpdk_get_sw_custom_stats(const struct netdev *netdev,
> +                                struct netdev_custom_stats
> +*custom_stats)
>  {
>      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> -    int i;
> +    int i, n;
>  
> -#define VHOST_CSTATS \
> -    VHOST_CSTAT(tx_retries)
> +#define SW_CSTATS \
> +    SW_CSTAT(tx_retries)
>  
> -#define VHOST_CSTAT(NAME) + 1
> -    custom_stats->size = VHOST_CSTATS;
> -#undef VHOST_CSTAT
> +#define SW_CSTAT(NAME) + 1
> +    custom_stats->size = SW_CSTATS;
> +#undef SW_CSTAT
>      custom_stats->counters = xcalloc(custom_stats->size,
>                                       sizeof *custom_stats->counters);
> -    i = 0;
> -#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);
>  
>      rte_spinlock_lock(&dev->stats_lock);
>      i = 0;
> -#define VHOST_CSTAT(NAME) \
> +#define SW_CSTAT(NAME) \
>      custom_stats->counters[i++].value = dev->NAME;
> -    VHOST_CSTATS;
> -#undef VHOST_CSTAT
> +    SW_CSTATS;
> +#undef SW_CSTAT
>      rte_spinlock_unlock(&dev->stats_lock);
>  
>      ovs_mutex_unlock(&dev->mutex);
>  
> +    i = 0;
> +    n = 0;
> +#define SW_CSTAT(NAME) \
> +    if (custom_stats->counters[i].value != UINT64_MAX) {
> \
> +        ovs_strlcpy(custom_stats->counters[n].name, #NAME,
> \
> +                    NETDEV_CUSTOM_STATS_NAME_SIZE);
> \
> +        custom_stats->counters[n].value = 
> + custom_stats->counters[i].value;
> \
> +        n++;
> \
> +    }
> \
> +    i++;
> +    SW_CSTATS;
> +#undef SW_CSTAT
> +
> +    custom_stats->size = n;
>      return 0;
>  }
>  
> @@ -4437,7 +4448,7 @@ static const struct netdev_class dpdk_vhost_class =
{
>      .send = netdev_dpdk_vhost_send,
>      .get_carrier = netdev_dpdk_vhost_get_carrier,
>      .get_stats = netdev_dpdk_vhost_get_stats,
> -    .get_custom_stats = netdev_dpdk_vhost_get_custom_stats,
> +    .get_custom_stats = netdev_dpdk_get_sw_custom_stats,
>      .get_status = netdev_dpdk_vhost_user_get_status,
>      .reconfigure = netdev_dpdk_vhost_reconfigure,
>      .rxq_recv = netdev_dpdk_vhost_rxq_recv, @@ -4453,7 +4464,7 @@ 
> static const struct netdev_class dpdk_vhost_client_class = {
>      .send = netdev_dpdk_vhost_send,
>      .get_carrier = netdev_dpdk_vhost_get_carrier,
>      .get_stats = netdev_dpdk_vhost_get_stats,
> -    .get_custom_stats = netdev_dpdk_vhost_get_custom_stats,
> +    .get_custom_stats = netdev_dpdk_get_sw_custom_stats,
>      .get_status = netdev_dpdk_vhost_user_get_status,
>      .reconfigure = netdev_dpdk_vhost_client_reconfigure,
>      .rxq_recv = netdev_dpdk_vhost_rxq_recv,
> --
> 2.20.1
> 


More information about the dev mailing list