On 2022-08-30 20:25 -0700, Jay Vosburgh wrote:
Benjamin Poirier bpoirier@nvidia.com wrote:
Netdev drivers are expected to call dev_{uc,mc}_sync() in their ndo_set_rx_mode method and dev_{uc,mc}_unsync() in their ndo_stop method. This is mentioned in the kerneldoc for those dev_* functions.
The bonding driver calls dev_{uc,mc}_unsync() during ndo_uninit instead of ndo_stop. This is ineffective because address lists (dev->{uc,mc}) have already been emptied in unregister_netdevice_many() before ndo_uninit is called. This mistake can result in addresses being leftover on former bond slaves after a bond has been deleted; see test_LAG_cleanup() in the last patch in this series.
Add unsync calls, via bond_hw_addr_flush(), at their expected location, bond_close(). Add dev_mc_add() call to bond_open() to match the above change. The existing call __bond_release_one->bond_hw_addr_flush is left in place because there are other call chains that lead to __bond_release_one(), not just ndo_uninit.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
I'm just going from memory here, so I'm probably wrong, but didn't the sync/unsync stuff for HW addresses happen several years after the git transition?
Yes, you're right. The bonding driver was converted to dev addr list functions in commit 303d1cbf610e ("bonding: Convert hw addr handling to sync/unsync, support ucast addresses", v3.11-rc1). However, the problem fixed by this patch ("addresses being leftover on former bond slaves after a bond has been deleted") was present before the conversion (at least for mc, uc was not handled at all before the conversion). Since the problem was not introduced by 303d1cbf610e, that's why I chose 1da177e4c3f4 for the Fixes tag. Does that make sense?