[PATCH net-next 0/2] selftests: drv-net: so_txtime: trivial fixes

[PATCH net-next 0/2] selftests: drv-net: so_txtime: trivial fixes

[Jakub Kicinski]

I noticed that so_txtime is only passing on NIPA setups which are
looped within a single host. The cross-machine cases just flat out
fail. The initial bug is obvious - the test does not deploy the binary.
But even with that I think more work would be needed to sync the
time / adjust the expectations for a dual-machine test.

Feel free to nack / take over, this series is admittedly achieving
very little in practice. The test will still not be usable on dual-host.

Jakub Kicinski (2):
  selftests: drv-net: so_txtime: remember to deploy the binaries
  selftests: drv-net: so_txtime: check IP versions

 tools/testing/selftests/drivers/net/so_txtime.py | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

-- 
2.54.0

[PATCH net-next 1/2] selftests: drv-net: so_txtime: remember to deploy the binaries

[Jakub Kicinski]

The test seems to be written with a single-host loopback
in mind. We need to deploy the binary to remote before
we run it. This is just fixing an obvious issue, but
more work will be needed to make the dual-host setup
work reliably. Most of the runs still fail with:

  FAIL: start time already passed

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/testing/selftests/drivers/net/so_txtime.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/so_txtime.py b/tools/testing/selftests/drivers/net/so_txtime.py
index 5d4388bfc6dd..ddcd2c0e06a8 100755
--- a/tools/testing/selftests/drivers/net/so_txtime.py
+++ b/tools/testing/selftests/drivers/net/so_txtime.py
@@ -18,14 +18,16 @@ from lib.py import NetDrvEpEnv, bkg, cmd, defer, tc
     """Main function. Run so_txtime as sender and receiver."""
     slow_machine = os.environ.get('KSFT_MACHINE_SLOW')
 
-    bin_path = cfg.test_dir / "so_txtime"
+    if not hasattr(cfg, "bin_remote"):
+        cfg.bin_local = cfg.test_dir / "so_txtime"
+        cfg.bin_remote = cfg.remote.deploy(cfg.bin_local)
 
     tstart = time.time_ns() + (2000_000_000 if slow_machine else 200_000_000)
 
     cmd_addr = f"-S {cfg.addr_v[ipver]} -D {cfg.remote_addr_v[ipver]}"
-    cmd_base = f"{bin_path} -{ipver} -c {clockid} -t {tstart} {cmd_addr}"
-    cmd_rx = f"{cmd_base} {args_rx} -r"
-    cmd_tx = f"{cmd_base} {args_tx}"
+    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}"
 
     expect_fail = not expect_success
     if slow_machine:
-- 
2.54.0

[PATCH net-next 2/2] selftests: drv-net: so_txtime: check IP versions

[Jakub Kicinski]

This test needs more work, and it fails in non-obvious way
when IPv4 connectivity is not available:

  # Exception| CMD[remote]: /tmp/vjquwblf/gukinuzqso_txtime -4 -c mono -t 1780939014114542914 -S None -D None a,0 -r
  # Exception|   EXIT: -15

Explicitly check for IPv4 support to make the future triage
less painful.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/testing/selftests/drivers/net/so_txtime.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/so_txtime.py b/tools/testing/selftests/drivers/net/so_txtime.py
index ddcd2c0e06a8..8fe061f52055 100755
--- a/tools/testing/selftests/drivers/net/so_txtime.py
+++ b/tools/testing/selftests/drivers/net/so_txtime.py
@@ -64,6 +64,7 @@ from lib.py import NetDrvEpEnv, bkg, cmd, defer, tc
 @ksft_variants(_test_variants_mono())
 def test_so_txtime_mono(cfg, ipver, args_tx, args_rx):
     """Run all variants of monotonic (fq) tests."""
+    cfg.require_ipver(ipver)
     _qdisc_setup(cfg.ifname, "fq")
     test_so_txtime(cfg, "mono", ipver, args_tx, args_rx, True)
 
