[PATCH RFC net-next 0/3] net: dsa: deny unsupported 8021q uppers on bridge ports

[PATCH RFC net-next 0/3] net: dsa: deny unsupported 8021q uppers on bridge ports

[Jonas Gorski]

Documentation/networking/switchdev.rst is quite strict on how VLAN
uppers on bridged ports should work:

- with VLAN filtering turned off, the bridge will process all ingress traffic
  for the port, except for the traffic tagged with a VLAN ID destined for a
  VLAN upper. (...)

- with VLAN filtering turned on, these VLAN devices can be created as long as
  the bridge does not have an existing VLAN entry with the same VID on any
  bridge port. (...)

Presumably with VLAN filtering on, the bridge should also not process
(i.e. forward) traffic destined for a VLAN upper.

But currently, there is no way to tell dsa drivers that a VLAN on a
bridged port is for a VLAN upper and should not be processed by the
bridge.

Both adding a VLAN to a bridge port and adding a VLAN upper to a bridged
port will call dsa_switch_ops::port_vlan_add(), with no way for the
driver to know which is which. But even so, most devices likely would
not support configuring forwarding per VLAN.

So in order to prevent the configuration of configurations with
unintended forwarding between ports:

* deny configuring more than one VLAN upper on bridged ports per VLAN on
  VLAN filtering bridges
* deny configuring any VLAN uppers on bridged ports on VLAN non
  filtering bridges
* And consequently, disallow disabling filtering as long as there are
  any VLAN uppers configured on bridged ports

An alternative solution suggested by switchdev.rst would be to treat
these ports as standalone, and do the filtering/forwarding in software.

But likely DSA supported switches are used on low power devices, where
the performance impact from this would be large.

While going through the code, I also found one corner case where it was
possible to add bridge VLANs shared with VLAN uppers, while adding
VLAN uppers shared with bridge VLANs was properly denied. This is the
first patch as this seems to be like the least controversial.

Sent as a RFC for now due to the potential impact, though a preliminary
test didn't should any failures with bridge_vlan_{un,}aware.sh and
local_termination.sh selftests on BCM63268.

A potential selftest for bridge_vlan_{un,}aware.sh I could think of

- bridge p3, p4
- add VLAN uppers on p1 - p4 with a unique VLAN
  if refused, treat as allowed failure
- check if p4 sees traffic from p1

If p1 and p4 are isolated (so implicitly p2 and p3), its fine, and if
the configuration is rejected is also fine, but forwarding is not.

Jonas Gorski (3):
  net: dsa: deny bridge VLAN with existing 8021q upper on any port
  net: dsa: deny multiple 8021q uppers on bridged ports for the same
    VLAN
  net: dsa: deny 8021q uppers on vlan unaware bridged ports

 net/dsa/port.c | 23 +++------------
 net/dsa/user.c | 80 ++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 71 insertions(+), 32 deletions(-)

-- 
2.43.0

[PATCH RFC net-next 1/3] net: dsa: deny bridge VLAN with existing 8021q upper on any port

[Jonas Gorski]

Currently adding a bridge vlan to a port only checks for an 8021q upper
of that vlan on the port, but does not check for matching 8021q uppers
on other ports.

This leads to the possibility of configuring shared vlans on ports after
adding uppers.

E.g. adding the upper after configuring the vlan would be rejected

$ ip link add br0 type bridge vlan filtering 1
$ ip link set swp1 master br0
$ ip link set swp2 master br0
$ bridge vlan add dev swp2 vid 100
$ ip link add swp1.100 link swp1 type vlan id 100
RTNETLINK answers: Resource busy

But the other way around would currently be accepted:

$ ip link add br0 type bridge vlan filtering 1
$ ip link set swp1 master br0
$ ip link set swp2 master br0
$ ip link add swp1.100 link swp1 type vlan id 100
$ bridge vlan add dev swp2 vid 100
$ bridge vlan
port              vlan-id
swp2              1 PVID Egress Untagged
                  100
swp1              1 PVID Egress Untagged
br0               1 PVID Egress Untagged

Fix this by checking all members of the bridge for a matching vlan
upper, and not the port itself.

After:

$ ip link add br0 type bridge vlan filtering 1
$ ip link set swp1 master br0
$ ip link set swp2 master br0
$ ip link add swp1.100 link swp1 type vlan id 100
$ bridge vlan add dev swp2 vid 100
RTNETLINK answers: Resource busy

