[PATCH v2 net-next 0/8] net: dsa: mv88e6xxx: Offload bridge port flags

[PATCH v2 net-next 0/8] net: dsa: mv88e6xxx: Offload bridge port flags

[Tobias Waldekranz]

Add support for offloading learning and broadcast flooding flags. With
this in place, mv88e6xx supports offloading of all bridge port flags
that are currently supported by the bridge.

Broadcast flooding is somewhat awkward to control as there is no
per-port bit for this like there is for unknown unicast and unknown
multicast. Instead we have to update the ATU entry for the broadcast
address for all currently used FIDs.

v1 -> v2:
  - Ensure that mv88e6xxx_vtu_get handles VID 0 (Vladimir)
  - Fixed off-by-one in mv88e6xxx_port_set_assoc_vector (Vladimir)
  - Fast age all entries on port when disabling learning (Vladimir)
  - Correctly detect bridge flags on LAG ports (Vladimir)

Tobias Waldekranz (8):
  net: dsa: Add helper to resolve bridge port from DSA port
  net: dsa: mv88e6xxx: Avoid useless attempts to fast-age LAGs
  net: dsa: mv88e6xxx: Provide generic VTU iterator
  net: dsa: mv88e6xxx: Remove some bureaucracy around querying the VTU
  net: dsa: mv88e6xxx: Use standard helper for broadcast address
  net: dsa: mv88e6xxx: Flood all traffic classes on standalone ports
  net: dsa: mv88e6xxx: Offload bridge learning flag
  net: dsa: mv88e6xxx: Offload bridge broadcast flooding flag

 drivers/net/dsa/mv88e6xxx/chip.c | 272 ++++++++++++++++++++++---------
 drivers/net/dsa/mv88e6xxx/port.c |  21 +++
 drivers/net/dsa/mv88e6xxx/port.h |   2 +
 include/net/dsa.h                |  14 ++
 net/dsa/dsa_priv.h               |  14 +-
 5 files changed, 234 insertions(+), 89 deletions(-)

-- 
2.25.1

[PATCH v2 net-next 1/8] net: dsa: Add helper to resolve bridge port from DSA port

[Tobias Waldekranz]

In order for a driver to be able to query a bridge for information
about itself, e.g. reading out port flags, it has to use a netdev that
is known to the bridge. In the simple case, that is just the netdev
representing the port, e.g. swp0 or swp1 in this example:

   br0
   / \
swp0 swp1

But in the case of an offloaded lag, this will be the bond or team
interface, e.g. bond0 in this example:

     br0
     /
  bond0
   / \
swp0 swp1

Add a helper that hides some of this complexity from the
drivers. Then, redefine dsa_port_offloads_bridge_port using the helper
to avoid double accounting of the set of possible offloaded uppers.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 include/net/dsa.h  | 14 ++++++++++++++
 net/dsa/dsa_priv.h | 14 +-------------
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index dac303edd33d..5c4340ecfeb2 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -493,6 +493,20 @@ static inline bool dsa_port_is_vlan_filtering(const struct dsa_port *dp)
 		return dp->vlan_filtering;
 }
 
