[PATCH v2 net-next 0/3] mv88e6xxx: Add MAB offload support

[PATCH v2 net-next 0/3] mv88e6xxx: Add MAB offload support

[Hans J. Schultz]

This patchset adds MAB [1] offload support in mv88e6xxx.

Patch #1: Correct default return value for mv88e6xxx_port_bridge_flags.

Patch #2: Change chip lock handling in ATU interrupt handler.

Patch #3: The MAB implementation for mv88e6xxx.

LOG:
        V2:     -FID reading patch already applied, so dropped here. [1]
                -Patch #2 here as separate patch instead of part of MAB
                 implementation patch.
                -Check if fid is MV88E6XXX_FID_STANDALONE, and not if
                 fid is zero, as that is the correct check. Do not
                 report an error.

[1] https://git.kernel.org/netdev/net-next/c/4bf24ad09bc0

Hans J. Schultz (3):
  net: dsa: mv88e6xxx: change default return of
    mv88e6xxx_port_bridge_flags
  net: dsa: mv88e6xxx: disable hold of chip lock for handling
  net: dsa: mv88e6xxx: mac-auth/MAB implementation

 drivers/net/dsa/mv88e6xxx/Makefile      |  1 +
 drivers/net/dsa/mv88e6xxx/chip.c        | 20 +++---
 drivers/net/dsa/mv88e6xxx/chip.h        | 15 +++++
 drivers/net/dsa/mv88e6xxx/global1_atu.c | 22 +++++--
 drivers/net/dsa/mv88e6xxx/switchdev.c   | 83 +++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/switchdev.h   | 19 ++++++
 6 files changed, 147 insertions(+), 13 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6xxx/switchdev.c
 create mode 100644 drivers/net/dsa/mv88e6xxx/switchdev.h

-- 
2.34.1

[PATCH v2 net-next 1/3] net: dsa: mv88e6xxx: change default return of mv88e6xxx_port_bridge_flags

[Hans J. Schultz]

The default return value -EOPNOTSUPP of mv88e6xxx_port_bridge_flags()
came from the return value of the DSA method port_egress_floods() in
commit 4f85901f0063 ("net: dsa: mv88e6xxx: add support for bridge flags"),
but the DSA API was changed in commit a8b659e7ff75 ("net: dsa: act as
passthrough for bridge port flags"), resulting in the return value
-EOPNOTSUPP not being valid anymore, and sections for new flags will not
need to set the return value to zero on success, as with the new mab flag
added in a following patch.

Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index ba4fff8690aa..d5930b287db4 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -6546,7 +6546,7 @@ static int mv88e6xxx_port_bridge_flags(struct dsa_switch *ds, int port,
 				       struct netlink_ext_ack *extack)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
-	int err = -EOPNOTSUPP;
+	int err = 0;
 
 	mv88e6xxx_reg_lock(chip);
 
-- 
2.34.1

Re: [PATCH v2 net-next 1/3] net: dsa: mv88e6xxx: change default return of mv88e6xxx_port_bridge_flags

[Alexander H Duyck]

On Tue, 2022-12-13 at 18:46 +0100, Hans J. Schultz wrote:
> The default return value -EOPNOTSUPP of mv88e6xxx_port_bridge_flags()
> came from the return value of the DSA method port_egress_floods() in
> commit 4f85901f0063 ("net: dsa: mv88e6xxx: add support for bridge flags"),
> but the DSA API was changed in commit a8b659e7ff75 ("net: dsa: act as
> passthrough for bridge port flags"), resulting in the return value
> -EOPNOTSUPP not being valid anymore, and sections for new flags will not
> need to set the return value to zero on success, as with the new mab flag
> added in a following patch.
> 
> Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index ba4fff8690aa..d5930b287db4 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -6546,7 +6546,7 @@ static int mv88e6xxx_port_bridge_flags(struct dsa_switch *ds, int port,
>  				       struct netlink_ext_ack *extack)
>  {
>  	struct mv88e6xxx_chip *chip = ds->priv;
> -	int err = -EOPNOTSUPP;
> +	int err = 0;
>  
>  	mv88e6xxx_reg_lock(chip);
>  

Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>

[PATCH v2 net-next 2/3] net: dsa: mv88e6xxx: disable hold of chip lock for handling

[Hans J. Schultz]

As functions called under the interrupt handler will need to take the
netlink lock, we need to release the chip lock before calling those
functions as otherwise double lock deadlocks will occur as userspace
calls towards the driver often take the netlink lock and then the
chip lock.

The deadlock would look like:

Interrupt handler: chip lock taken, but cannot take netlink lock as
                   userspace config call has netlink lock.
Userspace config: netlink lock taken, but cannot take chip lock as
                   the interrupt handler has the chip lock.

Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>
---
 drivers/net/dsa/mv88e6xxx/global1_atu.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
index 61ae2d61e25c..34203e112eef 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
@@ -409,11 +409,11 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
 
 	err = mv88e6xxx_g1_read_atu_violation(chip);
 	if (err)
-		goto out;
+		goto out_unlock;
 
 	err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_OP, &val);
 	if (err)
-		goto out;
+		goto out_unlock;
 
 	err = mv88e6xxx_g1_atu_fid_read(chip, &fid);
 	if (err)
@@ -421,11 +421,13 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
 
 	err = mv88e6xxx_g1_atu_data_read(chip, &entry);
 	if (err)
-		goto out;
+		goto out_unlock;
 
 	err = mv88e6xxx_g1_atu_mac_read(chip, &entry);
 	if (err)
-		goto out;
+		goto out_unlock;
+
+	mv88e6xxx_reg_unlock(chip);
 
 	spid = entry.state;
 
@@ -449,13 +451,13 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
 						   fid);
 		chip->ports[spid].atu_full_violation++;
 	}
-	mv88e6xxx_reg_unlock(chip);
 
 	return IRQ_HANDLED;
 
-out:
+out_unlock:
 	mv88e6xxx_reg_unlock(chip);
 
+out:
 	dev_err(chip->dev, "ATU problem: error %d while handling interrupt\n",
 		err);
 	return IRQ_HANDLED;
-- 
2.34.1

Re: [PATCH v2 net-next 2/3] net: dsa: mv88e6xxx: disable hold of chip lock for handling

[Alexander H Duyck]

On Tue, 2022-12-13 at 18:46 +0100, Hans J. Schultz wrote:
> As functions called under the interrupt handler will need to take the
> netlink lock, we need to release the chip lock before calling those
> functions as otherwise double lock deadlocks will occur as userspace
> calls towards the driver often take the netlink lock and then the
> chip lock.
> 
> The deadlock would look like:
> 
> Interrupt handler: chip lock taken, but cannot take netlink lock as
>                    userspace config call has netlink lock.
> Userspace config: netlink lock taken, but cannot take chip lock as
>                    the interrupt handler has the chip lock.
> 
> Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>

Just to confirm, in order to see the deadlocks I would imagine you need
something in the tracepoints that is taking the netlink lock?

If so you might want to reference the commit that switched out the
dev_err_ratelimited for the tracepoints 8646384d80f3 ("net: dsa:
mv88e6xxx: replace ATU violation prints with trace points") so that
this patch can be found an applied to any kernels that pull in those
tracepoints.

> ---
>  drivers/net/dsa/mv88e6xxx/global1_atu.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
> index 61ae2d61e25c..34203e112eef 100644
> --- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
> +++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
> @@ -409,11 +409,11 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
>  
>  	err = mv88e6xxx_g1_read_atu_violation(chip);
>  	if (err)
> -		goto out;
> +		goto out_unlock;
>  
>  	err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_OP, &val);
>  	if (err)
> -		goto out;
> +		goto out_unlock;
>  
>  	err = mv88e6xxx_g1_atu_fid_read(chip, &fid);
>  	if (err)
> @@ -421,11 +421,13 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
>  
>  	err = mv88e6xxx_g1_atu_data_read(chip, &entry);
>  	if (err)
> -		goto out;
> +		goto out_unlock;
>  
>  	err = mv88e6xxx_g1_atu_mac_read(chip, &entry);
>  	if (err)
> -		goto out;
> +		goto out_unlock;
> +
> +	mv88e6xxx_reg_unlock(chip);
>  
>  	spid = entry.state;
>  
> @@ -449,13 +451,13 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
>  						   fid);
>  		chip->ports[spid].atu_full_violation++;
>  	}
> -	mv88e6xxx_reg_unlock(chip);
>  
>  	return IRQ_HANDLED;
>  
> -out:
> +out_unlock:
>  	mv88e6xxx_reg_unlock(chip);
>  
> +out:
>  	dev_err(chip->dev, "ATU problem: error %d while handling interrupt\n",
>  		err);
>  	return IRQ_HANDLED;

Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>

[PATCH v2 net-next 3/3] net: dsa: mv88e6xxx: mac-auth/MAB implementation

[Hans J. Schultz]

This implementation for the Marvell mv88e6xxx chip series, is based on
handling ATU miss violations occurring when packets ingress on a port
that is locked with learning on. This will trigger a
SWITCHDEV_FDB_ADD_TO_BRIDGE event, which will result in the bridge module
adding a locked FDB entry. This bridge FDB entry will not age out as
it has the extern_learn flag set.

Userspace daemons can listen to these events and either accept or deny
access for the host, by either replacing the locked FDB entry with a
simple entry or leave the locked entry.

If the host MAC address is already present on another port, a ATU
member violation will occur, but to no real effect, and the packet will
be dropped in hardware. Statistics on these violations can be shown with
the command and example output of interest:

ethtool -S ethX
NIC statistics:
...
     atu_member_violation: 5
     atu_miss_violation: 23
...

Where ethX is the interface of the MAB enabled port.

Furthermore, as added vlan interfaces where the vid is not added to the
VTU will cause ATU miss violations reporting the FID as
MV88E6XXX_FID_STANDALONE, we need to check and skip the miss violations
handling in this case.

Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>
---
 drivers/net/dsa/mv88e6xxx/Makefile      |  1 +
 drivers/net/dsa/mv88e6xxx/chip.c        | 18 ++++--
 drivers/net/dsa/mv88e6xxx/chip.h        | 15 +++++
 drivers/net/dsa/mv88e6xxx/global1_atu.c |  8 +++
 drivers/net/dsa/mv88e6xxx/switchdev.c   | 83 +++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/switchdev.h   | 19 ++++++
 6 files changed, 138 insertions(+), 6 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6xxx/switchdev.c
 create mode 100644 drivers/net/dsa/mv88e6xxx/switchdev.h

diff --git a/drivers/net/dsa/mv88e6xxx/Makefile b/drivers/net/dsa/mv88e6xxx/Makefile
index 49bf358b9c4f..1409e691ab77 100644
--- a/drivers/net/dsa/mv88e6xxx/Makefile
+++ b/drivers/net/dsa/mv88e6xxx/Makefile
@@ -15,6 +15,7 @@ mv88e6xxx-objs += port_hidden.o
 mv88e6xxx-$(CONFIG_NET_DSA_MV88E6XXX_PTP) += ptp.o
 mv88e6xxx-objs += serdes.o
 mv88e6xxx-objs += smi.o
+mv88e6xxx-objs += switchdev.o
 mv88e6xxx-objs += trace.o
 
 # for tracing framework to find trace.h
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d5930b287db4..2682c2b29346 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1729,11 +1729,11 @@ static int mv88e6xxx_vtu_get(struct mv88e6xxx_chip *chip, u16 vid,
 	return err;
 }
 
-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)
+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),
@@ -6527,7 +6527,7 @@ static int mv88e6xxx_port_pre_bridge_flags(struct dsa_switch *ds, int port,
 	const struct mv88e6xxx_ops *ops;
 
 	if (flags.mask & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD |
-			   BR_BCAST_FLOOD | BR_PORT_LOCKED))
+			   BR_BCAST_FLOOD | BR_PORT_LOCKED | BR_PORT_MAB))
 		return -EINVAL;
 
 	ops = chip->info->ops;
@@ -6585,6 +6585,12 @@ static int mv88e6xxx_port_bridge_flags(struct dsa_switch *ds, int port,
 			goto out;
 	}
 
+	if (flags.mask & BR_PORT_MAB) {
+		bool mab = !!(flags.val & BR_PORT_MAB);
+
+		mv88e6xxx_port_set_mab(chip, port, mab);
+	}
+
 	if (flags.mask & BR_PORT_LOCKED) {
 		bool locked = !!(flags.val & BR_PORT_LOCKED);
 
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index e693154cf803..f635a5bb47ce 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -280,6 +280,9 @@ struct mv88e6xxx_port {
 	unsigned int serdes_irq;
 	char serdes_irq_name[64];
 	struct devlink_region *region;
+
+	/* MacAuth Bypass control flag */
+	bool mab;
 };
 
 enum mv88e6xxx_region_id {
@@ -784,6 +787,12 @@ static inline bool mv88e6xxx_is_invalid_port(struct mv88e6xxx_chip *chip, int po
 	return (chip->info->invalid_port_mask & BIT(port)) != 0;
 }
 
+static inline void mv88e6xxx_port_set_mab(struct mv88e6xxx_chip *chip,
+					  int port, bool mab)
+{
+	chip->ports[port].mab = mab;
+}
+
 int mv88e6xxx_read(struct mv88e6xxx_chip *chip, int addr, int reg, u16 *val);
 int mv88e6xxx_write(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val);
 int mv88e6xxx_wait_mask(struct mv88e6xxx_chip *chip, int addr, int reg,
@@ -802,6 +811,12 @@ static inline void mv88e6xxx_reg_unlock(struct mv88e6xxx_chip *chip)
 	mutex_unlock(&chip->reg_lock);
 }
 
+int mv88e6xxx_vtu_walk(struct mv88e6xxx_chip *chip,
+		       int (*cb)(struct mv88e6xxx_chip *chip,
+				 const struct mv88e6xxx_vtu_entry *entry,
+				 void *priv),
+		       void *priv);
+
 int mv88e6xxx_fid_map(struct mv88e6xxx_chip *chip, unsigned long *bitmap);
 
 #endif /* _MV88E6XXX_CHIP_H */
diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
index 34203e112eef..fc020161b7cf 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
@@ -12,6 +12,7 @@
 
 #include "chip.h"
 #include "global1.h"
+#include "switchdev.h"
 #include "trace.h"
 
 /* Offset 0x01: ATU FID Register */
@@ -443,6 +444,13 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
 						   entry.portvec, entry.mac,
 						   fid);
 		chip->ports[spid].atu_miss_violation++;
+
+		if (fid != MV88E6XXX_FID_STANDALONE && chip->ports[spid].mab) {
+			err = mv88e6xxx_handle_miss_violation(chip, spid,
+							      &entry, fid);
+			if (err)
+				goto out;
+		}
 	}
 
 	if (val & MV88E6XXX_G1_ATU_OP_FULL_VIOLATION) {
diff --git a/drivers/net/dsa/mv88e6xxx/switchdev.c b/drivers/net/dsa/mv88e6xxx/switchdev.c
new file mode 100644
index 000000000000..4c346a884fb2
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/switchdev.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * switchdev.c
+ *
+ *	Authors:
+ *	Hans J. Schultz		<netdev@kapio-technology.com>
+ *
+ */
+
+#include <net/switchdev.h>
+#include "chip.h"
+#include "global1.h"
+#include "switchdev.h"
+
+struct mv88e6xxx_fid_search_ctx {
+	u16 fid_search;
+	u16 vid_found;
+};
+
+static int __mv88e6xxx_find_vid(struct mv88e6xxx_chip *chip,
+				const struct mv88e6xxx_vtu_entry *entry,
+				void *priv)
+{
+	struct mv88e6xxx_fid_search_ctx *ctx = priv;
+
+	if (ctx->fid_search == entry->fid) {
+		ctx->vid_found = entry->vid;
+		return 1;
+	}
+
+	return 0;
+}
+
+static int mv88e6xxx_find_vid(struct mv88e6xxx_chip *chip, u16 fid, u16 *vid)
+{
+	struct mv88e6xxx_fid_search_ctx ctx;
+	int err;
+
+	ctx.fid_search = fid;
+	mv88e6xxx_reg_lock(chip);
+	err = mv88e6xxx_vtu_walk(chip, __mv88e6xxx_find_vid, &ctx);
+	mv88e6xxx_reg_unlock(chip);
+	if (err < 0)
+		return err;
+	if (err == 1)
+		*vid = ctx.vid_found;
+	else
+		return -ENOENT;
+
+	return 0;
+}
+
+int mv88e6xxx_handle_miss_violation(struct mv88e6xxx_chip *chip, int port,
+				    struct mv88e6xxx_atu_entry *entry, u16 fid)
+{
+	struct switchdev_notifier_fdb_info info = {
+		.addr = entry->mac,
+		.locked = true,
+	};
+	struct net_device *brport;
+	struct dsa_port *dp;
+	u16 vid;
+	int err;
+
+	err = mv88e6xxx_find_vid(chip, fid, &vid);
+	if (err)
+		return err;
+
+	info.vid = vid;
+	dp = dsa_to_port(chip->ds, port);
+
+	rtnl_lock();
+	brport = dsa_port_to_bridge_port(dp);
+	if (!brport) {
+		rtnl_unlock();
+		return -ENODEV;
+	}
+	err = call_switchdev_notifiers(SWITCHDEV_FDB_ADD_TO_BRIDGE,
+				       brport, &info.info, NULL);
+	rtnl_unlock();
+
+	return err;
+}
diff --git a/drivers/net/dsa/mv88e6xxx/switchdev.h b/drivers/net/dsa/mv88e6xxx/switchdev.h
new file mode 100644
index 000000000000..62214f9d62b0
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/switchdev.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * switchdev.h
+ *
+ *	Authors:
+ *	Hans J. Schultz		<netdev@kapio-technology.com>
+ *
+ */
+
+#ifndef _MV88E6XXX_SWITCHDEV_H_
+#define _MV88E6XXX_SWITCHDEV_H_
+
+#include "chip.h"
+
+int mv88e6xxx_handle_miss_violation(struct mv88e6xxx_chip *chip, int port,
+				    struct mv88e6xxx_atu_entry *entry,
+				    u16 fid);
+
+#endif /* _MV88E6XXX_SWITCHDEV_H_ */
-- 
2.34.1

Re: [PATCH v2 net-next 3/3] net: dsa: mv88e6xxx: mac-auth/MAB implementation

[Alexander H Duyck]

On Tue, 2022-12-13 at 18:46 +0100, Hans J. Schultz wrote:
> This implementation for the Marvell mv88e6xxx chip series, is based on
> handling ATU miss violations occurring when packets ingress on a port
> that is locked with learning on. This will trigger a
> SWITCHDEV_FDB_ADD_TO_BRIDGE event, which will result in the bridge module
> adding a locked FDB entry. This bridge FDB entry will not age out as
> it has the extern_learn flag set.
> 
> Userspace daemons can listen to these events and either accept or deny
> access for the host, by either replacing the locked FDB entry with a
> simple entry or leave the locked entry.
> 
> If the host MAC address is already present on another port, a ATU
> member violation will occur, but to no real effect, and the packet will
> be dropped in hardware. Statistics on these violations can be shown with
> the command and example output of interest:
> 
> ethtool -S ethX
> NIC statistics:
> ...
>      atu_member_violation: 5
>      atu_miss_violation: 23
> ...
> 
> Where ethX is the interface of the MAB enabled port.
> 
> Furthermore, as added vlan interfaces where the vid is not added to the
> VTU will cause ATU miss violations reporting the FID as
> MV88E6XXX_FID_STANDALONE, we need to check and skip the miss violations
> handling in this case.
> 
> Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>
> ---
>  drivers/net/dsa/mv88e6xxx/Makefile      |  1 +
>  drivers/net/dsa/mv88e6xxx/chip.c        | 18 ++++--
>  drivers/net/dsa/mv88e6xxx/chip.h        | 15 +++++
>  drivers/net/dsa/mv88e6xxx/global1_atu.c |  8 +++
>  drivers/net/dsa/mv88e6xxx/switchdev.c   | 83 +++++++++++++++++++++++++
>  drivers/net/dsa/mv88e6xxx/switchdev.h   | 19 ++++++
>  6 files changed, 138 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/net/dsa/mv88e6xxx/switchdev.c
>  create mode 100644 drivers/net/dsa/mv88e6xxx/switchdev.h
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/Makefile b/drivers/net/dsa/mv88e6xxx/Makefile
> index 49bf358b9c4f..1409e691ab77 100644
> --- a/drivers/net/dsa/mv88e6xxx/Makefile
> +++ b/drivers/net/dsa/mv88e6xxx/Makefile
> @@ -15,6 +15,7 @@ mv88e6xxx-objs += port_hidden.o
>  mv88e6xxx-$(CONFIG_NET_DSA_MV88E6XXX_PTP) += ptp.o
>  mv88e6xxx-objs += serdes.o
>  mv88e6xxx-objs += smi.o
> +mv88e6xxx-objs += switchdev.o
>  mv88e6xxx-objs += trace.o
>  
>  # for tracing framework to find trace.h
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index d5930b287db4..2682c2b29346 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -1729,11 +1729,11 @@ static int mv88e6xxx_vtu_get(struct mv88e6xxx_chip *chip, u16 vid,
>  	return err;
>  }
>  
> -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)
> +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),
> @@ -6527,7 +6527,7 @@ static int mv88e6xxx_port_pre_bridge_flags(struct dsa_switch *ds, int port,
>  	const struct mv88e6xxx_ops *ops;
>  
>  	if (flags.mask & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD |
> -			   BR_BCAST_FLOOD | BR_PORT_LOCKED))
> +			   BR_BCAST_FLOOD | BR_PORT_LOCKED | BR_PORT_MAB))
>  		return -EINVAL;
>  
>  	ops = chip->info->ops;
> @@ -6585,6 +6585,12 @@ static int mv88e6xxx_port_bridge_flags(struct dsa_switch *ds, int port,
>  			goto out;
>  	}
>  
> +	if (flags.mask & BR_PORT_MAB) {
> +		bool mab = !!(flags.val & BR_PORT_MAB);
> +
> +		mv88e6xxx_port_set_mab(chip, port, mab);
> +	}
> +
>  	if (flags.mask & BR_PORT_LOCKED) {
>  		bool locked = !!(flags.val & BR_PORT_LOCKED);
>  
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
> index e693154cf803..f635a5bb47ce 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.h
> +++ b/drivers/net/dsa/mv88e6xxx/chip.h
> @@ -280,6 +280,9 @@ struct mv88e6xxx_port {
>  	unsigned int serdes_irq;
>  	char serdes_irq_name[64];
>  	struct devlink_region *region;
> +
> +	/* MacAuth Bypass control flag */
> +	bool mab;
>  };
>  
>  enum mv88e6xxx_region_id {
> @@ -784,6 +787,12 @@ static inline bool mv88e6xxx_is_invalid_port(struct mv88e6xxx_chip *chip, int po
>  	return (chip->info->invalid_port_mask & BIT(port)) != 0;
>  }
>  
> +static inline void mv88e6xxx_port_set_mab(struct mv88e6xxx_chip *chip,
> +					  int port, bool mab)
> +{
> +	chip->ports[port].mab = mab;
> +}
> +
>  int mv88e6xxx_read(struct mv88e6xxx_chip *chip, int addr, int reg, u16 *val);
>  int mv88e6xxx_write(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val);
>  int mv88e6xxx_wait_mask(struct mv88e6xxx_chip *chip, int addr, int reg,
> @@ -802,6 +811,12 @@ static inline void mv88e6xxx_reg_unlock(struct mv88e6xxx_chip *chip)
>  	mutex_unlock(&chip->reg_lock);
>  }
>  
> +int mv88e6xxx_vtu_walk(struct mv88e6xxx_chip *chip,
> +		       int (*cb)(struct mv88e6xxx_chip *chip,
> +				 const struct mv88e6xxx_vtu_entry *entry,
> +				 void *priv),
> +		       void *priv);
> +
>  int mv88e6xxx_fid_map(struct mv88e6xxx_chip *chip, unsigned long *bitmap);
>  
>  #endif /* _MV88E6XXX_CHIP_H */
> diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
> index 34203e112eef..fc020161b7cf 100644
> --- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
> +++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
> @@ -12,6 +12,7 @@
>  
>  #include "chip.h"
>  #include "global1.h"
> +#include "switchdev.h"
>  #include "trace.h"
>  
>  /* Offset 0x01: ATU FID Register */
> @@ -443,6 +444,13 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
>  						   entry.portvec, entry.mac,
>  						   fid);
>  		chip->ports[spid].atu_miss_violation++;
> +
> +		if (fid != MV88E6XXX_FID_STANDALONE && chip->ports[spid].mab) {
> +			err = mv88e6xxx_handle_miss_violation(chip, spid,
> +							      &entry, fid);
> +			if (err)
> +				goto out;
> +		}
>  	}
>  
>  	if (val & MV88E6XXX_G1_ATU_OP_FULL_VIOLATION) {

In your earlier patch you had made it sound like you were encountering
an issue with the existing code. Now I realize that this is what you
needed to move the lock for. You might call that out in your earlier
patch as the description made it sound like it was solving an existing
deadlock instead of one that could be introduced with this change.

> diff --git a/drivers/net/dsa/mv88e6xxx/switchdev.c b/drivers/net/dsa/mv88e6xxx/switchdev.c
> new file mode 100644
> index 000000000000..4c346a884fb2
> --- /dev/null
> +++ b/drivers/net/dsa/mv88e6xxx/switchdev.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * switchdev.c
> + *
> + *	Authors:
> + *	Hans J. Schultz		<netdev@kapio-technology.com>
> + *
> + */
> +
> +#include <net/switchdev.h>
> +#include "chip.h"
> +#include "global1.h"
> +#include "switchdev.h"
> +
> +struct mv88e6xxx_fid_search_ctx {
> +	u16 fid_search;
> +	u16 vid_found;
> +};
> +
> +static int __mv88e6xxx_find_vid(struct mv88e6xxx_chip *chip,
> +				const struct mv88e6xxx_vtu_entry *entry,
> +				void *priv)
> +{
> +	struct mv88e6xxx_fid_search_ctx *ctx = priv;
> +
> +	if (ctx->fid_search == entry->fid) {
> +		ctx->vid_found = entry->vid;
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mv88e6xxx_find_vid(struct mv88e6xxx_chip *chip, u16 fid, u16 *vid)
> +{
> +	struct mv88e6xxx_fid_search_ctx ctx;
> +	int err;
> +
> +	ctx.fid_search = fid;
> +	mv88e6xxx_reg_lock(chip);
> +	err = mv88e6xxx_vtu_walk(chip, __mv88e6xxx_find_vid, &ctx);
> +	mv88e6xxx_reg_unlock(chip);
> +	if (err < 0)
> +		return err;
> +	if (err == 1)
> +		*vid = ctx.vid_found;
> +	else
> +		return -ENOENT;
> +
> +	return 0;
> +}
> +
> +int mv88e6xxx_handle_miss_violation(struct mv88e6xxx_chip *chip, int port,
> +				    struct mv88e6xxx_atu_entry *entry, u16 fid)
> +{
> +	struct switchdev_notifier_fdb_info info = {
> +		.addr = entry->mac,
> +		.locked = true,
> +	};
> +	struct net_device *brport;
> +	struct dsa_port *dp;
> +	u16 vid;
> +	int err;
> +
> +	err = mv88e6xxx_find_vid(chip, fid, &vid);
> +	if (err)
> +		return err;
> +
> +	info.vid = vid;
> +	dp = dsa_to_port(chip->ds, port);
> +
> +	rtnl_lock();
> +	brport = dsa_port_to_bridge_port(dp);
> +	if (!brport) {
> +		rtnl_unlock();
> +		return -ENODEV;
> +	}
> +	err = call_switchdev_notifiers(SWITCHDEV_FDB_ADD_TO_BRIDGE,
> +				       brport, &info.info, NULL);
> +	rtnl_unlock();
> +
> +	return err;
> +}
> diff --git a/drivers/net/dsa/mv88e6xxx/switchdev.h b/drivers/net/dsa/mv88e6xxx/switchdev.h
> new file mode 100644
> index 000000000000..62214f9d62b0
> --- /dev/null
> +++ b/drivers/net/dsa/mv88e6xxx/switchdev.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * switchdev.h
> + *
> + *	Authors:
> + *	Hans J. Schultz		<netdev@kapio-technology.com>
> + *
> + */
> +
> +#ifndef _MV88E6XXX_SWITCHDEV_H_
> +#define _MV88E6XXX_SWITCHDEV_H_
> +
> +#include "chip.h"
> +
> +int mv88e6xxx_handle_miss_violation(struct mv88e6xxx_chip *chip, int port,
> +				    struct mv88e6xxx_atu_entry *entry,
> +				    u16 fid);
> +
> +#endif /* _MV88E6XXX_SWITCHDEV_H_ */

Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>

Re: [PATCH v2 net-next 0/3] mv88e6xxx: Add MAB offload support

[Jakub Kicinski]

On Tue, 13 Dec 2022 18:46:47 +0100 Hans J. Schultz wrote:
> This patchset adds MAB [1] offload support in mv88e6xxx.
> 
> Patch #1: Correct default return value for mv88e6xxx_port_bridge_flags.
> 
> Patch #2: Change chip lock handling in ATU interrupt handler.
> 
> Patch #3: The MAB implementation for mv88e6xxx.

# Form letter - net-next is closed

We have already submitted the networking pull request to Linus
for v6.2 and therefore net-next is closed for new drivers, features,
code refactoring and optimizations. We are currently accepting
bug fixes only.

Please repost when net-next reopens after 6.2-rc1 is cut.

RFC patches sent for review only are obviously welcome at any time.

[PATCH v2 net-next 0/3] mv88e6xxx: Add MAB offload support

[Hans J. Schultz]

This patchset adds MAB [1] offload support in mv88e6xxx.

Patch #1: Correct default return value for mv88e6xxx_port_bridge_flags.

Patch #2: Change chip lock handling in ATU interrupt handler.

Patch #3: The MAB implementation for mv88e6xxx.

LOG:
        V2:     -FID reading patch already applied, so dropped here. [1]
                -Patch #2 here as separate patch instead of part of MAB
                 implementation patch.
                -Check if fid is MV88E6XXX_FID_STANDALONE, and not if
                 fid is zero, as that is the correct check. Do not
                 report an error.

[1] https://git.kernel.org/netdev/net-next/c/4bf24ad09bc0

Hans J. Schultz (3):
  net: dsa: mv88e6xxx: change default return of
    mv88e6xxx_port_bridge_flags
  net: dsa: mv88e6xxx: disable hold of chip lock for handling
  net: dsa: mv88e6xxx: mac-auth/MAB implementation

 drivers/net/dsa/mv88e6xxx/Makefile      |  1 +
 drivers/net/dsa/mv88e6xxx/chip.c        | 20 +++---
 drivers/net/dsa/mv88e6xxx/chip.h        | 15 +++++
 drivers/net/dsa/mv88e6xxx/global1_atu.c | 22 +++++--
 drivers/net/dsa/mv88e6xxx/switchdev.c   | 83 +++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/switchdev.h   | 19 ++++++
 6 files changed, 147 insertions(+), 13 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6xxx/switchdev.c
 create mode 100644 drivers/net/dsa/mv88e6xxx/switchdev.h

-- 
2.34.1