Re: [PATCH net-next v2] net: dsa: b53: Do not force CPU to be always tagged

On 6/9/2021 3:30 PM, Vladimir Oltean wrote:

On Tue, Jun 08, 2021 at 02:22:04PM -0700, Florian Fainelli wrote:

quoted

Commit ca8931948344 ("net: dsa: b53: Keep CPU port as tagged in all
VLANs") forced the CPU port to be always tagged in any VLAN membership.
This was necessary back then because we did not support Broadcom tags
for all configurations so the only way to differentiate tagged and
untagged traffic while DSA_TAG_PROTO_NONE was used was to force the CPU
port into being always tagged.

With most configurations enabling Broadcom tags, especially after
8fab459e69ab ("net: dsa: b53: Enable Broadcom tags for 531x5/539x
families") we do not need to apply this unconditional force tagging of
the CPU port in all VLANs.

A helper function is introduced to faciliate the encapsulation of the
specific condition requiring the CPU port to be tagged in all VLANs and
the dsa_switch_ops::untag_bridge_pvid boolean is moved to when
dsa_switch_ops::setup is called when we have already determined the
tagging protocol we will be using.

Reported-by: Matthew Hagan <redacted>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---

Florian, does the hardware behave in the same way if you disable
CONFIG_VLAN_8021Q?

Unfortunately it does not because there is no more code calling into
b53_vlan_add() with the desired egress untagged attribute:

00:01:23.015477 b8:ac:6f:80:af:7e (oui Unknown) > 00:10:18:cd:c9:c2 (oui
Unknown), BRCM tag OP: EG, CID: 0, RC: exception, TC: 0, port: 0,
ethertype 802.1Q (0x8100), length 106: vlan 0, p 0, ethertype IPv4
(0x0800), (tos 0x0, ttl 64, id 3662, offset 0, flags [none], proto ICMP
(1), length 84)
    192.168.1.254 > 192.168.1.10: ICMP echo reply, id 5127, seq 17,
length 64
        0x0000:  0010 18cd c9c2 b8ac 6f80 af7e 0000 2000  ........o..~....
        0x0010:  8100 0000 0800 4500 0054 0e4e 0000 4001  ......E..T.N..@.
        0x0020:  e802 c0a8 01fe c0a8 010a 0000 bc2c 1407  .............,..
        0x0030:  0011 3db6 f204 0000 0000 0000 0000 0000  ..=.............
        0x0040:  0000 0000 0000 0000 0000 0000 0000 0000  ................
        0x0050:  0000 0000 0000 0000 0000 0000 0000 0000  ................
        0x0060:  0000 0000 0000 0000 0000

Something like this works:

diff --git a/drivers/net/dsa/b53/b53_common.c

b/drivers/net/dsa/b53/b53_common.c
index 6e199454e41d..ac3bb5ef17e2 100644

--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c

@@ -748,9 +748,13 @@ int b53_configure_vlan(struct dsa_switch *ds)

        b53_enable_vlan(dev, -1, dev->vlan_enabled, ds->vlan_filtering);

-       b53_for_each_port(dev, i)
+       b53_for_each_port(dev, i) {
+               v = &dev->vlans[def_vid];
+               v->members |= BIT(i);
+               v->untag = v->members;
                b53_write16(dev, B53_VLAN_PAGE,
                            B53_VLAN_PORT_DEF_TAG(i), def_vid);
+       }

        /* Upon initial call we have not set-up any VLANs, but upon
         * system resume, we need to restore all VLAN entries.

-- 
Florian