[PATCH net] net: dsa: b53: fix untagged traffic sent via cpu tagged with VID 0

[PATCH net] net: dsa: b53: fix untagged traffic sent via cpu tagged with VID 0

[Jonas Gorski]

When Linux sends out untagged traffic from a port, it will enter the CPU
port without any VLAN tag, even if the port is a member of a vlan
filtering bridge with a PVID egress untagged VLAN.

This makes the CPU port's PVID take effect, and the PVID's VLAN
table entry controls if the packet will be tagged on egress.

Since commit 45e9d59d3950 ("net: dsa: b53: do not allow to configure
VLAN 0") we remove bridged ports from VLAN 0 when joining or leaving a
VLAN aware bridge. But we also clear the untagged bit, causing untagged
traffic from the controller to become tagged with VID 0 (and priority
0).

Fix this by not touching the untagged map of VLAN 0. Additionally,
always keep the CPU port as a member, as the untag map is only effective
as long as there is at least one member, and we would remove it when
bridging all ports and leaving no standalone ports.

Since Linux (and the switch) treats VLAN 0 tagged traffic like untagged,
the actual impact of this is rather low, but this also prevented earlier
detection of the issue.

Fixes: 45e9d59d3950 ("net: dsa: b53: do not allow to configure VLAN 0")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
My favourite kind of fix, just deleting code :-)

 drivers/net/dsa/b53/b53_common.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 132683ed3abe..6eac09a267d0 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -2051,9 +2051,6 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
 
 		b53_get_vlan_entry(dev, pvid, vl);
 		vl->members &= ~BIT(port);
-		if (vl->members == BIT(cpu_port))
-			vl->members &= ~BIT(cpu_port);
-		vl->untag = vl->members;
 		b53_set_vlan_entry(dev, pvid, vl);
 	}
 
@@ -2132,8 +2129,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
 		}
 
 		b53_get_vlan_entry(dev, pvid, vl);
-		vl->members |= BIT(port) | BIT(cpu_port);
-		vl->untag |= BIT(port) | BIT(cpu_port);
+		vl->members |= BIT(port);
 		b53_set_vlan_entry(dev, pvid, vl);
 	}
 }
-- 
2.43.0

Re: [PATCH net] net: dsa: b53: fix untagged traffic sent via cpu tagged with VID 0

[Paolo Abeni]

On 6/2/25 9:49 PM, Jonas Gorski wrote:
> When Linux sends out untagged traffic from a port, it will enter the CPU
> port without any VLAN tag, even if the port is a member of a vlan
> filtering bridge with a PVID egress untagged VLAN.
> 
> This makes the CPU port's PVID take effect, and the PVID's VLAN
> table entry controls if the packet will be tagged on egress.
> 
> Since commit 45e9d59d3950 ("net: dsa: b53: do not allow to configure
> VLAN 0") we remove bridged ports from VLAN 0 when joining or leaving a
> VLAN aware bridge. But we also clear the untagged bit, causing untagged
> traffic from the controller to become tagged with VID 0 (and priority
> 0).
> 
> Fix this by not touching the untagged map of VLAN 0. Additionally,
> always keep the CPU port as a member, as the untag map is only effective
> as long as there is at least one member, and we would remove it when
> bridging all ports and leaving no standalone ports.
> 
> Since Linux (and the switch) treats VLAN 0 tagged traffic like untagged,
> the actual impact of this is rather low, but this also prevented earlier
> detection of the issue.
> 
> Fixes: 45e9d59d3950 ("net: dsa: b53: do not allow to configure VLAN 0")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
> ---
> My favourite kind of fix, just deleting code :-)
> 
>  drivers/net/dsa/b53/b53_common.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index 132683ed3abe..6eac09a267d0 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -2051,9 +2051,6 @@ int b53_br_join(struct dsa_switch *ds, int port, struct dsa_bridge bridge,
>  
>  		b53_get_vlan_entry(dev, pvid, vl);
>  		vl->members &= ~BIT(port);
> -		if (vl->members == BIT(cpu_port))
> -			vl->members &= ~BIT(cpu_port);
> -		vl->untag = vl->members;
>  		b53_set_vlan_entry(dev, pvid, vl);
>  	}
>  
> @@ -2132,8 +2129,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct dsa_bridge bridge)
>  		}
>  
>  		b53_get_vlan_entry(dev, pvid, vl);
> -		vl->members |= BIT(port) | BIT(cpu_port);
> -		vl->untag |= BIT(port) | BIT(cpu_port);
> +		vl->members |= BIT(port);
>  		b53_set_vlan_entry(dev, pvid, vl);
>  	}
>  }

Makes sense to, but it could use an explicit ack from Florian...

/P

Re: [PATCH net] net: dsa: b53: fix untagged traffic sent via cpu tagged with VID 0

[Florian Fainelli]

