Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v2] selftests: drv-net: so_txtime: only send test traffic to sch_etf
@ 2026-08-08 16:00 Willem de Bruijn
  2026-08-11  1:20 ` patchwork-bot+netdevbpf
  2026-08-11  1:31 ` Jakub Kicinski
  0 siblings, 2 replies; 6+ messages in thread
From: Willem de Bruijn @ 2026-08-08 16:00 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, edumazet, pabeni, horms, andrew, Willem de Bruijn

From: Willem de Bruijn <willemb@google.com>

The ETF qdiscs drops traffic without a socket or txtime. Even with
parameter skip_sock_check regular traffic is affected by ETF.

This test ran fine when run manually in a pure software environment.
But with drv-net across two hosts tests fail as early as when calling
cfg.remote.deploy due to effectively losing connectivity.

Isolate the intended test traffic:
- mark that with SO_MARK 100
- install a regular permissive root prio qdisc for background traffic
- install the ETF qdisc as leaf
- install a filter that only directs SO_MARK 100 traffic to this leaf

Technically other high prio traffic will map onto this leaf based on
ToS band mapping too. But that is immaterial in practice.

Fixes: 5c6baef3885c ("selftests: drv-net: convert so_txtime to drv-net")
Signed-off-by: Willem de Bruijn <willemb@google.com>

---

Changes
  v1 - > v2
    - redirect to net-next
      (address new conflict on ../selftests/drivers/net/config)
  v1: https://lore.kernel.org/netdev/20260806173702.3857148-1-willemdebruijn.kernel@gmail.com/
---
 tools/testing/selftests/drivers/net/config       |  2 ++
 tools/testing/selftests/drivers/net/so_txtime.py | 16 +++++++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/config b/tools/testing/selftests/drivers/net/config
index f3933cf3e6be..b6989c7d3d9d 100644
--- a/tools/testing/selftests/drivers/net/config
+++ b/tools/testing/selftests/drivers/net/config
@@ -8,6 +8,7 @@ CONFIG_NET_ACT_SKBEDIT=m
 CONFIG_NET_CLS_ACT=y
 CONFIG_NET_CLS_BPF=y
 CONFIG_NET_CLS_FLOWER=m
+CONFIG_NET_CLS_FW=m
 CONFIG_NET_CLS_MATCHALL=m
 CONFIG_NETCONSOLE=m
 CONFIG_NETCONSOLE_DYNAMIC=y
@@ -17,6 +18,7 @@ CONFIG_NETKIT=y
 CONFIG_NET_SCH_ETF=m
 CONFIG_NET_SCH_FQ=m
 CONFIG_NET_SCH_INGRESS=y
+CONFIG_NET_SCH_PRIO=m
 CONFIG_PPP=y
 CONFIG_PPPOE=y
 CONFIG_VLAN_8021Q=m
diff --git a/tools/testing/selftests/drivers/net/so_txtime.py b/tools/testing/selftests/drivers/net/so_txtime.py
index adf6c848d6d8..9fbc0278d28b 100755
--- a/tools/testing/selftests/drivers/net/so_txtime.py
+++ b/tools/testing/selftests/drivers/net/so_txtime.py
@@ -27,7 +27,7 @@ def test_so_txtime(cfg, clockid, ipver, args_tx, args_rx, expect_success):
     cmd_addr = f"-S {cfg.addr_v[ipver]} -D {cfg.remote_addr_v[ipver]}"
     cmd_args = f"-{ipver} -c {clockid} -t {tstart} {cmd_addr}"
     cmd_rx = f"{cfg.bin_remote} {cmd_args} {args_rx} -r"
-    cmd_tx = f"{cfg.bin_local} {cmd_args} {args_tx}"
+    cmd_tx = f"{cfg.bin_local} -m 100 {cmd_args} {args_tx}"
 
     expect_fail = not expect_success
     if slow_machine:
