netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] net: dsa: qca8k: cleanup and port isolation
@ 2024-06-20 17:25 Matthias Schiffer
  2024-06-20 17:25 ` [PATCH net-next 1/3] net: dsa: qca8k: do not write port mask twice in bridge join/leave Matthias Schiffer
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Matthias Schiffer @ 2024-06-20 17:25 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Christian Marangi, netdev, linux-kernel, Matthias Schiffer

A small cleanup patch, and basically the same changes that were just
accepted for mt7530 to implement port isolation.

Matthias Schiffer (3):
  net: dsa: qca8k: do not write port mask twice in bridge join/leave
  net: dsa: qca8k: factor out bridge join/leave logic
  net: dsa: qca8k: add support for bridge port isolation

 drivers/net/dsa/qca/qca8k-common.c | 118 +++++++++++++++++------------
 drivers/net/dsa/qca/qca8k.h        |   1 +
 2 files changed, 70 insertions(+), 49 deletions(-)

-- 
2.45.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH net-next 1/3] net: dsa: qca8k: do not write port mask twice in bridge join/leave
  2024-06-20 17:25 [PATCH net-next 0/3] net: dsa: qca8k: cleanup and port isolation Matthias Schiffer
@ 2024-06-20 17:25 ` Matthias Schiffer
  2024-06-21 10:48   ` Wojciech Drewek
  2024-06-20 17:25 ` [PATCH net-next 2/3] net: dsa: qca8k: factor out bridge join/leave logic Matthias Schiffer
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Matthias Schiffer @ 2024-06-20 17:25 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Christian Marangi, netdev, linux-kernel, Matthias Schiffer

qca8k_port_bridge_join() set QCA8K_PORT_LOOKUP_CTRL() for i == port twice,
once in the loop handling all other port's masks, and finally at the end
with the accumulated port_mask.

The first time it would incorrectly set the port's own bit in the mask,
only to correct the mistake a moment later. qca8k_port_bridge_leave() had
the same issue, but here the regmap_clear_bits() was a no-op rather than
setting an unintended value.

Remove the duplicate assignment by skipping the whole loop iteration for
i == port. The unintended bit setting doesn't seem to have any negative
effects (even when not reverted right away), so the change is submitted
as a simple cleanup rather than a fix.

Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
---
 drivers/net/dsa/qca/qca8k-common.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c
index 7f80035c5441..b33df84070d3 100644
--- a/drivers/net/dsa/qca/qca8k-common.c
+++ b/drivers/net/dsa/qca/qca8k-common.c
@@ -653,6 +653,8 @@ int qca8k_port_bridge_join(struct dsa_switch *ds, int port,
 	port_mask = BIT(cpu_port);
 
 	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
+		if (i == port)
+			continue;
 		if (dsa_is_cpu_port(ds, i))
 			continue;
 		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
@@ -665,8 +667,7 @@ int qca8k_port_bridge_join(struct dsa_switch *ds, int port,
 				      BIT(port));
 		if (ret)
 			return ret;
-		if (i != port)
-			port_mask |= BIT(i);
+		port_mask |= BIT(i);
 	}
 
 	/* Add all other ports to this ports portvlan mask */
@@ -685,6 +686,8 @@ void qca8k_port_bridge_leave(struct dsa_switch *ds, int port,
 	cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
 
 	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
+		if (i == port)
+			continue;
 		if (dsa_is_cpu_port(ds, i))
 			continue;
 		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 2/3] net: dsa: qca8k: factor out bridge join/leave logic
  2024-06-20 17:25 [PATCH net-next 0/3] net: dsa: qca8k: cleanup and port isolation Matthias Schiffer
  2024-06-20 17:25 ` [PATCH net-next 1/3] net: dsa: qca8k: do not write port mask twice in bridge join/leave Matthias Schiffer
@ 2024-06-20 17:25 ` Matthias Schiffer
  2024-06-21 10:51   ` Wojciech Drewek
  2024-06-21 16:48   ` Vladimir Oltean
  2024-06-20 17:25 ` [PATCH net-next 3/3] net: dsa: qca8k: add support for bridge port isolation Matthias Schiffer
  2024-06-21 11:30 ` [PATCH net-next 0/3] net: dsa: qca8k: cleanup and " patchwork-bot+netdevbpf
  3 siblings, 2 replies; 10+ messages in thread
From: Matthias Schiffer @ 2024-06-20 17:25 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Christian Marangi, netdev, linux-kernel, Matthias Schiffer

Most of the logic in qca8k_port_bridge_join() and qca8k_port_bridge_leave()
is the same. Refactor to reduce duplication and prepare for reusing the
code for implementing bridge port isolation.

dsa_port_offloads_bridge_dev() is used instead of
dsa_port_offloads_bridge(), passing the bridge in as a struct netdevice *,
as we won't have a struct dsa_bridge in qca8k_port_bridge_flags().

The error handling is changed slightly in the bridge leave case,
returning early and emitting an error message when a regmap access fails.
This shouldn't matter in practice, as there isn't much we can do if
communication with the switch breaks down in the middle of reconfiguration.

Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
---
 drivers/net/dsa/qca/qca8k-common.c | 101 ++++++++++++++---------------
 1 file changed, 50 insertions(+), 51 deletions(-)

diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c
index b33df84070d3..09108fa99dbe 100644
--- a/drivers/net/dsa/qca/qca8k-common.c
+++ b/drivers/net/dsa/qca/qca8k-common.c
@@ -614,6 +614,49 @@ void qca8k_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
 	qca8k_port_configure_learning(ds, port, learning);
 }
 
+static int qca8k_update_port_member(struct qca8k_priv *priv, int port,
+				    const struct net_device *bridge_dev,
+				    bool join)
+{
+	struct dsa_port *dp = dsa_to_port(priv->ds, port), *other_dp;
+	u32 port_mask = BIT(dp->cpu_dp->index);
+	int i, ret;
+
+	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
+		if (i == port)
+			continue;
+		if (dsa_is_cpu_port(priv->ds, i))
+			continue;
+
+		other_dp = dsa_to_port(priv->ds, i);
+		if (!dsa_port_offloads_bridge_dev(other_dp, bridge_dev))
+			continue;
+
+		/* Add/remove this port to/from the portvlan mask of the other
+		 * ports in the bridge
+		 */
+		if (join) {
+			port_mask |= BIT(i);
+			ret = regmap_set_bits(priv->regmap,
+					      QCA8K_PORT_LOOKUP_CTRL(i),
+					      BIT(port));
+		} else {
+			ret = regmap_clear_bits(priv->regmap,
+						QCA8K_PORT_LOOKUP_CTRL(i),
+						BIT(port));
+		}
+
+		if (ret)
+			return ret;
+	}
+
+	/* Add/remove all other ports to/from this port's portvlan mask */
+	ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(port),
+			QCA8K_PORT_LOOKUP_MEMBER, port_mask);
+
+	return ret;
+}
+
 int qca8k_port_pre_bridge_flags(struct dsa_switch *ds, int port,
 				struct switchdev_brport_flags flags,
 				struct netlink_ext_ack *extack)
