* [RFC net-next v2 0/4] mt7530 software fallback bridging fix
@ 2021-07-31 19:10 DENG Qingfang
2021-07-31 19:10 ` [RFC net-next v2 1/4] net: dsa: mt7530: enable assisted learning on CPU port DENG Qingfang
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: DENG Qingfang @ 2021-07-31 19:10 UTC (permalink / raw)
To: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, David S. Miller,
Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel,
linux-mediatek, linux-kernel
Cc: Eric Woudstra, René van Dorst, Frank Wunderlich
DSA core has gained software fallback support since commit 2f5dc00f7a3e,
but it does not work properly on mt7530. This patch series fixes the
issues.
DENG Qingfang (4):
net: dsa: mt7530: enable assisted learning on CPU port
net: dsa: mt7530: use independent VLAN learning on VLAN-unaware
bridges
net: dsa: mt7530: set STP state also on filter ID 1
Revert "mt7530 mt7530_fdb_write only set ivl bit vid larger than 1"
drivers/net/dsa/mt7530.c | 86 +++++++++++++++++++++++++++-------------
drivers/net/dsa/mt7530.h | 6 ++-
2 files changed, 63 insertions(+), 29 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 17+ messages in thread* [RFC net-next v2 1/4] net: dsa: mt7530: enable assisted learning on CPU port 2021-07-31 19:10 [RFC net-next v2 0/4] mt7530 software fallback bridging fix DENG Qingfang @ 2021-07-31 19:10 ` DENG Qingfang 2021-08-02 13:07 ` Vladimir Oltean 2021-07-31 19:10 ` [RFC net-next v2 2/4] net: dsa: mt7530: use independent VLAN learning on VLAN-unaware bridges DENG Qingfang ` (2 subsequent siblings) 3 siblings, 1 reply; 17+ messages in thread From: DENG Qingfang @ 2021-07-31 19:10 UTC (permalink / raw) To: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel Cc: Eric Woudstra, René van Dorst, Frank Wunderlich Consider the following bridge configuration, where bond0 is not offloaded: +-- br0 --+ / / | \ / / | \ / | | bond0 / | | / \ swp0 swp1 swp2 swp3 swp4 . . . . . . A B C Address learning is enabled on offloaded ports (swp0~2) and the CPU port, so when client A sends a packet to C, the following will happen: 1. The switch learns that client A can be reached at swp0. 2. The switch probably already knows that client C can be reached at the CPU port, so it forwards the packet to the CPU. 3. The bridge core knows client C can be reached at bond0, so it forwards the packet back to the switch. 4. The switch learns that client A can be reached at the CPU port. 5. The switch forwards the packet to either swp3 or swp4, according to the packet's tag. That makes client A's MAC address flap between swp0 and the CPU port. If client B sends a packet to A, it is possible that the packet is forwarded to the CPU. With offload_fwd_mark = 1, the bridge core won't forward it back to the switch, resulting in packet loss. As we have the assisted_learning_on_cpu_port in DSA core now, enable that and disable hardware learning on the CPU port. Signed-off-by: DENG Qingfang <dqfext@gmail.com> --- drivers/net/dsa/mt7530.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index 69f21b71614c..7e7e0a35e351 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -2054,6 +2054,7 @@ mt7530_setup(struct dsa_switch *ds) * as two netdev instances. */ dn = dsa_to_port(ds, MT7530_CPU_PORT)->master->dev.of_node->parent; + ds->assisted_learning_on_cpu_port = true; ds->mtu_enforcement_ingress = true; if (priv->id == ID_MT7530) { @@ -2124,15 +2125,15 @@ mt7530_setup(struct dsa_switch *ds) mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK, PCR_MATRIX_CLR); + /* Disable learning by default on all ports */ + mt7530_set(priv, MT7530_PSC_P(i), SA_DIS); + if (dsa_is_cpu_port(ds, i)) { ret = mt753x_cpu_port_enable(ds, i); if (ret) return ret; } else { mt7530_port_disable(ds, i); - - /* Disable learning by default on all user ports */ - mt7530_set(priv, MT7530_PSC_P(i), SA_DIS); } /* Enable consistent egress tag */ mt7530_rmw(priv, MT7530_PVC_P(i), PVC_EG_TAG_MASK, @@ -2289,6 +2290,9 @@ mt7531_setup(struct dsa_switch *ds) mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK, PCR_MATRIX_CLR); + /* Disable learning by default on all ports */ + mt7530_set(priv, MT7530_PSC_P(i), SA_DIS); + mt7530_set(priv, MT7531_DBG_CNT(i), MT7531_DIS_CLR); if (dsa_is_cpu_port(ds, i)) { @@ -2297,9 +2301,6 @@ mt7531_setup(struct dsa_switch *ds) return ret; } else { mt7530_port_disable(ds, i); - - /* Disable learning by default on all user ports */ - mt7530_set(priv, MT7530_PSC_P(i), SA_DIS); } /* Enable consistent egress tag */ @@ -2307,6 +2308,7 @@ mt7531_setup(struct dsa_switch *ds) PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT)); } + ds->assisted_learning_on_cpu_port = true; ds->mtu_enforcement_ingress = true; /* Flush the FDB table */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [RFC net-next v2 1/4] net: dsa: mt7530: enable assisted learning on CPU port 2021-07-31 19:10 ` [RFC net-next v2 1/4] net: dsa: mt7530: enable assisted learning on CPU port DENG Qingfang @ 2021-08-02 13:07 ` Vladimir Oltean 0 siblings, 0 replies; 17+ messages in thread From: Vladimir Oltean @ 2021-08-02 13:07 UTC (permalink / raw) To: DENG Qingfang Cc: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, Eric Woudstra, René van Dorst, Frank Wunderlich On Sun, Aug 01, 2021 at 03:10:19AM +0800, DENG Qingfang wrote: > Consider the following bridge configuration, where bond0 is not > offloaded: > > +-- br0 --+ > / / | \ > / / | \ > / | | bond0 > / | | / \ > swp0 swp1 swp2 swp3 swp4 > . . . > . . . > A B C > > Address learning is enabled on offloaded ports (swp0~2) and the CPU > port, so when client A sends a packet to C, the following will happen: > > 1. The switch learns that client A can be reached at swp0. > 2. The switch probably already knows that client C can be reached at the > CPU port, so it forwards the packet to the CPU. > 3. The bridge core knows client C can be reached at bond0, so it > forwards the packet back to the switch. > 4. The switch learns that client A can be reached at the CPU port. > 5. The switch forwards the packet to either swp3 or swp4, according to > the packet's tag. > > That makes client A's MAC address flap between swp0 and the CPU port. If > client B sends a packet to A, it is possible that the packet is > forwarded to the CPU. With offload_fwd_mark = 1, the bridge core won't > forward it back to the switch, resulting in packet loss. > > As we have the assisted_learning_on_cpu_port in DSA core now, enable > that and disable hardware learning on the CPU port. > > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > --- Reviewed-by: Vladimir Oltean <oltean@gmail.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC net-next v2 2/4] net: dsa: mt7530: use independent VLAN learning on VLAN-unaware bridges 2021-07-31 19:10 [RFC net-next v2 0/4] mt7530 software fallback bridging fix DENG Qingfang 2021-07-31 19:10 ` [RFC net-next v2 1/4] net: dsa: mt7530: enable assisted learning on CPU port DENG Qingfang @ 2021-07-31 19:10 ` DENG Qingfang 2021-08-02 13:36 ` Vladimir Oltean 2021-07-31 19:10 ` [RFC net-next v2 3/4] net: dsa: mt7530: set STP state also on filter ID 1 DENG Qingfang 2021-07-31 19:10 ` [RFC net-next v2 4/4] Revert "mt7530 mt7530_fdb_write only set ivl bit vid larger than 1" DENG Qingfang 3 siblings, 1 reply; 17+ messages in thread From: DENG Qingfang @ 2021-07-31 19:10 UTC (permalink / raw) To: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel Cc: Eric Woudstra, René van Dorst, Frank Wunderlich Consider the following bridge configuration, where bond0 is not offloaded: +-- br0 --+ / / | \ / / | \ / | | bond0 / | | / \ swp0 swp1 swp2 swp3 swp4 . . . . . . A B C Ideally, when the switch receives a packet from swp3 or swp4, it should forward the packet to the CPU, according to the port matrix and unknown unicast flood settings. But packet loss will happen if the destination address is at one of the offloaded ports (swp0~2). For example, when client C sends a packet to A, the FDB lookup will indicate that it should be forwarded to swp0, but the port matrix of swp3 and swp4 is configured to only allow the CPU to be its destination, so it is dropped. However, this issue does not happen if the bridge is VLAN-aware. That is because VLAN-aware bridges use independent VLAN learning, i.e. use VID for FDB lookup, on offloaded ports. As swp3 and swp4 are not offloaded, shared VLAN learning with default filter ID of 0 is used instead. So the lookup for A with filter ID 0 never hits and the packet can be forwarded to the CPU. In the current code, only two combinations were used to toggle user ports' VLAN awareness: one is PCR.PORT_VLAN set to port matrix mode with PVC.VLAN_ATTR set to transparent port, the other is PCR.PORT_VLAN set to security mode with PVC.VLAN_ATTR set to user port. It turns out that only PVC.VLAN_ATTR contributes to VLAN awareness, and port matrix mode just skips the VLAN table lookup. The reference manual is somehow misleading when describing PORT_VLAN modes. It states that PORT_MEM (VLAN port member) is used for destination if the VLAN table lookup hits, but actually **PORT_MEM & PORT_MATRIX** (bitwise AND of VLAN port member and port matrix) is used instead, which means we can have two or more separate VLAN-aware bridges with the same PVID and traffic won't leak between them. Therefore, to solve this, enable independent VLAN learning with PVID 0 on VLAN-unaware bridges, by setting their PCR.PORT_VLAN to fallback mode, while leaving standalone ports in port matrix mode. The CPU port is always set to fallback mode to serve those bridges. During testing, it is found that FDB lookup with filter ID of 0 will also hit entries with VID 0 even with independent VLAN learning. To avoid that, install all VLANs with filter ID of 1. Signed-off-by: DENG Qingfang <dqfext@gmail.com> --- drivers/net/dsa/mt7530.c | 70 ++++++++++++++++++++++++++++------------ drivers/net/dsa/mt7530.h | 4 ++- 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index 7e7e0a35e351..3876e265f844 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -1021,6 +1021,10 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port) mt7530_write(priv, MT7530_PCR_P(port), PCR_MATRIX(dsa_user_ports(priv->ds))); + /* Set to fallback mode for independent VLAN learning */ + mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK, + MT7530_PORT_FALLBACK_MODE); + return 0; } @@ -1229,6 +1233,10 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port, PCR_MATRIX_MASK, PCR_MATRIX(port_bitmap)); priv->ports[port].pm |= PCR_MATRIX(port_bitmap); + /* Set to fallback mode for independent VLAN learning */ + mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK, + MT7530_PORT_FALLBACK_MODE); + mutex_unlock(&priv->reg_mutex); return 0; @@ -1241,16 +1249,21 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port) bool all_user_ports_removed = true; int i; - /* When a port is removed from the bridge, the port would be set up - * back to the default as is at initial boot which is a VLAN-unaware - * port. + /* This is called after .port_bridge_leave when leaving a VLAN-aware + * bridge. Don't set standalone ports to fallback mode. */ - mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK, - MT7530_PORT_MATRIX_MODE); + if (dsa_to_port(ds, port)->bridge_dev) + mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK, + MT7530_PORT_FALLBACK_MODE); + mt7530_rmw(priv, MT7530_PVC_P(port), VLAN_ATTR_MASK | PVC_EG_TAG_MASK, VLAN_ATTR(MT7530_VLAN_TRANSPARENT) | PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT)); + /* Set PVID to 0 */ + mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK, + G0_PORT_VID_DEF); + for (i = 0; i < MT7530_NUM_PORTS; i++) { if (dsa_is_user_port(ds, i) && dsa_port_is_vlan_filtering(dsa_to_port(ds, i))) { @@ -1276,15 +1289,14 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port) struct mt7530_priv *priv = ds->priv; /* Trapped into security mode allows packet forwarding through VLAN - * table lookup. CPU port is set to fallback mode to let untagged - * frames pass through. + * table lookup. */ - if (dsa_is_cpu_port(ds, port)) - mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK, - MT7530_PORT_FALLBACK_MODE); - else + if (dsa_is_user_port(ds, port)) { mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK, MT7530_PORT_SECURITY_MODE); + mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK, + G0_PORT_VID(priv->ports[port].pvid)); + } /* Set the port as a user port which is to be able to recognize VID * from incoming packets before fetching entry within the VLAN table. @@ -1329,6 +1341,13 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port, PCR_MATRIX(BIT(MT7530_CPU_PORT))); priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT)); + /* When a port is removed from the bridge, the port would be set up + * back to the default as is at initial boot which is a VLAN-unaware + * port. + */ + mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK, + MT7530_PORT_MATRIX_MODE); + mutex_unlock(&priv->reg_mutex); } @@ -1511,7 +1530,7 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv, /* Validate the entry with independent learning, create egress tag per * VLAN and joining the port as one of the port members. */ - val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) | VLAN_VALID; + val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) | FID(1) | VLAN_VALID; mt7530_write(priv, MT7530_VAWD1, val); /* Decide whether adding tag or not for those outgoing packets from the @@ -1601,9 +1620,13 @@ mt7530_port_vlan_add(struct dsa_switch *ds, int port, mt7530_hw_vlan_update(priv, vlan->vid, &new_entry, mt7530_hw_vlan_add); if (pvid) { - mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK, - G0_PORT_VID(vlan->vid)); priv->ports[port].pvid = vlan->vid; + + /* Only configure PVID if VLAN filtering is enabled */ + if (dsa_port_is_vlan_filtering(dsa_to_port(ds, port))) + mt7530_rmw(priv, MT7530_PPBV1_P(port), + G0_PORT_VID_MASK, + G0_PORT_VID(vlan->vid)); } mutex_unlock(&priv->reg_mutex); @@ -1617,11 +1640,9 @@ mt7530_port_vlan_del(struct dsa_switch *ds, int port, { struct mt7530_hw_vlan_entry target_entry; struct mt7530_priv *priv = ds->priv; - u16 pvid; mutex_lock(&priv->reg_mutex); - pvid = priv->ports[port].pvid; mt7530_hw_vlan_entry_init(&target_entry, port, 0); mt7530_hw_vlan_update(priv, vlan->vid, &target_entry, mt7530_hw_vlan_del); @@ -1629,11 +1650,12 @@ mt7530_port_vlan_del(struct dsa_switch *ds, int port, /* PVID is being restored to the default whenever the PVID port * is being removed from the VLAN. */ - if (pvid == vlan->vid) - pvid = G0_PORT_VID_DEF; + if (priv->ports[port].pvid == vlan->vid) { + priv->ports[port].pvid = G0_PORT_VID_DEF; + mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK, + G0_PORT_VID_DEF); + } - mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK, pvid); - priv->ports[port].pvid = pvid; mutex_unlock(&priv->reg_mutex); @@ -2134,6 +2156,10 @@ mt7530_setup(struct dsa_switch *ds) return ret; } else { mt7530_port_disable(ds, i); + + /* Set default PVID to 0 on all user ports */ + mt7530_rmw(priv, MT7530_PPBV1_P(i), G0_PORT_VID_MASK, + G0_PORT_VID_DEF); } /* Enable consistent egress tag */ mt7530_rmw(priv, MT7530_PVC_P(i), PVC_EG_TAG_MASK, @@ -2301,6 +2327,10 @@ mt7531_setup(struct dsa_switch *ds) return ret; } else { mt7530_port_disable(ds, i); + + /* Set default PVID to 0 on all user ports */ + mt7530_rmw(priv, MT7530_PPBV1_P(i), G0_PORT_VID_MASK, + G0_PORT_VID_DEF); } /* Enable consistent egress tag */ diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h index b19b389ff10a..a308886fdebc 100644 --- a/drivers/net/dsa/mt7530.h +++ b/drivers/net/dsa/mt7530.h @@ -148,6 +148,8 @@ enum mt7530_vlan_cmd { #define VTAG_EN BIT(28) /* VLAN Member Control */ #define PORT_MEM(x) (((x) & 0xff) << 16) +/* Filter ID */ +#define FID(x) (((x) & 0x7) << 1) /* VLAN Entry Valid */ #define VLAN_VALID BIT(0) #define PORT_MEM_SHFT 16 @@ -247,7 +249,7 @@ enum mt7530_vlan_port_attr { #define MT7530_PPBV1_P(x) (0x2014 + ((x) * 0x100)) #define G0_PORT_VID(x) (((x) & 0xfff) << 0) #define G0_PORT_VID_MASK G0_PORT_VID(0xfff) -#define G0_PORT_VID_DEF G0_PORT_VID(1) +#define G0_PORT_VID_DEF G0_PORT_VID(0) /* Register for port MAC control register */ #define MT7530_PMCR_P(x) (0x3000 + ((x) * 0x100)) -- 2.25.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [RFC net-next v2 2/4] net: dsa: mt7530: use independent VLAN learning on VLAN-unaware bridges 2021-07-31 19:10 ` [RFC net-next v2 2/4] net: dsa: mt7530: use independent VLAN learning on VLAN-unaware bridges DENG Qingfang @ 2021-08-02 13:36 ` Vladimir Oltean 0 siblings, 0 replies; 17+ messages in thread From: Vladimir Oltean @ 2021-08-02 13:36 UTC (permalink / raw) To: DENG Qingfang Cc: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, Eric Woudstra, René van Dorst, Frank Wunderlich On Sun, Aug 01, 2021 at 03:10:20AM +0800, DENG Qingfang wrote: > Consider the following bridge configuration, where bond0 is not > offloaded: > > +-- br0 --+ > / / | \ > / / | \ > / | | bond0 > / | | / \ > swp0 swp1 swp2 swp3 swp4 > . . . > . . . > A B C > > Ideally, when the switch receives a packet from swp3 or swp4, it should > forward the packet to the CPU, according to the port matrix and unknown > unicast flood settings. > > But packet loss will happen if the destination address is at one of the > offloaded ports (swp0~2). For example, when client C sends a packet to > A, the FDB lookup will indicate that it should be forwarded to swp0, but > the port matrix of swp3 and swp4 is configured to only allow the CPU to > be its destination, so it is dropped. > > However, this issue does not happen if the bridge is VLAN-aware. That is > because VLAN-aware bridges use independent VLAN learning, i.e. use VID > for FDB lookup, on offloaded ports. As swp3 and swp4 are not offloaded, > shared VLAN learning with default filter ID of 0 is used instead. So the > lookup for A with filter ID 0 never hits and the packet can be forwarded > to the CPU. > > In the current code, only two combinations were used to toggle user > ports' VLAN awareness: one is PCR.PORT_VLAN set to port matrix mode with > PVC.VLAN_ATTR set to transparent port, the other is PCR.PORT_VLAN set to > security mode with PVC.VLAN_ATTR set to user port. > > It turns out that only PVC.VLAN_ATTR contributes to VLAN awareness, and > port matrix mode just skips the VLAN table lookup. The reference manual > is somehow misleading when describing PORT_VLAN modes. It states that > PORT_MEM (VLAN port member) is used for destination if the VLAN table > lookup hits, but actually **PORT_MEM & PORT_MATRIX** (bitwise AND of > VLAN port member and port matrix) is used instead, which means we can > have two or more separate VLAN-aware bridges with the same PVID and > traffic won't leak between them. "traffic won't leak" is only half the story. It won't leak, but when multiple VLAN-unaware bridges share the same FID, they are still subject to shortcircuit attempts if the same MAC address is present in the FDB of both bridges. This patch doesn't solve that, maybe it is worth adding a big comment in the code itself that clarifies that FDBs between multiple VLAN-unaware bridges, as well as between multiple VLAN-aware bridges, are not isolated per se, only that standalone ports are placed under a different FID compared to bridges of whatever kind. > > Therefore, to solve this, enable independent VLAN learning with PVID 0 > on VLAN-unaware bridges, by setting their PCR.PORT_VLAN to fallback > mode, while leaving standalone ports in port matrix mode. The CPU port > is always set to fallback mode to serve those bridges. > > During testing, it is found that FDB lookup with filter ID of 0 will > also hit entries with VID 0 even with independent VLAN learning. To > avoid that, install all VLANs with filter ID of 1. > > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > --- > drivers/net/dsa/mt7530.c | 70 ++++++++++++++++++++++++++++------------ > drivers/net/dsa/mt7530.h | 4 ++- > 2 files changed, 53 insertions(+), 21 deletions(-) > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > index 7e7e0a35e351..3876e265f844 100644 > --- a/drivers/net/dsa/mt7530.c > +++ b/drivers/net/dsa/mt7530.c > @@ -1021,6 +1021,10 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port) > mt7530_write(priv, MT7530_PCR_P(port), > PCR_MATRIX(dsa_user_ports(priv->ds))); > > + /* Set to fallback mode for independent VLAN learning */ > + mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK, > + MT7530_PORT_FALLBACK_MODE); > + > return 0; > } > > @@ -1229,6 +1233,10 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port, > PCR_MATRIX_MASK, PCR_MATRIX(port_bitmap)); > priv->ports[port].pm |= PCR_MATRIX(port_bitmap); > > + /* Set to fallback mode for independent VLAN learning */ > + mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK, > + MT7530_PORT_FALLBACK_MODE); > + > mutex_unlock(&priv->reg_mutex); > > return 0; > @@ -1241,16 +1249,21 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port) > bool all_user_ports_removed = true; > int i; > > - /* When a port is removed from the bridge, the port would be set up > - * back to the default as is at initial boot which is a VLAN-unaware > - * port. > + /* This is called after .port_bridge_leave when leaving a VLAN-aware > + * bridge. Don't set standalone ports to fallback mode. > */ > - mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK, > - MT7530_PORT_MATRIX_MODE); > + if (dsa_to_port(ds, port)->bridge_dev) > + mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK, > + MT7530_PORT_FALLBACK_MODE); > + hmm, okay, the ordering between dsa_broadcast() and dsa_port_switchdev_unsync_attrs() in dsa_port_bridge_leave() is a bit awkward for you, but this looks okay. > mt7530_rmw(priv, MT7530_PVC_P(port), VLAN_ATTR_MASK | PVC_EG_TAG_MASK, > VLAN_ATTR(MT7530_VLAN_TRANSPARENT) | > PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT)); > > + /* Set PVID to 0 */ > + mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK, > + G0_PORT_VID_DEF); > + > for (i = 0; i < MT7530_NUM_PORTS; i++) { > if (dsa_is_user_port(ds, i) && > dsa_port_is_vlan_filtering(dsa_to_port(ds, i))) { > @@ -1276,15 +1289,14 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port) > struct mt7530_priv *priv = ds->priv; > > /* Trapped into security mode allows packet forwarding through VLAN > - * table lookup. CPU port is set to fallback mode to let untagged > - * frames pass through. > + * table lookup. > */ > - if (dsa_is_cpu_port(ds, port)) > - mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK, > - MT7530_PORT_FALLBACK_MODE); > - else > + if (dsa_is_user_port(ds, port)) { > mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK, > MT7530_PORT_SECURITY_MODE); > + mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK, > + G0_PORT_VID(priv->ports[port].pvid)); > + } > > /* Set the port as a user port which is to be able to recognize VID > * from incoming packets before fetching entry within the VLAN table. > @@ -1329,6 +1341,13 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port, > PCR_MATRIX(BIT(MT7530_CPU_PORT))); > priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT)); > > + /* When a port is removed from the bridge, the port would be set up > + * back to the default as is at initial boot which is a VLAN-unaware > + * port. > + */ > + mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK, > + MT7530_PORT_MATRIX_MODE); > + > mutex_unlock(&priv->reg_mutex); > } > > @@ -1511,7 +1530,7 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv, > /* Validate the entry with independent learning, create egress tag per > * VLAN and joining the port as one of the port members. > */ > - val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) | VLAN_VALID; > + val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) | FID(1) | VLAN_VALID; > mt7530_write(priv, MT7530_VAWD1, val); > > /* Decide whether adding tag or not for those outgoing packets from the > @@ -1601,9 +1620,13 @@ mt7530_port_vlan_add(struct dsa_switch *ds, int port, > mt7530_hw_vlan_update(priv, vlan->vid, &new_entry, mt7530_hw_vlan_add); > > if (pvid) { > - mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK, > - G0_PORT_VID(vlan->vid)); > priv->ports[port].pvid = vlan->vid; > + > + /* Only configure PVID if VLAN filtering is enabled */ > + if (dsa_port_is_vlan_filtering(dsa_to_port(ds, port))) > + mt7530_rmw(priv, MT7530_PPBV1_P(port), > + G0_PORT_VID_MASK, > + G0_PORT_VID(vlan->vid)); > } > > mutex_unlock(&priv->reg_mutex); > @@ -1617,11 +1640,9 @@ mt7530_port_vlan_del(struct dsa_switch *ds, int port, > { > struct mt7530_hw_vlan_entry target_entry; > struct mt7530_priv *priv = ds->priv; > - u16 pvid; > > mutex_lock(&priv->reg_mutex); > > - pvid = priv->ports[port].pvid; > mt7530_hw_vlan_entry_init(&target_entry, port, 0); > mt7530_hw_vlan_update(priv, vlan->vid, &target_entry, > mt7530_hw_vlan_del); > @@ -1629,11 +1650,12 @@ mt7530_port_vlan_del(struct dsa_switch *ds, int port, > /* PVID is being restored to the default whenever the PVID port > * is being removed from the VLAN. > */ > - if (pvid == vlan->vid) > - pvid = G0_PORT_VID_DEF; > + if (priv->ports[port].pvid == vlan->vid) { > + priv->ports[port].pvid = G0_PORT_VID_DEF; > + mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK, > + G0_PORT_VID_DEF); > + } > > - mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK, pvid); > - priv->ports[port].pvid = pvid; > > mutex_unlock(&priv->reg_mutex); > > @@ -2134,6 +2156,10 @@ mt7530_setup(struct dsa_switch *ds) > return ret; > } else { > mt7530_port_disable(ds, i); > + > + /* Set default PVID to 0 on all user ports */ > + mt7530_rmw(priv, MT7530_PPBV1_P(i), G0_PORT_VID_MASK, > + G0_PORT_VID_DEF); > } > /* Enable consistent egress tag */ > mt7530_rmw(priv, MT7530_PVC_P(i), PVC_EG_TAG_MASK, > @@ -2301,6 +2327,10 @@ mt7531_setup(struct dsa_switch *ds) > return ret; > } else { > mt7530_port_disable(ds, i); > + > + /* Set default PVID to 0 on all user ports */ > + mt7530_rmw(priv, MT7530_PPBV1_P(i), G0_PORT_VID_MASK, > + G0_PORT_VID_DEF); > } > > /* Enable consistent egress tag */ > diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h > index b19b389ff10a..a308886fdebc 100644 > --- a/drivers/net/dsa/mt7530.h > +++ b/drivers/net/dsa/mt7530.h > @@ -148,6 +148,8 @@ enum mt7530_vlan_cmd { > #define VTAG_EN BIT(28) > /* VLAN Member Control */ > #define PORT_MEM(x) (((x) & 0xff) << 16) > +/* Filter ID */ > +#define FID(x) (((x) & 0x7) << 1) > /* VLAN Entry Valid */ > #define VLAN_VALID BIT(0) > #define PORT_MEM_SHFT 16 > @@ -247,7 +249,7 @@ enum mt7530_vlan_port_attr { > #define MT7530_PPBV1_P(x) (0x2014 + ((x) * 0x100)) > #define G0_PORT_VID(x) (((x) & 0xfff) << 0) > #define G0_PORT_VID_MASK G0_PORT_VID(0xfff) > -#define G0_PORT_VID_DEF G0_PORT_VID(1) > +#define G0_PORT_VID_DEF G0_PORT_VID(0) > > /* Register for port MAC control register */ > #define MT7530_PMCR_P(x) (0x3000 + ((x) * 0x100)) > -- > 2.25.1 > Reviewed-by: Vladimir Oltean <olteanv@gmail.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC net-next v2 3/4] net: dsa: mt7530: set STP state also on filter ID 1 2021-07-31 19:10 [RFC net-next v2 0/4] mt7530 software fallback bridging fix DENG Qingfang 2021-07-31 19:10 ` [RFC net-next v2 1/4] net: dsa: mt7530: enable assisted learning on CPU port DENG Qingfang 2021-07-31 19:10 ` [RFC net-next v2 2/4] net: dsa: mt7530: use independent VLAN learning on VLAN-unaware bridges DENG Qingfang @ 2021-07-31 19:10 ` DENG Qingfang 2021-08-02 13:43 ` Vladimir Oltean 2021-07-31 19:10 ` [RFC net-next v2 4/4] Revert "mt7530 mt7530_fdb_write only set ivl bit vid larger than 1" DENG Qingfang 3 siblings, 1 reply; 17+ messages in thread From: DENG Qingfang @ 2021-07-31 19:10 UTC (permalink / raw) To: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel Cc: Eric Woudstra, René van Dorst, Frank Wunderlich As filter ID 1 is used, set STP state also on it. Signed-off-by: DENG Qingfang <dqfext@gmail.com> --- drivers/net/dsa/mt7530.c | 3 ++- drivers/net/dsa/mt7530.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index 3876e265f844..38d6ce37d692 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -1147,7 +1147,8 @@ mt7530_stp_state_set(struct dsa_switch *ds, int port, u8 state) break; } - mt7530_rmw(priv, MT7530_SSP_P(port), FID_PST_MASK, stp_state); + mt7530_rmw(priv, MT7530_SSP_P(port), FID_PST_MASK, + FID_PST(stp_state)); } static int diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h index a308886fdebc..294ff1cbd9e0 100644 --- a/drivers/net/dsa/mt7530.h +++ b/drivers/net/dsa/mt7530.h @@ -181,7 +181,7 @@ enum mt7530_vlan_egress_attr { /* Register for port STP state control */ #define MT7530_SSP_P(x) (0x2000 + ((x) * 0x100)) -#define FID_PST(x) ((x) & 0x3) +#define FID_PST(x) (((x) & 0x3) * 0x5) #define FID_PST_MASK FID_PST(0x3) enum mt7530_stp_state { -- 2.25.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [RFC net-next v2 3/4] net: dsa: mt7530: set STP state also on filter ID 1 2021-07-31 19:10 ` [RFC net-next v2 3/4] net: dsa: mt7530: set STP state also on filter ID 1 DENG Qingfang @ 2021-08-02 13:43 ` Vladimir Oltean 2021-08-02 15:31 ` DENG Qingfang 0 siblings, 1 reply; 17+ messages in thread From: Vladimir Oltean @ 2021-08-02 13:43 UTC (permalink / raw) To: DENG Qingfang Cc: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, Eric Woudstra, René van Dorst, Frank Wunderlich On Sun, Aug 01, 2021 at 03:10:21AM +0800, DENG Qingfang wrote: > As filter ID 1 is used, set STP state also on it. > > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > --- > drivers/net/dsa/mt7530.c | 3 ++- > drivers/net/dsa/mt7530.h | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > index 3876e265f844..38d6ce37d692 100644 > --- a/drivers/net/dsa/mt7530.c > +++ b/drivers/net/dsa/mt7530.c > @@ -1147,7 +1147,8 @@ mt7530_stp_state_set(struct dsa_switch *ds, int port, u8 state) > break; > } > > - mt7530_rmw(priv, MT7530_SSP_P(port), FID_PST_MASK, stp_state); > + mt7530_rmw(priv, MT7530_SSP_P(port), FID_PST_MASK, > + FID_PST(stp_state)); > } > > static int > diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h > index a308886fdebc..294ff1cbd9e0 100644 > --- a/drivers/net/dsa/mt7530.h > +++ b/drivers/net/dsa/mt7530.h > @@ -181,7 +181,7 @@ enum mt7530_vlan_egress_attr { > > /* Register for port STP state control */ > #define MT7530_SSP_P(x) (0x2000 + ((x) * 0x100)) > -#define FID_PST(x) ((x) & 0x3) Shouldn't these macros have _two_ arguments, the FID and the port state? > +#define FID_PST(x) (((x) & 0x3) * 0x5) "* 5": explanation? > #define FID_PST_MASK FID_PST(0x3) > > enum mt7530_stp_state { > -- > 2.25.1 > I don't exactly understand how this patch works, sorry. Are you altering port state only on bridged ports, or also on standalone ports after this patch? Are standalone ports in the proper STP state (FORWARDING)? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC net-next v2 3/4] net: dsa: mt7530: set STP state also on filter ID 1 2021-08-02 13:43 ` Vladimir Oltean @ 2021-08-02 15:31 ` DENG Qingfang 2021-08-02 15:42 ` Vladimir Oltean 0 siblings, 1 reply; 17+ messages in thread From: DENG Qingfang @ 2021-08-02 15:31 UTC (permalink / raw) To: Vladimir Oltean Cc: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, Eric Woudstra, René van Dorst, Frank Wunderlich On Mon, Aug 02, 2021 at 04:43:36PM +0300, Vladimir Oltean wrote: > On Sun, Aug 01, 2021 at 03:10:21AM +0800, DENG Qingfang wrote: > > --- a/drivers/net/dsa/mt7530.h > > +++ b/drivers/net/dsa/mt7530.h > > @@ -181,7 +181,7 @@ enum mt7530_vlan_egress_attr { > > > > /* Register for port STP state control */ > > #define MT7530_SSP_P(x) (0x2000 + ((x) * 0x100)) > > -#define FID_PST(x) ((x) & 0x3) > > Shouldn't these macros have _two_ arguments, the FID and the port state? > > > +#define FID_PST(x) (((x) & 0x3) * 0x5) > > "* 5": explanation? > > > #define FID_PST_MASK FID_PST(0x3) > > > > enum mt7530_stp_state { > > -- > > 2.25.1 > > > > I don't exactly understand how this patch works, sorry. > Are you altering port state only on bridged ports, or also on standalone > ports after this patch? Are standalone ports in the proper STP state > (FORWARDING)? The current code only sets FID 0's STP state. This patch sets both 0's and 1's states. The *5 part is binary magic. [1:0] is FID 0's state, [3:2] is FID 1's state and so on. Since 5 == 4'b0101, the value in [1:0] is copied to [3:2] after the multiplication. Perhaps I should only change FID 1's state. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC net-next v2 3/4] net: dsa: mt7530: set STP state also on filter ID 1 2021-08-02 15:31 ` DENG Qingfang @ 2021-08-02 15:42 ` Vladimir Oltean 2021-08-02 15:58 ` DENG Qingfang 0 siblings, 1 reply; 17+ messages in thread From: Vladimir Oltean @ 2021-08-02 15:42 UTC (permalink / raw) To: DENG Qingfang Cc: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, Eric Woudstra, René van Dorst, Frank Wunderlich On Mon, Aug 02, 2021 at 11:31:29PM +0800, DENG Qingfang wrote: > On Mon, Aug 02, 2021 at 04:43:36PM +0300, Vladimir Oltean wrote: > > On Sun, Aug 01, 2021 at 03:10:21AM +0800, DENG Qingfang wrote: > > > --- a/drivers/net/dsa/mt7530.h > > > +++ b/drivers/net/dsa/mt7530.h > > > @@ -181,7 +181,7 @@ enum mt7530_vlan_egress_attr { > > > > > > /* Register for port STP state control */ > > > #define MT7530_SSP_P(x) (0x2000 + ((x) * 0x100)) > > > -#define FID_PST(x) ((x) & 0x3) > > > > Shouldn't these macros have _two_ arguments, the FID and the port state? > > > > > +#define FID_PST(x) (((x) & 0x3) * 0x5) > > > > "* 5": explanation? > > > > > #define FID_PST_MASK FID_PST(0x3) > > > > > > enum mt7530_stp_state { > > > -- > > > 2.25.1 > > > > > > > I don't exactly understand how this patch works, sorry. > > Are you altering port state only on bridged ports, or also on standalone > > ports after this patch? Are standalone ports in the proper STP state > > (FORWARDING)? > > The current code only sets FID 0's STP state. This patch sets both 0's and > 1's states. > > The *5 part is binary magic. [1:0] is FID 0's state, [3:2] is FID 1's state > and so on. Since 5 == 4'b0101, the value in [1:0] is copied to [3:2] after > the multiplication. > > Perhaps I should only change FID 1's state. Keep the patches dumb for us mortals please. If you only change FID 1's state, I am concerned that the driver no longer initializes FID 0's port state, and might leave that to the default set by other pre-kernel initialization stage (bootloader?). So even if you might assume that standalone ports are FORWARDING, they might not be. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC net-next v2 3/4] net: dsa: mt7530: set STP state also on filter ID 1 2021-08-02 15:42 ` Vladimir Oltean @ 2021-08-02 15:58 ` DENG Qingfang 2021-08-02 21:00 ` Vladimir Oltean 0 siblings, 1 reply; 17+ messages in thread From: DENG Qingfang @ 2021-08-02 15:58 UTC (permalink / raw) To: Vladimir Oltean Cc: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, Eric Woudstra, René van Dorst, Frank Wunderlich On Mon, Aug 02, 2021 at 06:42:26PM +0300, Vladimir Oltean wrote: > On Mon, Aug 02, 2021 at 11:31:29PM +0800, DENG Qingfang wrote: > > The current code only sets FID 0's STP state. This patch sets both 0's and > > 1's states. > > > > The *5 part is binary magic. [1:0] is FID 0's state, [3:2] is FID 1's state > > and so on. Since 5 == 4'b0101, the value in [1:0] is copied to [3:2] after > > the multiplication. > > > > Perhaps I should only change FID 1's state. > > Keep the patches dumb for us mortals please. > If you only change FID 1's state, I am concerned that the driver no > longer initializes FID 0's port state, and might leave that to the > default set by other pre-kernel initialization stage (bootloader?). > So even if you might assume that standalone ports are FORWARDING, they > might not be. The default value is forwarding, and the switch is reset by the driver so any pre-kernel initialization stage is no more. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC net-next v2 3/4] net: dsa: mt7530: set STP state also on filter ID 1 2021-08-02 15:58 ` DENG Qingfang @ 2021-08-02 21:00 ` Vladimir Oltean 2021-08-03 8:23 ` DENG Qingfang 0 siblings, 1 reply; 17+ messages in thread From: Vladimir Oltean @ 2021-08-02 21:00 UTC (permalink / raw) To: DENG Qingfang Cc: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, Eric Woudstra, René van Dorst, Frank Wunderlich On Mon, Aug 02, 2021 at 11:58:10PM +0800, DENG Qingfang wrote: > On Mon, Aug 02, 2021 at 06:42:26PM +0300, Vladimir Oltean wrote: > > On Mon, Aug 02, 2021 at 11:31:29PM +0800, DENG Qingfang wrote: > > > The current code only sets FID 0's STP state. This patch sets both 0's and > > > 1's states. > > > > > > The *5 part is binary magic. [1:0] is FID 0's state, [3:2] is FID 1's state > > > and so on. Since 5 == 4'b0101, the value in [1:0] is copied to [3:2] after > > > the multiplication. > > > > > > Perhaps I should only change FID 1's state. > > > > Keep the patches dumb for us mortals please. > > If you only change FID 1's state, I am concerned that the driver no > > longer initializes FID 0's port state, and might leave that to the > > default set by other pre-kernel initialization stage (bootloader?). > > So even if you might assume that standalone ports are FORWARDING, they > > might not be. > > The default value is forwarding, and the switch is reset by the driver > so any pre-kernel initialization stage is no more. So then change the port STP state only for FID 1 and resend. Any other reason why this patch series is marked RFC? It looked okay to me otherwise. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC net-next v2 3/4] net: dsa: mt7530: set STP state also on filter ID 1 2021-08-02 21:00 ` Vladimir Oltean @ 2021-08-03 8:23 ` DENG Qingfang 2021-08-03 8:48 ` Vladimir Oltean 0 siblings, 1 reply; 17+ messages in thread From: DENG Qingfang @ 2021-08-03 8:23 UTC (permalink / raw) To: Vladimir Oltean Cc: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, Eric Woudstra, René van Dorst, Frank Wunderlich On Tue, Aug 03, 2021 at 12:00:06AM +0300, Vladimir Oltean wrote: > > So then change the port STP state only for FID 1 and resend. Any other > reason why this patch series is marked RFC? It looked okay to me otherwise. Okay, will resend with that change and without RFC. By the way, if I were to implement .port_fast_age, should I only flush dynamically learned FDB entries? What about MDB entries? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC net-next v2 3/4] net: dsa: mt7530: set STP state also on filter ID 1 2021-08-03 8:23 ` DENG Qingfang @ 2021-08-03 8:48 ` Vladimir Oltean 0 siblings, 0 replies; 17+ messages in thread From: Vladimir Oltean @ 2021-08-03 8:48 UTC (permalink / raw) To: DENG Qingfang Cc: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, Eric Woudstra, René van Dorst, Frank Wunderlich On Tue, Aug 03, 2021 at 04:23:16PM +0800, DENG Qingfang wrote: > On Tue, Aug 03, 2021 at 12:00:06AM +0300, Vladimir Oltean wrote: > > > > So then change the port STP state only for FID 1 and resend. Any other > > reason why this patch series is marked RFC? It looked okay to me otherwise. > > Okay, will resend with that change and without RFC. > > By the way, if I were to implement .port_fast_age, should I only flush > dynamically learned FDB entries? What about MDB entries? Yes, only dynamically learned entries. That should also answer the question about MDB entries, since a multicast address should never be a source MAC address so they should never be dynamically learned. The bridge should handle the deletion of static entries when needed. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC net-next v2 4/4] Revert "mt7530 mt7530_fdb_write only set ivl bit vid larger than 1" 2021-07-31 19:10 [RFC net-next v2 0/4] mt7530 software fallback bridging fix DENG Qingfang ` (2 preceding siblings ...) 2021-07-31 19:10 ` [RFC net-next v2 3/4] net: dsa: mt7530: set STP state also on filter ID 1 DENG Qingfang @ 2021-07-31 19:10 ` DENG Qingfang 2021-08-02 13:44 ` Vladimir Oltean 3 siblings, 1 reply; 17+ messages in thread From: DENG Qingfang @ 2021-07-31 19:10 UTC (permalink / raw) To: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel Cc: Eric Woudstra, René van Dorst, Frank Wunderlich 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [RFC net-next v2 4/4] Revert "mt7530 mt7530_fdb_write only set ivl bit vid larger than 1" 2021-07-31 19:10 ` [RFC net-next v2 4/4] Revert "mt7530 mt7530_fdb_write only set ivl bit vid larger than 1" DENG Qingfang @ 2021-08-02 13:44 ` Vladimir Oltean 2021-08-02 15:48 ` DENG Qingfang 0 siblings, 1 reply; 17+ messages in thread From: Vladimir Oltean @ 2021-08-02 13:44 UTC (permalink / raw) To: DENG Qingfang Cc: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, Eric Woudstra, René van Dorst, Frank Wunderlich 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? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC net-next v2 4/4] Revert "mt7530 mt7530_fdb_write only set ivl bit vid larger than 1" 2021-08-02 13:44 ` Vladimir Oltean @ 2021-08-02 15:48 ` DENG Qingfang 2021-08-02 20:59 ` Vladimir Oltean 0 siblings, 1 reply; 17+ messages in thread From: DENG Qingfang @ 2021-08-02 15:48 UTC (permalink / raw) To: Vladimir Oltean Cc: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, Eric Woudstra, René van Dorst, Frank Wunderlich 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC net-next v2 4/4] Revert "mt7530 mt7530_fdb_write only set ivl bit vid larger than 1" 2021-08-02 15:48 ` DENG Qingfang @ 2021-08-02 20:59 ` Vladimir Oltean 0 siblings, 0 replies; 17+ messages in thread From: Vladimir Oltean @ 2021-08-02 20:59 UTC (permalink / raw) To: DENG Qingfang Cc: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel, linux-mediatek, linux-kernel, Eric Woudstra, René van Dorst, Frank Wunderlich 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-08-03 8:49 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-31 19:10 [RFC net-next v2 0/4] mt7530 software fallback bridging fix DENG Qingfang 2021-07-31 19:10 ` [RFC net-next v2 1/4] net: dsa: mt7530: enable assisted learning on CPU port DENG Qingfang 2021-08-02 13:07 ` Vladimir Oltean 2021-07-31 19:10 ` [RFC net-next v2 2/4] net: dsa: mt7530: use independent VLAN learning on VLAN-unaware bridges DENG Qingfang 2021-08-02 13:36 ` Vladimir Oltean 2021-07-31 19:10 ` [RFC net-next v2 3/4] net: dsa: mt7530: set STP state also on filter ID 1 DENG Qingfang 2021-08-02 13:43 ` Vladimir Oltean 2021-08-02 15:31 ` DENG Qingfang 2021-08-02 15:42 ` Vladimir Oltean 2021-08-02 15:58 ` DENG Qingfang 2021-08-02 21:00 ` Vladimir Oltean 2021-08-03 8:23 ` DENG Qingfang 2021-08-03 8:48 ` Vladimir Oltean 2021-07-31 19:10 ` [RFC net-next v2 4/4] Revert "mt7530 mt7530_fdb_write only set ivl bit vid larger than 1" DENG Qingfang 2021-08-02 13:44 ` Vladimir Oltean 2021-08-02 15:48 ` DENG Qingfang 2021-08-02 20:59 ` Vladimir Oltean
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).