[ovs-dev,v2,1/2] netdev-dpdk: Fix not reporting rx_oversize_errors in stats.
diff mbox series

Message ID 20190819111849.5824-2-i.maximets@samsung.com
State Accepted
Headers show
Series
  • netdev-dpdk: Stats fix and refactoring.
Related show

Commit Message

Ilya Maximets Aug. 19, 2019, 11:18 a.m. UTC
There is a big code duplication issue with DPDK xstats that led to
missed "rx_oversize_errors" statistics. It's defined but not used.
Fix that by actually using this stat along with code refactoring that
will allow us to not make same mistakes in the future.
Macro definitions are perfectly suitable to automate code generation
in such cases and already used in a couple of places in OVS for similar
purposes.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 lib/netdev-dpdk.c | 102 +++++++++++++++-------------------------------
 1 file changed, 32 insertions(+), 70 deletions(-)

Comments

David Marchand Aug. 20, 2019, 9:42 a.m. UTC | #1
On Mon, Aug 19, 2019 at 1:19 PM Ilya Maximets <i.maximets@samsung.com> wrote:
>
> There is a big code duplication issue with DPDK xstats that led to
> missed "rx_oversize_errors" statistics. It's defined but not used.

Good catch.


> Fix that by actually using this stat along with code refactoring that
> will allow us to not make same mistakes in the future.
> Macro definitions are perfectly suitable to automate code generation
> in such cases and already used in a couple of places in OVS for similar
> purposes.
>
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>  lib/netdev-dpdk.c | 102 +++++++++++++++-------------------------------
>  1 file changed, 32 insertions(+), 70 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 48057835f..52ecf7576 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -110,34 +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);
>
> -/*
> - * DPDK XSTATS Counter names definition
> - */
> -#define XSTAT_RX_64_PACKETS              "rx_size_64_packets"
> -#define XSTAT_RX_65_TO_127_PACKETS       "rx_size_65_to_127_packets"
> -#define XSTAT_RX_128_TO_255_PACKETS      "rx_size_128_to_255_packets"
> -#define XSTAT_RX_256_TO_511_PACKETS      "rx_size_256_to_511_packets"
> -#define XSTAT_RX_512_TO_1023_PACKETS     "rx_size_512_to_1023_packets"
> -#define XSTAT_RX_1024_TO_1522_PACKETS    "rx_size_1024_to_1522_packets"
> -#define XSTAT_RX_1523_TO_MAX_PACKETS     "rx_size_1523_to_max_packets"
> -
> -#define XSTAT_TX_64_PACKETS              "tx_size_64_packets"
> -#define XSTAT_TX_65_TO_127_PACKETS       "tx_size_65_to_127_packets"
> -#define XSTAT_TX_128_TO_255_PACKETS      "tx_size_128_to_255_packets"
> -#define XSTAT_TX_256_TO_511_PACKETS      "tx_size_256_to_511_packets"
> -#define XSTAT_TX_512_TO_1023_PACKETS     "tx_size_512_to_1023_packets"
> -#define XSTAT_TX_1024_TO_1522_PACKETS    "tx_size_1024_to_1522_packets"
> -#define XSTAT_TX_1523_TO_MAX_PACKETS     "tx_size_1523_to_max_packets"
> -
> -#define XSTAT_RX_MULTICAST_PACKETS       "rx_multicast_packets"
> -#define XSTAT_TX_MULTICAST_PACKETS       "tx_multicast_packets"
> -#define XSTAT_RX_BROADCAST_PACKETS       "rx_broadcast_packets"
> -#define XSTAT_TX_BROADCAST_PACKETS       "tx_broadcast_packets"
> -#define XSTAT_RX_UNDERSIZED_ERRORS       "rx_undersized_errors"
> -#define XSTAT_RX_OVERSIZE_ERRORS         "rx_oversize_errors"
> -#define XSTAT_RX_FRAGMENTED_ERRORS       "rx_fragmented_errors"
> -#define XSTAT_RX_JABBER_ERRORS           "rx_jabber_errors"
> -
>  /* Size of vHost custom stats. */
>  #define VHOST_CUSTOM_STATS_SIZE          1
>
> @@ -2682,51 +2654,41 @@ netdev_dpdk_convert_xstats(struct netdev_stats *stats,
>                             const struct rte_eth_xstat_name *names,
>                             const unsigned int size)
>  {
> +/* DPDK XSTATS Counter names definition. */
> +#define DPDK_XSTATS \
> +    DPDK_XSTAT(multicast,               "rx_multicast_packets"            ) \
> +    DPDK_XSTAT(tx_multicast_packets,    "tx_multicast_packets"            ) \
> +    DPDK_XSTAT(rx_broadcast_packets,    "rx_broadcast_packets"            ) \
> +    DPDK_XSTAT(tx_broadcast_packets,    "tx_broadcast_packets"            ) \
> +    DPDK_XSTAT(rx_undersized_errors,    "rx_undersized_errors"            ) \
> +    DPDK_XSTAT(rx_oversize_errors,      "rx_oversize_errors"              ) \
> +    DPDK_XSTAT(rx_fragmented_errors,    "rx_fragmented_errors"            ) \
> +    DPDK_XSTAT(rx_jabber_errors,        "rx_jabber_errors"                ) \
> +    DPDK_XSTAT(rx_1_to_64_packets,      "rx_size_64_packets"              ) \
> +    DPDK_XSTAT(rx_65_to_127_packets,    "rx_size_65_to_127_packets"       ) \
> +    DPDK_XSTAT(rx_128_to_255_packets,   "rx_size_128_to_255_packets"      ) \
> +    DPDK_XSTAT(rx_256_to_511_packets,   "rx_size_256_to_511_packets"      ) \
> +    DPDK_XSTAT(rx_512_to_1023_packets,  "rx_size_512_to_1023_packets"     ) \
> +    DPDK_XSTAT(rx_1024_to_1522_packets, "rx_size_1024_to_1522_packets"    ) \
> +    DPDK_XSTAT(rx_1523_to_max_packets,  "rx_size_1523_to_max_packets"     ) \
> +    DPDK_XSTAT(tx_1_to_64_packets,      "tx_size_64_packets"              ) \
> +    DPDK_XSTAT(tx_65_to_127_packets,    "tx_size_65_to_127_packets"       ) \
> +    DPDK_XSTAT(tx_128_to_255_packets,   "tx_size_128_to_255_packets"      ) \
> +    DPDK_XSTAT(tx_256_to_511_packets,   "tx_size_256_to_511_packets"      ) \
> +    DPDK_XSTAT(tx_512_to_1023_packets,  "tx_size_512_to_1023_packets"     ) \
> +    DPDK_XSTAT(tx_1024_to_1522_packets, "tx_size_1024_to_1522_packets"    ) \
> +    DPDK_XSTAT(tx_1523_to_max_packets,  "tx_size_1523_to_max_packets"     )
> +
>      for (unsigned int i = 0; i < size; i++) {
> -        if (strcmp(XSTAT_RX_64_PACKETS, names[i].name) == 0) {
> -            stats->rx_1_to_64_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_65_TO_127_PACKETS, names[i].name) == 0) {
> -            stats->rx_65_to_127_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_128_TO_255_PACKETS, names[i].name) == 0) {
> -            stats->rx_128_to_255_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_256_TO_511_PACKETS, names[i].name) == 0) {
> -            stats->rx_256_to_511_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_512_TO_1023_PACKETS, names[i].name) == 0) {
> -            stats->rx_512_to_1023_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_1024_TO_1522_PACKETS, names[i].name) == 0) {
> -            stats->rx_1024_to_1522_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_1523_TO_MAX_PACKETS, names[i].name) == 0) {
> -            stats->rx_1523_to_max_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_64_PACKETS, names[i].name) == 0) {
> -            stats->tx_1_to_64_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_65_TO_127_PACKETS, names[i].name) == 0) {
> -            stats->tx_65_to_127_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_128_TO_255_PACKETS, names[i].name) == 0) {
> -            stats->tx_128_to_255_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_256_TO_511_PACKETS, names[i].name) == 0) {
> -            stats->tx_256_to_511_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_512_TO_1023_PACKETS, names[i].name) == 0) {
> -            stats->tx_512_to_1023_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_1024_TO_1522_PACKETS, names[i].name) == 0) {
> -            stats->tx_1024_to_1522_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_1523_TO_MAX_PACKETS, names[i].name) == 0) {
> -            stats->tx_1523_to_max_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_MULTICAST_PACKETS, names[i].name) == 0) {
> -            stats->multicast = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_MULTICAST_PACKETS, names[i].name) == 0) {
> -            stats->tx_multicast_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_BROADCAST_PACKETS, names[i].name) == 0) {
> -            stats->rx_broadcast_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_BROADCAST_PACKETS, names[i].name) == 0) {
> -            stats->tx_broadcast_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_UNDERSIZED_ERRORS, names[i].name) == 0) {
> -            stats->rx_undersized_errors = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_FRAGMENTED_ERRORS, names[i].name) == 0) {
> -            stats->rx_fragmented_errors = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_JABBER_ERRORS, names[i].name) == 0) {
> -            stats->rx_jabber_errors = xstats[i].value;
> +#define DPDK_XSTAT(MEMBER, NAME) \
> +        if (strcmp(NAME, names[i].name) == 0) {   \
> +            stats->MEMBER = xstats[i].value;      \
> +            continue;                             \
>          }
> +        DPDK_XSTATS;
> +#undef DPDK_XSTAT
>      }
> +#undef DPDK_XSTATS
>  }
>
>  static int
> --
> 2.17.1
>

