Re: [PATCH net-next] bonding: report duplicate MAC address in all situations

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 19 Feb 2025 07:55:15 +0000 Hangbin Liu wrote:
> Normally, a bond uses the MAC address of the first added slave as the
> bond’s MAC address. And the bond will set active slave’s MAC address to
> bond’s address if fail_over_mac is set to none (0) or follow (2).
> 
> When the first slave is removed, the bond will still use the removed
> slave’s MAC address, which can lead to a duplicate MAC address and
> potentially cause issues with the switch. To avoid confusion, let's warn
> the user in all situations, including when fail_over_mac is set to 2 or
> in active-backup mode.

Makes sense, thanks for the high quality commit message.

False positive warnings are annoying to users (especially users who
monitor all warnings in their fleet). Could we stick to filtering out
the BOND_FOM_ACTIVE case? Looks like this condition:

	if (bond->params.fail_over_mac != BOND_FOM_ACTIVE ||
	    BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) {

exists a few lines later in __bond_release_one()

> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index e45bba240cbc..ca66107776cc 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2551,13 +2551,11 @@ static int __bond_release_one(struct net_device *bond_dev,
>  
>  	RCU_INIT_POINTER(bond->current_arp_slave, NULL);
>  
> -	if (!all && (!bond->params.fail_over_mac ||
> -		     BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP)) {
> -		if (ether_addr_equal_64bits(bond_dev->dev_addr, slave->perm_hwaddr) &&
> -		    bond_has_slaves(bond))
> -			slave_warn(bond_dev, slave_dev, "the permanent HWaddr of slave - %pM - is still in use by bond - set the HWaddr of slave to a different address to avoid conflicts\n",
> -				   slave->perm_hwaddr);
> -	}
> +	if (!all &&
> +	    ether_addr_equal_64bits(bond_dev->dev_addr, slave->perm_hwaddr) &&
> +	    bond_has_slaves(bond))
> +		slave_warn(bond_dev, slave_dev, "the permanent HWaddr of slave - %pM - is still in use by bond - set the HWaddr of slave to a different address to avoid conflicts\n",
> +			   slave->perm_hwaddr);
-- 
pw-bot: cr