On 9/11/2020 12:39 PM, Florian Fainelli wrote:
On 9/11/2020 11:35 AM, Vladimir Oltean wrote:
On Fri, Sep 11, 2020 at 11:23:28AM -0700, Florian Fainelli wrote:
On 9/11/2020 8:43 AM, Vladimir Oltean wrote:
The slightly confusing part is that a vlan_filtering=1 bridge
accepts the
default_pvid either tagged or untagged whereas a vlan_filtering=0
bridge
does not, except for DHCP for instance. I would have to add a br0.1
802.1Q
upper to take care of the default_pvid being egress tagged on the
CPU port.
We could solve this in the DSA receive path, or the Broadcom tag
receive
path as you say since that is dependent on the tagging format and
switch
properties.
With Broadcom tags enabled now, all is well since we can differentiate
traffic from different source ports using that 4 bytes tag.
Where this broke was when using a 802.1Q separation because all
frames that
egressed the CPU were egress tagged and it was no longer possible to
differentiate whether they came from the LAN group in VID 1 or the
WAN group
in VID 2. But all of this should be a thing of the past now, ok,
all is
clear again now.
Or we could do this, what do you think?
Yes, this would be working, and I just tested it with the following
delta on
top of my b53 patch:
diff --git a/drivers/net/dsa/b53/b53_common.c
b/drivers/net/dsa/b53/b53_common.c
index 46ac8875f870..73507cff3bc4 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1427,7 +1427,7 @@ void b53_vlan_add(struct dsa_switch *ds, int port,
untagged = true;
vl->members |= BIT(port);
- if (untagged)
+ if (untagged && !dsa_is_cpu_port(ds, port))
vl->untag |= BIT(port);
else
vl->untag &= ~BIT(port);
@@ -1465,7 +1465,7 @@ int b53_vlan_del(struct dsa_switch *ds, int port,
if (pvid == vid)
pvid = b53_default_pvid(dev);
- if (untagged)
+ if (untagged && !dsa_is_cpu_port(ds, port))
vl->untag &= ~(BIT(port));
b53_set_vlan_entry(dev, vid, vl);
and it works, thanks!
I'm conflicted. So you prefer having the CPU port as egress-tagged?
I do, because I realized that some of the switches we support are still
configured in DSA_TAG_NONE mode because the CPU port they chose is not
Broadcom tag capable and there is an user out there who cares a lot
about that case not to "break".
Also, I think I'll also experiment with a version of this patch that is
local to the DSA RX path. The bridge people may not like it, and as far
as I understand, only DSA has this situation where pvid-tagged traffic
may end up with a vlan tag on ingress.
OK so something along the lines of: port is bridged, and bridge has
vlan_filtering=0 and switch does egress tagging and VID is bridge's
default_pvid then pop the tag?
Should this be a helper function that is utilized by the relevant tagger
drivers or do you want this in dsa_switch_rcv()?
The two drivers that appear to be untagging the CPU port unconditionally
are b53 and kzs9477.
--
Florian