Reviewed-by: David Marchand <david.marchand@redhat.com>


--
David Marchand
Kevin Traynor Aug. 23, 2019, 1:13 p.m. UTC | #2
On 19/08/2019 12:18, Ilya Maximets wrote:
> There is a big code duplication issue with DPDK xstats that led to
> missed "rx_oversize_errors" statistics. It's defined but not used.
> Fix that by actually using this stat along with code refactoring that
> will allow us to not make same mistakes in the future.
> Macro definitions are perfectly suitable to automate code generation
> in such cases and already used in a couple of places in OVS for similar
> purposes.
> 
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---

Acked-by: Kevin Traynor <ktraynor@redhat.com>
Stokes, Ian Aug. 23, 2019, 1:20 p.m. UTC | #3
On 8/19/2019 12:18 PM, Ilya Maximets wrote:
> There is a big code duplication issue with DPDK xstats that led to
> missed "rx_oversize_errors" statistics. It's defined but not used.
> Fix that by actually using this stat along with code refactoring that
> will allow us to not make same mistakes in the future.
> Macro definitions are perfectly suitable to automate code generation
> in such cases and already used in a couple of places in OVS for similar
> purposes.


LGTM, one minor query below.


> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>   lib/netdev-dpdk.c | 102 +++++++++++++++-------------------------------
>   1 file changed, 32 insertions(+), 70 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 48057835f..52ecf7576 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -110,34 +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);
>   
> -/*
> - * DPDK XSTATS Counter names definition
> - */
> -#define XSTAT_RX_64_PACKETS              "rx_size_64_packets"
> -#define XSTAT_RX_65_TO_127_PACKETS       "rx_size_65_to_127_packets"
> -#define XSTAT_RX_128_TO_255_PACKETS      "rx_size_128_to_255_packets"
> -#define XSTAT_RX_256_TO_511_PACKETS      "rx_size_256_to_511_packets"
> -#define XSTAT_RX_512_TO_1023_PACKETS     "rx_size_512_to_1023_packets"
> -#define XSTAT_RX_1024_TO_1522_PACKETS    "rx_size_1024_to_1522_packets"
> -#define XSTAT_RX_1523_TO_MAX_PACKETS     "rx_size_1523_to_max_packets"
> -
> -#define XSTAT_TX_64_PACKETS              "tx_size_64_packets"
> -#define XSTAT_TX_65_TO_127_PACKETS       "tx_size_65_to_127_packets"
> -#define XSTAT_TX_128_TO_255_PACKETS      "tx_size_128_to_255_packets"
> -#define XSTAT_TX_256_TO_511_PACKETS      "tx_size_256_to_511_packets"
> -#define XSTAT_TX_512_TO_1023_PACKETS     "tx_size_512_to_1023_packets"
> -#define XSTAT_TX_1024_TO_1522_PACKETS    "tx_size_1024_to_1522_packets"
> -#define XSTAT_TX_1523_TO_MAX_PACKETS     "tx_size_1523_to_max_packets"
> -
> -#define XSTAT_RX_MULTICAST_PACKETS       "rx_multicast_packets"
> -#define XSTAT_TX_MULTICAST_PACKETS       "tx_multicast_packets"
> -#define XSTAT_RX_BROADCAST_PACKETS       "rx_broadcast_packets"
> -#define XSTAT_TX_BROADCAST_PACKETS       "tx_broadcast_packets"
> -#define XSTAT_RX_UNDERSIZED_ERRORS       "rx_undersized_errors"
> -#define XSTAT_RX_OVERSIZE_ERRORS         "rx_oversize_errors"
> -#define XSTAT_RX_FRAGMENTED_ERRORS       "rx_fragmented_errors"
> -#define XSTAT_RX_JABBER_ERRORS           "rx_jabber_errors"
> -
>   /* Size of vHost custom stats. */
>   #define VHOST_CUSTOM_STATS_SIZE          1
>   
> @@ -2682,51 +2654,41 @@ netdev_dpdk_convert_xstats(struct netdev_stats *stats,
>                              const struct rte_eth_xstat_name *names,
>                              const unsigned int size)
>   {
> +/* DPDK XSTATS Counter names definition. */
> +#define DPDK_XSTATS \
> +    DPDK_XSTAT(multicast,               "rx_multicast_packets"            ) \

Should above be rx_multicast_packets to keep with the rx/tx naming 
convention with the rest of the stats?


Ian

> +    DPDK_XSTAT(tx_multicast_packets,    "tx_multicast_packets"            ) \
> +    DPDK_XSTAT(rx_broadcast_packets,    "rx_broadcast_packets"            ) \
> +    DPDK_XSTAT(tx_broadcast_packets,    "tx_broadcast_packets"            ) \
> +    DPDK_XSTAT(rx_undersized_errors,    "rx_undersized_errors"            ) \
> +    DPDK_XSTAT(rx_oversize_errors,      "rx_oversize_errors"              ) \
> +    DPDK_XSTAT(rx_fragmented_errors,    "rx_fragmented_errors"            ) \
> +    DPDK_XSTAT(rx_jabber_errors,        "rx_jabber_errors"                ) \
> +    DPDK_XSTAT(rx_1_to_64_packets,      "rx_size_64_packets"              ) \
> +    DPDK_XSTAT(rx_65_to_127_packets,    "rx_size_65_to_127_packets"       ) \
> +    DPDK_XSTAT(rx_128_to_255_packets,   "rx_size_128_to_255_packets"      ) \
> +    DPDK_XSTAT(rx_256_to_511_packets,   "rx_size_256_to_511_packets"      ) \
> +    DPDK_XSTAT(rx_512_to_1023_packets,  "rx_size_512_to_1023_packets"     ) \
> +    DPDK_XSTAT(rx_1024_to_1522_packets, "rx_size_1024_to_1522_packets"    ) \
> +    DPDK_XSTAT(rx_1523_to_max_packets,  "rx_size_1523_to_max_packets"     ) \
> +    DPDK_XSTAT(tx_1_to_64_packets,      "tx_size_64_packets"              ) \
> +    DPDK_XSTAT(tx_65_to_127_packets,    "tx_size_65_to_127_packets"       ) \
> +    DPDK_XSTAT(tx_128_to_255_packets,   "tx_size_128_to_255_packets"      ) \
> +    DPDK_XSTAT(tx_256_to_511_packets,   "tx_size_256_to_511_packets"      ) \
> +    DPDK_XSTAT(tx_512_to_1023_packets,  "tx_size_512_to_1023_packets"     ) \
> +    DPDK_XSTAT(tx_1024_to_1522_packets, "tx_size_1024_to_1522_packets"    ) \
> +    DPDK_XSTAT(tx_1523_to_max_packets,  "tx_size_1523_to_max_packets"     )
> +
>       for (unsigned int i = 0; i < size; i++) {
> -        if (strcmp(XSTAT_RX_64_PACKETS, names[i].name) == 0) {
> -            stats->rx_1_to_64_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_65_TO_127_PACKETS, names[i].name) == 0) {
> -            stats->rx_65_to_127_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_128_TO_255_PACKETS, names[i].name) == 0) {
> -            stats->rx_128_to_255_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_256_TO_511_PACKETS, names[i].name) == 0) {
> -            stats->rx_256_to_511_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_512_TO_1023_PACKETS, names[i].name) == 0) {
> -            stats->rx_512_to_1023_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_1024_TO_1522_PACKETS, names[i].name) == 0) {
> -            stats->rx_1024_to_1522_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_1523_TO_MAX_PACKETS, names[i].name) == 0) {
> -            stats->rx_1523_to_max_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_64_PACKETS, names[i].name) == 0) {
> -            stats->tx_1_to_64_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_65_TO_127_PACKETS, names[i].name) == 0) {
> -            stats->tx_65_to_127_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_128_TO_255_PACKETS, names[i].name) == 0) {
> -            stats->tx_128_to_255_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_256_TO_511_PACKETS, names[i].name) == 0) {
> -            stats->tx_256_to_511_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_512_TO_1023_PACKETS, names[i].name) == 0) {
> -            stats->tx_512_to_1023_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_1024_TO_1522_PACKETS, names[i].name) == 0) {
> -            stats->tx_1024_to_1522_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_1523_TO_MAX_PACKETS, names[i].name) == 0) {
> -            stats->tx_1523_to_max_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_MULTICAST_PACKETS, names[i].name) == 0) {
> -            stats->multicast = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_MULTICAST_PACKETS, names[i].name) == 0) {
> -            stats->tx_multicast_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_BROADCAST_PACKETS, names[i].name) == 0) {
> -            stats->rx_broadcast_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_TX_BROADCAST_PACKETS, names[i].name) == 0) {
> -            stats->tx_broadcast_packets = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_UNDERSIZED_ERRORS, names[i].name) == 0) {
> -            stats->rx_undersized_errors = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_FRAGMENTED_ERRORS, names[i].name) == 0) {
> -            stats->rx_fragmented_errors = xstats[i].value;
> -        } else if (strcmp(XSTAT_RX_JABBER_ERRORS, names[i].name) == 0) {
> -            stats->rx_jabber_errors = xstats[i].value;
> +#define DPDK_XSTAT(MEMBER, NAME) \
> +        if (strcmp(NAME, names[i].name) == 0) {   \
> +            stats->MEMBER = xstats[i].value;      \
> +            continue;                             \
>           }
> +        DPDK_XSTATS;
> +#undef DPDK_XSTAT
>       }
> +#undef DPDK_XSTATS
>   }
>   
>   static int
Ilya Maximets Aug. 23, 2019, 1:58 p.m. UTC | #4
On 23.08.2019 16:20, Stokes, Ian wrote:
> 
> On 8/19/2019 12:18 PM, Ilya Maximets wrote:
>> There is a big code duplication issue with DPDK xstats that led to
>> missed "rx_oversize_errors" statistics. It's defined but not used.
>> Fix that by actually using this stat along with code refactoring that
>> will allow us to not make same mistakes in the future.
>> Macro definitions are perfectly suitable to automate code generation
>> in such cases and already used in a couple of places in OVS for similar
>> purposes.
> 
> 
> LGTM, one minor query below.
> 
> 
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> ---
>>   lib/netdev-dpdk.c | 102 +++++++++++++++-------------------------------
>>   1 file changed, 32 insertions(+), 70 deletions(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 48057835f..52ecf7576 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -110,34 +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);
>>   -/*
>> - * DPDK XSTATS Counter names definition
>> - */
>> -#define XSTAT_RX_64_PACKETS              "rx_size_64_packets"
>> -#define XSTAT_RX_65_TO_127_PACKETS       "rx_size_65_to_127_packets"
>> -#define XSTAT_RX_128_TO_255_PACKETS      "rx_size_128_to_255_packets"
>> -#define XSTAT_RX_256_TO_511_PACKETS      "rx_size_256_to_511_packets"
>> -#define XSTAT_RX_512_TO_1023_PACKETS     "rx_size_512_to_1023_packets"
>> -#define XSTAT_RX_1024_TO_1522_PACKETS    "rx_size_1024_to_1522_packets"
>> -#define XSTAT_RX_1523_TO_MAX_PACKETS     "rx_size_1523_to_max_packets"
>> -
>> -#define XSTAT_TX_64_PACKETS              "tx_size_64_packets"
>> -#define XSTAT_TX_65_TO_127_PACKETS       "tx_size_65_to_127_packets"
>> -#define XSTAT_TX_128_TO_255_PACKETS      "tx_size_128_to_255_packets"
>> -#define XSTAT_TX_256_TO_511_PACKETS      "tx_size_256_to_511_packets"
>> -#define XSTAT_TX_512_TO_1023_PACKETS     "tx_size_512_to_1023_packets"
>> -#define XSTAT_TX_1024_TO_1522_PACKETS    "tx_size_1024_to_1522_packets"
>> -#define XSTAT_TX_1523_TO_MAX_PACKETS     "tx_size_1523_to_max_packets"
>> -
>> -#define XSTAT_RX_MULTICAST_PACKETS       "rx_multicast_packets"
>> -#define XSTAT_TX_MULTICAST_PACKETS       "tx_multicast_packets"
>> -#define XSTAT_RX_BROADCAST_PACKETS       "rx_broadcast_packets"
>> -#define XSTAT_TX_BROADCAST_PACKETS       "tx_broadcast_packets"
>> -#define XSTAT_RX_UNDERSIZED_ERRORS       "rx_undersized_errors"
>> -#define XSTAT_RX_OVERSIZE_ERRORS         "rx_oversize_errors"
>> -#define XSTAT_RX_FRAGMENTED_ERRORS       "rx_fragmented_errors"
>> -#define XSTAT_RX_JABBER_ERRORS           "rx_jabber_errors"
>> -
>>   /* Size of vHost custom stats. */
>>   #define VHOST_CUSTOM_STATS_SIZE          1
>>   @@ -2682,51 +2654,41 @@ netdev_dpdk_convert_xstats(struct netdev_stats *stats,
>>                              const struct rte_eth_xstat_name *names,
>>                              const unsigned int size)
>>   {
>> +/* DPDK XSTATS Counter names definition. */
>> +#define DPDK_XSTATS \
>> +    DPDK_XSTAT(multicast,               "rx_multicast_packets"            ) \
> 
> Should above be rx_multicast_packets to keep with the rx/tx naming convention with the rest of the stats?

It's a field of 'struct netdev_stats', we can't change it.
At least, not in this patch.

Best regards, Ilya Maximets.

> 
> 
> Ian
> 
>> +    DPDK_XSTAT(tx_multicast_packets,    "tx_multicast_packets"            ) \
>> +    DPDK_XSTAT(rx_broadcast_packets,    "rx_broadcast_packets"            ) \
>> +    DPDK_XSTAT(tx_broadcast_packets,    "tx_broadcast_packets"            ) \
>> +    DPDK_XSTAT(rx_undersized_errors,    "rx_undersized_errors"            ) \
>> +    DPDK_XSTAT(rx_oversize_errors,      "rx_oversize_errors"              ) \
>> +    DPDK_XSTAT(rx_fragmented_errors,    "rx_fragmented_errors"            ) \
>> +    DPDK_XSTAT(rx_jabber_errors,        "rx_jabber_errors"                ) \
>> +    DPDK_XSTAT(rx_1_to_64_packets,      "rx_size_64_packets"              ) \
>> +    DPDK_XSTAT(rx_65_to_127_packets,    "rx_size_65_to_127_packets"       ) \
>> +    DPDK_XSTAT(rx_128_to_255_packets,   "rx_size_128_to_255_packets"      ) \
>> +    DPDK_XSTAT(rx_256_to_511_packets,   "rx_size_256_to_511_packets"      ) \
>> +    DPDK_XSTAT(rx_512_to_1023_packets,  "rx_size_512_to_1023_packets"     ) \
>> +    DPDK_XSTAT(rx_1024_to_1522_packets, "rx_size_1024_to_1522_packets"    ) \
>> +    DPDK_XSTAT(rx_1523_to_max_packets,  "rx_size_1523_to_max_packets"     ) \
>> +    DPDK_XSTAT(tx_1_to_64_packets,      "tx_size_64_packets"              ) \
>> +    DPDK_XSTAT(tx_65_to_127_packets,    "tx_size_65_to_127_packets"       ) \
>> +    DPDK_XSTAT(tx_128_to_255_packets,   "tx_size_128_to_255_packets"      ) \
>> +    DPDK_XSTAT(tx_256_to_511_packets,   "tx_size_256_to_511_packets"      ) \
>> +    DPDK_XSTAT(tx_512_to_1023_packets,  "tx_size_512_to_1023_packets"     ) \
>> +    DPDK_XSTAT(tx_1024_to_1522_packets, "tx_size_1024_to_1522_packets"    ) \
>> +    DPDK_XSTAT(tx_1523_to_max_packets,  "tx_size_1523_to_max_packets"     )
>> +
>>       for (unsigned int i = 0; i < size; i++) {
>> -        if (strcmp(XSTAT_RX_64_PACKETS, names[i].name) == 0) {
>> -            stats->rx_1_to_64_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_RX_65_TO_127_PACKETS, names[i].name) == 0) {
>> -            stats->rx_65_to_127_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_RX_128_TO_255_PACKETS, names[i].name) == 0) {
>> -            stats->rx_128_to_255_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_RX_256_TO_511_PACKETS, names[i].name) == 0) {
>> -            stats->rx_256_to_511_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_RX_512_TO_1023_PACKETS, names[i].name) == 0) {
>> -            stats->rx_512_to_1023_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_RX_1024_TO_1522_PACKETS, names[i].name) == 0) {
>> -            stats->rx_1024_to_1522_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_RX_1523_TO_MAX_PACKETS, names[i].name) == 0) {
>> -            stats->rx_1523_to_max_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_TX_64_PACKETS, names[i].name) == 0) {
>> -            stats->tx_1_to_64_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_TX_65_TO_127_PACKETS, names[i].name) == 0) {
>> -            stats->tx_65_to_127_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_TX_128_TO_255_PACKETS, names[i].name) == 0) {
>> -            stats->tx_128_to_255_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_TX_256_TO_511_PACKETS, names[i].name) == 0) {
>> -            stats->tx_256_to_511_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_TX_512_TO_1023_PACKETS, names[i].name) == 0) {
>> -            stats->tx_512_to_1023_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_TX_1024_TO_1522_PACKETS, names[i].name) == 0) {
>> -            stats->tx_1024_to_1522_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_TX_1523_TO_MAX_PACKETS, names[i].name) == 0) {
>> -            stats->tx_1523_to_max_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_RX_MULTICAST_PACKETS, names[i].name) == 0) {
>> -            stats->multicast = xstats[i].value;
>> -        } else if (strcmp(XSTAT_TX_MULTICAST_PACKETS, names[i].name) == 0) {
>> -            stats->tx_multicast_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_RX_BROADCAST_PACKETS, names[i].name) == 0) {
>> -            stats->rx_broadcast_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_TX_BROADCAST_PACKETS, names[i].name) == 0) {
>> -            stats->tx_broadcast_packets = xstats[i].value;
>> -        } else if (strcmp(XSTAT_RX_UNDERSIZED_ERRORS, names[i].name) == 0) {
>> -            stats->rx_undersized_errors = xstats[i].value;
>> -        } else if (strcmp(XSTAT_RX_FRAGMENTED_ERRORS, names[i].name) == 0) {
>> -            stats->rx_fragmented_errors = xstats[i].value;
>> -        } else if (strcmp(XSTAT_RX_JABBER_ERRORS, names[i].name) == 0) {
>> -            stats->rx_jabber_errors = xstats[i].value;
>> +#define DPDK_XSTAT(MEMBER, NAME) \
>> +        if (strcmp(NAME, names[i].name) == 0) {   \
>> +            stats->MEMBER = xstats[i].value;      \
>> +            continue;                             \
>>           }
>> +        DPDK_XSTATS;
>> +#undef DPDK_XSTAT
>>       }
>> +#undef DPDK_XSTATS
>>   }
>>     static int
> 
>
Stokes, Ian Aug. 23, 2019, 2:05 p.m. UTC | #5
On 8/23/2019 2:58 PM, Ilya Maximets wrote:
> On 23.08.2019 16:20, Stokes, Ian wrote:
>> On 8/19/2019 12:18 PM, Ilya Maximets wrote:
>>> There is a big code duplication issue with DPDK xstats that led to
>>> missed "rx_oversize_errors" statistics. It's defined but not used.
>>> Fix that by actually using this stat along with code refactoring that
>>> will allow us to not make same mistakes in the future.
>>> Macro definitions are perfectly suitable to automate code generation
>>> in such cases and already used in a couple of places in OVS for similar
>>> purposes.
>>
>> LGTM, one minor query below.
>>
>>
>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>>> ---
>>>    lib/netdev-dpdk.c | 102 +++++++++++++++-------------------------------
>>>    1 file changed, 32 insertions(+), 70 deletions(-)
>>>
>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>> index 48057835f..52ecf7576 100644
>>> --- a/lib/netdev-dpdk.c
>>> +++ b/lib/netdev-dpdk.c
>>> @@ -110,34 +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);
>>>    -/*
>>> - * DPDK XSTATS Counter names definition
>>> - */
>>> -#define XSTAT_RX_64_PACKETS              "rx_size_64_packets"
>>> -#define XSTAT_RX_65_TO_127_PACKETS       "rx_size_65_to_127_packets"
>>> -#define XSTAT_RX_128_TO_255_PACKETS      "rx_size_128_to_255_packets"
>>> -#define XSTAT_RX_256_TO_511_PACKETS      "rx_size_256_to_511_packets"
>>> -#define XSTAT_RX_512_TO_1023_PACKETS     "rx_size_512_to_1023_packets"
>>> -#define XSTAT_RX_1024_TO_1522_PACKETS    "rx_size_1024_to_1522_packets"
>>> -#define XSTAT_RX_1523_TO_MAX_PACKETS     "rx_size_1523_to_max_packets"
>>> -
>>> -#define XSTAT_TX_64_PACKETS              "tx_size_64_packets"
>>> -#define XSTAT_TX_65_TO_127_PACKETS       "tx_size_65_to_127_packets"
>>> -#define XSTAT_TX_128_TO_255_PACKETS      "tx_size_128_to_255_packets"
>>> -#define XSTAT_TX_256_TO_511_PACKETS      "tx_size_256_to_511_packets"
>>> -#define XSTAT_TX_512_TO_1023_PACKETS     "tx_size_512_to_1023_packets"
>>> -#define XSTAT_TX_1024_TO_1522_PACKETS    "tx_size_1024_to_1522_packets"
>>> -#define XSTAT_TX_1523_TO_MAX_PACKETS     "tx_size_1523_to_max_packets"
>>> -
>>> -#define XSTAT_RX_MULTICAST_PACKETS       "rx_multicast_packets"
>>> -#define XSTAT_TX_MULTICAST_PACKETS       "tx_multicast_packets"
>>> -#define XSTAT_RX_BROADCAST_PACKETS       "rx_broadcast_packets"
>>> -#define XSTAT_TX_BROADCAST_PACKETS       "tx_broadcast_packets"
>>> -#define XSTAT_RX_UNDERSIZED_ERRORS       "rx_undersized_errors"
>>> -#define XSTAT_RX_OVERSIZE_ERRORS         "rx_oversize_errors"
>>> -#define XSTAT_RX_FRAGMENTED_ERRORS       "rx_fragmented_errors"
>>> -#define XSTAT_RX_JABBER_ERRORS           "rx_jabber_errors"
>>> -
>>>    /* Size of vHost custom stats. */
>>>    #define VHOST_CUSTOM_STATS_SIZE          1
>>>    @@ -2682,51 +2654,41 @@ netdev_dpdk_convert_xstats(struct netdev_stats *stats,
>>>                               const struct rte_eth_xstat_name *names,
>>>                               const unsigned int size)
>>>    {
>>> +/* DPDK XSTATS Counter names definition. */
>>> +#define DPDK_XSTATS \
>>> +    DPDK_XSTAT(multicast,               "rx_multicast_packets"            ) \
>> Should above be rx_multicast_packets to keep with the rx/tx naming convention with the rest of the stats?
> It's a field of 'struct netdev_stats', we can't change it.
> At least, not in this patch.


Ah, gotcha, just looked a bit funny at first glance, not sure why it 
wasn't named like that in the first place.

In that case Acked.

Ian

>
> Best regards, Ilya Maximets.
>
>>
>> Ian
>>
>>> +    DPDK_XSTAT(tx_multicast_packets,    "tx_multicast_packets"            ) \
>>> +    DPDK_XSTAT(rx_broadcast_packets,    "rx_broadcast_packets"            ) \
>>> +    DPDK_XSTAT(tx_broadcast_packets,    "tx_broadcast_packets"            ) \
>>> +    DPDK_XSTAT(rx_undersized_errors,    "rx_undersized_errors"            ) \
>>> +    DPDK_XSTAT(rx_oversize_errors,      "rx_oversize_errors"              ) \
>>> +    DPDK_XSTAT(rx_fragmented_errors,    "rx_fragmented_errors"            ) \
>>> +    DPDK_XSTAT(rx_jabber_errors,        "rx_jabber_errors"                ) \
>>> +    DPDK_XSTAT(rx_1_to_64_packets,      "rx_size_64_packets"              ) \
>>> +    DPDK_XSTAT(rx_65_to_127_packets,    "rx_size_65_to_127_packets"       ) \
>>> +    DPDK_XSTAT(rx_128_to_255_packets,   "rx_size_128_to_255_packets"      ) \
>>> +    DPDK_XSTAT(rx_256_to_511_packets,   "rx_size_256_to_511_packets"      ) \
>>> +    DPDK_XSTAT(rx_512_to_1023_packets,  "rx_size_512_to_1023_packets"     ) \
>>> +    DPDK_XSTAT(rx_1024_to_1522_packets, "rx_size_1024_to_1522_packets"    ) \
>>> +    DPDK_XSTAT(rx_1523_to_max_packets,  "rx_size_1523_to_max_packets"     ) \
>>> +    DPDK_XSTAT(tx_1_to_64_packets,      "tx_size_64_packets"              ) \
>>> +    DPDK_XSTAT(tx_65_to_127_packets,    "tx_size_65_to_127_packets"       ) \
>>> +    DPDK_XSTAT(tx_128_to_255_packets,   "tx_size_128_to_255_packets"      ) \
>>> +    DPDK_XSTAT(tx_256_to_511_packets,   "tx_size_256_to_511_packets"      ) \
>>> +    DPDK_XSTAT(tx_512_to_1023_packets,  "tx_size_512_to_1023_packets"     ) \
>>> +    DPDK_XSTAT(tx_1024_to_1522_packets, "tx_size_1024_to_1522_packets"    ) \
>>> +    DPDK_XSTAT(tx_1523_to_max_packets,  "tx_size_1523_to_max_packets"     )
>>> +
>>>        for (unsigned int i = 0; i < size; i++) {
>>> -        if (strcmp(XSTAT_RX_64_PACKETS, names[i].name) == 0) {
>>> -            stats->rx_1_to_64_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_RX_65_TO_127_PACKETS, names[i].name) == 0) {
>>> -            stats->rx_65_to_127_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_RX_128_TO_255_PACKETS, names[i].name) == 0) {
>>> -            stats->rx_128_to_255_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_RX_256_TO_511_PACKETS, names[i].name) == 0) {
>>> -            stats->rx_256_to_511_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_RX_512_TO_1023_PACKETS, names[i].name) == 0) {
>>> -            stats->rx_512_to_1023_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_RX_1024_TO_1522_PACKETS, names[i].name) == 0) {
>>> -            stats->rx_1024_to_1522_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_RX_1523_TO_MAX_PACKETS, names[i].name) == 0) {
>>> -            stats->rx_1523_to_max_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_TX_64_PACKETS, names[i].name) == 0) {
>>> -            stats->tx_1_to_64_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_TX_65_TO_127_PACKETS, names[i].name) == 0) {
>>> -            stats->tx_65_to_127_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_TX_128_TO_255_PACKETS, names[i].name) == 0) {
>>> -            stats->tx_128_to_255_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_TX_256_TO_511_PACKETS, names[i].name) == 0) {
>>> -            stats->tx_256_to_511_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_TX_512_TO_1023_PACKETS, names[i].name) == 0) {
>>> -            stats->tx_512_to_1023_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_TX_1024_TO_1522_PACKETS, names[i].name) == 0) {
>>> -            stats->tx_1024_to_1522_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_TX_1523_TO_MAX_PACKETS, names[i].name) == 0) {
>>> -            stats->tx_1523_to_max_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_RX_MULTICAST_PACKETS, names[i].name) == 0) {
>>> -            stats->multicast = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_TX_MULTICAST_PACKETS, names[i].name) == 0) {
>>> -            stats->tx_multicast_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_RX_BROADCAST_PACKETS, names[i].name) == 0) {
>>> -            stats->rx_broadcast_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_TX_BROADCAST_PACKETS, names[i].name) == 0) {
>>> -            stats->tx_broadcast_packets = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_RX_UNDERSIZED_ERRORS, names[i].name) == 0) {
>>> -            stats->rx_undersized_errors = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_RX_FRAGMENTED_ERRORS, names[i].name) == 0) {
>>> -            stats->rx_fragmented_errors = xstats[i].value;
>>> -        } else if (strcmp(XSTAT_RX_JABBER_ERRORS, names[i].name) == 0) {
>>> -            stats->rx_jabber_errors = xstats[i].value;
>>> +#define DPDK_XSTAT(MEMBER, NAME) \
>>> +        if (strcmp(NAME, names[i].name) == 0) {   \
>>> +            stats->MEMBER = xstats[i].value;      \
>>> +            continue;                             \
>>>            }
>>> +        DPDK_XSTATS;
>>> +#undef DPDK_XSTAT
>>>        }
>>> +#undef DPDK_XSTATS
>>>    }
>>>      static int
>>

