[PATCH 0/3] selftests: drv-net: Fix issues in devlink_rate_tc_bw.py

[PATCH 0/3] selftests: drv-net: Fix issues in devlink_rate_tc_bw.py

[Carolina Jubran]

This series fixes issues in devlink_rate_tc_bw.py selftest that made
its checks unreliable and its documentation inconsistent with the
actual configuration.

Thanks

Carolina Jubran (3):
  selftests: drv-net: Fix and clarify TC bandwidth split in
    devlink_rate_tc_bw.py
  selftests: drv-net: Fix tolerance calculation in devlink_rate_tc_bw.py
  selftests: drv-net: Relax total BW check in devlink_rate_tc_bw.py

 .../drivers/net/hw/devlink_rate_tc_bw.py      | 102 ++++++++----------
 1 file changed, 44 insertions(+), 58 deletions(-)

-- 
2.38.1

[PATCH 1/3] selftests: drv-net: Fix and clarify TC bandwidth split in devlink_rate_tc_bw.py

[Carolina Jubran]

Correct the documented bandwidth distribution between TC3 and TC4
from 80/20 to 20/80. Update test descriptions and printed messages
to consistently reflect the intended split.

Fixes: 23ca32e4ead4 ("selftests: drv-net: Add test for devlink-rate traffic class bandwidth distribution")
Tested-by: Carolina Jubran <cjubran@nvidia.com>
Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Nimrod Oren <noren@nvidia.com>
---
 .../drivers/net/hw/devlink_rate_tc_bw.py      | 26 +++++++++----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py b/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
index ead6784d1910..4da91e3292bf 100755
--- a/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
+++ b/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
@@ -21,21 +21,21 @@ Test Cases:
 ----------
 1. test_no_tc_mapping_bandwidth:
    - Verifies that without TC mapping, bandwidth is NOT distributed according to
-     the configured 80/20 split between TC4 and TC3
-   - This test should fail if bandwidth matches the 80/20 split without TC
+     the configured 20/80 split between TC3 and TC4
+   - This test should fail if bandwidth matches the 20/80 split without TC
      mapping
-   - Expected: Bandwidth should NOT be distributed as 80/20
+   - Expected: Bandwidth should NOT be distributed as 20/80
 
 2. test_tc_mapping_bandwidth:
    - Configures TC mapping using mqprio qdisc
    - Verifies that with TC mapping, bandwidth IS distributed according to the
-     configured 80/20 split between TC3 and TC4
-   - Expected: Bandwidth should be distributed as 80/20
+     configured 20/80 split between TC3 and TC4
+   - Expected: Bandwidth should be distributed as 20/80
 
 Bandwidth Distribution:
 ----------------------
-- TC3 (VLAN 101): Configured for 80% of total bandwidth
-- TC4 (VLAN 102): Configured for 20% of total bandwidth
+- TC3 (VLAN 101): Configured for 20% of total bandwidth
+- TC4 (VLAN 102): Configured for 80% of total bandwidth
 - Total bandwidth: 1Gbps
 - Tolerance: +-12%
 
@@ -413,10 +413,10 @@ def run_bandwidth_distribution_test(cfg, set_tc_mapping):
 
 def test_no_tc_mapping_bandwidth(cfg):
     """
-    Verifies that bandwidth is not split 80/20 without traffic class mapping.
+    Verifies that bandwidth is not split 20/80 without traffic class mapping.
     """
-    pass_bw_msg = "Bandwidth is NOT distributed as 80/20 without TC mapping"
-    fail_bw_msg = "Bandwidth matched 80/20 split without TC mapping"
+    pass_bw_msg = "Bandwidth is NOT distributed as 20/80 without TC mapping"
+    fail_bw_msg = "Bandwidth matched 20/80 split without TC mapping"
     is_mlx5 = "driver: mlx5" in ethtool(f"-i {cfg.ifname}").stdout
 
     if run_bandwidth_distribution_test(cfg, set_tc_mapping=False):
@@ -430,13 +430,13 @@ def test_no_tc_mapping_bandwidth(cfg):
 
 def test_tc_mapping_bandwidth(cfg):
     """
-    Verifies that bandwidth is correctly split 80/20 between TC3 and TC4
+    Verifies that bandwidth is correctly split 20/80 between TC3 and TC4
     when traffic class mapping is set.
     """
     if run_bandwidth_distribution_test(cfg, set_tc_mapping=True):
