All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: netdev@vger.kernel.org
Cc: Paolo Abeni <pabeni@redhat.com>, Jakub Kicinski <kuba@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Vladimir Oltean <olteanv@gmail.com>
Subject: [PATCH net-next] net: dsa: fix missing host-filtered multicast addresses
Date: Tue, 22 Mar 2022 02:37:01 +0200	[thread overview]
Message-ID: <20220322003701.2056895-1-vladimir.oltean@nxp.com> (raw)

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(-)

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)
-- 
2.25.1


             reply	other threads:[~2022-03-22  0:37 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-22  0:37 Vladimir Oltean [this message]
2022-03-22  2:07 ` [PATCH net-next] net: dsa: fix missing host-filtered multicast addresses Florian Fainelli
2022-03-23  5:30 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220322003701.2056895-1-vladimir.oltean@nxp.com \
    --to=vladimir.oltean@nxp.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=vivien.didelot@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.