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.maxim...@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 <ktray...@redhat.com> > >> + VHOST_CSTATS; >> +#undef VHOST_CSTAT >> rte_spinlock_unlock(&dev->stats_lock); >> >> ovs_mutex_unlock(&dev->mutex); >> > > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev