[PATCH net] net: dsa: microchip: make learning configurable and keep it off while standalone

[PATCH net] net: dsa: microchip: make learning configurable and keep it off while standalone

[Vladimir Oltean]

Address learning should initially be turned off by the driver for port
operation in standalone mode, then the DSA core handles changes to it
via ds->ops->port_bridge_flags().

Leaving address learning enabled while ports are standalone breaks any
kind of communication which involves port B receiving what port A has
sent.

This fixes a design oversight in the ksz9477 and ksz8795 drivers, which
unconditionally leave address learning enabled.

Link: https://lore.kernel.org/netdev/CAFZh4h-JVWt80CrQWkFji7tZJahMfOToUJQgKS5s0_=9zzpvYQ@mail.gmail.com/
Reported-by: Brian Hutchinson <b.hutchman@gmail.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
This is compile-tested only, but the equivalent change was tested by
Brian on a 5.10 kernel and it worked.

I'm targeting just the "net" tree here (today 5.19 release candidates),
but this needs to be fixed separately for net-next and essentially every
other stable branch, since we will be lacking the port_bridge_flags
callbacks, and there has been a lot of general refactoring in the
microchip driver.

Jakub, I wonder if I should let you do the merge resolution between
"net" and "net-next", or should I just resend against "net-next" and
keep this patch as one of the stable backports?

 drivers/net/dsa/microchip/ksz8795.c    |  2 ++
 drivers/net/dsa/microchip/ksz9477.c    |  2 ++
 drivers/net/dsa/microchip/ksz_common.c | 35 +++++++++++++++++++++++++-
 drivers/net/dsa/microchip/ksz_common.h |  7 ++++++
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 12a599d5e61a..17930858cacf 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -1408,6 +1408,8 @@ static const struct dsa_switch_ops ksz8_switch_ops = {
 	.port_bridge_join	= ksz_port_bridge_join,
 	.port_bridge_leave	= ksz_port_bridge_leave,
 	.port_stp_state_set	= ksz8_port_stp_state_set,
+	.port_pre_bridge_flags	= ksz_port_pre_bridge_flags,
+	.port_bridge_flags	= ksz_port_bridge_flags,
 	.port_fast_age		= ksz_port_fast_age,
 	.port_vlan_filtering	= ksz8_port_vlan_filtering,
 	.port_vlan_add		= ksz8_port_vlan_add,
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index ab40b700cf1a..811ba0a44ae8 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1339,6 +1339,8 @@ static const struct dsa_switch_ops ksz9477_switch_ops = {
 	.port_bridge_join	= ksz_port_bridge_join,
 	.port_bridge_leave	= ksz_port_bridge_leave,
 	.port_stp_state_set	= ksz9477_port_stp_state_set,
+	.port_pre_bridge_flags	= ksz_port_pre_bridge_flags,
+	.port_bridge_flags	= ksz_port_bridge_flags,
 	.port_fast_age		= ksz_port_fast_age,
 	.port_vlan_filtering	= ksz9477_port_vlan_filtering,
 	.port_vlan_add		= ksz9477_port_vlan_add,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 92a500e1ccd2..9bef51af49a0 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -900,6 +900,8 @@ void ksz_port_stp_state_set(struct dsa_switch *ds, int port,
 	ksz_pread8(dev, port, reg, &data);
 	data &= ~(PORT_TX_ENABLE | PORT_RX_ENABLE | PORT_LEARN_DISABLE);
 
+	p = &dev->ports[port];
+
 	switch (state) {
 	case BR_STATE_DISABLED:
 		data |= PORT_LEARN_DISABLE;
@@ -909,9 +911,13 @@ void ksz_port_stp_state_set(struct dsa_switch *ds, int port,
 		break;
 	case BR_STATE_LEARNING:
 		data |= PORT_RX_ENABLE;
+		if (!p->learning)
+			data |= PORT_LEARN_DISABLE;
 		break;
 	case BR_STATE_FORWARDING:
 		data |= (PORT_TX_ENABLE | PORT_RX_ENABLE);
+		if (!p->learning)
+			data |= PORT_LEARN_DISABLE;
 		break;
 	case BR_STATE_BLOCKING:
 		data |= PORT_LEARN_DISABLE;
@@ -923,13 +929,40 @@ void ksz_port_stp_state_set(struct dsa_switch *ds, int port,
 
 	ksz_pwrite8(dev, port, reg, data);
 
-	p = &dev->ports[port];
 	p->stp_state = state;
 
 	ksz_update_port_member(dev, port);
 }
 EXPORT_SYMBOL_GPL(ksz_port_stp_state_set);
 
+int ksz_port_pre_bridge_flags(struct dsa_switch *ds, int port,
+			      struct switchdev_brport_flags flags,
+			      struct netlink_ext_ack *extack)
+{
+	if (flags.mask & ~BR_LEARNING)
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ksz_port_pre_bridge_flags);
+
+int ksz_port_bridge_flags(struct dsa_switch *ds, int port,
+			  struct switchdev_brport_flags flags,
+			  struct netlink_ext_ack *extack)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_port *p = &dev->ports[port];
+
+	if (flags.mask & BR_LEARNING) {
+		p->learning = !!(flags.val & BR_LEARNING);
+
+		ds->ops->port_stp_state_set(ds, port, p->stp_state);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ksz_port_bridge_flags);
+
 struct ksz_device *ksz_switch_alloc(struct device *base, void *priv)
 {
 	struct dsa_switch *ds;
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 8500eaedad67..a0f1775a9960 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -54,6 +54,7 @@ struct ksz_chip_data {
 
 struct ksz_port {
 	bool remove_tag;		/* Remove Tag flag set, for ksz8795 only */
+	bool learning;
 	int stp_state;
 	struct phy_device phydev;
 
@@ -219,6 +220,12 @@ void ksz_port_bridge_leave(struct dsa_switch *ds, int port,
 			   struct dsa_bridge bridge);
 void ksz_port_stp_state_set(struct dsa_switch *ds, int port,
 			    u8 state, int reg);
+int ksz_port_pre_bridge_flags(struct dsa_switch *ds, int port,
+			      struct switchdev_brport_flags flags,
+			      struct netlink_ext_ack *extack);
+int ksz_port_bridge_flags(struct dsa_switch *ds, int port,
+			  struct switchdev_brport_flags flags,
+			  struct netlink_ext_ack *extack);
 void ksz_port_fast_age(struct dsa_switch *ds, int port);
 int ksz_port_fdb_dump(struct dsa_switch *ds, int port, dsa_fdb_dump_cb_t *cb,
 		      void *data);
-- 
2.34.1

Re: [PATCH net] net: dsa: microchip: make learning configurable and keep it off while standalone

[Jakub Kicinski]

On Tue,  2 Aug 2022 03:26:36 +0300 Vladimir Oltean wrote:
> This is compile-tested only, but the equivalent change was tested by
> Brian on a 5.10 kernel and it worked.
> 
> I'm targeting just the "net" tree here (today 5.19 release candidates),
> but this needs to be fixed separately for net-next and essentially every
> other stable branch, since we will be lacking the port_bridge_flags
> callbacks, and there has been a lot of general refactoring in the
> microchip driver.
> 
> Jakub, I wonder if I should let you do the merge resolution between
> "net" and "net-next", or should I just resend against "net-next" and
> keep this patch as one of the stable backports?

I missed this question, sorry. Let's do the latter.