netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q
@ 2021-02-13 22:37 Vladimir Oltean
  2021-02-13 22:37 ` [PATCH v2 net-next 01/12] net: mscc: ocelot: stop returning IRQ_NONE in ocelot_xtr_irq_handler Vladimir Oltean
                   ` (12 more replies)
  0 siblings, 13 replies; 25+ messages in thread
From: Vladimir Oltean @ 2021-02-13 22:37 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Richard Cochran,
	Claudiu Manoil, Alexandre Belloni, Vladimir Oltean,
	Maxim Kochetkov, UNGLinuxDriver

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Changes in v2:
Add stub definition for ocelot_port_inject_frame when switch driver is
not compiled in.

This is part two of the errata workaround begun here:
https://patchwork.kernel.org/project/netdevbpf/cover/20210129010009.3959398-1-olteanv@gmail.com/

Now that we have basic traffic support when we operate the Ocelot DSA
switches without an NPI port, it would be nice to regain some of the
features lost due to the lack of the NPI port functionality. An
important one is PTP timestamping, which is intimately tied to the DSA
frame header added by the NPI port: on TX, we put a "timestamp request
ID" in the Injection Frame Header, while on RX, the Extraction Frame
Header contains a partial 32-bit PTP timestamp. Get rid of the NPI port
and replace it with a VLAN-based tagger, and you lose PTP, right?

Well, not quite, this is what this patch series is about. The NPI port
is basically a regular Ethernet port configured to service the packets
in and out of the switch's CPU port module (which has other non-DSA I/O
mechanisms too, such as register-based MMIO and DMA). If we disable the
NPI port, we can in theory still access the packets delivered to the CPU
port module by doing exactly what the ocelot switchdev driver does:
extracting Ethernet packets through registers (yes, it is as icky as it
sounds).

However, there's a catch. The Felix switch was integrated into NXP
LS1028A with the idea in mind that it will operate as DSA, i.e. using
the CPU port module connected to the NPI port, not having I/O over
register-based MMIO which is painfully slow and CPU intensive. So
register-based packet I/O not supposed to work - those registers aren't
even documented in the hardware reference manual for Felix. However
they kinda do, with the exception of the fact that an RX interrupt was
really not wired to the CPU cores - so we don't know when the CPU port
module receives a new packet. But we can hack even around that, by
replicating every packet that goes to the CPU port module and making it
also go to a plain internal Ethernet port. Then drop the Ethernet packet
and read the other copy of it from the CPU port module, this time
annotated with the much-wanted RX timestamp.

This is all fine and it works, but it does raise some questions about
what DSA even is anymore, if we start having switches that inject some
of their packets over Ethernet and some through registers, where do we
draw the line. In principle I believe these concerns are founded, but at
the same time, the way that the Felix driver uses register MMIO based
packet I/O is fundamentally the same as any other DSA driver capable of
PTP makes use of a side-channel for timestamps like a FIFO (just that
this one is a lot more complicated, and comes with the entire actual
packet, not just the timestamp).

Nonetheless, I tried to keep the extra pressure added by this ERR
workaround upon the DSA subsystem as small as possible, so some of the
patches are just a revisit of some of Andrew's complaints w.r.t. the
fact that tag_ocelot already violates any driver <-> tagger boundary,
and as a consequence, is not able to be used on testbeds such as
dsa_loop (which it now can). So now, the tag_ocelot and tag_ocelot_8021q
drivers should be dsa_loop-clean, and have the ERR workarounds as
self-contained as possible, using all the designated features for PTP
timestamping and nothing more.

Comments appreciated.

Vladimir Oltean (12):
  net: mscc: ocelot: stop returning IRQ_NONE in ocelot_xtr_irq_handler
  net: mscc: ocelot: only drain extraction queue on error
  net: mscc: ocelot: better error handling in ocelot_xtr_irq_handler
  net: mscc: ocelot: use DIV_ROUND_UP helper in ocelot_port_inject_frame
  net: mscc: ocelot: refactor ocelot_port_inject_frame out of
    ocelot_port_xmit
  net: dsa: tag_ocelot: avoid accessing ds->priv in ocelot_rcv
  net: mscc: ocelot: use common tag parsing code with DSA
  net: dsa: tag_ocelot: single out PTP-related transmit tag processing
  net: dsa: tag_ocelot: create separate tagger for Seville
  net: mscc: ocelot: refactor ocelot_xtr_irq_handler into
    ocelot_xtr_poll
  net: dsa: felix: setup MMIO filtering rules for PTP when using
    tag_8021q
  net: dsa: tag_ocelot_8021q: add support for PTP timestamping

 drivers/net/dsa/ocelot/felix.c             | 224 +++++++++++++++++--
 drivers/net/dsa/ocelot/felix.h             |  14 +-
 drivers/net/dsa/ocelot/felix_vsc9959.c     |  29 +--
 drivers/net/dsa/ocelot/seville_vsc9953.c   |  30 +--
 drivers/net/ethernet/mscc/ocelot.c         | 216 ++++++++++++++++++
 drivers/net/ethernet/mscc/ocelot.h         |   9 -
 drivers/net/ethernet/mscc/ocelot_net.c     |  81 +------
 drivers/net/ethernet/mscc/ocelot_vsc7514.c | 178 +--------------
 include/linux/dsa/ocelot.h                 | 218 ++++++++++++++++++
 include/net/dsa.h                          |   2 +
 include/soc/mscc/ocelot.h                  |  41 +++-
 net/dsa/tag_ocelot.c                       | 244 +++++++--------------
 net/dsa/tag_ocelot_8021q.c                 |  34 +++
 13 files changed, 825 insertions(+), 495 deletions(-)
 create mode 100644 include/linux/dsa/ocelot.h

-- 
2.25.1


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

* [PATCH v2 net-next 01/12] net: mscc: ocelot: stop returning IRQ_NONE in ocelot_xtr_irq_handler
  2021-02-13 22:37 [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q Vladimir Oltean
@ 2021-02-13 22:37 ` Vladimir Oltean
  2021-02-13 22:37 ` [PATCH v2 net-next 02/12] net: mscc: ocelot: only drain extraction queue on error Vladimir Oltean
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Vladimir Oltean @ 2021-02-13 22:37 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Richard Cochran,
	Claudiu Manoil, Alexandre Belloni, Vladimir Oltean,
	Maxim Kochetkov, UNGLinuxDriver

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Since the xtr (extraction) IRQ of the ocelot switch is not shared, then
if it fired, it means that some data must be present in the queues of
the CPU port module. So simplify the code.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
None.

 drivers/net/ethernet/mscc/ocelot_vsc7514.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index 6b6eb92149ba..590297d5e144 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -604,10 +604,7 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
 	int i = 0, grp = 0;
 	int err = 0;
 
-	if (!(ocelot_read(ocelot, QS_XTR_DATA_PRESENT) & BIT(grp)))
-		return IRQ_NONE;
-
-	do {
+	while (ocelot_read(ocelot, QS_XTR_DATA_PRESENT) & BIT(grp)) {
 		struct skb_shared_hwtstamps *shhwtstamps;
 		struct ocelot_port_private *priv;
 		struct ocelot_port *ocelot_port;
@@ -702,7 +699,7 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
 			netif_rx(skb);
 		dev->stats.rx_bytes += len;
 		dev->stats.rx_packets++;
-	} while (ocelot_read(ocelot, QS_XTR_DATA_PRESENT) & BIT(grp));
+	}
 
 	if (err)
 		while (ocelot_read(ocelot, QS_XTR_DATA_PRESENT) & BIT(grp))
-- 
2.25.1


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

* [PATCH v2 net-next 02/12] net: mscc: ocelot: only drain extraction queue on error
  2021-02-13 22:37 [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q Vladimir Oltean
  2021-02-13 22:37 ` [PATCH v2 net-next 01/12] net: mscc: ocelot: stop returning IRQ_NONE in ocelot_xtr_irq_handler Vladimir Oltean
@ 2021-02-13 22:37 ` Vladimir Oltean
  2021-02-14  1:19   ` Florian Fainelli
  2021-02-13 22:37 ` [PATCH v2 net-next 03/12] net: mscc: ocelot: better error handling in ocelot_xtr_irq_handler Vladimir Oltean
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2021-02-13 22:37 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Richard Cochran,
	Claudiu Manoil, Alexandre Belloni, Vladimir Oltean,
	Maxim Kochetkov, UNGLinuxDriver

From: Vladimir Oltean <vladimir.oltean@nxp.com>

It appears that the intention of this snippet of code is to not exit
ocelot_xtr_irq_handler() while in the middle of extracting a frame.
The problem in extracting it word by word is that future extraction
attempts are really easy to get desynchronized, since the IRQ handler
assumes that the first 16 bytes are the IFH, which give further
information about the frame, such as frame length.

But during normal operation, "err" will not be 0, but 4, set from here:

		for (i = 0; i < OCELOT_TAG_LEN / 4; i++) {
			err = ocelot_rx_frame_word(ocelot, grp, true, &ifh[i]);
			if (err != 4)
				break;
		}

		if (err != 4)
			break;

In that case, draining the extraction queue is a no-op. So explicitly
make this code execute only on negative err.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
None.

 drivers/net/ethernet/mscc/ocelot_vsc7514.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index 590297d5e144..d19efbe6ffd3 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -701,7 +701,7 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
 		dev->stats.rx_packets++;
 	}
 
-	if (err)
+	if (err < 0)
 		while (ocelot_read(ocelot, QS_XTR_DATA_PRESENT) & BIT(grp))
 			ocelot_read_rix(ocelot, QS_XTR_RD, grp);
 
-- 
2.25.1


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

* [PATCH v2 net-next 03/12] net: mscc: ocelot: better error handling in ocelot_xtr_irq_handler
  2021-02-13 22:37 [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q Vladimir Oltean
  2021-02-13 22:37 ` [PATCH v2 net-next 01/12] net: mscc: ocelot: stop returning IRQ_NONE in ocelot_xtr_irq_handler Vladimir Oltean
  2021-02-13 22:37 ` [PATCH v2 net-next 02/12] net: mscc: ocelot: only drain extraction queue on error Vladimir Oltean
@ 2021-02-13 22:37 ` Vladimir Oltean
  2021-02-14  1:20   ` Florian Fainelli
  2021-02-13 22:37 ` [PATCH v2 net-next 04/12] net: mscc: ocelot: use DIV_ROUND_UP helper in ocelot_port_inject_frame Vladimir Oltean
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2021-02-13 22:37 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Richard Cochran,
	Claudiu Manoil, Alexandre Belloni, Vladimir Oltean,
	Maxim Kochetkov, UNGLinuxDriver

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The ocelot_rx_frame_word() function can return a negative error code,
however this isn't being checked for consistently. Errors being ignored
have not been seen in practice though.

Also, some constructs can be simplified by using "goto" instead of
repeated "break" statements.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
None.

 drivers/net/ethernet/mscc/ocelot_vsc7514.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index d19efbe6ffd3..b075dc13354a 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -619,12 +619,9 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
 		for (i = 0; i < OCELOT_TAG_LEN / 4; i++) {
 			err = ocelot_rx_frame_word(ocelot, grp, true, &ifh[i]);
 			if (err != 4)
-				break;
+				goto out;
 		}
 
-		if (err != 4)
-			break;
-
 		/* At this point the IFH was read correctly, so it is safe to
 		 * presume that there is no error. The err needs to be reset
 		 * otherwise a frame could come in CPU queue between the while
@@ -645,7 +642,7 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
 		if (unlikely(!skb)) {
 			netdev_err(dev, "Unable to allocate sk_buff\n");
 			err = -ENOMEM;
-			break;
+			goto out;
 		}
 		buf_len = info.len - ETH_FCS_LEN;
 		buf = (u32 *)skb_put(skb, buf_len);
@@ -653,12 +650,21 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
 		len = 0;
 		do {
 			sz = ocelot_rx_frame_word(ocelot, grp, false, &val);
+			if (sz < 0) {
+				err = sz;
+				goto out;
+			}
 			*buf++ = val;
 			len += sz;
 		} while (len < buf_len);
 
 		/* Read the FCS */
 		sz = ocelot_rx_frame_word(ocelot, grp, false, &val);
+		if (sz < 0) {
+			err = sz;
+			goto out;
+		}
+
 		/* Update the statistics if part of the FCS was read before */
 		len -= ETH_FCS_LEN - sz;
 
@@ -667,11 +673,6 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
 			*buf = val;
 		}
 
-		if (sz < 0) {
-			err = sz;
-			break;
-		}
-
 		if (ocelot->ptp) {
 			ocelot_ptp_gettime64(&ocelot->ptp_info, &ts);
 
@@ -701,6 +702,7 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
 		dev->stats.rx_packets++;
 	}
 
+out:
 	if (err < 0)
 		while (ocelot_read(ocelot, QS_XTR_DATA_PRESENT) & BIT(grp))
 			ocelot_read_rix(ocelot, QS_XTR_RD, grp);
-- 
2.25.1


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

* [PATCH v2 net-next 04/12] net: mscc: ocelot: use DIV_ROUND_UP helper in ocelot_port_inject_frame
  2021-02-13 22:37 [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q Vladimir Oltean
                   ` (2 preceding siblings ...)
  2021-02-13 22:37 ` [PATCH v2 net-next 03/12] net: mscc: ocelot: better error handling in ocelot_xtr_irq_handler Vladimir Oltean
@ 2021-02-13 22:37 ` Vladimir Oltean
  2021-02-14  1:20   ` Florian Fainelli
  2021-02-13 22:37 ` [PATCH v2 net-next 05/12] net: mscc: ocelot: refactor ocelot_port_inject_frame out of ocelot_port_xmit Vladimir Oltean
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2021-02-13 22:37 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Richard Cochran,
	Claudiu Manoil, Alexandre Belloni, Vladimir Oltean,
	Maxim Kochetkov, UNGLinuxDriver

From: Vladimir Oltean <vladimir.oltean@nxp.com>

This looks a bit nicer than the open-coded "(x + 3) % 4" idiom.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
None.

 drivers/net/ethernet/mscc/ocelot_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index b5ffe6724eb7..1ab453298a18 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -563,7 +563,7 @@ static int ocelot_port_xmit(struct sk_buff *skb, struct net_device *dev)
 		ocelot_write_rix(ocelot, (__force u32)cpu_to_be32(ifh[i]),
 				 QS_INJ_WR, grp);
 
-	count = (skb->len + 3) / 4;
+	count = DIV_ROUND_UP(skb->len, 4);
 	last = skb->len % 4;
 	for (i = 0; i < count; i++)
 		ocelot_write_rix(ocelot, ((u32 *)skb->data)[i], QS_INJ_WR, grp);
-- 
2.25.1


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

* [PATCH v2 net-next 05/12] net: mscc: ocelot: refactor ocelot_port_inject_frame out of ocelot_port_xmit
  2021-02-13 22:37 [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q Vladimir Oltean
                   ` (3 preceding siblings ...)
  2021-02-13 22:37 ` [PATCH v2 net-next 04/12] net: mscc: ocelot: use DIV_ROUND_UP helper in ocelot_port_inject_frame Vladimir Oltean
@ 2021-02-13 22:37 ` Vladimir Oltean
  2021-02-14  1:22   ` Florian Fainelli
  2021-02-13 22:37 ` [PATCH v2 net-next 06/12] net: dsa: tag_ocelot: avoid accessing ds->priv in ocelot_rcv Vladimir Oltean
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2021-02-13 22:37 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Richard Cochran,
	Claudiu Manoil, Alexandre Belloni, Vladimir Oltean,
	Maxim Kochetkov, UNGLinuxDriver

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The felix DSA driver will inject some frames through register MMIO, same
as ocelot switchdev currently does. So we need to be able to reuse the
common code.

Also create some shim definitions, since the DSA tagger can be compiled
without support for the switch driver.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
Create shim definitions.

 drivers/net/ethernet/mscc/ocelot.c     | 80 +++++++++++++++++++++++++
 drivers/net/ethernet/mscc/ocelot_net.c | 81 +++-----------------------
 include/soc/mscc/ocelot.h              | 22 +++++++
 3 files changed, 109 insertions(+), 74 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index d1a9cdbf7a3e..7106d9ee534a 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -628,6 +628,86 @@ void ocelot_get_txtstamp(struct ocelot *ocelot)
 }
 EXPORT_SYMBOL(ocelot_get_txtstamp);
 
+/* Generate the IFH for frame injection
+ *
+ * The IFH is a 128bit-value
+ * bit 127: bypass the analyzer processing
+ * bit 56-67: destination mask
+ * bit 28-29: pop_cnt: 3 disables all rewriting of the frame
+ * bit 20-27: cpu extraction queue mask
+ * bit 16: tag type 0: C-tag, 1: S-tag
+ * bit 0-11: VID
+ */
+static int ocelot_gen_ifh(u32 *ifh, struct frame_info *info)
+{
+	ifh[0] = IFH_INJ_BYPASS | ((0x1ff & info->rew_op) << 21);
+	ifh[1] = (0xf00 & info->port) >> 8;
+	ifh[2] = (0xff & info->port) << 24;
+	ifh[3] = (info->tag_type << 16) | info->vid;
+
+	return 0;
+}
+
+bool ocelot_can_inject(struct ocelot *ocelot, int grp)
+{
+	u32 val = ocelot_read(ocelot, QS_INJ_STATUS);
+
+	if (!(val & QS_INJ_STATUS_FIFO_RDY(BIT(grp))))
+		return false;
+	if (val & QS_INJ_STATUS_WMARK_REACHED(BIT(grp)))
+		return false;
+
+	return true;
+}
+EXPORT_SYMBOL(ocelot_can_inject);
+
+void ocelot_port_inject_frame(struct ocelot *ocelot, int port, int grp,
+			      u32 rew_op, struct sk_buff *skb)
+{
+	struct frame_info info = {};
+	u32 ifh[OCELOT_TAG_LEN / 4];
+	unsigned int i, count, last;
+
+	ocelot_write_rix(ocelot, QS_INJ_CTRL_GAP_SIZE(1) |
+			 QS_INJ_CTRL_SOF, QS_INJ_CTRL, grp);
+
+	info.port = BIT(port);
+	info.tag_type = IFH_TAG_TYPE_C;
+	info.vid = skb_vlan_tag_get(skb);
+	info.rew_op = rew_op;
+
+	ocelot_gen_ifh(ifh, &info);
+
+	for (i = 0; i < OCELOT_TAG_LEN / 4; i++)
+		ocelot_write_rix(ocelot, (__force u32)cpu_to_be32(ifh[i]),
+				 QS_INJ_WR, grp);
+
+	count = DIV_ROUND_UP(skb->len, 4);
+	last = skb->len % 4;
+	for (i = 0; i < count; i++)
+		ocelot_write_rix(ocelot, ((u32 *)skb->data)[i], QS_INJ_WR, grp);
+
+	/* Add padding */
+	while (i < (OCELOT_BUFFER_CELL_SZ / 4)) {
+		ocelot_write_rix(ocelot, 0, QS_INJ_WR, grp);
+		i++;
+	}
+
+	/* Indicate EOF and valid bytes in last word */
+	ocelot_write_rix(ocelot, QS_INJ_CTRL_GAP_SIZE(1) |
+			 QS_INJ_CTRL_VLD_BYTES(skb->len < OCELOT_BUFFER_CELL_SZ ? 0 : last) |
+			 QS_INJ_CTRL_EOF,
+			 QS_INJ_CTRL, grp);
+
+	/* Add dummy CRC */
+	ocelot_write_rix(ocelot, 0, QS_INJ_WR, grp);
+	skb_tx_timestamp(skb);
+
+	skb->dev->stats.tx_packets++;
+	skb->dev->stats.tx_bytes += skb->len;
+}
+EXPORT_SYMBOL(ocelot_port_inject_frame);
+
 int ocelot_fdb_add(struct ocelot *ocelot, int port,
 		   const unsigned char *addr, u16 vid)
 {
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index 1ab453298a18..6518262532f0 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -488,53 +488,20 @@ static int ocelot_port_stop(struct net_device *dev)
 	return 0;
 }
 
-/* Generate the IFH for frame injection
- *
- * The IFH is a 128bit-value
- * bit 127: bypass the analyzer processing
- * bit 56-67: destination mask
- * bit 28-29: pop_cnt: 3 disables all rewriting of the frame
- * bit 20-27: cpu extraction queue mask
- * bit 16: tag type 0: C-tag, 1: S-tag
- * bit 0-11: VID
- */
-static int ocelot_gen_ifh(u32 *ifh, struct frame_info *info)
-{
-	ifh[0] = IFH_INJ_BYPASS | ((0x1ff & info->rew_op) << 21);
-	ifh[1] = (0xf00 & info->port) >> 8;
-	ifh[2] = (0xff & info->port) << 24;
-	ifh[3] = (info->tag_type << 16) | info->vid;
-
-	return 0;
-}
-
-static int ocelot_port_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t ocelot_port_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct ocelot_port_private *priv = netdev_priv(dev);
-	struct skb_shared_info *shinfo = skb_shinfo(skb);
 	struct ocelot_port *ocelot_port = &priv->port;
 	struct ocelot *ocelot = ocelot_port->ocelot;
-	u32 val, ifh[OCELOT_TAG_LEN / 4];
-	struct frame_info info = {};
-	u8 grp = 0; /* Send everything on CPU group 0 */
-	unsigned int i, count, last;
 	int port = priv->chip_port;
+	u32 rew_op = 0;
 
-	val = ocelot_read(ocelot, QS_INJ_STATUS);
-	if (!(val & QS_INJ_STATUS_FIFO_RDY(BIT(grp))) ||
-	    (val & QS_INJ_STATUS_WMARK_REACHED(BIT(grp))))
+	if (!ocelot_can_inject(ocelot, 0))
 		return NETDEV_TX_BUSY;
 
-	ocelot_write_rix(ocelot, QS_INJ_CTRL_GAP_SIZE(1) |
-			 QS_INJ_CTRL_SOF, QS_INJ_CTRL, grp);
-
-	info.port = BIT(port);
-	info.tag_type = IFH_TAG_TYPE_C;
-	info.vid = skb_vlan_tag_get(skb);
-
 	/* Check if timestamping is needed */
-	if (ocelot->ptp && (shinfo->tx_flags & SKBTX_HW_TSTAMP)) {
-		info.rew_op = ocelot_port->ptp_cmd;
+	if (ocelot->ptp && (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
+		rew_op = ocelot_port->ptp_cmd;
 
 		if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP) {
 			struct sk_buff *clone;
@@ -547,45 +514,11 @@ static int ocelot_port_xmit(struct sk_buff *skb, struct net_device *dev)
 
 			ocelot_port_add_txtstamp_skb(ocelot, port, clone);
 
-			info.rew_op |= clone->cb[0] << 3;
+			rew_op |= clone->cb[0] << 3;
 		}
 	}
 
-	if (ocelot->ptp && shinfo->tx_flags & SKBTX_HW_TSTAMP) {
-		info.rew_op = ocelot_port->ptp_cmd;
-		if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP)
-			info.rew_op |= skb->cb[0] << 3;
-	}
-
-	ocelot_gen_ifh(ifh, &info);
-
-	for (i = 0; i < OCELOT_TAG_LEN / 4; i++)
-		ocelot_write_rix(ocelot, (__force u32)cpu_to_be32(ifh[i]),
-				 QS_INJ_WR, grp);
-
-	count = DIV_ROUND_UP(skb->len, 4);
-	last = skb->len % 4;
-	for (i = 0; i < count; i++)
-		ocelot_write_rix(ocelot, ((u32 *)skb->data)[i], QS_INJ_WR, grp);
-
-	/* Add padding */
-	while (i < (OCELOT_BUFFER_CELL_SZ / 4)) {
-		ocelot_write_rix(ocelot, 0, QS_INJ_WR, grp);
-		i++;
-	}
-
-	/* Indicate EOF and valid bytes in last word */
-	ocelot_write_rix(ocelot, QS_INJ_CTRL_GAP_SIZE(1) |
-			 QS_INJ_CTRL_VLD_BYTES(skb->len < OCELOT_BUFFER_CELL_SZ ? 0 : last) |
-			 QS_INJ_CTRL_EOF,
-			 QS_INJ_CTRL, grp);
-
-	/* Add dummy CRC */
-	ocelot_write_rix(ocelot, 0, QS_INJ_WR, grp);
-	skb_tx_timestamp(skb);
-
-	dev->stats.tx_packets++;
-	dev->stats.tx_bytes += skb->len;
+	ocelot_port_inject_frame(ocelot, port, 0, rew_op, skb);
 
 	kfree_skb(skb);
 
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 40792b37bb9f..a22aa944e921 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -742,6 +742,28 @@ u32 __ocelot_target_read_ix(struct ocelot *ocelot, enum ocelot_target target,
 void __ocelot_target_write_ix(struct ocelot *ocelot, enum ocelot_target target,
 			      u32 val, u32 reg, u32 offset);
 
+/* Packet I/O */
+#if IS_ENABLED(CONFIG_MSCC_OCELOT_SWITCH_LIB)
+
+bool ocelot_can_inject(struct ocelot *ocelot, int grp);
+void ocelot_port_inject_frame(struct ocelot *ocelot, int port, int grp,
+			      u32 rew_op, struct sk_buff *skb);
+
+#else
+
+static inline bool ocelot_can_inject(struct ocelot *ocelot, int grp)
+{
+	return false;
+}
+
+static inline void ocelot_port_inject_frame(struct ocelot *ocelot, int port,
+					    int grp, u32 rew_op,
+					    struct sk_buff *skb)
+{
+}
+
+#endif
+
 /* Hardware initialization */
 int ocelot_regfields_init(struct ocelot *ocelot,
 			  const struct reg_field *const regfields);
-- 
2.25.1


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

* [PATCH v2 net-next 06/12] net: dsa: tag_ocelot: avoid accessing ds->priv in ocelot_rcv
  2021-02-13 22:37 [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q Vladimir Oltean
                   ` (4 preceding siblings ...)
  2021-02-13 22:37 ` [PATCH v2 net-next 05/12] net: mscc: ocelot: refactor ocelot_port_inject_frame out of ocelot_port_xmit Vladimir Oltean
@ 2021-02-13 22:37 ` Vladimir Oltean
  2021-02-14  1:23   ` Florian Fainelli
  2021-02-13 22:37 ` [PATCH v2 net-next 07/12] net: mscc: ocelot: use common tag parsing code with DSA Vladimir Oltean
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2021-02-13 22:37 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Richard Cochran,
	Claudiu Manoil, Alexandre Belloni, Vladimir Oltean,
	Maxim Kochetkov, UNGLinuxDriver

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Taggers should be written to do something valid irrespective of the
switch driver that they are attached to. This is even more true now,
because since the introduction of the .change_tag_protocol method, a
certain tagger is not necessarily strictly associated with a driver any
longer, and I would like to be able to test all taggers with dsa_loop in
the future.

In the case of ocelot, it needs to move the classified VLAN from the DSA
tag into the skb if the port is VLAN-aware. We can allow it to do that
by looking at the dp->vlan_filtering property, no need to invoke
structures which are specific to ocelot.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
None.

 net/dsa/tag_ocelot.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/net/dsa/tag_ocelot.c b/net/dsa/tag_ocelot.c
index 16a1afd5b8e1..225b145fd131 100644
--- a/net/dsa/tag_ocelot.c
+++ b/net/dsa/tag_ocelot.c
@@ -177,12 +177,10 @@ static struct sk_buff *ocelot_rcv(struct sk_buff *skb,
 				  struct net_device *netdev,
 				  struct packet_type *pt)
 {
-	struct dsa_port *cpu_dp = netdev->dsa_ptr;
-	struct dsa_switch *ds = cpu_dp->ds;
-	struct ocelot *ocelot = ds->priv;
 	u64 src_port, qos_class;
 	u64 vlan_tci, tag_type;
 	u8 *start = skb->data;
+	struct dsa_port *dp;
 	u8 *extraction;
 	u16 vlan_tpid;
 
@@ -243,9 +241,10 @@ static struct sk_buff *ocelot_rcv(struct sk_buff *skb,
 	 * equal to the pvid of the ingress port and should not be used for
 	 * processing.
 	 */
+	dp = dsa_slave_to_port(skb->dev);
 	vlan_tpid = tag_type ? ETH_P_8021AD : ETH_P_8021Q;
 
-	if (ocelot->ports[src_port]->vlan_aware &&
+	if (dsa_port_is_vlan_filtering(dp) &&
 	    eth_hdr(skb)->h_proto == htons(vlan_tpid)) {
 		u16 dummy_vlan_tci;
 
-- 
2.25.1


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

* [PATCH v2 net-next 07/12] net: mscc: ocelot: use common tag parsing code with DSA
  2021-02-13 22:37 [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q Vladimir Oltean
                   ` (5 preceding siblings ...)
  2021-02-13 22:37 ` [PATCH v2 net-next 06/12] net: dsa: tag_ocelot: avoid accessing ds->priv in ocelot_rcv Vladimir Oltean
@ 2021-02-13 22:37 ` Vladimir Oltean
  2021-02-14  1:25   ` Florian Fainelli
  2021-02-13 22:37 ` [PATCH v2 net-next 08/12] net: dsa: tag_ocelot: single out PTP-related transmit tag processing Vladimir Oltean
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2021-02-13 22:37 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Richard Cochran,
	Claudiu Manoil, Alexandre Belloni, Vladimir Oltean,
	Maxim Kochetkov, UNGLinuxDriver

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The Injection Frame Header and Extraction Frame Header that the switch
prepends to frames over the NPI port is also prepended to frames
delivered over the CPU port module's queues.

Let's unify the handling of the frame headers by making the ocelot
driver call some helpers exported by the DSA tagger. Among other things,
this allows us to get rid of the strange cpu_to_be32 when transmitting
the Injection Frame Header on ocelot, since the packing API uses
network byte order natively (when "quirks" is 0).

The comments above ocelot_gen_ifh talk about setting pop_cnt to 3, and
the cpu extraction queue mask to something, but the code doesn't do it,
so we don't do it either.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
Added last paragraph of commit description.

 drivers/net/dsa/ocelot/felix.c             |   6 +-
 drivers/net/dsa/ocelot/felix_vsc9959.c     |   2 +-
 drivers/net/dsa/ocelot/seville_vsc9953.c   |   2 +-
 drivers/net/ethernet/mscc/ocelot.c         |  38 +---
 drivers/net/ethernet/mscc/ocelot.h         |   9 -
 drivers/net/ethernet/mscc/ocelot_vsc7514.c |  54 ++----
 include/linux/dsa/ocelot.h                 | 208 +++++++++++++++++++++
 include/soc/mscc/ocelot.h                  |   7 -
 net/dsa/tag_ocelot.c                       | 147 +--------------
 9 files changed, 246 insertions(+), 227 deletions(-)
 create mode 100644 include/linux/dsa/ocelot.h

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 1ae94e392145..4af1187f4d69 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -14,8 +14,8 @@
 #include <soc/mscc/ocelot_ptp.h>
 #include <soc/mscc/ocelot.h>
 #include <linux/dsa/8021q.h>
+#include <linux/dsa/ocelot.h>
 #include <linux/platform_device.h>
-#include <linux/packing.h>
 #include <linux/module.h>
 #include <linux/of_net.h>
 #include <linux/pci.h>
@@ -1171,9 +1171,9 @@ static int felix_hwtstamp_set(struct dsa_switch *ds, int port,
 static bool felix_rxtstamp(struct dsa_switch *ds, int port,
 			   struct sk_buff *skb, unsigned int type)
 {
+	u8 *extraction = skb->data - ETH_HLEN - OCELOT_TAG_LEN;
 	struct skb_shared_hwtstamps *shhwtstamps;
 	struct ocelot *ocelot = ds->priv;
-	u8 *extraction = skb->data - ETH_HLEN - OCELOT_TAG_LEN;
 	u32 tstamp_lo, tstamp_hi;
 	struct timespec64 ts;
 	u64 tstamp, val;
@@ -1181,7 +1181,7 @@ static bool felix_rxtstamp(struct dsa_switch *ds, int port,
 	ocelot_ptp_gettime64(&ocelot->ptp_info, &ts);
 	tstamp = ktime_set(ts.tv_sec, ts.tv_nsec);
 
-	packing(extraction, &val,  116, 85, OCELOT_TAG_LEN, UNPACK, 0);
+	ocelot_xfh_get_rew_val(extraction, &val);
 	tstamp_lo = (u32)val;
 
 	tstamp_hi = tstamp >> 32;
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index e944868cc120..cacc6f9c0113 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -8,7 +8,7 @@
 #include <soc/mscc/ocelot_ptp.h>
 #include <soc/mscc/ocelot_sys.h>
 #include <soc/mscc/ocelot.h>
-#include <linux/packing.h>
+#include <linux/dsa/ocelot.h>
 #include <linux/pcs-lynx.h>
 #include <net/pkt_sched.h>
 #include <linux/iopoll.h>
diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
index 512f677a6c1c..d7348ea4831e 100644
--- a/drivers/net/dsa/ocelot/seville_vsc9953.c
+++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
@@ -8,7 +8,7 @@
 #include <soc/mscc/ocelot.h>
 #include <linux/of_platform.h>
 #include <linux/pcs-lynx.h>
-#include <linux/packing.h>
+#include <linux/dsa/ocelot.h>
 #include <linux/iopoll.h>
 #include "felix.h"
 
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 7106d9ee534a..699b0c1c1780 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -4,6 +4,7 @@
  *
  * Copyright (c) 2017 Microsemi Corporation
  */
+#include <linux/dsa/ocelot.h>
 #include <linux/if_bridge.h>
 #include <soc/mscc/ocelot_vcap.h>
 #include "ocelot.h"
@@ -628,26 +629,6 @@ void ocelot_get_txtstamp(struct ocelot *ocelot)
 }
 EXPORT_SYMBOL(ocelot_get_txtstamp);
 
-/* Generate the IFH for frame injection
- *
- * The IFH is a 128bit-value
- * bit 127: bypass the analyzer processing
- * bit 56-67: destination mask
- * bit 28-29: pop_cnt: 3 disables all rewriting of the frame
- * bit 20-27: cpu extraction queue mask
- * bit 16: tag type 0: C-tag, 1: S-tag
- * bit 0-11: VID
- */
-static int ocelot_gen_ifh(u32 *ifh, struct frame_info *info)
-{
-	ifh[0] = IFH_INJ_BYPASS | ((0x1ff & info->rew_op) << 21);
-	ifh[1] = (0xf00 & info->port) >> 8;
-	ifh[2] = (0xff & info->port) << 24;
-	ifh[3] = (info->tag_type << 16) | info->vid;
-
-	return 0;
-}
-
 bool ocelot_can_inject(struct ocelot *ocelot, int grp)
 {
 	u32 val = ocelot_read(ocelot, QS_INJ_STATUS);
@@ -664,23 +645,20 @@ EXPORT_SYMBOL(ocelot_can_inject);
 void ocelot_port_inject_frame(struct ocelot *ocelot, int port, int grp,
 			      u32 rew_op, struct sk_buff *skb)
 {
-	struct frame_info info = {};
-	u32 ifh[OCELOT_TAG_LEN / 4];
+	u32 ifh[OCELOT_TAG_LEN / 4] = {0};
 	unsigned int i, count, last;
 
 	ocelot_write_rix(ocelot, QS_INJ_CTRL_GAP_SIZE(1) |
 			 QS_INJ_CTRL_SOF, QS_INJ_CTRL, grp);
 
-	info.port = BIT(port);
-	info.tag_type = IFH_TAG_TYPE_C;
-	info.vid = skb_vlan_tag_get(skb);
-	info.rew_op = rew_op;
-
-	ocelot_gen_ifh(ifh, &info);
+	ocelot_ifh_set_bypass(ifh, 1);
+	ocelot_ifh_set_dest(ifh, BIT(port));
+	ocelot_ifh_set_tag_type(ifh, IFH_TAG_TYPE_C);
+	ocelot_ifh_set_vid(ifh, skb_vlan_tag_get(skb));
+	ocelot_ifh_set_rew_op(ifh, rew_op);
 
 	for (i = 0; i < OCELOT_TAG_LEN / 4; i++)
-		ocelot_write_rix(ocelot, (__force u32)cpu_to_be32(ifh[i]),
-				 QS_INJ_WR, grp);
+		ocelot_write_rix(ocelot, ifh[i], QS_INJ_WR, grp);
 
 	count = DIV_ROUND_UP(skb->len, 4);
 	last = skb->len % 4;
diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h
index c485795c606b..db6b1a4c3926 100644
--- a/drivers/net/ethernet/mscc/ocelot.h
+++ b/drivers/net/ethernet/mscc/ocelot.h
@@ -32,15 +32,6 @@
 
 #define OCELOT_PTP_QUEUE_SZ	128
 
-struct frame_info {
-	u32 len;
-	u16 port;
-	u16 vid;
-	u8 tag_type;
-	u16 rew_op;
-	u32 timestamp;	/* rew_val */
-};
-
 struct ocelot_port_tc {
 	bool block_shared;
 	unsigned long offload_cnt;
diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index b075dc13354a..fe0f8d6a32ce 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -4,6 +4,7 @@
  *
  * Copyright (c) 2017 Microsemi Corporation
  */
+#include <linux/dsa/ocelot.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/of_net.h>
@@ -18,8 +19,6 @@
 #include <soc/mscc/ocelot_hsio.h>
 #include "ocelot.h"
 
-#define IFH_EXTRACT_BITFIELD64(x, o, w) (((x) >> (o)) & GENMASK_ULL((w) - 1, 0))
-
 static const u32 ocelot_ana_regmap[] = {
 	REG(ANA_ADVLEARN,				0x009000),
 	REG(ANA_VLANMASK,				0x009004),
@@ -532,29 +531,6 @@ static int ocelot_chip_init(struct ocelot *ocelot, const struct ocelot_ops *ops)
 	return 0;
 }
 
-static int ocelot_parse_ifh(u32 *_ifh, struct frame_info *info)
-{
-	u8 llen, wlen;
-	u64 ifh[2];
-
-	ifh[0] = be64_to_cpu(((__force __be64 *)_ifh)[0]);
-	ifh[1] = be64_to_cpu(((__force __be64 *)_ifh)[1]);
-
-	wlen = IFH_EXTRACT_BITFIELD64(ifh[0], 7,  8);
-	llen = IFH_EXTRACT_BITFIELD64(ifh[0], 15,  6);
-
-	info->len = OCELOT_BUFFER_CELL_SZ * wlen + llen - 80;
-
-	info->timestamp = IFH_EXTRACT_BITFIELD64(ifh[0], 21, 32);
-
-	info->port = IFH_EXTRACT_BITFIELD64(ifh[1], 43, 4);
-
-	info->tag_type = IFH_EXTRACT_BITFIELD64(ifh[1], 16,  1);
-	info->vid = IFH_EXTRACT_BITFIELD64(ifh[1], 0,  12);
-
-	return 0;
-}
-
 static int ocelot_rx_frame_word(struct ocelot *ocelot, u8 grp, bool ifh,
 				u32 *rval)
 {
@@ -609,20 +585,20 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
 		struct ocelot_port_private *priv;
 		struct ocelot_port *ocelot_port;
 		u64 tod_in_ns, full_ts_in_ns;
-		struct frame_info info = {};
+		u64 src_port, len, timestamp;
 		struct net_device *dev;
-		u32 ifh[4], val, *buf;
+		u32 xfh[4], val, *buf;
 		struct timespec64 ts;
-		int sz, len, buf_len;
 		struct sk_buff *skb;
+		int sz, buf_len;
 
 		for (i = 0; i < OCELOT_TAG_LEN / 4; i++) {
-			err = ocelot_rx_frame_word(ocelot, grp, true, &ifh[i]);
+			err = ocelot_rx_frame_word(ocelot, grp, true, &xfh[i]);
 			if (err != 4)
 				goto out;
 		}
 
-		/* At this point the IFH was read correctly, so it is safe to
+		/* At this point the XFH was read correctly, so it is safe to
 		 * presume that there is no error. The err needs to be reset
 		 * otherwise a frame could come in CPU queue between the while
 		 * condition and the check for error later on. And in that case
@@ -630,21 +606,23 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
 		 */
 		err = 0;
 
-		ocelot_parse_ifh(ifh, &info);
+		ocelot_xfh_get_src_port(xfh, &src_port);
+		ocelot_xfh_get_len(xfh, &len);
+		ocelot_xfh_get_rew_val(xfh, &timestamp);
 
-		ocelot_port = ocelot->ports[info.port];
+		ocelot_port = ocelot->ports[src_port];
 		priv = container_of(ocelot_port, struct ocelot_port_private,
 				    port);
 		dev = priv->dev;
 
-		skb = netdev_alloc_skb(dev, info.len);
+		skb = netdev_alloc_skb(dev, len);
 
 		if (unlikely(!skb)) {
 			netdev_err(dev, "Unable to allocate sk_buff\n");
 			err = -ENOMEM;
 			goto out;
 		}
-		buf_len = info.len - ETH_FCS_LEN;
+		buf_len = len - ETH_FCS_LEN;
 		buf = (u32 *)skb_put(skb, buf_len);
 
 		len = 0;
@@ -677,12 +655,12 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
 			ocelot_ptp_gettime64(&ocelot->ptp_info, &ts);
 
 			tod_in_ns = ktime_set(ts.tv_sec, ts.tv_nsec);
-			if ((tod_in_ns & 0xffffffff) < info.timestamp)
+			if ((tod_in_ns & 0xffffffff) < timestamp)
 				full_ts_in_ns = (((tod_in_ns >> 32) - 1) << 32) |
-						info.timestamp;
+						timestamp;
 			else
 				full_ts_in_ns = (tod_in_ns & GENMASK_ULL(63, 32)) |
-						info.timestamp;
+						timestamp;
 
 			shhwtstamps = skb_hwtstamps(skb);
 			memset(shhwtstamps, 0, sizeof(struct skb_shared_hwtstamps));
@@ -692,7 +670,7 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
 		/* Everything we see on an interface that is in the HW bridge
 		 * has already been forwarded.
 		 */
-		if (ocelot->bridge_mask & BIT(info.port))
+		if (ocelot->bridge_mask & BIT(src_port))
 			skb->offload_fwd_mark = 1;
 
 		skb->protocol = eth_type_trans(skb, dev);
diff --git a/include/linux/dsa/ocelot.h b/include/linux/dsa/ocelot.h
new file mode 100644
index 000000000000..763dbbfaff7a
--- /dev/null
+++ b/include/linux/dsa/ocelot.h
@@ -0,0 +1,208 @@
+/* SPDX-License-Identifier: GPL-2.0
+ * Copyright 2019-2021 NXP Semiconductors
+ */
+
+#ifndef _NET_DSA_TAG_OCELOT_H
+#define _NET_DSA_TAG_OCELOT_H
+
+#include <linux/packing.h>
+
+#define OCELOT_TAG_LEN			16
+#define OCELOT_SHORT_PREFIX_LEN		4
+#define OCELOT_LONG_PREFIX_LEN		16
+#define OCELOT_TOTAL_TAG_LEN	(OCELOT_SHORT_PREFIX_LEN + OCELOT_TAG_LEN)
+
+/* The CPU injection header and the CPU extraction header can have 3 types of
+ * prefixes: long, short and no prefix. The format of the header itself is the
+ * same in all 3 cases.
+ *
+ * Extraction with long prefix:
+ *
+ * +-------------------+-------------------+------+------+------------+-------+
+ * | ff:ff:ff:ff:ff:ff | fe:ff:ff:ff:ff:ff | 8880 | 000a | extraction | frame |
+ * |                   |                   |      |      |   header   |       |
+ * +-------------------+-------------------+------+------+------------+-------+
+ *        48 bits             48 bits      16 bits 16 bits  128 bits
+ *
+ * Extraction with short prefix:
+ *
+ *                                         +------+------+------------+-------+
+ *                                         | 8880 | 000a | extraction | frame |
+ *                                         |      |      |   header   |       |
+ *                                         +------+------+------------+-------+
+ *                                         16 bits 16 bits  128 bits
+ *
+ * Extraction with no prefix:
+ *
+ *                                                       +------------+-------+
+ *                                                       | extraction | frame |
+ *                                                       |   header   |       |
+ *                                                       +------------+-------+
+ *                                                          128 bits
+ *
+ *
+ * Injection with long prefix:
+ *
+ * +-------------------+-------------------+------+------+------------+-------+
+ * |      any dmac     |      any smac     | 8880 | 000a | injection  | frame |
+ * |                   |                   |      |      |   header   |       |
+ * +-------------------+-------------------+------+------+------------+-------+
+ *        48 bits             48 bits      16 bits 16 bits  128 bits
+ *
+ * Injection with short prefix:
+ *
+ *                                         +------+------+------------+-------+
+ *                                         | 8880 | 000a | injection  | frame |
+ *                                         |      |      |   header   |       |
+ *                                         +------+------+------------+-------+
+ *                                         16 bits 16 bits  128 bits
+ *
+ * Injection with no prefix:
+ *
+ *                                                       +------------+-------+
+ *                                                       | injection  | frame |
+ *                                                       |   header   |       |
+ *                                                       +------------+-------+
+ *                                                          128 bits
+ *
+ * The injection header looks like this (network byte order, bit 127
+ * is part of lowest address byte in memory, bit 0 is part of highest
+ * address byte):
+ *
+ *         +------+------+------+------+------+------+------+------+
+ * 127:120 |BYPASS| MASQ |          MASQ_PORT        |REW_OP|REW_OP|
+ *         +------+------+------+------+------+------+------+------+
+ * 119:112 |                         REW_OP                        |
+ *         +------+------+------+------+------+------+------+------+
+ * 111:104 |                         REW_VAL                       |
+ *         +------+------+------+------+------+------+------+------+
+ * 103: 96 |                         REW_VAL                       |
+ *         +------+------+------+------+------+------+------+------+
+ *  95: 88 |                         REW_VAL                       |
+ *         +------+------+------+------+------+------+------+------+
+ *  87: 80 |                         REW_VAL                       |
+ *         +------+------+------+------+------+------+------+------+
+ *  79: 72 |                          RSV                          |
+ *         +------+------+------+------+------+------+------+------+
+ *  71: 64 |            RSV            |           DEST            |
+ *         +------+------+------+------+------+------+------+------+
+ *  63: 56 |                         DEST                          |
+ *         +------+------+------+------+------+------+------+------+
+ *  55: 48 |                          RSV                          |
+ *         +------+------+------+------+------+------+------+------+
+ *  47: 40 |  RSV |         SRC_PORT          |     RSV     |TFRM_TIMER|
+ *         +------+------+------+------+------+------+------+------+
+ *  39: 32 |     TFRM_TIMER     |               RSV                |
+ *         +------+------+------+------+------+------+------+------+
+ *  31: 24 |  RSV |  DP  |   POP_CNT   |           CPUQ            |
+ *         +------+------+------+------+------+------+------+------+
+ *  23: 16 |           CPUQ            |      QOS_CLASS     |TAG_TYPE|
+ *         +------+------+------+------+------+------+------+------+
+ *  15:  8 |         PCP        |  DEI |            VID            |
+ *         +------+------+------+------+------+------+------+------+
+ *   7:  0 |                          VID                          |
+ *         +------+------+------+------+------+------+------+------+
+ *
+ * And the extraction header looks like this:
+ *
+ *         +------+------+------+------+------+------+------+------+
+ * 127:120 |  RSV |                  REW_OP                        |
+ *         +------+------+------+------+------+------+------+------+
+ * 119:112 |       REW_OP       |              REW_VAL             |
+ *         +------+------+------+------+------+------+------+------+
+ * 111:104 |                         REW_VAL                       |
+ *         +------+------+------+------+------+------+------+------+
+ * 103: 96 |                         REW_VAL                       |
+ *         +------+------+------+------+------+------+------+------+
+ *  95: 88 |                         REW_VAL                       |
+ *         +------+------+------+------+------+------+------+------+
+ *  87: 80 |       REW_VAL      |               LLEN               |
+ *         +------+------+------+------+------+------+------+------+
+ *  79: 72 | LLEN |                      WLEN                      |
+ *         +------+------+------+------+------+------+------+------+
+ *  71: 64 | WLEN |                      RSV                       |
+ *         +------+------+------+------+------+------+------+------+
+ *  63: 56 |                          RSV                          |
+ *         +------+------+------+------+------+------+------+------+
+ *  55: 48 |                          RSV                          |
+ *         +------+------+------+------+------+------+------+------+
+ *  47: 40 | RSV  |          SRC_PORT         |       ACL_ID       |
+ *         +------+------+------+------+------+------+------+------+
+ *  39: 32 |       ACL_ID       |  RSV |         SFLOW_ID          |
+ *         +------+------+------+------+------+------+------+------+
+ *  31: 24 |ACL_HIT| DP  |  LRN_FLAGS  |           CPUQ            |
+ *         +------+------+------+------+------+------+------+------+
+ *  23: 16 |           CPUQ            |      QOS_CLASS     |TAG_TYPE|
+ *         +------+------+------+------+------+------+------+------+
+ *  15:  8 |         PCP        |  DEI |            VID            |
+ *         +------+------+------+------+------+------+------+------+
+ *   7:  0 |                          VID                          |
+ *         +------+------+------+------+------+------+------+------+
+ */
+
+static inline void ocelot_xfh_get_rew_val(void *extraction, u64 *rew_val)
+{
+	packing(extraction, rew_val, 116, 85, OCELOT_TAG_LEN, UNPACK, 0);
+}
+
+static inline void ocelot_xfh_get_len(void *extraction, u64 *len)
+{
+	u64 llen, wlen;
+
+	packing(extraction, &llen, 84, 79, OCELOT_TAG_LEN, UNPACK, 0);
+	packing(extraction, &wlen, 78, 71, OCELOT_TAG_LEN, UNPACK, 0);
+
+	*len = 60 * wlen + llen - 80;
+}
+
+static inline void ocelot_xfh_get_src_port(void *extraction, u64 *src_port)
+{
+	packing(extraction, src_port, 46, 43, OCELOT_TAG_LEN, UNPACK, 0);
+}
+
+static inline void ocelot_xfh_get_qos_class(void *extraction, u64 *qos_class)
+{
+	packing(extraction, qos_class, 19, 17, OCELOT_TAG_LEN, UNPACK, 0);
+}
+
+static inline void ocelot_xfh_get_tag_type(void *extraction, u64 *tag_type)
+{
+	packing(extraction, tag_type, 16, 16, OCELOT_TAG_LEN, UNPACK, 0);
+}
+
+static inline void ocelot_xfh_get_vlan_tci(void *extraction, u64 *vlan_tci)
+{
+	packing(extraction, vlan_tci, 15, 0, OCELOT_TAG_LEN, UNPACK, 0);
+}
+
+static inline void ocelot_ifh_set_bypass(void *injection, u64 bypass)
+{
+	packing(injection, &bypass, 127, 127, OCELOT_TAG_LEN, PACK, 0);
+}
+
+static inline void ocelot_ifh_set_rew_op(void *injection, u64 rew_op)
+{
+	packing(injection, &rew_op, 125, 117, OCELOT_TAG_LEN, PACK, 0);
+}
+
+static inline void ocelot_ifh_set_dest(void *injection, u64 dest)
+{
+	packing(injection, &dest, 67, 56, OCELOT_TAG_LEN, PACK, 0);
+}
+
+static inline void ocelot_ifh_set_qos_class(void *injection, u64 qos_class)
+{
+	packing(injection, &qos_class, 19, 17, OCELOT_TAG_LEN, PACK, 0);
+}
+
+static inline void ocelot_ifh_set_tag_type(void *injection, u64 tag_type)
+{
+	packing(injection, &tag_type, 16, 16, OCELOT_TAG_LEN, PACK, 0);
+}
+
+static inline void ocelot_ifh_set_vid(void *injection, u64 vid)
+{
+	packing(injection, &vid, 11, 0, OCELOT_TAG_LEN, PACK, 0);
+}
+
+#endif
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index a22aa944e921..11cae2e968b5 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -87,9 +87,6 @@
 /* Source PGIDs, one per physical port */
 #define PGID_SRC			80
 
-#define IFH_INJ_BYPASS			BIT(31)
-#define IFH_INJ_POP_CNT_DISABLE		(3 << 28)
-
 #define IFH_TAG_TYPE_C			0
 #define IFH_TAG_TYPE_S			1
 
@@ -100,10 +97,6 @@
 #define IFH_REW_OP_ORIGIN_PTP		0x5
 
 #define OCELOT_NUM_TC			8
-#define OCELOT_TAG_LEN			16
-#define OCELOT_SHORT_PREFIX_LEN		4
-#define OCELOT_LONG_PREFIX_LEN		16
-#define OCELOT_TOTAL_TAG_LEN	(OCELOT_SHORT_PREFIX_LEN + OCELOT_TAG_LEN)
 
 #define OCELOT_SPEED_2500		0
 #define OCELOT_SPEED_1000		1
diff --git a/net/dsa/tag_ocelot.c b/net/dsa/tag_ocelot.c
index 225b145fd131..8ce0b26f3520 100644
--- a/net/dsa/tag_ocelot.c
+++ b/net/dsa/tag_ocelot.c
@@ -1,138 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright 2019 NXP Semiconductors
  */
+#include <linux/dsa/ocelot.h>
 #include <soc/mscc/ocelot.h>
-#include <linux/packing.h>
 #include "dsa_priv.h"
 
-/* The CPU injection header and the CPU extraction header can have 3 types of
- * prefixes: long, short and no prefix. The format of the header itself is the
- * same in all 3 cases.
- *
- * Extraction with long prefix:
- *
- * +-------------------+-------------------+------+------+------------+-------+
- * | ff:ff:ff:ff:ff:ff | ff:ff:ff:ff:ff:ff | 8880 | 000a | extraction | frame |
- * |                   |                   |      |      |   header   |       |
- * +-------------------+-------------------+------+------+------------+-------+
- *        48 bits             48 bits      16 bits 16 bits  128 bits
- *
- * Extraction with short prefix:
- *
- *                                         +------+------+------------+-------+
- *                                         | 8880 | 000a | extraction | frame |
- *                                         |      |      |   header   |       |
- *                                         +------+------+------------+-------+
- *                                         16 bits 16 bits  128 bits
- *
- * Extraction with no prefix:
- *
- *                                                       +------------+-------+
- *                                                       | extraction | frame |
- *                                                       |   header   |       |
- *                                                       +------------+-------+
- *                                                          128 bits
- *
- *
- * Injection with long prefix:
- *
- * +-------------------+-------------------+------+------+------------+-------+
- * |      any dmac     |      any smac     | 8880 | 000a | injection  | frame |
- * |                   |                   |      |      |   header   |       |
- * +-------------------+-------------------+------+------+------------+-------+
- *        48 bits             48 bits      16 bits 16 bits  128 bits
- *
- * Injection with short prefix:
- *
- *                                         +------+------+------------+-------+
- *                                         | 8880 | 000a | injection  | frame |
- *                                         |      |      |   header   |       |
- *                                         +------+------+------------+-------+
- *                                         16 bits 16 bits  128 bits
- *
- * Injection with no prefix:
- *
- *                                                       +------------+-------+
- *                                                       | injection  | frame |
- *                                                       |   header   |       |
- *                                                       +------------+-------+
- *                                                          128 bits
- *
- * The injection header looks like this (network byte order, bit 127
- * is part of lowest address byte in memory, bit 0 is part of highest
- * address byte):
- *
- *         +------+------+------+------+------+------+------+------+
- * 127:120 |BYPASS| MASQ |          MASQ_PORT        |REW_OP|REW_OP|
- *         +------+------+------+------+------+------+------+------+
- * 119:112 |                         REW_OP                        |
- *         +------+------+------+------+------+------+------+------+
- * 111:104 |                         REW_VAL                       |
- *         +------+------+------+------+------+------+------+------+
- * 103: 96 |                         REW_VAL                       |
- *         +------+------+------+------+------+------+------+------+
- *  95: 88 |                         REW_VAL                       |
- *         +------+------+------+------+------+------+------+------+
- *  87: 80 |                         REW_VAL                       |
- *         +------+------+------+------+------+------+------+------+
- *  79: 72 |                          RSV                          |
- *         +------+------+------+------+------+------+------+------+
- *  71: 64 |            RSV            |           DEST            |
- *         +------+------+------+------+------+------+------+------+
- *  63: 56 |                         DEST                          |
- *         +------+------+------+------+------+------+------+------+
- *  55: 48 |                          RSV                          |
- *         +------+------+------+------+------+------+------+------+
- *  47: 40 |  RSV |         SRC_PORT          |     RSV     |TFRM_TIMER|
- *         +------+------+------+------+------+------+------+------+
- *  39: 32 |     TFRM_TIMER     |               RSV                |
- *         +------+------+------+------+------+------+------+------+
- *  31: 24 |  RSV |  DP  |   POP_CNT   |           CPUQ            |
- *         +------+------+------+------+------+------+------+------+
- *  23: 16 |           CPUQ            |      QOS_CLASS     |TAG_TYPE|
- *         +------+------+------+------+------+------+------+------+
- *  15:  8 |         PCP        |  DEI |            VID            |
- *         +------+------+------+------+------+------+------+------+
- *   7:  0 |                          VID                          |
- *         +------+------+------+------+------+------+------+------+
- *
- * And the extraction header looks like this:
- *
- *         +------+------+------+------+------+------+------+------+
- * 127:120 |  RSV |                  REW_OP                        |
- *         +------+------+------+------+------+------+------+------+
- * 119:112 |       REW_OP       |              REW_VAL             |
- *         +------+------+------+------+------+------+------+------+
- * 111:104 |                         REW_VAL                       |
- *         +------+------+------+------+------+------+------+------+
- * 103: 96 |                         REW_VAL                       |
- *         +------+------+------+------+------+------+------+------+
- *  95: 88 |                         REW_VAL                       |
- *         +------+------+------+------+------+------+------+------+
- *  87: 80 |       REW_VAL      |               LLEN               |
- *         +------+------+------+------+------+------+------+------+
- *  79: 72 | LLEN |                      WLEN                      |
- *         +------+------+------+------+------+------+------+------+
- *  71: 64 | WLEN |                      RSV                       |
- *         +------+------+------+------+------+------+------+------+
- *  63: 56 |                          RSV                          |
- *         +------+------+------+------+------+------+------+------+
- *  55: 48 |                          RSV                          |
- *         +------+------+------+------+------+------+------+------+
- *  47: 40 | RSV  |          SRC_PORT         |       ACL_ID       |
- *         +------+------+------+------+------+------+------+------+
- *  39: 32 |       ACL_ID       |  RSV |         SFLOW_ID          |
- *         +------+------+------+------+------+------+------+------+
- *  31: 24 |ACL_HIT| DP  |  LRN_FLAGS  |           CPUQ            |
- *         +------+------+------+------+------+------+------+------+
- *  23: 16 |           CPUQ            |      QOS_CLASS     |TAG_TYPE|
- *         +------+------+------+------+------+------+------+------+
- *  15:  8 |         PCP        |  DEI |            VID            |
- *         +------+------+------+------+------+------+------+------+
- *   7:  0 |                          VID                          |
- *         +------+------+------+------+------+------+------+------+
- */
-
 static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
 				   struct net_device *netdev)
 {
@@ -142,7 +14,6 @@ static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
 	struct ocelot *ocelot = ds->priv;
 	struct ocelot_port *ocelot_port;
 	u8 *prefix, *injection;
-	u64 qos_class, rew_op;
 
 	ocelot_port = ocelot->ports[dp->index];
 
@@ -155,19 +26,19 @@ static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
 	/* Fix up the fields which are not statically determined
 	 * in the template
 	 */
-	qos_class = skb->priority;
-	packing(injection, &qos_class, 19,  17, OCELOT_TAG_LEN, PACK, 0);
+	ocelot_ifh_set_qos_class(injection, skb->priority);
 
 	/* TX timestamping was requested */
 	if (clone) {
-		rew_op = ocelot_port->ptp_cmd;
+		u64 rew_op = ocelot_port->ptp_cmd;
+
 		/* Retrieve timestamp ID populated inside skb->cb[0] of the
 		 * clone by ocelot_port_add_txtstamp_skb
 		 */
 		if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP)
 			rew_op |= clone->cb[0] << 3;
 
-		packing(injection, &rew_op, 125, 117, OCELOT_TAG_LEN, PACK, 0);
+		ocelot_ifh_set_rew_op(injection, rew_op);
 	}
 
 	return skb;
@@ -208,10 +79,10 @@ static struct sk_buff *ocelot_rcv(struct sk_buff *skb,
 	/* Remove from inet csum the extraction header */
 	skb_postpull_rcsum(skb, start, OCELOT_TOTAL_TAG_LEN);
 
-	packing(extraction, &src_port,  46, 43, OCELOT_TAG_LEN, UNPACK, 0);
-	packing(extraction, &qos_class, 19, 17, OCELOT_TAG_LEN, UNPACK, 0);
-	packing(extraction, &tag_type,  16, 16, OCELOT_TAG_LEN, UNPACK, 0);
-	packing(extraction, &vlan_tci,  15,  0, OCELOT_TAG_LEN, UNPACK, 0);
+	ocelot_xfh_get_src_port(extraction, &src_port);
+	ocelot_xfh_get_qos_class(extraction, &qos_class);
+	ocelot_xfh_get_tag_type(extraction, &tag_type);
+	ocelot_xfh_get_vlan_tci(extraction, &vlan_tci);
 
 	skb->dev = dsa_master_find_slave(netdev, 0, src_port);
 	if (!skb->dev)
-- 
2.25.1


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

* [PATCH v2 net-next 08/12] net: dsa: tag_ocelot: single out PTP-related transmit tag processing
  2021-02-13 22:37 [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q Vladimir Oltean
                   ` (6 preceding siblings ...)
  2021-02-13 22:37 ` [PATCH v2 net-next 07/12] net: mscc: ocelot: use common tag parsing code with DSA Vladimir Oltean
@ 2021-02-13 22:37 ` Vladimir Oltean
  2021-02-14  1:26   ` Florian Fainelli
  2021-02-13 22:37 ` [PATCH v2 net-next 09/12] net: dsa: tag_ocelot: create separate tagger for Seville Vladimir Oltean
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2021-02-13 22:37 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Richard Cochran,
	Claudiu Manoil, Alexandre Belloni, Vladimir Oltean,
	Maxim Kochetkov, UNGLinuxDriver

From: Vladimir Oltean <vladimir.oltean@nxp.com>

There is one place where we cannot avoid accessing driver data, and that
is 2-step PTP TX timestamping, since the switch wants us to provide a
timestamp request ID through the injection header, which naturally must
come from a sequence number kept by the driver (it is generated by the
.port_txtstamp method prior to the tagger's xmit).

However, since other drivers like dsa_loop do not claim PTP support
anyway, the DSA_SKB_CB(skb)->clone will always be NULL anyway, so if we
move all PTP-related dereferences of struct ocelot and struct ocelot_port
into a separate function, we can effectively ensure that this is dead
code when the ocelot tagger is attached to non-ocelot switches, and the
stateful portion of the tagger is more self-contained.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
None.

 net/dsa/tag_ocelot.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/net/dsa/tag_ocelot.c b/net/dsa/tag_ocelot.c
index 8ce0b26f3520..fe00519229d7 100644
--- a/net/dsa/tag_ocelot.c
+++ b/net/dsa/tag_ocelot.c
@@ -5,6 +5,25 @@
 #include <soc/mscc/ocelot.h>
 #include "dsa_priv.h"
 
+static void ocelot_xmit_ptp(struct dsa_port *dp, void *injection,
+			    struct sk_buff *clone)
+{
+	struct ocelot *ocelot = dp->ds->priv;
+	struct ocelot_port *ocelot_port;
+	u64 rew_op;
+
+	ocelot_port = ocelot->ports[dp->index];
+	rew_op = ocelot_port->ptp_cmd;
+
+	/* Retrieve timestamp ID populated inside skb->cb[0] of the
+	 * clone by ocelot_port_add_txtstamp_skb
+	 */
+	if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP)
+		rew_op |= clone->cb[0] << 3;
+
+	ocelot_ifh_set_rew_op(injection, rew_op);
+}
+
 static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
 				   struct net_device *netdev)
 {
@@ -29,17 +48,8 @@ static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
 	ocelot_ifh_set_qos_class(injection, skb->priority);
 
 	/* TX timestamping was requested */
-	if (clone) {
-		u64 rew_op = ocelot_port->ptp_cmd;
-
-		/* Retrieve timestamp ID populated inside skb->cb[0] of the
-		 * clone by ocelot_port_add_txtstamp_skb
-		 */
-		if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP)
-			rew_op |= clone->cb[0] << 3;
-
-		ocelot_ifh_set_rew_op(injection, rew_op);
-	}
+	if (clone)
+		ocelot_xmit_ptp(dp, injection, clone);
 
 	return skb;
 }
-- 
2.25.1


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

* [PATCH v2 net-next 09/12] net: dsa: tag_ocelot: create separate tagger for Seville
  2021-02-13 22:37 [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q Vladimir Oltean
                   ` (7 preceding siblings ...)
  2021-02-13 22:37 ` [PATCH v2 net-next 08/12] net: dsa: tag_ocelot: single out PTP-related transmit tag processing Vladimir Oltean
@ 2021-02-13 22:37 ` Vladimir Oltean
  2021-02-14  1:29   ` Florian Fainelli
  2021-02-13 22:37 ` [PATCH v2 net-next 10/12] net: mscc: ocelot: refactor ocelot_xtr_irq_handler into ocelot_xtr_poll Vladimir Oltean
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2021-02-13 22:37 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Richard Cochran,
	Claudiu Manoil, Alexandre Belloni, Vladimir Oltean,
	Maxim Kochetkov, UNGLinuxDriver

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The ocelot tagger is a hot mess currently, it relies on memory
initialized by the attached driver for basic frame transmission.
This is against all that DSA tagging protocols stand for, which is that
the transmission and reception of a DSA-tagged frame, the data path,
should be independent from the switch control path, because the tag
protocol is in principle hot-pluggable and reusable across switches
(even if in practice it wasn't until very recently). But if another
driver like dsa_loop wants to make use of tag_ocelot, it couldn't.

This was done to have common code between Felix and Ocelot, which have
one bit difference in the frame header format. Quoting from commit
67c2404922c2 ("net: dsa: felix: create a template for the DSA tags on
xmit"):

    Other alternatives have been analyzed, such as:
    - Create a separate tag_seville.c: too much code duplication for just 1
      bit field difference.
    - Create a separate DSA_TAG_PROTO_SEVILLE under tag_ocelot.c, just like
      tag_brcm.c, which would have a separate .xmit function. Again, too
      much code duplication for just 1 bit field difference.
    - Allocate the template from the init function of the tag_ocelot.c
      module, instead of from the driver: couldn't figure out a method of
      accessing the correct port template corresponding to the correct
      tagger in the .xmit function.

The really interesting part is that Seville should have had its own
tagging protocol defined - it is not compatible on the wire with Ocelot,
even for that single bit. In principle, a packet generated by
DSA_TAG_PROTO_OCELOT when booted on NXP LS1028A would look in a certain
way, but when booted on NXP T1040 it would look differently. The reverse
is also true: a packet generated by a Seville switch would be
interpreted incorrectly by Wireshark if it was told it was generated by
an Ocelot switch.

Actually things are a bit more nuanced. If we concentrate only on the
DSA tag, what I said above is true, but Ocelot/Seville also support an
optional DSA tag prefix, which can be short or long, and it is possible
to distinguish the two taggers based on an integer constant put in that
prefix. Nonetheless, creating a separate tagger is still justified,
since the tag prefix is optional, and without it, there is again no way
to distinguish.

Claiming backwards binary compatibility is a bit more tough, since I've
already changed the format of tag_ocelot once, in commit 5124197ce58b
("net: dsa: tag_ocelot: use a short prefix on both ingress and egress").
Therefore I am not very concerned with treating this as a bugfix and
backporting it to stable kernels (which would be another mess due to the
fact that there would be lots of conflicts with the other DSA_TAG_PROTO*
definitions). It's just simpler to say that the string values of the
taggers have ABI value starting with kernel 5.12, which will be when the
changing of tag protocol via /sys/class/net/<dsa-master>/dsa/tagging
goes live.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
None.

 drivers/net/dsa/ocelot/felix.c           | 18 ++-----
 drivers/net/dsa/ocelot/felix.h           |  1 -
 drivers/net/dsa/ocelot/felix_vsc9959.c   | 26 ---------
 drivers/net/dsa/ocelot/seville_vsc9953.c | 28 +---------
 include/linux/dsa/ocelot.h               | 10 ++++
 include/net/dsa.h                        |  2 +
 net/dsa/tag_ocelot.c                     | 68 ++++++++++++++++++------
 7 files changed, 70 insertions(+), 83 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 4af1187f4d69..f294f5f62505 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -431,6 +431,7 @@ static int felix_set_tag_protocol(struct dsa_switch *ds, int cpu,
 	int err;
 
 	switch (proto) {
+	case DSA_TAG_PROTO_SEVILLE:
 	case DSA_TAG_PROTO_OCELOT:
 		err = felix_setup_tag_npi(ds, cpu);
 		break;
@@ -448,6 +449,7 @@ static void felix_del_tag_protocol(struct dsa_switch *ds, int cpu,
 				   enum dsa_tag_protocol proto)
 {
 	switch (proto) {
+	case DSA_TAG_PROTO_SEVILLE:
 	case DSA_TAG_PROTO_OCELOT:
 		felix_teardown_tag_npi(ds, cpu);
 		break;
@@ -471,7 +473,8 @@ static int felix_change_tag_protocol(struct dsa_switch *ds, int cpu,
 	enum dsa_tag_protocol old_proto = felix->tag_proto;
 	int err;
 
-	if (proto != DSA_TAG_PROTO_OCELOT &&
+	if (proto != DSA_TAG_PROTO_SEVILLE &&
+	    proto != DSA_TAG_PROTO_OCELOT &&
 	    proto != DSA_TAG_PROTO_OCELOT_8021Q)
 		return -EPROTONOSUPPORT;
 
@@ -1003,7 +1006,6 @@ static int felix_init_structs(struct felix *felix, int num_phys_ports)
 	for (port = 0; port < num_phys_ports; port++) {
 		struct ocelot_port *ocelot_port;
 		struct regmap *target;
-		u8 *template;
 
 		ocelot_port = devm_kzalloc(ocelot->dev,
 					   sizeof(struct ocelot_port),
@@ -1029,22 +1031,10 @@ static int felix_init_structs(struct felix *felix, int num_phys_ports)
 			return PTR_ERR(target);
 		}
 
-		template = devm_kzalloc(ocelot->dev, OCELOT_TOTAL_TAG_LEN,
-					GFP_KERNEL);
-		if (!template) {
-			dev_err(ocelot->dev,
-				"Failed to allocate memory for DSA tag\n");
-			kfree(port_phy_modes);
-			return -ENOMEM;
-		}
-
 		ocelot_port->phy_mode = port_phy_modes[port];
 		ocelot_port->ocelot = ocelot;
 		ocelot_port->target = target;
-		ocelot_port->xmit_template = template;
 		ocelot->ports[port] = ocelot_port;
-
-		felix->info->xmit_template_populate(ocelot, port);
 	}
 
 	kfree(port_phy_modes);
diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index 9d4459f2fffb..b2ea425c5803 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h
@@ -34,7 +34,6 @@ struct felix_info {
 				 enum tc_setup_type type, void *type_data);
 	void	(*port_sched_speed_set)(struct ocelot *ocelot, int port,
 					u32 speed);
-	void	(*xmit_template_populate)(struct ocelot *ocelot, int port);
 };
 
 extern const struct dsa_switch_ops felix_switch_ops;
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index cacc6f9c0113..32b885fcaf90 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -1339,31 +1339,6 @@ static int vsc9959_port_setup_tc(struct dsa_switch *ds, int port,
 	}
 }
 
-static void vsc9959_xmit_template_populate(struct ocelot *ocelot, int port)
-{
-	struct ocelot_port *ocelot_port = ocelot->ports[port];
-	u8 *template = ocelot_port->xmit_template;
-	u64 bypass, dest, src;
-	__be32 *prefix;
-	u8 *injection;
-
-	/* Set the source port as the CPU port module and not the
-	 * NPI port
-	 */
-	src = ocelot->num_phys_ports;
-	dest = BIT(port);
-	bypass = true;
-
-	injection = template + OCELOT_SHORT_PREFIX_LEN;
-	prefix = (__be32 *)template;
-
-	packing(injection, &bypass, 127, 127, OCELOT_TAG_LEN, PACK, 0);
-	packing(injection, &dest,    68,  56, OCELOT_TAG_LEN, PACK, 0);
-	packing(injection, &src,     46,  43, OCELOT_TAG_LEN, PACK, 0);
-
-	*prefix = cpu_to_be32(0x8880000a);
-}
-
 static const struct felix_info felix_info_vsc9959 = {
 	.target_io_res		= vsc9959_target_io_res,
 	.port_io_res		= vsc9959_port_io_res,
@@ -1386,7 +1361,6 @@ static const struct felix_info felix_info_vsc9959 = {
 	.prevalidate_phy_mode	= vsc9959_prevalidate_phy_mode,
 	.port_setup_tc		= vsc9959_port_setup_tc,
 	.port_sched_speed_set	= vsc9959_sched_speed_set,
-	.xmit_template_populate	= vsc9959_xmit_template_populate,
 };
 
 static irqreturn_t felix_irq_handler(int irq, void *data)
diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
index d7348ea4831e..84f93a874d50 100644
--- a/drivers/net/dsa/ocelot/seville_vsc9953.c
+++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
@@ -1165,31 +1165,6 @@ static void vsc9953_mdio_bus_free(struct ocelot *ocelot)
 	mdiobus_unregister(felix->imdio);
 }
 
-static void vsc9953_xmit_template_populate(struct ocelot *ocelot, int port)
-{
-	struct ocelot_port *ocelot_port = ocelot->ports[port];
-	u8 *template = ocelot_port->xmit_template;
-	u64 bypass, dest, src;
-	__be32 *prefix;
-	u8 *injection;
-
-	/* Set the source port as the CPU port module and not the
-	 * NPI port
-	 */
-	src = ocelot->num_phys_ports;
-	dest = BIT(port);
-	bypass = true;
-
-	injection = template + OCELOT_SHORT_PREFIX_LEN;
-	prefix = (__be32 *)template;
-
-	packing(injection, &bypass, 127, 127, OCELOT_TAG_LEN, PACK, 0);
-	packing(injection, &dest,    67,  57, OCELOT_TAG_LEN, PACK, 0);
-	packing(injection, &src,     46,  43, OCELOT_TAG_LEN, PACK, 0);
-
-	*prefix = cpu_to_be32(0x88800005);
-}
-
 static const struct felix_info seville_info_vsc9953 = {
 	.target_io_res		= vsc9953_target_io_res,
 	.port_io_res		= vsc9953_port_io_res,
@@ -1206,7 +1181,6 @@ static const struct felix_info seville_info_vsc9953 = {
 	.mdio_bus_free		= vsc9953_mdio_bus_free,
 	.phylink_validate	= vsc9953_phylink_validate,
 	.prevalidate_phy_mode	= vsc9953_prevalidate_phy_mode,
-	.xmit_template_populate	= vsc9953_xmit_template_populate,
 };
 
 static int seville_probe(struct platform_device *pdev)
@@ -1246,7 +1220,7 @@ static int seville_probe(struct platform_device *pdev)
 	ds->ops = &felix_switch_ops;
 	ds->priv = ocelot;
 	felix->ds = ds;
-	felix->tag_proto = DSA_TAG_PROTO_OCELOT;
+	felix->tag_proto = DSA_TAG_PROTO_SEVILLE;
 
 	err = dsa_register_switch(ds);
 	if (err) {
diff --git a/include/linux/dsa/ocelot.h b/include/linux/dsa/ocelot.h
index 763dbbfaff7a..c6bc45ae5e03 100644
--- a/include/linux/dsa/ocelot.h
+++ b/include/linux/dsa/ocelot.h
@@ -195,6 +195,16 @@ static inline void ocelot_ifh_set_qos_class(void *injection, u64 qos_class)
 	packing(injection, &qos_class, 19, 17, OCELOT_TAG_LEN, PACK, 0);
 }
 
+static inline void seville_ifh_set_dest(void *injection, u64 dest)
+{
+	packing(injection, &dest, 67, 57, OCELOT_TAG_LEN, PACK, 0);
+}
+
+static inline void ocelot_ifh_set_src(void *injection, u64 src)
+{
+	packing(injection, &src, 46, 43, OCELOT_TAG_LEN, PACK, 0);
+}
+
 static inline void ocelot_ifh_set_tag_type(void *injection, u64 tag_type)
 {
 	packing(injection, &tag_type, 16, 16, OCELOT_TAG_LEN, PACK, 0);
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 74457aaffec7..b095ef114fe8 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -48,6 +48,7 @@ struct phylink_link_state;
 #define DSA_TAG_PROTO_HELLCREEK_VALUE		18
 #define DSA_TAG_PROTO_XRS700X_VALUE		19
 #define DSA_TAG_PROTO_OCELOT_8021Q_VALUE	20
+#define DSA_TAG_PROTO_SEVILLE_VALUE		21
 
 enum dsa_tag_protocol {
 	DSA_TAG_PROTO_NONE		= DSA_TAG_PROTO_NONE_VALUE,
@@ -71,6 +72,7 @@ enum dsa_tag_protocol {
 	DSA_TAG_PROTO_HELLCREEK		= DSA_TAG_PROTO_HELLCREEK_VALUE,
 	DSA_TAG_PROTO_XRS700X		= DSA_TAG_PROTO_XRS700X_VALUE,
 	DSA_TAG_PROTO_OCELOT_8021Q	= DSA_TAG_PROTO_OCELOT_8021Q_VALUE,
+	DSA_TAG_PROTO_SEVILLE		= DSA_TAG_PROTO_SEVILLE_VALUE,
 };
 
 struct packet_type;
diff --git a/net/dsa/tag_ocelot.c b/net/dsa/tag_ocelot.c
index fe00519229d7..a7dd61c8e005 100644
--- a/net/dsa/tag_ocelot.c
+++ b/net/dsa/tag_ocelot.c
@@ -24,33 +24,52 @@ static void ocelot_xmit_ptp(struct dsa_port *dp, void *injection,
 	ocelot_ifh_set_rew_op(injection, rew_op);
 }
 
-static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
-				   struct net_device *netdev)
+static void ocelot_xmit_common(struct sk_buff *skb, struct net_device *netdev,
+			       __be32 ifh_prefix, void **ifh)
 {
 	struct dsa_port *dp = dsa_slave_to_port(netdev);
 	struct sk_buff *clone = DSA_SKB_CB(skb)->clone;
 	struct dsa_switch *ds = dp->ds;
-	struct ocelot *ocelot = ds->priv;
-	struct ocelot_port *ocelot_port;
-	u8 *prefix, *injection;
-
-	ocelot_port = ocelot->ports[dp->index];
+	void *injection;
+	__be32 *prefix;
 
 	injection = skb_push(skb, OCELOT_TAG_LEN);
-
 	prefix = skb_push(skb, OCELOT_SHORT_PREFIX_LEN);
 
-	memcpy(prefix, ocelot_port->xmit_template, OCELOT_TOTAL_TAG_LEN);
-
-	/* Fix up the fields which are not statically determined
-	 * in the template
-	 */
+	*prefix = ifh_prefix;
+	memset(injection, 0, OCELOT_TAG_LEN);
+	ocelot_ifh_set_bypass(injection, 1);
+	ocelot_ifh_set_src(injection, ds->num_ports);
 	ocelot_ifh_set_qos_class(injection, skb->priority);
 
 	/* TX timestamping was requested */
 	if (clone)
 		ocelot_xmit_ptp(dp, injection, clone);
 
+	*ifh = injection;
+}
+
+static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
+				   struct net_device *netdev)
+{
+	struct dsa_port *dp = dsa_slave_to_port(netdev);
+	void *injection;
+
+	ocelot_xmit_common(skb, netdev, cpu_to_be32(0x8880000a), &injection);
+	ocelot_ifh_set_dest(injection, BIT(dp->index));
+
+	return skb;
+}
+
+static struct sk_buff *seville_xmit(struct sk_buff *skb,
+				    struct net_device *netdev)
+{
+	struct dsa_port *dp = dsa_slave_to_port(netdev);
+	void *injection;
+
+	ocelot_xmit_common(skb, netdev, cpu_to_be32(0x88800005), &injection);
+	seville_ifh_set_dest(injection, BIT(dp->index));
+
 	return skb;
 }
 
@@ -147,7 +166,26 @@ static const struct dsa_device_ops ocelot_netdev_ops = {
 	.promisc_on_master	= true,
 };
 
-MODULE_LICENSE("GPL v2");
+DSA_TAG_DRIVER(ocelot_netdev_ops);
 MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_OCELOT);
 
-module_dsa_tag_driver(ocelot_netdev_ops);
+static const struct dsa_device_ops seville_netdev_ops = {
+	.name			= "seville",
+	.proto			= DSA_TAG_PROTO_SEVILLE,
+	.xmit			= seville_xmit,
+	.rcv			= ocelot_rcv,
+	.overhead		= OCELOT_TOTAL_TAG_LEN,
+	.promisc_on_master	= true,
+};
+
+DSA_TAG_DRIVER(seville_netdev_ops);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_SEVILLE);
+
+static struct dsa_tag_driver *ocelot_tag_driver_array[] = {
+	&DSA_TAG_DRIVER_NAME(ocelot_netdev_ops),
+	&DSA_TAG_DRIVER_NAME(seville_netdev_ops),
+};
+
+module_dsa_tag_drivers(ocelot_tag_driver_array);
+
+MODULE_LICENSE("GPL v2");
-- 
2.25.1


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

* [PATCH v2 net-next 10/12] net: mscc: ocelot: refactor ocelot_xtr_irq_handler into ocelot_xtr_poll
  2021-02-13 22:37 [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q Vladimir Oltean
                   ` (8 preceding siblings ...)
  2021-02-13 22:37 ` [PATCH v2 net-next 09/12] net: dsa: tag_ocelot: create separate tagger for Seville Vladimir Oltean
@ 2021-02-13 22:37 ` Vladimir Oltean
  2021-02-14  2:57   ` Florian Fainelli
  2021-02-13 22:38 ` [PATCH v2 net-next 11/12] net: dsa: felix: setup MMIO filtering rules for PTP when using tag_8021q Vladimir Oltean
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2021-02-13 22:37 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Richard Cochran,
	Claudiu Manoil, Alexandre Belloni, Vladimir Oltean,
	Maxim Kochetkov, UNGLinuxDriver

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Since the felix DSA driver will need to poll the CPU port module for
extracted frames as well, let's create some common functions that read
an Extraction Frame Header, and then an skb, from a CPU extraction
group.

We abuse the struct ocelot_ops :: port_to_netdev function a little bit,
in order to retrieve the DSA port net_device or the ocelot switchdev
net_device based on the source port information from the Extraction
Frame Header, but it's all in the benefit of code simplification -
netdev_alloc_skb needs it. Originally, the port_to_netdev method was
intended for parsing act->dev from tc flower offload code.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
Added shim definitions.

 drivers/net/ethernet/mscc/ocelot.c         | 151 +++++++++++++++++++++
 drivers/net/ethernet/mscc/ocelot_vsc7514.c | 140 +------------------
 include/soc/mscc/ocelot.h                  |   7 +
 3 files changed, 163 insertions(+), 135 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 699b0c1c1780..981811ffcdae 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -629,6 +629,157 @@ void ocelot_get_txtstamp(struct ocelot *ocelot)
 }
 EXPORT_SYMBOL(ocelot_get_txtstamp);
 
+static int ocelot_rx_frame_word(struct ocelot *ocelot, u8 grp, bool ifh,
+				u32 *rval)
+{
+	u32 bytes_valid, val;
+
+	val = ocelot_read_rix(ocelot, QS_XTR_RD, grp);
+	if (val == XTR_NOT_READY) {
+		if (ifh)
+			return -EIO;
+
+		do {
+			val = ocelot_read_rix(ocelot, QS_XTR_RD, grp);
+		} while (val == XTR_NOT_READY);
+	}
+
+	switch (val) {
+	case XTR_ABORT:
+		return -EIO;
+	case XTR_EOF_0:
+	case XTR_EOF_1:
+	case XTR_EOF_2:
+	case XTR_EOF_3:
+	case XTR_PRUNED:
+		bytes_valid = XTR_VALID_BYTES(val);
+		val = ocelot_read_rix(ocelot, QS_XTR_RD, grp);
+		if (val == XTR_ESCAPE)
+			*rval = ocelot_read_rix(ocelot, QS_XTR_RD, grp);
+		else
+			*rval = val;
+
+		return bytes_valid;
+	case XTR_ESCAPE:
+		*rval = ocelot_read_rix(ocelot, QS_XTR_RD, grp);
+
+		return 4;
+	default:
+		*rval = val;
+
+		return 4;
+	}
+}
+
+static int ocelot_xtr_poll_xfh(struct ocelot *ocelot, int grp, u32 *xfh)
+{
+	int i, err = 0;
+
+	for (i = 0; i < OCELOT_TAG_LEN / 4; i++) {
+		err = ocelot_rx_frame_word(ocelot, grp, true, &xfh[i]);
+		if (err != 4)
+			return (err < 0) ? err : -EIO;
+	}
+
+	return 0;
+}
+
+int ocelot_xtr_poll_frame(struct ocelot *ocelot, int grp, struct sk_buff **nskb)
+{
+	struct skb_shared_hwtstamps *shhwtstamps;
+	u64 tod_in_ns, full_ts_in_ns;
+	u64 timestamp, src_port, len;
+	u32 xfh[OCELOT_TAG_LEN / 4];
+	struct net_device *dev;
+	struct timespec64 ts;
+	struct sk_buff *skb;
+	int sz, buf_len;
+	u32 val, *buf;
+	int err;
+
+	err = ocelot_xtr_poll_xfh(ocelot, grp, xfh);
+	if (err)
+		return err;
+
+	ocelot_xfh_get_src_port(xfh, &src_port);
+	ocelot_xfh_get_len(xfh, &len);
+	ocelot_xfh_get_rew_val(xfh, &timestamp);
+
+	if (WARN_ON(src_port >= ocelot->num_phys_ports))
+		return -EINVAL;
+
+	dev = ocelot->ops->port_to_netdev(ocelot, src_port);
+	if (!dev)
+		return -EINVAL;
+
+	skb = netdev_alloc_skb(dev, len);
+	if (unlikely(!skb)) {
+		netdev_err(dev, "Unable to allocate sk_buff\n");
+		return -ENOMEM;
+	}
+
+	buf_len = len - ETH_FCS_LEN;
+	buf = (u32 *)skb_put(skb, buf_len);
+
+	len = 0;
+	do {
+		sz = ocelot_rx_frame_word(ocelot, grp, false, &val);
+		if (sz < 0) {
+			err = sz;
+			goto out_free_skb;
+		}
+		*buf++ = val;
+		len += sz;
+	} while (len < buf_len);
+
+	/* Read the FCS */
+	sz = ocelot_rx_frame_word(ocelot, grp, false, &val);
+	if (sz < 0) {
+		err = sz;
+		goto out_free_skb;
+	}
+
+	/* Update the statistics if part of the FCS was read before */
+	len -= ETH_FCS_LEN - sz;
+
+	if (unlikely(dev->features & NETIF_F_RXFCS)) {
+		buf = (u32 *)skb_put(skb, ETH_FCS_LEN);
+		*buf = val;
+	}
+
+	if (ocelot->ptp) {
+		ocelot_ptp_gettime64(&ocelot->ptp_info, &ts);
+
+		tod_in_ns = ktime_set(ts.tv_sec, ts.tv_nsec);
+		if ((tod_in_ns & 0xffffffff) < timestamp)
+			full_ts_in_ns = (((tod_in_ns >> 32) - 1) << 32) |
+					timestamp;
+		else
+			full_ts_in_ns = (tod_in_ns & GENMASK_ULL(63, 32)) |
+					timestamp;
+
+		shhwtstamps = skb_hwtstamps(skb);
+		memset(shhwtstamps, 0, sizeof(struct skb_shared_hwtstamps));
+		shhwtstamps->hwtstamp = full_ts_in_ns;
+	}
+
+	/* Everything we see on an interface that is in the HW bridge
+	 * has already been forwarded.
+	 */
+	if (ocelot->bridge_mask & BIT(src_port))
+		skb->offload_fwd_mark = 1;
+
+	skb->protocol = eth_type_trans(skb, dev);
+	*nskb = skb;
+
+	return 0;
+
+out_free_skb:
+	kfree_skb(skb);
+	return err;
+}
+EXPORT_SYMBOL(ocelot_xtr_poll_frame);
+
 bool ocelot_can_inject(struct ocelot *ocelot, int grp)
 {
 	u32 val = ocelot_read(ocelot, QS_INJ_STATUS);
diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index fe0f8d6a32ce..47ee06832a51 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -531,153 +531,23 @@ static int ocelot_chip_init(struct ocelot *ocelot, const struct ocelot_ops *ops)
 	return 0;
 }
 
-static int ocelot_rx_frame_word(struct ocelot *ocelot, u8 grp, bool ifh,
-				u32 *rval)
-{
-	u32 val;
-	u32 bytes_valid;
-
-	val = ocelot_read_rix(ocelot, QS_XTR_RD, grp);
-	if (val == XTR_NOT_READY) {
-		if (ifh)
-			return -EIO;
-
-		do {
-			val = ocelot_read_rix(ocelot, QS_XTR_RD, grp);
-		} while (val == XTR_NOT_READY);
-	}
-
-	switch (val) {
-	case XTR_ABORT:
-		return -EIO;
-	case XTR_EOF_0:
-	case XTR_EOF_1:
-	case XTR_EOF_2:
-	case XTR_EOF_3:
-	case XTR_PRUNED:
-		bytes_valid = XTR_VALID_BYTES(val);
-		val = ocelot_read_rix(ocelot, QS_XTR_RD, grp);
-		if (val == XTR_ESCAPE)
-			*rval = ocelot_read_rix(ocelot, QS_XTR_RD, grp);
-		else
-			*rval = val;
-
-		return bytes_valid;
-	case XTR_ESCAPE:
-		*rval = ocelot_read_rix(ocelot, QS_XTR_RD, grp);
-
-		return 4;
-	default:
-		*rval = val;
-
-		return 4;
-	}
-}
-
 static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
 {
 	struct ocelot *ocelot = arg;
-	int i = 0, grp = 0;
-	int err = 0;
+	int grp = 0, err;
 
 	while (ocelot_read(ocelot, QS_XTR_DATA_PRESENT) & BIT(grp)) {
-		struct skb_shared_hwtstamps *shhwtstamps;
-		struct ocelot_port_private *priv;
-		struct ocelot_port *ocelot_port;
-		u64 tod_in_ns, full_ts_in_ns;
-		u64 src_port, len, timestamp;
-		struct net_device *dev;
-		u32 xfh[4], val, *buf;
-		struct timespec64 ts;
 		struct sk_buff *skb;
-		int sz, buf_len;
-
-		for (i = 0; i < OCELOT_TAG_LEN / 4; i++) {
-			err = ocelot_rx_frame_word(ocelot, grp, true, &xfh[i]);
-			if (err != 4)
-				goto out;
-		}
-
-		/* At this point the XFH was read correctly, so it is safe to
-		 * presume that there is no error. The err needs to be reset
-		 * otherwise a frame could come in CPU queue between the while
-		 * condition and the check for error later on. And in that case
-		 * the new frame is just removed and not processed.
-		 */
-		err = 0;
-
-		ocelot_xfh_get_src_port(xfh, &src_port);
-		ocelot_xfh_get_len(xfh, &len);
-		ocelot_xfh_get_rew_val(xfh, &timestamp);
-
-		ocelot_port = ocelot->ports[src_port];
-		priv = container_of(ocelot_port, struct ocelot_port_private,
-				    port);
-		dev = priv->dev;
 
-		skb = netdev_alloc_skb(dev, len);
-
-		if (unlikely(!skb)) {
-			netdev_err(dev, "Unable to allocate sk_buff\n");
-			err = -ENOMEM;
-			goto out;
-		}
-		buf_len = len - ETH_FCS_LEN;
-		buf = (u32 *)skb_put(skb, buf_len);
-
-		len = 0;
-		do {
-			sz = ocelot_rx_frame_word(ocelot, grp, false, &val);
-			if (sz < 0) {
-				err = sz;
-				goto out;
-			}
-			*buf++ = val;
-			len += sz;
-		} while (len < buf_len);
-
-		/* Read the FCS */
-		sz = ocelot_rx_frame_word(ocelot, grp, false, &val);
-		if (sz < 0) {
-			err = sz;
+		err = ocelot_xtr_poll_frame(ocelot, grp, &skb);
+		if (err)
 			goto out;
-		}
-
-		/* Update the statistics if part of the FCS was read before */
-		len -= ETH_FCS_LEN - sz;
-
-		if (unlikely(dev->features & NETIF_F_RXFCS)) {
-			buf = (u32 *)skb_put(skb, ETH_FCS_LEN);
-			*buf = val;
-		}
-
-		if (ocelot->ptp) {
-			ocelot_ptp_gettime64(&ocelot->ptp_info, &ts);
-
-			tod_in_ns = ktime_set(ts.tv_sec, ts.tv_nsec);
-			if ((tod_in_ns & 0xffffffff) < timestamp)
-				full_ts_in_ns = (((tod_in_ns >> 32) - 1) << 32) |
-						timestamp;
-			else
-				full_ts_in_ns = (tod_in_ns & GENMASK_ULL(63, 32)) |
-						timestamp;
-
-			shhwtstamps = skb_hwtstamps(skb);
-			memset(shhwtstamps, 0, sizeof(struct skb_shared_hwtstamps));
-			shhwtstamps->hwtstamp = full_ts_in_ns;
-		}
 
-		/* Everything we see on an interface that is in the HW bridge
-		 * has already been forwarded.
-		 */
-		if (ocelot->bridge_mask & BIT(src_port))
-			skb->offload_fwd_mark = 1;
+		skb->dev->stats.rx_bytes += skb->len;
+		skb->dev->stats.rx_packets++;
 
-		skb->protocol = eth_type_trans(skb, dev);
 		if (!skb_defer_rx_timestamp(skb))
 			netif_rx(skb);
-		dev->stats.rx_bytes += len;
-		dev->stats.rx_packets++;
 	}
 
 out:
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 11cae2e968b5..ded9ae1149bc 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -741,6 +741,7 @@ void __ocelot_target_write_ix(struct ocelot *ocelot, enum ocelot_target target,
 bool ocelot_can_inject(struct ocelot *ocelot, int grp);
 void ocelot_port_inject_frame(struct ocelot *ocelot, int port, int grp,
 			      u32 rew_op, struct sk_buff *skb);
+int ocelot_xtr_poll_frame(struct ocelot *ocelot, int grp, struct sk_buff **skb);
 
 #else
 
@@ -755,6 +756,12 @@ static inline void ocelot_port_inject_frame(struct ocelot *ocelot, int port,
 {
 }
 
+static inline int ocelot_xtr_poll_frame(struct ocelot *ocelot, int grp,
+					struct sk_buff **skb)
+{
+	return -EIO;
+}
+
 #endif
 
 /* Hardware initialization */
-- 
2.25.1


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

* [PATCH v2 net-next 11/12] net: dsa: felix: setup MMIO filtering rules for PTP when using tag_8021q
  2021-02-13 22:37 [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q Vladimir Oltean
                   ` (9 preceding siblings ...)
  2021-02-13 22:37 ` [PATCH v2 net-next 10/12] net: mscc: ocelot: refactor ocelot_xtr_irq_handler into ocelot_xtr_poll Vladimir Oltean
@ 2021-02-13 22:38 ` Vladimir Oltean
  2021-02-14  3:01   ` Florian Fainelli
  2021-02-13 22:38 ` [PATCH v2 net-next 12/12] net: dsa: tag_ocelot_8021q: add support for PTP timestamping Vladimir Oltean
  2021-02-15  1:40 ` [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q patchwork-bot+netdevbpf
  12 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2021-02-13 22:38 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Richard Cochran,
	Claudiu Manoil, Alexandre Belloni, Vladimir Oltean,
	Maxim Kochetkov, UNGLinuxDriver

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Since the tag_8021q tagger is software-defined, it has no means by
itself for retrieving hardware timestamps of PTP event messages.

Because we do want to support PTP on ocelot even with tag_8021q, we need
to use the CPU port module for that. The RX timestamp is present in the
Extraction Frame Header. And because we can't use NPI mode which redirects
the CPU queues to an "external CPU" (meaning the ARM CPU running Linux),
then we need to poll the CPU port module through the MMIO registers to
retrieve TX and RX timestamps.

Sadly, on NXP LS1028A, the Felix switch was integrated into the SoC
without wiring the extraction IRQ line to the ARM GIC. So, if we want to
be notified of any PTP packets received on the CPU port module, we have
a problem.

There is a possible workaround, which is to use the Ethernet CPU port as
a notification channel that packets are available on the CPU port module
as well. When a PTP packet is received by the DSA tagger (without timestamp,
of course), we go to the CPU extraction queues, poll for it there, then
we drop the original Ethernet packet and masquerade the packet retrieved
over MMIO (plus the timestamp) as the original when we inject it up the
stack.

Create a quirk in struct felix is selected by the Felix driver (but not
by Seville, since that doesn't support PTP at all). We want to do this
such that the workaround is minimally invasive for future switches that
don't require this workaround.

The only traffic for which we need timestamps is PTP traffic, so add a
redirection rule to the CPU port module for this. Currently we only have
the need for PTP over L2, so redirection rules for UDP ports 319 and 320
are TBD for now.

Note that for the workaround of matching of PTP-over-Ethernet-port with
PTP-over-MMIO queues to work properly, both channels need to be
absolutely lossless. There are two parts to achieving that:
- We keep flow control enabled on the tag_8021q CPU port
- We put the DSA master interface in promiscuous mode, so it will never
  drop a PTP frame (for the profiles we are interested in, these are
  sent to the multicast MAC addresses of 01-80-c2-00-00-0e and
  01-1b-19-00-00-00).

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
None.

 drivers/net/dsa/ocelot/felix.c         | 131 ++++++++++++++++++++++++-
 drivers/net/dsa/ocelot/felix.h         |  13 +++
 drivers/net/dsa/ocelot/felix_vsc9959.c |   1 +
 net/dsa/tag_ocelot_8021q.c             |   1 +
 4 files changed, 143 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index f294f5f62505..d3b18aa6a582 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -264,6 +264,120 @@ static void felix_8021q_cpu_port_deinit(struct ocelot *ocelot, int port)
 	ocelot_apply_bridge_fwd_mask(ocelot);
 }
 
+/* Set up a VCAP IS2 rule for delivering PTP frames to the CPU port module.
+ * If the quirk_no_xtr_irq is in place, then also copy those PTP frames to the
+ * tag_8021q CPU port.
+ */
+static int felix_setup_mmio_filtering(struct felix *felix)
+{
+	unsigned long user_ports = 0, cpu_ports = 0;
+	struct ocelot_vcap_filter *redirect_rule;
+	struct ocelot_vcap_filter *tagging_rule;
+	struct ocelot *ocelot = &felix->ocelot;
+	struct dsa_switch *ds = felix->ds;
+	int port, ret;
+
+	tagging_rule = kzalloc(sizeof(struct ocelot_vcap_filter), GFP_KERNEL);
+	if (!tagging_rule)
+		return -ENOMEM;
+
+	redirect_rule = kzalloc(sizeof(struct ocelot_vcap_filter), GFP_KERNEL);
+	if (!redirect_rule) {
+		kfree(tagging_rule);
+		return -ENOMEM;
+	}
+
+	for (port = 0; port < ocelot->num_phys_ports; port++) {
+		if (dsa_is_user_port(ds, port))
+			user_ports |= BIT(port);
+		if (dsa_is_cpu_port(ds, port))
+			cpu_ports |= BIT(port);
+	}
+
+	tagging_rule->key_type = OCELOT_VCAP_KEY_ETYPE;
+	*(__be16 *)tagging_rule->key.etype.etype.value = htons(ETH_P_1588);
+	*(__be16 *)tagging_rule->key.etype.etype.mask = htons(0xffff);
+	tagging_rule->ingress_port_mask = user_ports;
+	tagging_rule->prio = 1;
+	tagging_rule->id.cookie = ocelot->num_phys_ports;
+	tagging_rule->id.tc_offload = false;
+	tagging_rule->block_id = VCAP_IS1;
+	tagging_rule->type = OCELOT_VCAP_FILTER_OFFLOAD;
+	tagging_rule->lookup = 0;
+	tagging_rule->action.pag_override_mask = 0xff;
+	tagging_rule->action.pag_val = ocelot->num_phys_ports;
+
+	ret = ocelot_vcap_filter_add(ocelot, tagging_rule, NULL);
+	if (ret) {
+		kfree(tagging_rule);
+		kfree(redirect_rule);
+		return ret;
+	}
+
+	redirect_rule->key_type = OCELOT_VCAP_KEY_ANY;
+	redirect_rule->ingress_port_mask = user_ports;
+	redirect_rule->pag = ocelot->num_phys_ports;
+	redirect_rule->prio = 1;
+	redirect_rule->id.cookie = ocelot->num_phys_ports;
+	redirect_rule->id.tc_offload = false;
+	redirect_rule->block_id = VCAP_IS2;
+	redirect_rule->type = OCELOT_VCAP_FILTER_OFFLOAD;
+	redirect_rule->lookup = 0;
+	redirect_rule->action.cpu_copy_ena = true;
+	if (felix->info->quirk_no_xtr_irq) {
+		/* Redirect to the tag_8021q CPU but also copy PTP packets to
+		 * the CPU port module
+		 */
+		redirect_rule->action.mask_mode = OCELOT_MASK_MODE_REDIRECT;
+		redirect_rule->action.port_mask = cpu_ports;
+	} else {
+		/* Trap PTP packets only to the CPU port module (which is
+		 * redirected to the NPI port)
+		 */
+		redirect_rule->action.mask_mode = OCELOT_MASK_MODE_PERMIT_DENY;
+		redirect_rule->action.port_mask = 0;
+	}
+
+	ret = ocelot_vcap_filter_add(ocelot, redirect_rule, NULL);
+	if (ret) {
+		ocelot_vcap_filter_del(ocelot, tagging_rule);
+		kfree(redirect_rule);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int felix_teardown_mmio_filtering(struct felix *felix)
+{
+	struct ocelot_vcap_filter *tagging_rule, *redirect_rule;
+	struct ocelot_vcap_block *block_vcap_is1;
+	struct ocelot_vcap_block *block_vcap_is2;
+	struct ocelot *ocelot = &felix->ocelot;
+	int err;
+
+	block_vcap_is1 = &ocelot->block[VCAP_IS1];
+	block_vcap_is2 = &ocelot->block[VCAP_IS2];
+
+	tagging_rule = ocelot_vcap_block_find_filter_by_id(block_vcap_is1,
+							   ocelot->num_phys_ports,
+							   false);
+	if (!tagging_rule)
+		return -ENOENT;
+
+	err = ocelot_vcap_filter_del(ocelot, tagging_rule);
+	if (err)
+		return err;
+
+	redirect_rule = ocelot_vcap_block_find_filter_by_id(block_vcap_is2,
+							    ocelot->num_phys_ports,
+							    false);
+	if (!redirect_rule)
+		return -ENOENT;
+
+	return ocelot_vcap_filter_del(ocelot, redirect_rule);
+}
+
 static int felix_setup_tag_8021q(struct dsa_switch *ds, int cpu)
 {
 	struct ocelot *ocelot = ds->priv;
@@ -292,9 +406,9 @@ static int felix_setup_tag_8021q(struct dsa_switch *ds, int cpu)
 				 ANA_PORT_CPU_FWD_BPDU_CFG, port);
 	}
 
-	/* In tag_8021q mode, the CPU port module is unused. So we
-	 * want to disable flooding of any kind to the CPU port module,
-	 * since packets going there will end in a black hole.
+	/* In tag_8021q mode, the CPU port module is unused, except for PTP
+	 * frames. So we want to disable flooding of any kind to the CPU port
+	 * module, since packets going there will end in a black hole.
 	 */
 	cpu_flood = ANA_PGID_PGID_PGID(BIT(ocelot->num_phys_ports));
 	ocelot_rmw_rix(ocelot, 0, cpu_flood, ANA_PGID_PGID, PGID_UC);
@@ -314,8 +428,14 @@ static int felix_setup_tag_8021q(struct dsa_switch *ds, int cpu)
 	if (err)
 		goto out_free_dsa_8021_ctx;
 
+	err = felix_setup_mmio_filtering(felix);
+	if (err)
+		goto out_teardown_dsa_8021q;
+
 	return 0;
 
+out_teardown_dsa_8021q:
+	dsa_8021q_setup(felix->dsa_8021q_ctx, false);
 out_free_dsa_8021_ctx:
 	kfree(felix->dsa_8021q_ctx);
 	return err;
@@ -327,6 +447,11 @@ static void felix_teardown_tag_8021q(struct dsa_switch *ds, int cpu)
 	struct felix *felix = ocelot_to_felix(ocelot);
 	int err, port;
 
+	err = felix_teardown_mmio_filtering(felix);
+	if (err)
+		dev_err(ds->dev, "felix_teardown_mmio_filtering returned %d",
+			err);
+
 	err = dsa_8021q_setup(felix->dsa_8021q_ctx, false);
 	if (err)
 		dev_err(ds->dev, "dsa_8021q_setup returned %d", err);
diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index b2ea425c5803..4d96cad815d5 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h
@@ -23,6 +23,19 @@ struct felix_info {
 	int				switch_pci_bar;
 	int				imdio_pci_bar;
 	const struct ptp_clock_info	*ptp_caps;
+
+	/* Some Ocelot switches are integrated into the SoC without the
+	 * extraction IRQ line connected to the ARM GIC. By enabling this
+	 * workaround, the few packets that are delivered to the CPU port
+	 * module (currently only PTP) are copied not only to the hardware CPU
+	 * port module, but also to the 802.1Q Ethernet CPU port, and polling
+	 * the extraction registers is triggered once the DSA tagger sees a PTP
+	 * frame. The Ethernet frame is only used as a notification: it is
+	 * dropped, and the original frame is extracted over MMIO and annotated
+	 * with the RX timestamp.
+	 */
+	bool				quirk_no_xtr_irq;
+
 	int	(*mdio_bus_alloc)(struct ocelot *ocelot);
 	void	(*mdio_bus_free)(struct ocelot *ocelot);
 	void	(*phylink_validate)(struct ocelot *ocelot, int port,
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 32b885fcaf90..5ff623ee76a6 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -1354,6 +1354,7 @@ static const struct felix_info felix_info_vsc9959 = {
 	.num_tx_queues		= OCELOT_NUM_TC,
 	.switch_pci_bar		= 4,
 	.imdio_pci_bar		= 0,
+	.quirk_no_xtr_irq	= true,
 	.ptp_caps		= &vsc9959_ptp_caps,
 	.mdio_bus_alloc		= vsc9959_mdio_bus_alloc,
 	.mdio_bus_free		= vsc9959_mdio_bus_free,
diff --git a/net/dsa/tag_ocelot_8021q.c b/net/dsa/tag_ocelot_8021q.c
index 8991ebf098a3..190255d06bef 100644
--- a/net/dsa/tag_ocelot_8021q.c
+++ b/net/dsa/tag_ocelot_8021q.c
@@ -60,6 +60,7 @@ static const struct dsa_device_ops ocelot_8021q_netdev_ops = {
 	.xmit			= ocelot_xmit,
 	.rcv			= ocelot_rcv,
 	.overhead		= VLAN_HLEN,
+	.promisc_on_master	= true,
 };
 
 MODULE_LICENSE("GPL v2");
-- 
2.25.1


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

* [PATCH v2 net-next 12/12] net: dsa: tag_ocelot_8021q: add support for PTP timestamping
  2021-02-13 22:37 [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q Vladimir Oltean
                   ` (10 preceding siblings ...)
  2021-02-13 22:38 ` [PATCH v2 net-next 11/12] net: dsa: felix: setup MMIO filtering rules for PTP when using tag_8021q Vladimir Oltean
@ 2021-02-13 22:38 ` Vladimir Oltean
  2021-02-14  3:06   ` Florian Fainelli
  2021-02-15  1:40 ` [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q patchwork-bot+netdevbpf
  12 siblings, 1 reply; 25+ messages in thread
From: Vladimir Oltean @ 2021-02-13 22:38 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Richard Cochran,
	Claudiu Manoil, Alexandre Belloni, Vladimir Oltean,
	Maxim Kochetkov, UNGLinuxDriver

From: Vladimir Oltean <vladimir.oltean@nxp.com>

For TX timestamping, we use the felix_txtstamp method which is common
with the regular (non-8021q) ocelot tagger. This method says that skb
deferral is needed, prepares a timestamp request ID, and puts a clone of
the skb in a queue waiting for the timestamp IRQ.

felix_txtstamp is called by dsa_skb_tx_timestamp() just before the
tagger's xmit method. In the tagger xmit, we divert the packets
classified by dsa_skb_tx_timestamp() as PTP towards the MMIO-based
injection registers, and we declare them as dead towards dsa_slave_xmit.
If not PTP, we proceed with normal tag_8021q stuff.

Then the timestamp IRQ fires, the clone queued up from felix_txtstamp is
matched to the TX timestamp retrieved from the switch's FIFO based on
the timestamp request ID, and the clone is delivered to the stack.

On RX, thanks to the VCAP IS2 rule that redirects the frames with an
EtherType for 1588 towards two destinations:
- the CPU port module (for MMIO based extraction) and
- if the "no XTR IRQ" workaround is in place, the dsa_8021q CPU port
the relevant data path processing starts in the ptp_classify_raw BPF
classifier installed by DSA in the RX data path (post tagger, which is
completely unaware that it saw a PTP packet).

This time we can't reuse the same implementation of .port_rxtstamp that
also works with the default ocelot tagger. That is because felix_rxtstamp
is given an skb with a freshly stripped DSA header, and it says "I don't
need deferral for its RX timestamp, it's right in it, let me show you";
and it just points to the header right behind skb->data, from where it
unpacks the timestamp and annotates the skb with it.

The same thing cannot happen with tag_ocelot_8021q, because for one
thing, the skb did not have an extraction frame header in the first
place, but a VLAN tag with no timestamp information. So the code paths
in felix_rxtstamp for the regular and 8021q tagger are completely
independent. With tag_8021q, the timestamp must come from the packet's
duplicate delivered to the CPU port module, but there is potentially
complex logic to be handled [ and prone to reordering ] if we were to
just start reading packets from the CPU port module, and try to match
them to the one we received over Ethernet and which needs an RX
timestamp. So we do something simple: we tell DSA "give me some time to
think" (we request skb deferral by returning false from .port_rxtstamp)
and we just drop the frame we got over Ethernet with no attempt to match
it to anything - we just treat it as a notification that there's data to
be processed from the CPU port module's queues. Then we proceed to read
the packets from those, one by one, which we deliver up the stack,
timestamped, using netif_rx - the same function that any driver would
use anyway if it needed RX timestamp deferral. So the assumption is that
we'll come across the PTP packet that triggered the CPU extraction
notification eventually, but we don't know when exactly. Thanks to the
VCAP IS2 trap/redirect rule and the exclusion of the CPU port module
from the flooding replicators, only PTP frames should be present in the
CPU port module's RX queues anyway.

There is just one conflict between the VCAP IS2 trapping rule and the
semantics of the BPF classifier. Namely, ptp_classify_raw() deems
general messages as non-timestampable, but still, those are trapped to
the CPU port module since they have an EtherType of ETH_P_1588. So, if
the "no XTR IRQ" workaround is in place, we need to run another BPF
classifier on the frames extracted over MMIO, to avoid duplicates being
sent to the stack (once over Ethernet, once over MMIO). It doesn't look
like it's possible to install VCAP IS2 rules based on keys extracted
from the 1588 frame headers.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
- added the "ocelot_can_inject" check
- expanded a bit more on the patch implementation aspects

 drivers/net/dsa/ocelot/felix.c             | 69 ++++++++++++++++++++++
 drivers/net/ethernet/mscc/ocelot.c         |  7 +++
 drivers/net/ethernet/mscc/ocelot_vsc7514.c |  3 +-
 include/soc/mscc/ocelot.h                  |  5 ++
 net/dsa/tag_ocelot_8021q.c                 | 33 +++++++++++
 5 files changed, 115 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index d3b18aa6a582..b2e6a5b14f02 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -16,6 +16,7 @@
 #include <linux/dsa/8021q.h>
 #include <linux/dsa/ocelot.h>
 #include <linux/platform_device.h>
+#include <linux/ptp_classify.h>
 #include <linux/module.h>
 #include <linux/of_net.h>
 #include <linux/pci.h>
@@ -345,6 +346,15 @@ static int felix_setup_mmio_filtering(struct felix *felix)
 		return ret;
 	}
 
+	/* The ownership of the CPU port module's queues might have just been
+	 * transferred to the tag_8021q tagger from the NPI-based tagger.
+	 * So there might still be all sorts of crap in the queues. On the
+	 * other hand, the MMIO-based matching of PTP frames is very brittle,
+	 * so we need to be careful that there are no extra frames to be
+	 * dequeued over MMIO, since we would never know to discard them.
+	 */
+	ocelot_drain_cpu_queue(ocelot, 0);
+
 	return 0;
 }
 
@@ -1283,6 +1293,55 @@ static int felix_hwtstamp_set(struct dsa_switch *ds, int port,
 	return ocelot_hwstamp_set(ocelot, port, ifr);
 }
 
+static bool felix_check_xtr_pkt(struct ocelot *ocelot, unsigned int ptp_type)
+{
+	struct felix *felix = ocelot_to_felix(ocelot);
+	int err, grp = 0;
+
+	if (felix->tag_proto != DSA_TAG_PROTO_OCELOT_8021Q)
+		return false;
+
+	if (!felix->info->quirk_no_xtr_irq)
+		return false;
+
+	if (ptp_type == PTP_CLASS_NONE)
+		return false;
+
+	while (ocelot_read(ocelot, QS_XTR_DATA_PRESENT) & BIT(grp)) {
+		struct sk_buff *skb;
+		unsigned int type;
+
+		err = ocelot_xtr_poll_frame(ocelot, grp, &skb);
+		if (err)
+			goto out;
+
+		/* We trap to the CPU port module all PTP frames, but
+		 * felix_rxtstamp() only gets called for event frames.
+		 * So we need to avoid sending duplicate general
+		 * message frames by running a second BPF classifier
+		 * here and dropping those.
+		 */
+		__skb_push(skb, ETH_HLEN);
+
+		type = ptp_classify_raw(skb);
+
+		__skb_pull(skb, ETH_HLEN);
+
+		if (type == PTP_CLASS_NONE) {
+			kfree_skb(skb);
+			continue;
+		}
+
+		netif_rx(skb);
+	}
+
+out:
+	if (err < 0)
+		ocelot_drain_cpu_queue(ocelot, 0);
+
+	return true;
+}
+
 static bool felix_rxtstamp(struct dsa_switch *ds, int port,
 			   struct sk_buff *skb, unsigned int type)
 {
@@ -1293,6 +1352,16 @@ static bool felix_rxtstamp(struct dsa_switch *ds, int port,
 	struct timespec64 ts;
 	u64 tstamp, val;
 
+	/* If the "no XTR IRQ" workaround is in use, tell DSA to defer this skb
+	 * for RX timestamping. Then free it, and poll for its copy through
+	 * MMIO in the CPU port module, and inject that into the stack from
+	 * ocelot_xtr_poll().
+	 */
+	if (felix_check_xtr_pkt(ocelot, type)) {
+		kfree_skb(skb);
+		return true;
+	}
+
 	ocelot_ptp_gettime64(&ocelot->ptp_info, &ts);
 	tstamp = ktime_set(ts.tv_sec, ts.tv_nsec);
 
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 981811ffcdae..8d97c731e953 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -837,6 +837,13 @@ void ocelot_port_inject_frame(struct ocelot *ocelot, int port, int grp,
 }
 EXPORT_SYMBOL(ocelot_port_inject_frame);
 
+void ocelot_drain_cpu_queue(struct ocelot *ocelot, int grp)
+{
+	while (ocelot_read(ocelot, QS_XTR_DATA_PRESENT) & BIT(grp))
+		ocelot_read_rix(ocelot, QS_XTR_RD, grp);
+}
+EXPORT_SYMBOL(ocelot_drain_cpu_queue);
+
 int ocelot_fdb_add(struct ocelot *ocelot, int port,
 		   const unsigned char *addr, u16 vid)
 {
diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index 47ee06832a51..4bd7e9d9ec61 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -552,8 +552,7 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
 
 out:
 	if (err < 0)
-		while (ocelot_read(ocelot, QS_XTR_DATA_PRESENT) & BIT(grp))
-			ocelot_read_rix(ocelot, QS_XTR_RD, grp);
+		ocelot_drain_cpu_queue(ocelot, 0);
 
 	return IRQ_HANDLED;
 }
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index ded9ae1149bc..1f2d90976564 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -742,6 +742,7 @@ bool ocelot_can_inject(struct ocelot *ocelot, int grp);
 void ocelot_port_inject_frame(struct ocelot *ocelot, int port, int grp,
 			      u32 rew_op, struct sk_buff *skb);
 int ocelot_xtr_poll_frame(struct ocelot *ocelot, int grp, struct sk_buff **skb);
+void ocelot_drain_cpu_queue(struct ocelot *ocelot, int grp);
 
 #else
 
@@ -762,6 +763,10 @@ static inline int ocelot_xtr_poll_frame(struct ocelot *ocelot, int grp,
 	return -EIO;
 }
 
+static inline void ocelot_drain_cpu_queue(struct ocelot *ocelot, int grp)
+{
+}
+
 #endif
 
 /* Hardware initialization */
diff --git a/net/dsa/tag_ocelot_8021q.c b/net/dsa/tag_ocelot_8021q.c
index 190255d06bef..5f3e8e124a82 100644
--- a/net/dsa/tag_ocelot_8021q.c
+++ b/net/dsa/tag_ocelot_8021q.c
@@ -9,8 +9,36 @@
  *   that on egress
  */
 #include <linux/dsa/8021q.h>
+#include <soc/mscc/ocelot.h>
+#include <soc/mscc/ocelot_ptp.h>
 #include "dsa_priv.h"
 
+static struct sk_buff *ocelot_xmit_ptp(struct dsa_port *dp,
+				       struct sk_buff *skb,
+				       struct sk_buff *clone)
+{
+	struct ocelot *ocelot = dp->ds->priv;
+	struct ocelot_port *ocelot_port;
+	int port = dp->index;
+	u32 rew_op;
+
+	if (!ocelot_can_inject(ocelot, 0))
+		return NULL;
+
+	ocelot_port = ocelot->ports[port];
+	rew_op = ocelot_port->ptp_cmd;
+
+	/* Retrieve timestamp ID populated inside skb->cb[0] of the
+	 * clone by ocelot_port_add_txtstamp_skb
+	 */
+	if (ocelot_port->ptp_cmd == IFH_REW_OP_TWO_STEP_PTP)
+		rew_op |= clone->cb[0] << 3;
+
+	ocelot_port_inject_frame(ocelot, port, 0, rew_op, skb);
+
+	return NULL;
+}
+
 static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
 				   struct net_device *netdev)
 {
@@ -18,6 +46,11 @@ static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
 	u16 tx_vid = dsa_8021q_tx_vid(dp->ds, dp->index);
 	u16 queue_mapping = skb_get_queue_mapping(skb);
 	u8 pcp = netdev_txq_to_tc(netdev, queue_mapping);
+	struct sk_buff *clone = DSA_SKB_CB(skb)->clone;
+
+	/* TX timestamping was requested, so inject through MMIO */
+	if (clone)
+		return ocelot_xmit_ptp(dp, skb, clone);
 
 	return dsa_8021q_xmit(skb, netdev, ETH_P_8021Q,
 			      ((pcp << VLAN_PRIO_SHIFT) | tx_vid));
-- 
2.25.1


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

* Re: [PATCH v2 net-next 02/12] net: mscc: ocelot: only drain extraction queue on error
  2021-02-13 22:37 ` [PATCH v2 net-next 02/12] net: mscc: ocelot: only drain extraction queue on error Vladimir Oltean
@ 2021-02-14  1:19   ` Florian Fainelli
  0 siblings, 0 replies; 25+ messages in thread
From: Florian Fainelli @ 2021-02-14  1:19 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Richard Cochran, Claudiu Manoil,
	Alexandre Belloni, Vladimir Oltean, Maxim Kochetkov,
	UNGLinuxDriver



On 2/13/2021 14:37, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> It appears that the intention of this snippet of code is to not exit
> ocelot_xtr_irq_handler() while in the middle of extracting a frame.
> The problem in extracting it word by word is that future extraction
> attempts are really easy to get desynchronized, since the IRQ handler
> assumes that the first 16 bytes are the IFH, which give further
> information about the frame, such as frame length.
> 
> But during normal operation, "err" will not be 0, but 4, set from here:
> 
> 		for (i = 0; i < OCELOT_TAG_LEN / 4; i++) {
> 			err = ocelot_rx_frame_word(ocelot, grp, true, &ifh[i]);
> 			if (err != 4)
> 				break;
> 		}
> 
> 		if (err != 4)
> 			break;
> 
> In that case, draining the extraction queue is a no-op. So explicitly
> make this code execute only on negative err.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

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

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

* Re: [PATCH v2 net-next 03/12] net: mscc: ocelot: better error handling in ocelot_xtr_irq_handler
  2021-02-13 22:37 ` [PATCH v2 net-next 03/12] net: mscc: ocelot: better error handling in ocelot_xtr_irq_handler Vladimir Oltean
@ 2021-02-14  1:20   ` Florian Fainelli
  0 siblings, 0 replies; 25+ messages in thread
From: Florian Fainelli @ 2021-02-14  1:20 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Richard Cochran, Claudiu Manoil,
	Alexandre Belloni, Vladimir Oltean, Maxim Kochetkov,
	UNGLinuxDriver



On 2/13/2021 14:37, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The ocelot_rx_frame_word() function can return a negative error code,
> however this isn't being checked for consistently. Errors being ignored
> have not been seen in practice though.
> 
> Also, some constructs can be simplified by using "goto" instead of
> repeated "break" statements.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

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

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

* Re: [PATCH v2 net-next 04/12] net: mscc: ocelot: use DIV_ROUND_UP helper in ocelot_port_inject_frame
  2021-02-13 22:37 ` [PATCH v2 net-next 04/12] net: mscc: ocelot: use DIV_ROUND_UP helper in ocelot_port_inject_frame Vladimir Oltean
@ 2021-02-14  1:20   ` Florian Fainelli
  0 siblings, 0 replies; 25+ messages in thread
From: Florian Fainelli @ 2021-02-14  1:20 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Richard Cochran, Claudiu Manoil,
	Alexandre Belloni, Vladimir Oltean, Maxim Kochetkov,
	UNGLinuxDriver



On 2/13/2021 14:37, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> This looks a bit nicer than the open-coded "(x + 3) % 4" idiom.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

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

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

* Re: [PATCH v2 net-next 05/12] net: mscc: ocelot: refactor ocelot_port_inject_frame out of ocelot_port_xmit
  2021-02-13 22:37 ` [PATCH v2 net-next 05/12] net: mscc: ocelot: refactor ocelot_port_inject_frame out of ocelot_port_xmit Vladimir Oltean
@ 2021-02-14  1:22   ` Florian Fainelli
  0 siblings, 0 replies; 25+ messages in thread
From: Florian Fainelli @ 2021-02-14  1:22 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Richard Cochran, Claudiu Manoil,
	Alexandre Belloni, Vladimir Oltean, Maxim Kochetkov,
	UNGLinuxDriver



On 2/13/2021 14:37, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The felix DSA driver will inject some frames through register MMIO, same
> as ocelot switchdev currently does. So we need to be able to reuse the
> common code.
> 
> Also create some shim definitions, since the DSA tagger can be compiled
> without support for the switch driver.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

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

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

* Re: [PATCH v2 net-next 06/12] net: dsa: tag_ocelot: avoid accessing ds->priv in ocelot_rcv
  2021-02-13 22:37 ` [PATCH v2 net-next 06/12] net: dsa: tag_ocelot: avoid accessing ds->priv in ocelot_rcv Vladimir Oltean
@ 2021-02-14  1:23   ` Florian Fainelli
  0 siblings, 0 replies; 25+ messages in thread
From: Florian Fainelli @ 2021-02-14  1:23 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Richard Cochran, Claudiu Manoil,
	Alexandre Belloni, Vladimir Oltean, Maxim Kochetkov,
	UNGLinuxDriver



On 2/13/2021 14:37, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Taggers should be written to do something valid irrespective of the
> switch driver that they are attached to. This is even more true now,
> because since the introduction of the .change_tag_protocol method, a
> certain tagger is not necessarily strictly associated with a driver any
> longer, and I would like to be able to test all taggers with dsa_loop in
> the future.
> 
> In the case of ocelot, it needs to move the classified VLAN from the DSA
> tag into the skb if the port is VLAN-aware. We can allow it to do that
> by looking at the dp->vlan_filtering property, no need to invoke
> structures which are specific to ocelot.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

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

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

* Re: [PATCH v2 net-next 07/12] net: mscc: ocelot: use common tag parsing code with DSA
  2021-02-13 22:37 ` [PATCH v2 net-next 07/12] net: mscc: ocelot: use common tag parsing code with DSA Vladimir Oltean
@ 2021-02-14  1:25   ` Florian Fainelli
  0 siblings, 0 replies; 25+ messages in thread
From: Florian Fainelli @ 2021-02-14  1:25 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Richard Cochran, Claudiu Manoil,
	Alexandre Belloni, Vladimir Oltean, Maxim Kochetkov,
	UNGLinuxDriver



On 2/13/2021 14:37, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The Injection Frame Header and Extraction Frame Header that the switch
> prepends to frames over the NPI port is also prepended to frames
> delivered over the CPU port module's queues.
> 
> Let's unify the handling of the frame headers by making the ocelot
> driver call some helpers exported by the DSA tagger. Among other things,
> this allows us to get rid of the strange cpu_to_be32 when transmitting
> the Injection Frame Header on ocelot, since the packing API uses
> network byte order natively (when "quirks" is 0).
> 
> The comments above ocelot_gen_ifh talk about setting pop_cnt to 3, and
> the cpu extraction queue mask to something, but the code doesn't do it,
> so we don't do it either.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

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

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

* Re: [PATCH v2 net-next 08/12] net: dsa: tag_ocelot: single out PTP-related transmit tag processing
  2021-02-13 22:37 ` [PATCH v2 net-next 08/12] net: dsa: tag_ocelot: single out PTP-related transmit tag processing Vladimir Oltean
@ 2021-02-14  1:26   ` Florian Fainelli
  0 siblings, 0 replies; 25+ messages in thread
From: Florian Fainelli @ 2021-02-14  1:26 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Richard Cochran, Claudiu Manoil,
	Alexandre Belloni, Vladimir Oltean, Maxim Kochetkov,
	UNGLinuxDriver



On 2/13/2021 14:37, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> There is one place where we cannot avoid accessing driver data, and that
> is 2-step PTP TX timestamping, since the switch wants us to provide a
> timestamp request ID through the injection header, which naturally must
> come from a sequence number kept by the driver (it is generated by the
> .port_txtstamp method prior to the tagger's xmit).
> 
> However, since other drivers like dsa_loop do not claim PTP support
> anyway, the DSA_SKB_CB(skb)->clone will always be NULL anyway, so if we
> move all PTP-related dereferences of struct ocelot and struct ocelot_port
> into a separate function, we can effectively ensure that this is dead
> code when the ocelot tagger is attached to non-ocelot switches, and the
> stateful portion of the tagger is more self-contained.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

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

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

* Re: [PATCH v2 net-next 09/12] net: dsa: tag_ocelot: create separate tagger for Seville
  2021-02-13 22:37 ` [PATCH v2 net-next 09/12] net: dsa: tag_ocelot: create separate tagger for Seville Vladimir Oltean
@ 2021-02-14  1:29   ` Florian Fainelli
  0 siblings, 0 replies; 25+ messages in thread
From: Florian Fainelli @ 2021-02-14  1:29 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Richard Cochran, Claudiu Manoil,
	Alexandre Belloni, Vladimir Oltean, Maxim Kochetkov,
	UNGLinuxDriver



On 2/13/2021 14:37, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The ocelot tagger is a hot mess currently, it relies on memory
> initialized by the attached driver for basic frame transmission.
> This is against all that DSA tagging protocols stand for, which is that
> the transmission and reception of a DSA-tagged frame, the data path,
> should be independent from the switch control path, because the tag
> protocol is in principle hot-pluggable and reusable across switches
> (even if in practice it wasn't until very recently). But if another
> driver like dsa_loop wants to make use of tag_ocelot, it couldn't.
> 
> This was done to have common code between Felix and Ocelot, which have
> one bit difference in the frame header format. Quoting from commit
> 67c2404922c2 ("net: dsa: felix: create a template for the DSA tags on
> xmit"):
> 
>      Other alternatives have been analyzed, such as:
>      - Create a separate tag_seville.c: too much code duplication for just 1
>        bit field difference.
>      - Create a separate DSA_TAG_PROTO_SEVILLE under tag_ocelot.c, just like
>        tag_brcm.c, which would have a separate .xmit function. Again, too
>        much code duplication for just 1 bit field difference.
>      - Allocate the template from the init function of the tag_ocelot.c
>        module, instead of from the driver: couldn't figure out a method of
>        accessing the correct port template corresponding to the correct
>        tagger in the .xmit function.
> 
> The really interesting part is that Seville should have had its own
> tagging protocol defined - it is not compatible on the wire with Ocelot,
> even for that single bit. In principle, a packet generated by
> DSA_TAG_PROTO_OCELOT when booted on NXP LS1028A would look in a certain
> way, but when booted on NXP T1040 it would look differently. The reverse
> is also true: a packet generated by a Seville switch would be
> interpreted incorrectly by Wireshark if it was told it was generated by
> an Ocelot switch.
> 
> Actually things are a bit more nuanced. If we concentrate only on the
> DSA tag, what I said above is true, but Ocelot/Seville also support an
> optional DSA tag prefix, which can be short or long, and it is possible
> to distinguish the two taggers based on an integer constant put in that
> prefix. Nonetheless, creating a separate tagger is still justified,
> since the tag prefix is optional, and without it, there is again no way
> to distinguish.
> 
> Claiming backwards binary compatibility is a bit more tough, since I've
> already changed the format of tag_ocelot once, in commit 5124197ce58b
> ("net: dsa: tag_ocelot: use a short prefix on both ingress and egress").
> Therefore I am not very concerned with treating this as a bugfix and
> backporting it to stable kernels (which would be another mess due to the
> fact that there would be lots of conflicts with the other DSA_TAG_PROTO*
> definitions). It's just simpler to say that the string values of the
> taggers have ABI value starting with kernel 5.12, which will be when the
> changing of tag protocol via /sys/class/net/<dsa-master>/dsa/tagging
> goes live.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

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

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

* Re: [PATCH v2 net-next 10/12] net: mscc: ocelot: refactor ocelot_xtr_irq_handler into ocelot_xtr_poll
  2021-02-13 22:37 ` [PATCH v2 net-next 10/12] net: mscc: ocelot: refactor ocelot_xtr_irq_handler into ocelot_xtr_poll Vladimir Oltean
@ 2021-02-14  2:57   ` Florian Fainelli
  0 siblings, 0 replies; 25+ messages in thread
From: Florian Fainelli @ 2021-02-14  2:57 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Richard Cochran, Claudiu Manoil,
	Alexandre Belloni, Vladimir Oltean, Maxim Kochetkov,
	UNGLinuxDriver



On 2/13/2021 14:37, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Since the felix DSA driver will need to poll the CPU port module for
> extracted frames as well, let's create some common functions that read
> an Extraction Frame Header, and then an skb, from a CPU extraction
> group.
> 
> We abuse the struct ocelot_ops :: port_to_netdev function a little bit,
> in order to retrieve the DSA port net_device or the ocelot switchdev
> net_device based on the source port information from the Extraction
> Frame Header, but it's all in the benefit of code simplification -
> netdev_alloc_skb needs it. Originally, the port_to_netdev method was
> intended for parsing act->dev from tc flower offload code.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

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

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

* Re: [PATCH v2 net-next 11/12] net: dsa: felix: setup MMIO filtering rules for PTP when using tag_8021q
  2021-02-13 22:38 ` [PATCH v2 net-next 11/12] net: dsa: felix: setup MMIO filtering rules for PTP when using tag_8021q Vladimir Oltean
@ 2021-02-14  3:01   ` Florian Fainelli
  0 siblings, 0 replies; 25+ messages in thread
From: Florian Fainelli @ 2021-02-14  3:01 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Richard Cochran, Claudiu Manoil,
	Alexandre Belloni, Vladimir Oltean, Maxim Kochetkov,
	UNGLinuxDriver



On 2/13/2021 14:38, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Since the tag_8021q tagger is software-defined, it has no means by
> itself for retrieving hardware timestamps of PTP event messages.
> 
> Because we do want to support PTP on ocelot even with tag_8021q, we need
> to use the CPU port module for that. The RX timestamp is present in the
> Extraction Frame Header. And because we can't use NPI mode which redirects
> the CPU queues to an "external CPU" (meaning the ARM CPU running Linux),
> then we need to poll the CPU port module through the MMIO registers to
> retrieve TX and RX timestamps.
> 
> Sadly, on NXP LS1028A, the Felix switch was integrated into the SoC
> without wiring the extraction IRQ line to the ARM GIC. So, if we want to
> be notified of any PTP packets received on the CPU port module, we have
> a problem.
> 
> There is a possible workaround, which is to use the Ethernet CPU port as
> a notification channel that packets are available on the CPU port module
> as well. When a PTP packet is received by the DSA tagger (without timestamp,
> of course), we go to the CPU extraction queues, poll for it there, then
> we drop the original Ethernet packet and masquerade the packet retrieved
> over MMIO (plus the timestamp) as the original when we inject it up the
> stack.
> 
> Create a quirk in struct felix is selected by the Felix driver (but not
> by Seville, since that doesn't support PTP at all). We want to do this
> such that the workaround is minimally invasive for future switches that
> don't require this workaround.
> 
> The only traffic for which we need timestamps is PTP traffic, so add a
> redirection rule to the CPU port module for this. Currently we only have
> the need for PTP over L2, so redirection rules for UDP ports 319 and 320
> are TBD for now.
> 
> Note that for the workaround of matching of PTP-over-Ethernet-port with
> PTP-over-MMIO queues to work properly, both channels need to be
> absolutely lossless. There are two parts to achieving that:
> - We keep flow control enabled on the tag_8021q CPU port
> - We put the DSA master interface in promiscuous mode, so it will never
>    drop a PTP frame (for the profiles we are interested in, these are
>    sent to the multicast MAC addresses of 01-80-c2-00-00-0e and
>    01-1b-19-00-00-00).
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

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

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

* Re: [PATCH v2 net-next 12/12] net: dsa: tag_ocelot_8021q: add support for PTP timestamping
  2021-02-13 22:38 ` [PATCH v2 net-next 12/12] net: dsa: tag_ocelot_8021q: add support for PTP timestamping Vladimir Oltean
@ 2021-02-14  3:06   ` Florian Fainelli
  0 siblings, 0 replies; 25+ messages in thread
From: Florian Fainelli @ 2021-02-14  3:06 UTC (permalink / raw)
  To: Vladimir Oltean, David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Richard Cochran, Claudiu Manoil,
	Alexandre Belloni, Vladimir Oltean, Maxim Kochetkov,
	UNGLinuxDriver



On 2/13/2021 14:38, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> For TX timestamping, we use the felix_txtstamp method which is common
> with the regular (non-8021q) ocelot tagger. This method says that skb
> deferral is needed, prepares a timestamp request ID, and puts a clone of
> the skb in a queue waiting for the timestamp IRQ.
> 
> felix_txtstamp is called by dsa_skb_tx_timestamp() just before the
> tagger's xmit method. In the tagger xmit, we divert the packets
> classified by dsa_skb_tx_timestamp() as PTP towards the MMIO-based
> injection registers, and we declare them as dead towards dsa_slave_xmit.
> If not PTP, we proceed with normal tag_8021q stuff.
> 
> Then the timestamp IRQ fires, the clone queued up from felix_txtstamp is
> matched to the TX timestamp retrieved from the switch's FIFO based on
> the timestamp request ID, and the clone is delivered to the stack.
> 
> On RX, thanks to the VCAP IS2 rule that redirects the frames with an
> EtherType for 1588 towards two destinations:
> - the CPU port module (for MMIO based extraction) and
> - if the "no XTR IRQ" workaround is in place, the dsa_8021q CPU port
> the relevant data path processing starts in the ptp_classify_raw BPF
> classifier installed by DSA in the RX data path (post tagger, which is
> completely unaware that it saw a PTP packet).
> 
> This time we can't reuse the same implementation of .port_rxtstamp that
> also works with the default ocelot tagger. That is because felix_rxtstamp
> is given an skb with a freshly stripped DSA header, and it says "I don't
> need deferral for its RX timestamp, it's right in it, let me show you";
> and it just points to the header right behind skb->data, from where it
> unpacks the timestamp and annotates the skb with it.
> 
> The same thing cannot happen with tag_ocelot_8021q, because for one
> thing, the skb did not have an extraction frame header in the first
> place, but a VLAN tag with no timestamp information. So the code paths
> in felix_rxtstamp for the regular and 8021q tagger are completely
> independent. With tag_8021q, the timestamp must come from the packet's
> duplicate delivered to the CPU port module, but there is potentially
> complex logic to be handled [ and prone to reordering ] if we were to
> just start reading packets from the CPU port module, and try to match
> them to the one we received over Ethernet and which needs an RX
> timestamp. So we do something simple: we tell DSA "give me some time to
> think" (we request skb deferral by returning false from .port_rxtstamp)
> and we just drop the frame we got over Ethernet with no attempt to match
> it to anything - we just treat it as a notification that there's data to
> be processed from the CPU port module's queues. Then we proceed to read
> the packets from those, one by one, which we deliver up the stack,
> timestamped, using netif_rx - the same function that any driver would
> use anyway if it needed RX timestamp deferral. So the assumption is that
> we'll come across the PTP packet that triggered the CPU extraction
> notification eventually, but we don't know when exactly. Thanks to the
> VCAP IS2 trap/redirect rule and the exclusion of the CPU port module
> from the flooding replicators, only PTP frames should be present in the
> CPU port module's RX queues anyway.
> 
> There is just one conflict between the VCAP IS2 trapping rule and the
> semantics of the BPF classifier. Namely, ptp_classify_raw() deems
> general messages as non-timestampable, but still, those are trapped to
> the CPU port module since they have an EtherType of ETH_P_1588. So, if
> the "no XTR IRQ" workaround is in place, we need to run another BPF
> classifier on the frames extracted over MMIO, to avoid duplicates being
> sent to the stack (once over Ethernet, once over MMIO). It doesn't look
> like it's possible to install VCAP IS2 rules based on keys extracted
> from the 1588 frame headers.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

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

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

* Re: [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q
  2021-02-13 22:37 [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q Vladimir Oltean
                   ` (11 preceding siblings ...)
  2021-02-13 22:38 ` [PATCH v2 net-next 12/12] net: dsa: tag_ocelot_8021q: add support for PTP timestamping Vladimir Oltean
@ 2021-02-15  1:40 ` patchwork-bot+netdevbpf
  12 siblings, 0 replies; 25+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-02-15  1:40 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: davem, kuba, netdev, andrew, f.fainelli, vivien.didelot,
	richardcochran, claudiu.manoil, alexandre.belloni,
	vladimir.oltean, fido_max, UNGLinuxDriver

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Sun, 14 Feb 2021 00:37:49 +0200 you wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Changes in v2:
> Add stub definition for ocelot_port_inject_frame when switch driver is
> not compiled in.
> 
> This is part two of the errata workaround begun here:
> https://patchwork.kernel.org/project/netdevbpf/cover/20210129010009.3959398-1-olteanv@gmail.com/
> 
> [...]

Here is the summary with links:
  - [v2,net-next,01/12] net: mscc: ocelot: stop returning IRQ_NONE in ocelot_xtr_irq_handler
    https://git.kernel.org/netdev/net-next/c/f833ca293dd1
  - [v2,net-next,02/12] net: mscc: ocelot: only drain extraction queue on error
    https://git.kernel.org/netdev/net-next/c/d7795f8f26d9
  - [v2,net-next,03/12] net: mscc: ocelot: better error handling in ocelot_xtr_irq_handler
    https://git.kernel.org/netdev/net-next/c/a94306cea56f
  - [v2,net-next,04/12] net: mscc: ocelot: use DIV_ROUND_UP helper in ocelot_port_inject_frame
    https://git.kernel.org/netdev/net-next/c/5f016f42d342
  - [v2,net-next,05/12] net: mscc: ocelot: refactor ocelot_port_inject_frame out of ocelot_port_xmit
    https://git.kernel.org/netdev/net-next/c/137ffbc4bb86
  - [v2,net-next,06/12] net: dsa: tag_ocelot: avoid accessing ds->priv in ocelot_rcv
    https://git.kernel.org/netdev/net-next/c/8a678bb29bd2
  - [v2,net-next,07/12] net: mscc: ocelot: use common tag parsing code with DSA
    https://git.kernel.org/netdev/net-next/c/40d3f295b5fe
  - [v2,net-next,08/12] net: dsa: tag_ocelot: single out PTP-related transmit tag processing
    https://git.kernel.org/netdev/net-next/c/62bf5fde5e14
  - [v2,net-next,09/12] net: dsa: tag_ocelot: create separate tagger for Seville
    https://git.kernel.org/netdev/net-next/c/7c4bb540e917
  - [v2,net-next,10/12] net: mscc: ocelot: refactor ocelot_xtr_irq_handler into ocelot_xtr_poll
    https://git.kernel.org/netdev/net-next/c/924ee317f724
  - [v2,net-next,11/12] net: dsa: felix: setup MMIO filtering rules for PTP when using tag_8021q
    https://git.kernel.org/netdev/net-next/c/c8c0ba4fe247
  - [v2,net-next,12/12] net: dsa: tag_ocelot_8021q: add support for PTP timestamping
    https://git.kernel.org/netdev/net-next/c/0a6f17c6ae21

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



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

end of thread, other threads:[~2021-02-15  1:41 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-13 22:37 [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q Vladimir Oltean
2021-02-13 22:37 ` [PATCH v2 net-next 01/12] net: mscc: ocelot: stop returning IRQ_NONE in ocelot_xtr_irq_handler Vladimir Oltean
2021-02-13 22:37 ` [PATCH v2 net-next 02/12] net: mscc: ocelot: only drain extraction queue on error Vladimir Oltean
2021-02-14  1:19   ` Florian Fainelli
2021-02-13 22:37 ` [PATCH v2 net-next 03/12] net: mscc: ocelot: better error handling in ocelot_xtr_irq_handler Vladimir Oltean
2021-02-14  1:20   ` Florian Fainelli
2021-02-13 22:37 ` [PATCH v2 net-next 04/12] net: mscc: ocelot: use DIV_ROUND_UP helper in ocelot_port_inject_frame Vladimir Oltean
2021-02-14  1:20   ` Florian Fainelli
2021-02-13 22:37 ` [PATCH v2 net-next 05/12] net: mscc: ocelot: refactor ocelot_port_inject_frame out of ocelot_port_xmit Vladimir Oltean
2021-02-14  1:22   ` Florian Fainelli
2021-02-13 22:37 ` [PATCH v2 net-next 06/12] net: dsa: tag_ocelot: avoid accessing ds->priv in ocelot_rcv Vladimir Oltean
2021-02-14  1:23   ` Florian Fainelli
2021-02-13 22:37 ` [PATCH v2 net-next 07/12] net: mscc: ocelot: use common tag parsing code with DSA Vladimir Oltean
2021-02-14  1:25   ` Florian Fainelli
2021-02-13 22:37 ` [PATCH v2 net-next 08/12] net: dsa: tag_ocelot: single out PTP-related transmit tag processing Vladimir Oltean
2021-02-14  1:26   ` Florian Fainelli
2021-02-13 22:37 ` [PATCH v2 net-next 09/12] net: dsa: tag_ocelot: create separate tagger for Seville Vladimir Oltean
2021-02-14  1:29   ` Florian Fainelli
2021-02-13 22:37 ` [PATCH v2 net-next 10/12] net: mscc: ocelot: refactor ocelot_xtr_irq_handler into ocelot_xtr_poll Vladimir Oltean
2021-02-14  2:57   ` Florian Fainelli
2021-02-13 22:38 ` [PATCH v2 net-next 11/12] net: dsa: felix: setup MMIO filtering rules for PTP when using tag_8021q Vladimir Oltean
2021-02-14  3:01   ` Florian Fainelli
2021-02-13 22:38 ` [PATCH v2 net-next 12/12] net: dsa: tag_ocelot_8021q: add support for PTP timestamping Vladimir Oltean
2021-02-14  3:06   ` Florian Fainelli
2021-02-15  1:40 ` [PATCH v2 net-next 00/12] PTP for DSA tag_ocelot_8021q patchwork-bot+netdevbpf

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