Patch
diff mbox series

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 48057835f..52ecf7576 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -110,34 +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);
 
-/*
- * DPDK XSTATS Counter names definition
- */
-#define XSTAT_RX_64_PACKETS              "rx_size_64_packets"
-#define XSTAT_RX_65_TO_127_PACKETS       "rx_size_65_to_127_packets"
-#define XSTAT_RX_128_TO_255_PACKETS      "rx_size_128_to_255_packets"
-#define XSTAT_RX_256_TO_511_PACKETS      "rx_size_256_to_511_packets"
-#define XSTAT_RX_512_TO_1023_PACKETS     "rx_size_512_to_1023_packets"
-#define XSTAT_RX_1024_TO_1522_PACKETS    "rx_size_1024_to_1522_packets"
-#define XSTAT_RX_1523_TO_MAX_PACKETS     "rx_size_1523_to_max_packets"
-
-#define XSTAT_TX_64_PACKETS              "tx_size_64_packets"
-#define XSTAT_TX_65_TO_127_PACKETS       "tx_size_65_to_127_packets"
-#define XSTAT_TX_128_TO_255_PACKETS      "tx_size_128_to_255_packets"
-#define XSTAT_TX_256_TO_511_PACKETS      "tx_size_256_to_511_packets"
-#define XSTAT_TX_512_TO_1023_PACKETS     "tx_size_512_to_1023_packets"
-#define XSTAT_TX_1024_TO_1522_PACKETS    "tx_size_1024_to_1522_packets"
-#define XSTAT_TX_1523_TO_MAX_PACKETS     "tx_size_1523_to_max_packets"
-
-#define XSTAT_RX_MULTICAST_PACKETS       "rx_multicast_packets"
-#define XSTAT_TX_MULTICAST_PACKETS       "tx_multicast_packets"
-#define XSTAT_RX_BROADCAST_PACKETS       "rx_broadcast_packets"
-#define XSTAT_TX_BROADCAST_PACKETS       "tx_broadcast_packets"
-#define XSTAT_RX_UNDERSIZED_ERRORS       "rx_undersized_errors"
-#define XSTAT_RX_OVERSIZE_ERRORS         "rx_oversize_errors"
-#define XSTAT_RX_FRAGMENTED_ERRORS       "rx_fragmented_errors"
-#define XSTAT_RX_JABBER_ERRORS           "rx_jabber_errors"
-
 /* Size of vHost custom stats. */
 #define VHOST_CUSTOM_STATS_SIZE          1
 
