netdev - Re: [PATCH net-next 1/7] net: mscc: ocelot: use the pvid of zero when bridged with 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: <20201102153548.njizmvohmum6rxdj@skbuf>
Date:   Mon, 2 Nov 2020 15:35:49 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Alexandre Belloni <alexandre.belloni@...tlin.com>
CC:     Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        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>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 1/7] net: mscc: ocelot: use the pvid of zero when
 bridged with vlan_filtering=0

On Mon, Nov 02, 2020 at 09:47:20AM +0100, Alexandre Belloni wrote:
> IIRC, we are using pvid 1 because else bridging breaks when
> CONFIG_VLAN_8021Q is not enabled. Did you test that configuration?

Pertinent question.
I hadn't tested that, but I did now.

[root@...028ARDB ~] # zcat /proc/config.gz | grep 8021Q
# CONFIG_VLAN_8021Q is not set
[root@...028ARDB ~] # ip addr flush swp0
[root@...028ARDB ~] # ip addr add 192.168.1.2/24 dev swp0
[root@...028ARDB ~] # ping 192.168.1.1
PING 192.168.1.1 (192.168.1.1): 56 data bytes
64 bytes from 192.168.1.1: seq=0 ttl=64 time=0.717 ms
64 bytes from 192.168.1.1: seq=1 ttl=64 time=0.442 ms
^C
--- 192.168.1.1 ping statistics ---
2 packets transmitted, 2 packets received, 0% packet loss
round-trip min/avg/max = 0.442/0.579/0.717 ms
[root@...028ARDB ~] # ip addr flush swp0
[root@...028ARDB ~] # ip link add br0 type bridge
[root@...028ARDB ~] # ip link set swp0 master br0
[  409.576303] br0: port 1(swp0) entered blocking state
[  409.581380] br0: port 1(swp0) entered disabled state
[  409.586738] device swp0 entered promiscuous mode
[  409.591866] br0: port 1(swp0) entered blocking state
[  409.596852] br0: port 1(swp0) entered forwarding state
[root@...028ARDB ~] # ip addr add 192.168.1.2/24 dev br0
[root@...028ARDB ~] # ping 192.168.1.1
PING 192.168.1.1 (192.168.1.1): 56 data bytes
64 bytes from 192.168.1.1: seq=0 ttl=64 time=0.768 ms
64 bytes from 192.168.1.1: seq=1 ttl=64 time=0.657 ms
64 bytes from 192.168.1.1: seq=2 ttl=64 time=0.509 ms
64 bytes from 192.168.1.1: seq=3 ttl=64 time=0.513 ms
64 bytes from 192.168.1.1: seq=4 ttl=64 time=0.496 ms
^C
--- 192.168.1.1 ping statistics ---
5 packets transmitted, 5 packets received, 0% packet loss
round-trip min/avg/max = 0.496/0.588/0.768 ms
[root@...028ARDB ~] # ip link del br0
[  135.526825] device swp0 left promiscuous mode
[  135.531729] br0: port 1(swp0) entered disabled state
[root@...028ARDB ~] # ip addr add 192.168.1.2/24 dev swp0
[root@...028ARDB ~] # ping 192.168.1.1
PING 192.168.1.1 (192.168.1.1): 56 data bytes
64 bytes from 192.168.1.1: seq=0 ttl=64 time=0.783 ms
64 bytes from 192.168.1.1: seq=1 ttl=64 time=0.289 ms
64 bytes from 192.168.1.1: seq=2 ttl=64 time=0.412 ms
64 bytes from 192.168.1.1: seq=3 ttl=64 time=0.399 ms
64 bytes from 192.168.1.1: seq=4 ttl=64 time=0.396 ms
64 bytes from 192.168.1.1: seq=5 ttl=64 time=0.390 ms
^C
--- 192.168.1.1 ping statistics ---
6 packets transmitted, 6 packets received, 0% packet loss
round-trip min/avg/max = 0.289/0.444/0.783 ms

There's no logical reason why it wouldn't work. Thanks to your code
which ensures VLAN 0 is in the VLAN table. Nobody is removing VLAN 0
right now.

	/* Because VLAN filtering is enabled, we need VID 0 to get untagged
	 * traffic.  It is added automatically if 8021q module is loaded, but
	 * we can't rely on it since module may be not loaded.
	 */
	ocelot->vlan_mask[0] = GENMASK(ocelot->num_phys_ports - 1, 0);
	ocelot_vlant_set_mask(ocelot, 0, ocelot->vlan_mask[0]);

I cannot test the mscc_ocelot driver, I am only testing felix DSA, and
for that reason I can't even go very far down the history. Remember that
when CONFIG_VLAN_8021Q is disabled, CONFIG_BRIDGE_VLAN_FILTERING also
needs to be disabled. So logically speaking, nobody calls any VLAN
function when CONFIG_VLAN_8021Q is disabled. The standalone
configuration should work in this mode too, shouldn't it? I am not sure
if there's any relevant difference for mscc_ocelot.