Re: [PATCH net 1/2] bonding: fix xfrm offload feature setup on active-backup mode

[Hangbin Liu]

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;