diff mbox series

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

Message ID 20211202211616.10726-2-david.marchand@redhat.com
State Accepted
Headers show
Series [ovs-dev,1/2] netdev-dpdk: Fix statistics when changing Rx/Tx queues count. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

David Marchand Dec. 2, 2021, 9:16 p.m. UTC
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).
OVS only filters DPDK statistics, there is nothing to request in DPDK API.
So the only change is to extend the filter on xstats.

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@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
Changes since RFC:
- dropped regex and used simpler string manipulations,

---
 lib/netdev-dpdk.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Comments

Kevin Traynor Dec. 14, 2021, 1:33 p.m. UTC | #1
On 02/12/2021 21:16, 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).
> OVS only filters DPDK statistics, there is nothing to request in DPDK API.
> So the only change is to extend the filter on xstats.
> 
> 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@redhat.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Tested and working when increasing/decreasing rx/tx queues.

'rx_q15_bytes=0, rx_q15_errors=0, rx_q15_packets=0'

Acked-by: Kevin Traynor <ktraynor@redhat.com>

> ---
> Changes since RFC:
> - dropped regex and used simpler string manipulations,
> 
> ---
>   lib/netdev-dpdk.c | 17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 51bb41551b..1e6079d544 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1582,6 +1582,16 @@ netdev_dpdk_get_xstat_name(struct netdev_dpdk *dev, uint64_t id)
>       return dev->rte_xstats_names[id].name;
>   }
>   
> +static bool
> +is_queue_stat(const char *s)
> +{
> +    uint16_t tmp;
> +
> +    return (s[0] == 'r' || s[0] == 't') &&
> +            (ovs_scan(s + 1, "x_q%"SCNu16"_packets", &tmp) ||
> +             ovs_scan(s + 1, "x_q%"SCNu16"_bytes", &tmp));
> +}
> +
>   static void
>   netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
>       OVS_REQUIRES(dev->mutex)
> @@ -1632,9 +1642,10 @@ netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
>           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") ||
> +        /* For custom stats, we filter out everything except per rxq/txq basic
> +         * stats, and dropped, error and management counters. */
> +        if (is_queue_stat(name) ||
> +            string_ends_with(name, "_errors") ||
>               strstr(name, "_management_") ||
>               string_ends_with(name, "_dropped")) {
>   
>
Ilya Maximets Jan. 12, 2022, 12:08 p.m. UTC | #2
On 12/14/21 14:33, Kevin Traynor wrote:
> On 02/12/2021 21:16, 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).
>> OVS only filters DPDK statistics, there is nothing to request in DPDK API.
>> So the only change is to extend the filter on xstats.
>>
>> 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@redhat.com>
>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> Tested and working when increasing/decreasing rx/tx queues.
> 
> 'rx_q15_bytes=0, rx_q15_errors=0, rx_q15_packets=0'
> 
> Acked-by: Kevin Traynor <ktraynor@redhat.com>

Thanks, David, Maxime and Kevin!  I applied the series.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 51bb41551b..1e6079d544 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1582,6 +1582,16 @@  netdev_dpdk_get_xstat_name(struct netdev_dpdk *dev, uint64_t id)
     return dev->rte_xstats_names[id].name;
 }
 
+static bool
+is_queue_stat(const char *s)
+{
+    uint16_t tmp;
+
+    return (s[0] == 'r' || s[0] == 't') &&
+            (ovs_scan(s + 1, "x_q%"SCNu16"_packets", &tmp) ||
+             ovs_scan(s + 1, "x_q%"SCNu16"_bytes", &tmp));
+}
+
 static void
 netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
     OVS_REQUIRES(dev->mutex)
@@ -1632,9 +1642,10 @@  netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
         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") ||
+        /* For custom stats, we filter out everything except per rxq/txq basic
+         * stats, and dropped, error and management counters. */
+        if (is_queue_stat(name) ||
+            string_ends_with(name, "_errors") ||
             strstr(name, "_management_") ||
             string_ends_with(name, "_dropped")) {