[PATCH net] selftests/net: set link down before enslave

Set the link down before enslave it to a bond device, to avoid
Error: Device can not be enslaved while up.

Fixes: 6374a5606990 ("selftests: rtnetlink: Test bridge enslavement with different parent IDs")
Signed-off-by: Hangbin Liu <redacted>
---
 tools/testing/selftests/net/rtnetlink.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index c9ce3dfa42ee..a26fddc63992 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh

@@ -1205,6 +1205,8 @@ kci_test_bridge_parent_id()
 	dev20=`ls ${sysfsnet}20/net/`
 
 	ip link add name test-bond0 type bond mode 802.3ad
+	ip link set dev $dev10 down
+	ip link set dev $dev20 down
 	ip link set dev $dev10 master test-bond0
 	ip link set dev $dev20 master test-bond0
 	ip link add name test-br0 type bridge

-- 
2.26.2

On Wed, Jan 20, 2021 at 06:29:47PM +0800, Hangbin Liu wrote:

But these netdevs are created with their administrative state set to
'DOWN'. Who is setting them to up?

 	ip link set dev $dev10 master test-bond0
 	ip link set dev $dev20 master test-bond0
 	ip link add name test-br0 type bridge
-- 
2.26.2

Hi Ido,

On Wed, Jan 20, 2021 at 12:42:10PM +0200, Ido Schimmel wrote:

quoted

diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index c9ce3dfa42ee..a26fddc63992 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh

@@ -1205,6 +1205,8 @@ kci_test_bridge_parent_id()
 	dev20=`ls ${sysfsnet}20/net/`
 
 	ip link add name test-bond0 type bond mode 802.3ad
+	ip link set dev $dev10 down
+	ip link set dev $dev20 down

But these netdevs are created with their administrative state set to
'DOWN'. Who is setting them to up?

Would you please point me where we set the state to 'DOWN'? Cause on my
host it is init as UP:

++ ls /sys/bus/netdevsim/devices/netdevsim10/net/
+ dev10=eth3
++ ls /sys/bus/netdevsim/devices/netdevsim20/net/
+ dev20=eth4
+ ip link add name test-bond0 type bond mode 802.3ad
+ ip link show eth3
66: eth3: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
    link/ether 1e:52:27:5f:a5:3c brd ff:ff:ff:ff:ff:ff

# uname -r
5.11.0-rc3+

Thanks
Hangbin

On Wed, Jan 20, 2021 at 10:38:47PM +0800, Hangbin Liu wrote:

Hi Ido,

On Wed, Jan 20, 2021 at 12:42:10PM +0200, Ido Schimmel wrote:

quoted

quoted

diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index c9ce3dfa42ee..a26fddc63992 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh

@@ -1205,6 +1205,8 @@ kci_test_bridge_parent_id()
 	dev20=`ls ${sysfsnet}20/net/`
 
 	ip link add name test-bond0 type bond mode 802.3ad
+	ip link set dev $dev10 down
+	ip link set dev $dev20 down

But these netdevs are created with their administrative state set to
'DOWN'. Who is setting them to up?

Would you please point me where we set the state to 'DOWN'? Cause on my
host it is init as UP:

++ ls /sys/bus/netdevsim/devices/netdevsim10/net/
+ dev10=eth3
++ ls /sys/bus/netdevsim/devices/netdevsim20/net/
+ dev20=eth4
+ ip link add name test-bond0 type bond mode 802.3ad
+ ip link show eth3
66: eth3: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
    link/ether 1e:52:27:5f:a5:3c brd ff:ff:ff:ff:ff:ff

I didn't have time to look into this today, but I suspect the problem is
either:

1. Some interface manager on your end that is setting these interfaces
up after they are created

2. A bug in netdevsim that does not initialize the carrier to off.
Maybe try with this patch (didn't test):

diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index aec92440eef1..1e0dc298bf20 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c

@@ -312,6 +312,8 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port)
 
        nsim_ipsec_init(ns);
 
+       netif_carrier_off(dev);
+
        err = register_netdevice(dev);
        if (err)
                goto err_ipsec_teardown;

# uname -r
5.11.0-rc3+

Thanks
Hangbin

On Wed, 20 Jan 2021 21:43:28 +0200 Ido Schimmel wrote:

On Wed, Jan 20, 2021 at 10:38:47PM +0800, Hangbin Liu wrote:

quoted

Hi Ido,

On Wed, Jan 20, 2021 at 12:42:10PM +0200, Ido Schimmel wrote:  

quoted

quoted

diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index c9ce3dfa42ee..a26fddc63992 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh

@@ -1205,6 +1205,8 @@ kci_test_bridge_parent_id()
 	dev20=`ls ${sysfsnet}20/net/`
 
 	ip link add name test-bond0 type bond mode 802.3ad
+	ip link set dev $dev10 down
+	ip link set dev $dev20 down  

But these netdevs are created with their administrative state set to
'DOWN'. Who is setting them to up?  

Would you please point me where we set the state to 'DOWN'? Cause on my
host it is init as UP:

++ ls /sys/bus/netdevsim/devices/netdevsim10/net/
+ dev10=eth3
++ ls /sys/bus/netdevsim/devices/netdevsim20/net/
+ dev20=eth4
+ ip link add name test-bond0 type bond mode 802.3ad
+ ip link show eth3
66: eth3: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
    link/ether 1e:52:27:5f:a5:3c brd ff:ff:ff:ff:ff:ff  

I didn't have time to look into this today, but I suspect the problem is
either:

1. Some interface manager on your end that is setting these interfaces
up after they are created

This must be the case, the kernel doesn't open/up devices by itself.

2. A bug in netdevsim that does not initialize the carrier to off.
Maybe try with this patch (didn't test):

Yeah, but that's fine, SW devices don't have to manage carrier state.

On Wed, Jan 20, 2021 at 05:46:51PM -0800, Jakub Kicinski wrote:

On Wed, 20 Jan 2021 21:43:28 +0200 Ido Schimmel wrote:

quoted

On Wed, Jan 20, 2021 at 10:38:47PM +0800, Hangbin Liu wrote:

quoted

Hi Ido,

On Wed, Jan 20, 2021 at 12:42:10PM +0200, Ido Schimmel wrote:  

quoted

quoted

diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index c9ce3dfa42ee..a26fddc63992 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh

@@ -1205,6 +1205,8 @@ kci_test_bridge_parent_id()
 	dev20=`ls ${sysfsnet}20/net/`
 
 	ip link add name test-bond0 type bond mode 802.3ad
+	ip link set dev $dev10 down
+	ip link set dev $dev20 down  

But these netdevs are created with their administrative state set to
'DOWN'. Who is setting them to up?  

Would you please point me where we set the state to 'DOWN'? Cause on my
host it is init as UP:

++ ls /sys/bus/netdevsim/devices/netdevsim10/net/
+ dev10=eth3
++ ls /sys/bus/netdevsim/devices/netdevsim20/net/
+ dev20=eth4
+ ip link add name test-bond0 type bond mode 802.3ad
+ ip link show eth3
66: eth3: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
    link/ether 1e:52:27:5f:a5:3c brd ff:ff:ff:ff:ff:ff  

I didn't have time to look into this today, but I suspect the problem is
either:

1. Some interface manager on your end that is setting these interfaces
up after they are created

This must be the case, the kernel doesn't open/up devices by itself.

Ah, yes, My bad. I found NetworkManager enabled it by default.

Thanks
Hangbin