On Thu, Dec 12, 2024 at 11:43:15AM +0200, Nikolay Aleksandrov wrote:
> >>> --- 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.
OK, I will drop it.
Thanks
Hangbin
