On 12/12/24 11:39, Hangbin Liu wrote:
On Thu, Dec 12, 2024 at 11:19:33AM +0200, Nikolay Aleksandrov wrote:
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 49dd4fe195e5..7daeab67e7b5 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -4389,7 +4389,7 @@ void bond_work_init_all(struct bonding *bond) INIT_DELAYED_WORK(&bond->slave_arr_work, bond_slave_arr_handler); } -static void bond_work_cancel_all(struct bonding *bond) +void bond_work_cancel_all(struct bonding *bond) { cancel_delayed_work_sync(&bond->mii_work); cancel_delayed_work_sync(&bond->arp_work); diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c index 2a6a424806aa..7fe8c62366eb 100644 --- a/drivers/net/bonding/bond_netlink.c +++ b/drivers/net/bonding/bond_netlink.c @@ -568,18 +568,21 @@ static int bond_newlink(struct net *src_net, struct net_device *bond_dev, struct nlattr *tb[], struct nlattr *data[], struct netlink_ext_ack *extack) {
- struct bonding *bond = netdev_priv(bond_dev); int err;
- err = bond_changelink(bond_dev, tb, data, extack);
- if (err < 0)
- err = register_netdevice(bond_dev);
- if (err) return err;
- err = register_netdevice(bond_dev);
- if (!err) {
struct bonding *bond = netdev_priv(bond_dev);
- netif_carrier_off(bond_dev);
- bond_work_init_all(bond);
netif_carrier_off(bond_dev);bond_work_init_all(bond);
- err = bond_changelink(bond_dev, tb, data, extack);
- if (err) {
bond_work_cancel_all(bond);netif_carrier_on(bond_dev);The patch looks good, but I'm curious why the carrier on here?
The current code set netif_carrier_off(bond_dev) after register_netdevice() success, So I make it on if register failed.
Thanks hangbin
I don't like adding code just for symmetry alone, I think you should drop it unless there is an actual reason to turn carrier on.
unregister_netdevice(bond_dev);return err;