| Message ID | 20220322003701.2056895-1-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5077e2c8cf4d6b22a95a6c54a917f764e8887978 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dsa: fix missing host-filtered multicast addresses | expand |
On 3/21/2022 5:37 PM, Vladimir Oltean wrote: > DSA ports are stacked devices, so they use dev_mc_add() to sync their > address list to their lower interface (DSA master). But they are also > hardware devices, so they program those addresses to hardware using the > __dev_mc_add() sync and unsync callbacks. > > Unfortunately both cannot work at the same time, and it seems that the > multicast addresses which are already present on the DSA master, like > 33:33:00:00:00:01 (added by addrconf.c as in6addr_linklocal_allnodes) > are synced to the master via dev_mc_sync(), but not to hardware by > __dev_mc_sync(). > > This happens because both the dev_mc_sync() -> __hw_addr_sync_one() > code path, as well as __dev_mc_sync() -> __hw_addr_sync_dev(), operate > on the same variable: ha->sync_cnt, in a way that causes the "sync" > method (dsa_slave_sync_mc) to no longer be called. > > To fix the issue we need to work with the API in the way in which it was > intended to be used, and therefore, call dev_uc_add() and friends for > each individual hardware address, from the sync and unsync callbacks. > > Fixes: 5e8a1e03aa4d ("net: dsa: install secondary unicast and multicast addresses as host FDB/MDB") > Link: https://lore.kernel.org/netdev/20220321163213.lrn5sk7m6grighbl@skbuf/ > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Hello: This patch was applied to netdev/net-next.git (master) by Jakub Kicinski <kuba@kernel.org>: On Tue, 22 Mar 2022 02:37:01 +0200 you wrote: > DSA ports are stacked devices, so they use dev_mc_add() to sync their > address list to their lower interface (DSA master). But they are also > hardware devices, so they program those addresses to hardware using the > __dev_mc_add() sync and unsync callbacks. > > Unfortunately both cannot work at the same time, and it seems that the > multicast addresses which are already present on the DSA master, like > 33:33:00:00:00:01 (added by addrconf.c as in6addr_linklocal_allnodes) > are synced to the master via dev_mc_sync(), but not to hardware by > __dev_mc_sync(). > > [...] Here is the summary with links: - [net-next] net: dsa: fix missing host-filtered multicast addresses https://git.kernel.org/netdev/net-next/c/5077e2c8cf4d You are awesome, thank you!
diff --git a/net/dsa/slave.c b/net/dsa/slave.c index d1a3be158d8d..41c69a6e7854 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -111,24 +111,56 @@ static int dsa_slave_schedule_standalone_work(struct net_device *dev, static int dsa_slave_sync_uc(struct net_device *dev, const unsigned char *addr) { + struct net_device *master = dsa_slave_to_master(dev); + struct dsa_port *dp = dsa_slave_to_port(dev); + + dev_uc_add(master, addr); + + if (!dsa_switch_supports_uc_filtering(dp->ds)) + return 0; + return dsa_slave_schedule_standalone_work(dev, DSA_UC_ADD, addr, 0); } static int dsa_slave_unsync_uc(struct net_device *dev, const unsigned char *addr) { + struct net_device *master = dsa_slave_to_master(dev); + struct dsa_port *dp = dsa_slave_to_port(dev); + + dev_uc_del(master, addr); + + if (!dsa_switch_supports_uc_filtering(dp->ds)) + return 0; + return dsa_slave_schedule_standalone_work(dev, DSA_UC_DEL, addr, 0); } static int dsa_slave_sync_mc(struct net_device *dev, const unsigned char *addr) { + struct net_device *master = dsa_slave_to_master(dev); + struct dsa_port *dp = dsa_slave_to_port(dev); + + dev_mc_add(master, addr); + + if (!dsa_switch_supports_mc_filtering(dp->ds)) + return 0; + return dsa_slave_schedule_standalone_work(dev, DSA_MC_ADD, addr, 0); } static int dsa_slave_unsync_mc(struct net_device *dev, const unsigned char *addr) { + struct net_device *master = dsa_slave_to_master(dev); + struct dsa_port *dp = dsa_slave_to_port(dev); + + dev_mc_del(master, addr); + + if (!dsa_switch_supports_mc_filtering(dp->ds)) + return 0; + return dsa_slave_schedule_standalone_work(dev, DSA_MC_DEL, addr, 0); } @@ -283,16 +315,8 @@ static void dsa_slave_change_rx_flags(struct net_device *dev, int change) static void dsa_slave_set_rx_mode(struct net_device *dev) { - struct net_device *master = dsa_slave_to_master(dev); - struct dsa_port *dp = dsa_slave_to_port(dev); - struct dsa_switch *ds = dp->ds; - - dev_mc_sync(master, dev); - dev_uc_sync(master, dev); - if (dsa_switch_supports_mc_filtering(ds)) - __dev_mc_sync(dev, dsa_slave_sync_mc, dsa_slave_unsync_mc); - if (dsa_switch_supports_uc_filtering(ds)) - __dev_uc_sync(dev, dsa_slave_sync_uc, dsa_slave_unsync_uc); + __dev_mc_sync(dev, dsa_slave_sync_mc, dsa_slave_unsync_mc); + __dev_uc_sync(dev, dsa_slave_sync_uc, dsa_slave_unsync_uc); } static int dsa_slave_set_mac_address(struct net_device *dev, void *a)
DSA ports are stacked devices, so they use dev_mc_add() to sync their address list to their lower interface (DSA master). But they are also hardware devices, so they program those addresses to hardware using the __dev_mc_add() sync and unsync callbacks. Unfortunately both cannot work at the same time, and it seems that the multicast addresses which are already present on the DSA master, like 33:33:00:00:00:01 (added by addrconf.c as in6addr_linklocal_allnodes) are synced to the master via dev_mc_sync(), but not to hardware by __dev_mc_sync(). This happens because both the dev_mc_sync() -> __hw_addr_sync_one() code path, as well as __dev_mc_sync() -> __hw_addr_sync_dev(), operate on the same variable: ha->sync_cnt, in a way that causes the "sync" method (dsa_slave_sync_mc) to no longer be called. To fix the issue we need to work with the API in the way in which it was intended to be used, and therefore, call dev_uc_add() and friends for each individual hardware address, from the sync and unsync callbacks. Fixes: 5e8a1e03aa4d ("net: dsa: install secondary unicast and multicast addresses as host FDB/MDB") Link: https://lore.kernel.org/netdev/20220321163213.lrn5sk7m6grighbl@skbuf/ Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- net/dsa/slave.c | 44 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-)