[ovs-dev,2/2] netdev-dpdk: Refactor vhost custom stats for extensibility.
diff mbox series

Message ID 20190809121347.4845-3-i.maximets@samsung.com
State Superseded
Headers show
Series
  • netdev-dpdk: Stats fix and refactoring.
Related show

Commit Message

Ilya Maximets Aug. 9, 2019, 12:13 p.m. UTC
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@samsung.com>
---
 lib/netdev-dpdk.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

Comments

Sriram Vatala via dev Aug. 19, 2019, 9:39 a.m. UTC | #1
-----Original Message-----
From: Ilya Maximets <i.maximets@samsung.com>
Sent: 09 August 2019 17:44
To: ovs-dev@openvswitch.org
Cc: Ian Stokes <ian.stokes@intel.com>; Kevin Traynor <ktraynor@redhat.com>; 
David Marchand <david.marchand@redhat.com>; Sriram Vatala 
<sriram.v@altencalsoftlabs.com>; Ilya Maximets <i.maximets@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@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.

     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.
Ilya Maximets Aug. 19, 2019, 9:51 a.m. UTC | #2
On 19.08.2019 12:39, Sriram Vatala wrote:
> 
> -----Original Message-----
> From: Ilya Maximets <i.maximets@samsung.com>
> Sent: 09 August 2019 17:44
> To: ovs-dev@openvswitch.org
> Cc: Ian Stokes <ian.stokes@intel.com>; Kevin Traynor <ktraynor@redhat.com>; 
> David Marchand <david.marchand@redhat.com>; Sriram Vatala 
> <sriram.v@altencalsoftlabs.com>; Ilya Maximets <i.maximets@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@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.
>

Patch
diff mbox series

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);
 
     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);
 
     ovs_mutex_unlock(&dev->mutex);