| Message ID | 20210803124022.2912298-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, 17 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Tue, Aug 03, 2021 at 08:40:22PM +0800, DENG Qingfang wrote: > This reverts commit 7e777021780e ("mt7530 mt7530_fdb_write only set ivl > bit vid larger than 1"). > > Before this series, the default value of all ports' PVID is 1, which is > copied into the FDB entry, even if the ports are VLAN unaware. So > `bridge fdb show` will show entries like `dev swp0 vlan 1 self` even on > a VLAN-unaware bridge. > > The blamed commit does not solve that issue completely, instead it may > 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 that special > case in fdb_write is not required anymore. > > Set FDB entries' filter ID to 1 to match the VLAN table. > > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > --- > RFC -> v1: Detailed commit message. Also set FDB entries' FID to 1. > > drivers/net/dsa/mt7530.c | 4 ++-- > drivers/net/dsa/mt7530.h | 1 + > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > index 8d84d7ddad38..ac2b45e472bd 100644 > --- a/drivers/net/dsa/mt7530.c > +++ b/drivers/net/dsa/mt7530.c > @@ -366,8 +366,8 @@ 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[1] |= ATA2_FID(1); Maybe it would be good to resend with a set of #defines for the standalone/bridged port FID values after all, what do you think? > 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 > diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h > index 53b7bb1f5368..73b0e0eb8f2f 100644 > --- a/drivers/net/dsa/mt7530.h > +++ b/drivers/net/dsa/mt7530.h > @@ -80,6 +80,7 @@ enum mt753x_bpdu_port_fw { > #define STATIC_ENT 3 > #define MT7530_ATA2 0x78 > #define ATA2_IVL BIT(15) > +#define ATA2_FID(x) (((x) & 0x7) << 12) > > /* Register for address table write data */ > #define MT7530_ATWD 0x7c > -- > 2.25.1 >
On Tue, Aug 03, 2021 at 05:45:52PM +0300, Vladimir Oltean wrote: > > Maybe it would be good to resend with a set of #defines for the > standalone/bridged port FID values after all, what do you think? Will do.
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index 8d84d7ddad38..ac2b45e472bd 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -366,8 +366,8 @@ 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[1] |= ATA2_FID(1); 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 diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h index 53b7bb1f5368..73b0e0eb8f2f 100644 --- a/drivers/net/dsa/mt7530.h +++ b/drivers/net/dsa/mt7530.h @@ -80,6 +80,7 @@ enum mt753x_bpdu_port_fw { #define STATIC_ENT 3 #define MT7530_ATA2 0x78 #define ATA2_IVL BIT(15) +#define ATA2_FID(x) (((x) & 0x7) << 12) /* Register for address table write data */ #define MT7530_ATWD 0x7c
This reverts commit 7e777021780e ("mt7530 mt7530_fdb_write only set ivl bit vid larger than 1"). Before this series, the default value of all ports' PVID is 1, which is copied into the FDB entry, even if the ports are VLAN unaware. So `bridge fdb show` will show entries like `dev swp0 vlan 1 self` even on a VLAN-unaware bridge. The blamed commit does not solve that issue completely, instead it may 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 that special case in fdb_write is not required anymore. Set FDB entries' filter ID to 1 to match the VLAN table. Signed-off-by: DENG Qingfang <dqfext@gmail.com> --- RFC -> v1: Detailed commit message. Also set FDB entries' FID to 1. drivers/net/dsa/mt7530.c | 4 ++-- drivers/net/dsa/mt7530.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)