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

Kevin Traynor ktraynor at redhat.com
Tue Oct 15 13:35:14 UTC 2019


With corrected email for Ilya.

On 15/10/2019 14:33, Kevin Traynor wrote:
> On 21/09/2019 03:40, Sriram Vatala wrote:
>> From: Ilya Maximets <i.maximets at samsung.com>
>>
>> This is yet another refactoring for upcoming detailed drop stats.
>> It allowes 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.
>>
> 
> Hi Ilya, one comment below,
> 
> thanks,
> Kevin.
> 
>> 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 | 67 +++++++++++++++++++++++++++--------------------
>>  1 file changed, 39 insertions(+), 28 deletions(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index bc20d6843..652b57e3b 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -471,6 +471,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);
>> @@ -1171,7 +1173,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;
>>  }
>> @@ -2771,7 +2773,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);
>>  
>> @@ -2786,12 +2790,13 @@ 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++) {
>> +            for (i = sw_stats_size; i < sw_stats_size + rte_xstats_ret; i++) {
>>                  ovs_strlcpy(custom_stats->counters[i].name,
>>                              netdev_dpdk_get_xstat_name(dev,
>>                                                         dev->rte_xstats_ids[i]),
> 
> I think you need to add another array index counter for ret_xstats_ids[]
> and values[] as they are still using i, but i is now starting with
> sw_stats_size and not 0 anymore.
> 
>> @@ -2801,8 +2806,6 @@ netdev_dpdk_get_custom_stats(const struct netdev *netdev,
>>          } 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);
>> @@ -2817,39 +2820,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;
>>  }
>>  
>> @@ -4433,7 +4444,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,
>> @@ -4449,7 +4460,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,
>>
> 



More information about the dev mailing list