diff mbox series

[RFC,net-next,v2,4/4] Revert "mt7530 mt7530_fdb_write only set ivl bit vid larger than 1"

Message ID 20210731191023.1329446-5-dqfext@gmail.com
State New
Headers show
Series mt7530 software fallback bridging fix | expand

Commit Message

Qingfang Deng July 31, 2021, 7:10 p.m. UTC
This reverts commit 7e777021780e9c373fc0c04d40b8407ce8c3b5d5.

As independent VLAN learning is also used on VID 0 and 1, remove the
special case.

Signed-off-by: DENG Qingfang <dqfext@gmail.com>
---
 drivers/net/dsa/mt7530.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Vladimir Oltean Aug. 2, 2021, 1:44 p.m. UTC | #1
On Sun, Aug 01, 2021 at 03:10:22AM +0800, DENG Qingfang wrote:
> This reverts commit 7e777021780e9c373fc0c04d40b8407ce8c3b5d5.

> 

> As independent VLAN learning is also used on VID 0 and 1, remove the

> special case.

> 

> Signed-off-by: DENG Qingfang <dqfext@gmail.com>

> ---

>  drivers/net/dsa/mt7530.c | 3 +--

>  1 file changed, 1 insertion(+), 2 deletions(-)

> 

> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c

> index 38d6ce37d692..d72e04011cc5 100644

> --- a/drivers/net/dsa/mt7530.c

> +++ b/drivers/net/dsa/mt7530.c

> @@ -366,8 +366,7 @@ mt7530_fdb_write(struct mt7530_priv *priv, u16 vid,

>  	int i;

>  

>  	reg[1] |= vid & CVID_MASK;

> -	if (vid > 1)

> -		reg[1] |= ATA2_IVL;

> +	reg[1] |= ATA2_IVL;

>  	reg[2] |= (aging & AGE_TIMER_MASK) << AGE_TIMER;

>  	reg[2] |= (port_mask & PORT_MAP_MASK) << PORT_MAP;

>  	/* STATIC_ENT indicate that entry is static wouldn't

> -- 

> 2.25.1

> 


Would you mind explaining what made VID 1 special in Eric's patch in the
first place?
Qingfang Deng Aug. 2, 2021, 3:48 p.m. UTC | #2
On Mon, Aug 02, 2021 at 04:44:09PM +0300, Vladimir Oltean wrote:
> Would you mind explaining what made VID 1 special in Eric's patch in the

> first place?


The default value of all ports' PVID is 1, which is copied into the FDB
entry, even if the ports are VLAN unaware. So running `bridge fdb show`
will show entries like `dev sw0p0 vlan 1 self` even on a VLAN-unaware
bridge.

Eric probably thought VID 1 is the FDB of all VLAN-unaware bridges, but
that is not true. And his patch probably cause a new issue that FDB is
inaccessible in a VLAN-**aware** bridge with PVID 1.

This series sets PVID to 0 on VLAN-unaware ports, so `bridge fdb show`
will no longer print `vlan 1` on VLAN-unaware bridges, and we don't
need special case in port_fdb_{add,del} for assisted learning.
Vladimir Oltean Aug. 2, 2021, 8:59 p.m. UTC | #3
On Mon, Aug 02, 2021 at 11:48:38PM +0800, DENG Qingfang wrote:
> On Mon, Aug 02, 2021 at 04:44:09PM +0300, Vladimir Oltean wrote:

> > Would you mind explaining what made VID 1 special in Eric's patch in the

> > first place?

>

> The default value of all ports' PVID is 1, which is copied into the FDB

> entry, even if the ports are VLAN unaware. So running `bridge fdb show`

> will show entries like `dev sw0p0 vlan 1 self` even on a VLAN-unaware

> bridge.

>

> Eric probably thought VID 1 is the FDB of all VLAN-unaware bridges, but

> that is not true. And his patch probably cause a new issue that FDB is

> inaccessible in a VLAN-**aware** bridge with PVID 1.

>

> This series sets PVID to 0 on VLAN-unaware ports, so `bridge fdb show`

> will no longer print `vlan 1` on VLAN-unaware bridges, and we don't

> need special case in port_fdb_{add,del} for assisted learning.


All things seriously worth mentioning in the commit message.
diff mbox series

Patch

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 38d6ce37d692..d72e04011cc5 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -366,8 +366,7 @@  mt7530_fdb_write(struct mt7530_priv *priv, u16 vid,
 	int i;
 
 	reg[1] |= vid & CVID_MASK;
-	if (vid > 1)
-		reg[1] |= ATA2_IVL;
+	reg[1] |= ATA2_IVL;
 	reg[2] |= (aging & AGE_TIMER_MASK) << AGE_TIMER;
 	reg[2] |= (port_mask & PORT_MAP_MASK) << PORT_MAP;
 	/* STATIC_ENT indicate that entry is static wouldn't