diff mbox series

[net-next] net: dsa: don't fast age standalone ports

Message ID 20210808111637.3819465-1-vladimir.oltean@nxp.com
State New
Headers show
Series [net-next] net: dsa: don't fast age standalone ports | expand

Commit Message

Vladimir Oltean Aug. 8, 2021, 11:16 a.m. UTC 
DSA drives the procedure to flush dynamic FDB entries from a port based
on the change of STP state: whenever we go from a state where address
learning is enabled (LEARNING, FORWARDING) to a state where it isn't
(LISTENING, BLOCKING, DISABLED), we need to flush the existing dynamic
entries.

However, there are cases when this is not needed. Internally, when a
DSA switch interface is not under a bridge, DSA still keeps it in the
"FORWARDING" STP state. And when that interface joins a bridge, the
bridge will meticulously iterate that port through all STP states,
starting with BLOCKING and ending with FORWARDING. Because there is a
state transition from the standalone version of FORWARDING into the
temporary BLOCKING bridge port state, DSA calls the fast age procedure.

Since commit 5e38c15856e9 ("net: dsa: configure better brport flags when
ports leave the bridge"), DSA asks standalone ports to disable address
learning. Therefore, there can be no dynamic FDB entries on a standalone
port. Therefore, it does not make sense to flush dynamic FDB entries on
one.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/dsa_priv.h |  2 +-
 net/dsa/port.c     | 20 ++++++++++++--------
 net/dsa/slave.c    |  2 +-
 3 files changed, 14 insertions(+), 10 deletions(-)

Comments

Andrew Lunn Aug. 8, 2021, 3:37 p.m. UTC | #1 
On Sun, Aug 08, 2021 at 02:16:37PM +0300, Vladimir Oltean wrote:
> DSA drives the procedure to flush dynamic FDB entries from a port based
> on the change of STP state: whenever we go from a state where address
> learning is enabled (LEARNING, FORWARDING) to a state where it isn't
> (LISTENING, BLOCKING, DISABLED), we need to flush the existing dynamic
> entries.
> 
> However, there are cases when this is not needed. Internally, when a
> DSA switch interface is not under a bridge, DSA still keeps it in the
> "FORWARDING" STP state. And when that interface joins a bridge, the
> bridge will meticulously iterate that port through all STP states,
> starting with BLOCKING and ending with FORWARDING. Because there is a
> state transition from the standalone version of FORWARDING into the
> temporary BLOCKING bridge port state, DSA calls the fast age procedure.
> 
> Since commit 5e38c15856e9 ("net: dsa: configure better brport flags when
> ports leave the bridge"), DSA asks standalone ports to disable address
> learning. Therefore, there can be no dynamic FDB entries on a standalone
> port. Therefore, it does not make sense to flush dynamic FDB entries on
> one.

Hi Vladimir

Do all DSA drivers actually support disabling learning on a port?  If
there are any which cannot disable learning, we still need the flush
somehow.

	Andrew
Vladimir Oltean Aug. 8, 2021, 8:46 p.m. UTC | #2 
Hi Andrew,

On Sun, Aug 08, 2021 at 05:37:04PM +0200, Andrew Lunn wrote:
> Hi Vladimir
> 
> Do all DSA drivers actually support disabling learning on a port?  If
> there are any which cannot disable learning, we still need the flush
> somehow.

Indeed, I will send a patch to restore the behavior for drivers that
cannot toggle address learning.
diff mbox series

Patch

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 7841b3957516..8dad40b2cf5c 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -199,7 +199,7 @@  static inline struct net_device *dsa_master_find_slave(struct net_device *dev,
 /* port.c */
 void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp,
 			       const struct dsa_device_ops *tag_ops);
-int dsa_port_set_state(struct dsa_port *dp, u8 state);
+int dsa_port_set_state(struct dsa_port *dp, u8 state, bool do_fast_age);
 int dsa_port_enable_rt(struct dsa_port *dp, struct phy_device *phy);
 int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy);
 void dsa_port_disable_rt(struct dsa_port *dp);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 797a3269a964..ef5e08b09bb7 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -30,7 +30,7 @@  static int dsa_port_notify(const struct dsa_port *dp, unsigned long e, void *v)
 	return dsa_tree_notify(dp->ds->dst, e, v);
 }
 
-int dsa_port_set_state(struct dsa_port *dp, u8 state)
+int dsa_port_set_state(struct dsa_port *dp, u8 state, bool do_fast_age)
 {
 	struct dsa_switch *ds = dp->ds;
 	int port = dp->index;
@@ -40,10 +40,13 @@  int dsa_port_set_state(struct dsa_port *dp, u8 state)
 
 	ds->ops->port_stp_state_set(ds, port, state);
 
-	if (ds->ops->port_fast_age) {
+	if (do_fast_age && ds->ops->port_fast_age) {
 		/* Fast age FDB entries or flush appropriate forwarding database
 		 * for the given port, if we are moving it from Learning or
 		 * Forwarding state, to Disabled or Blocking or Listening state.
+		 * Ports that were standalone before the STP state change don't
+		 * need to fast age the FDB, since address learning is off in
+		 * standalone mode.
 		 */
 
 		if ((dp->stp_state == BR_STATE_LEARNING ||
@@ -59,11 +62,12 @@  int dsa_port_set_state(struct dsa_port *dp, u8 state)
 	return 0;
 }
 
-static void dsa_port_set_state_now(struct dsa_port *dp, u8 state)
+static void dsa_port_set_state_now(struct dsa_port *dp, u8 state,
+				   bool do_fast_age)
 {
 	int err;
 
-	err = dsa_port_set_state(dp, state);
+	err = dsa_port_set_state(dp, state, do_fast_age);
 	if (err)
 		pr_err("DSA: failed to set STP state %u (%d)\n", state, err);
 }
@@ -81,7 +85,7 @@  int dsa_port_enable_rt(struct dsa_port *dp, struct phy_device *phy)
 	}
 
 	if (!dp->bridge_dev)
-		dsa_port_set_state_now(dp, BR_STATE_FORWARDING);
+		dsa_port_set_state_now(dp, BR_STATE_FORWARDING, false);
 
 	if (dp->pl)
 		phylink_start(dp->pl);
@@ -109,7 +113,7 @@  void dsa_port_disable_rt(struct dsa_port *dp)
 		phylink_stop(dp->pl);
 
 	if (!dp->bridge_dev)
-		dsa_port_set_state_now(dp, BR_STATE_DISABLED);
+		dsa_port_set_state_now(dp, BR_STATE_DISABLED, false);
 
 	if (ds->ops->port_disable)
 		ds->ops->port_disable(ds, port);
@@ -178,7 +182,7 @@  static int dsa_port_switchdev_sync_attrs(struct dsa_port *dp,
 	if (err)
 		return err;
 
-	err = dsa_port_set_state(dp, br_port_get_stp_state(brport_dev));
+	err = dsa_port_set_state(dp, br_port_get_stp_state(brport_dev), false);
 	if (err && err != -EOPNOTSUPP)
 		return err;
 
@@ -211,7 +215,7 @@  static void dsa_port_switchdev_unsync_attrs(struct dsa_port *dp)
 	/* Port left the bridge, put in BR_STATE_DISABLED by the bridge layer,
 	 * so allow it to be in BR_STATE_FORWARDING to be kept functional
 	 */
-	dsa_port_set_state_now(dp, BR_STATE_FORWARDING);
+	dsa_port_set_state_now(dp, BR_STATE_FORWARDING, true);
 
 	/* VLAN filtering is handled by dsa_switch_bridge_leave */
 
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 022174635bc1..acf73db5cafc 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -286,7 +286,7 @@  static int dsa_slave_port_attr_set(struct net_device *dev, const void *ctx,
 		if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
 			return -EOPNOTSUPP;
 
-		ret = dsa_port_set_state(dp, attr->u.stp_state);
+		ret = dsa_port_set_state(dp, attr->u.stp_state, true);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
 		if (!dsa_port_offloads_bridge(dp, attr->orig_dev))