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

Ilya Maximets i.maximets at samsung.com
Fri Aug 23 14:05:29 UTC 2019


On 23.08.2019 16:16, Kevin Traynor wrote:
> On 19/08/2019 12:18, Ilya Maximets wrote:
>> 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 | 28 ++++++++++++++++++----------
>>  1 file changed, 18 insertions(+), 10 deletions(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 52ecf7576..bc20d6843 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,31 @@ 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,
>> +    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);
>> -    custom_stats->counters[0].value = dev->tx_retries;
>> +    i = 0;
>> +#define VHOST_CSTAT(NAME) \
>> +    custom_stats->counters[i++].value = dev->NAME;
> 
> That would fit on one line, but maybe you want it to look consistent
> with the previous #define.

I wanted the code to have same indentation level to look like the same code
flow for better understanding.

> 
> Also, I wonder would having a blank line after the #define's make them
> easier to distinguish?

#define's are tightly coupled with their usage by VHOST_CSTATS.
IMHO, separating them by blank lines makes the code harder to understand.
I'd prefer keeping them as a solid define-undef block.

> 
> Either way, Acked-by: Kevin Traynor <ktraynor at redhat.com>
> 
>> +    VHOST_CSTATS;
>> +#undef VHOST_CSTAT
>>      rte_spinlock_unlock(&dev->stats_lock);
>>  
>>      ovs_mutex_unlock(&dev->mutex);
>>
> 
> 
> 


More information about the dev mailing list