+static inline
+struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp)
+{
+	if (!dsa_is_user_port(dp->ds, dp->index))
+		return NULL;
+
+	if (dp->lag_dev)
+		return dp->lag_dev;
+	else if (dp->hsr_dev)
+		return dp->hsr_dev;
+
+	return dp->slave;
+}
+
 typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid,
 			      bool is_static, void *data);
 struct dsa_switch_ops {
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 9d4b0e9b1aa1..4c43c5406834 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -233,19 +233,7 @@ extern const struct phylink_mac_ops dsa_port_phylink_mac_ops;
 static inline bool dsa_port_offloads_bridge_port(struct dsa_port *dp,
 						 struct net_device *dev)
 {
-	/* Switchdev offloading can be configured on: */
-
-	if (dev == dp->slave)
-		/* DSA ports directly connected to a bridge, and event
-		 * was emitted for the ports themselves.
-		 */
-		return true;
-
-	if (dp->lag_dev == dev)
-		/* DSA ports connected to a bridge via a LAG */
-		return true;
-
-	return false;
+	return dsa_port_to_bridge_port(dp) == dev;
 }
 
 static inline bool dsa_port_offloads_bridge(struct dsa_port *dp,
-- 
2.25.1

Re: [PATCH v2 net-next 1/8] net: dsa: Add helper to resolve bridge port from DSA port

[Vladimir Oltean]

On Thu, Mar 18, 2021 at 03:15:43PM +0100, Tobias Waldekranz wrote:
> In order for a driver to be able to query a bridge for information
> about itself, e.g. reading out port flags, it has to use a netdev that
> is known to the bridge. In the simple case, that is just the netdev
> representing the port, e.g. swp0 or swp1 in this example:
> 
>    br0
>    / \
> swp0 swp1
> 
> But in the case of an offloaded lag, this will be the bond or team
> interface, e.g. bond0 in this example:
> 
>      br0
>      /
>   bond0
>    / \
> swp0 swp1
> 
> Add a helper that hides some of this complexity from the
> drivers. Then, redefine dsa_port_offloads_bridge_port using the helper
> to avoid double accounting of the set of possible offloaded uppers.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
>  include/net/dsa.h  | 14 ++++++++++++++
>  net/dsa/dsa_priv.h | 14 +-------------
>  2 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index dac303edd33d..5c4340ecfeb2 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -493,6 +493,20 @@ static inline bool dsa_port_is_vlan_filtering(const struct dsa_port *dp)
>  		return dp->vlan_filtering;
>  }
>  
> +static inline
> +struct net_device *dsa_port_to_bridge_port(const struct dsa_port *dp)
> +{
> +	if (!dsa_is_user_port(dp->ds, dp->index))
> +		return NULL;
> +

According to my comment from 8/8, you could have replaced this here with

	if (!dp->bridge_dev)
		return NULL;

I think it's more intuitive to not return a bridge port if there isn't
any bridge to speak of. Whether you prefer to do that or not is up to
you, here's my review tag anyway.

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

> +	if (dp->lag_dev)
> +		return dp->lag_dev;
> +	else if (dp->hsr_dev)
> +		return dp->hsr_dev;
> +
> +	return dp->slave;
> +}
> +
>  typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid,
>  			      bool is_static, void *data);
>  struct dsa_switch_ops {

Re: [PATCH v2 net-next 1/8] net: dsa: Add helper to resolve bridge port from DSA port

[Florian Fainelli]

On 3/18/2021 7:15 AM, Tobias Waldekranz wrote:
> In order for a driver to be able to query a bridge for information
> about itself, e.g. reading out port flags, it has to use a netdev that
> is known to the bridge. In the simple case, that is just the netdev
> representing the port, e.g. swp0 or swp1 in this example:
> 
>    br0
>    / \
> swp0 swp1
> 
> But in the case of an offloaded lag, this will be the bond or team
> interface, e.g. bond0 in this example:
> 
>      br0
>      /
>   bond0
>    / \
> swp0 swp1
> 
> Add a helper that hides some of this complexity from the
> drivers. Then, redefine dsa_port_offloads_bridge_port using the helper
> to avoid double accounting of the set of possible offloaded uppers.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH v2 net-next 2/8] net: dsa: mv88e6xxx: Avoid useless attempts to fast-age LAGs

[Tobias Waldekranz]

When a port is a part of a LAG, the ATU will create dynamic entries
belonging to the LAG ID when learning is enabled. So trying to
fast-age those out using the constituent port will have no
effect. Unfortunately the hardware does not support move operations on
LAGs so there is no obvious way to transform the request to target the
LAG instead.

Instead we document this known limitation and at least avoid wasting
any time on it.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index f0a9423af85d..ed38b4431d74 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1479,6 +1479,13 @@ static void mv88e6xxx_port_fast_age(struct dsa_switch *ds, int port)
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
 
+	if (dsa_to_port(ds, port)->lag_dev)
+		/* Hardware is incapable of fast-aging a LAG through a
+		 * regular ATU move operation. Until we have something
+		 * more fancy in place this is a no-op.
+		 */
+		return;
+
 	mv88e6xxx_reg_lock(chip);
 	err = mv88e6xxx_g1_atu_remove(chip, 0, port, false);
 	mv88e6xxx_reg_unlock(chip);
-- 
2.25.1

Re: [PATCH v2 net-next 2/8] net: dsa: mv88e6xxx: Avoid useless attempts to fast-age LAGs

[Vladimir Oltean]

On Thu, Mar 18, 2021 at 03:15:44PM +0100, Tobias Waldekranz wrote:
> When a port is a part of a LAG, the ATU will create dynamic entries
> belonging to the LAG ID when learning is enabled. So trying to
> fast-age those out using the constituent port will have no
> effect. Unfortunately the hardware does not support move operations on
> LAGs so there is no obvious way to transform the request to target the
> LAG instead.
> 
> Instead we document this known limitation and at least avoid wasting
> any time on it.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---

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

Re: [PATCH v2 net-next 2/8] net: dsa: mv88e6xxx: Avoid useless attempts to fast-age LAGs

[Florian Fainelli]

On 3/18/2021 7:15 AM, Tobias Waldekranz wrote:
> When a port is a part of a LAG, the ATU will create dynamic entries
> belonging to the LAG ID when learning is enabled. So trying to
> fast-age those out using the constituent port will have no
> effect. Unfortunately the hardware does not support move operations on
> LAGs so there is no obvious way to transform the request to target the
> LAG instead.
> 
> Instead we document this known limitation and at least avoid wasting
> any time on it.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH v2 net-next 3/8] net: dsa: mv88e6xxx: Provide generic VTU iterator