@@ -86,6 +87,7 @@ from lib.py import NetDrvEpEnv, bkg, cmd, defer, tc
 @ksft_variants(_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)
     try:
         _qdisc_setup(cfg.ifname, "etf", "clockid CLOCK_TAI delta 400000")
     except Exception as e:
-- 
2.54.0

Re: [PATCH net-next 0/2] selftests: drv-net: so_txtime: trivial fixes

[Willem de Bruijn]

Jakub Kicinski wrote:
> I noticed that so_txtime is only passing on NIPA setups which are
> looped within a single host. The cross-machine cases just flat out
> fail. The initial bug is obvious - the test does not deploy the binary.
> But even with that I think more work would be needed to sync the
> time / adjust the expectations for a dual-machine test.
> 
> Feel free to nack / take over, this series is admittedly achieving
> very little in practice. The test will still not be usable on dual-host.

Thanks for these initial fixes.

I have indeed focused on vng and physical prod, where I cannot deploy
kselftests as is, run them a bit differently. And evidently missed
important parts as a result.

Let me spin up some cloud instances instead and iron out these kinks.
 
> Jakub Kicinski (2):
>   selftests: drv-net: so_txtime: remember to deploy the binaries
>   selftests: drv-net: so_txtime: check IP versions
> 
>  tools/testing/selftests/drivers/net/so_txtime.py | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> -- 
> 2.54.0
> 

Re: [PATCH net-next 1/2] selftests: drv-net: so_txtime: remember to deploy the binaries

[Willem de Bruijn]

Jakub Kicinski wrote:
> The test seems to be written with a single-host loopback
> in mind. We need to deploy the binary to remote before
> we run it. This is just fixing an obvious issue, but
> more work will be needed to make the dual-host setup
> work reliably. Most of the runs still fail with:
> 
>   FAIL: start time already passed
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Re: [PATCH net-next 2/2] selftests: drv-net: so_txtime: check IP versions

[Willem de Bruijn]

Jakub Kicinski wrote:
> This test needs more work, and it fails in non-obvious way
> when IPv4 connectivity is not available:
> 
>   # Exception| CMD[remote]: /tmp/vjquwblf/gukinuzqso_txtime -4 -c mono -t 1780939014114542914 -S None -D None a,0 -r
>   # Exception|   EXIT: -15
> 
> Explicitly check for IPv4 support to make the future triage
> less painful.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Willem de Bruijn <willemb@google.com>

but two comments FYI:

> ---
>  tools/testing/selftests/drivers/net/so_txtime.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/testing/selftests/drivers/net/so_txtime.py b/tools/testing/selftests/drivers/net/so_txtime.py
> index ddcd2c0e06a8..8fe061f52055 100755
> --- a/tools/testing/selftests/drivers/net/so_txtime.py
> +++ b/tools/testing/selftests/drivers/net/so_txtime.py
> @@ -64,6 +64,7 @@ from lib.py import NetDrvEpEnv, bkg, cmd, defer, tc
>  @ksft_variants(_test_variants_mono())
>  def test_so_txtime_mono(cfg, ipver, args_tx, args_rx):
>      """Run all variants of monotonic (fq) tests."""
> +    cfg.require_ipver(ipver)

This may cause aconflict with my outstanding SO_TXTIME series.
And more importantly miss having it in the new test_so_txtime_fq_tai
variant.

Could this test be moved to test_so_txtime that all of them call?

(there is WIP to add another mono_hw variant too)

>      _qdisc_setup(cfg.ifname, "fq")
>      test_so_txtime(cfg, "mono", ipver, args_tx, args_rx, True)
> 
> @@ -86,6 +87,7 @@ from lib.py import NetDrvEpEnv, bkg, cmd, defer, tc
>  @ksft_variants(_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)
>      try:
>          _qdisc_setup(cfg.ifname, "etf", "clockid CLOCK_TAI delta 400000")
>      except Exception as e:
> -- 
> 2.54.0
>