netdev - Re: [net-next PATCH 3/3 v1] net: dsa: rtl8366: Use DSA core to set up VLAN

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <164d3477-df6d-103c-b9ed-55f5d7705e7a@gmail.com>
Date:   Wed, 8 Jul 2020 19:29:09 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Linus Walleij <linus.walleij@...aro.org>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>
Cc:     DENG Qingfang <dqfext@...il.com>,
        Mauri Sandberg <sandberg@...lfence.com>,
        Vladimir Oltean <olteanv@...il.com>
Subject: Re: [net-next PATCH 3/3 v1] net: dsa: rtl8366: Use DSA core to set up
 VLAN



On 7/8/2020 1:44 PM, Linus Walleij wrote:
> The current code in the RTL8366 VLAN handling code
> initializes the default VLANs like this:
> 
> Ingress packets:
> 
>  port 0   ---> VLAN 1 ---> CPU port (5)
>  port 1   ---> VLAN 2 ---> CPU port (5)
>  port 2   ---> VLAN 3 ---> CPU port (5)
>  port 3   ---> VLAN 4 ---> CPU port (5)
>  port 4   ---> VLAN 5 ---> CPU port (5)
> 
> Egress packets:
>  port 5 (CPU) ---> VLAN 6 ---> port 0, 1, 2, 3, 4
> 
> So 5 VLANs for ingress packets and one VLAN for
> egress packets. Further it sets the PVID
> for each port to further restrict the packets to
> this VLAN only, and sets them as untagged.
> 
> This is a neat set-up in a way and a leftover
> from the OpenWrt driver and the vendor code drop.
> 
> However the DSA core can be instructed to assign
> all ports to a default VLAN, which will be
> VLAN 1. This patch will change the above picture to
> this:
> 
> Ingress packets:
> 
>  port 0   ---> VLAN 1 ---> CPU port (5)
>  port 1   ---> VLAN 1 ---> CPU port (5)
>  port 2   ---> VLAN 1 ---> CPU port (5)
>  port 3   ---> VLAN 1 ---> CPU port (5)
>  port 4   ---> VLAN 1 ---> CPU port (5)
> 
> Egress packets:
>  port 5 (CPU) ---> VLAN 1 ---> port 0, 1, 2, 3, 4
> 
> So all traffic in the switch will by default pass
> on VLAN 1. No PVID is set for ports by the DSA
> core in this case.
> 
> This might have performance impact since the switch
> hardware probably can sort packets into VLANs as
> they pass through the fabric, but it is better
> to fix the above set-up using generic code in that
> case so that it can be reused by other switches.
> 
> The tested scenarios sure work fine with this
> set-up including video streaming from a NAS device.

Does this maintain the requirement that by default, all DSA ports must
be isolated from one another? For instance, if you have broadcast
traffic on port 2, by virtue of having port 1 and port 2 now in VLAN ID
1, do you see that broadcast traffic from port 1?

If you do, then you need to find a way to maintain isolation between ports.

It looks like the FID is used for implementing VLAN filtering so maybe
you need to dedicate a FID per port number here, and add them all to VLAN 1?
-- 
Florian