@@ -45,7 +45,7 @@ def _qdisc_setup(ifname, qdisc, optargs=""):
     """
     orig = tc(f"qdisc show dev {ifname} root", json=True)[0].get("kind", None)
     defer(tc, f"qdisc replace dev {ifname} root {orig}")
-    tc(f"qdisc replace dev {ifname} root {qdisc} {optargs}")
+    tc(f"qdisc replace dev {ifname} root handle 1: {qdisc} {optargs}")
 
 
 def _test_variants_fq():
@@ -96,11 +96,21 @@ def _test_variants_etf():
 def test_so_txtime_etf(cfg, ipver, args_tx, args_rx, expect_fail):
     """Run all variants of etf tests."""
     cfg.require_ipver(ipver)
+
+    # root qdisc for background traffic (e.g., bkg())
+    _qdisc_setup(cfg.ifname, "prio")
+
+    # leaf ETF qdisc only for intended packets
     try:
-        _qdisc_setup(cfg.ifname, "etf", "clockid CLOCK_TAI delta 400000")
+        etf_args = "clockid CLOCK_TAI delta 400000"
+        tc(f"qdisc add dev {cfg.ifname} parent 1:1 handle 10: etf {etf_args}")
     except Exception as e:
         raise KsftSkipEx("tc does not support qdisc etf. skipping") from e
 
+    # redirect mark 100 to leaf
+    filter_args = "protocol all handle 100 fw flowid 1:1"
+    tc(f"filter add dev {cfg.ifname} parent 1: {filter_args}")
+
     test_so_txtime(cfg, "tai", ipver, args_tx, args_rx, expect_fail)
 
 
-- 
2.55.0.679.g6767b8d81c-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v2] selftests: drv-net: so_txtime: only send test traffic to sch_etf
  2026-08-08 16:00 [PATCH net-next v2] selftests: drv-net: so_txtime: only send test traffic to sch_etf Willem de Bruijn
@ 2026-08-11  1:20 ` patchwork-bot+netdevbpf
  2026-08-11  1:31 ` Jakub Kicinski
  1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-08-11  1:20 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: netdev, davem, kuba, edumazet, pabeni, horms, andrew, willemb

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sat,  8 Aug 2026 12:00:44 -0400 you wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> The ETF qdiscs drops traffic without a socket or txtime. Even with
> parameter skip_sock_check regular traffic is affected by ETF.
> 
> This test ran fine when run manually in a pure software environment.
> But with drv-net across two hosts tests fail as early as when calling
> cfg.remote.deploy due to effectively losing connectivity.
> 
> [...]

Here is the summary with links:
  - [net-next,v2] selftests: drv-net: so_txtime: only send test traffic to sch_etf
    https://git.kernel.org/netdev/net-next/c/ef3d6cca02c8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v2] selftests: drv-net: so_txtime: only send test traffic to sch_etf
  2026-08-08 16:00 [PATCH net-next v2] selftests: drv-net: so_txtime: only send test traffic to sch_etf Willem de Bruijn
  2026-08-11  1:20 ` patchwork-bot+netdevbpf
@ 2026-08-11  1:31 ` Jakub Kicinski
  2026-08-11 14:29   ` Willem de Bruijn
  1 sibling, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2026-08-11  1:31 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: netdev, davem, edumazet, pabeni, horms, andrew, Willem de Bruijn

On Sat,  8 Aug 2026 12:00:44 -0400 Willem de Bruijn wrote:
>      orig = tc(f"qdisc show dev {ifname} root", json=True)[0].get("kind", None)
>      defer(tc, f"qdisc replace dev {ifname} root {orig}")
> -    tc(f"qdisc replace dev {ifname} root {qdisc} {optargs}")
> +    tc(f"qdisc replace dev {ifname} root handle 1: {qdisc} {optargs}")

Ugh, sorry, I should have checked before applying.
Looks like this regresses the test on all setup in NIPA.

Random example:
https://netdev.bots.linux.dev/logview.html?f=/logs/hwksft/X710/results/772526/test-outputs/11-so-txtime-py/stdout

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v2] selftests: drv-net: so_txtime: only send test traffic to sch_etf
  2026-08-11  1:31 ` Jakub Kicinski
@ 2026-08-11 14:29   ` Willem de Bruijn
  2026-08-11 15:27     ` Jakub Kicinski
  0 siblings, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2026-08-11 14:29 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, davem, edumazet, pabeni, horms, andrew, Willem de Bruijn

On Mon, Aug 10, 2026 at 9:31 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sat,  8 Aug 2026 12:00:44 -0400 Willem de Bruijn wrote:
> >      orig = tc(f"qdisc show dev {ifname} root", json=True)[0].get("kind", None)
> >      defer(tc, f"qdisc replace dev {ifname} root {orig}")
> > -    tc(f"qdisc replace dev {ifname} root {qdisc} {optargs}")
> > +    tc(f"qdisc replace dev {ifname} root handle 1: {qdisc} {optargs}")
>
> Ugh, sorry, I should have checked before applying.
> Looks like this regresses the test on all setup in NIPA.
>
> Random example:
> https://netdev.bots.linux.dev/logview.html?f=/logs/hwksft/X710/results/772526/test-outputs/11-so-txtime-py/stdout

Ugh apologies. This did not trigger for me on my debian trixie vm (nor
netdevsim). Where the mq root qdisc handle at boot is not 1:

And I was entirely unfamiliar with the subtle difference in tc qdisc
replace behavior when f"handle {handle}" is added.

Apparently

1. no handle -> can replace with a qdisc of a different kind
2. handle && !match -> same
3. handle && match -> expect modify in place, so kind must be same.

which makes sense.

I can reproduce after

tc qdisc replace dev eth0 root handle pfifo_fast
tc qdisc replace dev eth0 root handle 1: pfifo_fast

It is not possible to replace tc qdisc replace with a del + add pair,
due to "Error: Cannot delete qdisc with handle of zero." on systems
with the default 0: qdisc, like my debian trixie.

The simplest change, then, is to replace the qdisc twice, first for
the kind, then for the handle.

@@ -45,17 +45,18 @@ def _qdisc_setup(ifname, qdisc, optargs=""):
     """
     orig = tc(f"qdisc show dev {ifname} root", json=True)[0].get("kind", None)
     defer(tc, f"qdisc replace dev {ifname} root {orig}")
+    tc(f"qdisc replace dev {ifname} root {qdisc} {optargs}")
     tc(f"qdisc replace dev {ifname} root handle 1: {qdisc} {optargs}")

It's an extra replace, so a bit ugly. But avoiding that requires
parsing the json for existing handle, branching on that.

I'm testing the above some more, will send that if no concerns.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v2] selftests: drv-net: so_txtime: only send test traffic to sch_etf
  2026-08-11 14:29   ` Willem de Bruijn
@ 2026-08-11 15:27     ` Jakub Kicinski
  2026-08-11 18:36       ` Willem de Bruijn
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2026-08-11 15:27 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: netdev, davem, edumazet, pabeni, horms, andrew, Willem de Bruijn

On Tue, 11 Aug 2026 10:29:12 -0400 Willem de Bruijn wrote:
> I can reproduce after
> 
> tc qdisc replace dev eth0 root handle pfifo_fast
> tc qdisc replace dev eth0 root handle 1: pfifo_fast
> 
> It is not possible to replace tc qdisc replace with a del + add pair,
> due to "Error: Cannot delete qdisc with handle of zero." on systems
> with the default 0: qdisc, like my debian trixie.
> 
> The simplest change, then, is to replace the qdisc twice, first for
> the kind, then for the handle.
> 
> @@ -45,17 +45,18 @@ def _qdisc_setup(ifname, qdisc, optargs=""):
>      """
>      orig = tc(f"qdisc show dev {ifname} root", json=True)[0].get("kind", None)
>      defer(tc, f"qdisc replace dev {ifname} root {orig}")
> +    tc(f"qdisc replace dev {ifname} root {qdisc} {optargs}")
>      tc(f"qdisc replace dev {ifname} root handle 1: {qdisc} {optargs}")
> 
> It's an extra replace, so a bit ugly. But avoiding that requires
> parsing the json for existing handle, branching on that.
> 
> I'm testing the above some more, will send that if no concerns.

If we are adding an extra command adding del would seem more intuitive?
But no preference.

The issue may be because the ring reconfig test now also sets up qdiscs.
And we don't have a proper way to restore previous qdisc config in the
tests. If another test needs to change qdiscs we should probably add
better infra, but for now I think the simple workaround you suggest is
best.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next v2] selftests: drv-net: so_txtime: only send test traffic to sch_etf
  2026-08-11 15:27     ` Jakub Kicinski