[Tobias Waldekranz]

Move the intricacies of correctly iterating over the VTU to a common
implementation.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 100 ++++++++++++++++++++-----------
 1 file changed, 64 insertions(+), 36 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index ed38b4431d74..0a4e467179c9 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1511,6 +1511,37 @@ static int mv88e6xxx_vtu_getnext(struct mv88e6xxx_chip *chip,
 	return chip->info->ops->vtu_getnext(chip, entry);
 }
 
+static int mv88e6xxx_vtu_walk(struct mv88e6xxx_chip *chip,
+			      int (*cb)(struct mv88e6xxx_chip *chip,
+					const struct mv88e6xxx_vtu_entry *entry,
+					void *priv),
+			      void *priv)
+{
+	struct mv88e6xxx_vtu_entry entry = {
+		.vid = mv88e6xxx_max_vid(chip),
+		.valid = false,
+	};
+	int err;
+
+	if (!chip->info->ops->vtu_getnext)
+		return -EOPNOTSUPP;
+
+	do {
+		err = chip->info->ops->vtu_getnext(chip, &entry);
+		if (err)
+			return err;
+
+		if (!entry.valid)
+			break;
+
+		err = cb(chip, &entry, priv);
+		if (err)
+			return err;
+	} while (entry.vid < mv88e6xxx_max_vid(chip));
+
+	return 0;
+}
+
 static int mv88e6xxx_vtu_loadpurge(struct mv88e6xxx_chip *chip,
 				   struct mv88e6xxx_vtu_entry *entry)
 {
@@ -1520,9 +1551,18 @@ static int mv88e6xxx_vtu_loadpurge(struct mv88e6xxx_chip *chip,
 	return chip->info->ops->vtu_loadpurge(chip, entry);
 }
 
+static int mv88e6xxx_fid_map_vlan(struct mv88e6xxx_chip *chip,
+				  const struct mv88e6xxx_vtu_entry *entry,
+				  void *_fid_bitmap)
+{
+	unsigned long *fid_bitmap = _fid_bitmap;
+
+	set_bit(entry->fid, fid_bitmap);
+	return 0;
+}
+
 int mv88e6xxx_fid_map(struct mv88e6xxx_chip *chip, unsigned long *fid_bitmap)
 {
-	struct mv88e6xxx_vtu_entry vlan;
 	int i, err;
 	u16 fid;
 
@@ -1538,21 +1578,7 @@ int mv88e6xxx_fid_map(struct mv88e6xxx_chip *chip, unsigned long *fid_bitmap)
 	}
 
 	/* Set every FID bit used by the VLAN entries */
-	vlan.vid = mv88e6xxx_max_vid(chip);
-	vlan.valid = false;
-
-	do {
-		err = mv88e6xxx_vtu_getnext(chip, &vlan);
-		if (err)
-			return err;
-
-		if (!vlan.valid)
-			break;
-
-		set_bit(vlan.fid, fid_bitmap);
-	} while (vlan.vid < mv88e6xxx_max_vid(chip));
-
-	return 0;
+	return mv88e6xxx_vtu_walk(chip, mv88e6xxx_fid_map_vlan, fid_bitmap);
 }
 
 static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
@@ -2198,10 +2224,30 @@ static int mv88e6xxx_port_db_dump_fid(struct mv88e6xxx_chip *chip,
 	return err;
 }
 
+struct mv88e6xxx_port_db_dump_vlan_ctx {
+	int port;
+	dsa_fdb_dump_cb_t *cb;
+	void *data;
+};
+
+static int mv88e6xxx_port_db_dump_vlan(struct mv88e6xxx_chip *chip,
+				       const struct mv88e6xxx_vtu_entry *entry,
+				       void *_data)
+{
+	struct mv88e6xxx_port_db_dump_vlan_ctx *ctx = _data;
+
+	return mv88e6xxx_port_db_dump_fid(chip, entry->fid, entry->vid,
+					  ctx->port, ctx->cb, ctx->data);
+}
+
 static int mv88e6xxx_port_db_dump(struct mv88e6xxx_chip *chip, int port,
 				  dsa_fdb_dump_cb_t *cb, void *data)
 {
-	struct mv88e6xxx_vtu_entry vlan;
+	struct mv88e6xxx_port_db_dump_vlan_ctx ctx = {
+		.port = port,
+		.cb = cb,
+		.data = data,
+	};
 	u16 fid;
 	int err;
 
@@ -2214,25 +2260,7 @@ static int mv88e6xxx_port_db_dump(struct mv88e6xxx_chip *chip, int port,
 	if (err)
 		return err;
 
-	/* Dump VLANs' Filtering Information Databases */
-	vlan.vid = mv88e6xxx_max_vid(chip);
-	vlan.valid = false;
-
-	do {
-		err = mv88e6xxx_vtu_getnext(chip, &vlan);
-		if (err)
-			return err;
-
-		if (!vlan.valid)
-			break;
-
-		err = mv88e6xxx_port_db_dump_fid(chip, vlan.fid, vlan.vid, port,
-						 cb, data);
-		if (err)
-			return err;
-	} while (vlan.vid < mv88e6xxx_max_vid(chip));
-
-	return err;
+	return mv88e6xxx_vtu_walk(chip, mv88e6xxx_port_db_dump_vlan, &ctx);
 }
 
 static int mv88e6xxx_port_fdb_dump(struct dsa_switch *ds, int port,
-- 
2.25.1

Re: [PATCH v2 net-next 3/8] net: dsa: mv88e6xxx: Provide generic VTU iterator

[Vladimir Oltean]

On Thu, Mar 18, 2021 at 03:15:45PM +0100, Tobias Waldekranz wrote:
> Move the intricacies of correctly iterating over the VTU to a common
> implementation.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> ---

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

Re: [PATCH v2 net-next 3/8] net: dsa: mv88e6xxx: Provide generic VTU iterator

[Florian Fainelli]

On 3/18/2021 7:15 AM, Tobias Waldekranz wrote:
> Move the intricacies of correctly iterating over the VTU to a common
> implementation.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH v2 net-next 4/8] net: dsa: mv88e6xxx: Remove some bureaucracy around querying the VTU

[Tobias Waldekranz]

The hardware has a somewhat quirky protocol for reading out the VTU
entry for a particular VID. But there is no reason why we cannot
create a better API for ourselves in the driver.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 45 ++++++++++++++------------------
 1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 0a4e467179c9..c18c55e1617e 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1502,13 +1502,23 @@ static int mv88e6xxx_vtu_setup(struct mv88e6xxx_chip *chip)
 	return mv88e6xxx_g1_vtu_flush(chip);
 }
 
-static int mv88e6xxx_vtu_getnext(struct mv88e6xxx_chip *chip,
-				 struct mv88e6xxx_vtu_entry *entry)
+static int mv88e6xxx_vtu_get(struct mv88e6xxx_chip *chip, u16 vid,
+			     struct mv88e6xxx_vtu_entry *entry)
 {
+	int err;
+
 	if (!chip->info->ops->vtu_getnext)
 		return -EOPNOTSUPP;
 
-	return chip->info->ops->vtu_getnext(chip, entry);
+	entry->vid = vid ? vid - 1 : mv88e6xxx_max_vid(chip);
+	entry->valid = false;
+
+	err = chip->info->ops->vtu_getnext(chip, entry);
+
+	if (entry->vid != vid)
+		entry->valid = false;
+
+	return err;
 }
 
 static int mv88e6xxx_vtu_walk(struct mv88e6xxx_chip *chip,
@@ -1615,19 +1625,13 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 	if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
 		return 0;
 
-	vlan.vid = vid - 1;
-	vlan.valid = false;
-
-	err = mv88e6xxx_vtu_getnext(chip, &vlan);
+	err = mv88e6xxx_vtu_get(chip, vid, &vlan);
 	if (err)
 		return err;
 
 	if (!vlan.valid)
 		return 0;
 
-	if (vlan.vid != vid)
-		return 0;
-
 	for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
 		if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i))
 			continue;
@@ -1709,15 +1713,12 @@ static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
 		if (err)
 			return err;
 	} else {
-		vlan.vid = vid - 1;
-		vlan.valid = false;
-
-		err = mv88e6xxx_vtu_getnext(chip, &vlan);
+		err = mv88e6xxx_vtu_get(chip, vid, &vlan);
 		if (err)
 			return err;
 
 		/* switchdev expects -EOPNOTSUPP to honor software VLANs */
-		if (vlan.vid != vid || !vlan.valid)
+		if (!vlan.valid)
 			return -EOPNOTSUPP;
 
 		fid = vlan.fid;
@@ -1994,14 +1995,11 @@ static int mv88e6xxx_port_vlan_join(struct mv88e6xxx_chip *chip, int port,
 	struct mv88e6xxx_vtu_entry vlan;
 	int i, err;
 
-	vlan.vid = vid - 1;
-	vlan.valid = false;
-
-	err = mv88e6xxx_vtu_getnext(chip, &vlan);
+	err = mv88e6xxx_vtu_get(chip, vid, &vlan);
 	if (err)
 		return err;
 
-	if (vlan.vid != vid || !vlan.valid) {
+	if (!vlan.valid) {
 		memset(&vlan, 0, sizeof(vlan));
 
 		err = mv88e6xxx_atu_new(chip, &vlan.fid);
@@ -2097,17 +2095,14 @@ static int mv88e6xxx_port_vlan_leave(struct mv88e6xxx_chip *chip,
 	if (!vid)
 		return -EOPNOTSUPP;
 
-	vlan.vid = vid - 1;
-	vlan.valid = false;
-
-	err = mv88e6xxx_vtu_getnext(chip, &vlan);
+	err = mv88e6xxx_vtu_get(chip, vid, &vlan);
 	if (err)
 		return err;
 
 	/* If the VLAN doesn't exist in hardware or the port isn't a member,
 	 * tell switchdev that this VLAN is likely handled in software.
 	 */
-	if (vlan.vid != vid || !vlan.valid ||
+	if (!vlan.valid ||
 	    vlan.member[port] == MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER)
 		return -EOPNOTSUPP;
 
-- 
2.25.1

Re: [PATCH v2 net-next 4/8] net: dsa: mv88e6xxx: Remove some bureaucracy around querying the VTU

[Vladimir Oltean]

On Thu, Mar 18, 2021 at 03:15:46PM +0100, Tobias Waldekranz wrote:
> The hardware has a somewhat quirky protocol for reading out the VTU
> entry for a particular VID. But there is no reason why we cannot
> create a better API for ourselves in the driver.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---

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

Re: [PATCH v2 net-next 4/8] net: dsa: mv88e6xxx: Remove some bureaucracy around querying the VTU

[Florian Fainelli]

On 3/18/2021 7:15 AM, Tobias Waldekranz wrote:
> The hardware has a somewhat quirky protocol for reading out the VTU
> entry for a particular VID. But there is no reason why we cannot
> create a better API for ourselves in the driver.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH v2 net-next 5/8] net: dsa: mv88e6xxx: Use standard helper for broadcast address

[Tobias Waldekranz]

Use the conventional declaration style of a MAC address in the
kernel (u8 addr[ETH_ALEN]) for the broadcast address, then set it
using the existing helper.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index c18c55e1617e..17578f774683 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1968,8 +1968,10 @@ static int mv88e6xxx_set_rxnfc(struct dsa_switch *ds, int port,
 static int mv88e6xxx_port_add_broadcast(struct mv88e6xxx_chip *chip, int port,
 					u16 vid)
 {
-	const char broadcast[6] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 	u8 state = MV88E6XXX_G1_ATU_DATA_STATE_MC_STATIC;
+	u8 broadcast[ETH_ALEN];
+
+	eth_broadcast_addr(broadcast);
 
 	return mv88e6xxx_port_db_load_purge(chip, port, broadcast, vid, state);
 }
-- 
2.25.1

Re: [PATCH v2 net-next 5/8] net: dsa: mv88e6xxx: Use standard helper for broadcast address

[Vladimir Oltean]

On Thu, Mar 18, 2021 at 03:15:47PM +0100, Tobias Waldekranz wrote:
> Use the conventional declaration style of a MAC address in the
> kernel (u8 addr[ETH_ALEN]) for the broadcast address, then set it
> using the existing helper.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---

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

Re: [PATCH v2 net-next 5/8] net: dsa: mv88e6xxx: Use standard helper for broadcast address

[Florian Fainelli]

On 3/18/2021 7:15 AM, Tobias Waldekranz wrote:
> Use the conventional declaration style of a MAC address in the
> kernel (u8 addr[ETH_ALEN]) for the broadcast address, then set it
> using the existing helper.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH v2 net-next 6/8] net: dsa: mv88e6xxx: Flood all traffic classes on standalone ports

[Tobias Waldekranz]

In accordance with the comment in dsa_port_bridge_leave, standalone
ports shall be configured to flood all types of traffic. This change
aligns the mv88e6xxx driver with that policy.

Previously a standalone port would initially not egress any unknown
traffic, but after joining and then leaving a bridge, it would.

This does not matter that much since we only ever send FROM_CPUs on
standalone ports, but it seems prudent to make sure that the initial
values match those that are applied after a bridging/unbridging cycle.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 17578f774683..587959b78c7f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2489,19 +2489,15 @@ static int mv88e6xxx_setup_message_port(struct mv88e6xxx_chip *chip, int port)
 
 static int mv88e6xxx_setup_egress_floods(struct mv88e6xxx_chip *chip, int port)
 {
-	struct dsa_switch *ds = chip->ds;
-	bool flood;
 	int err;
 
-	/* Upstream ports flood frames with unknown unicast or multicast DA */
-	flood = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port);
 	if (chip->info->ops->port_set_ucast_flood) {
-		err = chip->info->ops->port_set_ucast_flood(chip, port, flood);
+		err = chip->info->ops->port_set_ucast_flood(chip, port, true);
 		if (err)
 			return err;
 	}
 	if (chip->info->ops->port_set_mcast_flood) {
-		err = chip->info->ops->port_set_mcast_flood(chip, port, flood);
+		err = chip->info->ops->port_set_mcast_flood(chip, port, true);
 		if (err)
 			return err;
 	}
-- 
2.25.1

[PATCH v2 net-next 7/8] net: dsa: mv88e6xxx: Offload bridge learning flag

[Tobias Waldekranz]

Allow a user to control automatic learning per port.

Many chips have an explicit "LearningDisable"-bit that can be used for
this, but we opt for setting/clearing the PAV instead, as it works on
all devices at least as far back as 6083.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 37 +++++++++++++++++++++++++-------
 drivers/net/dsa/mv88e6xxx/port.c | 21 ++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/port.h |  2 ++
 3 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 587959b78c7f..7976fb699086 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2740,15 +2740,20 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 			return err;
 	}
 
-	/* Port Association Vector: when learning source addresses
-	 * of packets, add the address to the address database using
-	 * a port bitmap that has only the bit for this port set and
-	 * the other bits clear.
+	/* Port Association Vector: disable automatic address learning
+	 * on all user ports since they start out in standalone
+	 * mode. When joining a bridge, learning will be configured to
+	 * match the bridge port settings. Enable learning on all
+	 * DSA/CPU ports. NOTE: FROM_CPU frames always bypass the
+	 * learning process.
+	 *
+	 * Disable HoldAt1, IntOnAgeOut, LockedPort, IgnoreWrongData,
+	 * and RefreshLocked. I.e. setup standard automatic learning.
 	 */
-	reg = 1 << port;
-	/* Disable learning for CPU port */
-	if (dsa_is_cpu_port(ds, port))
+	if (dsa_is_user_port(ds, port))
 		reg = 0;
+	else
+		reg = 1 << port;
 
 	err = mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_ASSOC_VECTOR,
 				   reg);
@@ -5604,7 +5609,7 @@ static int mv88e6xxx_port_pre_bridge_flags(struct dsa_switch *ds, int port,
 	struct mv88e6xxx_chip *chip = ds->priv;
 	const struct mv88e6xxx_ops *ops;
 
-	if (flags.mask & ~(BR_FLOOD | BR_MCAST_FLOOD))
+	if (flags.mask & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD))
 		return -EINVAL;
 
 	ops = chip->info->ops;
@@ -5623,10 +5628,23 @@ static int mv88e6xxx_port_bridge_flags(struct dsa_switch *ds, int port,
 				       struct netlink_ext_ack *extack)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
+	bool do_fast_age = false;
 	int err = -EOPNOTSUPP;
 
 	mv88e6xxx_reg_lock(chip);
 
+	if (flags.mask & BR_LEARNING) {
+		bool learning = !!(flags.val & BR_LEARNING);
+		u16 pav = learning ? (1 << port) : 0;
+
+		err = mv88e6xxx_port_set_assoc_vector(chip, port, pav);
+		if (err)
+			goto out;
+
+		if (!learning)
+			do_fast_age = true;
+	}
+
 	if (flags.mask & BR_FLOOD) {
 		bool unicast = !!(flags.val & BR_FLOOD);
 
@@ -5648,6 +5666,9 @@ static int mv88e6xxx_port_bridge_flags(struct dsa_switch *ds, int port,
 out:
 	mv88e6xxx_reg_unlock(chip);
 
+	if (do_fast_age)
+		mv88e6xxx_port_fast_age(ds, port);
+
 	return err;
 }
 
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index 6a9c45c2127a..f77e2ee64a60 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -1309,6 +1309,27 @@ int mv88e6097_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port)
 				    0x0001);
 }
 
+/* Offset 0x0B: Port Association Vector */
+
+int mv88e6xxx_port_set_assoc_vector(struct mv88e6xxx_chip *chip, int port,
+				    u16 pav)
+{
+	u16 reg, mask;
+	int err;
+
+	err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_ASSOC_VECTOR,
+				  &reg);
+	if (err)
+		return err;
+
+	mask = mv88e6xxx_port_mask(chip);
+	reg &= ~mask;
+	reg |= pav & mask;
+
+	return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_ASSOC_VECTOR,
+				    reg);
+}
+
 /* Offset 0x0C: Port ATU Control */
 
 int mv88e6xxx_port_disable_learn_limit(struct mv88e6xxx_chip *chip, int port)
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index 921d54969dad..b10e5aebacf6 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -407,6 +407,8 @@ int mv88e6165_port_set_jumbo_size(struct mv88e6xxx_chip *chip, int port,
 				  size_t size);
 int mv88e6095_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port);
 int mv88e6097_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port);
