All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: netdev@vger.kernel.org
Cc: 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>,
	Ido Schimmel <idosch@nvidia.com>,
	Tobias Waldekranz <tobias@waldekranz.com>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	UNGLinuxDriver@microchip.com
Subject: [PATCH net-next 01/10] net: dsa: remove workarounds for changing master promisc/allmulti only while up
Date: Wed,  2 Mar 2022 21:14:08 +0200	[thread overview]
Message-ID: <20220302191417.1288145-2-vladimir.oltean@nxp.com> (raw)
In-Reply-To: <20220302191417.1288145-1-vladimir.oltean@nxp.com>

Lennert Buytenhek explains in commit df02c6ff2e39 ("dsa: fix master
interface allmulti/promisc handling"), dated Nov 2008, that changing the
promiscuity of interfaces that are down (here the master) is broken.

This fact regarding promisc/allmulti has changed since commit
b6c40d68ff64 ("net: only invoke dev->change_rx_flags when device is UP")
by Vlad Yasevich, dated Nov 2013.

Therefore, DSA now has unnecessary complexity to handle master state
transitions from down to up. In fact, syncing the unicast and multicast
addresses can happen completely asynchronously to the administrative
state changes.

This change reduces that complexity by effectively fully reverting
commit df02c6ff2e39 ("dsa: fix master interface allmulti/promisc
handling").

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/slave.c | 45 ++++++++-------------------------------------
 1 file changed, 8 insertions(+), 37 deletions(-)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 089616206b11..52d1316368c9 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -81,29 +81,12 @@ static int dsa_slave_open(struct net_device *dev)
 			goto out;
 	}
 
-	if (dev->flags & IFF_ALLMULTI) {
-		err = dev_set_allmulti(master, 1);
-		if (err < 0)
-			goto del_unicast;
-	}
-	if (dev->flags & IFF_PROMISC) {
-		err = dev_set_promiscuity(master, 1);
-		if (err < 0)
-			goto clear_allmulti;
-	}
-
 	err = dsa_port_enable_rt(dp, dev->phydev);
 	if (err)
-		goto clear_promisc;
+		goto del_unicast;
 
 	return 0;
 
-clear_promisc:
-	if (dev->flags & IFF_PROMISC)
-		dev_set_promiscuity(master, -1);
-clear_allmulti:
-	if (dev->flags & IFF_ALLMULTI)
-		dev_set_allmulti(master, -1);
 del_unicast:
 	if (!ether_addr_equal(dev->dev_addr, master->dev_addr))
 		dev_uc_del(master, dev->dev_addr);
@@ -118,13 +101,6 @@ static int dsa_slave_close(struct net_device *dev)
 
 	dsa_port_disable_rt(dp);
 
-	dev_mc_unsync(master, dev);
-	dev_uc_unsync(master, dev);
-	if (dev->flags & IFF_ALLMULTI)
-		dev_set_allmulti(master, -1);
-	if (dev->flags & IFF_PROMISC)
-		dev_set_promiscuity(master, -1);
-
 	if (!ether_addr_equal(dev->dev_addr, master->dev_addr))
 		dev_uc_del(master, dev->dev_addr);
 
@@ -134,14 +110,13 @@ static int dsa_slave_close(struct net_device *dev)
 static void dsa_slave_change_rx_flags(struct net_device *dev, int change)
 {
 	struct net_device *master = dsa_slave_to_master(dev);
-	if (dev->flags & IFF_UP) {
-		if (change & IFF_ALLMULTI)
-			dev_set_allmulti(master,
-					 dev->flags & IFF_ALLMULTI ? 1 : -1);
-		if (change & IFF_PROMISC)
-			dev_set_promiscuity(master,
-					    dev->flags & IFF_PROMISC ? 1 : -1);
-	}
+
+	if (change & IFF_ALLMULTI)
+		dev_set_allmulti(master,
+				 dev->flags & IFF_ALLMULTI ? 1 : -1);
+	if (change & IFF_PROMISC)
+		dev_set_promiscuity(master,
+				    dev->flags & IFF_PROMISC ? 1 : -1);
 }
 
 static void dsa_slave_set_rx_mode(struct net_device *dev)
@@ -161,9 +136,6 @@ static int dsa_slave_set_mac_address(struct net_device *dev, void *a)
 	if (!is_valid_ether_addr(addr->sa_data))
 		return -EADDRNOTAVAIL;
 
-	if (!(dev->flags & IFF_UP))
-		goto out;
-
 	if (!ether_addr_equal(addr->sa_data, master->dev_addr)) {
 		err = dev_uc_add(master, addr->sa_data);
 		if (err < 0)
@@ -173,7 +145,6 @@ static int dsa_slave_set_mac_address(struct net_device *dev, void *a)
 	if (!ether_addr_equal(dev->dev_addr, master->dev_addr))
 		dev_uc_del(master, dev->dev_addr);
 
-out:
 	eth_hw_addr_set(dev, addr->sa_data);
 
 	return 0;
-- 
2.25.1


  reply	other threads:[~2022-03-02 19:15 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-02 19:14 [PATCH net-next 00/10] DSA unicast filtering Vladimir Oltean
2022-03-02 19:14 ` Vladimir Oltean [this message]
2022-03-02 19:14 ` [PATCH net-next 02/10] net: dsa: rename the host FDB and MDB methods to contain the "bridge" namespace Vladimir Oltean
2022-03-02 19:14 ` [PATCH net-next 03/10] net: dsa: install secondary unicast and multicast addresses as host FDB/MDB Vladimir Oltean
2022-03-02 19:14 ` [PATCH net-next 04/10] net: dsa: install the primary unicast MAC address as standalone port host FDB Vladimir Oltean
2022-03-02 19:14 ` [PATCH net-next 05/10] net: dsa: manage flooding on the CPU ports Vladimir Oltean
2022-03-02 19:14 ` [PATCH net-next 06/10] net: dsa: felix: migrate host FDB and MDB entries when changing tag proto Vladimir Oltean
2022-03-02 19:14 ` [PATCH net-next 07/10] net: dsa: felix: migrate flood settings from NPI to tag_8021q CPU port Vladimir Oltean
2022-03-02 19:14 ` [PATCH net-next 08/10] net: dsa: felix: start off with flooding disabled on the " Vladimir Oltean
2022-03-02 19:14 ` [PATCH net-next 09/10] net: dsa: felix: stop clearing CPU flooding in felix_setup_tag_8021q Vladimir Oltean
2022-03-02 19:14 ` [PATCH net-next 10/10] net: mscc: ocelot: accept configuring bridge port flags on the NPI port Vladimir Oltean
2022-03-02 19:30 ` [PATCH net-next 00/10] DSA unicast filtering Florian Fainelli
2022-03-02 22:05   ` Vladimir Oltean
2022-03-03 12:16 ` Alvin Šipraga
2022-03-03 13:18   ` Vladimir Oltean
2022-03-03 14:20     ` Alvin Šipraga
2022-03-03 14:35       ` Vladimir Oltean
2022-03-03 14:48         ` Vladimir Oltean
2022-03-03 15:13         ` Alvin Šipraga
2022-03-03 15:35           ` Vladimir Oltean
2022-03-03 14:20 ` 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=20220302191417.1288145-2-vladimir.oltean@nxp.com \
    --to=vladimir.oltean@nxp.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andrew@lunn.ch \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=idosch@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=tobias@waldekranz.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.