On 2022-08-30 20:25 -0700, Jay Vosburgh wrote:
> Benjamin Poirier <bpoirier@xxxxxxxxxx> 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?
