* [PATCH net] selftests/net: set link down before enslave
@ 2021-01-20 10:29 Hangbin Liu
2021-01-20 10:42 ` Ido Schimmel
0 siblings, 1 reply; 6+ messages in thread
From: Hangbin Liu @ 2021-01-20 10:29 UTC (permalink / raw)
To: netdev
Cc: Ido Schimmel, Jiri Pirko, Nikolay Aleksandrov, David S . Miller,
Hangbin Liu
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
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net] selftests/net: set link down before enslave
2021-01-20 10:29 [PATCH net] selftests/net: set link down before enslave Hangbin Liu
@ 2021-01-20 10:42 ` Ido Schimmel
2021-01-20 14:38 ` Hangbin Liu
0 siblings, 1 reply; 6+ messages in thread
From: Ido Schimmel @ 2021-01-20 10:42 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Jiri Pirko, Nikolay Aleksandrov, David S . Miller
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] selftests/net: set link down before enslave
2021-01-20 10:42 ` Ido Schimmel
@ 2021-01-20 14:38 ` Hangbin Liu
2021-01-20 19:43 ` Ido Schimmel
0 siblings, 1 reply; 6+ messages in thread
From: Hangbin Liu @ 2021-01-20 14:38 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, Jiri Pirko, Nikolay Aleksandrov, David S . Miller
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] selftests/net: set link down before enslave
2021-01-20 14:38 ` Hangbin Liu
@ 2021-01-20 19:43 ` Ido Schimmel
2021-01-21 1:46 ` Jakub Kicinski
0 siblings, 1 reply; 6+ messages in thread
From: Ido Schimmel @ 2021-01-20 19:43 UTC (permalink / raw)
To: Hangbin Liu
Cc: Ido Schimmel, netdev, Jiri Pirko, Nikolay Aleksandrov, David S . Miller
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net] selftests/net: set link down before enslave
2021-01-20 19:43 ` Ido Schimmel
@ 2021-01-21 1:46 ` Jakub Kicinski
2021-01-21 3:02 ` Hangbin Liu
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2021-01-21 1:46 UTC (permalink / raw)
To: Ido Schimmel
Cc: Hangbin Liu, Ido Schimmel, netdev, Jiri Pirko,
Nikolay Aleksandrov, David S . Miller
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] selftests/net: set link down before enslave
2021-01-21 1:46 ` Jakub Kicinski
@ 2021-01-21 3:02 ` Hangbin Liu
0 siblings, 0 replies; 6+ messages in thread
From: Hangbin Liu @ 2021-01-21 3:02 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Ido Schimmel, Ido Schimmel, netdev, Jiri Pirko,
Nikolay Aleksandrov, David S . Miller
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-21 6:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 10:29 [PATCH net] selftests/net: set link down before enslave Hangbin Liu
2021-01-20 10:42 ` Ido Schimmel
2021-01-20 14:38 ` Hangbin Liu
2021-01-20 19:43 ` Ido Schimmel
2021-01-21 1:46 ` Jakub Kicinski
2021-01-21 3:02 ` Hangbin Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.