| Message ID | 20210120102947.2887543-1-liuhangbin@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] selftests/net: set link down before enslave | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 3 maintainers not CCed: shuah@kernel.org linux-kselftest@vger.kernel.org kuba@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Wed, Jan 20, 2021 at 06:29:47PM +0800, Hangbin Liu wrote: > 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 <liuhangbin@gmail.com> > --- > 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 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: > > 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: > > > 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: > > Hi Ido, > > > > On Wed, Jan 20, 2021 at 12:42:10PM +0200, Ido Schimmel wrote: > > > > 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: > > 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: > > > > > 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
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
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 <liuhangbin@gmail.com> --- tools/testing/selftests/net/rtnetlink.sh | 2 ++ 1 file changed, 2 insertions(+)