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 (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series mt7530 software fallback bridging fix | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 13 of 13 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 9 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

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