[ovs-dev] [RFC PATCH] netdev-dpdk: Expose per rxq/txq basic statistics.

Kevin Traynor ktraynor at redhat.com
Thu Nov 18 15:31:11 UTC 2021


On 15/10/2021 16:04, David Marchand wrote:
> When troubleshooting multiqueue setups, having per queue statistics helps
> checking packets repartition in rx and tx queues.
> 
> Per queue statistics are exported by most DPDK drivers (with capability
> RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS). But since OVS only filters statistics
> it exposes, there is nothing to request in DPDK API.
> 
> Extend existing filter with a regular expression.
> string_ends_with() helper is not used anymore, and removed as a
> consequence.
> 
> Querying statistics with
> $ ovs-vsctl get interface dpdk0 statistics | \
>    sed -e 's#[{}]##g' -e 's#, #\n#g'
> 
> and comparing gives:
> @@ -13,7 +13,12 @@
>   rx_phy_crc_errors=0
>   rx_phy_in_range_len_errors=0
>   rx_phy_symbol_errors=0
> +rx_q0_bytes=0
>   rx_q0_errors=0
> +rx_q0_packets=0
> +rx_q1_bytes=0
> +rx_q1_errors=0
> +rx_q1_packets=0
>   rx_wqe_errors=0
>   tx_broadcast_packets=0
>   tx_bytes=0
> @@ -27,3 +32,13 @@
>   tx_pp_rearm_queue_errors=0
>   tx_pp_timestamp_future_errors=0
>   tx_pp_timestamp_past_errors=0
> +tx_q0_bytes=0
> +tx_q0_packets=0
> +tx_q1_bytes=0
> +tx_q1_packets=0
> +tx_q2_bytes=0
> +tx_q2_packets=0
> +tx_q3_bytes=0
> +tx_q3_packets=0
> +tx_q4_bytes=0
> +tx_q4_packets=0
> 
> Signed-off-by: David Marchand <david.marchand at redhat.com>

The patch looks good but I tested with with ixgbe, and seeing something 
I am unsure of. I changed the number of rxqs to 11 but I am only seeing 
stats for q0. i'm not sure if this because there was no traffic yet 
(even though there is also no traffic on q0). i will set up a traffic 
gen and see what happens when the q1-q10 receives traffic.

One other comment below.

# /root/ovs/utilities/ovs-appctl dpif-netdev/pmd-rxq-show
pmd thread numa_id 0 core_id 8:
   isolated : false
   port: myport            queue-id:  0 (enabled)   pmd usage:  0 %
   port: myport            queue-id:  3 (enabled)   pmd usage:  0 %
   port: myport            queue-id:  4 (enabled)   pmd usage:  0 %
   port: myport            queue-id:  7 (enabled)   pmd usage:  0 %
   port: myport            queue-id:  8 (enabled)   pmd usage:  0 %
   port: urport            queue-id:  0 (enabled)   pmd usage:  0 %
   overhead:  0 %
pmd thread numa_id 0 core_id 10:
   isolated : false
   port: myport            queue-id:  1 (enabled)   pmd usage:  0 %
   port: myport            queue-id:  2 (enabled)   pmd usage:  0 %
   port: myport            queue-id:  5 (enabled)   pmd usage:  0 %
   port: myport            queue-id:  6 (enabled)   pmd usage:  0 %
   port: myport            queue-id:  9 (enabled)   pmd usage:  0 %
   port: myport            queue-id: 10 (enabled)   pmd usage:  0 %
   overhead:  0 %

# /root/ovs/utilities/ovs-vsctl get Interface myport statistics
{flow_director_filter_add_errors=0, 
flow_director_filter_remove_errors=0, mac_local_errors=0, 
mac_remote_errors=0, ovs_rx_qos_drops=0, ovs_tx_failure_drops=0, 
ovs_tx_invalid_hwol_drops=0, ovs_tx_mtu_exceeded_drops=0, 
ovs_tx_qos_drops=0, rx_128_to_255_packets=0, rx_1_to_64_packets=0, 
rx_256_to_511_packets=0, rx_512_to_1023_packets=0, 
rx_65_to_127_packets=0, rx_broadcast_packets=0, rx_bytes=0, 
rx_crc_errors=0, rx_dropped=0, rx_errors=0, rx_fcoe_crc_errors=0, 
rx_fcoe_dropped=0, rx_fcoe_mbuf_allocation_errors=0, 
rx_fragment_errors=0, rx_illegal_byte_errors=0, rx_jabber_errors=0, 
rx_length_errors=0, rx_mac_short_packet_dropped=0, 
rx_management_dropped=0, rx_management_packets=0, 
rx_mbuf_allocation_errors=0, rx_missed_errors=0, rx_oversize_errors=0, 
rx_packets=0, rx_priority0_dropped=0, 
rx_priority0_mbuf_allocation_errors=0, rx_priority1_dropped=0, 
rx_priority1_mbuf_allocation_errors=0, rx_priority2_dropped=0, 
rx_priority2_mbuf_allocation_errors=0, rx_priority3_dropped=0, 
rx_priority3_mbuf_allocation_errors=0, rx_priority4_dropped=0, 
rx_priority4_mbuf_allocation_errors=0, rx_priority5_dropped=0, 
rx_priority5_mbuf_allocation_errors=0, rx_priority6_dropped=0, 
rx_priority6_mbuf_allocation_errors=0, rx_priority7_dropped=0, 
rx_priority7_mbuf_allocation_errors=0, rx_q0_bytes=0, rx_q0_errors=0, 
rx_q0_packets=0, rx_undersize_errors=0, tx_128_to_255_packets=0, 
tx_1_to_64_packets=0, tx_256_to_511_packets=0, tx_512_to_1023_packets=0, 
tx_65_to_127_packets=0, tx_broadcast_packets=0, tx_bytes=0, 
tx_dropped=0, tx_errors=0, tx_management_packets=0, 
tx_multicast_packets=0, tx_packets=0, tx_q0_bytes=0, tx_q0_packets=0, 
tx_q1_bytes=0, tx_q1_packets=0, tx_q2_bytes=0, tx_q2_packets=0}


