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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210925183811.jsq2qps257jeqgmf@skbuf>
Date:   Sat, 25 Sep 2021 21:38:11 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Alvin Šipraga <ALSI@...g-olufsen.dk>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Mauri Sandberg <sandberg@...lfence.com>,
        DENG Qingfang <dqfext@...il.com>
Subject: 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.