@@ -646,65 +689,21 @@ int qca8k_port_bridge_join(struct dsa_switch *ds, int port,
 			   struct netlink_ext_ack *extack)
 {
 	struct qca8k_priv *priv = ds->priv;
-	int port_mask, cpu_port;
-	int i, ret;
-
-	cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
-	port_mask = BIT(cpu_port);
 
-	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
-		if (i == port)
-			continue;
-		if (dsa_is_cpu_port(ds, i))
-			continue;
-		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
-			continue;
-		/* Add this port to the portvlan mask of the other ports
-		 * in the bridge
-		 */
-		ret = regmap_set_bits(priv->regmap,
-				      QCA8K_PORT_LOOKUP_CTRL(i),
-				      BIT(port));
-		if (ret)
-			return ret;
-		port_mask |= BIT(i);
-	}
-
-	/* Add all other ports to this ports portvlan mask */
-	ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(port),
-			QCA8K_PORT_LOOKUP_MEMBER, port_mask);
-
-	return ret;
+	return qca8k_update_port_member(priv, port, bridge.dev, true);
 }
 
 void qca8k_port_bridge_leave(struct dsa_switch *ds, int port,
 			     struct dsa_bridge bridge)
 {
 	struct qca8k_priv *priv = ds->priv;
-	int cpu_port, i;
-
-	cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
+	int err;
 
-	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
-		if (i == port)
-			continue;
-		if (dsa_is_cpu_port(ds, i))
-			continue;
-		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
-			continue;
-		/* Remove this port to the portvlan mask of the other ports
-		 * in the bridge
-		 */
-		regmap_clear_bits(priv->regmap,
-				  QCA8K_PORT_LOOKUP_CTRL(i),
-				  BIT(port));
-	}
-
-	/* Set the cpu port to be the only one in the portvlan mask of
-	 * this port
-	 */
-	qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(port),
-		  QCA8K_PORT_LOOKUP_MEMBER, BIT(cpu_port));
+	err = qca8k_update_port_member(priv, port, bridge.dev, false);
+	if (err)
+		dev_err(priv->dev,
+			"Failed to update switch config for bridge leave: %d\n",
+			err);
 }
 
 void qca8k_port_fast_age(struct dsa_switch *ds, int port)
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 3/3] net: dsa: qca8k: add support for bridge port isolation
  2024-06-20 17:25 [PATCH net-next 0/3] net: dsa: qca8k: cleanup and port isolation Matthias Schiffer
  2024-06-20 17:25 ` [PATCH net-next 1/3] net: dsa: qca8k: do not write port mask twice in bridge join/leave Matthias Schiffer
  2024-06-20 17:25 ` [PATCH net-next 2/3] net: dsa: qca8k: factor out bridge join/leave logic Matthias Schiffer
@ 2024-06-20 17:25 ` Matthias Schiffer
  2024-06-21 10:54   ` Wojciech Drewek
  2024-06-21 16:46   ` Vladimir Oltean
  2024-06-21 11:30 ` [PATCH net-next 0/3] net: dsa: qca8k: cleanup and " patchwork-bot+netdevbpf
  3 siblings, 2 replies; 10+ messages in thread
From: Matthias Schiffer @ 2024-06-20 17:25 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Christian Marangi, netdev, linux-kernel, Matthias Schiffer

