DSA core has gained software fallback support since commit 2f5dc00f7a3e
("net: bridge: switchdev: let drivers inform which bridge ports are
offloaded"), 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 on filter ID 1
net: dsa: mt7530: always install FDB entries with IVL and FID 1
drivers/net/dsa/mt7530.c | 88 ++++++++++++++++++++++++++++------------
drivers/net/dsa/mt7530.h | 14 +++++--
2 files changed, 72 insertions(+), 30 deletions(-)
--
2.25.1
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 <[email protected]>
---
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(-)
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
--
2.25.1
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 <[email protected]>
Reviewed-by: Vladimir Oltean <[email protected]>
---
v1 -> v2: no changes.
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 b6e0b347947e..abe57b04fc39 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2046,6 +2046,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) {
@@ -2116,15 +2117,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,
@@ -2281,6 +2282,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)) {
@@ -2289,9 +2293,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 */
@@ -2299,6 +2300,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
As filter ID 1 is the only one used for bridges, set STP state on it.
Signed-off-by: DENG Qingfang <[email protected]>
---
v1 -> v2: use FID enum instead of hardcoding.
drivers/net/dsa/mt7530.c | 3 ++-
drivers/net/dsa/mt7530.h | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 606a9f4db579..9b39ccd9dd4c 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_BRIDGED),
+ FID_PST(FID_BRIDGED, stp_state));
}
static int
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index d44640bbd865..5b70ccef9459 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -186,8 +186,8 @@ 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_MASK FID_PST(0x3)
+#define FID_PST(fid, state) (((state) & 0x3) << ((fid) * 2))
+#define FID_PST_MASK(fid) FID_PST(fid, 0x3)
enum mt7530_stp_state {
MT7530_STP_DISABLED = 0,
--
2.25.1
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 <[email protected]>
> ---
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 <[email protected]>
On Wed, Aug 04, 2021 at 12:04:03AM +0800, DENG Qingfang wrote:
> As filter ID 1 is the only one used for bridges, set STP state on it.
>
> Signed-off-by: DENG Qingfang <[email protected]>
> ---
Reviewed-by: Vladimir Oltean <[email protected]>
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Wed, 4 Aug 2021 00:04:00 +0800 you wrote:
> DSA core has gained software fallback support since commit 2f5dc00f7a3e
> ("net: bridge: switchdev: let drivers inform which bridge ports are
> offloaded"), 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 on filter ID 1
> net: dsa: mt7530: always install FDB entries with IVL and FID 1
>
> [...]
Here is the summary with links:
- [net-next,v2,1/4] net: dsa: mt7530: enable assisted learning on CPU port
https://git.kernel.org/netdev/net-next/c/0b69c54c74bc
- [net-next,v2,2/4] net: dsa: mt7530: use independent VLAN learning on VLAN-unaware bridges
https://git.kernel.org/netdev/net-next/c/6087175b7991
- [net-next,v2,3/4] net: dsa: mt7530: set STP state on filter ID 1
https://git.kernel.org/netdev/net-next/c/a9e3f62dff3c
- [net-next,v2,4/4] net: dsa: mt7530: always install FDB entries with IVL and FID 1
https://git.kernel.org/netdev/net-next/c/73c447cacbbd
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html