Fixes: 1ce39f0ee8da ("net: dsa: convert denying bridge VLAN with existing 8021q upper to PRECHANGEUPPER")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 net/dsa/user.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/net/dsa/user.c b/net/dsa/user.c
index f59d66f0975d..fa1fe0f1493a 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -653,21 +653,30 @@ static int dsa_user_port_attr_set(struct net_device *dev, const void *ctx,
 
 /* Must be called under rcu_read_lock() */
 static int
-dsa_user_vlan_check_for_8021q_uppers(struct net_device *user,
+dsa_user_vlan_check_for_8021q_uppers(struct dsa_port *dp,
 				     const struct switchdev_obj_port_vlan *vlan)
 {
-	struct net_device *upper_dev;
-	struct list_head *iter;
+	struct dsa_switch *ds = dp->ds;
+	struct dsa_port *other_dp;
 
-	netdev_for_each_upper_dev_rcu(user, upper_dev, iter) {
-		u16 vid;
+	dsa_switch_for_each_user_port(other_dp, ds) {
+		struct net_device *user = other_dp->user;
+		struct net_device *upper_dev;
+		struct list_head *iter;
 
-		if (!is_vlan_dev(upper_dev))
+		if (!dsa_port_bridge_same(dp, other_dp))
 			continue;
 
-		vid = vlan_dev_vlan_id(upper_dev);
-		if (vid == vlan->vid)
-			return -EBUSY;
+		netdev_for_each_upper_dev_rcu(user, upper_dev, iter) {
+			u16 vid;
+
+			if (!is_vlan_dev(upper_dev))
+				continue;
+
+			vid = vlan_dev_vlan_id(upper_dev);
+			if (vid == vlan->vid)
+				return -EBUSY;
+		}
 	}
 
 	return 0;
@@ -693,11 +702,11 @@ static int dsa_user_vlan_add(struct net_device *dev,
 	 */
 	if (br_vlan_enabled(dsa_port_bridge_dev_get(dp))) {
 		rcu_read_lock();
-		err = dsa_user_vlan_check_for_8021q_uppers(dev, vlan);
+		err = dsa_user_vlan_check_for_8021q_uppers(dp, vlan);
 		rcu_read_unlock();
 		if (err) {
 			NL_SET_ERR_MSG_MOD(extack,
-					   "Port already has a VLAN upper with this VID");
+					   "This VLAN already has an upper configured on a bridge port");
 			return err;
 		}
 	}
-- 
2.43.0

[PATCH RFC net-next 2/3] net: dsa: deny multiple 8021q uppers on bridged ports for the same VLAN

[Jonas Gorski]

When creating 8021q uppers on bridged ports on a vlan filtering bridge,
we will configure the VLAN on the ports. For the dsa driver, there is no
difference between a 8021q upper on bridged port and a port vlan
configured within the bridge.

For that reason, if we configure a second 8021q upper for the same VLAN
on a different port of the bridge, we implicitly enable forwarding
between these ports on that VLAN.

This breaks the requirement for 8021q uppers for the VLAN to be
consumed, so we need to reject these configurations. Reuse
dsa_user_vlan_check_for_8021q_uppers() and change its argument to just
the vlan id.

Before:

$ ip link add br0 type bridge vlan_filtering 1
$ ip link set swp1 master br0
$ ip link set swp2 master br0
$ ip link add swp1.100 link GbE1 type vlan id 100
$ ip link add swp2.100 link GbE2 type vlan id 100
$

After:

$ ip link add br0 type bridge vlan_filtering 1
$ ip link set swp1 master br0
$ ip link set swp2 master br0
$ ip link add swp1.100 link GbE1 type vlan id 100
$ ip link add swp2.100 link GbE2 type vlan id 100
RTNETLINK answers: Resource busy

Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 net/dsa/user.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/net/dsa/user.c b/net/dsa/user.c
index fa1fe0f1493a..e8c6452780b0 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -653,8 +653,7 @@ static int dsa_user_port_attr_set(struct net_device *dev, const void *ctx,
 
 /* Must be called under rcu_read_lock() */
 static int
-dsa_user_vlan_check_for_8021q_uppers(struct dsa_port *dp,
-				     const struct switchdev_obj_port_vlan *vlan)
+dsa_user_vlan_check_for_8021q_uppers(struct dsa_port *dp, u16 other_vid)
 {
 	struct dsa_switch *ds = dp->ds;
 	struct dsa_port *other_dp;
@@ -674,7 +673,7 @@ dsa_user_vlan_check_for_8021q_uppers(struct dsa_port *dp,
 				continue;
 
 			vid = vlan_dev_vlan_id(upper_dev);
-			if (vid == vlan->vid)
+			if (vid == other_vid)
 				return -EBUSY;
 		}
 	}
@@ -702,7 +701,7 @@ static int dsa_user_vlan_add(struct net_device *dev,
 	 */
 	if (br_vlan_enabled(dsa_port_bridge_dev_get(dp))) {
 		rcu_read_lock();
-		err = dsa_user_vlan_check_for_8021q_uppers(dp, vlan);
+		err = dsa_user_vlan_check_for_8021q_uppers(dp, vlan->vid);
 		rcu_read_unlock();
 		if (err) {
 			NL_SET_ERR_MSG_MOD(extack,
@@ -3185,6 +3184,16 @@ dsa_user_check_8021q_upper(struct net_device *dev,
 		return notifier_from_errno(-EBUSY);
 	}
 
+	rcu_read_lock();
+	err = dsa_user_vlan_check_for_8021q_uppers(dp, vid);
+	rcu_read_unlock();
+
+	if (err) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "This VLAN already has an upper configured on a bridge port");
+		return notifier_from_errno(err);
+	}
+
 	return NOTIFY_DONE;
 }
 
-- 
2.43.0

[PATCH RFC net-next 3/3] net: dsa: deny 8021q uppers on vlan unaware bridged ports

[Jonas Gorski]

Documentation/networking/switchdev.rst says:

- with VLAN filtering turned off, the bridge will process all ingress
  traffic for the port, except for the traffic tagged with a VLAN ID
  destined for a VLAN upper.

But there is currently no way to configure this in dsa. The vlan upper
will trigger a vlan add to the driver, but it is the same message as a
newly configured bridge VLAN.

Therefore traffic tagged with the VID will continue to be forwarded to
other ports, and therefore we cannot support VLAN uppers on ports of a
VLAN unaware bridges.

Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 net/dsa/port.c | 23 ++++-------------------
 net/dsa/user.c | 38 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/net/dsa/port.c b/net/dsa/port.c
index 082573ae6864..d7746885f7e0 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -728,35 +728,20 @@ static bool dsa_port_can_apply_vlan_filtering(struct dsa_port *dp,
 {
 	struct dsa_switch *ds = dp->ds;
 	struct dsa_port *other_dp;
-	int err;
 
-	/* VLAN awareness was off, so the question is "can we turn it on".
+	/* VLAN awareness was on, so the question is "can we turn it off".
 	 * We may have had 8021q uppers, those need to go. Make sure we don't
 	 * enter an inconsistent state: deny changing the VLAN awareness state
 	 * as long as we have 8021q uppers.
 	 */
-	if (vlan_filtering && dsa_port_is_user(dp)) {
-		struct net_device *br = dsa_port_bridge_dev_get(dp);
+	if (!vlan_filtering && dsa_port_is_user(dp)) {
 		struct net_device *upper_dev, *user = dp->user;
 		struct list_head *iter;
 
 		netdev_for_each_upper_dev_rcu(user, upper_dev, iter) {
-			struct bridge_vlan_info br_info;
-			u16 vid;
-
-			if (!is_vlan_dev(upper_dev))
-				continue;
-
-			vid = vlan_dev_vlan_id(upper_dev);
-
-			/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
-			 * device, respectively the VID is not found, returning
-			 * 0 means success, which is a failure for us here.
-			 */
-			err = br_vlan_get_info(br, vid, &br_info);
-			if (err == 0) {
+			if (is_vlan_dev(upper_dev)) {
 				NL_SET_ERR_MSG_MOD(extack,
-						   "Must first remove VLAN uppers having VIDs also present in bridge");
+						   "Must first remove VLAN uppers from bridged ports");
 				return false;
 			}
 		}
diff --git a/net/dsa/user.c b/net/dsa/user.c
index e8c6452780b0..35265829aa90 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -3156,6 +3156,30 @@ dsa_prevent_bridging_8021q_upper(struct net_device *dev,
 	return NOTIFY_DONE;
 }
 
+/* Must be called under rcu_read_lock() */
+static int
+dsa_user_vlan_check_for_any_8021q_uppers(struct dsa_port *dp)
+{
+	struct dsa_switch *ds = dp->ds;
+	struct dsa_port *other_dp;
+
+	dsa_switch_for_each_user_port(other_dp, ds) {
+		struct net_device *user = other_dp->user;
+		struct net_device *upper_dev;
+		struct list_head *iter;
+
+		if (!dsa_port_bridge_same(dp, other_dp))
+			continue;
+
+		netdev_for_each_upper_dev_rcu(user, upper_dev, iter) {
+			if (is_vlan_dev(upper_dev))
+				return -EBUSY;
+		}
+	}
+
+	return 0;
+}
+
 static int
 dsa_user_check_8021q_upper(struct net_device *dev,
 			   struct netdev_notifier_changeupper_info *info)
@@ -3167,10 +3191,22 @@ dsa_user_check_8021q_upper(struct net_device *dev,
 	int err = NOTIFY_DONE;
 	u16 vid;
 
-	if (!br || !br_vlan_enabled(br))
+	if (!br)
 		return NOTIFY_DONE;
 
 	extack = netdev_notifier_info_to_extack(&info->info);
+
+	if (!br_vlan_enabled(br)) {
+		rcu_read_lock();
+		err = dsa_user_vlan_check_for_any_8021q_uppers(dp);
+		rcu_read_unlock();
+		if (err) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "VLAN uppers not supported with non filtering bridges");
+			return notifier_from_errno(err);
+		}
+	}
+
 	vid = vlan_dev_vlan_id(info->upper_dev);
 
 	/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
-- 
2.43.0

Re: [PATCH RFC net-next 3/3] net: dsa: deny 8021q uppers on vlan unaware bridged ports

[Vladimir Oltean]

On Mon, Nov 10, 2025 at 10:44:43PM +0100, Jonas Gorski wrote:
> Documentation/networking/switchdev.rst says:
> 
> - with VLAN filtering turned off, the bridge will process all ingress
>   traffic for the port, except for the traffic tagged with a VLAN ID
>   destined for a VLAN upper.
> 
> But there is currently no way to configure this in dsa. The vlan upper
> will trigger a vlan add to the driver, but it is the same message as a
> newly configured bridge VLAN.

hmm, not necessarily. vlan_vid_add() will only go through with
vlan_add_rx_filter_info() -> dev->netdev_ops->ndo_vlan_rx_add_vid()
if the device is vlan_hw_filter_capable().

And that's the key, DSA user ports only(*) become vlan_hw_filter_capable()
when under a VLAN _aware_ bridge. (*)actually here is the exception
you're probably hitting: due to the ds->vlan_filtering_is_global quirk,
unrelated ports become vlan_hw_filter_capable() too, not just the ones
under the VLAN-aware bridge. This is possibly what you're seeing and the
reason for the incorrect conclusion that VLAN-unaware bridge ports have
the behaviour you mention.

> Therefore traffic tagged with the VID will continue to be forwarded to
> other ports, and therefore we cannot support VLAN uppers on ports of a
> VLAN unaware bridges.

Incorrect premise => incorrect conclusion.
(not to say that an uncaught problem isn't there for ds->vlan_filtering_is_global
switches, but this isn't it)

Re: [PATCH RFC net-next 3/3] net: dsa: deny 8021q uppers on vlan unaware bridged ports

[Jonas Gorski]

On Mon, Nov 10, 2025 at 11:25 PM Vladimir Oltean
<vladimir.oltean@nxp.com> wrote:
>
> On Mon, Nov 10, 2025 at 10:44:43PM +0100, Jonas Gorski wrote:
> > Documentation/networking/switchdev.rst says:
> >
> > - with VLAN filtering turned off, the bridge will process all ingress
> >   traffic for the port, except for the traffic tagged with a VLAN ID
> >   destined for a VLAN upper.
> >
> > But there is currently no way to configure this in dsa. The vlan upper
> > will trigger a vlan add to the driver, but it is the same message as a
> > newly configured bridge VLAN.
>
> hmm, not necessarily. vlan_vid_add() will only go through with
> vlan_add_rx_filter_info() -> dev->netdev_ops->ndo_vlan_rx_add_vid()
> if the device is vlan_hw_filter_capable().
>
> And that's the key, DSA user ports only(*) become vlan_hw_filter_capable()
> when under a VLAN _aware_ bridge. (*)actually here is the exception
> you're probably hitting: due to the ds->vlan_filtering_is_global quirk,
> unrelated ports become vlan_hw_filter_capable() too, not just the ones
> under the VLAN-aware bridge. This is possibly what you're seeing and the
> reason for the incorrect conclusion that VLAN-unaware bridge ports have
> the behaviour you mention.

Ah, right, no call at all.

But this is about a bridge with VLAN filtering disabled, so filtering
isn't enabled on any port, so ds->vlan_filtering_is_global does not
matter.

See my examples in my reply to 0/3, which hopefully clarify what I am
trying to prevent here.

> > Therefore traffic tagged with the VID will continue to be forwarded to
> > other ports, and therefore we cannot support VLAN uppers on ports of a
> > VLAN unaware bridges.
>
> Incorrect premise => incorrect conclusion.
> (not to say that an uncaught problem isn't there for ds->vlan_filtering_is_global
> switches, but this isn't it)

This should happen regardless of vlan filtering is global.

But I noticed while testing that apparently b53 in filtering=0 mode
does not forward any tagged traffic (and I think I know why ...).

Is there a way to ask for a replay of the fdb (static) entries? To fix
this for older switches, we need to disable 802.1q mode, but this also
switches the ARL from IVL to SVL, which changes the hashing, and would
break any existing entries. So we need to flush the ARL before
toggling 802.1q mode, and then reprogram any static entries.

Best regards,
Jonas

Re: [PATCH RFC net-next 3/3] net: dsa: deny 8021q uppers on vlan unaware bridged ports

[Vladimir Oltean]

On Tue, Nov 11, 2025 at 11:06:48AM +0100, Jonas Gorski wrote:
> But I noticed while testing that apparently b53 in filtering=0 mode
> does not forward any tagged traffic (and I think I know why ...).
> 
> Is there a way to ask for a replay of the fdb (static) entries? To fix
> this for older switches, we need to disable 802.1q mode, but this also
> switches the ARL from IVL to SVL, which changes the hashing, and would
> break any existing entries. So we need to flush the ARL before
> toggling 802.1q mode, and then reprogram any static entries.

I'm not clear on what happens. "Broken" FDB entries in the incorrect
bridge vlan_filtering mode sounds like normal behaviour (FDB entries
with VID=0 while vlan_filtering=1, or FDB entries with VID!=0 while
vlan_filtering=0). They should just sit idle in the ARL until the VLAN
filtering mode makes them active.

Re: [PATCH RFC net-next 3/3] net: dsa: deny 8021q uppers on vlan unaware bridged ports

[Jonas Gorski]

On Tue, Nov 11, 2025 at 12:56 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Tue, Nov 11, 2025 at 11:06:48AM +0100, Jonas Gorski wrote:
> > But I noticed while testing that apparently b53 in filtering=0 mode
> > does not forward any tagged traffic (and I think I know why ...).
> >
> > Is there a way to ask for a replay of the fdb (static) entries? To fix
> > this for older switches, we need to disable 802.1q mode, but this also
> > switches the ARL from IVL to SVL, which changes the hashing, and would
> > break any existing entries. So we need to flush the ARL before
> > toggling 802.1q mode, and then reprogram any static entries.
>
> I'm not clear on what happens. "Broken" FDB entries in the incorrect
> bridge vlan_filtering mode sounds like normal behaviour (FDB entries
> with VID=0 while vlan_filtering=1, or FDB entries with VID!=0 while
> vlan_filtering=0). They should just sit idle in the ARL until the VLAN
> filtering mode makes them active.

When in SVL mode (vlan disabled), the ARL switches from mac+vid to
just mac for hashing ARL entries. And I don't know if mac+vid=0 yields
the same hash as only mac. It would it the switch uses vid=0 when not
vlan aware, but if it skips the vid then it wouldn't.

And we automatically install static entries for the MAC addresses of
ports (and maybe other non-dsa bridged devices), so we may need to
have these twice in the ARL table (once for non-filtering, once for
filtering).

If the hash is not the same, this can happen:

vlan_enabled=1, ARL hashing uses mac+vid
add static entry mac=abc,vid=0 for port 1 => hash(mac, vid) -> entry 123
vlan_enabled => 0, ARL hashing uses only mac
packet received on port 2 for mac=abc => hash(mac) => entry 456 => no
entry found => flood (which may not include port 1).

when trying to delete the static entry => lookup for mac=abc,vid=0 =>
hash(mac) => entry 456 => no such entry.

Then maybe we ignore the error, but the moment we enable vlan again,
the hashing changes back to mac+vid, so the "deleted" static entry
becomes active again (despite the linux fdb not knowing about it
anymore).

And even if the hash is the same, it would mean we cannot interact
with any preexisting entries for vid!=1 that were added with vlan
filtering = 1. So we cannot delete them when e.g. removing a port from
the bridge, or deleting the bridge.

So the safest would be to remove all static entries before changing
vlan filtering, and then re-adding them afterwards.

Best regards,
Jonas

Re: [PATCH RFC net-next 3/3] net: dsa: deny 8021q uppers on vlan unaware bridged ports

[Vladimir Oltean]

On Tue, Nov 11, 2025 at 03:09:08PM +0100, Jonas Gorski wrote:
> On Tue, Nov 11, 2025 at 12:56 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> >
> > On Tue, Nov 11, 2025 at 11:06:48AM +0100, Jonas Gorski wrote:
> > > But I noticed while testing that apparently b53 in filtering=0 mode
> > > does not forward any tagged traffic (and I think I know why ...).
> > >
> > > Is there a way to ask for a replay of the fdb (static) entries? To fix
> > > this for older switches, we need to disable 802.1q mode, but this also
> > > switches the ARL from IVL to SVL, which changes the hashing, and would
> > > break any existing entries. So we need to flush the ARL before
> > > toggling 802.1q mode, and then reprogram any static entries.
> >
> > I'm not clear on what happens. "Broken" FDB entries in the incorrect
> > bridge vlan_filtering mode sounds like normal behaviour (FDB entries
> > with VID=0 while vlan_filtering=1, or FDB entries with VID!=0 while
> > vlan_filtering=0). They should just sit idle in the ARL until the VLAN
> > filtering mode makes them active.
> 
> When in SVL mode (vlan disabled), the ARL switches from mac+vid to
> just mac for hashing ARL entries. And I don't know if mac+vid=0 yields
> the same hash as only mac. It would it the switch uses vid=0 when not
> vlan aware, but if it skips the vid then it wouldn't.
> 
> And we automatically install static entries for the MAC addresses of
> ports (and maybe other non-dsa bridged devices), so we may need to
> have these twice in the ARL table (once for non-filtering, once for
> filtering).
> 
> If the hash is not the same, this can happen:
> 
> vlan_enabled=1, ARL hashing uses mac+vid
> add static entry mac=abc,vid=0 for port 1 => hash(mac, vid) -> entry 123
> vlan_enabled => 0, ARL hashing uses only mac
> packet received on port 2 for mac=abc => hash(mac) => entry 456 => no
> entry found => flood (which may not include port 1).
> 
> when trying to delete the static entry => lookup for mac=abc,vid=0 =>
> hash(mac) => entry 456 => no such entry.
> 
> Then maybe we ignore the error, but the moment we enable vlan again,
> the hashing changes back to mac+vid, so the "deleted" static entry
> becomes active again (despite the linux fdb not knowing about it
> anymore).
> 
> And even if the hash is the same, it would mean we cannot interact
> with any preexisting entries for vid!=1 that were added with vlan
> filtering = 1. So we cannot delete them when e.g. removing a port from
> the bridge, or deleting the bridge.
> 
> So the safest would be to remove all static entries before changing
> vlan filtering, and then re-adding them afterwards.
> 
> Best regards,
> Jonas

If you just want to debug whether this is the case or not, then as I
understand, for the moment you only care about static FDB entries on the
CPU port, not on user ports added with 'bridge fdb add ... master static".
If so, these FDB entries are available in the cpu_dp->fdbs list. For
user ports we don't bother keeping track.

Regarding switchdev FDB replay, it's possible but has very high
complexity. The base call would be to switchdev_bridge_port_replay(),
then you'd need to set up two parallel notifier blocks through which
you're informed of the existing objects (not the usual dsa_user_switchdev_notifier
and dsa_user_switchdev_blocking_notifier), whose internal processing is
partly similar (the event filtering and replication) and partly different:
instead of calling dsa_schedule_work() to program the FDB entries to
hardware, you just add them to a list that is kept in a context
structure, which is passed to the caller once the replay is over and the
list is complete.

For the moment, dp->fdbs should be sufficient to prove/disprove a point.

Re: [PATCH RFC net-next 0/3] net: dsa: deny unsupported 8021q uppers on bridge ports

[Vladimir Oltean]

Hi Jonas,

On Mon, Nov 10, 2025 at 10:44:40PM +0100, Jonas Gorski wrote:
> Documentation/networking/switchdev.rst is quite strict on how VLAN
> uppers on bridged ports should work:
> 
> - with VLAN filtering turned off, the bridge will process all ingress traffic
>   for the port, except for the traffic tagged with a VLAN ID destined for a
>   VLAN upper. (...)
> 
> - with VLAN filtering turned on, these VLAN devices can be created as long as
>   the bridge does not have an existing VLAN entry with the same VID on any
>   bridge port. (...)
> 
> Presumably with VLAN filtering on, the bridge should also not process
> (i.e. forward) traffic destined for a VLAN upper.
> 
> But currently, there is no way to tell dsa drivers that a VLAN on a
> bridged port is for a VLAN upper and should not be processed by the
> bridge.

You say this as if it mattered. We can add a distinguishing mechanism
(for example we can pass a struct dsa_db to .port_vlan_add(), set to
DSA_DB_PORT for VLAN RX filtering and DSA_DB_BRIDGE for bridge VLANs),
but the premise was that drivers don't need to care, because HW won't do
anything useful with that information.

> Both adding a VLAN to a bridge port and adding a VLAN upper to a bridged
> port will call dsa_switch_ops::port_vlan_add(), with no way for the
> driver to know which is which. But even so, most devices likely would
> not support configuring forwarding per VLAN.

Yes, this is why the status quo is that DSA tries to ensure that VLAN
uppers do not cause ports to forward packets between each other.
You are not really changing the status quo in any way, just fixing some
bugs where that didn't happen effectively. Perhaps you could make that a
bit more clear.

> So in order to prevent the configuration of configurations with
> unintended forwarding between ports:
> 
> * deny configuring more than one VLAN upper on bridged ports per VLAN on
>   VLAN filtering bridges
> * deny configuring any VLAN uppers on bridged ports on VLAN non
>   filtering bridges
> * And consequently, disallow disabling filtering as long as there are
>   any VLAN uppers configured on bridged ports

First bullet makes some sense, bullets 2 and 3 not so much.

The first bullet makes just "some" sense because I don't understand why
limit to just bridged ports. We should extend to all NETIF_F_HW_VLAN_CTAG_FILTER
ports as per the dsa_user_manage_vlan_filtering() definitions.

Bullets 2 and 3 don't make sense because it isn't explained how VLAN
non-filtering bridge ports could gain the NETIF_F_HW_VLAN_CTAG_FILTER
feature required for them to see RX filtering VLANs programmed to
hardware in the first place.

> An alternative solution suggested by switchdev.rst would be to treat
> these ports as standalone, and do the filtering/forwarding in software.
> 
> But likely DSA supported switches are used on low power devices, where
> the performance impact from this would be large.
> 
> While going through the code, I also found one corner case where it was
> possible to add bridge VLANs shared with VLAN uppers, while adding
> VLAN uppers shared with bridge VLANs was properly denied. This is the
> first patch as this seems to be like the least controversial.
> 
> Sent as a RFC for now due to the potential impact, though a preliminary
> test didn't should any failures with bridge_vlan_{un,}aware.sh and
> local_termination.sh selftests on BCM63268.
> 
> A potential selftest for bridge_vlan_{un,}aware.sh I could think of
> 
> - bridge p3, p4
> - add VLAN uppers on p1 - p4 with a unique VLAN
>   if refused, treat as allowed failure
> - check if p4 sees traffic from p1
> 
> If p1 and p4 are isolated (so implicitly p2 and p3), its fine, and if
> the configuration is rejected is also fine, but forwarding is not.

Sounds like something which would be fit for
tools/testing/selftests/net/forwarding/no_forwarding.sh.

Re: [PATCH RFC net-next 0/3] net: dsa: deny unsupported 8021q uppers on bridge ports

[Jonas Gorski]

Hi Vladimir,

On Tue, Nov 11, 2025 at 12:01 AM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> Hi Jonas,
>
> On Mon, Nov 10, 2025 at 10:44:40PM +0100, Jonas Gorski wrote:
> > Documentation/networking/switchdev.rst is quite strict on how VLAN
> > uppers on bridged ports should work:
> >
> > - with VLAN filtering turned off, the bridge will process all ingress traffic
> >   for the port, except for the traffic tagged with a VLAN ID destined for a
> >   VLAN upper. (...)
> >
> > - with VLAN filtering turned on, these VLAN devices can be created as long as
> >   the bridge does not have an existing VLAN entry with the same VID on any
> >   bridge port. (...)
> >
> > Presumably with VLAN filtering on, the bridge should also not process
> > (i.e. forward) traffic destined for a VLAN upper.
> >
> > But currently, there is no way to tell dsa drivers that a VLAN on a
> > bridged port is for a VLAN upper and should not be processed by the
> > bridge.
>
> You say this as if it mattered. We can add a distinguishing mechanism
> (for example we can pass a struct dsa_db to .port_vlan_add(), set to
> DSA_DB_PORT for VLAN RX filtering and DSA_DB_BRIDGE for bridge VLANs),
> but the premise was that drivers don't need to care, because HW won't do
> anything useful with that information.

It matters in the case of VLAN uppers on bridged ports. It does not
matter for VLAN uppers on standalone ports.

> > Both adding a VLAN to a bridge port and adding a VLAN upper to a bridged
> > port will call dsa_switch_ops::port_vlan_add(), with no way for the
> > driver to know which is which. But even so, most devices likely would
> > not support configuring forwarding per VLAN.
>
> Yes, this is why the status quo is that DSA tries to ensure that VLAN
> uppers do not cause ports to forward packets between each other.
> You are not really changing the status quo in any way, just fixing some
> bugs where that didn't happen effectively. Perhaps you could make that a
> bit more clear.

Right, I'm trying to prevent situations where the forwarding will
happen despite not being supposed to happen.

> > So in order to prevent the configuration of configurations with
> > unintended forwarding between ports:
> >
> > * deny configuring more than one VLAN upper on bridged ports per VLAN on
> >   VLAN filtering bridges
> > * deny configuring any VLAN uppers on bridged ports on VLAN non
> >   filtering bridges
> > * And consequently, disallow disabling filtering as long as there are
> >   any VLAN uppers configured on bridged ports
>
> First bullet makes some sense, bullets 2 and 3 not so much.
>
> The first bullet makes just "some" sense because I don't understand why
> limit to just bridged ports. We should extend to all NETIF_F_HW_VLAN_CTAG_FILTER
> ports as per the dsa_user_manage_vlan_filtering() definitions.

Standalone ports are isolated from each other, so the configured VLAN
uppers do not matter for forwarding. They will (should) never forward
traffic to other ports, regardless of any VLAN (filtering)
configuration on the bridge, so there is no issue here (pending
correct programming of the switch). Usually isolation trumps any VLAN
memberships.

This is purely about unintended/forbidden forwarding between bridged ports.

> Bullets 2 and 3 don't make sense because it isn't explained how VLAN
> non-filtering bridge ports could gain the NETIF_F_HW_VLAN_CTAG_FILTER
> feature required for them to see RX filtering VLANs programmed to
> hardware in the first place.

Let me try with an example:

let's say we have swp1 - swp4, standalone.

allowed forward destinations for all are the cpu port, no filtering.

now we create a bridge between swp2 and swp3.

now swp2 may also forward to swp3, and swp3 to swp2.

swp1 and swp4 may still only forward to cpu (and this doesn't change
here. We can ignore them).

Bullet point 1:

If vlan_filtering is enabled, swp2 and swp3 will only forward configured VLANs.

swp2 and swp3 will have NETIF_F_HW_VLAN_CTAG_FILTER (as VLAN filtering
is enabled on these ports).

If we enable VID 10 on both ports, the driver will be called with
port_vlan_add(.. vid = 10), and they forward VLAN 10 between each
other.
If we instead create uppers for VID 10 for both ports, the driver will
be called with port_vlan_add(... vid = 10) (as
NETIF_F_HW_VLAN_CTAG_FILTER is is set), and they forward VLAN 10
between each other (oops).

Bullet point 2:

If vlan_filtering is disabled, swp2 and swp3 forward any VID between each other.

swp2 and swp3 won't have NETIF_F_HW_VLAN_CTAG_FILTER (as vlan
filtering is disabled on these ports).

If we now create an upper for VID 10 on swp2, then VLAN 10 should not
be forwarded to swp3 anymore (as VLAN 10 is now "consumed" by the host
on this port).

But since there is no port_vlan_add() call due to filtering disabled
(NETIF_F_HW_VLAN_CTAG_FILTER not set), the dsa driver does not know
that the forwarding should be inhibited between these ports, and VLAN
10 is still forwarded from swp2 to swp3 (oops).

Bullet point 3:
And since having uppers on a bridged ports on a non-filtering bridge
does not inhibit forwarding at all, we cannot allow disabling
filtering as long as VLAN uppers on bridged ports exist.

Does this now make it clearer what situations I am talking about?

The easy way is to disallow these configurations, which is what I try
to attempt (explicitly allowed by switchdev.rst).

One other more expensive options are making bridge ports with VLAN
uppers (or more than one upper for a VLAN) standalone and disable
forwarding, and only do forwarding in software.

Or add the above mentioned DSA_DB_PORT for vlan uppers on (bridged)
ports, regardless of filtering being enabled, and then let the dsa
driver handle forwarding per VLAN. This may or may not be possible,
depending on the hardware. One workaround I can think of is to enable
a VLAN membership violation trap and then remove the port from the
VLAN. But this also has the potential to pull a lot of traffic to the
cpu. And requires drivers to be adapted to handle it. And would
require filtering, which may get complicated for the non-filtering
bridge case.

> > An alternative solution suggested by switchdev.rst would be to treat
> > these ports as standalone, and do the filtering/forwarding in software.
> >
> > But likely DSA supported switches are used on low power devices, where
> > the performance impact from this would be large.
> >
> > While going through the code, I also found one corner case where it was
> > possible to add bridge VLANs shared with VLAN uppers, while adding
> > VLAN uppers shared with bridge VLANs was properly denied. This is the
> > first patch as this seems to be like the least controversial.
> >
> > Sent as a RFC for now due to the potential impact, though a preliminary
> > test didn't should any failures with bridge_vlan_{un,}aware.sh and
> > local_termination.sh selftests on BCM63268.
> >
> > A potential selftest for bridge_vlan_{un,}aware.sh I could think of
> >
> > - bridge p3, p4
> > - add VLAN uppers on p1 - p4 with a unique VLAN
> >   if refused, treat as allowed failure
> > - check if p4 sees traffic from p1
> >
> > If p1 and p4 are isolated (so implicitly p2 and p3), its fine, and if
> > the configuration is rejected is also fine, but forwarding is not.
>
> Sounds like something which would be fit for
> tools/testing/selftests/net/forwarding/no_forwarding.sh.

Oh, wasn't aware of this one, yeah, that seems appropriate. Thanks!
Will try to come up with a test.

Best Regards,
Jonas

Re: [PATCH RFC net-next 0/3] net: dsa: deny unsupported 8021q uppers on bridge ports

[Vladimir Oltean]

On Tue, Nov 11, 2025 at 10:53:00AM +0100, Jonas Gorski wrote:
> Hi Vladimir,
> 
> On Tue, Nov 11, 2025 at 12:01 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> >
> > Hi Jonas,
> >
> > On Mon, Nov 10, 2025 at 10:44:40PM +0100, Jonas Gorski wrote:
> > > Documentation/networking/switchdev.rst is quite strict on how VLAN
> > > uppers on bridged ports should work:
> > >
> > > - with VLAN filtering turned off, the bridge will process all ingress traffic
> > >   for the port, except for the traffic tagged with a VLAN ID destined for a
> > >   VLAN upper. (...)
> > >
> > > - with VLAN filtering turned on, these VLAN devices can be created as long as
> > >   the bridge does not have an existing VLAN entry with the same VID on any
> > >   bridge port. (...)
> > >
> > > Presumably with VLAN filtering on, the bridge should also not process
> > > (i.e. forward) traffic destined for a VLAN upper.
> > >
> > > But currently, there is no way to tell dsa drivers that a VLAN on a
> > > bridged port is for a VLAN upper and should not be processed by the
> > > bridge.
> >
> > You say this as if it mattered. We can add a distinguishing mechanism
> > (for example we can pass a struct dsa_db to .port_vlan_add(), set to
> > DSA_DB_PORT for VLAN RX filtering and DSA_DB_BRIDGE for bridge VLANs),
> > but the premise was that drivers don't need to care, because HW won't do
> > anything useful with that information.
> 
> It matters in the case of VLAN uppers on bridged ports. It does not
> matter for VLAN uppers on standalone ports.

Ok, and what would a driver do with the info that a port_vlan_add() call
came from 8021q and not from the bridge?

> > > Both adding a VLAN to a bridge port and adding a VLAN upper to a bridged
> > > port will call dsa_switch_ops::port_vlan_add(), with no way for the
> > > driver to know which is which. But even so, most devices likely would
> > > not support configuring forwarding per VLAN.
> >
> > Yes, this is why the status quo is that DSA tries to ensure that VLAN
> > uppers do not cause ports to forward packets between each other.
> > You are not really changing the status quo in any way, just fixing some
> > bugs where that didn't happen effectively. Perhaps you could make that a
> > bit more clear.
> 
> Right, I'm trying to prevent situations where the forwarding will
> happen despite not being supposed to happen.
> 
> > > So in order to prevent the configuration of configurations with
> > > unintended forwarding between ports:
> > >
> > > * deny configuring more than one VLAN upper on bridged ports per VLAN on
> > >   VLAN filtering bridges
> > > * deny configuring any VLAN uppers on bridged ports on VLAN non
> > >   filtering bridges
> > > * And consequently, disallow disabling filtering as long as there are
> > >   any VLAN uppers configured on bridged ports
> >
> > First bullet makes some sense, bullets 2 and 3 not so much.
> >
> > The first bullet makes just "some" sense because I don't understand why
> > limit to just bridged ports. We should extend to all NETIF_F_HW_VLAN_CTAG_FILTER
> > ports as per the dsa_user_manage_vlan_filtering() definitions.
> 
> Standalone ports are isolated from each other, so the configured VLAN
> uppers do not matter for forwarding. They will (should) never forward
> traffic to other ports, regardless of any VLAN (filtering)
> configuration on the bridge, so there is no issue here (pending
> correct programming of the switch). Usually isolation trumps any VLAN
> memberships.

So we would hope, that standalone ports are completely isolated from
each other, but unless drivers implement ds->fdb_isolation, that isn't a
given fact. Forwarding might be prevented, but FDB lookups might still
take place, so when you have this setup:

swp1.100     br0
 |         /     \
swp1     swp2    swp3  (bridge vlan add dev swp3 vid 100 master)

and you ping station 00:01:02:03:04:05 from swp1.100, you'd expect it
goes out the wire on swp1. But if swp3 had previously learned 00:01:02:03:04:05,
I wouldn't be surprised if the switch tried to forward it in that
direction instead (failing of course, but dropping the packet in that
process). We would be saved if the tagger's xmit() would force the
packet to bypass FDB lookup, but that isn't a given either...

As I'm saying, swp1 can have NETIF_F_HW_VLAN_CTAG_FILTER set due to any
of the quirks described in dsa_user_manage_vlan_filtering().

It might be a moot point because I haven't verified what are the drivers
which fulfill all conditions for this to be a practical problem. It might
as well be the empty set. For example, sja1105 fulfills them all, but
sja1105_prechangeupper() rejects all 8021q uppers so it is not affected.

> This is purely about unintended/forbidden forwarding between bridged ports.
> 
> > Bullets 2 and 3 don't make sense because it isn't explained how VLAN
> > non-filtering bridge ports could gain the NETIF_F_HW_VLAN_CTAG_FILTER
> > feature required for them to see RX filtering VLANs programmed to
> > hardware in the first place.
> 
> Let me try with an example:
> 
> let's say we have swp1 - swp4, standalone.
> 
> allowed forward destinations for all are the cpu port, no filtering.
> 
> now we create a bridge between swp2 and swp3.
> 
> now swp2 may also forward to swp3, and swp3 to swp2.
> 
> swp1 and swp4 may still only forward to cpu (and this doesn't change
> here. We can ignore them).
> 
> Bullet point 1:
> 
> If vlan_filtering is enabled, swp2 and swp3 will only forward configured VLANs.
> 
> swp2 and swp3 will have NETIF_F_HW_VLAN_CTAG_FILTER (as VLAN filtering
> is enabled on these ports).
> 
> If we enable VID 10 on both ports, the driver will be called with
> port_vlan_add(.. vid = 10), and they forward VLAN 10 between each
> other.
> If we instead create uppers for VID 10 for both ports, the driver will
> be called with port_vlan_add(... vid = 10) (as
> NETIF_F_HW_VLAN_CTAG_FILTER is is set), and they forward VLAN 10
> between each other (oops).

I didn't contest that, and the bridged port example is clear. I just
said I don't think you're seeing the picture broadly enough on this
bullet point. I may be wrong though - just want to clarify what I'm
saying.

> Bullet point 2:
> 
> If vlan_filtering is disabled, swp2 and swp3 forward any VID between each other.
> 
> swp2 and swp3 won't have NETIF_F_HW_VLAN_CTAG_FILTER (as vlan
> filtering is disabled on these ports).
> 
> If we now create an upper for VID 10 on swp2, then VLAN 10 should not
> be forwarded to swp3 anymore (as VLAN 10 is now "consumed" by the host
> on this port).
> 
> But since there is no port_vlan_add() call due to filtering disabled
> (NETIF_F_HW_VLAN_CTAG_FILTER not set), the dsa driver does not know
> that the forwarding should be inhibited between these ports, and VLAN
> 10 is still forwarded from swp2 to swp3 (oops).

Is this the behaviour with veth bridge ports (that VID 10 packets are
trapped as opposed to bridged)? I need a software-based reference to
clearly understand the gap vs DSA's hardware offload.  I don't think
there's any test for that, but it is good to have one.

> Bullet point 3:
> And since having uppers on a bridged ports on a non-filtering bridge
> does not inhibit forwarding at all, we cannot allow disabling
> filtering as long as VLAN uppers on bridged ports exist.
> 
> Does this now make it clearer what situations I am talking about?
> 
> The easy way is to disallow these configurations, which is what I try
> to attempt (explicitly allowed by switchdev.rst).
> 
> One other more expensive options are making bridge ports with VLAN
> uppers (or more than one upper for a VLAN) standalone and disable
> forwarding, and only do forwarding in software.
> 
> Or add the above mentioned DSA_DB_PORT for vlan uppers on (bridged)
> ports, regardless of filtering being enabled, and then let the dsa
> driver handle forwarding per VLAN.

But you still won't get ndo_vlan_rx_add_vid() calls from the 8021q layer
if you're under a VLAN-unaware bridge, so it doesn't help case #2.
You'd have to remove the ndo_vlan_rx_add_vid() handling and manually
track CHANGEUPPER events from 8021q interfaces.

> This may or may not be possible, depending on the hardware. One
> workaround I can think of is to enable a VLAN membership violation
> trap and then remove the port from the VLAN. But this also has the
> potential to pull a lot of traffic to the cpu. And requires drivers to
> be adapted to handle it. And would require filtering, which may get
> complicated for the non-filtering bridge case.

Re: [PATCH RFC net-next 0/3] net: dsa: deny unsupported 8021q uppers on bridge ports

[Jonas Gorski]

On Tue, Nov 11, 2025 at 11:29 AM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Tue, Nov 11, 2025 at 10:53:00AM +0100, Jonas Gorski wrote:
> > Hi Vladimir,
> >
> > On Tue, Nov 11, 2025 at 12:01 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> > >
> > > Hi Jonas,
> > >
> > > On Mon, Nov 10, 2025 at 10:44:40PM +0100, Jonas Gorski wrote:
> > > > Documentation/networking/switchdev.rst is quite strict on how VLAN
> > > > uppers on bridged ports should work:
> > > >
> > > > - with VLAN filtering turned off, the bridge will process all ingress traffic
> > > >   for the port, except for the traffic tagged with a VLAN ID destined for a
> > > >   VLAN upper. (...)
> > > >
> > > > - with VLAN filtering turned on, these VLAN devices can be created as long as
> > > >   the bridge does not have an existing VLAN entry with the same VID on any
> > > >   bridge port. (...)
> > > >
> > > > Presumably with VLAN filtering on, the bridge should also not process
> > > > (i.e. forward) traffic destined for a VLAN upper.
> > > >
> > > > But currently, there is no way to tell dsa drivers that a VLAN on a
> > > > bridged port is for a VLAN upper and should not be processed by the
> > > > bridge.
> > >
> > > You say this as if it mattered. We can add a distinguishing mechanism
> > > (for example we can pass a struct dsa_db to .port_vlan_add(), set to
> > > DSA_DB_PORT for VLAN RX filtering and DSA_DB_BRIDGE for bridge VLANs),
> > > but the premise was that drivers don't need to care, because HW won't do
> > > anything useful with that information.
> >
> > It matters in the case of VLAN uppers on bridged ports. It does not
> > matter for VLAN uppers on standalone ports.
>
> Ok, and what would a driver do with the info that a port_vlan_add() call
> came from 8021q and not from the bridge?

Prevent forwarding of that VID on that port and instead send/redirect
it to the CPU port, if it can do it. Or reject the configuration (e.g.
with not supported).

And if DSA sees the rejection it can decide to switch the port to
standalone mode, so it works.

> > > > Both adding a VLAN to a bridge port and adding a VLAN upper to a bridged
> > > > port will call dsa_switch_ops::port_vlan_add(), with no way for the
> > > > driver to know which is which. But even so, most devices likely would
> > > > not support configuring forwarding per VLAN.
> > >
> > > Yes, this is why the status quo is that DSA tries to ensure that VLAN
> > > uppers do not cause ports to forward packets between each other.
> > > You are not really changing the status quo in any way, just fixing some
> > > bugs where that didn't happen effectively. Perhaps you could make that a
> > > bit more clear.
> >
> > Right, I'm trying to prevent situations where the forwarding will
> > happen despite not being supposed to happen.
> >
> > > > So in order to prevent the configuration of configurations with
> > > > unintended forwarding between ports:
> > > >
> > > > * deny configuring more than one VLAN upper on bridged ports per VLAN on
> > > >   VLAN filtering bridges
> > > > * deny configuring any VLAN uppers on bridged ports on VLAN non
> > > >   filtering bridges
> > > > * And consequently, disallow disabling filtering as long as there are
> > > >   any VLAN uppers configured on bridged ports
> > >
> > > First bullet makes some sense, bullets 2 and 3 not so much.
> > >
> > > The first bullet makes just "some" sense because I don't understand why
> > > limit to just bridged ports. We should extend to all NETIF_F_HW_VLAN_CTAG_FILTER
> > > ports as per the dsa_user_manage_vlan_filtering() definitions.
> >
> > Standalone ports are isolated from each other, so the configured VLAN
> > uppers do not matter for forwarding. They will (should) never forward
> > traffic to other ports, regardless of any VLAN (filtering)
> > configuration on the bridge, so there is no issue here (pending
> > correct programming of the switch). Usually isolation trumps any VLAN
> > memberships.
>
> So we would hope, that standalone ports are completely isolated from
> each other, but unless drivers implement ds->fdb_isolation, that isn't a
> given fact. Forwarding might be prevented, but FDB lookups might still
> take place, so when you have this setup:
>
> swp1.100     br0
>  |         /     \
> swp1     swp2    swp3  (bridge vlan add dev swp3 vid 100 master)
>
> and you ping station 00:01:02:03:04:05 from swp1.100, you'd expect it
> goes out the wire on swp1. But if swp3 had previously learned 00:01:02:03:04:05,
> I wouldn't be surprised if the switch tried to forward it in that
> direction instead (failing of course, but dropping the packet in that
> process). We would be saved if the tagger's xmit() would force the
> packet to bypass FDB lookup, but that isn't a given either...
>
> As I'm saying, swp1 can have NETIF_F_HW_VLAN_CTAG_FILTER set due to any
> of the quirks described in dsa_user_manage_vlan_filtering().
>
> It might be a moot point because I haven't verified what are the drivers
> which fulfill all conditions for this to be a practical problem. It might
> as well be the empty set. For example, sja1105 fulfills them all, but
> sja1105_prechangeupper() rejects all 8021q uppers so it is not affected.

There doesn't need to be a VLAN upper, it's enough if the native VLAN
of standalone ports overlaps with the VLANs used in the bridge.

AFAICT that is at least an issue for those that use 8021q taggers. And
there is the opposite direction, where a similar issue can occur:

If swp1 receives a packet with 00:01:02:03:04:05 the switch may try to
forward it to swp3 (and drop it) instead of delivering it to the host.

But at least for b53 these are not an issue. The xmit header overrides
any fdb lookups, and non-bridged ports are configured to trap all
traffic to the cpu port, skipping fdb lookups. And since they do not
learn, they have no fdb entries configured, so the bridged ports
should never attempt to forward to them.

> > This is purely about unintended/forbidden forwarding between bridged ports.
> >
> > > Bullets 2 and 3 don't make sense because it isn't explained how VLAN
> > > non-filtering bridge ports could gain the NETIF_F_HW_VLAN_CTAG_FILTER
> > > feature required for them to see RX filtering VLANs programmed to
> > > hardware in the first place.
> >
> > Let me try with an example:
> >
> > let's say we have swp1 - swp4, standalone.
> >
> > allowed forward destinations for all are the cpu port, no filtering.
> >
> > now we create a bridge between swp2 and swp3.
> >
> > now swp2 may also forward to swp3, and swp3 to swp2.
> >
> > swp1 and swp4 may still only forward to cpu (and this doesn't change
> > here. We can ignore them).
> >
> > Bullet point 1:
> >
> > If vlan_filtering is enabled, swp2 and swp3 will only forward configured VLANs.
> >
> > swp2 and swp3 will have NETIF_F_HW_VLAN_CTAG_FILTER (as VLAN filtering
> > is enabled on these ports).
> >
> > If we enable VID 10 on both ports, the driver will be called with
> > port_vlan_add(.. vid = 10), and they forward VLAN 10 between each
> > other.
> > If we instead create uppers for VID 10 for both ports, the driver will
> > be called with port_vlan_add(... vid = 10) (as
> > NETIF_F_HW_VLAN_CTAG_FILTER is is set), and they forward VLAN 10
> > between each other (oops).
>
> I didn't contest that, and the bridged port example is clear. I just
> said I don't think you're seeing the picture broadly enough on this
> bullet point. I may be wrong though - just want to clarify what I'm
> saying.
>
> > Bullet point 2:
> >
> > If vlan_filtering is disabled, swp2 and swp3 forward any VID between each other.
> >
> > swp2 and swp3 won't have NETIF_F_HW_VLAN_CTAG_FILTER (as vlan
> > filtering is disabled on these ports).
> >
> > If we now create an upper for VID 10 on swp2, then VLAN 10 should not
> > be forwarded to swp3 anymore (as VLAN 10 is now "consumed" by the host
> > on this port).
> >
> > But since there is no port_vlan_add() call due to filtering disabled
> > (NETIF_F_HW_VLAN_CTAG_FILTER not set), the dsa driver does not know
> > that the forwarding should be inhibited between these ports, and VLAN
> > 10 is still forwarded from swp2 to swp3 (oops).
>
> Is this the behaviour with veth bridge ports (that VID 10 packets are
> trapped as opposed to bridged)? I need a software-based reference to
> clearly understand the gap vs DSA's hardware offload.  I don't think
> there's any test for that, but it is good to have one.

It's at least written that way in switchdev.rst (I admit I haven't
verified that this is true):

"When there is a VLAN device (e.g: sw0p1.100) configured on top of a switchdev
network device which is a bridge port member, the behavior of the software
network stack must be preserved, or the configuration must be refused if that
is not possible.

- with VLAN filtering turned off, the bridge will process all ingress traffic
  for the port, except for the traffic tagged with a VLAN ID destined for a
  VLAN upper. The VLAN upper interface (which consumes the VLAN tag) can even
  be added to a second bridge, which includes other switch ports or software
  interfaces. Some approaches to ensure that the forwarding domain for traffic
  belonging to the VLAN upper interfaces are managed properly:

    * If forwarding destinations can be managed per VLAN, the hardware could be
      configured to map all traffic, except the packets tagged with a VID
      belonging to a VLAN upper interface, to an internal VID corresponding to
      untagged packets. This internal VID spans all ports of the VLAN-unaware
      bridge. The VID corresponding to the VLAN upper interface spans the
      physical port of that VLAN interface, as well as the other ports that
      might be bridged with it. ..."

This very clearly says to me: if there is a VLAN upper on a bridged
port, then any packets tagged with this VLAN must not be forwarded to
other ports, unless the upper is part of a different bridge.

The text for filtering bridges isn't super clear though:

"- with VLAN filtering turned on, these VLAN devices can be created as long as
  the bridge does not have an existing VLAN entry with the same VID on any
  bridge port. These VLAN devices cannot be enslaved into the bridge since they
  duplicate functionality/use case with the bridge's VLAN data path processing."

But I understand this as "same behavior, but no bridging them."

So I don't think that DSA in its current form can support/describe
VLAN uppers on bridged ports (of a non filtering bridge).

And bridging VLAN uppers with physical ports becomes even more fun: I
verified that having e.g. eth1.100 and eth2 in a non-ffiltering
bridge, if if a VLAN tagged packet ingresses on eth2, it will egress
as double tagged on eth1.

Maybe we should just ban non-filtering bridges, would make things so
much easier (j/k).

> > Bullet point 3:
> > And since having uppers on a bridged ports on a non-filtering bridge
> > does not inhibit forwarding at all, we cannot allow disabling
> > filtering as long as VLAN uppers on bridged ports exist.
> >
> > Does this now make it clearer what situations I am talking about?
> >
> > The easy way is to disallow these configurations, which is what I try
> > to attempt (explicitly allowed by switchdev.rst).
> >
> > One other more expensive options are making bridge ports with VLAN
> > uppers (or more than one upper for a VLAN) standalone and disable
> > forwarding, and only do forwarding in software.
> >
> > Or add the above mentioned DSA_DB_PORT for vlan uppers on (bridged)
> > ports, regardless of filtering being enabled, and then let the dsa
> > driver handle forwarding per VLAN.
>
> But you still won't get ndo_vlan_rx_add_vid() calls from the 8021q layer
> if you're under a VLAN-unaware bridge, so it doesn't help case #2.
> You'd have to remove the ndo_vlan_rx_add_vid() handling and manually
> track CHANGEUPPER events from 8021q interfaces.

Right, that would be prerequisite for that, which is probably quite a
bit of work. That's why I'm for now trying to reject this as it is
broken, instead trying to make this work (if it even can be made to
work).

Best regards,
Jonas