[PATCH net] selftests: netdevsim: fix the udp_tunnel_nic test

[PATCH net] selftests: netdevsim: fix the udp_tunnel_nic test

[Jakub Kicinski]

This test is missing a whole bunch of checks for interface
renaming and one ifup. Presumably it was only used on a system
with renaming disabled and NetworkManager running.

Fixes: 91f430b2c49d ("selftests: net: add a test for UDP tunnel info infra")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: shuah@kernel.org
CC: horms@kernel.org
CC: linux-kselftest@vger.kernel.org
---
 .../selftests/drivers/net/netdevsim/udp_tunnel_nic.sh    | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh b/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh
index 4855ef597a15..f98435c502f6 100755
--- a/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh
+++ b/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh
@@ -270,6 +270,7 @@ for port in 0 1; do
 	echo 1 > $NSIM_DEV_SYS/new_port
     fi
     NSIM_NETDEV=`get_netdev_name old_netdevs`
+    ifconfig $NSIM_NETDEV up
 
     msg="new NIC device created"
     exp0=( 0 0 0 0 )
@@ -431,6 +432,7 @@ for port in 0 1; do
     fi
 
     echo $port > $NSIM_DEV_SYS/new_port
+    NSIM_NETDEV=`get_netdev_name old_netdevs`
     ifconfig $NSIM_NETDEV up
 
     overflow_table0 "overflow NIC table"
@@ -488,6 +490,7 @@ for port in 0 1; do
     fi
 
     echo $port > $NSIM_DEV_SYS/new_port
+    NSIM_NETDEV=`get_netdev_name old_netdevs`
     ifconfig $NSIM_NETDEV up
 
     overflow_table0 "overflow NIC table"
@@ -544,6 +547,7 @@ for port in 0 1; do
     fi
 
     echo $port > $NSIM_DEV_SYS/new_port
+    NSIM_NETDEV=`get_netdev_name old_netdevs`
     ifconfig $NSIM_NETDEV up
 
     overflow_table0 "destroy NIC"
@@ -573,6 +577,7 @@ for port in 0 1; do
     fi
 
     echo $port > $NSIM_DEV_SYS/new_port
+    NSIM_NETDEV=`get_netdev_name old_netdevs`
     ifconfig $NSIM_NETDEV up
 
     msg="create VxLANs v6"
@@ -633,6 +638,7 @@ for port in 0 1; do
     fi
 
     echo $port > $NSIM_DEV_SYS/new_port
+    NSIM_NETDEV=`get_netdev_name old_netdevs`
     ifconfig $NSIM_NETDEV up
 
     echo 110 > $NSIM_DEV_DFS/ports/$port/udp_ports_inject_error
@@ -688,6 +694,7 @@ for port in 0 1; do
     fi
 
     echo $port > $NSIM_DEV_SYS/new_port
+    NSIM_NETDEV=`get_netdev_name old_netdevs`
     ifconfig $NSIM_NETDEV up
 
     msg="create VxLANs v6"
@@ -747,6 +754,7 @@ for port in 0 1; do
     fi
 
     echo $port > $NSIM_DEV_SYS/new_port
+    NSIM_NETDEV=`get_netdev_name old_netdevs`
     ifconfig $NSIM_NETDEV up
 
     msg="create VxLANs v6"
@@ -877,6 +885,7 @@ msg="re-add a port"
 
 echo 2 > $NSIM_DEV_SYS/del_port
 echo 2 > $NSIM_DEV_SYS/new_port
+NSIM_NETDEV=`get_netdev_name old_netdevs`
 check_tables
 
 msg="replace VxLAN in overflow table"
-- 
2.43.0

Re: [PATCH net] selftests: netdevsim: fix the udp_tunnel_nic test

[Paolo Abeni]

On Mon, 2024-01-22 at 22:05 -0800, Jakub Kicinski wrote:
> This test is missing a whole bunch of checks for interface
> renaming and one ifup. Presumably it was only used on a system
> with renaming disabled and NetworkManager running.
> 
> Fixes: 91f430b2c49d ("selftests: net: add a test for UDP tunnel info infra")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: shuah@kernel.org
> CC: horms@kernel.org
> CC: linux-kselftest@vger.kernel.org
> ---
>  .../selftests/drivers/net/netdevsim/udp_tunnel_nic.sh    | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh b/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh
> index 4855ef597a15..f98435c502f6 100755
> --- a/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh
> +++ b/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh
> @@ -270,6 +270,7 @@ for port in 0 1; do
>  	echo 1 > $NSIM_DEV_SYS/new_port
>      fi
>      NSIM_NETDEV=`get_netdev_name old_netdevs`
> +    ifconfig $NSIM_NETDEV up

WoW! I initially thought the above was a typo, before noticing it's
actually consistent with the whole script :)

Do you think we should look at dropping ifconfig usage from self-tests?
I guess that in the long run most systems should not have such command
available in the default install.

In any case the patch LGTM.

Acked-by: Paolo Abeni <pabeni@redhat.com>

Cheers,

Paolo

Re: [PATCH net] selftests: netdevsim: fix the udp_tunnel_nic test

[Jakub Kicinski]

On Tue, 23 Jan 2024 09:27:17 +0100 Paolo Abeni wrote:
> > @@ -270,6 +270,7 @@ for port in 0 1; do
> >  	echo 1 > $NSIM_DEV_SYS/new_port
> >      fi
> >      NSIM_NETDEV=`get_netdev_name old_netdevs`
> > +    ifconfig $NSIM_NETDEV up  
> 
> WoW! I initially thought the above was a typo, before noticing it's
> actually consistent with the whole script :)
> 
> Do you think we should look at dropping ifconfig usage from self-tests?
> I guess that in the long run most systems should not have such command
> available in the default install.

Good point, there's only one use outside of this script.
I'll queue up a conversion for -next!

Re: [PATCH net] selftests: netdevsim: fix the udp_tunnel_nic test

[Jakub Kicinski]

On Tue, 23 Jan 2024 07:39:25 -0800 Jakub Kicinski wrote:
> On Tue, 23 Jan 2024 09:27:17 +0100 Paolo Abeni wrote:
> > > @@ -270,6 +270,7 @@ for port in 0 1; do
> > >  	echo 1 > $NSIM_DEV_SYS/new_port
> > >      fi
> > >      NSIM_NETDEV=`get_netdev_name old_netdevs`
> > > +    ifconfig $NSIM_NETDEV up    
> > 
> > WoW! I initially thought the above was a typo, before noticing it's
> > actually consistent with the whole script :)
> > 
> > Do you think we should look at dropping ifconfig usage from self-tests?
> > I guess that in the long run most systems should not have such command
> > available in the default install.  
> 
> Good point, there's only one use outside of this script.
> I'll queue up a conversion for -next!

Hm, Hi Florian! Is the use of ifconfig in kci_test_promote_secondaries,
commit bb2bd090854cc6 on purpose or can I switch that to ip addr, too?

Re: [PATCH net] selftests: netdevsim: fix the udp_tunnel_nic test

[Simon Horman]

On Mon, Jan 22, 2024 at 10:05:29PM -0800, Jakub Kicinski wrote:
> This test is missing a whole bunch of checks for interface
> renaming and one ifup. Presumably it was only used on a system
> with renaming disabled and NetworkManager running.
> 
> Fixes: 91f430b2c49d ("selftests: net: add a test for UDP tunnel info infra")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

I see some discussion of a follow-up to move away from ifconfig which I
agree with (which century are we in now?). But this patch looks good in
it's own right.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net] selftests: netdevsim: fix the udp_tunnel_nic test

[patchwork-bot+netdevbpf]

Hello:

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

On Mon, 22 Jan 2024 22:05:29 -0800 you wrote:
> This test is missing a whole bunch of checks for interface
> renaming and one ifup. Presumably it was only used on a system
> with renaming disabled and NetworkManager running.
> 
> Fixes: 91f430b2c49d ("selftests: net: add a test for UDP tunnel info infra")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> 
> [...]

Here is the summary with links:
  - [net] selftests: netdevsim: fix the udp_tunnel_nic test
    https://git.kernel.org/netdev/net/c/0879020a7817

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