Re: [PATCH net-next 3/6 v6] net: dsa: rtl8366rb: Rewrite weird VLAN filering enablement

On Sat, Sep 25, 2021 at 05:12:24PM +0000, Alvin Šipraga wrote:

 From the previous version of this patch I understood that CTRL1_REG is
for controlling whether or not to accept untagged frames, and CTRL2 for
accepting tagged frames that don't match the port member set. Is that
correct?

It might not hurt to add comments which explain what these registers do.

Do you know whether DSA _always_ wants ports to accept untagged frames,
or only if the port has PVID set? I'm also asking for my own
understanding. In the latter case I think you might have to set CTRL1 in
rtl8366rb_vlan_filtering() (depending on whether the port has a PVID),
as well as whenever a PVID is set or unset. Of course it depends on the
switch semantics - maybe it ignores CTRL1(port) == 1 if the port has a
PVID, in which case your previous version of the patch would be OK.

Documentation/networking/switchdev.rst says:

When the bridge has VLAN filtering enabled and a PVID is not configured on the
ingress port, untagged and 802.1p tagged packets must be dropped. When the bridge
has VLAN filtering enabled and a PVID exists on the ingress port, untagged and
priority-tagged packets must be accepted and forwarded according to the
bridge's port membership of the PVID VLAN. When the bridge has VLAN filtering
disabled, the presence/lack of a PVID should not influence the packet
forwarding decision.

Anyway, I suppose Linus can make that adjustment after the fact too, if
everything else is ok in the other patches.