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
>
> > + unregister_netdevice(bond_dev);
> > }
> >
> > return err;
