Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Alexandre Belloni <alexandre.belloni@bootlin.com>
CC:     Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next 1/7] net: mscc: ocelot: use the pvid of zero when
 bridged with vlan_filtering=0
Thread-Topic: [PATCH net-next 1/7] net: mscc: ocelot: use the pvid of zero
 when bridged with vlan_filtering=0
Thread-Index: AQHWr3C1eyZYc+uP1E2/DC1hu+Q/h6m0iqsAgAByIAA=
Date:   Mon, 2 Nov 2020 15:35:49 +0000
Message-ID: <20201102153548.njizmvohmum6rxdj@skbuf>
References: <20201031102916.667619-1-vladimir.oltean@nxp.com>
 <20201031102916.667619-2-vladimir.oltean@nxp.com>
 <20201102084720.GA7761@piout.net>
In-Reply-To: <20201102084720.GA7761@piout.net>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <9D609BECEE87184AB68E4734120A2FE5@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: f737fb17-31bf-458b-c370-08d87f44f9bc
X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Nov 2020 15:35:49.6646
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 2DlvpCxZP96ZwRAetyFqPVH5HaC/X7at7XP0htNk1l6yVhvonZKFlBdSJwdf28m/Fh2Djzt7DdxYHoYNXXxkAg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB3965
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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=3D0 ttl=3D64 time=3D0.717 ms
64 bytes from 192.168.1.1: seq=3D1 ttl=3D64 time=3D0.442 ms
^C
--- 192.168.1.1 ping statistics ---
2 packets transmitted, 2 packets received, 0% packet loss
round-trip min/avg/max =3D 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=3D0 ttl=3D64 time=3D0.768 ms
64 bytes from 192.168.1.1: seq=3D1 ttl=3D64 time=3D0.657 ms
64 bytes from 192.168.1.1: seq=3D2 ttl=3D64 time=3D0.509 ms
64 bytes from 192.168.1.1: seq=3D3 ttl=3D64 time=3D0.513 ms
64 bytes from 192.168.1.1: seq=3D4 ttl=3D64 time=3D0.496 ms
^C
--- 192.168.1.1 ping statistics ---
5 packets transmitted, 5 packets received, 0% packet loss
round-trip min/avg/max =3D 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=3D0 ttl=3D64 time=3D0.783 ms
64 bytes from 192.168.1.1: seq=3D1 ttl=3D64 time=3D0.289 ms
64 bytes from 192.168.1.1: seq=3D2 ttl=3D64 time=3D0.412 ms
64 bytes from 192.168.1.1: seq=3D3 ttl=3D64 time=3D0.399 ms
64 bytes from 192.168.1.1: seq=3D4 ttl=3D64 time=3D0.396 ms
64 bytes from 192.168.1.1: seq=3D5 ttl=3D64 time=3D0.390 ms
^C
--- 192.168.1.1 ping statistics ---
6 packets transmitted, 6 packets received, 0% packet loss
round-trip min/avg/max =3D 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] =3D 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.=