| Message ID | 20230314111426.1254998-3-razor@blackwall.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | bonding: properly restore flags when bond changes ether type | expand |
On Tue, Mar 14, 2023 at 01:14:24PM +0200, Nikolay Aleksandrov wrote: > If the bond enslaves non-ARPHRD_ETHER device (changes its type), then > releases it and enslaves ARPHRD_ETHER device (changes back) then we > use ether_setup() to restore the bond device type but it also resets its > flags and removes IFF_MASTER and IFF_SLAVE[1]. Use the bond_ether_setup > helper to restore both after such transition. > > [1] reproduce (nlmon is non-ARPHRD_ETHER): > $ ip l add nlmon0 type nlmon > $ ip l add bond2 type bond mode active-backup > $ ip l set nlmon0 master bond2 > $ ip l set nlmon0 nomaster > $ ip l add bond1 type bond > (we use bond1 as ARPHRD_ETHER device to restore bond2's mode) > $ ip l set bond1 master bond2 > $ ip l sh dev bond2 > 37: bond2: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000 > link/ether be:d7:c5:40:5b:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 1500 > (notice bond2's IFF_MASTER is missing) > > Fixes: e36b9d16c6a6 ("bonding: clean muticast addresses when device changes type") > Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org> > --- > drivers/net/bonding/bond_main.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c > index d41024ad2c18..cd94baccdac5 100644 > --- a/drivers/net/bonding/bond_main.c > +++ b/drivers/net/bonding/bond_main.c > @@ -1878,10 +1878,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev, > > if (slave_dev->type != ARPHRD_ETHER) > bond_setup_by_slave(bond_dev, slave_dev); > - else { > - ether_setup(bond_dev); > - bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING; > - } > + else > + bond_ether_setup(bond_dev); As I already commented on your previous patch: there is the first call of "bond_ether_setup()". Please think about merging this patch with the previous one to avoid the compilation warning. Thanks, Michal > > call_netdevice_notifiers(NETDEV_POST_TYPE_CHANGE, > bond_dev); > -- > 2.39.2 >
On 14/03/2023 17:09, Michal Kubiak wrote: > On Tue, Mar 14, 2023 at 01:14:24PM +0200, Nikolay Aleksandrov wrote: >> If the bond enslaves non-ARPHRD_ETHER device (changes its type), then >> releases it and enslaves ARPHRD_ETHER device (changes back) then we >> use ether_setup() to restore the bond device type but it also resets its >> flags and removes IFF_MASTER and IFF_SLAVE[1]. Use the bond_ether_setup >> helper to restore both after such transition. >> >> [1] reproduce (nlmon is non-ARPHRD_ETHER): >> $ ip l add nlmon0 type nlmon >> $ ip l add bond2 type bond mode active-backup >> $ ip l set nlmon0 master bond2 >> $ ip l set nlmon0 nomaster >> $ ip l add bond1 type bond >> (we use bond1 as ARPHRD_ETHER device to restore bond2's mode) >> $ ip l set bond1 master bond2 >> $ ip l sh dev bond2 >> 37: bond2: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000 >> link/ether be:d7:c5:40:5b:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 1500 >> (notice bond2's IFF_MASTER is missing) >> >> Fixes: e36b9d16c6a6 ("bonding: clean muticast addresses when device changes type") >> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org> >> --- >> drivers/net/bonding/bond_main.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >> index d41024ad2c18..cd94baccdac5 100644 >> --- a/drivers/net/bonding/bond_main.c >> +++ b/drivers/net/bonding/bond_main.c >> @@ -1878,10 +1878,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev, >> >> if (slave_dev->type != ARPHRD_ETHER) >> bond_setup_by_slave(bond_dev, slave_dev); >> - else { >> - ether_setup(bond_dev); >> - bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING; >> - } >> + else >> + bond_ether_setup(bond_dev); > > As I already commented on your previous patch: there is the first call > of "bond_ether_setup()". > Please think about merging this patch with the previous one to avoid the > compilation warning. > > Thanks, > Michal > Please check my replies to your comment of the first patch. Thanks
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index d41024ad2c18..cd94baccdac5 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1878,10 +1878,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev, if (slave_dev->type != ARPHRD_ETHER) bond_setup_by_slave(bond_dev, slave_dev); - else { - ether_setup(bond_dev); - bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING; - } + else + bond_ether_setup(bond_dev); call_netdevice_notifiers(NETDEV_POST_TYPE_CHANGE, bond_dev);
If the bond enslaves non-ARPHRD_ETHER device (changes its type), then releases it and enslaves ARPHRD_ETHER device (changes back) then we use ether_setup() to restore the bond device type but it also resets its flags and removes IFF_MASTER and IFF_SLAVE[1]. Use the bond_ether_setup helper to restore both after such transition. [1] reproduce (nlmon is non-ARPHRD_ETHER): $ ip l add nlmon0 type nlmon $ ip l add bond2 type bond mode active-backup $ ip l set nlmon0 master bond2 $ ip l set nlmon0 nomaster $ ip l add bond1 type bond (we use bond1 as ARPHRD_ETHER device to restore bond2's mode) $ ip l set bond1 master bond2 $ ip l sh dev bond2 37: bond2: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000 link/ether be:d7:c5:40:5b:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 1500 (notice bond2's IFF_MASTER is missing) Fixes: e36b9d16c6a6 ("bonding: clean muticast addresses when device changes type") Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org> --- drivers/net/bonding/bond_main.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)