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

[Hangbin Liu <liuhangbin@gmail.com>]

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;