@@ -2682,51 +2654,41 @@  netdev_dpdk_convert_xstats(struct netdev_stats *stats,
                            const struct rte_eth_xstat_name *names,
                            const unsigned int size)
 {
+/* DPDK XSTATS Counter names definition. */
+#define DPDK_XSTATS \
+    DPDK_XSTAT(multicast,               "rx_multicast_packets"            ) \
+    DPDK_XSTAT(tx_multicast_packets,    "tx_multicast_packets"            ) \
+    DPDK_XSTAT(rx_broadcast_packets,    "rx_broadcast_packets"            ) \
+    DPDK_XSTAT(tx_broadcast_packets,    "tx_broadcast_packets"            ) \
+    DPDK_XSTAT(rx_undersized_errors,    "rx_undersized_errors"            ) \
+    DPDK_XSTAT(rx_oversize_errors,      "rx_oversize_errors"              ) \
+    DPDK_XSTAT(rx_fragmented_errors,    "rx_fragmented_errors"            ) \
+    DPDK_XSTAT(rx_jabber_errors,        "rx_jabber_errors"                ) \
+    DPDK_XSTAT(rx_1_to_64_packets,      "rx_size_64_packets"              ) \
+    DPDK_XSTAT(rx_65_to_127_packets,    "rx_size_65_to_127_packets"       ) \
+    DPDK_XSTAT(rx_128_to_255_packets,   "rx_size_128_to_255_packets"      ) \
+    DPDK_XSTAT(rx_256_to_511_packets,   "rx_size_256_to_511_packets"      ) \
+    DPDK_XSTAT(rx_512_to_1023_packets,  "rx_size_512_to_1023_packets"     ) \
+    DPDK_XSTAT(rx_1024_to_1522_packets, "rx_size_1024_to_1522_packets"    ) \
+    DPDK_XSTAT(rx_1523_to_max_packets,  "rx_size_1523_to_max_packets"     ) \
+    DPDK_XSTAT(tx_1_to_64_packets,      "tx_size_64_packets"              ) \
+    DPDK_XSTAT(tx_65_to_127_packets,    "tx_size_65_to_127_packets"       ) \
+    DPDK_XSTAT(tx_128_to_255_packets,   "tx_size_128_to_255_packets"      ) \
+    DPDK_XSTAT(tx_256_to_511_packets,   "tx_size_256_to_511_packets"      ) \
+    DPDK_XSTAT(tx_512_to_1023_packets,  "tx_size_512_to_1023_packets"     ) \
+    DPDK_XSTAT(tx_1024_to_1522_packets, "tx_size_1024_to_1522_packets"    ) \
+    DPDK_XSTAT(tx_1523_to_max_packets,  "tx_size_1523_to_max_packets"     )
+
     for (unsigned int i = 0; i < size; i++) {
-        if (strcmp(XSTAT_RX_64_PACKETS, names[i].name) == 0) {
-            stats->rx_1_to_64_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_RX_65_TO_127_PACKETS, names[i].name) == 0) {
-            stats->rx_65_to_127_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_RX_128_TO_255_PACKETS, names[i].name) == 0) {
-            stats->rx_128_to_255_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_RX_256_TO_511_PACKETS, names[i].name) == 0) {
-            stats->rx_256_to_511_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_RX_512_TO_1023_PACKETS, names[i].name) == 0) {
-            stats->rx_512_to_1023_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_RX_1024_TO_1522_PACKETS, names[i].name) == 0) {
-            stats->rx_1024_to_1522_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_RX_1523_TO_MAX_PACKETS, names[i].name) == 0) {
-            stats->rx_1523_to_max_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_TX_64_PACKETS, names[i].name) == 0) {
-            stats->tx_1_to_64_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_TX_65_TO_127_PACKETS, names[i].name) == 0) {
-            stats->tx_65_to_127_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_TX_128_TO_255_PACKETS, names[i].name) == 0) {
-            stats->tx_128_to_255_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_TX_256_TO_511_PACKETS, names[i].name) == 0) {
-            stats->tx_256_to_511_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_TX_512_TO_1023_PACKETS, names[i].name) == 0) {
-            stats->tx_512_to_1023_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_TX_1024_TO_1522_PACKETS, names[i].name) == 0) {
-            stats->tx_1024_to_1522_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_TX_1523_TO_MAX_PACKETS, names[i].name) == 0) {
-            stats->tx_1523_to_max_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_RX_MULTICAST_PACKETS, names[i].name) == 0) {
-            stats->multicast = xstats[i].value;
-        } else if (strcmp(XSTAT_TX_MULTICAST_PACKETS, names[i].name) == 0) {
-            stats->tx_multicast_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_RX_BROADCAST_PACKETS, names[i].name) == 0) {
-            stats->rx_broadcast_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_TX_BROADCAST_PACKETS, names[i].name) == 0) {
-            stats->tx_broadcast_packets = xstats[i].value;
-        } else if (strcmp(XSTAT_RX_UNDERSIZED_ERRORS, names[i].name) == 0) {
-            stats->rx_undersized_errors = xstats[i].value;
-        } else if (strcmp(XSTAT_RX_FRAGMENTED_ERRORS, names[i].name) == 0) {
-            stats->rx_fragmented_errors = xstats[i].value;
-        } else if (strcmp(XSTAT_RX_JABBER_ERRORS, names[i].name) == 0) {
-            stats->rx_jabber_errors = xstats[i].value;
+#define DPDK_XSTAT(MEMBER, NAME) \
+        if (strcmp(NAME, names[i].name) == 0) {   \
+            stats->MEMBER = xstats[i].value;      \
+            continue;                             \
         }
+        DPDK_XSTATS;
+#undef DPDK_XSTAT
     }
+#undef DPDK_XSTATS
 }
 
 static int