-        ksft_pr("Bandwidth is distributed as 80/20 with TC mapping")
+        ksft_pr("Bandwidth is distributed as 20/80 with TC mapping")
     else:
-        raise KsftFailEx("Bandwidth did not match 80/20 split with TC mapping")
+        raise KsftFailEx("Bandwidth did not match 20/80 split with TC mapping")
 
 
 def main() -> None:
-- 
2.38.1

Re: [PATCH 1/3] selftests: drv-net: Fix and clarify TC bandwidth split in devlink_rate_tc_bw.py

[Simon Horman]

On Sun, Aug 31, 2025 at 11:06:39AM +0300, Carolina Jubran wrote:
> Correct the documented bandwidth distribution between TC3 and TC4
> from 80/20 to 20/80. Update test descriptions and printed messages
> to consistently reflect the intended split.
> 
> Fixes: 23ca32e4ead4 ("selftests: drv-net: Add test for devlink-rate traffic class bandwidth distribution")
> Tested-by: Carolina Jubran <cjubran@nvidia.com>
> Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
> Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
> Reviewed-by: Nimrod Oren <noren@nvidia.com>

Reviewed-by: Simon Horman <horms@kernel.org>

[PATCH 2/3] selftests: drv-net: Fix tolerance calculation in devlink_rate_tc_bw.py

[Carolina Jubran]

Currently, tolerance is computed against the TC’s expected percentage,
making TC3 (20%) validation overly strict and TC4 (80%) overly loose.

Update BandwidthValidator to take a dict of shares and compute bounds
relative to the overall total, so that all shares are validated
consistently.

Fixes: 23ca32e4ead4 ("selftests: drv-net: Add test for devlink-rate traffic class bandwidth distribution")
Tested-by: Carolina Jubran <cjubran@nvidia.com>
Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Nimrod Oren <noren@nvidia.com>
---
 .../drivers/net/hw/devlink_rate_tc_bw.py      | 74 ++++++++-----------
 1 file changed, 30 insertions(+), 44 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py b/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
index 4da91e3292bf..abc20bc4a34a 100755
--- a/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
+++ b/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
@@ -68,39 +68,35 @@ from lib.py import cmd, defer, ethtool, ip
 
 class BandwidthValidator:
     """
-    Validates bandwidth totals and per-TC shares against expected values
-    with a tolerance.
+    Validates total bandwidth and individual shares with tolerance
+    relative to the overall total.
     """
 
-    def __init__(self):
+    def __init__(self, shares):
         self.tolerance_percent = 12
-        self.expected_total_gbps = 1.0
-        self.total_min_expected = self.min_expected(self.expected_total_gbps)
-        self.total_max_expected = self.max_expected(self.expected_total_gbps)
-        self.tc_expected_percent = {
-            3: 20.0,
-            4: 80.0,
-        }
+        self.expected_total = sum(shares.values())
+        self.bounds = {}
+
+        for name, exp in shares.items():
+            self.bounds[name] = (self.min_expected(exp), self.max_expected(exp))
 
     def min_expected(self, value):
         """Calculates the minimum acceptable value based on tolerance."""
-        return value - (value * self.tolerance_percent / 100)
+        return value - (self.expected_total * self.tolerance_percent / 100)
 
     def max_expected(self, value):
         """Calculates the maximum acceptable value based on tolerance."""
-        return value + (value * self.tolerance_percent / 100)
-
-    def bound(self, expected, value):
-        """Returns True if value is within expected tolerance."""
-        return self.min_expected(expected) <= value <= self.max_expected(expected)
+        return value + (self.expected_total * self.tolerance_percent / 100)
 
-    def tc_bandwidth_bound(self, value, tc_ix):
+    def bound(self, values):
         """
-        Returns True if the given bandwidth value is within tolerance
-        for the TC's expected bandwidth.
+        Return True if all given values fall within tolerance.
         """
-        expected = self.tc_expected_percent[tc_ix]
-        return self.bound(expected, value)
+        for name, value in values.items():
+            low, high = self.bounds[name]
+            if not low <= value <= high:
+                return False
+        return True
 
 
 def setup_vf(cfg, set_tc_mapping=True):
@@ -364,38 +360,26 @@ def verify_total_bandwidth(bw_data, validator):
     """
     total = bw_data['total_bw']
 
-    if validator.bound(validator.expected_total_gbps, total):
+    if validator.bound({"total": total}):
         return
 
-    if total < validator.total_min_expected:
+    low, high = validator.bounds["total"]
+
+    if total < low:
         raise KsftSkipEx(
             f"Total bandwidth {total:.2f} Gbps < minimum "
-            f"{validator.total_min_expected:.2f} Gbps; "
-            f"parent tx_max ({validator.expected_total_gbps:.1f} G) "
+            f"{low:.2f} Gbps; "
+            f"parent tx_max ({validator.expected_total:.1f} G) "
             f"not reached, cannot validate share"
         )
 
     raise KsftFailEx(
         f"Total bandwidth {total:.2f} Gbps exceeds allowed ceiling "
-        f"{validator.total_max_expected:.2f} Gbps "
-        f"(VF tx_max set to {validator.expected_total_gbps:.1f} G)"
+        f"{high:.2f} Gbps "
+        f"(VF tx_max set to {validator.expected_total:.1f} G)"
     )
 
 
-def check_bandwidth_distribution(bw_data, validator):
-    """
-    Checks whether the measured TC3 and TC4 bandwidth percentages
-    fall within their expected tolerance ranges.
-
-    Returns:
-        bool: True if both TC3 and TC4 percentages are within bounds.
-    """
-    tc3_valid = validator.tc_bandwidth_bound(bw_data['tc3_percentage'], 3)
-    tc4_valid = validator.tc_bandwidth_bound(bw_data['tc4_percentage'], 4)
-
-    return tc3_valid and tc4_valid
-
-
 def run_bandwidth_distribution_test(cfg, set_tc_mapping):
     """
     Runs parallel iperf3 tests for both TCs and collects results.
@@ -406,9 +390,10 @@ def run_bandwidth_distribution_test(cfg, set_tc_mapping):
     test_name = "with TC mapping" if set_tc_mapping else "without TC mapping"
     print_bandwidth_results(bw_data, test_name)
 
-    verify_total_bandwidth(bw_data, cfg.bw_validator)
+    verify_total_bandwidth(bw_data, cfg.traffic_bw_validator)
 
-    return check_bandwidth_distribution(bw_data, cfg.bw_validator)
+    return cfg.tc_bw_validator.bound({"tc3": bw_data['tc3_percentage'],
+                                     "tc4": bw_data['tc4_percentage']})
 
 
 def test_no_tc_mapping_bandwidth(cfg):
@@ -453,7 +438,8 @@ def main() -> None:
             raise KsftSkipEx("Could not get PCI address of the interface")
         cfg.require_cmd("iperf3", local=True, remote=True)
 
-        cfg.bw_validator = BandwidthValidator()
+        cfg.traffic_bw_validator = BandwidthValidator({"total": 1})
+        cfg.tc_bw_validator = BandwidthValidator({"tc3": 20, "tc4": 80})
 
         cases = [test_no_tc_mapping_bandwidth, test_tc_mapping_bandwidth]
 
-- 
2.38.1

Re: [PATCH 2/3] selftests: drv-net: Fix tolerance calculation in devlink_rate_tc_bw.py

[Simon Horman]

On Sun, Aug 31, 2025 at 11:06:40AM +0300, Carolina Jubran wrote:
> Currently, tolerance is computed against the TC’s expected percentage,
> making TC3 (20%) validation overly strict and TC4 (80%) overly loose.
> 
> Update BandwidthValidator to take a dict of shares and compute bounds
> relative to the overall total, so that all shares are validated
> consistently.
> 
> Fixes: 23ca32e4ead4 ("selftests: drv-net: Add test for devlink-rate traffic class bandwidth distribution")
> Tested-by: Carolina Jubran <cjubran@nvidia.com>
> Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
> Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
> Reviewed-by: Nimrod Oren <noren@nvidia.com>

Reviewed-by: Simon Horman <horms@kernel.org>

[PATCH 3/3] selftests: drv-net: Relax total BW check in devlink_rate_tc_bw.py

[Carolina Jubran]

This test is meant to check TC bandwidth distribution, not the tx_max
of the VF. The total bandwidth check is only there to make sure that FW
tokens limit traffic, because the per-TC share is only meaningful when
the link is fully saturated.

Because the measured total is the sum of two iperf3 streams that do not
always start or stop at the same time, using a strict 1 Gbps target
caused random failures. This change adds a tolerance parameter to
BandwidthValidator, keeps per-TC checks tight at +-12%, and relaxes the
total bandwidth check to +-25% around 1 Gbps.

This avoids false failures while still confirming that the TC share
validation is meaningful.

Fixes: 23ca32e4ead4 ("selftests: drv-net: Add test for devlink-rate traffic class bandwidth distribution")
Tested-by: Carolina Jubran <cjubran@nvidia.com>
Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Nimrod Oren <noren@nvidia.com>
---
 .../selftests/drivers/net/hw/devlink_rate_tc_bw.py        | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py b/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
index abc20bc4a34a..1713ca11f845 100755
--- a/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
+++ b/tools/testing/selftests/drivers/net/hw/devlink_rate_tc_bw.py
@@ -72,8 +72,8 @@ class BandwidthValidator:
     relative to the overall total.
     """
 
-    def __init__(self, shares):
-        self.tolerance_percent = 12
+    def __init__(self, shares, tolerance):
+        self.tolerance_percent = tolerance
         self.expected_total = sum(shares.values())
         self.bounds = {}
 
@@ -438,8 +438,8 @@ def main() -> None:
             raise KsftSkipEx("Could not get PCI address of the interface")
         cfg.require_cmd("iperf3", local=True, remote=True)
 
-        cfg.traffic_bw_validator = BandwidthValidator({"total": 1})
-        cfg.tc_bw_validator = BandwidthValidator({"tc3": 20, "tc4": 80})
+        cfg.traffic_bw_validator = BandwidthValidator({"total": 1}, 25)
+        cfg.tc_bw_validator = BandwidthValidator({"tc3": 20, "tc4": 80}, 12)
 
         cases = [test_no_tc_mapping_bandwidth, test_tc_mapping_bandwidth]
 
-- 
2.38.1

Re: [PATCH 3/3] selftests: drv-net: Relax total BW check in devlink_rate_tc_bw.py

[Simon Horman]

On Sun, Aug 31, 2025 at 11:06:41AM +0300, Carolina Jubran wrote:
> This test is meant to check TC bandwidth distribution, not the tx_max
> of the VF. The total bandwidth check is only there to make sure that FW
> tokens limit traffic, because the per-TC share is only meaningful when
> the link is fully saturated.
> 
> Because the measured total is the sum of two iperf3 streams that do not
> always start or stop at the same time, using a strict 1 Gbps target
> caused random failures. This change adds a tolerance parameter to
> BandwidthValidator, keeps per-TC checks tight at +-12%, and relaxes the
> total bandwidth check to +-25% around 1 Gbps.
> 
> This avoids false failures while still confirming that the TC share
> validation is meaningful.
> 
> Fixes: 23ca32e4ead4 ("selftests: drv-net: Add test for devlink-rate traffic class bandwidth distribution")
> Tested-by: Carolina Jubran <cjubran@nvidia.com>
> Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
> Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
> Reviewed-by: Nimrod Oren <noren@nvidia.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH 3/3] selftests: drv-net: Relax total BW check in devlink_rate_tc_bw.py

[Jakub Kicinski]

On Sun, 31 Aug 2025 11:06:41 +0300 Carolina Jubran wrote:
> Because the measured total is the sum of two iperf3 streams that do not
> always start or stop at the same time

That's solvable, tho? iperf3 has --json support, it will give you 
the b/w readings in the configured intervals (1sec by default).
With the interval based samples at hand you should be able to select
only the period in which b/w is stable ("middle" of the test).

While at it it may make sense to switch to lib/py/load.py wrappers
rather than threading the python locally in the test.

Re: [PATCH 3/3] selftests: drv-net: Relax total BW check in devlink_rate_tc_bw.py

[Jakub Kicinski]

On Tue, 2 Sep 2025 16:21:01 -0700 Jakub Kicinski wrote:
> On Sun, 31 Aug 2025 11:06:41 +0300 Carolina Jubran wrote:
> > Because the measured total is the sum of two iperf3 streams that do not
> > always start or stop at the same time  
> 
> That's solvable, tho? iperf3 has --json support, it will give you 
> the b/w readings in the configured intervals (1sec by default).
> With the interval based samples at hand you should be able to select
> only the period in which b/w is stable ("middle" of the test).
> 
> While at it it may make sense to switch to lib/py/load.py wrappers
> rather than threading the python locally in the test.

Hi Carolina! I think you replied to me but the reply never reached 
the list, I purged it from my inbox before realizing. 
I think you said that the direction of the flows is wrong for load.py.
Perhaps adding a reverse= attr which will translate the --reverse in
the client process would do?

Re: [PATCH 3/3] selftests: drv-net: Relax total BW check in devlink_rate_tc_bw.py

[Carolina Jubran]

On 06/09/2025 1:32, Jakub Kicinski wrote:
> On Tue, 2 Sep 2025 16:21:01 -0700 Jakub Kicinski wrote:
>> On Sun, 31 Aug 2025 11:06:41 +0300 Carolina Jubran wrote:
>>> Because the measured total is the sum of two iperf3 streams that do not
>>> always start or stop at the same time
>> That's solvable, tho? iperf3 has --json support, it will give you
>> the b/w readings in the configured intervals (1sec by default).
>> With the interval based samples at hand you should be able to select
>> only the period in which b/w is stable ("middle" of the test).
>>
>> While at it it may make sense to switch to lib/py/load.py wrappers
>> rather than threading the python locally in the test.
> Hi Carolina! I think you replied to me but the reply never reached
> the list, I purged it from my inbox before realizing.
:O Sorry about that
> I think you said that the direction of the flows is wrong for load.py.
Yes, that’s exactly what I said.
> Perhaps adding a reverse= attr which will translate the --reverse in
> the client process would do?

However, I’ll also need to extend load.py:

1. Binding support to ensure traffic flows through the specific VLAN
     interface.
2. Interval-based measurement for iperf3 --json to analyze only the
     stable period.

So my plan is:

1. Send v2 for net to fix the current test with interval-based
     measurement.
2. Follow up with a patch to extend load.py with reverse/binding/interval
     support and then migrate the test to use it.

Does that sound good to you?

Thanks again for the suggestion :)

Re: [PATCH 3/3] selftests: drv-net: Relax total BW check in devlink_rate_tc_bw.py

[Jakub Kicinski]

On Mon, 8 Sep 2025 22:16:29 +0300 Carolina Jubran wrote:
> However, I’ll also need to extend load.py:
> 
> 1. Binding support to ensure traffic flows through the specific VLAN
>      interface.
> 2. Interval-based measurement for iperf3 --json to analyze only the
>      stable period.
> 
> So my plan is:
> 
> 1. Send v2 for net to fix the current test with interval-based
>      measurement.
> 2. Follow up with a patch to extend load.py with reverse/binding/interval
>      support and then migrate the test to use it.
> 
> Does that sound good to you?

Sounds too complicated, this is just a stability improvement for a test
which works on single device, and is not exercised / reported upstream.
Let's jump straight to step 2.

Re: [PATCH 3/3] selftests: drv-net: Relax total BW check in devlink_rate_tc_bw.py

[Carolina Jubran]

On 08/09/2025 23:19, Jakub Kicinski wrote:
> On Mon, 8 Sep 2025 22:16:29 +0300 Carolina Jubran wrote:
>> However, I’ll also need to extend load.py:
>>
>> 1. Binding support to ensure traffic flows through the specific VLAN
>>       interface.
>> 2. Interval-based measurement for iperf3 --json to analyze only the
>>       stable period.
>>
>> So my plan is:
>>
>> 1. Send v2 for net to fix the current test with interval-based
>>       measurement.
>> 2. Follow up with a patch to extend load.py with reverse/binding/interval
>>       support and then migrate the test to use it.
>>
>> Does that sound good to you?
> Sounds too complicated, this is just a stability improvement for a test
> which works on single device, and is not exercised / reported upstream.
> Let's jump straight to step 2.


Ack, I’ll drop this patch from the series and handle the migration to
load.py and reliability improvements in a follow-up.

Re: [PATCH 3/3] selftests: drv-net: Relax total BW check in devlink_rate_tc_bw.py

[Jakub Kicinski]

On Tue, 9 Sep 2025 13:06:22 +0300 Carolina Jubran wrote:
> > Sounds too complicated, this is just a stability improvement for a test
> > which works on single device, and is not exercised / reported upstream.
> > Let's jump straight to step 2.  
> 
> Ack, I’ll drop this patch from the series and handle the migration to
> load.py and reliability improvements in a follow-up.

I think you misunderstood me, let's redo the whole series against
net-next. No need for Fixes tags.