> ---
> Sending this as a RFC for feedback.
> vhost user ports are left untouched for now, but could be added in the
> future.
> 
> ---
>   lib/netdev-dpdk.c | 32 ++++++++++++++++++++++++++------
>   lib/util.c        | 13 -------------
>   lib/util.h        |  2 --
>   3 files changed, 26 insertions(+), 21 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index ca92c947a2..12447cc4b2 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -18,6 +18,7 @@
>   #include "netdev-dpdk.h"
>   
>   #include <errno.h>
> +#include <regex.h>
>   #include <signal.h>
>   #include <stdlib.h>
>   #include <string.h>
> @@ -1583,6 +1584,13 @@ netdev_dpdk_get_xstat_name(struct netdev_dpdk *dev, uint64_t id)
>       return dev->rte_xstats_names[id].name;
>   }
>   
> +/* We filter out everything except per rxq/txq basic stats, and dropped,
> + * error and management counters.
> + * Note: rx_qX_errors is handled by the _errors$ pattern.
> + */
> +#define DPDK_STATS_REGEX_FILTER \
> +    "(^(rx_q|tx_q)[0-9]*_(packets|bytes)$|_errors$|_dropped$|_management_)"
> +

iirc, there was some pmds that were not conforming correctly with this 
rx_q syntax, but I *think* they were fixed a few releases ago in DPDK. 
Just mentioning because if there was still some, they could be fixed or 
the regex could be expanded to capture them.

>   static bool
>   netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
>       OVS_REQUIRES(dev->mutex)
> @@ -1617,6 +1625,20 @@ netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
>                                               sizeof *dev->rte_xstats_names);
>   
>               if (dev->rte_xstats_names) {
> +                int err;
> +                regex_t r;
> +
> +                err = regcomp(&r, DPDK_STATS_REGEX_FILTER, REG_EXTENDED);
> +                if (err != 0) {
> +                    size_t errbuf_size = regerror(err, &r, NULL, 0);
> +                    char *errbuf = xcalloc(1, errbuf_size);
> +
> +                    regerror(err, &r, errbuf, errbuf_size);
> +                    VLOG_DBG("Could not setup stats regexp: %s", errbuf);
> +                    free(errbuf);
> +                    goto out;
> +                }
> +
>                   /* Retreive xstats names */
>                   rte_xstats_len =
>                           rte_eth_xstats_get_names(dev->port_id,
> @@ -1626,10 +1648,12 @@ netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
>                   if (rte_xstats_len < 0) {
>                       VLOG_WARN("Cannot get XSTATS names for port: "
>                                 DPDK_PORT_ID_FMT, dev->port_id);
> +                    regfree(&r);
>                       goto out;
>                   } else if (rte_xstats_len != dev->rte_xstats_names_size) {
>                       VLOG_WARN("XSTATS size doesn't match for port: "
>                                 DPDK_PORT_ID_FMT, dev->port_id);
> +                    regfree(&r);
>                       goto out;
>                   }
>   
> @@ -1648,12 +1672,7 @@ netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
>                       for (uint32_t i = 0; i < rte_xstats_len; i++) {
>                           id = rte_xstats[i].id;
>                           name = netdev_dpdk_get_xstat_name(dev, id);
> -                        /* We need to filter out everything except
> -                         * dropped, error and management counters */
> -                        if (string_ends_with(name, "_errors") ||
> -                            strstr(name, "_management_") ||
> -                            string_ends_with(name, "_dropped")) {
> -
> +                        if (regexec(&r, name, 0, NULL, 0) == 0) {
>                               dev->rte_xstats_ids[xstats_no] = id;
>                               xstats_no++;
>                           }
> @@ -1666,6 +1685,7 @@ netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
>                   }
>   
>                   free(rte_xstats);
> +                regfree(&r);
>               }
>           }
>       } else {
> diff --git a/lib/util.c b/lib/util.c
> index 1195c79821..34755a4dad 100644
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -405,19 +405,6 @@ ovs_strzcpy(char *dst, const char *src, size_t size)
>       }
>   }
>   
> -/*
> - * Returns true if 'str' ends with given 'suffix'.
> - */
> -int
> -string_ends_with(const char *str, const char *suffix)
> -{
> -    int str_len = strlen(str);
> -    int suffix_len = strlen(suffix);
> -
> -    return (str_len >= suffix_len) &&
> -           (0 == strcmp(str + (str_len - suffix_len), suffix));
> -}
> -
>   /* Prints 'format' on stderr, formatting it like printf() does.  If 'err_no' is
>    * nonzero, then it is formatted with ovs_retval_to_string() and appended to
>    * the message inside parentheses.  Then, terminates with abort().
> diff --git a/lib/util.h b/lib/util.h
> index aea19d45fa..4ad6e28274 100644
> --- a/lib/util.h
> +++ b/lib/util.h
> @@ -183,8 +183,6 @@ void free_cacheline(void *);
>   void ovs_strlcpy(char *dst, const char *src, size_t size);
>   void ovs_strzcpy(char *dst, const char *src, size_t size);
>   
> -int string_ends_with(const char *str, const char *suffix);
> -
>   void *xmalloc_pagealign(size_t) MALLOC_LIKE;
>   void free_pagealign(void *);
>   void *xmalloc_size_align(size_t, size_t) MALLOC_LIKE;
> 



More information about the dev mailing list