Re: [PATCH net-next 1/7] net: mscc: ocelot: use the pvid of zero when bridged with vlan_filtering=0

[Vladimir Oltean]

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@LS1028ARDB ~] # zcat /proc/config.gz | grep 8021Q
# CONFIG_VLAN_8021Q is not set
[root@LS1028ARDB ~] # ip addr flush swp0
[root@LS1028ARDB ~] # ip addr add 192.168.1.2/24 dev swp0
[root@LS1028ARDB ~] # 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@LS1028ARDB ~] # ip addr flush swp0
[root@LS1028ARDB ~] # ip link add br0 type bridge
[root@LS1028ARDB ~] # 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@LS1028ARDB ~] # ip addr add 192.168.1.2/24 dev br0
[root@LS1028ARDB ~] # 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@LS1028ARDB ~] # ip link del br0
[  135.526825] device swp0 left promiscuous mode
[  135.531729] br0: port 1(swp0) entered disabled state
[root@LS1028ARDB ~] # ip addr add 192.168.1.2/24 dev swp0
[root@LS1028ARDB ~] # 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.