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

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

[Hans J. Schultz]

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

Patch #1: Fix a problem when reading the FID needed to get the VID.

Patch #2: Correct default return value for mv88e6xxx_port_bridge_flags.

Patch #2: The MAB implementation for mv88e6xxx.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=a35ec8e38cdd1766f29924ca391a01de20163931

Hans J. Schultz (3):
  net: dsa: mv88e6xxx: allow reading FID when handling ATU violations
  net: dsa: mv88e6xxx: change default return of
    mv88e6xxx_port_bridge_flags
  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 | 87 +++++++++++++++++++++----
 drivers/net/dsa/mv88e6xxx/switchdev.c   | 83 +++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/switchdev.h   | 19 ++++++
 6 files changed, 207 insertions(+), 18 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6xxx/switchdev.c
 create mode 100644 drivers/net/dsa/mv88e6xxx/switchdev.h

-- 
2.34.1

[PATCH net-next 1/3] net: dsa: mv88e6xxx: allow reading FID when handling ATU violations

[Hans J. Schultz]

The FID is needed to get hold of which VID was involved in a violation,
thus the need to be able to read the FID.

For convenience the function mv88e6xxx_g1_atu_op() has been used to read
ATU violations, but the function invalidates reading the fid, so to both
read ATU violations without zeroing the fid, and read the fid, functions
have been added to ensure both are done correctly.

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

diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
index 40bd67a5c8e9..8a874b6fc8e1 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
@@ -114,6 +114,19 @@ static int mv88e6xxx_g1_atu_op_wait(struct mv88e6xxx_chip *chip)
 	return mv88e6xxx_g1_wait_bit(chip, MV88E6XXX_G1_ATU_OP, bit, 0);
 }
 
+static int mv88e6xxx_g1_read_atu_violation(struct mv88e6xxx_chip *chip)
+{
+	int err;
+
+	err = mv88e6xxx_g1_write(chip, MV88E6XXX_G1_ATU_OP,
+				 MV88E6XXX_G1_ATU_OP_BUSY |
+				 MV88E6XXX_G1_ATU_OP_GET_CLR_VIOLATION);
+	if (err)
+		return err;
+
+	return mv88e6xxx_g1_atu_op_wait(chip);
+}
+
 static int mv88e6xxx_g1_atu_op(struct mv88e6xxx_chip *chip, u16 fid, u16 op)
 {
 	u16 val;
@@ -159,6 +172,41 @@ int mv88e6xxx_g1_atu_get_next(struct mv88e6xxx_chip *chip, u16 fid)
 	return mv88e6xxx_g1_atu_op(chip, fid, MV88E6XXX_G1_ATU_OP_GET_NEXT_DB);
 }
 
+static int mv88e6xxx_g1_atu_fid_read(struct mv88e6xxx_chip *chip, u16 *fid)
+{
+	u16 val = 0, upper = 0, op = 0;
+	int err = -EOPNOTSUPP;
+
+	if (mv88e6xxx_num_databases(chip) > 256) {
+		err = mv88e6xxx_g1_read(chip, MV88E6352_G1_ATU_FID, &val);
+		val &= 0xfff;
+		if (err)
+			return err;
+	} else {
+		err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_OP, &op);
+		if (err)
+			return err;
+		if (mv88e6xxx_num_databases(chip) > 64) {
+			/* ATU DBNum[7:4] are located in ATU Control 15:12 */
+			err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_CTL,
+						&upper);
+			if (err)
+				return err;
+
+			upper = (upper >> 8) & 0x00f0;
+		} else if (mv88e6xxx_num_databases(chip) > 16) {
+			/* ATU DBNum[5:4] are located in ATU Operation 9:8 */
+			upper = (op >> 4) & 0x30;
+		}
+
+		/* ATU DBNum[3:0] are located in ATU Operation 3:0 */
+		val = (op & 0xf) | upper;
+	}
+	*fid = val;
+
+	return err;
+}
+
 /* Offset 0x0C: ATU Data Register */
 
 static int mv88e6xxx_g1_atu_data_read(struct mv88e6xxx_chip *chip,
@@ -353,14 +401,12 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
 {
 	struct mv88e6xxx_chip *chip = dev_id;
 	struct mv88e6xxx_atu_entry entry;
-	int spid;
-	int err;
-	u16 val;
+	int err, spid;
+	u16 val, fid;
 
 	mv88e6xxx_reg_lock(chip);
 
-	err = mv88e6xxx_g1_atu_op(chip, 0,
-				  MV88E6XXX_G1_ATU_OP_GET_CLR_VIOLATION);
+	err = mv88e6xxx_g1_read_atu_violation(chip);
 	if (err)
 		goto out;
 
@@ -368,6 +414,10 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
 	if (err)
 		goto out;
 
+	err = mv88e6xxx_g1_atu_fid_read(chip, &fid);
+	if (err)
+		goto out;
+
 	err = mv88e6xxx_g1_atu_data_read(chip, &entry);
 	if (err)
 		goto out;
-- 
2.34.1

[PATCH net-next 2/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.

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 ccfa4751d3b7..66d7eae24ce0 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -6543,7 +6543,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 net-next 2/3] net: dsa: mv88e6xxx: change default return of mv88e6xxx_port_bridge_flags

[Ido Schimmel]

On Mon, Dec 05, 2022 at 07:59:07PM +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.

The commit message needs to explain the motivation for the change and
why the change is not a bug fix / safe. I guess the motivation is the
next patch where a change in the MAB flag cannot fail and therefore it
has no reason to reset the 'err' variable. The change is only safe if
upper layers only invoke the operation when supported bridge port flags
are changed. That is, the default error code is never used.

[PATCH 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. 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.

An anomaly has been observed, where the ATU op to read the FID reports
FID=0 even though it is not a valid read. An -EINVAL error will be logged
in this case. This was seen on a mv88e6097.

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 | 29 ++++++---
 drivers/net/dsa/mv88e6xxx/switchdev.c   | 83 +++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/switchdev.h   | 19 ++++++
 6 files changed, 152 insertions(+), 13 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 c8eca2b6f959..be903a983780 100644
--- a/drivers/net/dsa/mv88e6xxx/Makefile
+++ b/drivers/net/dsa/mv88e6xxx/Makefile
@@ -15,3 +15,4 @@ 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
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 66d7eae24ce0..732d7a2e2a07 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1726,11 +1726,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),
@@ -6524,7 +6524,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;
@@ -6582,6 +6582,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 8a874b6fc8e1..bb004df517b2 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"
 
 /* Offset 0x01: ATU FID Register */
 
@@ -408,23 +409,25 @@ 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)
-		goto out;
+		goto out_unlock;
 
 	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;
 
@@ -446,6 +449,18 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
 				    "ATU miss violation for %pM portvec %x spid %d\n",
 				    entry.mac, entry.portvec, spid);
 		chip->ports[spid].atu_miss_violation++;
+
+		if (!fid) {
+			err = -EINVAL;
+			goto out;
+		}
+
+		if (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) {
@@ -454,13 +469,13 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
 				    entry.mac, entry.portvec, spid);
 		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;
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 net-next 3/3] net: dsa: mv88e6xxx: mac-auth/MAB implementation

[Ido Schimmel]

On Mon, Dec 05, 2022 at 07:59:08PM +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, right?

> 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.
> 
> An anomaly has been observed, where the ATU op to read the FID reports
> FID=0 even though it is not a valid read. An -EINVAL error will be logged
> in this case. This was seen on a mv88e6097.
> 
> Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>
> ---

The changelog from previous versions is missing.

>  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 | 29 ++++++---
>  drivers/net/dsa/mv88e6xxx/switchdev.c   | 83 +++++++++++++++++++++++++
>  drivers/net/dsa/mv88e6xxx/switchdev.h   | 19 ++++++
>  6 files changed, 152 insertions(+), 13 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 c8eca2b6f959..be903a983780 100644
> --- a/drivers/net/dsa/mv88e6xxx/Makefile
> +++ b/drivers/net/dsa/mv88e6xxx/Makefile
> @@ -15,3 +15,4 @@ 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
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 66d7eae24ce0..732d7a2e2a07 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -1726,11 +1726,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),
> @@ -6524,7 +6524,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;
> @@ -6582,6 +6582,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 8a874b6fc8e1..bb004df517b2 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"
>  
>  /* Offset 0x01: ATU FID Register */
>  
> @@ -408,23 +409,25 @@ 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)
> -		goto out;
> +		goto out_unlock;
>  
>  	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);

I was under the impression that we agreed that the locking change will
be split to a separate patch.

>  
>  	spid = entry.state;

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

[netdev]

On 2022-12-06 13:53, Ido Schimmel wrote:
> On Mon, Dec 05, 2022 at 07:59:08PM +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, right?

Every packet that enters a locked port and does not have a matching ATU 
entry on the port will be dropped (in HW) afaik.

>> ---
> 
> The changelog from previous versions is missing.
> 

I am afraid because I made a mistake with the version string last, this 
should be regarded as a first. Therefore no changelog.

>>  	err = mv88e6xxx_g1_atu_mac_read(chip, &entry);
>>  	if (err)
>> -		goto out;
>> +		goto out_unlock;
>> +
>> +	mv88e6xxx_reg_unlock(chip);
> 
> I was under the impression that we agreed that the locking change will
> be split to a separate patch.
> 

Sorry, I guess that because of the quite long time that has passed as I 
needed to get this FID=0 issue sorted out, and had many other different 
changes to attend, I forgot. I see an updated version is needed anyhow, 
so I will do it there.

>> 
>>  	spid = entry.state;

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

[Vladimir Oltean]

On Tue, Dec 06, 2022 at 05:36:42PM +0100, netdev@kapio-technology.com wrote:
> > I was under the impression that we agreed that the locking change will
> > be split to a separate patch.
> 
> Sorry, I guess that because of the quite long time that has passed as I
> needed to get this FID=0 issue sorted out, and had many other different
> changes to attend, I forgot.

Well, at least you got the FID=0 issue sorted out... right?
What was the cause, what is the solution?

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

[netdev]

On 2022-12-07 21:29, Vladimir Oltean wrote:
> On Tue, Dec 06, 2022 at 05:36:42PM +0100, netdev@kapio-technology.com 
> wrote:
>> > I was under the impression that we agreed that the locking change will
>> > be split to a separate patch.
>> 
>> Sorry, I guess that because of the quite long time that has passed as 
>> I
>> needed to get this FID=0 issue sorted out, and had many other 
>> different
>> changes to attend, I forgot.
> 
> Well, at least you got the FID=0 issue sorted out... right?
> What was the cause, what is the solution?

Well I got it sorted out in the way that I have identified that it is 
the ATU op that fails some times. I don't think there is anything that 
can be done about that, other than what I do and let the interrupt 
routing return an error.

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

[Vladimir Oltean]

On Thu, Dec 08, 2022 at 01:28:27PM +0100, netdev@kapio-technology.com wrote:
> On 2022-12-07 21:29, Vladimir Oltean wrote:
> > On Tue, Dec 06, 2022 at 05:36:42PM +0100, netdev@kapio-technology.com wrote:
> > > > I was under the impression that we agreed that the locking change will
> > > > be split to a separate patch.
> > > 
> > > Sorry, I guess that because of the quite long time that has passed as I
> > > needed to get this FID=0 issue sorted out, and had many other different
> > > changes to attend, I forgot.
> > 
> > Well, at least you got the FID=0 issue sorted out... right?
> > What was the cause, what is the solution?
> 
> Well I got it sorted out in the way that I have identified that it is the
> ATU op that fails some times. I don't think there is anything that can be
> done about that, other than what I do and let the interrupt routing return
> an error.

Yikes. But why would you call that "sorted out", though? Just to make it
appear as though you really spent some time on it, and use it as an
excuse for something else?

> it is the ATU op that fails some times.

Let's start with the assumption that this is correct. A person with
critical thinking will ask "can I prove that it is?".

If the ATU operation fails sometimes, I would expect that it always
fails in the same way, by returning FID 0, where 0 is some kind of
"invalid" value.

But if FID 0 is actually FID_STANDALONE, then you'd read FID_STANDALONE
even if you change the value of FID_STANDALONE in the driver to
something else, like 1.

Something ultra hackish like this will install VLAN 3050 as first VID in
the switch, and that will gain FID 0. Then, MV886XXX_VID_STANDALONE will
gain FID 1. So we need to adjust the definitions.

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index ccfa4751d3b7..5923cbb172f9 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3410,9 +3410,15 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 	if (err)
 		return err;
 
+	err = mv88e6xxx_port_vlan_join(chip, port, 3050,
+				       MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_UNMODIFIED,
+				       false);
+	if (err)
+		return err;
+
 	/* Bind MV88E6XXX_VID_STANDALONE to MV88E6XXX_FID_STANDALONE by
 	 * virtue of the fact that mv88e6xxx_atu_new() will pick it as
-	 * the first free FID. This will be used as the private PVID for
+	 * the second free FID. This will be used as the private PVID for
 	 * unbridged ports. Shared (DSA and CPU) ports must also be
 	 * members of this VID, in order to trap all frames assigned to
 	 * it to the CPU.
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index e693154cf803..48d4db4f2adf 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -22,8 +22,8 @@
 #define MV88E6XXX_N_FID		4096
 #define MV88E6XXX_N_SID		64
 
-#define MV88E6XXX_FID_STANDALONE	0
-#define MV88E6XXX_FID_BRIDGED		1
+#define MV88E6XXX_FID_STANDALONE	1
+#define MV88E6XXX_FID_BRIDGED		2
 
 /* PVT limits for 4-bit port and 5-bit switch */
 #define MV88E6XXX_MAX_PVT_SWITCHES	32

Now back to running the ./bridge_locked_port.sh selftest. One can now
see that whereas before, the errors we got from time to time with FID 0
are now reported with FID 1.

So it is MV88E6XXX_FID_STANDALONE, after all, not just 0.

So why, then? And most importantly, when in the selftest does this
problem occur, and as a result of which traffic?

If you actually open the selftest and put some prints in the areas of
the failure, you might be tempted to think that it's the "ping_do $h1 192.0.2.2"
command that causes it.

locked_port_mab()
{
	RET=0
	check_port_mab_support || return 0

	ping_do $h1 192.0.2.2
	check_err $? "Ping did not work before locking port"

	bridge link set dev $swp1 learning on locked on

	ping_do $h1 192.0.2.2
	check_fail $? "Ping worked on a locked port without an FDB entry"

	bridge fdb get `mac_get $h1` br br0 vlan 1 &> /dev/null
	check_fail $? "FDB entry created before enabling MAB"

	bridge link set dev $swp1 learning on locked on mab on

	set -x

	ping_do $h1 192.0.2.2
	check_fail $? "Ping worked on MAB enabled port without an FDB entry"

	set +x
	bash

	bridge fdb get `mac_get $h1` br br0 vlan 1 | grep "dev $swp1" | grep -q "locked"
	check_err $? "Locked FDB entry not created"

	bridge fdb replace `mac_get $h1` dev $swp1 master static

	ping_do $h1 192.0.2.2
	check_err $? "Ping did not work after replacing FDB entry"

	bridge fdb get `mac_get $h1` br br0 vlan 1 | grep "dev $swp1" | grep -q "locked"
	check_fail $? "FDB entry marked as locked after replacement"

	bridge fdb del `mac_get $h1` dev $swp1 master
	bridge link set dev $swp1 learning off locked off mab off

	log_test "Locked port MAB"
}

"Interesting", you might say. So stop the selftest execution there, and
run that ping again, and again, and again.

But the packets from $h1 consistently get classified to the correct FID
(the FID of the bridge port's VLAN-aware PVID).

But from time to time, those ATU errors with FID_STANDALONE keep popping up.
Strangely, it doesn't seem to be related to the ping command, at all, in
that the errors appear even while there's no pinging going on.

And then you realize, but hey, there's also a VLAN interface in the
network, created by "vlan_create $h1 100 v$h1 198.51.100.1/24".
And VID 100, at the time of the locked_port_mab() selftest, is not
present in the bridge VLAN database of port $h2.

So instead of
	ping_do $h1 192.0.2.2

why don't we try to do

	ping_do $h1.100 198.51.100.2

and actually ping over that VLAN interface?

And surprise surprise, now every packet with VID 100 gets classified to
FID_STANDALONE, and the problem is 100% reproducible.

The reverse is also true. You delete the "vlan_create" commands and you
skip the selftests that use the $h1.100 interface, and the problem goes
away.

Then, the next step is opening the documentation. If you look at Figure 23
"Relationship between VTU, STU and ATU", it will say that "the port's
Default FID is used if 802.1Q is disabled on the port or if the VID
assigned to the frame points to an empty VTU entry".

VID 100 indeed points to an empty VTU entry.

The port Default FID is set with this call:

	/* Port based VLAN map: give each port the same default address
	 * database, and allow bidirectional communication between the
	 * CPU and DSA port(s), and the other ports.
	 */
	err = mv88e6xxx_port_set_fid(chip, port, MV88E6XXX_FID_STANDALONE);
	if (err)
		return err;

So it appears that frames which get a VTU miss will still also cause an
ATU miss, and that's what you're seeing.

The solution would be to acknowledge this fact, and not print any error
message from the ATU IRQ handler for unknown FID/VID, which would just
alarm the user.

What I wanted to say with this is that it doesn't take a mad scientist
to figure this stuff out, just somebody who is motivated not to throw
half assed stuff over the fence to the mainline kernel, and just call it
a day when his project is over. I don't even interact with Marvell
hardware on a day to day basis, what I know is just information gathered
during patch review, and I could still figure out what's going on.

There's actually a deeper problem which concerns me more. I'm extremely
fed up with seeing this patch set progress so slowly, to the point where
I've considered a few times to just say fsck it, it's good enough, it's
not going to get better in your hands, let it be merged, and I'll take a
second look when I'll have some time to spare and I'll clean up what I
can. Now with Ido's help for the software bridge and MAB offload part,
the patch set is really so small and so close to getting accepted, that
I don't see what's holding you up, really. The review comments are
absolutely actionable. My dilemma is that I don't think it's okay that
there exists this "merge patch set through reviewer exhaustion" loophole.
But on the other hand, if as a reviewer I don't want that to happen,
I have to waste my time with people who simply don't want to use their
critical thinking, test on actual hardware what they've done, find
problems that they didn't want to tackle. This is also the reason why I
sent the tracepoints patch set, which I really wanted *you* to do, not
later than the MAB/locked port support itself. I find the current state
of your patch set fairly unusable from the perspective of a user who
uses a serial console. But I can't waste an unlimited amount of time,
I have other stuff to do, too. I hope we can find a compromise where you
can be more responsive to what you're being told during review.

Thanks for listening.

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

[netdev]

On 2022-12-08 14:35, Vladimir Oltean wrote:
> On Thu, Dec 08, 2022 at 01:28:27PM +0100, netdev@kapio-technology.com 
> wrote:
>> On 2022-12-07 21:29, Vladimir Oltean wrote:
>> > On Tue, Dec 06, 2022 at 05:36:42PM +0100, netdev@kapio-technology.com wrote:
>> > > > I was under the impression that we agreed that the locking change will
>> > > > be split to a separate patch.
>> > >
>> > > Sorry, I guess that because of the quite long time that has passed as I
>> > > needed to get this FID=0 issue sorted out, and had many other different
>> > > changes to attend, I forgot.
>> >
>> > Well, at least you got the FID=0 issue sorted out... right?
>> > What was the cause, what is the solution?
>> 
>> Well I got it sorted out in the way that I have identified that it is 
>> the
>> ATU op that fails some times. I don't think there is anything that can 
>> be
>> done about that, other than what I do and let the interrupt routing 
>> return
>> an error.
> 
> Yikes. But why would you call that "sorted out", though? Just to make 
> it
> appear as though you really spent some time on it, and use it as an
> excuse for something else?
> 
>> it is the ATU op that fails some times.
> 
> Let's start with the assumption that this is correct. A person with
> critical thinking will ask "can I prove that it is?".
> 
> If the ATU operation fails sometimes, I would expect that it always
> fails in the same way, by returning FID 0, where 0 is some kind of
> "invalid" value.
> 
> But if FID 0 is actually FID_STANDALONE, then you'd read FID_STANDALONE
> even if you change the value of FID_STANDALONE in the driver to
> something else, like 1.
> 
> Something ultra hackish like this will install VLAN 3050 as first VID 
> in
> the switch, and that will gain FID 0. Then, MV886XXX_VID_STANDALONE 
> will
> gain FID 1. So we need to adjust the definitions.
> 

Here is an example of the output I have when running the 
locked_port_mab() under the selftests...

mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU problem: error -22 while handling 
interrupt
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU problem: error -22 while handling 
interrupt
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU problem: error -22 while handling 
interrupt
mv88e6085 1002b000.ethernet-1:04: ATU problem: error -22 while handling 
interrupt
mv88e6085 1002b000.ethernet-1:04: ATU problem: error -22 while handling 
interrupt
mv88e6xxx_g1_atu_prob_irq_thread_fn: 13 callbacks suppressed
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU problem: error -22 while handling 
interrupt
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU problem: error -22 while handling 
interrupt
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2

the -22 errors are all when it returns FID=0, and it is the same mac all 
the way.


I have other logs, where the -22 occurs at random other times, f.ex. 
same test:

mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU problem: error -22 while handling 
interrupt
mv88e6xxx_g1_atu_prob_irq_thread_fn: 4 callbacks suppressed
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU problem: error -22 while handling 
interrupt
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU miss violation for 
8e:a9:fc:14:58:06 portvec 0 spid 2
mv88e6085 1002b000.ethernet-1:04: ATU problem: error -22 while handling 
interrupt

What else conclusion than it is the ATU op that fails?

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

[Vladimir Oltean]

On Thu, Dec 08, 2022 at 03:41:24PM +0100, netdev@kapio-technology.com wrote:
> What else conclusion than it is the ATU op that fails?

I don't have any more time than to say "read the rest of my email", sorry.

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

[netdev]

On 2022-12-08 14:35, Vladimir Oltean wrote:
> 
> So it appears that frames which get a VTU miss will still also cause an
> ATU miss, and that's what you're seeing.
> 
> The solution would be to acknowledge this fact, and not print any error
> message from the ATU IRQ handler for unknown FID/VID, which would just
> alarm the user.

Thanks for clearing that up!

At leisure, do you have an idea why it will encounter a VTU miss 
violation at random?

I guess I must check if FID != FID_STANDALONE instead then...

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

[Vladimir Oltean]

On Thu, Dec 08, 2022 at 05:03:59PM +0100, netdev@kapio-technology.com wrote:
> At leisure, do you have an idea why it will encounter a VTU miss violation
> at random?

Do you understand that any packets with a VID that isn't present in the
VTU will trigger a VTU miss, and also an ATU miss if the port is locked
and the ATU doesn't have an entry with that FID?

Your selftest creates a VLAN interface on top of $h1 with a VID that
isn't present in the VTU of the switch ("bridge vlan add .. vid 100" is
run elsewhere; we run "bridge vlan del ... vid 100" when we no longer
need it). But the $h1.100 interface is persistent across the selftest.
And it's not silent. Linux does all sorts of crap by default, like IPv6
neighbor discovery, even if you don't use the interface. So it will send
packets from time to time. And that's when you get those ATU and VTU
violations. The MAC address of $h1.100 is the same as the MAC address of
$h1, of course.