Re: [PATCH] selftests: net: local_termination: Wait for interfaces to come up

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Tue, Nov 04, 2025 at 07:17:21AM +0100, A. Sverdlin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> 
> It seems that most of the tests prepare the interfaces once before the test
> run (setup_prepare()), rely on setup_wait() to wait for link and only then
> run the test(s).
> 
> local_termination brings the physical interfaces down and up during test
> run but never wait for them to come up. If the auto-negotiation takes
> some seconds, first test packets are being lost, which leads to
> false-negative test results.

Yes, sorry, dropping the link is an unfortunate, undesirable and
unaccounted for side effect of simple_if_init and simple_if_fini,
mainly used for moving the IP addresses from the physical to the various
virtual upper interfaces.

> 
> Use setup_wait_dev() after corresponding simple_if_init() on physical
> interfaces to make sure auto-negotiation has been completed and test
> packets will not be lost because of the race against link establishment.
> 
> The wait has to be done in each individual test because the interfaces
> have to be brough up first and only then we can wait for link (not
> individually, because they are expected to be looped in pairs).
> 
> Fixes: 90b9566aa5cd3f ("selftests: forwarding: add a test for local_termination.sh")
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> ---
>  .../selftests/net/forwarding/local_termination.sh      | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/forwarding/local_termination.sh b/tools/testing/selftests/net/forwarding/local_termination.sh
> index ecd34f364125c..369c8b2c1f4a2 100755
> --- a/tools/testing/selftests/net/forwarding/local_termination.sh
> +++ b/tools/testing/selftests/net/forwarding/local_termination.sh
> @@ -430,6 +430,8 @@ standalone()
>  	h1_create
>  	h2_create
>  	macvlan_create $h2
> +	setup_wait_dev $h1
> +	setup_wait_dev $h2
>  
>  	run_test $h1 $h2 $skip_ptp $no_unicast_flt "$h2"
>  
> @@ -448,6 +450,8 @@ test_bridge()
>  	bridge_create $vlan_filtering
>  	simple_if_init br0 $H2_IPV4/24 $H2_IPV6/64
>  	macvlan_create br0
> +	setup_wait_dev $h1
> +	setup_wait_dev $h2
>  
>  	run_test $h1 br0 $skip_ptp $no_unicast_flt \
>  		"vlan_filtering=$vlan_filtering bridge"
> @@ -480,6 +484,8 @@ test_vlan()
>  	h1_vlan_create
>  	h2_vlan_create
>  	macvlan_create $h2.100
> +	setup_wait_dev $h1
> +	setup_wait_dev $h2
>  
>  	run_test $h1.100 $h2.100 $skip_ptp $no_unicast_flt "VLAN upper"
>  
> @@ -505,6 +511,8 @@ vlan_over_bridged_port()
>  	h2_vlan_create
>  	bridge_create $vlan_filtering
>  	macvlan_create $h2.100
> +	setup_wait_dev $h1
> +	setup_wait_dev $h2
>  
>  	run_test $h1.100 $h2.100 $skip_ptp $no_unicast_flt \
>  		"VLAN over vlan_filtering=$vlan_filtering bridged port"
> @@ -536,6 +544,8 @@ vlan_over_bridge()
>  	simple_if_init br0
>  	vlan_create br0 100 vbr0 $H2_IPV4/24 $H2_IPV6/64
>  	macvlan_create br0.100
> +	setup_wait_dev $h1
> +	setup_wait_dev $h2
>  
>  	if [ $vlan_filtering = 1 ]; then
>  		bridge vlan add dev $h2 vid 100 master
> -- 
> 2.51.1
>

Functionally I have nothing against this change.

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Two ideas to minimize the delta in a more obviously correct way.
They can perhaps be implemented together, independently of one another,
or not at all:

- setup_wait() could be used directly, as it waits for $NUM_NETIFS, aka
  $h1 and $h2.
- There is no case where run_test() does not need a prior setup_wait()
  call, so it can just as well be placed as the first thing of that
  function.