[PATCH net-next 0/3] Trace points for mv88e6xxx

[PATCH net-next 0/3] Trace points for mv88e6xxx

[Vladimir Oltean]

While testing Hans Schultz' attempt at offloading MAB on mv88e6xxx:
https://patchwork.kernel.org/project/netdevbpf/cover/20221205185908.217520-1-netdev@kapio-technology.com/
I noticed that he still didn't get rid of the huge log spam caused by
ATU and VTU violations, even if we discussed about this:
https://patchwork.kernel.org/project/netdevbpf/cover/20221112203748.68995-1-netdev@kapio-technology.com/#25091076

It seems unlikely he's going to ever do this, so here is my own stab at
converting those messages to trace points. This is IMO an improvement
regardless of whether Hans' work with MAB lands or not, especially the
VTU violations which were quite annoying to me as well.

A small sample of before:

$ ./bridge_locked_port.sh lan1 lan2 lan3 lan4
[  114.465272] mv88e6085 d0032004.mdio-mii:10: VTU member violation for vid 100, source port 9
[  119.550508] mv88e6xxx_g1_vtu_prob_irq_thread_fn: 34 callbacks suppressed
[  120.369586] mv88e6085 d0032004.mdio-mii:10: VTU member violation for vid 100, source port 9
[  120.473658] mv88e6085 d0032004.mdio-mii:10: VTU member violation for vid 100, source port 9
[  125.535209] mv88e6xxx_g1_vtu_prob_irq_thread_fn: 21 callbacks suppressed
[  125.535243] mv88e6085 d0032004.mdio-mii:10: VTU member violation for vid 100, source port 9
[  125.981327] mv88e6085 d0032004.mdio-mii:10: VTU member violation for vid 100, source port 9
[  126.048694] mv88e6085 d0032004.mdio-mii:10: VTU member violation for vid 100, source port 9
[  126.090625] mv88e6085 d0032004.mdio-mii:10: VTU member violation for vid 100, source port 9
[  126.174558] mv88e6085 d0032004.mdio-mii:10: VTU member violation for vid 100, source port 9
[  129.400356] mv88e6085 d0032004.mdio-mii:10: ATU miss violation for 00:01:02:03:04:01 fid 3 portvec 4 spid 2
[  130.234055] mv88e6085 d0032004.mdio-mii:10: ATU miss violation for 00:01:02:03:04:01 fid 3 portvec 4 spid 2
[  130.338193] mv88e6085 d0032004.mdio-mii:10: ATU miss violation for 00:01:02:03:04:01 fid 3 portvec 4 spid 2
[  134.626099] mv88e6xxx_g1_atu_prob_irq_thread_fn: 38 callbacks suppressed
[  134.626132] mv88e6085 d0032004.mdio-mii:10: ATU miss violation for 00:01:02:03:04:01 fid 3 portvec 4 spid 2

and after:

$ trace-cmd record -e mv88e6xxx ./bridge_locked_port.sh lan1 lan2 lan3 lan4
$ trace-cmd report
   irq/35-moxtet-60    [000]    74.386799: mv88e6xxx_vtu_miss_violation: dev d0032004.mdio-mii:10 port 9 vid 100
   irq/35-moxtet-60    [000]    76.834759: mv88e6xxx_vtu_miss_violation: dev d0032004.mdio-mii:10 port 9 vid 100
   irq/35-moxtet-60    [000]    86.537973: mv88e6xxx_vtu_miss_violation: dev d0032004.mdio-mii:10 port 9 vid 100
   irq/35-moxtet-60    [000]    87.553885: mv88e6xxx_vtu_miss_violation: dev d0032004.mdio-mii:10 port 9 vid 100
   irq/35-moxtet-60    [000]   100.583426: mv88e6xxx_vtu_miss_violation: dev d0032004.mdio-mii:10 port 9 vid 100
   irq/35-moxtet-60    [000]   108.550520: mv88e6xxx_vtu_member_violation: dev d0032004.mdio-mii:10 port 9 vid 100
   irq/35-moxtet-60    [000]   109.054410: mv88e6xxx_vtu_member_violation: dev d0032004.mdio-mii:10 port 9 vid 100
   irq/35-moxtet-60    [000]   123.586896: mv88e6xxx_atu_miss_violation: dev d0032004.mdio-mii:10 port 2 addr 00:01:02:03:04:01 fid 3
   irq/35-moxtet-60    [000]   126.315529: mv88e6xxx_atu_miss_violation: dev d0032004.mdio-mii:10 port 2 addr 00:01:02:03:04:01 fid 3
   irq/35-moxtet-60    [000]   126.400709: mv88e6xxx_atu_miss_violation: dev d0032004.mdio-mii:10 port 2 addr 00:01:02:03:04:01 fid 3
   irq/35-moxtet-60    [000]   126.947391: mv88e6xxx_atu_miss_violation: dev d0032004.mdio-mii:10 port 2 addr 00:01:02:03:04:01 fid 3
   irq/35-moxtet-60    [000]   127.985090: mv88e6xxx_vtu_miss_violation: dev d0032004.mdio-mii:10 port 9 vid 100
   irq/35-moxtet-60    [000]   128.059140: mv88e6xxx_atu_miss_violation: dev d0032004.mdio-mii:10 port 2 addr 00:01:02:03:04:01 fid 3
   irq/35-moxtet-60    [000]   128.163132: mv88e6xxx_atu_miss_violation: dev d0032004.mdio-mii:10 port 2 addr 00:01:02:03:04:01 fid 3

Hans J. Schultz (1):
  net: dsa: mv88e6xxx: read FID when handling ATU violations

Vladimir Oltean (2):
  net: dsa: mv88e6xxx: replace ATU violation prints with trace points
  net: dsa: mv88e6xxx: replace VTU violation prints with trace points

 drivers/net/dsa/mv88e6xxx/Makefile      |  4 +
 drivers/net/dsa/mv88e6xxx/global1_atu.c | 81 +++++++++++++++-----
 drivers/net/dsa/mv88e6xxx/global1_vtu.c |  7 +-
 drivers/net/dsa/mv88e6xxx/trace.c       |  6 ++
 drivers/net/dsa/mv88e6xxx/trace.h       | 98 +++++++++++++++++++++++++
 5 files changed, 175 insertions(+), 21 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6xxx/trace.c
 create mode 100644 drivers/net/dsa/mv88e6xxx/trace.h

-- 
2.34.1

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

[Vladimir Oltean]

From: "Hans J. Schultz" <netdev@kapio-technology.com>

When an ATU violation occurs, the switch uses the ATU FID register to
report the FID of the MAC address that incurred the violation. It would
be good for the driver to know the FID value for purposes such as
logging and CPU-based authentication.

Up until now, the driver has been calling the mv88e6xxx_g1_atu_op()
function to read ATU violations, but that doesn't do exactly what we
want, namely it calls mv88e6xxx_g1_atu_fid_write() with FID 0.
(side note, the documentation for the ATU Get/Clear Violation command
says that writes to the ATU FID register have no effect before the
operation starts, it's only that we disregard the value that this
register provides once the operation completes)

So mv88e6xxx_g1_atu_fid_write() is not what we want, but rather
mv88e6xxx_g1_atu_fid_read(). However, the latter doesn't exist, we need
to write it.

The remainder of mv88e6xxx_g1_atu_op() except for
mv88e6xxx_g1_atu_fid_write() is still needed, namely to send a
GET_CLR_VIOLATION command to the ATU. In principle we could have still
kept calling mv88e6xxx_g1_atu_op(), but the MDIO writes to the ATU FID
register are pointless, but in the interest of doing less CPU work per
interrupt, write a new function called mv88e6xxx_g1_read_atu_violation()
and call it.

Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/mv88e6xxx/global1_atu.c | 76 ++++++++++++++++++++-----
 1 file changed, 63 insertions(+), 13 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
index 40bd67a5c8e9..a9e2ff7d0e52 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;
@@ -380,28 +430,28 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
 
 	if (val & MV88E6XXX_G1_ATU_OP_AGE_OUT_VIOLATION) {
 		dev_err_ratelimited(chip->dev,
-				    "ATU age out violation for %pM\n",
-				    entry.mac);
+				    "ATU age out violation for %pM fid %u\n",
+				    entry.mac, fid);
 	}
 
 	if (val & MV88E6XXX_G1_ATU_OP_MEMBER_VIOLATION) {
 		dev_err_ratelimited(chip->dev,
-				    "ATU member violation for %pM portvec %x spid %d\n",
-				    entry.mac, entry.portvec, spid);
+				    "ATU member violation for %pM fid %u portvec %x spid %d\n",
+				    entry.mac, fid, entry.portvec, spid);
 		chip->ports[spid].atu_member_violation++;
 	}
 
 	if (val & MV88E6XXX_G1_ATU_OP_MISS_VIOLATION) {
 		dev_err_ratelimited(chip->dev,
-				    "ATU miss violation for %pM portvec %x spid %d\n",
-				    entry.mac, entry.portvec, spid);
+				    "ATU miss violation for %pM fid %u portvec %x spid %d\n",
+				    entry.mac, fid, entry.portvec, spid);
 		chip->ports[spid].atu_miss_violation++;
 	}
 
 	if (val & MV88E6XXX_G1_ATU_OP_FULL_VIOLATION) {
 		dev_err_ratelimited(chip->dev,
-				    "ATU full violation for %pM portvec %x spid %d\n",
-				    entry.mac, entry.portvec, spid);
+				    "ATU full violation for %pM fid %u portvec %x spid %d\n",
+				    entry.mac, fid, entry.portvec, spid);
 		chip->ports[spid].atu_full_violation++;
 	}
 	mv88e6xxx_reg_unlock(chip);
-- 
2.34.1

[PATCH net-next 2/3] net: dsa: mv88e6xxx: replace ATU violation prints with trace points

[Vladimir Oltean]

In applications where the switch ports must perform 802.1X based
authentication and are therefore locked, ATU violation interrupts are
quite to be expected as part of normal operation. The problem is that
they currently spam the kernel log, even if rate limited.

Create a series of trace points, all derived from the same event class,
which log these violations to the kernel's trace buffer, which is both
much faster and much easier to ignore than printing to a serial console.

I've deliberately stopped reporting the portvec, since in my experience
it contains redundant information with the spid (port) field: portvec ==
1 << spid.

New usage model:

$ trace-cmd list | grep mv88e6xxx
mv88e6xxx
mv88e6xxx:mv88e6xxx_atu_full_violation
mv88e6xxx:mv88e6xxx_atu_miss_violation
mv88e6xxx:mv88e6xxx_atu_member_violation
mv88e6xxx:mv88e6xxx_atu_age_out_violation
$ trace-cmd record -e mv88e6xxx sleep 10

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/mv88e6xxx/Makefile      |  4 ++
 drivers/net/dsa/mv88e6xxx/global1_atu.c | 21 ++++----
 drivers/net/dsa/mv88e6xxx/trace.c       |  6 +++
 drivers/net/dsa/mv88e6xxx/trace.h       | 68 +++++++++++++++++++++++++
 4 files changed, 87 insertions(+), 12 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6xxx/trace.c
 create mode 100644 drivers/net/dsa/mv88e6xxx/trace.h

diff --git a/drivers/net/dsa/mv88e6xxx/Makefile b/drivers/net/dsa/mv88e6xxx/Makefile
index c8eca2b6f959..49bf358b9c4f 100644
--- a/drivers/net/dsa/mv88e6xxx/Makefile
+++ b/drivers/net/dsa/mv88e6xxx/Makefile
@@ -15,3 +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 += trace.o
+
+# for tracing framework to find trace.h
+CFLAGS_trace.o := -I$(src)
diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
index a9e2ff7d0e52..6ba65b723b42 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 "trace.h"
 
 /* Offset 0x01: ATU FID Register */
 
@@ -429,29 +430,25 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
 	spid = entry.state;
 
 	if (val & MV88E6XXX_G1_ATU_OP_AGE_OUT_VIOLATION) {
-		dev_err_ratelimited(chip->dev,
-				    "ATU age out violation for %pM fid %u\n",
-				    entry.mac, fid);
+		trace_mv88e6xxx_atu_age_out_violation(chip->dev, spid,
+						      entry.mac, fid);
 	}
 
 	if (val & MV88E6XXX_G1_ATU_OP_MEMBER_VIOLATION) {
-		dev_err_ratelimited(chip->dev,
-				    "ATU member violation for %pM fid %u portvec %x spid %d\n",
-				    entry.mac, fid, entry.portvec, spid);
+		trace_mv88e6xxx_atu_member_violation(chip->dev, spid,
+						     entry.mac, fid);
 		chip->ports[spid].atu_member_violation++;
 	}
 
 	if (val & MV88E6XXX_G1_ATU_OP_MISS_VIOLATION) {
-		dev_err_ratelimited(chip->dev,
-				    "ATU miss violation for %pM fid %u portvec %x spid %d\n",
-				    entry.mac, fid, entry.portvec, spid);
+		trace_mv88e6xxx_atu_miss_violation(chip->dev, spid,
+						   entry.mac, fid);
 		chip->ports[spid].atu_miss_violation++;
 	}
 
 	if (val & MV88E6XXX_G1_ATU_OP_FULL_VIOLATION) {
-		dev_err_ratelimited(chip->dev,
-				    "ATU full violation for %pM fid %u portvec %x spid %d\n",
-				    entry.mac, fid, entry.portvec, spid);
+		trace_mv88e6xxx_atu_full_violation(chip->dev, spid,
+						   entry.mac, fid);
 		chip->ports[spid].atu_full_violation++;
 	}
 	mv88e6xxx_reg_unlock(chip);
diff --git a/drivers/net/dsa/mv88e6xxx/trace.c b/drivers/net/dsa/mv88e6xxx/trace.c
new file mode 100644
index 000000000000..7833cb50ca5d
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/trace.c
@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Copyright 2022 NXP
+ */
+
+#define CREATE_TRACE_POINTS
+#include "trace.h"
diff --git a/drivers/net/dsa/mv88e6xxx/trace.h b/drivers/net/dsa/mv88e6xxx/trace.h
new file mode 100644
index 000000000000..dc24dbd77f77
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/trace.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Copyright 2022 NXP
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM	mv88e6xxx
+
+#if !defined(_MV88E6XXX_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _MV88E6XXX_TRACE_H
+
+#include <linux/device.h>
+#include <linux/if_ether.h>
+#include <linux/tracepoint.h>
+
+DECLARE_EVENT_CLASS(mv88e6xxx_atu_violation,
+
+	TP_PROTO(const struct device *dev, int port,
+		 const unsigned char *addr, u16 fid),
+
+	TP_ARGS(dev, port, addr, fid),
+
+	TP_STRUCT__entry(
+		__string(name, dev_name(dev))
+		__field(int, port)
+		__array(unsigned char, addr, ETH_ALEN)
+		__field(u16, fid)
+	),
+
+	TP_fast_assign(
+		__assign_str(name, dev_name(dev));
+		__entry->port = port;
+		memcpy(__entry->addr, addr, ETH_ALEN);
+		__entry->fid = fid;
+	),
+
+	TP_printk("dev %s port %d addr %pM fid %u",
+		  __get_str(name), __entry->port, __entry->addr, __entry->fid)
+);
+
+DEFINE_EVENT(mv88e6xxx_atu_violation, mv88e6xxx_atu_age_out_violation,
+	     TP_PROTO(const struct device *dev, int port,
+		      const unsigned char *addr, u16 fid),
+	     TP_ARGS(dev, port, addr, fid));
+
+DEFINE_EVENT(mv88e6xxx_atu_violation, mv88e6xxx_atu_member_violation,
+	     TP_PROTO(const struct device *dev, int port,
+		      const unsigned char *addr, u16 fid),
+	     TP_ARGS(dev, port, addr, fid));
+
+DEFINE_EVENT(mv88e6xxx_atu_violation, mv88e6xxx_atu_miss_violation,
+	     TP_PROTO(const struct device *dev, int port,
+		      const unsigned char *addr, u16 fid),
+	     TP_ARGS(dev, port, addr, fid));
+
+DEFINE_EVENT(mv88e6xxx_atu_violation, mv88e6xxx_atu_full_violation,
+	     TP_PROTO(const struct device *dev, int port,
+		      const unsigned char *addr, u16 fid),
+	     TP_ARGS(dev, port, addr, fid));
+
+#endif /* _MV88E6XXX_TRACE_H */
+
+/* We don't want to use include/trace/events */
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE	trace
+/* This part must be outside protection */
+#include <trace/define_trace.h>
-- 
2.34.1

[PATCH net-next 3/3] net: dsa: mv88e6xxx: replace VTU violation prints with trace points

[Vladimir Oltean]

It is possible to trigger these VTU violation messages very easily,
it's only necessary to send packets with an unknown VLAN ID to a port
that belongs to a VLAN-aware bridge.

Do a similar thing as for ATU violation messages, and hide them in the
kernel's trace buffer.

New usage model:

$ trace-cmd list | grep mv88e6xxx
mv88e6xxx
mv88e6xxx:mv88e6xxx_vtu_miss_violation
mv88e6xxx:mv88e6xxx_vtu_member_violation
mv88e6xxx:mv88e6xxx_atu_full_violation
mv88e6xxx:mv88e6xxx_atu_miss_violation
mv88e6xxx:mv88e6xxx_atu_member_violation
mv88e6xxx:mv88e6xxx_atu_age_out_violation
$ trace-cmd record -e mv88e6xxx sleep 10
$ trace-cmd report

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/mv88e6xxx/global1_vtu.c |  7 +++---
 drivers/net/dsa/mv88e6xxx/trace.h       | 30 +++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/global1_vtu.c b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
index 38e18f5811bf..bcfb4a812055 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_vtu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
@@ -13,6 +13,7 @@
 
 #include "chip.h"
 #include "global1.h"
+#include "trace.h"
 
 /* Offset 0x02: VTU FID Register */
 
@@ -628,14 +629,12 @@ static irqreturn_t mv88e6xxx_g1_vtu_prob_irq_thread_fn(int irq, void *dev_id)
 	spid = val & MV88E6XXX_G1_VTU_OP_SPID_MASK;
 
 	if (val & MV88E6XXX_G1_VTU_OP_MEMBER_VIOLATION) {
-		dev_err_ratelimited(chip->dev, "VTU member violation for vid %d, source port %d\n",
-				    vid, spid);
+		trace_mv88e6xxx_vtu_member_violation(chip->dev, spid, vid);
 		chip->ports[spid].vtu_member_violation++;
 	}
 
 	if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION) {
-		dev_dbg_ratelimited(chip->dev, "VTU miss violation for vid %d, source port %d\n",
-				    vid, spid);
+		trace_mv88e6xxx_vtu_miss_violation(chip->dev, spid, vid);
 		chip->ports[spid].vtu_miss_violation++;
 	}
 
diff --git a/drivers/net/dsa/mv88e6xxx/trace.h b/drivers/net/dsa/mv88e6xxx/trace.h
index dc24dbd77f77..d8d31e862545 100644
--- a/drivers/net/dsa/mv88e6xxx/trace.h
+++ b/drivers/net/dsa/mv88e6xxx/trace.h
@@ -57,6 +57,36 @@ DEFINE_EVENT(mv88e6xxx_atu_violation, mv88e6xxx_atu_full_violation,
 		      const unsigned char *addr, u16 fid),
 	     TP_ARGS(dev, port, addr, fid));
 
+DECLARE_EVENT_CLASS(mv88e6xxx_vtu_violation,
+
+	TP_PROTO(const struct device *dev, int port, u16 vid),
+
+	TP_ARGS(dev, port, vid),
+
+	TP_STRUCT__entry(
+		__string(name, dev_name(dev))
+		__field(int, port)
+		__field(u16, vid)
+	),
+
+	TP_fast_assign(
+		__assign_str(name, dev_name(dev));
+		__entry->port = port;
+		__entry->vid = vid;
+	),
+
+	TP_printk("dev %s port %d vid %u",
+		  __get_str(name), __entry->port, __entry->vid)
+);
+
+DEFINE_EVENT(mv88e6xxx_vtu_violation, mv88e6xxx_vtu_member_violation,
+	     TP_PROTO(const struct device *dev, int port, u16 vid),
+	     TP_ARGS(dev, port, vid));
+
+DEFINE_EVENT(mv88e6xxx_vtu_violation, mv88e6xxx_vtu_miss_violation,
+	     TP_PROTO(const struct device *dev, int port, u16 vid),
+	     TP_ARGS(dev, port, vid));
+
 #endif /* _MV88E6XXX_TRACE_H */
 
 /* We don't want to use include/trace/events */
-- 
2.34.1

Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: replace ATU violation prints with trace points

[Saeed Mahameed]

On 08 Dec 01:39, Vladimir Oltean wrote:
>In applications where the switch ports must perform 802.1X based
>authentication and are therefore locked, ATU violation interrupts are
>quite to be expected as part of normal operation. The problem is that
>they currently spam the kernel log, even if rate limited.
>

+1 

>Create a series of trace points, all derived from the same event class,
>which log these violations to the kernel's trace buffer, which is both
>much faster and much easier to ignore than printing to a serial console.
>
>I've deliberately stopped reporting the portvec, since in my experience
>it contains redundant information with the spid (port) field: portvec ==
>1 << spid.
>
>New usage model:
>
>$ trace-cmd list | grep mv88e6xxx
>mv88e6xxx
>mv88e6xxx:mv88e6xxx_atu_full_violation
>mv88e6xxx:mv88e6xxx_atu_miss_violation
>mv88e6xxx:mv88e6xxx_atu_member_violation
>mv88e6xxx:mv88e6xxx_atu_age_out_violation
>$ trace-cmd record -e mv88e6xxx sleep 10
>
>Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>---
> drivers/net/dsa/mv88e6xxx/Makefile      |  4 ++
> drivers/net/dsa/mv88e6xxx/global1_atu.c | 21 ++++----
> drivers/net/dsa/mv88e6xxx/trace.c       |  6 +++
> drivers/net/dsa/mv88e6xxx/trace.h       | 68 +++++++++++++++++++++++++
> 4 files changed, 87 insertions(+), 12 deletions(-)
> create mode 100644 drivers/net/dsa/mv88e6xxx/trace.c
> create mode 100644 drivers/net/dsa/mv88e6xxx/trace.h
>
>diff --git a/drivers/net/dsa/mv88e6xxx/Makefile b/drivers/net/dsa/mv88e6xxx/Makefile
>index c8eca2b6f959..49bf358b9c4f 100644
>--- a/drivers/net/dsa/mv88e6xxx/Makefile
>+++ b/drivers/net/dsa/mv88e6xxx/Makefile
>@@ -15,3 +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 += trace.o
>+
>+# for tracing framework to find trace.h
>+CFLAGS_trace.o := -I$(src)
>diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
>index a9e2ff7d0e52..6ba65b723b42 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 "trace.h"
>
> /* Offset 0x01: ATU FID Register */
>
>@@ -429,29 +430,25 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
> 	spid = entry.state;
>
> 	if (val & MV88E6XXX_G1_ATU_OP_AGE_OUT_VIOLATION) {
>-		dev_err_ratelimited(chip->dev,
>-				    "ATU age out violation for %pM fid %u\n",
>-				    entry.mac, fid);
>+		trace_mv88e6xxx_atu_age_out_violation(chip->dev, spid,
>+						      entry.mac, fid);

no stats here? tracepoints are disabled by default and this event will go
unnoticed, users usually monitor light weight indicators such as stats, then
turn on tracepoints to see what's actually happening.. 

> 	}
>

Re: [PATCH net-next 0/3] Trace points for mv88e6xxx

[netdev]

On 2022-12-08 00:39, Vladimir Oltean wrote:
> While testing Hans Schultz' attempt at offloading MAB on mv88e6xxx:
> https://patchwork.kernel.org/project/netdevbpf/cover/20221205185908.217520-1-netdev@kapio-technology.com/
> I noticed that he still didn't get rid of the huge log spam caused by
> ATU and VTU violations, even if we discussed about this:
> https://patchwork.kernel.org/project/netdevbpf/cover/20221112203748.68995-1-netdev@kapio-technology.com/#25091076
> 
> It seems unlikely he's going to ever do this

Ohh, I didn't expect it to be part of the MAB patch set, but rather 
something after, as there is enough for me already for the moment. But I 
welcome your contribution of course. :-)

Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: replace ATU violation prints with trace points

[Vladimir Oltean]

On Wed, Dec 07, 2022 at 04:14:16PM -0800, Saeed Mahameed wrote:
> > 	if (val & MV88E6XXX_G1_ATU_OP_AGE_OUT_VIOLATION) {
> > -		dev_err_ratelimited(chip->dev,
> > -				    "ATU age out violation for %pM fid %u\n",
> > -				    entry.mac, fid);
> > +		trace_mv88e6xxx_atu_age_out_violation(chip->dev, spid,
> > +						      entry.mac, fid);
> 
> no stats here? tracepoints are disabled by default and this event will go
> unnoticed, users usually monitor light weight indicators such as stats, then
> turn on tracepoints to see what's actually happening..

I believe that the ATU age out violation handler is dead code currently.
The driver does not enable the MV88E6XXX_PORT_ASSOC_VECTOR_INT_AGE_OUT bit
(interrupt on age out).

I just converted the existing debugging prints to trace points. Open to
more suggestions, but I believe that if I introduce a counter, it would
always return 0.

Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: replace ATU violation prints with trace points

[netdev]

On 2022-12-08 15:49, Vladimir Oltean wrote:
> On Wed, Dec 07, 2022 at 04:14:16PM -0800, Saeed Mahameed wrote:
>> > 	if (val & MV88E6XXX_G1_ATU_OP_AGE_OUT_VIOLATION) {
>> > -		dev_err_ratelimited(chip->dev,
>> > -				    "ATU age out violation for %pM fid %u\n",
>> > -				    entry.mac, fid);
>> > +		trace_mv88e6xxx_atu_age_out_violation(chip->dev, spid,
>> > +						      entry.mac, fid);
>> 
>> no stats here? tracepoints are disabled by default and this event will 
>> go
>> unnoticed, users usually monitor light weight indicators such as 
>> stats, then
>> turn on tracepoints to see what's actually happening..
> 
> I believe that the ATU age out violation handler is dead code 
> currently.
> The driver does not enable the MV88E6XXX_PORT_ASSOC_VECTOR_INT_AGE_OUT 
> bit
> (interrupt on age out).
> 
> I just converted the existing debugging prints to trace points. Open to
> more suggestions, but I believe that if I introduce a counter, it would
> always return 0.

The follow-up patch set to the MAB patch set I have, will make use of 
the age
out violation.

Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: replace ATU violation prints with trace points

[Vladimir Oltean]

On Thu, Dec 08, 2022 at 04:22:53PM +0100, netdev@kapio-technology.com wrote:
> The follow-up patch set to the MAB patch set I have, will make use of the age
> out violation.

Ok, so for v2 I can delete the debugging print, since it's currently
dead code, and you can add the counter and the trace point when the code
will be actually exercised, how does that sound?

Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: replace ATU violation prints with trace points

[netdev]

On 2022-12-08 16:27, Vladimir Oltean wrote:
> On Thu, Dec 08, 2022 at 04:22:53PM +0100, netdev@kapio-technology.com 
> wrote:
>> The follow-up patch set to the MAB patch set I have, will make use of 
>> the age
>> out violation.
> 
> Ok, so for v2 I can delete the debugging print, since it's currently
> dead code, and you can add the counter and the trace point when the 
> code
> will be actually exercised, how does that sound?

Ok, sounds like a plan.

Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: replace ATU violation prints with trace points

[netdev]

On 2022-12-08 15:49, Vladimir Oltean wrote:
> On Wed, Dec 07, 2022 at 04:14:16PM -0800, Saeed Mahameed wrote:
>> > 	if (val & MV88E6XXX_G1_ATU_OP_AGE_OUT_VIOLATION) {
>> > -		dev_err_ratelimited(chip->dev,
>> > -				    "ATU age out violation for %pM fid %u\n",
>> > -				    entry.mac, fid);
>> > +		trace_mv88e6xxx_atu_age_out_violation(chip->dev, spid,
>> > +						      entry.mac, fid);
>> 
>> no stats here? tracepoints are disabled by default and this event will 
>> go
>> unnoticed, users usually monitor light weight indicators such as 
>> stats, then
>> turn on tracepoints to see what's actually happening..
> 
> I believe that the ATU age out violation handler is dead code 
> currently.
> The driver does not enable the MV88E6XXX_PORT_ASSOC_VECTOR_INT_AGE_OUT 
> bit
> (interrupt on age out).
> 
> I just converted the existing debugging prints to trace points. Open to
> more suggestions, but I believe that if I introduce a counter, it would
> always return 0.

If I am not mistaken, I will have to wait for your patch set to be 
accepted before
I can send the next version of the MAB patch set.

Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: replace ATU violation prints with trace points

[Vladimir Oltean]

On Thu, Dec 08, 2022 at 07:33:50PM +0100, netdev@kapio-technology.com wrote:
> If I am not mistaken, I will have to wait for your patch set to be accepted before
> I can send the next version of the MAB patch set.

Yes, you are not mistaken.

Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: replace ATU violation prints with trace points

[Saeed Mahameed]

On 08 Dec 16:35, netdev@kapio-technology.com wrote:
>On 2022-12-08 16:27, Vladimir Oltean wrote:
>>On Thu, Dec 08, 2022 at 04:22:53PM +0100, 
>>netdev@kapio-technology.com wrote:
>>>The follow-up patch set to the MAB patch set I have, will make use 
>>>of the age
>>>out violation.
>>
>>Ok, so for v2 I can delete the debugging print, since it's currently
>>dead code, and you can add the counter and the trace point when the 
>>code
>>will be actually exercised, how does that sound?
>
>Ok, sounds like a plan.

FWIW +1 :)