+int mv88e6xxx_port_set_assoc_vector(struct mv88e6xxx_chip *chip, int port,
+				    u16 pav);
 int mv88e6097_port_pause_limit(struct mv88e6xxx_chip *chip, int port, u8 in,
 			       u8 out);
 int mv88e6390_port_pause_limit(struct mv88e6xxx_chip *chip, int port, u8 in,
-- 
2.25.1

Re: [PATCH v2 net-next 7/8] net: dsa: mv88e6xxx: Offload bridge learning flag

[Vladimir Oltean]

On Thu, Mar 18, 2021 at 03:15:49PM +0100, Tobias Waldekranz wrote:
> Allow a user to control automatic learning per port.
> 
> Many chips have an explicit "LearningDisable"-bit that can be used for
> this, but we opt for setting/clearing the PAV instead, as it works on
> all devices at least as far back as 6083.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---

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

Re: [PATCH v2 net-next 7/8] net: dsa: mv88e6xxx: Offload bridge learning flag

[Florian Fainelli]

On 3/18/2021 7:15 AM, Tobias Waldekranz wrote:
> Allow a user to control automatic learning per port.
> 
> Many chips have an explicit "LearningDisable"-bit that can be used for
> this, but we opt for setting/clearing the PAV instead, as it works on
> all devices at least as far back as 6083.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH v2 net-next 8/8] net: dsa: mv88e6xxx: Offload bridge broadcast flooding flag

