| Message ID | 20210803160405.3025624-5-dqfext@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mt7530 software fallback bridging fix | expand |
On Wed, Aug 04, 2021 at 12:04:04AM +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> > --- The way FDB entries are installed now makes a lot more intuitive sense.
On Tue, Aug 03, 2021 at 07:51:38PM +0300, Vladimir Oltean wrote: > > The way FDB entries are installed now makes a lot more intuitive sense. Did you forget to add the Reviewed-by tag?
On Wed, Aug 04, 2021 at 01:53:54AM +0800, DENG Qingfang wrote: > On Tue, Aug 03, 2021 at 07:51:38PM +0300, Vladimir Oltean wrote: > > > > The way FDB entries are installed now makes a lot more intuitive sense. > > Did you forget to add the Reviewed-by tag? Yeah, in fact I did. I was in a rush and by the time I figured that I didn't do that, I had already left home. Still, in fact the patch is obviously correct, which is nice. Standalone ports never learn and the FDB lookup will always miss because it's in FID 0 (a separate FID compared to the bridge ports) and there is no FDB entry there. Packets will always be flooded to their only possible destination, the CPU port. Ports under VLAN-unaware bridges always use IVL and learn using the pvid, which is zero, and which happily coincides with the VID which the bridge uses to install VLAN-unaware FDB entries. Ports under VLAN-aware bridges again use IVL but the VID is taken from the packet not just from the pvid, so the VID will be >= 1 there. Again it coincides with the vid that the bridge uses to offload FDB entries. Nice, I really like it, it is really simple. Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index 9b39ccd9dd4c..385e169080d9 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(FID_BRIDGED); 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 5b70ccef9459..4a91d80f51bb 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> --- v1 -> v2: use FID enum instead of hardcoding. drivers/net/dsa/mt7530.c | 4 ++-- drivers/net/dsa/mt7530.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)