diff mbox series

[net-next] selftests: bonding: delete link1_1 in the cleanup path

Message ID 20230823032640.3609934-1-shaozhengchao@huawei.com
State New
Headers show
Series [net-next] selftests: bonding: delete link1_1 in the cleanup path | expand

Commit Message

shaozhengchao Aug. 23, 2023, 3:26 a.m. UTC
If failed to set link1_1 to netns client, we should delete link1_1 in the
cleanup path. But if set link1_1 to netns client successfully, delete
link1_1 will report warning. So delete link1_1 in the cleanup path and
drop any warning message.

Reported-by: Hangbin Liu <liuhangbin@gmail.com>
Closes: https://lore.kernel.org/all/ZNyJx1HtXaUzOkNA@Laptop-X1/
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 .../drivers/net/bonding/bond-arp-interval-causes-panic.sh        | 1 +
 1 file changed, 1 insertion(+)

Comments

Simon Horman Aug. 24, 2023, 8:50 a.m. UTC | #1
On Wed, Aug 23, 2023 at 11:26:40AM +0800, Zhengchao Shao wrote:
> If failed to set link1_1 to netns client, we should delete link1_1 in the
> cleanup path. But if set link1_1 to netns client successfully, delete
> link1_1 will report warning. So delete link1_1 in the cleanup path and
> drop any warning message.

Hi Zhengchao Shao,

It seems unfortunate to drop all warning and error messages.
What if the message is about something other than link1_1 not existing?

Would it be practical to check if link1_1 exists,
say by looking in sysfs, before trying to delete it?

> Reported-by: Hangbin Liu <liuhangbin@gmail.com>
> Closes: https://lore.kernel.org/all/ZNyJx1HtXaUzOkNA@Laptop-X1/
> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
> ---
>  .../drivers/net/bonding/bond-arp-interval-causes-panic.sh        | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/testing/selftests/drivers/net/bonding/bond-arp-interval-causes-panic.sh b/tools/testing/selftests/drivers/net/bonding/bond-arp-interval-causes-panic.sh
> index 7b2d421f09cf..2b3c678c5205 100755
> --- a/tools/testing/selftests/drivers/net/bonding/bond-arp-interval-causes-panic.sh
> +++ b/tools/testing/selftests/drivers/net/bonding/bond-arp-interval-causes-panic.sh
> @@ -11,6 +11,7 @@ finish()
>  {
>  	ip netns delete server || true
>  	ip netns delete client || true
> +	ip link del link1_1 >/dev/null 2>&1
>  }
>  
>  trap finish EXIT
> -- 
> 2.34.1
> 
>
Paolo Abeni Aug. 24, 2023, 10:31 a.m. UTC | #2
On Wed, 2023-08-23 at 11:26 +0800, Zhengchao Shao wrote:
> If failed to set link1_1 to netns client, we should delete link1_1 in the
> cleanup path. But if set link1_1 to netns client successfully, delete
> link1_1 will report warning. So delete link1_1 in the cleanup path and
> drop any warning message.

I think the same could happen also for the link1_2 device.

It would probably be safer creating directly the devices in the target
namespaces, with the 'final' name

ip link add dev eth0 netns client type veth peer name eth0 netns server

Cheers,

Paolo
shaozhengchao Aug. 24, 2023, 1:25 p.m. UTC | #3
On 2023/8/24 18:31, Paolo Abeni wrote:
> On Wed, 2023-08-23 at 11:26 +0800, Zhengchao Shao wrote:
>> If failed to set link1_1 to netns client, we should delete link1_1 in the
>> cleanup path. But if set link1_1 to netns client successfully, delete
>> link1_1 will report warning. So delete link1_1 in the cleanup path and
>> drop any warning message.
> 
> I think the same could happen also for the link1_2 device.
> 
> It would probably be safer creating directly the devices in the target
> namespaces, with the 'final' name
> 
> ip link add dev eth0 netns client type veth peer name eth0 netns server
> 
> Cheers,
> 
> Paolo
> 
Yeah, it looks good, I will send v2.
Thank you.

Zhengchao Shao
diff mbox series

Patch

diff --git a/tools/testing/selftests/drivers/net/bonding/bond-arp-interval-causes-panic.sh b/tools/testing/selftests/drivers/net/bonding/bond-arp-interval-causes-panic.sh
index 7b2d421f09cf..2b3c678c5205 100755
--- a/tools/testing/selftests/drivers/net/bonding/bond-arp-interval-causes-panic.sh
+++ b/tools/testing/selftests/drivers/net/bonding/bond-arp-interval-causes-panic.sh
@@ -11,6 +11,7 @@  finish()
 {
 	ip netns delete server || true
 	ip netns delete client || true
+	ip link del link1_1 >/dev/null 2>&1
 }
 
 trap finish EXIT