[Tobias Waldekranz]

These switches have two modes of classifying broadcast:

1. Broadcast is multicast.
2. Broadcast is its own unique thing that is always flooded
   everywhere.

This driver uses the first option, making sure to load the broadcast
address into all active databases. Because of this, we can support
per-port broadcast flooding by (1) making sure to only set the subset
of ports that have it enabled whenever joining a new bridge or VLAN,
and (2) by updating all active databases whenever the setting is
changed on a port.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 73 +++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 7976fb699086..3baa4dadb0dd 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1982,6 +1982,21 @@ static int mv88e6xxx_broadcast_setup(struct mv88e6xxx_chip *chip, u16 vid)
 	int err;
 
 	for (port = 0; port < mv88e6xxx_num_ports(chip); port++) {
+		struct dsa_port *dp = dsa_to_port(chip->ds, port);
+		struct net_device *brport;
+
+		if (dsa_is_unused_port(chip->ds, port))
+			continue;
+
+		brport = dsa_port_to_bridge_port(dp);
+
+		if (dp->bridge_dev &&
+		    !br_port_flag_is_set(brport, BR_BCAST_FLOOD))
+			/* Skip bridged user ports where broadcast
+			 * flooding is disabled.
+			 */
+			continue;
+
 		err = mv88e6xxx_port_add_broadcast(chip, port, vid);
 		if (err)
 			return err;
@@ -1990,6 +2005,53 @@ static int mv88e6xxx_broadcast_setup(struct mv88e6xxx_chip *chip, u16 vid)
 	return 0;
 }
 
+struct mv88e6xxx_port_broadcast_sync_ctx {
+	int port;
+	bool flood;
+};
+
+static int
+mv88e6xxx_port_broadcast_sync_vlan(struct mv88e6xxx_chip *chip,
+				   const struct mv88e6xxx_vtu_entry *vlan,
+				   void *_ctx)
+{
+	struct mv88e6xxx_port_broadcast_sync_ctx *ctx = _ctx;
+	u8 broadcast[ETH_ALEN];
+	u8 state;
+
+	if (ctx->flood)
+		state = MV88E6XXX_G1_ATU_DATA_STATE_MC_STATIC;
+	else
+		state = MV88E6XXX_G1_ATU_DATA_STATE_MC_UNUSED;
+
+	eth_broadcast_addr(broadcast);
+
+	return mv88e6xxx_port_db_load_purge(chip, ctx->port, broadcast,
+					    vlan->vid, state);
+}
+
+static int mv88e6xxx_port_broadcast_sync(struct mv88e6xxx_chip *chip, int port,
+					 bool flood)
+{
+	struct mv88e6xxx_port_broadcast_sync_ctx ctx = {
+		.port = port,
+		.flood = flood,
+	};
+	struct mv88e6xxx_vtu_entry vid0 = {
+		.vid = 0,
+	};
+	int err;
+
+	/* Update the port's private database... */
+	err = mv88e6xxx_port_broadcast_sync_vlan(chip, &vid0, &ctx);
+	if (err)
+		return err;
+
+	/* ...and the database for all VLANs. */
+	return mv88e6xxx_vtu_walk(chip, mv88e6xxx_port_broadcast_sync_vlan,
+				  &ctx);
+}
+
 static int mv88e6xxx_port_vlan_join(struct mv88e6xxx_chip *chip, int port,
 				    u16 vid, u8 member, bool warn)
 {
@@ -5609,7 +5671,8 @@ static int mv88e6xxx_port_pre_bridge_flags(struct dsa_switch *ds, int port,
 	struct mv88e6xxx_chip *chip = ds->priv;
 	const struct mv88e6xxx_ops *ops;
 
-	if (flags.mask & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD))
+	if (flags.mask & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD |
+			   BR_BCAST_FLOOD))
 		return -EINVAL;
 
 	ops = chip->info->ops;