On 6/2/25 12:49, Jonas Gorski wrote:
> When Linux sends out untagged traffic from a port, it will enter the CPU
> port without any VLAN tag, even if the port is a member of a vlan
> filtering bridge with a PVID egress untagged VLAN.
> 
> This makes the CPU port's PVID take effect, and the PVID's VLAN
> table entry controls if the packet will be tagged on egress.
> 
> Since commit 45e9d59d3950 ("net: dsa: b53: do not allow to configure
> VLAN 0") we remove bridged ports from VLAN 0 when joining or leaving a
> VLAN aware bridge. But we also clear the untagged bit, causing untagged
> traffic from the controller to become tagged with VID 0 (and priority
> 0).
> 
> Fix this by not touching the untagged map of VLAN 0. Additionally,
> always keep the CPU port as a member, as the untag map is only effective
> as long as there is at least one member, and we would remove it when
> bridging all ports and leaving no standalone ports.
> 
> Since Linux (and the switch) treats VLAN 0 tagged traffic like untagged,
> the actual impact of this is rather low, but this also prevented earlier
> detection of the issue.
> 
> Fixes: 45e9d59d3950 ("net: dsa: b53: do not allow to configure VLAN 0")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH net] net: dsa: b53: fix untagged traffic sent via cpu tagged with VID 0

[Vladimir Oltean]

Hi Jonas,

On Mon, Jun 02, 2025 at 09:49:14PM +0200, Jonas Gorski wrote:
> When Linux sends out untagged traffic from a port, it will enter the CPU
> port without any VLAN tag, even if the port is a member of a vlan
> filtering bridge with a PVID egress untagged VLAN.
> 
> This makes the CPU port's PVID take effect, and the PVID's VLAN
> table entry controls if the packet will be tagged on egress.
> 
> Since commit 45e9d59d3950 ("net: dsa: b53: do not allow to configure
> VLAN 0") we remove bridged ports from VLAN 0 when joining or leaving a
> VLAN aware bridge. But we also clear the untagged bit, causing untagged
> traffic from the controller to become tagged with VID 0 (and priority
> 0).
> 
> Fix this by not touching the untagged map of VLAN 0. Additionally,
> always keep the CPU port as a member, as the untag map is only effective
> as long as there is at least one member, and we would remove it when
> bridging all ports and leaving no standalone ports.
> 
> Since Linux (and the switch) treats VLAN 0 tagged traffic like untagged,
> the actual impact of this is rather low, but this also prevented earlier
> detection of the issue.
> 
> Fixes: 45e9d59d3950 ("net: dsa: b53: do not allow to configure VLAN 0")
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
> ---

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

However, this situation triggered a bell in my mind. But it looks like
I am about to open a can of worms, so I don't expect that to gate this
patch.

In the configuration below, what will the packet look like on the wire?

$ ip link add br0 type bridge vlan_filtering 1 && ip link set br0 up
$ ip link set lan1 master br0 && ip link set lan1 up
$ bridge vlan add dev br0 vid 2 pvid untagged self
$ bridge vlan add dev lan1 vid 2
$ mausezahn br0 -t ip -b 00:01:02:03:04:05 -c 1 -p 1480

Testing both on sja1105, as well as on veth using software bridging, the
answer should be "tagged with VID 2". However, you make it sounds like
the answer on b53 is "untagged".

Depending on the answer, I can try to make some suggestions what to do
about this.

Re: [PATCH net] net: dsa: b53: fix untagged traffic sent via cpu tagged with VID 0

[Vladimir Oltean]

On Thu, Jun 05, 2025 at 11:20:43PM +0300, Vladimir Oltean wrote:
> Testing both on sja1105, as well as on veth using software bridging, the
> answer should be "tagged with VID 2". However, you make it sounds like
> the answer on b53 is "untagged".

Sorry, some neurons misfired. I don't expect the b53 answer to be
anything other than "tagged with VID 2". The bridge passes the skb as
tagged with VID 2 already.

Only if the bridge port VLAN 2 were egress-untagged, would the skb be
coming to ndo_start_xmit() as VLAN-untagged. Then it would be processed
in VID 2 in software, and in VID 0 in hardware. But the fact that VID 2
is egress-untagged in software just like VID 0 is egress-untagged in
hardware will mask that. That would only potentially matter for source
address learning, which I'm not sure whether it exists on CPU-injected
packets on b53.

Re: [PATCH net] net: dsa: b53: fix untagged traffic sent via cpu tagged with VID 0

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon,  2 Jun 2025 21:49:14 +0200 you wrote:
> When Linux sends out untagged traffic from a port, it will enter the CPU
> port without any VLAN tag, even if the port is a member of a vlan
> filtering bridge with a PVID egress untagged VLAN.
> 
> This makes the CPU port's PVID take effect, and the PVID's VLAN
> table entry controls if the packet will be tagged on egress.
> 
> [...]

Here is the summary with links:
  - [net] net: dsa: b53: fix untagged traffic sent via cpu tagged with VID 0
    https://git.kernel.org/netdev/net/c/692eb9f8a5b7

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