[PATCH v3 net-next 0/2] Allow unknown unicast traffic to CPU for Felix DSA

[PATCH v3 net-next 0/2] Allow unknown unicast traffic to CPU for Felix DSA

[Vladimir Oltean]

This is the continuation of the previous "[PATCH net-next] net: mscc:
ocelot: Workaround to allow traffic to CPU in standalone mode":

https://www.spinics.net/lists/netdev/msg631067.html

Following the feedback received from Allan Nielsen, the Ocelot and Felix
drivers were made to use the CPU port module in the same way (patch 1),
and Felix was made to additionally allow unknown unicast frames towards
the CPU port module (patch 2).

Vladimir Oltean (2):
  net: mscc: ocelot: eliminate confusion between CPU and NPI port
  net: dsa: felix: Allow unknown unicast traffic towards the CPU port
    module

 drivers/net/dsa/ocelot/felix.c           | 16 +++++-
 drivers/net/ethernet/mscc/ocelot.c       | 62 +++++++++++---------
 drivers/net/ethernet/mscc/ocelot.h       | 10 ----
 drivers/net/ethernet/mscc/ocelot_board.c |  7 +--
 include/soc/mscc/ocelot.h                | 72 ++++++++++++++++++++++--
 net/dsa/tag_ocelot.c                     |  3 +-
 6 files changed, 121 insertions(+), 49 deletions(-)

-- 
2.17.1

[PATCH v3 net-next 1/2] net: mscc: ocelot: eliminate confusion between CPU and NPI port

[Vladimir Oltean]

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

Ocelot has the concept of a CPU port. The CPU port is represented in the
forwarding and the queueing system, but it is not a physical device. The
CPU port can either be accessed via register-based injection/extraction
(which is the case of Ocelot), via Frame-DMA (similar to the first one),
or "connected" to a physical Ethernet port (called NPI in the datasheet)
which is the case of the Felix DSA switch.

In Ocelot the CPU port is at index 11.
In Felix the CPU port is at index 6.

The CPU bit is treated special in the forwarding, as it is never cleared
from the forwarding port mask (once added to it). Other than that, it is
treated the same as a normal front port.

Both Felix and Ocelot should use the CPU port in the same way. This
means that Felix should not use the NPI port directly when forwarding to
the CPU, but instead use the CPU port.

This patch is fixing this such that Felix will use port 6 as its CPU
port, and just use the NPI port to carry the traffic.

Therefore, eliminate the "ocelot->cpu" variable which was holding the
index of the NPI port for Felix, and the index of the CPU port module
for Ocelot, so the variable was actually configuring different things
for different drivers and causing at least part of the confusion.

Also remove the "ocelot->num_cpu_ports" variable, which is the result of
another confusion. The 2 CPU ports mentioned in the datasheet are
because there are two frame extraction channels (register based or DMA
based). This is of no relevance to the driver at the moment, and
invisible to the analyzer module.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Suggested-by: Allan W. Nielsen <allan.nielsen@microchip.com>
---
Changes in v3:
Replaced commit message verbiage with text from Allan Nielsen.

 drivers/net/dsa/ocelot/felix.c           |  7 +--
 drivers/net/ethernet/mscc/ocelot.c       | 62 ++++++++++++++----------
 drivers/net/ethernet/mscc/ocelot_board.c |  7 ++-
 include/soc/mscc/ocelot.h                | 12 +++--
 net/dsa/tag_ocelot.c                     |  3 +-
 5 files changed, 52 insertions(+), 39 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 7e66821b05b4..65b753b78221 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -512,10 +512,11 @@ static int felix_setup(struct dsa_switch *ds)
 	for (port = 0; port < ds->num_ports; port++) {
 		ocelot_init_port(ocelot, port);
 
+		/* Bring up the CPU port module and configure the NPI port */
 		if (dsa_is_cpu_port(ds, port))
-			ocelot_set_cpu_port(ocelot, port,
-					    OCELOT_TAG_PREFIX_NONE,
-					    OCELOT_TAG_PREFIX_LONG);
+			ocelot_configure_cpu(ocelot, port,
+					     OCELOT_TAG_PREFIX_NONE,
+					     OCELOT_TAG_PREFIX_LONG);
 	}
 
 	/* It looks like the MAC/PCS interrupt register - PM0_IEVENT (0x8040)
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 86d543ab1ab9..341092f9097c 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1398,7 +1398,7 @@ void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state)
 	 * a source for the other ports.
 	 */
 	for (p = 0; p < ocelot->num_phys_ports; p++) {
-		if (p == ocelot->cpu || (ocelot->bridge_fwd_mask & BIT(p))) {
+		if (ocelot->bridge_fwd_mask & BIT(p)) {
 			unsigned long mask = ocelot->bridge_fwd_mask & ~BIT(p);
 
 			for (i = 0; i < ocelot->num_phys_ports; i++) {
@@ -1413,18 +1413,10 @@ void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state)
 				}
 			}
 
-			/* Avoid the NPI port from looping back to itself */
-			if (p != ocelot->cpu)
-				mask |= BIT(ocelot->cpu);
-
 			ocelot_write_rix(ocelot, mask,
 					 ANA_PGID_PGID, PGID_SRC + p);
 		} else {
-			/* Only the CPU port, this is compatible with link
-			 * aggregation.
-			 */
-			ocelot_write_rix(ocelot,
-					 BIT(ocelot->cpu),
+			ocelot_write_rix(ocelot, 0,
 					 ANA_PGID_PGID, PGID_SRC + p);
 		}
 	}
@@ -2293,27 +2285,34 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 port,
 }
 EXPORT_SYMBOL(ocelot_probe_port);
 
-void ocelot_set_cpu_port(struct ocelot *ocelot, int cpu,
-			 enum ocelot_tag_prefix injection,
-			 enum ocelot_tag_prefix extraction)
+/* Configure and enable the CPU port module, which is a set of queues.
+ * If @npi contains a valid port index, the CPU port module is connected
+ * to the Node Processor Interface (NPI). This is the mode through which
+ * frames can be injected from and extracted to an external CPU,
+ * over Ethernet.
+ */
+void ocelot_configure_cpu(struct ocelot *ocelot, int npi,
+			  enum ocelot_tag_prefix injection,
+			  enum ocelot_tag_prefix extraction)
 {
-	/* Configure and enable the CPU port. */
+	int cpu = ocelot->num_phys_ports;
+
+	/* The unicast destination PGID for the CPU port module is unused */
 	ocelot_write_rix(ocelot, 0, ANA_PGID_PGID, cpu);
+	/* Instead set up a multicast destination PGID for traffic copied to
+	 * the CPU. Whitelisted MAC addresses like the port netdevice MAC
+	 * addresses will be copied to the CPU via this PGID.
+	 */
 	ocelot_write_rix(ocelot, BIT(cpu), ANA_PGID_PGID, PGID_CPU);
 	ocelot_write_gix(ocelot, ANA_PORT_PORT_CFG_RECV_ENA |
 			 ANA_PORT_PORT_CFG_PORTID_VAL(cpu),
 			 ANA_PORT_PORT_CFG, cpu);
 
-	/* If the CPU port is a physical port, set up the port in Node
-	 * Processor Interface (NPI) mode. This is the mode through which
-	 * frames can be injected from and extracted to an external CPU.
-	 * Only one port can be an NPI at the same time.
-	 */
-	if (cpu < ocelot->num_phys_ports) {
+	if (npi >= 0 && npi < ocelot->num_phys_ports) {
 		int mtu = VLAN_ETH_FRAME_LEN + OCELOT_TAG_LEN;
 
 		ocelot_write(ocelot, QSYS_EXT_CPU_CFG_EXT_CPUQ_MSK_M |
-			     QSYS_EXT_CPU_CFG_EXT_CPU_PORT(cpu),
+			     QSYS_EXT_CPU_CFG_EXT_CPU_PORT(npi),
 			     QSYS_EXT_CPU_CFG);
 
 		if (injection == OCELOT_TAG_PREFIX_SHORT)
@@ -2321,14 +2320,27 @@ void ocelot_set_cpu_port(struct ocelot *ocelot, int cpu,
 		else if (injection == OCELOT_TAG_PREFIX_LONG)
 			mtu += OCELOT_LONG_PREFIX_LEN;
 
-		ocelot_port_set_mtu(ocelot, cpu, mtu);
+		ocelot_port_set_mtu(ocelot, npi, mtu);
+
+		/* Enable NPI port */
+		ocelot_write_rix(ocelot,
+				 QSYS_SWITCH_PORT_MODE_INGRESS_DROP_MODE |
+				 QSYS_SWITCH_PORT_MODE_SCH_NEXT_CFG(1) |
+				 QSYS_SWITCH_PORT_MODE_PORT_ENA,
+				 QSYS_SWITCH_PORT_MODE, npi);
+		/* NPI port Injection/Extraction configuration */
+		ocelot_write_rix(ocelot,
+				 SYS_PORT_MODE_INCL_XTR_HDR(extraction) |
+				 SYS_PORT_MODE_INCL_INJ_HDR(injection),
+				 SYS_PORT_MODE, npi);
 	}
 
-	/* CPU port Injection/Extraction configuration */
+	/* Enable CPU port module */
 	ocelot_write_rix(ocelot, QSYS_SWITCH_PORT_MODE_INGRESS_DROP_MODE |
 			 QSYS_SWITCH_PORT_MODE_SCH_NEXT_CFG(1) |
 			 QSYS_SWITCH_PORT_MODE_PORT_ENA,
 			 QSYS_SWITCH_PORT_MODE, cpu);
+	/* CPU port Injection/Extraction configuration */
 	ocelot_write_rix(ocelot, SYS_PORT_MODE_INCL_XTR_HDR(extraction) |
 			 SYS_PORT_MODE_INCL_INJ_HDR(injection),
 			 SYS_PORT_MODE, cpu);
@@ -2338,10 +2350,8 @@ void ocelot_set_cpu_port(struct ocelot *ocelot, int cpu,
 				 ANA_PORT_VLAN_CFG_VLAN_AWARE_ENA |
 				 ANA_PORT_VLAN_CFG_VLAN_POP_CNT(1),
 			 ANA_PORT_VLAN_CFG, cpu);
-
-	ocelot->cpu = cpu;
 }
-EXPORT_SYMBOL(ocelot_set_cpu_port);
+EXPORT_SYMBOL(ocelot_configure_cpu);
 
 int ocelot_init(struct ocelot *ocelot)
 {
diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
index 1135a18019c7..87ff775897da 100644
--- a/drivers/net/ethernet/mscc/ocelot_board.c
+++ b/drivers/net/ethernet/mscc/ocelot_board.c
@@ -349,8 +349,6 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 		ocelot->ptp = 1;
 	}
 
-	ocelot->num_cpu_ports = 1; /* 1 port on the switch, two groups */
-
 	ports = of_get_child_by_name(np, "ethernet-ports");
 	if (!ports) {
 		dev_err(&pdev->dev, "no ethernet-ports child node found\n");
@@ -363,8 +361,9 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 				     sizeof(struct ocelot_port *), GFP_KERNEL);
 
 	ocelot_init(ocelot);
-	ocelot_set_cpu_port(ocelot, ocelot->num_phys_ports,
-			    OCELOT_TAG_PREFIX_NONE, OCELOT_TAG_PREFIX_NONE);
+	/* No NPI port */
+	ocelot_configure_cpu(ocelot, -1, OCELOT_TAG_PREFIX_NONE,
+			     OCELOT_TAG_PREFIX_NONE);
 
 	for_each_available_child_of_node(ports, portnp) {
 		struct ocelot_port_private *priv;
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 068f96b1a83e..247b537fc7ef 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -447,9 +447,11 @@ struct ocelot {
 	/* Keep track of the vlan port masks */
 	u32				vlan_mask[VLAN_N_VID];
 
+	/* In tables like ANA:PORT and the ANA:PGID:PGID mask,
+	 * the CPU is located after the physical ports (at the
+	 * num_phys_ports index).
+	 */
 	u8				num_phys_ports;
-	u8				num_cpu_ports;
-	u8				cpu;
 
 	u32				*lags;
 
@@ -500,9 +502,9 @@ void __ocelot_rmw_ix(struct ocelot *ocelot, u32 val, u32 mask, u32 reg,
 int ocelot_regfields_init(struct ocelot *ocelot,
 			  const struct reg_field *const regfields);
 struct regmap *ocelot_regmap_init(struct ocelot *ocelot, struct resource *res);
-void ocelot_set_cpu_port(struct ocelot *ocelot, int cpu,
-			 enum ocelot_tag_prefix injection,
-			 enum ocelot_tag_prefix extraction);
+void ocelot_configure_cpu(struct ocelot *ocelot, int npi,
+			  enum ocelot_tag_prefix injection,
+			  enum ocelot_tag_prefix extraction);
 int ocelot_init(struct ocelot *ocelot);
 void ocelot_deinit(struct ocelot *ocelot);
 void ocelot_init_port(struct ocelot *ocelot, int port);
diff --git a/net/dsa/tag_ocelot.c b/net/dsa/tag_ocelot.c
index 8e3e7283d430..59de1315100f 100644
--- a/net/dsa/tag_ocelot.c
+++ b/net/dsa/tag_ocelot.c
@@ -153,7 +153,8 @@ static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
 
 	memset(injection, 0, OCELOT_TAG_LEN);
 
-	src = dsa_upstream_port(ds, port);
+	/* Set the source port as the CPU port module and not the NPI port */
+	src = ocelot->num_phys_ports;
 	dest = BIT(port);
 	bypass = true;
 	qos_class = skb->priority;
-- 
2.17.1

[PATCH v3 net-next 2/2] net: dsa: felix: Allow unknown unicast traffic towards the CPU port module

[Vladimir Oltean]

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

Compared to other DSA switches, in the Ocelot cores, the RX filtering is
a much more important concern.

Firstly, the primary use case for Ocelot is non-DSA, so there isn't any
secondary Ethernet MAC [the DSA master's one] to implicitly drop frames
having a DMAC we are not interested in.  So the switch driver itself
needs to install FDB entries towards the CPU port module (PGID_CPU) for
the MAC address of each switch port, in each VLAN installed on the port.
Every address that is not whitelisted is implicitly dropped. This is in
order to achieve a behavior similar to N standalone net devices.

Secondly, even in the secondary use case of DSA, such as illustrated by
Felix with the NPI port mode, that secondary Ethernet MAC is present,
but its RX filter is bypassed. This is because the DSA tags themselves
are placed before Ethernet, so the DMAC that the switch ports see is
not seen by the DSA master too (since it's shifter to the right).

So RX filtering is pretty important. A good RX filter won't bother the
CPU in case the switch port receives a frame that it's not interested
in, and there exists no other line of defense.

Ocelot is pretty strict when it comes to RX filtering: non-IP multicast
and broadcast traffic is allowed to go to the CPU port module, but
unknown unicast isn't. This means that traffic reception for any other
MAC addresses than the ones configured on each switch port net device
won't work. This includes use cases such as macvlan or bridging with a
non-Ocelot (so-called "foreign") interface. But this seems to be fine
for the scenarios that the Linux system embedded inside an Ocelot switch
is intended for - it is simply not interested in unknown unicast
traffic, as explained in Allan Nielsen's presentation [0].

On the other hand, the Felix DSA switch is integrated in more
general-purpose Linux systems, so it can't afford to drop that sort of
traffic in hardware, even if it will end up doing so later, in software.

Actually, unknown unicast means more for Felix than it does for Ocelot.
Felix doesn't attempt to perform the whitelisting of switch port MAC
addresses towards PGID_CPU at all, mainly because it is too complicated
to be feasible: while the MAC addresses are unique in Ocelot, by default
in DSA all ports are equal and inherited from the DSA master. This adds
into account the question of reference counting MAC addresses (delayed
ocelot_mact_forget), not to mention reference counting for the VLAN IDs
that those MAC addresses are installed in. This reference counting
should be done in the DSA core, and the fact that it wasn't needed so
far is due to the fact that the other DSA switches don't have the DSA
tag placed before Ethernet, so the DSA master is able to whitelist the
MAC addresses in hardware.

So this means that even regular traffic termination on a Felix switch
port happens through flooding (because neither Felix nor Ocelot learn
source MAC addresses from CPU-injected frames).

So far we've explained that whitelisting towards PGID_CPU:
- helps to reduce the likelihood of spamming the CPU with frames it
  won't process very far anyway
- is implemented in the ocelot driver
- is sufficient for the ocelot use cases
- is not feasible in DSA
- breaks use cases in DSA, in the current status (whitelisting enabled
  but no MAC address whitelisted)

So the proposed patch allows unknown unicast frames to be sent to the
CPU port module. This is done for the Felix DSA driver only, as Ocelot
seems to be happy without it.

[0]: https://www.youtube.com/watch?v=B1HhxEcU7Jg

Suggested-by: Allan W. Nielsen <allan.nielsen@microchip.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
None.

 drivers/net/dsa/ocelot/felix.c     |  9 +++++
 drivers/net/ethernet/mscc/ocelot.h | 10 -----
 include/soc/mscc/ocelot.h          | 60 ++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 65b753b78221..f14595b8dad5 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -519,6 +519,15 @@ static int felix_setup(struct dsa_switch *ds)
 					     OCELOT_TAG_PREFIX_LONG);
 	}
 
+	/* Include the CPU port module in the forwarding mask for unknown
+	 * unicast - the hardware default value for ANA_FLOODING_FLD_UNICAST
+	 * excludes BIT(ocelot->num_phys_ports), and so does ocelot_init, since
+	 * Ocelot relies on whitelisting MAC addresses towards PGID_CPU.
+	 */
+	ocelot_write_rix(ocelot,
+			 ANA_PGID_PGID_PGID(GENMASK(ocelot->num_phys_ports, 0)),
+			 ANA_PGID_PGID, PGID_UC);
+
 	/* It looks like the MAC/PCS interrupt register - PM0_IEVENT (0x8040)
 	 * isn't instantiated for the Felix PF.
 	 * In-band AN may take a few ms to complete, so we need to poll.
diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h
index 04372ba72fec..e34ef8380eb3 100644
--- a/drivers/net/ethernet/mscc/ocelot.h
+++ b/drivers/net/ethernet/mscc/ocelot.h
@@ -28,16 +28,6 @@
 #include "ocelot_tc.h"
 #include "ocelot_ptp.h"
 
-#define PGID_AGGR    64
-#define PGID_SRC     80
-
-/* Reserved PGIDs */
-#define PGID_CPU     (PGID_AGGR - 5)
-#define PGID_UC      (PGID_AGGR - 4)
-#define PGID_MC      (PGID_AGGR - 3)
-#define PGID_MCIPV4  (PGID_AGGR - 2)
-#define PGID_MCIPV6  (PGID_AGGR - 1)
-
 #define OCELOT_BUFFER_CELL_SZ 60
 
 #define OCELOT_STATS_CHECK_DELAY (2 * HZ)
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 247b537fc7ef..a4c72ae0b60e 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -11,6 +11,66 @@
 #include <linux/regmap.h>
 #include <net/dsa.h>
 
+/* Port Group IDs (PGID) are masks of destination ports.
+ *
+ * For L2 forwarding, the switch performs 3 lookups in the PGID table for each
+ * frame, and forwards the frame to the ports that are present in the logical
+ * AND of all 3 PGIDs.
+ *
+ * These PGID lookups are:
+ * - In one of PGID[0-63]: for the destination masks. There are 2 paths by
+ *   which the switch selects a destination PGID:
+ *     - The {DMAC, VID} is present in the MAC table. In that case, the
+ *       destination PGID is given by the DEST_IDX field of the MAC table entry
+ *       that matched.
+ *     - The {DMAC, VID} is not present in the MAC table (it is unknown). The
+ *       frame is disseminated as being either unicast, multicast or broadcast,
+ *       and according to that, the destination PGID is chosen as being the
+ *       value contained by ANA_FLOODING_FLD_UNICAST,
+ *       ANA_FLOODING_FLD_MULTICAST or ANA_FLOODING_FLD_BROADCAST.
+ *   The destination PGID can be an unicast set: the first PGIDs, 0 to
+ *   ocelot->num_phys_ports - 1, or a multicast set: the PGIDs from
+ *   ocelot->num_phys_ports to 63. By convention, a unicast PGID corresponds to
+ *   a physical port and has a single bit set in the destination ports mask:
+ *   that corresponding to the port number itself. In contrast, a multicast
+ *   PGID will have potentially more than one single bit set in the destination
+ *   ports mask.
+ * - In one of PGID[64-79]: for the aggregation mask. The switch classifier
+ *   dissects each frame and generates a 4-bit Link Aggregation Code which is
+ *   used for this second PGID table lookup. The goal of link aggregation is to
+ *   hash multiple flows within the same LAG on to different destination ports.
+ *   The first lookup will result in a PGID with all the LAG members present in
+ *   the destination ports mask, and the second lookup, by Link Aggregation
+ *   Code, will ensure that each flow gets forwarded only to a single port out
+ *   of that mask (there are no duplicates).
+ * - In one of PGID[80-90]: for the source mask. The third time, the PGID table
+ *   is indexed with the ingress port (plus 80). These PGIDs answer the
+ *   question "is port i allowed to forward traffic to port j?" If yes, then
+ *   BIT(j) of PGID 80+i will be found set. The third PGID lookup can be used
+ *   to enforce the L2 forwarding matrix imposed by e.g. a Linux bridge.
+ */
+
+/* Reserve some destination PGIDs at the end of the range:
+ * PGID_CPU: used for whitelisting certain MAC addresses, such as the addresses
+ *           of the switch port net devices, towards the CPU port module.
+ * PGID_UC: the flooding destinations for unknown unicast traffic.
+ * PGID_MC: the flooding destinations for broadcast and non-IP multicast
+ *          traffic.
+ * PGID_MCIPV4: the flooding destinations for IPv4 multicast traffic.
+ * PGID_MCIPV6: the flooding destinations for IPv6 multicast traffic.
+ */
+#define PGID_CPU			59
+#define PGID_UC				60
+#define PGID_MC				61
+#define PGID_MCIPV4			62
+#define PGID_MCIPV6			63
+
+/* Aggregation PGIDs, one per Link Aggregation Code */
+#define PGID_AGGR			64
+
+/* Source PGIDs, one per physical port */
+#define PGID_SRC			80
+
 #define IFH_INJ_BYPASS			BIT(31)
 #define IFH_INJ_POP_CNT_DISABLE		(3 << 28)
 
-- 
2.17.1

Re: [PATCH v3 net-next 2/2] net: dsa: felix: Allow unknown unicast traffic towards the CPU port module

[Vladimir Oltean]

On Sat, 29 Feb 2020 at 16:50, Vladimir Oltean <olteanv@gmail.com> wrote:
>
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> Compared to other DSA switches, in the Ocelot cores, the RX filtering is
> a much more important concern.
>
> Firstly, the primary use case for Ocelot is non-DSA, so there isn't any
> secondary Ethernet MAC [the DSA master's one] to implicitly drop frames
> having a DMAC we are not interested in.  So the switch driver itself
> needs to install FDB entries towards the CPU port module (PGID_CPU) for
> the MAC address of each switch port, in each VLAN installed on the port.
> Every address that is not whitelisted is implicitly dropped. This is in
> order to achieve a behavior similar to N standalone net devices.
>
> Secondly, even in the secondary use case of DSA, such as illustrated by
> Felix with the NPI port mode, that secondary Ethernet MAC is present,
> but its RX filter is bypassed. This is because the DSA tags themselves
> are placed before Ethernet, so the DMAC that the switch ports see is
> not seen by the DSA master too (since it's shifter to the right).
>
> So RX filtering is pretty important. A good RX filter won't bother the
> CPU in case the switch port receives a frame that it's not interested
> in, and there exists no other line of defense.
>
> Ocelot is pretty strict when it comes to RX filtering: non-IP multicast
> and broadcast traffic is allowed to go to the CPU port module, but
> unknown unicast isn't. This means that traffic reception for any other
> MAC addresses than the ones configured on each switch port net device
> won't work. This includes use cases such as macvlan or bridging with a
> non-Ocelot (so-called "foreign") interface. But this seems to be fine
> for the scenarios that the Linux system embedded inside an Ocelot switch
> is intended for - it is simply not interested in unknown unicast
> traffic, as explained in Allan Nielsen's presentation [0].
>
> On the other hand, the Felix DSA switch is integrated in more
> general-purpose Linux systems, so it can't afford to drop that sort of
> traffic in hardware, even if it will end up doing so later, in software.
>
> Actually, unknown unicast means more for Felix than it does for Ocelot.
> Felix doesn't attempt to perform the whitelisting of switch port MAC
> addresses towards PGID_CPU at all, mainly because it is too complicated
> to be feasible: while the MAC addresses are unique in Ocelot, by default
> in DSA all ports are equal and inherited from the DSA master. This adds
> into account the question of reference counting MAC addresses (delayed
> ocelot_mact_forget), not to mention reference counting for the VLAN IDs
> that those MAC addresses are installed in. This reference counting
> should be done in the DSA core, and the fact that it wasn't needed so
> far is due to the fact that the other DSA switches don't have the DSA
> tag placed before Ethernet, so the DSA master is able to whitelist the
> MAC addresses in hardware.
>
> So this means that even regular traffic termination on a Felix switch
> port happens through flooding (because neither Felix nor Ocelot learn
> source MAC addresses from CPU-injected frames).
>
> So far we've explained that whitelisting towards PGID_CPU:
> - helps to reduce the likelihood of spamming the CPU with frames it
>   won't process very far anyway
> - is implemented in the ocelot driver
> - is sufficient for the ocelot use cases
> - is not feasible in DSA
> - breaks use cases in DSA, in the current status (whitelisting enabled
>   but no MAC address whitelisted)
>
> So the proposed patch allows unknown unicast frames to be sent to the
> CPU port module. This is done for the Felix DSA driver only, as Ocelot
> seems to be happy without it.
>
> [0]: https://www.youtube.com/watch?v=B1HhxEcU7Jg
>
> Suggested-by: Allan W. Nielsen <allan.nielsen@microchip.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

I see this patch has "Needs Review / ACK" in patchwork.

There is in fact a tag:

Reviewed-by: Allan W. Nielsen <allan.nielsen@microchip.com>

which I had forgotten to copy over from v2:
https://www.spinics.net/lists/netdev/msg633098.html

Hope there are no particular issues with this approach from DSA perspective.

Thanks,
-Vladimir

Re: [PATCH v3 net-next 2/2] net: dsa: felix: Allow unknown unicast traffic towards the CPU port module

[Florian Fainelli]

On 3/3/20 8:04 AM, Vladimir Oltean wrote:
> On Sat, 29 Feb 2020 at 16:50, Vladimir Oltean <olteanv@gmail.com> wrote:
>>
>> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>>
>> Compared to other DSA switches, in the Ocelot cores, the RX filtering is
>> a much more important concern.
>>
>> Firstly, the primary use case for Ocelot is non-DSA, so there isn't any
>> secondary Ethernet MAC [the DSA master's one] to implicitly drop frames
>> having a DMAC we are not interested in.  So the switch driver itself
>> needs to install FDB entries towards the CPU port module (PGID_CPU) for
>> the MAC address of each switch port, in each VLAN installed on the port.
>> Every address that is not whitelisted is implicitly dropped. This is in
>> order to achieve a behavior similar to N standalone net devices.
>>
>> Secondly, even in the secondary use case of DSA, such as illustrated by
>> Felix with the NPI port mode, that secondary Ethernet MAC is present,
>> but its RX filter is bypassed. This is because the DSA tags themselves
>> are placed before Ethernet, so the DMAC that the switch ports see is
>> not seen by the DSA master too (since it's shifter to the right).
>>
>> So RX filtering is pretty important. A good RX filter won't bother the
>> CPU in case the switch port receives a frame that it's not interested
>> in, and there exists no other line of defense.
>>
>> Ocelot is pretty strict when it comes to RX filtering: non-IP multicast
>> and broadcast traffic is allowed to go to the CPU port module, but
>> unknown unicast isn't. This means that traffic reception for any other
>> MAC addresses than the ones configured on each switch port net device
>> won't work. This includes use cases such as macvlan or bridging with a
>> non-Ocelot (so-called "foreign") interface. But this seems to be fine
>> for the scenarios that the Linux system embedded inside an Ocelot switch
>> is intended for - it is simply not interested in unknown unicast
>> traffic, as explained in Allan Nielsen's presentation [0].
>>
>> On the other hand, the Felix DSA switch is integrated in more
>> general-purpose Linux systems, so it can't afford to drop that sort of
>> traffic in hardware, even if it will end up doing so later, in software.
>>
>> Actually, unknown unicast means more for Felix than it does for Ocelot.
>> Felix doesn't attempt to perform the whitelisting of switch port MAC
>> addresses towards PGID_CPU at all, mainly because it is too complicated
>> to be feasible: while the MAC addresses are unique in Ocelot, by default
>> in DSA all ports are equal and inherited from the DSA master. This adds
>> into account the question of reference counting MAC addresses (delayed
>> ocelot_mact_forget), not to mention reference counting for the VLAN IDs
>> that those MAC addresses are installed in. This reference counting
>> should be done in the DSA core, and the fact that it wasn't needed so
>> far is due to the fact that the other DSA switches don't have the DSA
>> tag placed before Ethernet, so the DSA master is able to whitelist the
>> MAC addresses in hardware.
>>
>> So this means that even regular traffic termination on a Felix switch
>> port happens through flooding (because neither Felix nor Ocelot learn
>> source MAC addresses from CPU-injected frames).
>>
>> So far we've explained that whitelisting towards PGID_CPU:
>> - helps to reduce the likelihood of spamming the CPU with frames it
>>   won't process very far anyway
>> - is implemented in the ocelot driver
>> - is sufficient for the ocelot use cases
>> - is not feasible in DSA
>> - breaks use cases in DSA, in the current status (whitelisting enabled
>>   but no MAC address whitelisted)
>>
>> So the proposed patch allows unknown unicast frames to be sent to the
>> CPU port module. This is done for the Felix DSA driver only, as Ocelot
>> seems to be happy without it.
>>
>> [0]: https://www.youtube.com/watch?v=B1HhxEcU7Jg
>>
>> Suggested-by: Allan W. Nielsen <allan.nielsen@microchip.com>
>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>> ---
> 
> I see this patch has "Needs Review / ACK" in patchwork.
> 
> There is in fact a tag:
> 
> Reviewed-by: Allan W. Nielsen <allan.nielsen@microchip.com>
> 
> which I had forgotten to copy over from v2:
> https://www.spinics.net/lists/netdev/msg633098.html
> 
> Hope there are no particular issues with this approach from DSA perspective.

Not really, until/if we implement UC and MC filtering on standalone DSA
ports, this will be revisited, but for now, this is what is expected.
-- 
Florian

Re: [PATCH v3 net-next 0/2] Allow unknown unicast traffic to CPU for Felix DSA

[David Miller]

From: Vladimir Oltean <olteanv@gmail.com>
Date: Sat, 29 Feb 2020 16:50:01 +0200

> This is the continuation of the previous "[PATCH net-next] net: mscc:
> ocelot: Workaround to allow traffic to CPU in standalone mode":
> 
> https://www.spinics.net/lists/netdev/msg631067.html
> 
> Following the feedback received from Allan Nielsen, the Ocelot and Felix
> drivers were made to use the CPU port module in the same way (patch 1),
> and Felix was made to additionally allow unknown unicast frames towards
> the CPU port module (patch 2).

Series applied, thanks.