@@ -5663,6 +5726,14 @@ static int mv88e6xxx_port_bridge_flags(struct dsa_switch *ds, int port,
 			goto out;
 	}
 
+	if (flags.mask & BR_BCAST_FLOOD) {
+		bool broadcast = !!(flags.val & BR_BCAST_FLOOD);
+
+		err = mv88e6xxx_port_broadcast_sync(chip, port, broadcast);
+		if (err)
+			goto out;
+	}
+
 out:
 	mv88e6xxx_reg_unlock(chip);
 
-- 
2.25.1

Re: [PATCH v2 net-next 8/8] net: dsa: mv88e6xxx: Offload bridge broadcast flooding flag

[Vladimir Oltean]

On Thu, Mar 18, 2021 at 03:15:50PM +0100, Tobias Waldekranz wrote:
> These switches have two modes of classifying broadcast:
> 
> 1. Broadcast is multicast.
> 2. Broadcast is its own unique thing that is always flooded
>    everywhere.
> 
> This driver uses the first option, making sure to load the broadcast
> address into all active databases. Because of this, we can support
> per-port broadcast flooding by (1) making sure to only set the subset
> of ports that have it enabled whenever joining a new bridge or VLAN,
> and (2) by updating all active databases whenever the setting is
> changed on a port.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 73 +++++++++++++++++++++++++++++++-
>  1 file changed, 72 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 7976fb699086..3baa4dadb0dd 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -1982,6 +1982,21 @@ static int mv88e6xxx_broadcast_setup(struct mv88e6xxx_chip *chip, u16 vid)
>  	int err;
>  
>  	for (port = 0; port < mv88e6xxx_num_ports(chip); port++) {
> +		struct dsa_port *dp = dsa_to_port(chip->ds, port);
> +		struct net_device *brport;
> +
> +		if (dsa_is_unused_port(chip->ds, port))
> +			continue;
> +
> +		brport = dsa_port_to_bridge_port(dp);
> +
> +		if (dp->bridge_dev &&
> +		    !br_port_flag_is_set(brport, BR_BCAST_FLOOD))

I think I would have liked to see a dsa_port_to_bridge_port helper that
actually returns NULL when dp->bridge_dev is NULL.

This would make your piece of code look as follows:

		brport = dsa_port_to_bridge_port(dp);
		if (brport && !br_port_flag_is_set(brport, BR_BCAST_FLOOD)
			continue;

> +			/* Skip bridged user ports where broadcast
> +			 * flooding is disabled.
> +			 */
> +			continue;
> +
>  		err = mv88e6xxx_port_add_broadcast(chip, port, vid);
>  		if (err)
>  			return err;

Re: [PATCH v2 net-next 8/8] net: dsa: mv88e6xxx: Offload bridge broadcast flooding flag

[Florian Fainelli]

On 3/18/2021 7:35 AM, Vladimir Oltean wrote:
> On Thu, Mar 18, 2021 at 03:15:50PM +0100, Tobias Waldekranz wrote:
>> These switches have two modes of classifying broadcast:
>>
>> 1. Broadcast is multicast.
>> 2. Broadcast is its own unique thing that is always flooded
>>    everywhere.
>>
>> This driver uses the first option, making sure to load the broadcast
>> address into all active databases. Because of this, we can support
>> per-port broadcast flooding by (1) making sure to only set the subset
>> of ports that have it enabled whenever joining a new bridge or VLAN,
>> and (2) by updating all active databases whenever the setting is
>> changed on a port.
>>
>> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
>> ---
>>  drivers/net/dsa/mv88e6xxx/chip.c | 73 +++++++++++++++++++++++++++++++-
>>  1 file changed, 72 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
>> index 7976fb699086..3baa4dadb0dd 100644
>> --- a/drivers/net/dsa/mv88e6xxx/chip.c
>> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
>> @@ -1982,6 +1982,21 @@ static int mv88e6xxx_broadcast_setup(struct mv88e6xxx_chip *chip, u16 vid)
>>  	int err;
>>  
>>  	for (port = 0; port < mv88e6xxx_num_ports(chip); port++) {
>> +		struct dsa_port *dp = dsa_to_port(chip->ds, port);
>> +		struct net_device *brport;
>> +
>> +		if (dsa_is_unused_port(chip->ds, port))
>> +			continue;
>> +
>> +		brport = dsa_port_to_bridge_port(dp);
>> +
>> +		if (dp->bridge_dev &&
>> +		    !br_port_flag_is_set(brport, BR_BCAST_FLOOD))
> 
> I think I would have liked to see a dsa_port_to_bridge_port helper that
> actually returns NULL when dp->bridge_dev is NULL.
> 
> This would make your piece of code look as follows:
> 
> 		brport = dsa_port_to_bridge_port(dp);
> 		if (brport && !br_port_flag_is_set(brport, BR_BCAST_FLOOD)
> 			continue;

Agreed, with that fixed:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian