Re: [PATCH net-next v3] selftests/net: convert so_txtime to drv-net

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Willem de Bruijn wrote:
> Jakub Kicinski wrote:
> > On Sun,  5 Apr 2026 22:49:22 -0400 Willem de Bruijn wrote:
> > > +@ksft_variants(_test_variants_mono())
> > > +def test_so_txtime_mono(cfg, ipver, args_tx, args_rx):
> > > +    """Run all variants of monotonic (fq) tests."""
> > > +    cmd(f"tc qdisc replace dev {cfg.ifname} root fq")
> > > +    test_so_txtime(cfg, "mono", ipver, args_tx, args_rx, False)
> > > +
> > > +
> > > +def _test_variants_etf():
> > > +    for ipver in ["4", "6"]:
> > > +        for testcase in [
> > > +            ["no_delay", "a,-1", "a,-1", True],
> > > +            ["zero_delay", "a,0", "a,0", True],
> > > +            ["one_pkt", "a,10", "a,10", False],
> > > +            ["in_order", "a,10,b,20", "a,10,b,20", False],
> > > +            ["reverse_order", "a,20,b,10", "b,10,a,20", False],
> > > +        ]:
> > > +            name = f"_v{ipver}_{testcase[0]}"
> > 
> > nit: looking at the results in NIPA:
> > https://netdev-ctrl.bots.linux.dev/logs/vmksft/net-drv/results/593442/5-so-txtime-py/stdout
> > the leading _ seems unnecessary? 
> >
> > > +            yield KsftNamedVariant(
> > > +                name, ipver, testcase[1], testcase[2], testcase[3]
> > > +            )
> > > +
> > > +
> > > +@ksft_variants(_test_variants_etf())
> > > +def test_so_txtime_etf(cfg, ipver, args_tx, args_rx, expect_fail):
> > > +    """Run all variants of etf tests."""
> > > +    try:
> > > +        # ETF does not support change, so remove and re-add it instead.
> > > +        cmd_prefix = f"tc qdisc replace dev {cfg.ifname} root"
> > > +        cmd(f"{cmd_prefix} pfifo_fast")
> > > +        cmd(f"{cmd_prefix} etf clockid CLOCK_TAI delta 400000")
> > > +    except Exception as e:
> > > +        raise KsftSkipEx("tc does not support qdisc etf. skipping") from e
> > > +
> > > +    test_so_txtime(cfg, "tai", ipver, args_tx, args_rx, expect_fail)
> > 
> > I _think_ we'll leave ETF installed on the device after the test?
> > That seems not super great. As we discussed before rebuilding the 
> > whole hierarchy will be tedious but we could at least replace with
> > mq on exit and let it put whatever the default qdisc is as its leaves?
> 
> Good point. We can not set mq on netkit. It fails netif_is_multiqueue
> in mq_init_common. I'll do the following.
> 
> @@ -81,6 +81,8 @@ def main() -> None:
>      """Boilerplate ksft main."""
>      with NetDrvEpEnv(__file__) as cfg:
>          ksft_run([test_so_txtime_mono, test_so_txtime_etf], args=(cfg,))
> +        if not cfg._ns:
> +            cmd(f"tc qdisc replace dev {cfg.ifname} root mq")
>      ksft_exit()

Actually, looking at a private field is not a good idea.
 
> Alternatively could record the root qdisc at the start of the test and
> restore that.

This should work:

    def main() -> None:
        """Boilerplate ksft main."""
        with NetDrvEpEnv(__file__) as cfg:
    +        # Record original root qdisc
    +        cmd_obj = cmd((f"tc -j qdisc show dev {cfg.ifname} root"))
    +        qdisc_root = json.loads(cmd_obj.stdout)[0].get("kind", None)
    +
    	ksft_run([test_so_txtime_mono, test_so_txtime_etf], args=(cfg,))
    +
    +        # Restore original root qdisc. If mq, populate with default_qdisc nodes
    +        if (qdisc_root):
    +            cmd(f"tc qdisc replace dev {cfg.ifname} root {qdisc_root}")
        ksft_exit()


Do we want to add a tc command similar to ip, bpftool, etc.