@ 2026-08-11 18:36       ` Willem de Bruijn
  0 siblings, 0 replies; 6+ messages in thread
From: Willem de Bruijn @ 2026-08-11 18:36 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, davem, edumazet, pabeni, horms, andrew, Willem de Bruijn

On Tue, Aug 11, 2026 at 11:27 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 11 Aug 2026 10:29:12 -0400 Willem de Bruijn wrote:
> > I can reproduce after
> >
> > tc qdisc replace dev eth0 root handle pfifo_fast
> > tc qdisc replace dev eth0 root handle 1: pfifo_fast
> >
> > It is not possible to replace tc qdisc replace with a del + add pair,
> > due to "Error: Cannot delete qdisc with handle of zero." on systems
> > with the default 0: qdisc, like my debian trixie.
> >
> > The simplest change, then, is to replace the qdisc twice, first for
> > the kind, then for the handle.
> >
> > @@ -45,17 +45,18 @@ def _qdisc_setup(ifname, qdisc, optargs=""):
> >      """
> >      orig = tc(f"qdisc show dev {ifname} root", json=True)[0].get("kind", None)
> >      defer(tc, f"qdisc replace dev {ifname} root {orig}")
> > +    tc(f"qdisc replace dev {ifname} root {qdisc} {optargs}")
> >      tc(f"qdisc replace dev {ifname} root handle 1: {qdisc} {optargs}")
> >
> > It's an extra replace, so a bit ugly. But avoiding that requires
> > parsing the json for existing handle, branching on that.
> >
> > I'm testing the above some more, will send that if no concerns.
>
> If we are adding an extra command adding del would seem more intuitive?
> But no preference.

Agreed. It's a slightly bigger patch, because it has to be wrapped in a try
block. But its purpose is more obvious.

https://lore.kernel.org/netdev/20260811182856.2702163-1-willemdebruijn.kernel@gmail.com/T/#u

> The issue may be because the ring reconfig test now also sets up qdiscs.
> And we don't have a proper way to restore previous qdisc config in the
> tests. If another test needs to change qdiscs we should probably add
> better infra, but for now I think the simple workaround you suggest is
> best.

Interesting, and that also has special logic to work around default handle 0:

"
    elif root is None or root["kind"] in ("mq", "noqueue"):
        # The auto-attached root mq has handle 0: on any device (real or sim),
        # which the kernel rejects as a qdisc parent. A 0: handle means the mq
        # is the untouched kernel default - no custom child qdiscs can hang off
        # an unaddressable parent - so installing a real handle and restoring
        # the default mq on exit preserves the device's effective policy.
        mq_handle = "1:"
"

Again, sorry for needing an extra fix to what was already a fix :/ We hit a
corner case that even with my testing I just did not reach.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-11 18:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-08 16:00 [PATCH net-next v2] selftests: drv-net: so_txtime: only send test traffic to sch_etf Willem de Bruijn
2026-08-11  1:20 ` patchwork-bot+netdevbpf
2026-08-11  1:31 ` Jakub Kicinski
2026-08-11 14:29   ` Willem de Bruijn
2026-08-11 15:27     ` Jakub Kicinski
2026-08-11 18:36       ` Willem de Bruijn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox