[PATCH net-next 0/9] Traffic termination for sja1105 ports under VLAN-aware bridge

[PATCH net-next 0/9] Traffic termination for sja1105 ports under VLAN-aware bridge

[Vladimir Oltean]

This set of patches updates the sja1105 DSA driver to be able to send
and receive network stack packets on behalf of a VLAN-aware upper bridge
interface.

The reasons why this has traditionally been a problem are explained in
the "Traffic support" section of Documentation/networking/dsa/sja1105.rst.
(the entire documentation will be revised in a separate patch series).

The limitations that have prevented us from doing this so far have now
been partially lifted by the bridge's ability to send a packet with
skb->offload_fwd_mark = true, which means that the accelerator is
allowed to look up its hardware FDB when sending a packet and deliver it
to those destination ports. Basically skb->dev is now just a conduit to
the switchdev driver's ndo_start_xmit(), and does not guarantee that the
packet will really be transmitted on that port (but it will be
transmitted where it should, nonetheless).

Apart from the ability to perform IP termination on VLAN-aware bridges
on top of sja1105 interfaces, we also gain the following features:
- VLAN-aware software bridging between sja1105 ports and "foreign"
  (non-DSA) interfaces
- software bridging between sja1105 bridge ports, and software LAG
  uppers of sja1105 ports (as long as the bridge is VLAN-aware)

The only things that don't work are:
1. to create an AF_PACKET socket on top of a sja1105 port that is under
   a VLAN-aware bridge. This is because the "imprecise RX" procedure
   selects an RX port for data plane* packets based on the assumption
   that the packet will land in the bridge's data path.  If ebtables
   rules are added to remove some packets from the bridge's data path,
   that assumption will be broken. Nonetheless, this is not a limitation
   that negatively impacts the known use cases with this switch.  If
   there was a way to impose user space restrictions against creating
   AF_PACKET sockets on this particular configuration, I could be
   interested in adding those restrictions, but I think there are other
   known broken configs already which are not checked by the kernel
   today (like for example that the bridge's rx_handler steals packets
   anyway from AF_PACKET sockets with exact-match ptype handlers, as
   opposed to ptype_all which are processed earlier; this is precisely
   the reason why ebtables rules are generally needed to avoid that).
2. to send traffic on behalf of an 8021q upper of a standalone interface,
   while other sja1105 ports are part of a VLAN-aware bridge. This is
   because sja1105 sets ds->vlan_filtering_is_global = true, so we
   cannot make the standalone port ignore the VLAN header from the
   packet on RX, so we cannot make tag_8021q enforce its own pvid for
   the packets belonging to that port's 8021q upper. So we cannot
   determine in the first place that packets come from that port, unless
   we iterate through all 8021q uppers of all ports, and enforce
   uniqueness of VLAN IDs. I am not sure if this is what I want / if it
   is worth it, so currently all 8021q uppers are denied, regardless of
   whether the switch has ports under a VLAN-aware bridge or not
   (otherwise it becomes complicated even to track the state).
   Nonetheless, the VID uniqueness of all 8021q uppers does raise
   another question: what to do with VID 0, which has no 8021q upper,
   but the 8021q module adds it to our RX filter with vlan_vid_add().
   I am honestly not sure what to do. The best I can do is enable a
   hardware bit in sja1105 which reclassifies VID 0 frames to the PVID,
   and they will be sent on the CPU port using either the tag_8021q pvid
   of standalone ports, or the bridge pvid of VLAN-aware ports. So at
   the very least, those packets are still 'kinda' processed as if they
   were untagged, but the VID 0 is lost, though. In my defence, Marvell
   appears to do the same thing with reclassifying VID 0 frames, see
   commit b8b79c414eca ("net: dsa: mv88e6xxx: Fix adding vlan 0").

*Control packets (currently hardcoded in sja1105 as link-local packets
for MAC DA ranges 01-80-c2-xx-xx-xx and 01-1b-19-xx-xx-xx) are received
based on packet traps and their precise source port is always known.

I have taken one patch from Colin because my work conflicts with his,
and integrating it all through the same series avoids that.

Cc: Roopa Prabhu <roopa@nvidia.com>
Cc: Nikolay Aleksandrov <nikolay@nvidia.com>
Cc: Ido Schimmel <idosch@nvidia.com>
Cc: Jiri Pirko <jiri@nvidia.com>
Cc: Colin Ian King <colin.king@canonical.com>

Colin Ian King (1):
  net: dsa: sja1105: remove redundant re-assignment of pointer table

Vladimir Oltean (8):
  net: bridge: update BROPT_VLAN_ENABLED before notifying switchdev in
    br_vlan_filter_toggle
  net: bridge: add a helper for retrieving port VLANs from the data path
  net: dsa: sja1105: delete vlan delta save/restore logic
  net: dsa: sja1105: deny 8021q uppers on ports
  net: dsa: sja1105: deny more than one VLAN-aware bridge
  net: dsa: sja1105: add support for imprecise RX
  net: dsa: sja1105: add bridge TX data plane offload based on tag_8021q
  Revert "net: dsa: Allow drivers to filter packets they can decode
    source port from"

 drivers/net/dsa/sja1105/sja1105.h      |  12 +-
 drivers/net/dsa/sja1105/sja1105_main.c | 440 +++++++++----------------
 include/linux/dsa/8021q.h              |  10 +
 include/linux/if_bridge.h              |   8 +
 include/net/dsa.h                      |  15 -
 net/bridge/br_vlan.c                   |  34 +-
 net/dsa/dsa_priv.h                     |  43 +++
 net/dsa/port.c                         |   1 -
 net/dsa/tag_8021q.c                    |  48 ++-
 net/dsa/tag_sja1105.c                  | 118 ++++---
 net/ethernet/eth.c                     |   6 +-
 11 files changed, 363 insertions(+), 372 deletions(-)

-- 
2.25.1

[PATCH net-next 1/9] net: bridge: update BROPT_VLAN_ENABLED before notifying switchdev in br_vlan_filter_toggle

[Vladimir Oltean]

SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING is notified by the bridge from
two places:
- nbp_vlan_init(), during bridge port creation
- br_vlan_filter_toggle(), during a netlink/sysfs/ioctl change requested
  by user space

If a switchdev driver uses br_vlan_enabled(br_dev) inside its handler
for the SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING attribute notifier,
different things will be seen depending on whether the bridge calls from
the first path or the second:
- in nbp_vlan_init(), br_vlan_enabled() reflects the current state of
  the bridge
- in br_vlan_filter_toggle(), br_vlan_enabled() reflects the past state
  of the bridge

This can lead in some cases to complications in driver implementation,
which can be avoided if these could reliably use br_vlan_enabled().

Nothing seems to depend on this behavior, and it seems overall more
straightforward for br_vlan_enabled() to return the proper value even
during the SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING notifier, so
temporarily enable the bridge option, then revert it if the switchdev
notifier failed.

Cc: Roopa Prabhu <roopa@nvidia.com>
Cc: Nikolay Aleksandrov <nikolay@nvidia.com>
Cc: Ido Schimmel <idosch@nvidia.com>
Cc: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/bridge/br_vlan.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 325600361487..805206f31795 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -840,11 +840,14 @@ int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val,
 	if (br_opt_get(br, BROPT_VLAN_ENABLED) == !!val)
 		return 0;
 
+	br_opt_toggle(br, BROPT_VLAN_ENABLED, !!val);
+
 	err = switchdev_port_attr_set(br->dev, &attr, extack);
-	if (err && err != -EOPNOTSUPP)
+	if (err && err != -EOPNOTSUPP) {
+		br_opt_toggle(br, BROPT_VLAN_ENABLED, !val);
 		return err;
+	}
 
-	br_opt_toggle(br, BROPT_VLAN_ENABLED, !!val);
 	br_manage_promisc(br);
 	recalculate_group_addr(br);
 	br_recalculate_fwd_mask(br);
-- 
2.25.1

[PATCH net-next 2/9] net: bridge: add a helper for retrieving port VLANs from the data path

[Vladimir Oltean]

Introduce a brother of br_vlan_get_info() which is protected by the RCU
mechanism, as opposed to br_vlan_get_info() which relies on taking the
write-side rtnl_mutex.

This is needed for drivers which need to find out whether a bridge port
has a VLAN configured or not. For example, certain DSA switches might
not offer complete source port identification to the CPU on RX, just the
VLAN in which the packet was received. Based on this VLAN, we cannot set
an accurate skb->dev ingress port, but at least we can configure one
that behaves the same as the correct one would (this is possible because
DSA sets skb->offload_fwd_mark = 1).

When we look at the bridge RX handler (br_handle_frame), we see that
what matters regarding skb->dev is the VLAN ID and the port STP state.
So we need to select an skb->dev that has the same bridge VLAN as the
packet we're receiving, and is in the LEARNING or FORWARDING STP state.
The latter is easy, but for the former, we should somehow keep a shadow
list of the bridge VLANs on each port, and a lookup table between VLAN
ID and the 'designated port for imprecise RX'. That is rather
complicated to keep in sync properly (the designated port per VLAN needs
to be updated on the addition and removal of a VLAN, as well as on the
join/leave events of the bridge on that port).

So, to avoid all that complexity, let's just iterate through our finite
number of ports and ask the bridge, for each packet: "do you have this
VLAN configured on this port?".

Cc: Roopa Prabhu <roopa@nvidia.com>
Cc: Nikolay Aleksandrov <nikolay@nvidia.com>
Cc: Ido Schimmel <idosch@nvidia.com>
Cc: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 include/linux/if_bridge.h |  8 ++++++++
 net/bridge/br_vlan.c      | 27 +++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index f0b4ffbd8582..b73b4ff749e1 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -111,6 +111,8 @@ int br_vlan_get_pvid_rcu(const struct net_device *dev, u16 *p_pvid);
 int br_vlan_get_proto(const struct net_device *dev, u16 *p_proto);
 int br_vlan_get_info(const struct net_device *dev, u16 vid,
 		     struct bridge_vlan_info *p_vinfo);
+int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
+			 struct bridge_vlan_info *p_vinfo);
 #else
 static inline bool br_vlan_enabled(const struct net_device *dev)
 {
@@ -137,6 +139,12 @@ static inline int br_vlan_get_info(const struct net_device *dev, u16 vid,
 {
 	return -EINVAL;
 }
+
+static inline int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
+				       struct bridge_vlan_info *p_vinfo)
+{
+	return -EINVAL;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_BRIDGE)
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 805206f31795..8cfd035bbaf9 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -1449,6 +1449,33 @@ int br_vlan_get_info(const struct net_device *dev, u16 vid,
 }
 EXPORT_SYMBOL_GPL(br_vlan_get_info);
 
+int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
+			 struct bridge_vlan_info *p_vinfo)
+{
+	struct net_bridge_vlan_group *vg;
+	struct net_bridge_vlan *v;
+	struct net_bridge_port *p;
+
+	p = br_port_get_check_rcu(dev);
+	if (p)
+		vg = nbp_vlan_group_rcu(p);
+	else if (netif_is_bridge_master(dev))
+		vg = br_vlan_group_rcu(netdev_priv(dev));
+	else
+		return -EINVAL;
+
+	v = br_vlan_find(vg, vid);
+	if (!v)
+		return -ENOENT;
+
+	p_vinfo->vid = vid;
+	p_vinfo->flags = v->flags;
+	if (vid == br_get_pvid(vg))
+		p_vinfo->flags |= BRIDGE_VLAN_INFO_PVID;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(br_vlan_get_info_rcu);
+
 static int br_vlan_is_bind_vlan_dev(const struct net_device *dev)
 {
 	return is_vlan_dev(dev) &&
-- 
2.25.1

[PATCH net-next 3/9] net: dsa: sja1105: remove redundant re-assignment of pointer table

[Vladimir Oltean]

From: Colin Ian King <colin.king@canonical.com>

The pointer table is being re-assigned with a value that is never
read. The assignment is redundant and can be removed.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 07bb65a36083..4f1331ff5053 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2163,8 +2163,6 @@ static int sja1105_build_vlan_table(struct sja1105_private *priv)
 	if (!new_vlan)
 		return -ENOMEM;
 
-	table = &priv->static_config.tables[BLK_IDX_VLAN_LOOKUP];
-
 	for (i = 0; i < VLAN_N_VID; i++)
 		new_vlan[i].vlanid = VLAN_N_VID;
 
-- 
2.25.1

[PATCH net-next 4/9] net: dsa: sja1105: delete vlan delta save/restore logic

[Vladimir Oltean]

With the best_effort_vlan_filtering mode now gone, the driver does not
have 3 operating modes anymore (VLAN-unaware, VLAN-aware and best effort),
but only 2.

The idea is that we will gain support for network stack I/O through a
VLAN-aware bridge, using the data plane offload framework (imprecise RX,
imprecise TX). So the VLAN-aware use case will be more functional.

But standalone ports that are part of the same switch when some other
ports are under a VLAN-aware bridge should work too. Termination on
those should work through the tag_8021q RX VLAN and TX VLAN.

This was not possible using the old logic, because:
- in VLAN-unaware mode, only the tag_8021q VLANs were committed to hw
- in VLAN-aware mode, only the bridge VLANs were committed to hw
- in best-effort VLAN mode, both the tag_8021q and bridge VLANs were
  committed to hw

The strategy for the new VLAN-aware mode is to allow the bridge and the
tag_8021q VLANs to coexist in the VLAN table at the same time.

[ yes, we need to make sure that the bridge cannot install a tag_8021q
  VLAN, but ]

This means that the save/restore logic introduced by commit ec5ae61076d0
("net: dsa: sja1105: save/restore VLANs using a delta commit method")
does not serve a purpose any longer. We can delete it and restore the
old code that simply adds a VLAN to the VLAN table and calls it a day.

Note that we keep the sja1105_commit_pvid() function from those days,
but adapt it slightly. Ports that are under a VLAN-aware bridge use the
bridge's pvid, ports that are standalone or under a VLAN-unaware bridge
use the tag_8021q pvid, for local termination or VLAN-unaware forwarding.

Now, when the vlan_filtering property is toggled for the bridge, the
pvid of the ports beneath it is the only thing that's changing, we no
longer delete some VLANs and restore others.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105.h      |  12 +-
 drivers/net/dsa/sja1105/sja1105_main.c | 402 +++++++------------------
 2 files changed, 114 insertions(+), 300 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h
index 068be8afd322..9cd7dbdd7db9 100644
--- a/drivers/net/dsa/sja1105/sja1105.h
+++ b/drivers/net/dsa/sja1105/sja1105.h
@@ -226,14 +226,6 @@ struct sja1105_flow_block {
 	int num_virtual_links;
 };
 
-struct sja1105_bridge_vlan {
-	struct list_head list;
-	int port;
-	u16 vid;
-	bool pvid;
-	bool untagged;
-};
-
 struct sja1105_private {
 	struct sja1105_static_config static_config;
 	bool rgmii_rx_delay[SJA1105_MAX_NUM_PORTS];
@@ -249,8 +241,8 @@ struct sja1105_private {
 	struct gpio_desc *reset_gpio;
 	struct spi_device *spidev;
 	struct dsa_switch *ds;
-	struct list_head dsa_8021q_vlans;
-	struct list_head bridge_vlans;
+	u16 bridge_pvid[SJA1105_MAX_NUM_PORTS];
+	u16 tag_8021q_pvid[SJA1105_MAX_NUM_PORTS];
 	struct sja1105_flow_block flow_block;
 	struct sja1105_port ports[SJA1105_MAX_NUM_PORTS];
 	/* Serializes transmission of management frames so that
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 4f1331ff5053..309e6a933df7 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -378,8 +378,6 @@ static int sja1105_init_static_vlan(struct sja1105_private *priv)
 	table->entry_count = 1;
 
 	for (port = 0; port < ds->num_ports; port++) {
-		struct sja1105_bridge_vlan *v;
-
 		if (dsa_is_unused_port(ds, port))
 			continue;
 
@@ -387,22 +385,10 @@ static int sja1105_init_static_vlan(struct sja1105_private *priv)
 		pvid.vlan_bc |= BIT(port);
 		pvid.tag_port &= ~BIT(port);
 
-		v = kzalloc(sizeof(*v), GFP_KERNEL);
-		if (!v)
-			return -ENOMEM;
-
-		v->port = port;
-		v->vid = SJA1105_DEFAULT_VLAN;
-		v->untagged = true;
-		if (dsa_is_cpu_port(ds, port))
-			v->pvid = true;
-		list_add(&v->list, &priv->dsa_8021q_vlans);
-
-		v = kmemdup(v, sizeof(*v), GFP_KERNEL);
-		if (!v)
-			return -ENOMEM;
-
-		list_add(&v->list, &priv->bridge_vlans);
+		if (dsa_is_cpu_port(ds, port)) {
+			priv->tag_8021q_pvid[port] = SJA1105_DEFAULT_VLAN;
+			priv->bridge_pvid[port] = SJA1105_DEFAULT_VLAN;
+		}
 	}
 
 	((struct sja1105_vlan_lookup_entry *)table->entries)[0] = pvid;
@@ -1990,12 +1976,29 @@ static int sja1105_pvid_apply(struct sja1105_private *priv, int port, u16 pvid)
 
 	mac = priv->static_config.tables[BLK_IDX_MAC_CONFIG].entries;
 
+	if (mac[port].vlanid == pvid)
+		return 0;
+
 	mac[port].vlanid = pvid;
 
 	return sja1105_dynamic_config_write(priv, BLK_IDX_MAC_CONFIG, port,
 					   &mac[port], true);
 }
 
+static int sja1105_commit_pvid(struct dsa_switch *ds, int port)
+{
+	struct dsa_port *dp = dsa_to_port(ds, port);
+	struct sja1105_private *priv = ds->priv;
+	u16 pvid;
+
+	if (dp->bridge_dev && br_vlan_enabled(dp->bridge_dev))
+		pvid = priv->bridge_pvid[port];
+	else
+		pvid = priv->tag_8021q_pvid[port];
+
+	return sja1105_pvid_apply(priv, port, pvid);
+}
+
 static enum dsa_tag_protocol
 sja1105_get_tag_protocol(struct dsa_switch *ds, int port,
 			 enum dsa_tag_protocol mp)
@@ -2021,179 +2024,6 @@ static int sja1105_is_vlan_configured(struct sja1105_private *priv, u16 vid)
 	return -1;
 }
 
-static int sja1105_commit_vlans(struct sja1105_private *priv,
-				struct sja1105_vlan_lookup_entry *new_vlan)
-{
-	struct sja1105_vlan_lookup_entry *vlan;
-	struct sja1105_table *table;
-	int num_vlans = 0;
-	int rc, i, k = 0;
-
-	/* VLAN table */
-	table = &priv->static_config.tables[BLK_IDX_VLAN_LOOKUP];
-	vlan = table->entries;
-
-	for (i = 0; i < VLAN_N_VID; i++) {
-		int match = sja1105_is_vlan_configured(priv, i);
-
-		if (new_vlan[i].vlanid != VLAN_N_VID)
-			num_vlans++;
-
-		if (new_vlan[i].vlanid == VLAN_N_VID && match >= 0) {
-			/* Was there before, no longer is. Delete */
-			dev_dbg(priv->ds->dev, "Deleting VLAN %d\n", i);
-			rc = sja1105_dynamic_config_write(priv,
-							  BLK_IDX_VLAN_LOOKUP,
-							  i, &vlan[match], false);
-			if (rc < 0)
-				return rc;
-		} else if (new_vlan[i].vlanid != VLAN_N_VID) {
-			/* Nothing changed, don't do anything */
-			if (match >= 0 &&
-			    vlan[match].vlanid == new_vlan[i].vlanid &&
-			    vlan[match].tag_port == new_vlan[i].tag_port &&
-			    vlan[match].vlan_bc == new_vlan[i].vlan_bc &&
-			    vlan[match].vmemb_port == new_vlan[i].vmemb_port)
-				continue;
-			/* Update entry */
-			dev_dbg(priv->ds->dev, "Updating VLAN %d\n", i);
-			rc = sja1105_dynamic_config_write(priv,
-							  BLK_IDX_VLAN_LOOKUP,
-							  i, &new_vlan[i],
-							  true);
-			if (rc < 0)
-				return rc;
-		}
-	}
-
-	if (table->entry_count)
-		kfree(table->entries);
-
-	table->entries = kcalloc(num_vlans, table->ops->unpacked_entry_size,
-				 GFP_KERNEL);
-	if (!table->entries)
-		return -ENOMEM;
-
-	table->entry_count = num_vlans;
-	vlan = table->entries;
-
-	for (i = 0; i < VLAN_N_VID; i++) {
-		if (new_vlan[i].vlanid == VLAN_N_VID)
-			continue;
-		vlan[k++] = new_vlan[i];
-	}
-
-	return 0;
-}
-
-static int sja1105_commit_pvid(struct sja1105_private *priv)
-{
-	struct sja1105_bridge_vlan *v;
-	struct list_head *vlan_list;
-	int rc = 0;
-
-	if (priv->vlan_aware)
-		vlan_list = &priv->bridge_vlans;
-	else
-		vlan_list = &priv->dsa_8021q_vlans;
-
-	list_for_each_entry(v, vlan_list, list) {
-		if (v->pvid) {
-			rc = sja1105_pvid_apply(priv, v->port, v->vid);
-			if (rc)
-				break;
-		}
-	}
-
-	return rc;
-}
-
-static int
-sja1105_build_bridge_vlans(struct sja1105_private *priv,
-			   struct sja1105_vlan_lookup_entry *new_vlan)
-{
-	struct sja1105_bridge_vlan *v;
-
-	if (!priv->vlan_aware)
-		return 0;
-
-	list_for_each_entry(v, &priv->bridge_vlans, list) {
-		int match = v->vid;
-
-		new_vlan[match].vlanid = v->vid;
-		new_vlan[match].vmemb_port |= BIT(v->port);
-		new_vlan[match].vlan_bc |= BIT(v->port);
-		if (!v->untagged)
-			new_vlan[match].tag_port |= BIT(v->port);
-		new_vlan[match].type_entry = SJA1110_VLAN_D_TAG;
-	}
-
-	return 0;
-}
-
-static int
-sja1105_build_dsa_8021q_vlans(struct sja1105_private *priv,
-			      struct sja1105_vlan_lookup_entry *new_vlan)
-{
-	struct sja1105_bridge_vlan *v;
-
-	list_for_each_entry(v, &priv->dsa_8021q_vlans, list) {
-		int match = v->vid;
-
-		new_vlan[match].vlanid = v->vid;
-		new_vlan[match].vmemb_port |= BIT(v->port);
-		new_vlan[match].vlan_bc |= BIT(v->port);
-		if (!v->untagged)
-			new_vlan[match].tag_port |= BIT(v->port);
-		new_vlan[match].type_entry = SJA1110_VLAN_D_TAG;
-	}
-
-	return 0;
-}
-
-static int sja1105_build_vlan_table(struct sja1105_private *priv)
-{
-	struct sja1105_vlan_lookup_entry *new_vlan;
-	struct sja1105_table *table;
-	int rc, i;
-
-	table = &priv->static_config.tables[BLK_IDX_VLAN_LOOKUP];
-	new_vlan = kcalloc(VLAN_N_VID,
-			   table->ops->unpacked_entry_size, GFP_KERNEL);
-	if (!new_vlan)
-		return -ENOMEM;
-
-	for (i = 0; i < VLAN_N_VID; i++)
-		new_vlan[i].vlanid = VLAN_N_VID;
-
-	/* Bridge VLANs */
-	rc = sja1105_build_bridge_vlans(priv, new_vlan);
-	if (rc)
-		goto out;
-
-	/* VLANs necessary for dsa_8021q operation, given to us by tag_8021q.c:
-	 * - RX VLANs
-	 * - TX VLANs
-	 * - Crosschip links
-	 */
-	rc = sja1105_build_dsa_8021q_vlans(priv, new_vlan);
-	if (rc)
-		goto out;
-
-	rc = sja1105_commit_vlans(priv, new_vlan);
-	if (rc)
-		goto out;
-
-	rc = sja1105_commit_pvid(priv);
-	if (rc)
-		goto out;
-
-out:
-	kfree(new_vlan);
-
-	return rc;
-}
-
 /* The TPID setting belongs to the General Parameters table,
  * which can only be partially reconfigured at runtime (and not the TPID).
  * So a switch reset is required.
@@ -2275,9 +2105,14 @@ int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
 	l2_lookup_params = table->entries;
 	l2_lookup_params->shared_learn = !priv->vlan_aware;
 
-	rc = sja1105_build_vlan_table(priv);
-	if (rc)
-		return rc;
+	for (port = 0; port < ds->num_ports; port++) {
+		if (dsa_is_unused_port(ds, port))
+			continue;
+
+		rc = sja1105_commit_pvid(ds, port);
+		if (rc)
+			return rc;
+	}
 
 	rc = sja1105_static_config_reload(priv, SJA1105_VLAN_FILTERING);
 	if (rc)
@@ -2286,71 +2121,86 @@ int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
 	return rc;
 }
 
-/* Returns number of VLANs added (0 or 1) on success,
- * or a negative error code.
- */
-static int sja1105_vlan_add_one(struct dsa_switch *ds, int port, u16 vid,
-				u16 flags, struct list_head *vlan_list)
-{
-	bool untagged = flags & BRIDGE_VLAN_INFO_UNTAGGED;
-	bool pvid = flags & BRIDGE_VLAN_INFO_PVID;
-	struct sja1105_bridge_vlan *v;
-
-	list_for_each_entry(v, vlan_list, list) {
-		if (v->port == port && v->vid == vid) {
-			/* Already added */
-			if (v->untagged == untagged && v->pvid == pvid)
-				/* Nothing changed */
-				return 0;
-
-			/* It's the same VLAN, but some of the flags changed
-			 * and the user did not bother to delete it first.
-			 * Update it and trigger sja1105_build_vlan_table.
-			 */
-			v->untagged = untagged;
-			v->pvid = pvid;
-			return 1;
-		}
-	}
+static int sja1105_vlan_add(struct sja1105_private *priv, int port, u16 vid,
+			    u16 flags)
+{
+	struct sja1105_vlan_lookup_entry *vlan;
+	struct sja1105_table *table;
+	int match, rc;
 
-	v = kzalloc(sizeof(*v), GFP_KERNEL);
-	if (!v) {
-		dev_err(ds->dev, "Out of memory while storing VLAN\n");
-		return -ENOMEM;
+	table = &priv->static_config.tables[BLK_IDX_VLAN_LOOKUP];
+
+	match = sja1105_is_vlan_configured(priv, vid);
+	if (match < 0) {
+		rc = sja1105_table_resize(table, table->entry_count + 1);
+		if (rc)
+			return rc;
+		match = table->entry_count - 1;
 	}
 
-	v->port = port;
-	v->vid = vid;
-	v->untagged = untagged;
-	v->pvid = pvid;
-	list_add(&v->list, vlan_list);
+	/* Assign pointer after the resize (it's new memory) */
+	vlan = table->entries;
+
+	vlan[match].type_entry = SJA1110_VLAN_D_TAG;
+	vlan[match].vlanid = vid;
+	vlan[match].vlan_bc |= BIT(port);
+	vlan[match].vmemb_port |= BIT(port);
+	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
+		vlan[match].tag_port &= ~BIT(port);
+	else
+		vlan[match].tag_port |= BIT(port);
 
-	return 1;
+	return sja1105_dynamic_config_write(priv, BLK_IDX_VLAN_LOOKUP, vid,
+					    &vlan[match], true);
 }
 
-/* Returns number of VLANs deleted (0 or 1) */
-static int sja1105_vlan_del_one(struct dsa_switch *ds, int port, u16 vid,
-				struct list_head *vlan_list)
+static int sja1105_vlan_del(struct sja1105_private *priv, int port, u16 vid)
 {
-	struct sja1105_bridge_vlan *v, *n;
+	struct sja1105_vlan_lookup_entry *vlan;
+	struct sja1105_table *table;
+	bool keep = true;
+	int match, rc;
 
-	list_for_each_entry_safe(v, n, vlan_list, list) {
-		if (v->port == port && v->vid == vid) {
-			list_del(&v->list);
-			kfree(v);
-			return 1;
-		}
-	}
+	table = &priv->static_config.tables[BLK_IDX_VLAN_LOOKUP];
+
+	match = sja1105_is_vlan_configured(priv, vid);
+	/* Can't delete a missing entry. */
+	if (match < 0)
+		return 0;
+
+	/* Assign pointer after the resize (it's new memory) */
+	vlan = table->entries;
+
+	vlan[match].vlanid = vid;
+	vlan[match].vlan_bc &= ~BIT(port);
+	vlan[match].vmemb_port &= ~BIT(port);
+	/* Also unset tag_port, just so we don't have a confusing bitmap
+	 * (no practical purpose).
+	 */
+	vlan[match].tag_port &= ~BIT(port);
+
+	/* If there's no port left as member of this VLAN,
+	 * it's time for it to go.
+	 */
+	if (!vlan[match].vmemb_port)
+		keep = false;
+
+	rc = sja1105_dynamic_config_write(priv, BLK_IDX_VLAN_LOOKUP, vid,
+					  &vlan[match], keep);
+	if (rc < 0)
+		return rc;
+
+	if (!keep)
+		return sja1105_table_delete_entry(table, match);
 
 	return 0;
 }
 
-static int sja1105_vlan_add(struct dsa_switch *ds, int port,
-			    const struct switchdev_obj_port_vlan *vlan,
-			    struct netlink_ext_ack *extack)
+static int sja1105_bridge_vlan_add(struct dsa_switch *ds, int port,
+				   const struct switchdev_obj_port_vlan *vlan,
+				   struct netlink_ext_ack *extack)
 {
 	struct sja1105_private *priv = ds->priv;
-	bool vlan_table_changed = false;
 	int rc;
 
 	/* Be sure to deny alterations to the configuration done by tag_8021q.
@@ -2361,34 +2211,22 @@ static int sja1105_vlan_add(struct dsa_switch *ds, int port,
 		return -EBUSY;
 	}
 
-	rc = sja1105_vlan_add_one(ds, port, vlan->vid, vlan->flags,
-				  &priv->bridge_vlans);
-	if (rc < 0)
+	rc = sja1105_vlan_add(priv, port, vlan->vid, vlan->flags);
+	if (rc)
 		return rc;
-	if (rc > 0)
-		vlan_table_changed = true;
 
-	if (!vlan_table_changed)
-		return 0;
+	if (vlan->flags & BRIDGE_VLAN_INFO_PVID)
+		priv->bridge_pvid[port] = vlan->vid;
 
-	return sja1105_build_vlan_table(priv);
+	return sja1105_commit_pvid(ds, port);
 }
 
-static int sja1105_vlan_del(struct dsa_switch *ds, int port,
-			    const struct switchdev_obj_port_vlan *vlan)
+static int sja1105_bridge_vlan_del(struct dsa_switch *ds, int port,
+				   const struct switchdev_obj_port_vlan *vlan)
 {
 	struct sja1105_private *priv = ds->priv;
-	bool vlan_table_changed = false;
-	int rc;
-
-	rc = sja1105_vlan_del_one(ds, port, vlan->vid, &priv->bridge_vlans);
-	if (rc > 0)
-		vlan_table_changed = true;
-
-	if (!vlan_table_changed)
-		return 0;
 
-	return sja1105_build_vlan_table(priv);
+	return sja1105_vlan_del(priv, port, vlan->vid);
 }
 
 static int sja1105_dsa_8021q_vlan_add(struct dsa_switch *ds, int port, u16 vid,
@@ -2397,23 +2235,21 @@ static int sja1105_dsa_8021q_vlan_add(struct dsa_switch *ds, int port, u16 vid,
 	struct sja1105_private *priv = ds->priv;
 	int rc;
 
-	rc = sja1105_vlan_add_one(ds, port, vid, flags, &priv->dsa_8021q_vlans);
-	if (rc <= 0)
+	rc = sja1105_vlan_add(priv, port, vid, flags);
+	if (rc)
 		return rc;
 
-	return sja1105_build_vlan_table(priv);
+	if (flags & BRIDGE_VLAN_INFO_PVID)
+		priv->tag_8021q_pvid[port] = vid;
+
+	return sja1105_commit_pvid(ds, port);
 }
 
 static int sja1105_dsa_8021q_vlan_del(struct dsa_switch *ds, int port, u16 vid)
 {
 	struct sja1105_private *priv = ds->priv;
-	int rc;
-
-	rc = sja1105_vlan_del_one(ds, port, vid, &priv->dsa_8021q_vlans);
-	if (!rc)
-		return 0;
 
-	return sja1105_build_vlan_table(priv);
+	return sja1105_vlan_del(priv, port, vid);
 }
 
 /* The programming model for the SJA1105 switch is "all-at-once" via static
@@ -2531,7 +2367,6 @@ static int sja1105_setup(struct dsa_switch *ds)
 static void sja1105_teardown(struct dsa_switch *ds)
 {
 	struct sja1105_private *priv = ds->priv;
-	struct sja1105_bridge_vlan *v, *n;
 	int port;
 
 	rtnl_lock();
@@ -2553,16 +2388,6 @@ static void sja1105_teardown(struct dsa_switch *ds)
 	sja1105_tas_teardown(ds);
 	sja1105_ptp_clock_unregister(ds);
 	sja1105_static_config_free(&priv->static_config);
-
-	list_for_each_entry_safe(v, n, &priv->dsa_8021q_vlans, list) {
-		list_del(&v->list);
-		kfree(v);
-	}
-
-	list_for_each_entry_safe(v, n, &priv->bridge_vlans, list) {
-		list_del(&v->list);
-		kfree(v);
-	}
 }
 
 static void sja1105_port_disable(struct dsa_switch *ds, int port)
@@ -3002,8 +2827,8 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
 	.port_bridge_flags	= sja1105_port_bridge_flags,
 	.port_stp_state_set	= sja1105_bridge_stp_state_set,
 	.port_vlan_filtering	= sja1105_vlan_filtering,
-	.port_vlan_add		= sja1105_vlan_add,
-	.port_vlan_del		= sja1105_vlan_del,
+	.port_vlan_add		= sja1105_bridge_vlan_add,
+	.port_vlan_del		= sja1105_bridge_vlan_del,
 	.port_mdb_add		= sja1105_mdb_add,
 	.port_mdb_del		= sja1105_mdb_del,
 	.port_hwtstamp_get	= sja1105_hwtstamp_get,
@@ -3164,9 +2989,6 @@ static int sja1105_probe(struct spi_device *spi)
 	mutex_init(&priv->ptp_data.lock);
 	mutex_init(&priv->mgmt_lock);
 
-	INIT_LIST_HEAD(&priv->bridge_vlans);
-	INIT_LIST_HEAD(&priv->dsa_8021q_vlans);
-
 	sja1105_tas_setup(ds);
 	sja1105_flower_setup(ds);
 
-- 
2.25.1

[PATCH net-next 5/9] net: dsa: sja1105: deny 8021q uppers on ports

[Vladimir Oltean]

Now that best-effort VLAN filtering is gone and we are left with the
imprecise RX and imprecise TX based in VLAN-aware mode, where the tagger
just guesses the source port based on plausibility of the VLAN ID, 8021q
uppers installed on top of a standalone port, while other ports of that
switch are under a VLAN-aware bridge don't quite "just work".

In fact it could be possible to restrict the VLAN IDs used by the 8021q
uppers to not be shared with VLAN IDs used by that VLAN-aware bridge,
but then the tagger needs to be patched to search for 8021q uppers too,
not just for the "designated bridge port" which will be introduced in a
later patch.

I haven't given a possible implementation full thought, it seems maybe
possible but not worth the effort right now. The only certain thing is
that currently the tagger won't be able to figure out the source port
for these packets because they will come with the VLAN ID of the 8021q
upper and are no longer retagged to a tag_8021q sub-VLAN like the best
effort VLAN filtering code used to do. So just deny these for the
moment.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 309e6a933df7..a380f37fd22d 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2252,6 +2252,20 @@ static int sja1105_dsa_8021q_vlan_del(struct dsa_switch *ds, int port, u16 vid)
 	return sja1105_vlan_del(priv, port, vid);
 }
 
+static int sja1105_prechangeupper(struct dsa_switch *ds, int port,
+				  struct netdev_notifier_changeupper_info *info)
+{
+	struct netlink_ext_ack *extack = info->info.extack;
+	struct net_device *upper = info->upper_dev;
+
+	if (is_vlan_dev(upper)) {
+		NL_SET_ERR_MSG_MOD(extack, "8021q uppers are not supported");
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
 /* The programming model for the SJA1105 switch is "all-at-once" via static
  * configuration tables. Some of these can be dynamically modified at runtime,
  * but not the xMII mode parameters table.
@@ -2846,6 +2860,7 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
 	.devlink_info_get	= sja1105_devlink_info_get,
 	.tag_8021q_vlan_add	= sja1105_dsa_8021q_vlan_add,
 	.tag_8021q_vlan_del	= sja1105_dsa_8021q_vlan_del,
+	.port_prechangeupper	= sja1105_prechangeupper,
 };
 
 static const struct of_device_id sja1105_dt_ids[];
-- 
2.25.1

[PATCH net-next 6/9] net: dsa: sja1105: deny more than one VLAN-aware bridge

[Vladimir Oltean]

With tag_sja1105.c's only ability being to perform an imprecise RX
procedure and identify whether a packet comes from a VLAN-aware bridge
or not, we have no way to determine whether a packet with VLAN ID 5
comes from, say, br0 or br1. Actually we could, but it would mean that
we need to restrict all VLANs from br0 to be different from all VLANs
from br1, and this includes the default_pvid, which makes a setup with 2
VLAN-aware bridges highly imprectical.

The fact of the matter is that this isn't even that big of a practical
limitation, since even with a single VLAN-aware bridge we can pretty
much enforce forwarding isolation based on the VLAN port membership.

So in the end, tell the user that they need to model their setup using a
single VLAN-aware bridge.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index a380f37fd22d..ef63226fed2b 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2257,12 +2257,25 @@ static int sja1105_prechangeupper(struct dsa_switch *ds, int port,
 {
 	struct netlink_ext_ack *extack = info->info.extack;
 	struct net_device *upper = info->upper_dev;
+	struct dsa_switch_tree *dst = ds->dst;
+	struct dsa_port *dp;
 
 	if (is_vlan_dev(upper)) {
 		NL_SET_ERR_MSG_MOD(extack, "8021q uppers are not supported");
 		return -EBUSY;
 	}
 
+	if (netif_is_bridge_master(upper)) {
+		list_for_each_entry(dp, &dst->ports, list) {
+			if (dp->bridge_dev && dp->bridge_dev != upper &&
+			    br_vlan_enabled(dp->bridge_dev)) {
+				NL_SET_ERR_MSG_MOD(extack,
+						   "Only one VLAN-aware bridge is supported");
+				return -EBUSY;
+			}
+		}
+	}
+
 	return 0;
 }
 
-- 
2.25.1

[PATCH net-next 7/9] net: dsa: sja1105: add support for imprecise RX

[Vladimir Oltean]

This is already common knowledge by now, but the sja1105 does not have
hardware support for DSA tagging for data plane packets, and tag_8021q
sets up a unique pvid per port, transmitted as VLAN-tagged towards the
CPU, for the source port to be decoded nonetheless.

When the port is part of a VLAN-aware bridge, the pvid committed to
hardware is taken from the bridge and not from tag_8021q, so we need to
work with that the best we can.

Configure the switches to send all packets to the CPU as VLAN-tagged
(even ones that were originally untagged on the wire) and make use of
dsa_untag_bridge_pvid() to get rid of it before we send those packets up
the network stack.

With the classified VLAN used by hardware known to the tagger, we first
peek at the VID in an attempt to figure out if the packet was received
from a VLAN-unaware port (standalone or under a VLAN-unaware bridge),
case in which we can continue to call dsa_8021q_rcv(). If that is not
the case, the packet probably came from a VLAN-aware bridge. So we call
the DSA helper that finds for us a "designated bridge port" - one that
is a member of the VLAN ID from the packet, and is in the proper STP
state - basically these are all checks performed by br_handle_frame() in
the software RX data path.

The bridge will accept the packet as valid even if the source port was
maybe wrong. So it will maybe learn the MAC SA of the packet on the
wrong port, and its software FDB will be out of sync with the hardware
FDB. So replies towards this same MAC DA will not work, because the
bridge will send towards a different netdev.

This is where the bridge data plane offload ("imprecise TX") added by
the next patch comes in handy. The software FDB is wrong, true, but the
hardware FDB isn't, and by offloading the bridge forwarding plane we
have a chance to right a wrong, and have the hardware look up the FDB
for us for the reply packet. So it all cancels out.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c |  8 ++-
 net/dsa/dsa_priv.h                     | 43 +++++++++++++
 net/dsa/tag_sja1105.c                  | 87 +++++++++++++-------------
 3 files changed, 93 insertions(+), 45 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index ef63226fed2b..a6a671f0fca5 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2201,6 +2201,7 @@ static int sja1105_bridge_vlan_add(struct dsa_switch *ds, int port,
 				   struct netlink_ext_ack *extack)
 {
 	struct sja1105_private *priv = ds->priv;
+	u16 flags = vlan->flags;
 	int rc;
 
 	/* Be sure to deny alterations to the configuration done by tag_8021q.
@@ -2211,7 +2212,11 @@ static int sja1105_bridge_vlan_add(struct dsa_switch *ds, int port,
 		return -EBUSY;
 	}
 
-	rc = sja1105_vlan_add(priv, port, vlan->vid, vlan->flags);
+	/* Always install bridge VLANs as egress-tagged on the CPU port. */
+	if (dsa_is_cpu_port(ds, port))
+		flags = 0;
+
+	rc = sja1105_vlan_add(priv, port, vlan->vid, flags);
 	if (rc)
 		return rc;
 
@@ -2361,6 +2366,7 @@ static int sja1105_setup(struct dsa_switch *ds)
 	 * TPID is ETH_P_SJA1105, and the VLAN ID is the port pvid.
 	 */
 	ds->vlan_filtering_is_global = true;
+	ds->untag_bridge_pvid = true;
 
 	/* Advertise the 8 egress queues */
 	ds->num_tx_queues = SJA1105_NUM_TC;
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index b1d9aa4d313c..da3ad02d6ceb 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -397,6 +397,49 @@ static inline struct sk_buff *dsa_untag_bridge_pvid(struct sk_buff *skb)
 	return skb;
 }
 
+/* For switches without hardware support for DSA tagging to be able
+ * to support termination through the bridge.
+ */
+static inline struct net_device *
+dsa_find_designated_bridge_port_by_vid(struct net_device *master, u16 vid)
+{
+	struct dsa_port *cpu_dp = master->dsa_ptr;
+	struct dsa_switch_tree *dst = cpu_dp->dst;
+	struct bridge_vlan_info vinfo;
+	struct net_device *slave;
+	struct dsa_port *dp;
+	int err;
+
+	list_for_each_entry(dp, &dst->ports, list) {
+		if (dp->type != DSA_PORT_TYPE_USER)
+			continue;
+
+		if (!dp->bridge_dev)
+			continue;
+
+		if (dp->stp_state != BR_STATE_LEARNING &&
+		    dp->stp_state != BR_STATE_FORWARDING)
+			continue;
+
+		/* Since the bridge might learn this packet, keep the CPU port
+		 * affinity with the port that will be used for the reply on
+		 * xmit.
+		 */
+		if (dp->cpu_dp != cpu_dp)
+			continue;
+
+		slave = dp->slave;
+
+		err = br_vlan_get_info_rcu(slave, vid, &vinfo);
+		if (err)
+			continue;
+
+		return slave;
+	}
+
+	return NULL;
+}
+
 /* switch.c */
 int dsa_switch_register_notifier(struct dsa_switch *ds);
 void dsa_switch_unregister_notifier(struct dsa_switch *ds);
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index 7c92c329a092..f142a933c5e2 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -115,40 +115,6 @@ static inline bool sja1105_is_meta_frame(const struct sk_buff *skb)
 	return true;
 }
 
-static bool sja1105_can_use_vlan_as_tags(const struct sk_buff *skb)
-{
-	struct vlan_ethhdr *hdr = vlan_eth_hdr(skb);
-	u16 vlan_tci;
-
-	if (hdr->h_vlan_proto == htons(ETH_P_SJA1105))
-		return true;
-
-	if (hdr->h_vlan_proto != htons(ETH_P_8021Q) &&
-	    !skb_vlan_tag_present(skb))
-		return false;
-
-	if (skb_vlan_tag_present(skb))
-		vlan_tci = skb_vlan_tag_get(skb);
-	else
-		vlan_tci = ntohs(hdr->h_vlan_TCI);
-
-	return vid_is_dsa_8021q(vlan_tci & VLAN_VID_MASK);
-}
-
-/* This is the first time the tagger sees the frame on RX.
- * Figure out if we can decode it.
- */
-static bool sja1105_filter(const struct sk_buff *skb, struct net_device *dev)
-{
-	if (sja1105_can_use_vlan_as_tags(skb))
-		return true;
-	if (sja1105_is_link_local(skb))
-		return true;
-	if (sja1105_is_meta_frame(skb))
-		return true;
-	return false;
-}
-
 /* Calls sja1105_port_deferred_xmit in sja1105_main.c */
 static struct sk_buff *sja1105_defer_xmit(struct sja1105_port *sp,
 					  struct sk_buff *skb)
@@ -371,15 +337,42 @@ static bool sja1110_skb_has_inband_control_extension(const struct sk_buff *skb)
 	return ntohs(eth_hdr(skb)->h_proto) == ETH_P_SJA1110;
 }
 
+/* Returns true for imprecise RX and sets the @vid.
+ * Returns false for precise RX and sets @source_port and @switch_id.
+ */
+static bool sja1105_vlan_rcv(struct sk_buff *skb, int *source_port,
+			     int *switch_id, u16 *vid)
+{
+	struct vlan_ethhdr *hdr = (struct vlan_ethhdr *)skb_mac_header(skb);
+	u16 vlan_tci;
+
+	if (skb_vlan_tag_present(skb))
+		vlan_tci = skb_vlan_tag_get(skb);
+	else
+		vlan_tci = ntohs(hdr->h_vlan_TCI);
+
+	if (vid_is_dsa_8021q_rxvlan(vlan_tci & VLAN_VID_MASK)) {
+		dsa_8021q_rcv(skb, source_port, switch_id);
+		return false;
+	}
+
+	/* Try our best with imprecise RX */
+	*vid = vlan_tci & VLAN_VID_MASK;
+
+	return true;
+}
+
 static struct sk_buff *sja1105_rcv(struct sk_buff *skb,
 				   struct net_device *netdev,
 				   struct packet_type *pt)
 {
+	int source_port = -1, switch_id = -1;
 	struct sja1105_meta meta = {0};
-	int source_port, switch_id;
+	bool imprecise_rx = false;
 	struct ethhdr *hdr;
 	bool is_link_local;
 	bool is_meta;
+	u16 vid;
 
 	hdr = eth_hdr(skb);
 	is_link_local = sja1105_is_link_local(skb);
@@ -389,7 +382,8 @@ static struct sk_buff *sja1105_rcv(struct sk_buff *skb,
 
 	if (sja1105_skb_has_tag_8021q(skb)) {
 		/* Normal traffic path. */
-		dsa_8021q_rcv(skb, &source_port, &switch_id);
+		imprecise_rx = sja1105_vlan_rcv(skb, &source_port, &switch_id,
+						&vid);
 	} else if (is_link_local) {
 		/* Management traffic path. Switch embeds the switch ID and
 		 * port ID into bytes of the destination MAC, courtesy of
@@ -408,7 +402,10 @@ static struct sk_buff *sja1105_rcv(struct sk_buff *skb,
 		return NULL;
 	}
 
-	skb->dev = dsa_master_find_slave(netdev, switch_id, source_port);
+	if (imprecise_rx)
+		skb->dev = dsa_find_designated_bridge_port_by_vid(netdev, vid);
+	else
+		skb->dev = dsa_master_find_slave(netdev, switch_id, source_port);
 	if (!skb->dev) {
 		netdev_warn(netdev, "Couldn't decode source port\n");
 		return NULL;
@@ -522,6 +519,8 @@ static struct sk_buff *sja1110_rcv(struct sk_buff *skb,
 				   struct packet_type *pt)
 {
 	int source_port = -1, switch_id = -1;
+	bool imprecise_rx = false;
+	u16 vid;
 
 	skb->offload_fwd_mark = 1;
 
@@ -534,13 +533,15 @@ static struct sk_buff *sja1110_rcv(struct sk_buff *skb,
 
 	/* Packets with in-band control extensions might still have RX VLANs */
 	if (likely(sja1105_skb_has_tag_8021q(skb)))
-		dsa_8021q_rcv(skb, &source_port, &switch_id);
+		imprecise_rx = sja1105_vlan_rcv(skb, &source_port, &switch_id,
+						&vid);
 
-	skb->dev = dsa_master_find_slave(netdev, switch_id, source_port);
+	if (imprecise_rx)
+		skb->dev = dsa_find_designated_bridge_port_by_vid(netdev, vid);
+	else
+		skb->dev = dsa_master_find_slave(netdev, switch_id, source_port);
 	if (!skb->dev) {
-		netdev_warn(netdev,
-			    "Couldn't decode source port %d and switch id %d\n",
-			    source_port, switch_id);
+		netdev_warn(netdev, "Couldn't decode source port\n");
 		return NULL;
 	}
 
@@ -576,7 +577,6 @@ static const struct dsa_device_ops sja1105_netdev_ops = {
 	.proto = DSA_TAG_PROTO_SJA1105,
 	.xmit = sja1105_xmit,
 	.rcv = sja1105_rcv,
-	.filter = sja1105_filter,
 	.needed_headroom = VLAN_HLEN,
 	.flow_dissect = sja1105_flow_dissect,
 	.promisc_on_master = true,
@@ -590,7 +590,6 @@ static const struct dsa_device_ops sja1110_netdev_ops = {
 	.proto = DSA_TAG_PROTO_SJA1110,
 	.xmit = sja1110_xmit,
 	.rcv = sja1110_rcv,
-	.filter = sja1105_filter,
 	.flow_dissect = sja1110_flow_dissect,
 	.needed_headroom = SJA1110_HEADER_LEN + VLAN_HLEN,
 	.needed_tailroom = SJA1110_RX_TRAILER_LEN + SJA1110_MAX_PADDING_LEN,
-- 
2.25.1

[PATCH net-next 8/9] net: dsa: sja1105: add bridge TX data plane offload based on tag_8021q

[Vladimir Oltean]

The main desire for having this feature in sja1105 is to support network
stack termination for traffic coming from a VLAN-aware bridge.

For sja1105, offloading the bridge data plane means sending packets
as-is, with the proper VLAN tag, to the chip. The chip will look up its
FDB and forward them to the correct destination port.

But we support bridge data plane offload even for VLAN-unaware bridges,
and the implementation there is different. In fact, VLAN-unaware
bridging is governed by tag_8021q, so it makes sense to have the
.bridge_fwd_offload_add() implementation fully within tag_8021q.
The key difference is that we only support 1 VLAN-aware bridge, but we
support multiple VLAN-unaware bridges. So we need to make sure that the
forwarding domain is not crossed by packets injected from the stack.

For this, we introduce the concept of a tag_8021q TX VLAN for bridge
forwarding offload. As opposed to the regular TX VLANs which contain
only 2 ports (the user port and the CPU port), a bridge data plane TX
VLAN is "multicast" (or "imprecise"): it contains all the ports that are
part of a certain bridge, and the hardware will select where the packet
goes within this "imprecise" forwarding domain.

Each VLAN-unaware bridge has its own "imprecise" TX VLAN, so we make use
of the unique "bridge_num" provided by DSA for the data plane offload.
We use the same 3 bits from the tag_8021q VLAN ID format to encode this
bridge number.

Note that these 3 bit positions have been used before for sub-VLANs in
best-effort VLAN filtering mode. The difference is that for best-effort,
the sub-VLANs were only valid on RX (and it was documented that the
sub-VLAN field needed to be transmitted as zero). Whereas for the bridge
data plane offload, these 3 bits are only valid on TX.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_main.c |  4 +++
 include/linux/dsa/8021q.h              | 10 ++++++
 net/dsa/tag_8021q.c                    | 48 +++++++++++++++++++++++---
 net/dsa/tag_sja1105.c                  | 31 +++++++++++++++++
 4 files changed, 89 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index a6a671f0fca5..da042e211dda 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2367,6 +2367,8 @@ static int sja1105_setup(struct dsa_switch *ds)
 	 */
 	ds->vlan_filtering_is_global = true;
 	ds->untag_bridge_pvid = true;
+	/* tag_8021q has 3 bits for the VBID, and the value 0 is reserved */
+	ds->num_fwd_offloading_bridges = 7;
 
 	/* Advertise the 8 egress queues */
 	ds->num_tx_queues = SJA1105_NUM_TC;
@@ -2880,6 +2882,8 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
 	.tag_8021q_vlan_add	= sja1105_dsa_8021q_vlan_add,
 	.tag_8021q_vlan_del	= sja1105_dsa_8021q_vlan_del,
 	.port_prechangeupper	= sja1105_prechangeupper,
+	.port_bridge_tx_fwd_offload = dsa_tag_8021q_bridge_tx_fwd_offload,
+	.port_bridge_tx_fwd_unoffload = dsa_tag_8021q_bridge_tx_fwd_unoffload,
 };
 
 static const struct of_device_id sja1105_dt_ids[];
diff --git a/include/linux/dsa/8021q.h b/include/linux/dsa/8021q.h
index ec5abfcdefd1..c7fa4a3498fe 100644
--- a/include/linux/dsa/8021q.h
+++ b/include/linux/dsa/8021q.h
@@ -35,6 +35,16 @@ struct sk_buff *dsa_8021q_xmit(struct sk_buff *skb, struct net_device *netdev,
 
 void dsa_8021q_rcv(struct sk_buff *skb, int *source_port, int *switch_id);
 
+int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
+					struct net_device *br,
+					int bridge_num);
+
+void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
+					   struct net_device *br,
+					   int bridge_num);
+
+u16 dsa_8021q_bridge_tx_fwd_offload_vid(int bridge_num);
+
 u16 dsa_8021q_tx_vid(struct dsa_switch *ds, int port);
 
 u16 dsa_8021q_rx_vid(struct dsa_switch *ds, int port);
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 51dcde7db26b..654697ebb6f3 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -17,7 +17,7 @@
  *
  * | 11  | 10  |  9  |  8  |  7  |  6  |  5  |  4  |  3  |  2  |  1  |  0  |
  * +-----------+-----+-----------------+-----------+-----------------------+
- * |    DIR    | RSV |    SWITCH_ID    |    RSV    |          PORT         |
+ * |    DIR    | VBID|    SWITCH_ID    |   VBID    |          PORT         |
  * +-----------+-----+-----------------+-----------+-----------------------+
  *
  * DIR - VID[11:10]:
@@ -30,9 +30,10 @@
  * SWITCH_ID - VID[8:6]:
  *	Index of switch within DSA tree. Must be between 0 and 7.
  *
- * RSV - VID[5:4]:
- *	To be used for further expansion of PORT or for other purposes.
- *	Must be transmitted as zero and ignored on receive.
+ * VBID - { VID[9], VID[5:4] }:
+ *	Virtual bridge ID. If between 1 and 7, packet targets the broadcast
+ *	domain of a bridge. If transmitted as zero, packet targets a single
+ *	port. Field only valid on transmit, must be ignored on receive.
  *
  * PORT - VID[3:0]:
  *	Index of switch port. Must be between 0 and 15.
@@ -50,11 +51,30 @@
 #define DSA_8021Q_SWITCH_ID(x)		(((x) << DSA_8021Q_SWITCH_ID_SHIFT) & \
 						 DSA_8021Q_SWITCH_ID_MASK)
 
+#define DSA_8021Q_VBID_HI_SHIFT		9
+#define DSA_8021Q_VBID_HI_MASK		GENMASK(9, 9)
+#define DSA_8021Q_VBID_LO_SHIFT		4
+#define DSA_8021Q_VBID_LO_MASK		GENMASK(5, 4)
+#define DSA_8021Q_VBID_HI(x)		(((x) & GENMASK(2, 2)) >> 2)
+#define DSA_8021Q_VBID_LO(x)		((x) & GENMASK(1, 0))
+#define DSA_8021Q_VBID(x)		\
+		(((DSA_8021Q_VBID_LO(x) << DSA_8021Q_VBID_LO_SHIFT) & \
+		  DSA_8021Q_VBID_LO_MASK) | \
+		 ((DSA_8021Q_VBID_HI(x) << DSA_8021Q_VBID_HI_SHIFT) & \
+		  DSA_8021Q_VBID_HI_MASK))
+
 #define DSA_8021Q_PORT_SHIFT		0
 #define DSA_8021Q_PORT_MASK		GENMASK(3, 0)
 #define DSA_8021Q_PORT(x)		(((x) << DSA_8021Q_PORT_SHIFT) & \
 						 DSA_8021Q_PORT_MASK)
 
+u16 dsa_8021q_bridge_tx_fwd_offload_vid(int bridge_num)
+{
+	/* The VBID value of 0 is reserved for precise TX */
+	return DSA_8021Q_DIR_TX | DSA_8021Q_VBID(bridge_num + 1);
+}
+EXPORT_SYMBOL_GPL(dsa_8021q_bridge_tx_fwd_offload_vid);
+
 /* Returns the VID to be inserted into the frame from xmit for switch steering
  * instructions on egress. Encodes switch ID and port ID.
  */
@@ -387,6 +407,26 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds,
 	return 0;
 }
 
+int dsa_tag_8021q_bridge_tx_fwd_offload(struct dsa_switch *ds, int port,
+					struct net_device *br,
+					int bridge_num)
+{
+	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
+
+	return dsa_port_tag_8021q_vlan_add(dsa_to_port(ds, port), tx_vid);
+}
+EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_offload);
+
+void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
+					   struct net_device *br,
+					   int bridge_num)
+{
+	u16 tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(bridge_num);
+
+	dsa_port_tag_8021q_vlan_del(dsa_to_port(ds, port), tx_vid);
+}
+EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_unoffload);
+
 /* Set up a port's tag_8021q RX and TX VLAN for standalone mode operation */
 static int dsa_tag_8021q_port_setup(struct dsa_switch *ds, int port)
 {
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index f142a933c5e2..cddee4b499d8 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -133,6 +133,31 @@ static u16 sja1105_xmit_tpid(struct sja1105_port *sp)
 	return sp->xmit_tpid;
 }
 
+static struct sk_buff *sja1105_imprecise_xmit(struct sk_buff *skb,
+					      struct net_device *netdev)
+{
+	struct dsa_port *dp = dsa_slave_to_port(netdev);
+	struct net_device *br = dp->bridge_dev;
+	u16 tx_vid;
+
+	/* If the port is under a VLAN-aware bridge, just slide the
+	 * VLAN-tagged packet into the FDB and hope for the best.
+	 * This works because we support a single VLAN-aware bridge
+	 * across the entire dst, and its VLANs cannot be shared with
+	 * any standalone port.
+	 */
+	if (br_vlan_enabled(br))
+		return skb;
+
+	/* If the port is under a VLAN-unaware bridge, use an imprecise
+	 * TX VLAN that targets the bridge's entire broadcast domain,
+	 * instead of just the specific port.
+	 */
+	tx_vid = dsa_8021q_bridge_tx_fwd_offload_vid(dp->bridge_num);
+
+	return dsa_8021q_xmit(skb, netdev, sja1105_xmit_tpid(dp->priv), tx_vid);
+}
+
 static struct sk_buff *sja1105_xmit(struct sk_buff *skb,
 				    struct net_device *netdev)
 {
@@ -141,6 +166,9 @@ static struct sk_buff *sja1105_xmit(struct sk_buff *skb,
 	u16 queue_mapping = skb_get_queue_mapping(skb);
 	u8 pcp = netdev_txq_to_tc(netdev, queue_mapping);
 
+	if (skb->offload_fwd_mark)
+		return sja1105_imprecise_xmit(skb, netdev);
+
 	/* Transmitting management traffic does not rely upon switch tagging,
 	 * but instead SPI-installed management routes. Part 2 of this
 	 * is the .port_deferred_xmit driver callback.
@@ -165,6 +193,9 @@ static struct sk_buff *sja1110_xmit(struct sk_buff *skb,
 	__be16 *tx_header;
 	int trailer_pos;
 
+	if (skb->offload_fwd_mark)
+		return sja1105_imprecise_xmit(skb, netdev);
+
 	/* Transmitting control packets is done using in-band control
 	 * extensions, while data packets are transmitted using
 	 * tag_8021q TX VLANs.
-- 
2.25.1

[PATCH net-next 9/9] Revert "net: dsa: Allow drivers to filter packets they can decode source port from"

[Vladimir Oltean]

This reverts commit cc1939e4b3aaf534fb2f3706820012036825731c.

Currently 2 classes of DSA drivers are able to send/receive packets
directly through the DSA master:
- drivers with DSA_TAG_PROTO_NONE
- sja1105

Now that sja1105 has gained the ability to perform traffic termination
even under the tricky case (VLAN-aware bridge), and that is much more
functional (we can perform VLAN-aware bridging with foreign interfaces),
there is no reason to keep this code in the receive path of the network
core. So delete it.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 include/net/dsa.h  | 15 ---------------
 net/dsa/port.c     |  1 -
 net/ethernet/eth.c |  6 +-----
 3 files changed, 1 insertion(+), 21 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index f8eb2dc3fbef..55fcac854058 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -88,11 +88,6 @@ struct dsa_device_ops {
 			       struct packet_type *pt);
 	void (*flow_dissect)(const struct sk_buff *skb, __be16 *proto,
 			     int *offset);
-	/* Used to determine which traffic should match the DSA filter in
-	 * eth_type_trans, and which, if any, should bypass it and be processed
-	 * as regular on the master net device.
-	 */
-	bool (*filter)(const struct sk_buff *skb, struct net_device *dev);
 	unsigned int needed_headroom;
 	unsigned int needed_tailroom;
 	const char *name;
@@ -246,7 +241,6 @@ struct dsa_port {
 	struct dsa_switch_tree *dst;
 	struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev,
 			       struct packet_type *pt);
-	bool (*filter)(const struct sk_buff *skb, struct net_device *dev);
 
 	enum {
 		DSA_PORT_TYPE_UNUSED = 0,
@@ -985,15 +979,6 @@ static inline bool netdev_uses_dsa(const struct net_device *dev)
 	return false;
 }
 
-static inline bool dsa_can_decode(const struct sk_buff *skb,
-				  struct net_device *dev)
-{
-#if IS_ENABLED(CONFIG_NET_DSA)
-	return !dev->dsa_ptr->filter || dev->dsa_ptr->filter(skb, dev);
-#endif
-	return false;
-}
-
 /* All DSA tags that push the EtherType to the right (basically all except tail
  * tags, which don't break dissection) can be treated the same from the
  * perspective of the flow dissector.
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 7b9bf45a76b6..b927d94b6934 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -888,7 +888,6 @@ int dsa_port_mrp_del_ring_role(const struct dsa_port *dp,
 void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp,
 			       const struct dsa_device_ops *tag_ops)
 {
-	cpu_dp->filter = tag_ops->filter;
 	cpu_dp->rcv = tag_ops->rcv;
 	cpu_dp->tag_ops = tag_ops;
 }
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 9cce612e8976..171ba75b74c9 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -182,12 +182,8 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev)
 	 * at all, so we check here whether one of those tagging
 	 * variants has been configured on the receiving interface,
 	 * and if so, set skb->protocol without looking at the packet.
-	 * The DSA tagging protocol may be able to decode some but not all
-	 * traffic (for example only for management). In that case give it the
-	 * option to filter the packets from which it can decode source port
-	 * information.
 	 */
-	if (unlikely(netdev_uses_dsa(dev)) && dsa_can_decode(skb, dev))
+	if (unlikely(netdev_uses_dsa(dev)))
 		return htons(ETH_P_XDSA);
 
 	if (likely(eth_proto_is_802_3(eth->h_proto)))
-- 
2.25.1

Re: [PATCH net-next 0/9] Traffic termination for sja1105 ports under VLAN-aware bridge

[patchwork-bot+netdevbpf]

Hello:

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

On Mon, 26 Jul 2021 19:55:27 +0300 you wrote:
> This set of patches updates the sja1105 DSA driver to be able to send
> and receive network stack packets on behalf of a VLAN-aware upper bridge
> interface.
> 
> The reasons why this has traditionally been a problem are explained in
> the "Traffic support" section of Documentation/networking/dsa/sja1105.rst.
> (the entire documentation will be revised in a separate patch series).
> 
> [...]

Here is the summary with links:
  - [net-next,1/9] net: bridge: update BROPT_VLAN_ENABLED before notifying switchdev in br_vlan_filter_toggle
    https://git.kernel.org/netdev/net-next/c/f7cdb3ecc9b7
  - [net-next,2/9] net: bridge: add a helper for retrieving port VLANs from the data path
    https://git.kernel.org/netdev/net-next/c/ee80dd2e89ec
  - [net-next,3/9] net: dsa: sja1105: remove redundant re-assignment of pointer table
    https://git.kernel.org/netdev/net-next/c/d63f8877c48c
  - [net-next,4/9] net: dsa: sja1105: delete vlan delta save/restore logic
    https://git.kernel.org/netdev/net-next/c/6dfd23d35e75
  - [net-next,5/9] net: dsa: sja1105: deny 8021q uppers on ports
    https://git.kernel.org/netdev/net-next/c/4fbc08bd3665
  - [net-next,6/9] net: dsa: sja1105: deny more than one VLAN-aware bridge
    https://git.kernel.org/netdev/net-next/c/19fa937a391e
  - [net-next,7/9] net: dsa: sja1105: add support for imprecise RX
    https://git.kernel.org/netdev/net-next/c/884be12f8566
  - [net-next,8/9] net: dsa: sja1105: add bridge TX data plane offload based on tag_8021q
    https://git.kernel.org/netdev/net-next/c/b6ad86e6ad6c
  - [net-next,9/9] Revert "net: dsa: Allow drivers to filter packets they can decode source port from"
    https://git.kernel.org/netdev/net-next/c/edac6f6332d9

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