Remove a pair of ports from the port matrix when both ports have the
isolated flag set.

Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
---
 drivers/net/dsa/qca/qca8k-common.c | 22 ++++++++++++++++++++--
 drivers/net/dsa/qca/qca8k.h        |  1 +
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c
index 09108fa99dbe..560c74c4ac3d 100644
--- a/drivers/net/dsa/qca/qca8k-common.c
+++ b/drivers/net/dsa/qca/qca8k-common.c
@@ -618,6 +618,7 @@ static int qca8k_update_port_member(struct qca8k_priv *priv, int port,
 				    const struct net_device *bridge_dev,
 				    bool join)
 {
+	bool isolated = !!(priv->port_isolated_map & BIT(port)), other_isolated;
 	struct dsa_port *dp = dsa_to_port(priv->ds, port), *other_dp;
 	u32 port_mask = BIT(dp->cpu_dp->index);
 	int i, ret;
@@ -632,10 +633,12 @@ static int qca8k_update_port_member(struct qca8k_priv *priv, int port,
 		if (!dsa_port_offloads_bridge_dev(other_dp, bridge_dev))
 			continue;
 
+		other_isolated = !!(priv->port_isolated_map & BIT(i));
+
 		/* Add/remove this port to/from the portvlan mask of the other
 		 * ports in the bridge
 		 */
-		if (join) {
+		if (join && !(isolated && other_isolated)) {
 			port_mask |= BIT(i);
 			ret = regmap_set_bits(priv->regmap,
 					      QCA8K_PORT_LOOKUP_CTRL(i),
@@ -661,7 +664,7 @@ int qca8k_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)
+	if (flags.mask & ~(BR_LEARNING | BR_ISOLATED))
 		return -EINVAL;
 
 	return 0;
@@ -671,6 +674,7 @@ int qca8k_port_bridge_flags(struct dsa_switch *ds, int port,
 			    struct switchdev_brport_flags flags,
 			    struct netlink_ext_ack *extack)
 {
+	struct qca8k_priv *priv = ds->priv;
 	int ret;
 
 	if (flags.mask & BR_LEARNING) {
@@ -680,6 +684,20 @@ int qca8k_port_bridge_flags(struct dsa_switch *ds, int port,
 			return ret;
 	}
 
+	if (flags.mask & BR_ISOLATED) {
+		struct dsa_port *dp = dsa_to_port(ds, port);
+		struct net_device *bridge_dev = dsa_port_bridge_dev_get(dp);
+
+		if (flags.val & BR_ISOLATED)
+			priv->port_isolated_map |= BIT(port);
+		else
+			priv->port_isolated_map &= ~BIT(port);
+
+		ret = qca8k_update_port_member(priv, port, bridge_dev, true);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
index 2184d8d2d5a9..3664a2e2f1f6 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h
@@ -451,6 +451,7 @@ struct qca8k_priv {
 	 * Bit 1: port enabled. Bit 0: port disabled.
 	 */
 	u8 port_enabled_map;
+	u8 port_isolated_map;
 	struct qca8k_ports_config ports_config;
 	struct regmap *regmap;
 	struct mii_bus *bus;
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 1/3] net: dsa: qca8k: do not write port mask twice in bridge join/leave
  2024-06-20 17:25 ` [PATCH net-next 1/3] net: dsa: qca8k: do not write port mask twice in bridge join/leave Matthias Schiffer
@ 2024-06-21 10:48   ` Wojciech Drewek
  0 siblings, 0 replies; 10+ messages in thread
From: Wojciech Drewek @ 2024-06-21 10:48 UTC (permalink / raw)
  To: Matthias Schiffer, Andrew Lunn, Florian Fainelli, Vladimir Oltean
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Christian Marangi, netdev, linux-kernel



On 20.06.2024 19:25, Matthias Schiffer wrote:
> qca8k_port_bridge_join() set QCA8K_PORT_LOOKUP_CTRL() for i == port twice,
> once in the loop handling all other port's masks, and finally at the end
> with the accumulated port_mask.
> 
> The first time it would incorrectly set the port's own bit in the mask,
> only to correct the mistake a moment later. qca8k_port_bridge_leave() had
> the same issue, but here the regmap_clear_bits() was a no-op rather than
> setting an unintended value.
> 
> Remove the duplicate assignment by skipping the whole loop iteration for
> i == port. The unintended bit setting doesn't seem to have any negative
> effects (even when not reverted right away), so the change is submitted
> as a simple cleanup rather than a fix.
> 
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> ---

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

>  drivers/net/dsa/qca/qca8k-common.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c
> index 7f80035c5441..b33df84070d3 100644
> --- a/drivers/net/dsa/qca/qca8k-common.c
> +++ b/drivers/net/dsa/qca/qca8k-common.c
> @@ -653,6 +653,8 @@ int qca8k_port_bridge_join(struct dsa_switch *ds, int port,
>  	port_mask = BIT(cpu_port);
>  
>  	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
> +		if (i == port)
> +			continue;
>  		if (dsa_is_cpu_port(ds, i))
>  			continue;
>  		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))
> @@ -665,8 +667,7 @@ int qca8k_port_bridge_join(struct dsa_switch *ds, int port,
>  				      BIT(port));
>  		if (ret)
>  			return ret;
> -		if (i != port)
> -			port_mask |= BIT(i);
> +		port_mask |= BIT(i);
>  	}
>  
>  	/* Add all other ports to this ports portvlan mask */
> @@ -685,6 +686,8 @@ void qca8k_port_bridge_leave(struct dsa_switch *ds, int port,
>  	cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
>  
>  	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
> +		if (i == port)
> +			continue;
>  		if (dsa_is_cpu_port(ds, i))
>  			continue;
>  		if (!dsa_port_offloads_bridge(dsa_to_port(ds, i), &bridge))

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 2/3] net: dsa: qca8k: factor out bridge join/leave logic
  2024-06-20 17:25 ` [PATCH net-next 2/3] net: dsa: qca8k: factor out bridge join/leave logic Matthias Schiffer
@ 2024-06-21 10:51   ` Wojciech Drewek
  2024-06-21 16:48   ` Vladimir Oltean
  1 sibling, 0 replies; 10+ messages in thread
From: Wojciech Drewek @ 2024-06-21 10:51 UTC (permalink / raw)
  To: Matthias Schiffer, Andrew Lunn, Florian Fainelli, Vladimir Oltean
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Christian Marangi, netdev, linux-kernel



On 20.06.2024 19:25, Matthias Schiffer wrote:
> Most of the logic in qca8k_port_bridge_join() and qca8k_port_bridge_leave()
> is the same. Refactor to reduce duplication and prepare for reusing the
> code for implementing bridge port isolation.
> 
> dsa_port_offloads_bridge_dev() is used instead of
> dsa_port_offloads_bridge(), passing the bridge in as a struct netdevice *,
> as we won't have a struct dsa_bridge in qca8k_port_bridge_flags().
> 
> The error handling is changed slightly in the bridge leave case,
> returning early and emitting an error message when a regmap access fails.
> This shouldn't matter in practice, as there isn't much we can do if
> communication with the switch breaks down in the middle of reconfiguration.
> 
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> ---

One nit, other than that:
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

>  drivers/net/dsa/qca/qca8k-common.c | 101 ++++++++++++++---------------
>  1 file changed, 50 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c
> index b33df84070d3..09108fa99dbe 100644
> --- a/drivers/net/dsa/qca/qca8k-common.c
> +++ b/drivers/net/dsa/qca/qca8k-common.c
> @@ -614,6 +614,49 @@ void qca8k_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
>  	qca8k_port_configure_learning(ds, port, learning);
>  }
>  
> +static int qca8k_update_port_member(struct qca8k_priv *priv, int port,
> +				    const struct net_device *bridge_dev,
> +				    bool join)
> +{
> +	struct dsa_port *dp = dsa_to_port(priv->ds, port), *other_dp;
> +	u32 port_mask = BIT(dp->cpu_dp->index);
> +	int i, ret;
> +
> +	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
> +		if (i == port)
> +			continue;
> +		if (dsa_is_cpu_port(priv->ds, i))
> +			continue;
> +
> +		other_dp = dsa_to_port(priv->ds, i);
> +		if (!dsa_port_offloads_bridge_dev(other_dp, bridge_dev))
> +			continue;
> +
> +		/* Add/remove this port to/from the portvlan mask of the other
> +		 * ports in the bridge
> +		 */
> +		if (join) {
> +			port_mask |= BIT(i);
> +			ret = regmap_set_bits(priv->regmap,
> +					      QCA8K_PORT_LOOKUP_CTRL(i),
> +					      BIT(port));
> +		} else {
> +			ret = regmap_clear_bits(priv->regmap,
> +						QCA8K_PORT_LOOKUP_CTRL(i),
> +						BIT(port));
> +		}
> +
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* Add/remove all other ports to/from this port's portvlan mask */
> +	ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(port),
> +			QCA8K_PORT_LOOKUP_MEMBER, port_mask);
> +
> +	return ret;

just return qca8k_rmw(...);

> +}
> +

<...>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 3/3] net: dsa: qca8k: add support for bridge port isolation
  2024-06-20 17:25 ` [PATCH net-next 3/3] net: dsa: qca8k: add support for bridge port isolation Matthias Schiffer
@ 2024-06-21 10:54   ` Wojciech Drewek
  2024-06-21 16:46   ` Vladimir Oltean
  1 sibling, 0 replies; 10+ messages in thread
From: Wojciech Drewek @ 2024-06-21 10:54 UTC (permalink / raw)
  To: Matthias Schiffer, Andrew Lunn, Florian Fainelli, Vladimir Oltean
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Christian Marangi, netdev, linux-kernel



On 20.06.2024 19:25, Matthias Schiffer wrote:
> Remove a pair of ports from the port matrix when both ports have the
> isolated flag set.
> 
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> ---

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

>  drivers/net/dsa/qca/qca8k-common.c | 22 ++++++++++++++++++++--
>  drivers/net/dsa/qca/qca8k.h        |  1 +
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c
> index 09108fa99dbe..560c74c4ac3d 100644
> --- a/drivers/net/dsa/qca/qca8k-common.c
> +++ b/drivers/net/dsa/qca/qca8k-common.c
> @@ -618,6 +618,7 @@ static int qca8k_update_port_member(struct qca8k_priv *priv, int port,
>  				    const struct net_device *bridge_dev,
>  				    bool join)
>  {
> +	bool isolated = !!(priv->port_isolated_map & BIT(port)), other_isolated;
>  	struct dsa_port *dp = dsa_to_port(priv->ds, port), *other_dp;
>  	u32 port_mask = BIT(dp->cpu_dp->index);
>  	int i, ret;
> @@ -632,10 +633,12 @@ static int qca8k_update_port_member(struct qca8k_priv *priv, int port,
>  		if (!dsa_port_offloads_bridge_dev(other_dp, bridge_dev))
>  			continue;
>  
> +		other_isolated = !!(priv->port_isolated_map & BIT(i));
> +
>  		/* Add/remove this port to/from the portvlan mask of the other
>  		 * ports in the bridge
>  		 */
> -		if (join) {
> +		if (join && !(isolated && other_isolated)) {
>  			port_mask |= BIT(i);
>  			ret = regmap_set_bits(priv->regmap,
>  					      QCA8K_PORT_LOOKUP_CTRL(i),
> @@ -661,7 +664,7 @@ int qca8k_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)
> +	if (flags.mask & ~(BR_LEARNING | BR_ISOLATED))
>  		return -EINVAL;
>  
>  	return 0;
> @@ -671,6 +674,7 @@ int qca8k_port_bridge_flags(struct dsa_switch *ds, int port,
>  			    struct switchdev_brport_flags flags,
>  			    struct netlink_ext_ack *extack)
>  {
> +	struct qca8k_priv *priv = ds->priv;
>  	int ret;
>  
>  	if (flags.mask & BR_LEARNING) {
> @@ -680,6 +684,20 @@ int qca8k_port_bridge_flags(struct dsa_switch *ds, int port,
>  			return ret;
>  	}
>  
> +	if (flags.mask & BR_ISOLATED) {
> +		struct dsa_port *dp = dsa_to_port(ds, port);
> +		struct net_device *bridge_dev = dsa_port_bridge_dev_get(dp);
> +
> +		if (flags.val & BR_ISOLATED)
> +			priv->port_isolated_map |= BIT(port);
> +		else
> +			priv->port_isolated_map &= ~BIT(port);
> +
> +		ret = qca8k_update_port_member(priv, port, bridge_dev, true);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
> index 2184d8d2d5a9..3664a2e2f1f6 100644
> --- a/drivers/net/dsa/qca/qca8k.h
> +++ b/drivers/net/dsa/qca/qca8k.h
> @@ -451,6 +451,7 @@ struct qca8k_priv {
>  	 * Bit 1: port enabled. Bit 0: port disabled.
>  	 */
>  	u8 port_enabled_map;
> +	u8 port_isolated_map;
>  	struct qca8k_ports_config ports_config;
>  	struct regmap *regmap;
>  	struct mii_bus *bus;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 0/3] net: dsa: qca8k: cleanup and port isolation
  2024-06-20 17:25 [PATCH net-next 0/3] net: dsa: qca8k: cleanup and port isolation Matthias Schiffer
                   ` (2 preceding siblings ...)
  2024-06-20 17:25 ` [PATCH net-next 3/3] net: dsa: qca8k: add support for bridge port isolation Matthias Schiffer
@ 2024-06-21 11:30 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-06-21 11:30 UTC (permalink / raw)
  To: Matthias Schiffer
  Cc: andrew, f.fainelli, olteanv, davem, edumazet, kuba, pabeni,
	ansuelsmth, netdev, linux-kernel

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu, 20 Jun 2024 19:25:47 +0200 you wrote:
> A small cleanup patch, and basically the same changes that were just
> accepted for mt7530 to implement port isolation.
> 
> Matthias Schiffer (3):
>   net: dsa: qca8k: do not write port mask twice in bridge join/leave
>   net: dsa: qca8k: factor out bridge join/leave logic
>   net: dsa: qca8k: add support for bridge port isolation
> 
> [...]

Here is the summary with links:
  - [net-next,1/3] net: dsa: qca8k: do not write port mask twice in bridge join/leave
    https://git.kernel.org/netdev/net-next/c/e85d3e6fea05
  - [net-next,2/3] net: dsa: qca8k: factor out bridge join/leave logic
    https://git.kernel.org/netdev/net-next/c/412e1775f413
  - [net-next,3/3] net: dsa: qca8k: add support for bridge port isolation
    https://git.kernel.org/netdev/net-next/c/422b64025ec1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 3/3] net: dsa: qca8k: add support for bridge port isolation
  2024-06-20 17:25 ` [PATCH net-next 3/3] net: dsa: qca8k: add support for bridge port isolation Matthias Schiffer
  2024-06-21 10:54   ` Wojciech Drewek
@ 2024-06-21 16:46   ` Vladimir Oltean
  1 sibling, 0 replies; 10+ messages in thread
From: Vladimir Oltean @ 2024-06-21 16:46 UTC (permalink / raw)
  To: Matthias Schiffer
  Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Christian Marangi, netdev,
	linux-kernel

On Thu, Jun 20, 2024 at 07:25:50PM +0200, Matthias Schiffer wrote:
> Remove a pair of ports from the port matrix when both ports have the
> isolated flag set.
> 
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> ---

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 2/3] net: dsa: qca8k: factor out bridge join/leave logic
  2024-06-20 17:25 ` [PATCH net-next 2/3] net: dsa: qca8k: factor out bridge join/leave logic Matthias Schiffer
  2024-06-21 10:51   ` Wojciech Drewek
@ 2024-06-21 16:48   ` Vladimir Oltean
  1 sibling, 0 replies; 10+ messages in thread
From: Vladimir Oltean @ 2024-06-21 16:48 UTC (permalink / raw)
  To: Matthias Schiffer
  Cc: Andrew Lunn, Florian Fainelli, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Christian Marangi, netdev,
	linux-kernel

On Thu, Jun 20, 2024 at 07:25:49PM +0200, Matthias Schiffer wrote:
> diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c
> index b33df84070d3..09108fa99dbe 100644
> --- a/drivers/net/dsa/qca/qca8k-common.c
> +++ b/drivers/net/dsa/qca/qca8k-common.c
> @@ -614,6 +614,49 @@ void qca8k_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
>  	qca8k_port_configure_learning(ds, port, learning);
>  }
>  
> +static int qca8k_update_port_member(struct qca8k_priv *priv, int port,
> +				    const struct net_device *bridge_dev,
> +				    bool join)
> +{
> +	struct dsa_port *dp = dsa_to_port(priv->ds, port), *other_dp;
> +	u32 port_mask = BIT(dp->cpu_dp->index);
> +	int i, ret;
> +
> +	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
> +		if (i == port)
> +			continue;
> +		if (dsa_is_cpu_port(priv->ds, i))
> +			continue;
> +
> +		other_dp = dsa_to_port(priv->ds, i);

I would have liked to see less of the "dsa_to_port() in a loop"
antipattern.
https://lore.kernel.org/netdev/20211018152136.2595220-7-vladimir.oltean@nxp.com/T/

I'll send a patch which refactors the new function to use
dsa_switch_for_each_user_port().

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-06-21 16:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-20 17:25 [PATCH net-next 0/3] net: dsa: qca8k: cleanup and port isolation Matthias Schiffer
2024-06-20 17:25 ` [PATCH net-next 1/3] net: dsa: qca8k: do not write port mask twice in bridge join/leave Matthias Schiffer
2024-06-21 10:48   ` Wojciech Drewek
2024-06-20 17:25 ` [PATCH net-next 2/3] net: dsa: qca8k: factor out bridge join/leave logic Matthias Schiffer
2024-06-21 10:51   ` Wojciech Drewek
2024-06-21 16:48   ` Vladimir Oltean
2024-06-20 17:25 ` [PATCH net-next 3/3] net: dsa: qca8k: add support for bridge port isolation Matthias Schiffer
2024-06-21 10:54   ` Wojciech Drewek
2024-06-21 16:46   ` Vladimir Oltean
2024-06-21 11:30 ` [PATCH net-next 0/3] net: dsa: qca8k: cleanup and " patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).