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 |
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 |
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?
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.
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 --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
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(-)