| Message ID | 20200425120207.5400-1-dqfext@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,net-next] net: dsa: mt7530: fix roaming from DSA user ports | expand |
Hi Qingfang, On Sat, 25 Apr 2020 at 15:03, DENG Qingfang <dqfext@gmail.com> wrote: > > When a client moves from a DSA user port to a software port in a bridge, > it cannot reach any other clients that connected to the DSA user ports. > That is because SA learning on the CPU port is disabled, so the switch > ignores the client's frames from the CPU port and still thinks it is at > the user port. > > Fix it by enabling SA learning on the CPU port. > > To prevent the switch from learning from flooding frames from the CPU > port, set skb->offload_fwd_mark to 1 for unicast and broadcast frames, > and let the switch flood them instead of trapping to the CPU port. > Multicast frames still need to be trapped to the CPU port for snooping, > so set the SA_DIS bit of the MTK tag to 1 when transmitting those frames > to disable SA learning. > > Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch") > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > --- I think enabling learning on the CPU port would fix the problem sometimes, but not always. (actually nothing can solve it always, see below) The switch learns the new route only if it receives any packets from the CPU port, with a SA equal to the station you're trying to reach. But what if the station is not sending any traffic at the moment, because it is simply waiting for connections to it first (just an example)? Unless there is any traffic already coming from the destination station too, your patch won't work. I am currently facing a similar situation with the ocelot/felix switches, but in that case, enabling SA learning on the CPU port is not possible. The way I dealt with it is by forcing a flush of the FDB entries on the port, in the following scenarios: - link goes down - port leaves its bridge So traffic towards a destination that has migrated away will temporarily be flooded again (towards the CPU port as well). There is still one case which isn't treated using this approach: when the station migrates away from a switch port that is not directly connected to this one. So no "link down" events would get generated in that case. We would still have to wait until the address expires in that case. I don't think that particular situation can be solved. My point is: if we agree that this is a larger problem, then DSA should have a .port_fdb_flush method and schedule a workqueue whenever necessary. Yes, it is a costly operation, but it will still probably take a lot less than the 300 seconds that the bridge configures for address ageing. Thoughts? > drivers/net/dsa/mt7530.c | 9 ++------- > drivers/net/dsa/mt7530.h | 1 + > net/dsa/tag_mtk.c | 15 +++++++++++++++ > 3 files changed, 18 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > index 5c444cd722bd..34e4aadfa705 100644 > --- a/drivers/net/dsa/mt7530.c > +++ b/drivers/net/dsa/mt7530.c > @@ -628,11 +628,8 @@ mt7530_cpu_port_enable(struct mt7530_priv *priv, > mt7530_write(priv, MT7530_PVC_P(port), > PORT_SPEC_TAG); > > - /* Disable auto learning on the cpu port */ > - mt7530_set(priv, MT7530_PSC_P(port), SA_DIS); > - > - /* Unknown unicast frame fordwarding to the cpu port */ > - mt7530_set(priv, MT7530_MFC, UNU_FFP(BIT(port))); > + /* Unknown multicast frame forwarding to the cpu port */ > + mt7530_rmw(priv, MT7530_MFC, UNM_FFP_MASK, UNM_FFP(BIT(port))); > > /* Set CPU port number */ > if (priv->id == ID_MT7621) > @@ -1294,8 +1291,6 @@ mt7530_setup(struct dsa_switch *ds) > /* Enable and reset MIB counters */ > mt7530_mib_reset(ds); > > - mt7530_clear(priv, MT7530_MFC, UNU_FFP_MASK); > - > for (i = 0; i < MT7530_NUM_PORTS; i++) { > /* Disable forwarding by default on all ports */ > mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK, > diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h > index 979bb6374678..82af4d2d406e 100644 > --- a/drivers/net/dsa/mt7530.h > +++ b/drivers/net/dsa/mt7530.h > @@ -31,6 +31,7 @@ enum { > #define MT7530_MFC 0x10 > #define BC_FFP(x) (((x) & 0xff) << 24) > #define UNM_FFP(x) (((x) & 0xff) << 16) > +#define UNM_FFP_MASK UNM_FFP(~0) > #define UNU_FFP(x) (((x) & 0xff) << 8) > #define UNU_FFP_MASK UNU_FFP(~0) > #define CPU_EN BIT(7) > diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c > index b5705cba8318..d6619edd53e5 100644 > --- a/net/dsa/tag_mtk.c > +++ b/net/dsa/tag_mtk.c > @@ -15,6 +15,7 @@ > #define MTK_HDR_XMIT_TAGGED_TPID_8100 1 > #define MTK_HDR_RECV_SOURCE_PORT_MASK GENMASK(2, 0) > #define MTK_HDR_XMIT_DP_BIT_MASK GENMASK(5, 0) > +#define MTK_HDR_XMIT_SA_DIS BIT(6) > > static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, > struct net_device *dev) > @@ -22,6 +23,9 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, > struct dsa_port *dp = dsa_slave_to_port(dev); > u8 *mtk_tag; > bool is_vlan_skb = true; > + unsigned char *dest = eth_hdr(skb)->h_dest; > + bool is_multicast_skb = is_multicast_ether_addr(dest) && > + !is_broadcast_ether_addr(dest); > > /* Build the special tag after the MAC Source Address. If VLAN header > * is present, it's required that VLAN header and special tag is > @@ -47,6 +51,10 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, > MTK_HDR_XMIT_UNTAGGED; > mtk_tag[1] = (1 << dp->index) & MTK_HDR_XMIT_DP_BIT_MASK; > > + /* Disable SA learning for multicast frames */ > + if (unlikely(is_multicast_skb)) > + mtk_tag[1] |= MTK_HDR_XMIT_SA_DIS; > + > /* Tag control information is kept for 802.1Q */ > if (!is_vlan_skb) { > mtk_tag[2] = 0; > @@ -61,6 +69,9 @@ static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev, > { > int port; > __be16 *phdr, hdr; > + unsigned char *dest = eth_hdr(skb)->h_dest; > + bool is_multicast_skb = is_multicast_ether_addr(dest) && > + !is_broadcast_ether_addr(dest); > > if (unlikely(!pskb_may_pull(skb, MTK_HDR_LEN))) > return NULL; > @@ -86,6 +97,10 @@ static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev, > if (!skb->dev) > return NULL; > > + /* Only unicast or broadcast frames are offloaded */ > + if (likely(!is_multicast_skb)) > + skb->offload_fwd_mark = 1; > + > return skb; > } > > -- > 2.26.1 > Thanks, -Vladimir
Hi Vladimir, On Mon, May 4, 2020 at 6:23 PM Vladimir Oltean <olteanv@gmail.com> wrote: > > Hi Qingfang, > > On Sat, 25 Apr 2020 at 15:03, DENG Qingfang <dqfext@gmail.com> wrote: > > > > When a client moves from a DSA user port to a software port in a bridge, > > it cannot reach any other clients that connected to the DSA user ports. > > That is because SA learning on the CPU port is disabled, so the switch > > ignores the client's frames from the CPU port and still thinks it is at > > the user port. > > > > Fix it by enabling SA learning on the CPU port. > > > > To prevent the switch from learning from flooding frames from the CPU > > port, set skb->offload_fwd_mark to 1 for unicast and broadcast frames, > > and let the switch flood them instead of trapping to the CPU port. > > Multicast frames still need to be trapped to the CPU port for snooping, > > so set the SA_DIS bit of the MTK tag to 1 when transmitting those frames > > to disable SA learning. > > > > Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch") > > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > > --- > > I think enabling learning on the CPU port would fix the problem > sometimes, but not always. (actually nothing can solve it always, see > below) > The switch learns the new route only if it receives any packets from > the CPU port, with a SA equal to the station you're trying to reach. > But what if the station is not sending any traffic at the moment, > because it is simply waiting for connections to it first (just an > example)? > Unless there is any traffic already coming from the destination > station too, your patch won't work. > I am currently facing a similar situation with the ocelot/felix > switches, but in that case, enabling SA learning on the CPU port is > not possible. Why is it not possible? Then try my previous RFC patch "net: bridge: fix client roaming from DSA user port" It tries removing entries from the switch when the client moves to another port. > The way I dealt with it is by forcing a flush of the FDB entries on > the port, in the following scenarios: > - link goes down > - port leaves its bridge > So traffic towards a destination that has migrated away will > temporarily be flooded again (towards the CPU port as well). > There is still one case which isn't treated using this approach: when > the station migrates away from a switch port that is not directly > connected to this one. So no "link down" events would get generated in > that case. We would still have to wait until the address expires in > that case. I don't think that particular situation can be solved. You're right. Every switch has this issue, even Linux bridge. > My point is: if we agree that this is a larger problem, then DSA > should have a .port_fdb_flush method and schedule a workqueue whenever > necessary. Yes, it is a costly operation, but it will still probably > take a lot less than the 300 seconds that the bridge configures for > address ageing. > > Thoughts? > > > drivers/net/dsa/mt7530.c | 9 ++------- > > drivers/net/dsa/mt7530.h | 1 + > > net/dsa/tag_mtk.c | 15 +++++++++++++++ > > 3 files changed, 18 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > > index 5c444cd722bd..34e4aadfa705 100644 > > --- a/drivers/net/dsa/mt7530.c > > +++ b/drivers/net/dsa/mt7530.c > > @@ -628,11 +628,8 @@ mt7530_cpu_port_enable(struct mt7530_priv *priv, > > mt7530_write(priv, MT7530_PVC_P(port), > > PORT_SPEC_TAG); > > > > - /* Disable auto learning on the cpu port */ > > - mt7530_set(priv, MT7530_PSC_P(port), SA_DIS); > > - > > - /* Unknown unicast frame fordwarding to the cpu port */ > > - mt7530_set(priv, MT7530_MFC, UNU_FFP(BIT(port))); > > + /* Unknown multicast frame forwarding to the cpu port */ > > + mt7530_rmw(priv, MT7530_MFC, UNM_FFP_MASK, UNM_FFP(BIT(port))); > > > > /* Set CPU port number */ > > if (priv->id == ID_MT7621) > > @@ -1294,8 +1291,6 @@ mt7530_setup(struct dsa_switch *ds) > > /* Enable and reset MIB counters */ > > mt7530_mib_reset(ds); > > > > - mt7530_clear(priv, MT7530_MFC, UNU_FFP_MASK); > > - > > for (i = 0; i < MT7530_NUM_PORTS; i++) { > > /* Disable forwarding by default on all ports */ > > mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK, > > diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h > > index 979bb6374678..82af4d2d406e 100644 > > --- a/drivers/net/dsa/mt7530.h > > +++ b/drivers/net/dsa/mt7530.h > > @@ -31,6 +31,7 @@ enum { > > #define MT7530_MFC 0x10 > > #define BC_FFP(x) (((x) & 0xff) << 24) > > #define UNM_FFP(x) (((x) & 0xff) << 16) > > +#define UNM_FFP_MASK UNM_FFP(~0) > > #define UNU_FFP(x) (((x) & 0xff) << 8) > > #define UNU_FFP_MASK UNU_FFP(~0) > > #define CPU_EN BIT(7) > > diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c > > index b5705cba8318..d6619edd53e5 100644 > > --- a/net/dsa/tag_mtk.c > > +++ b/net/dsa/tag_mtk.c > > @@ -15,6 +15,7 @@ > > #define MTK_HDR_XMIT_TAGGED_TPID_8100 1 > > #define MTK_HDR_RECV_SOURCE_PORT_MASK GENMASK(2, 0) > > #define MTK_HDR_XMIT_DP_BIT_MASK GENMASK(5, 0) > > +#define MTK_HDR_XMIT_SA_DIS BIT(6) > > > > static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, > > struct net_device *dev) > > @@ -22,6 +23,9 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, > > struct dsa_port *dp = dsa_slave_to_port(dev); > > u8 *mtk_tag; > > bool is_vlan_skb = true; > > + unsigned char *dest = eth_hdr(skb)->h_dest; > > + bool is_multicast_skb = is_multicast_ether_addr(dest) && > > + !is_broadcast_ether_addr(dest); > > > > /* Build the special tag after the MAC Source Address. If VLAN header > > * is present, it's required that VLAN header and special tag is > > @@ -47,6 +51,10 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, > > MTK_HDR_XMIT_UNTAGGED; > > mtk_tag[1] = (1 << dp->index) & MTK_HDR_XMIT_DP_BIT_MASK; > > > > + /* Disable SA learning for multicast frames */ > > + if (unlikely(is_multicast_skb)) > > + mtk_tag[1] |= MTK_HDR_XMIT_SA_DIS; > > + > > /* Tag control information is kept for 802.1Q */ > > if (!is_vlan_skb) { > > mtk_tag[2] = 0; > > @@ -61,6 +69,9 @@ static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev, > > { > > int port; > > __be16 *phdr, hdr; > > + unsigned char *dest = eth_hdr(skb)->h_dest; > > + bool is_multicast_skb = is_multicast_ether_addr(dest) && > > + !is_broadcast_ether_addr(dest); > > > > if (unlikely(!pskb_may_pull(skb, MTK_HDR_LEN))) > > return NULL; > > @@ -86,6 +97,10 @@ static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev, > > if (!skb->dev) > > return NULL; > > > > + /* Only unicast or broadcast frames are offloaded */ > > + if (likely(!is_multicast_skb)) > > + skb->offload_fwd_mark = 1; > > + > > return skb; > > } > > > > -- > > 2.26.1 > > > > Thanks, > -Vladimir
Hi Qingfang, On Mon, 4 May 2020 at 15:47, DENG Qingfang <dqfext@gmail.com> wrote: > > Hi Vladimir, > > On Mon, May 4, 2020 at 6:23 PM Vladimir Oltean <olteanv@gmail.com> wrote: > > > > Hi Qingfang, > > > > On Sat, 25 Apr 2020 at 15:03, DENG Qingfang <dqfext@gmail.com> wrote: > > > > > > When a client moves from a DSA user port to a software port in a bridge, > > > it cannot reach any other clients that connected to the DSA user ports. > > > That is because SA learning on the CPU port is disabled, so the switch > > > ignores the client's frames from the CPU port and still thinks it is at > > > the user port. > > > > > > Fix it by enabling SA learning on the CPU port. > > > > > > To prevent the switch from learning from flooding frames from the CPU > > > port, set skb->offload_fwd_mark to 1 for unicast and broadcast frames, > > > and let the switch flood them instead of trapping to the CPU port. > > > Multicast frames still need to be trapped to the CPU port for snooping, > > > so set the SA_DIS bit of the MTK tag to 1 when transmitting those frames > > > to disable SA learning. > > > > > > Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch") > > > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > > > --- > > > > I think enabling learning on the CPU port would fix the problem > > sometimes, but not always. (actually nothing can solve it always, see > > below) > > The switch learns the new route only if it receives any packets from > > the CPU port, with a SA equal to the station you're trying to reach. > > But what if the station is not sending any traffic at the moment, > > because it is simply waiting for connections to it first (just an > > example)? > > Unless there is any traffic already coming from the destination > > station too, your patch won't work. > > I am currently facing a similar situation with the ocelot/felix > > switches, but in that case, enabling SA learning on the CPU port is > > not possible. > > Why is it not possible? > Because learning on the CPU port is not supported on this hardware. > Then try my previous RFC patch > "net: bridge: fix client roaming from DSA user port" > It tries removing entries from the switch when the client moves to another port. > Your patch only deletes FDB entries of packets received in the fastpath by the software bridge, which as I said, won't work if the software bridge doesn't receive packets in the first place due to a stale FDB entry. > > The way I dealt with it is by forcing a flush of the FDB entries on > > the port, in the following scenarios: > > - link goes down > > - port leaves its bridge > > So traffic towards a destination that has migrated away will > > temporarily be flooded again (towards the CPU port as well). > > There is still one case which isn't treated using this approach: when > > the station migrates away from a switch port that is not directly > > connected to this one. So no "link down" events would get generated in > > that case. We would still have to wait until the address expires in > > that case. I don't think that particular situation can be solved. > > You're right. Every switch has this issue, even Linux bridge. > > > My point is: if we agree that this is a larger problem, then DSA > > should have a .port_fdb_flush method and schedule a workqueue whenever > > necessary. Yes, it is a costly operation, but it will still probably > > take a lot less than the 300 seconds that the bridge configures for > > address ageing. > > > > Thoughts? > > > > > > Thanks, > > -Vladimir Regards, -Vladimir
Hi Vladimir, On Mon, May 4, 2020 at 9:15 PM Vladimir Oltean <olteanv@gmail.com> wrote: > > Hi Qingfang, > > On Mon, 4 May 2020 at 15:47, DENG Qingfang <dqfext@gmail.com> wrote: > > > > Hi Vladimir, > > > > On Mon, May 4, 2020 at 6:23 PM Vladimir Oltean <olteanv@gmail.com> wrote: > > > > > > Hi Qingfang, > > > > > > On Sat, 25 Apr 2020 at 15:03, DENG Qingfang <dqfext@gmail.com> wrote: > > > > > > > > When a client moves from a DSA user port to a software port in a bridge, > > > > it cannot reach any other clients that connected to the DSA user ports. > > > > That is because SA learning on the CPU port is disabled, so the switch > > > > ignores the client's frames from the CPU port and still thinks it is at > > > > the user port. > > > > > > > > Fix it by enabling SA learning on the CPU port. > > > > > > > > To prevent the switch from learning from flooding frames from the CPU > > > > port, set skb->offload_fwd_mark to 1 for unicast and broadcast frames, > > > > and let the switch flood them instead of trapping to the CPU port. > > > > Multicast frames still need to be trapped to the CPU port for snooping, > > > > so set the SA_DIS bit of the MTK tag to 1 when transmitting those frames > > > > to disable SA learning. > > > > > > > > Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch") > > > > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > > > > --- > > > > > > I think enabling learning on the CPU port would fix the problem > > > sometimes, but not always. (actually nothing can solve it always, see > > > below) > > > The switch learns the new route only if it receives any packets from > > > the CPU port, with a SA equal to the station you're trying to reach. > > > But what if the station is not sending any traffic at the moment, > > > because it is simply waiting for connections to it first (just an > > > example)? > > > Unless there is any traffic already coming from the destination > > > station too, your patch won't work. > > > I am currently facing a similar situation with the ocelot/felix > > > switches, but in that case, enabling SA learning on the CPU port is > > > not possible. > > > > Why is it not possible? > > > > Because learning on the CPU port is not supported on this hardware. > > > Then try my previous RFC patch > > "net: bridge: fix client roaming from DSA user port" > > It tries removing entries from the switch when the client moves to another port. > > > > Your patch only deletes FDB entries of packets received in the > fastpath by the software bridge, which as I said, won't work if the > software bridge doesn't receive packets in the first place due to a > stale FDB entry. As I said before, ALL switches including software linux bridge have this issue. In this case, you'd better ensure the client sends packets first after migration, which most clients already do in switches + wireless APs setup. > > > > The way I dealt with it is by forcing a flush of the FDB entries on > > > the port, in the following scenarios: > > > - link goes down > > > - port leaves its bridge > > > So traffic towards a destination that has migrated away will > > > temporarily be flooded again (towards the CPU port as well). > > > There is still one case which isn't treated using this approach: when > > > the station migrates away from a switch port that is not directly > > > connected to this one. So no "link down" events would get generated in > > > that case. We would still have to wait until the address expires in > > > that case. I don't think that particular situation can be solved. > > > > You're right. Every switch has this issue, even Linux bridge. > > > > > My point is: if we agree that this is a larger problem, then DSA > > > should have a .port_fdb_flush method and schedule a workqueue whenever > > > necessary. Yes, it is a costly operation, but it will still probably > > > take a lot less than the 300 seconds that the bridge configures for > > > address ageing. > > > > > > Thoughts? > > > > > > > > > > Thanks, > > > -Vladimir > > Regards, > -Vladimir Regards, Qingfang
Hi! On Mon, May 4, 2020 at 6:23 PM Vladimir Oltean <olteanv@gmail.com> wrote: > I think enabling learning on the CPU port would fix the problem > sometimes, but not always. (actually nothing can solve it always, see > below) > The switch learns the new route only if it receives any packets from > the CPU port, with a SA equal to the station you're trying to reach. > But what if the station is not sending any traffic at the moment, > because it is simply waiting for connections to it first (just an > example)? > Unless there is any traffic already coming from the destination > station too, your patch won't work. This is just the limitation of connecting two bridges together. > I am currently facing a similar situation with the ocelot/felix > switches, but in that case, enabling SA learning on the CPU port is > not possible. > The way I dealt with it is by forcing a flush of the FDB entries on > the port, in the following scenarios: > - link goes down > - port leaves its bridge > So traffic towards a destination that has migrated away will > temporarily be flooded again (towards the CPU port as well). In previous discussion in thread: "net: bridge: fix client roaming from DSA user port" It's currently established that linux treats a DSA switch with forwarding offload capability as its own bridge. If the switch can't learn from cpu port, you either need to propose a change of this already established behaviour so that software bridge can sync its fdb with hardware (making sw bridge and hardware switch behave as one bridge instead of two) or write extra code to help managing hardware fdb. (so that the switch matches current behaviour.) > There is still one case which isn't treated using this approach: when > the station migrates away from a switch port that is not directly > connected to this one. So no "link down" events would get generated in > that case. We would still have to wait until the address expires in > that case. I don't think that particular situation can be solved. > My point is: if we agree that this is a larger problem, then DSA > should have a .port_fdb_flush method and schedule a workqueue whenever > necessary. Yes, it is a costly operation, but it will still probably > take a lot less than the 300 seconds that the bridge configures for > address ageing. I think flushing fdb on port topology changes doesn't solve the problem targeted by this patch. Anyway, this mt7530 patch is proposed because current mt7530 driver failed to match the established behaviour for DSA/switchdev. I think it's better to start a new thread if you'd like to propose these fundamental behaviour changes, because this patch is already a result of previously proposed behaviour changes being rejected.
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index 5c444cd722bd..34e4aadfa705 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -628,11 +628,8 @@ mt7530_cpu_port_enable(struct mt7530_priv *priv, mt7530_write(priv, MT7530_PVC_P(port), PORT_SPEC_TAG); - /* Disable auto learning on the cpu port */ - mt7530_set(priv, MT7530_PSC_P(port), SA_DIS); - - /* Unknown unicast frame fordwarding to the cpu port */ - mt7530_set(priv, MT7530_MFC, UNU_FFP(BIT(port))); + /* Unknown multicast frame forwarding to the cpu port */ + mt7530_rmw(priv, MT7530_MFC, UNM_FFP_MASK, UNM_FFP(BIT(port))); /* Set CPU port number */ if (priv->id == ID_MT7621) @@ -1294,8 +1291,6 @@ mt7530_setup(struct dsa_switch *ds) /* Enable and reset MIB counters */ mt7530_mib_reset(ds); - mt7530_clear(priv, MT7530_MFC, UNU_FFP_MASK); - for (i = 0; i < MT7530_NUM_PORTS; i++) { /* Disable forwarding by default on all ports */ mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK, diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h index 979bb6374678..82af4d2d406e 100644 --- a/drivers/net/dsa/mt7530.h +++ b/drivers/net/dsa/mt7530.h @@ -31,6 +31,7 @@ enum { #define MT7530_MFC 0x10 #define BC_FFP(x) (((x) & 0xff) << 24) #define UNM_FFP(x) (((x) & 0xff) << 16) +#define UNM_FFP_MASK UNM_FFP(~0) #define UNU_FFP(x) (((x) & 0xff) << 8) #define UNU_FFP_MASK UNU_FFP(~0) #define CPU_EN BIT(7) diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c index b5705cba8318..d6619edd53e5 100644 --- a/net/dsa/tag_mtk.c +++ b/net/dsa/tag_mtk.c @@ -15,6 +15,7 @@ #define MTK_HDR_XMIT_TAGGED_TPID_8100 1 #define MTK_HDR_RECV_SOURCE_PORT_MASK GENMASK(2, 0) #define MTK_HDR_XMIT_DP_BIT_MASK GENMASK(5, 0) +#define MTK_HDR_XMIT_SA_DIS BIT(6) static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, struct net_device *dev) @@ -22,6 +23,9 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, struct dsa_port *dp = dsa_slave_to_port(dev); u8 *mtk_tag; bool is_vlan_skb = true; + unsigned char *dest = eth_hdr(skb)->h_dest; + bool is_multicast_skb = is_multicast_ether_addr(dest) && + !is_broadcast_ether_addr(dest); /* Build the special tag after the MAC Source Address. If VLAN header * is present, it's required that VLAN header and special tag is @@ -47,6 +51,10 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, MTK_HDR_XMIT_UNTAGGED; mtk_tag[1] = (1 << dp->index) & MTK_HDR_XMIT_DP_BIT_MASK; + /* Disable SA learning for multicast frames */ + if (unlikely(is_multicast_skb)) + mtk_tag[1] |= MTK_HDR_XMIT_SA_DIS; + /* Tag control information is kept for 802.1Q */ if (!is_vlan_skb) { mtk_tag[2] = 0; @@ -61,6 +69,9 @@ static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev, { int port; __be16 *phdr, hdr; + unsigned char *dest = eth_hdr(skb)->h_dest; + bool is_multicast_skb = is_multicast_ether_addr(dest) && + !is_broadcast_ether_addr(dest); if (unlikely(!pskb_may_pull(skb, MTK_HDR_LEN))) return NULL; @@ -86,6 +97,10 @@ static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev, if (!skb->dev) return NULL; + /* Only unicast or broadcast frames are offloaded */ + if (likely(!is_multicast_skb)) + skb->offload_fwd_mark = 1; + return skb; }
When a client moves from a DSA user port to a software port in a bridge, it cannot reach any other clients that connected to the DSA user ports. That is because SA learning on the CPU port is disabled, so the switch ignores the client's frames from the CPU port and still thinks it is at the user port. Fix it by enabling SA learning on the CPU port. To prevent the switch from learning from flooding frames from the CPU port, set skb->offload_fwd_mark to 1 for unicast and broadcast frames, and let the switch flood them instead of trapping to the CPU port. Multicast frames still need to be trapped to the CPU port for snooping, so set the SA_DIS bit of the MTK tag to 1 when transmitting those frames to disable SA learning. Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch") Signed-off-by: DENG Qingfang <dqfext@gmail.com> --- drivers/net/dsa/mt7530.c | 9 ++------- drivers/net/dsa/mt7530.h | 1 + net/dsa/tag_mtk.c | 15 +++++++++++++++ 3 files changed, 18 insertions(+), 7 deletions(-)