Hangbin Liu liuhangbin@gmail.com wrote:
Commit 5c3bf6cba791 ("bonding: assign random address if device address is same as bond") fixed an issue where, after releasing the first slave and re-adding it to the bond with fail_over_mac=follow, both the active and backup slaves could end up with duplicate MAC addresses. To avoid this, the new slave was assigned a random address.
However, if this happens when adding the very first slave, the bond’s hardware address is set to match the slave’s. Later, during the fail_over_mac=follow check, the slave’s MAC is randomized because it naturally matches the bond, which is incorrect.
The description here seems confusing to me; what does "this" refer to? I don't understand the sequence of events that lead to the issue being fixed here.
I wonder if there's another bug somewhere, since nominally when releasing the last interface in the bond, __bond_release_one() should randomize the bond's MAC address, so it shouldn't match when adding (or re-adding ?) the first interface to the bond.
-J
The issue is normally hidden since the first slave usually becomes the active one, which restores the bond's MAC address. However, if another slave is selected as the initial active interface, the issue becomes visible.
Fix this by assigning a random address only when slaves already exist in the bond.
Fixes: 5c3bf6cba791 ("bonding: assign random address if device address is same as bond") Reported-by: Qiuling Ren qren@redhat.com Signed-off-by: Hangbin Liu liuhangbin@gmail.com
drivers/net/bonding/bond_main.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 257333c88710..8832bc9f107b 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2132,6 +2132,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev, memcpy(ss.__data, bond_dev->dev_addr, bond_dev->addr_len); } else if (bond->params.fail_over_mac == BOND_FOM_FOLLOW && BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP &&
bond_has_slaves(bond) &&
- address in later fail over.
-- 2.50.1
--- -Jay Vosburgh, jv@jvosburgh.net