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

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

Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> In preparation for extending to pacing hardware offload, convert the
> so_txtime.sh test to a drv-net test that can be run against netdevsim
> and real hardware.
> 
> Also move so_txtime.c to lib so that it is easily accessible for the
> new drv-net test, similar to gro.c in commit 8888bf4fb980 ("selftests:
> net: move gro to lib for HW vs SW reuse").
> 
> Also update so_txtime.c to not exit on first failure, but run to
> completion and report exit code there. This helps with debugging
> unexpected results, especially when processing multiple packets,
> as in the "reverse_order" testcase.
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>

Sashiko already has a few correct comments (below). Will fix in v2.

Also, indentation incorrectly uses 2 instead of 4 spaces.

> +def test_so_txtime(cfg, clockid, ipver, args_tx, args_rx, expect_fail):
> +  bin_path = cfg.net_lib_dir / "so_txtime"
> +
> +  tstart = time.time_ns() + 100_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}"
> +
> +  try:
> +    with bkg(cmd_rx, host=cfg.remote, exit_wait=True):
> +      cmd(cmd_tx)
> +  except:
> +    if not expect_fail:
> +      raise

Also test against a test succeeding when failure is expected. Will add
an else: clause.


> @@ -131,13 +135,15 @@ static void do_recv_one(int fdr, struct timed_send *ts)
>  	fprintf(stderr, "payload:%c delay:%lld expected:%lld (us)\n",
>  			rbuf[0], (long long)tstop, (long long)texpect);
>  
> -	if (rbuf[0] != ts->data)
> -		error(1, 0, "payload mismatch. expected %c", ts->data);
> +	if (rbuf[0] != ts->data) {
> +		fprintf(stderr, "payload mismatch. expected %c", ts->data);

Add \n when converting from error to fprintf

> @@ -255,8 +261,10 @@ static void start_time_wait(void)
>  		return;
>  
>  	now = gettime_ns(CLOCK_REALTIME);
> -	if (cfg_start_time_ns < now)
> -		return;
> +	if (cfg_start_time_ns < now) {
> +		fprintf(stderr, "FAIL: start time already passed\n");
> +		errors++;
> +	}
>  
>  	err = usleep((cfg_start_time_ns - now) / 1000);

Do not call usleep when already late: uint64_t will have negative
overflow. Also, waiting makes no sense when late. Add return to the
error branch.