| Message ID | 20210315170940.2414854-1-dqfext@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [net-next] net: dsa: mt7530: support MDB and bridge flag operations | expand |
On 3/15/2021 10:09 AM, DENG Qingfang wrote: > Support port MDB and bridge flag operations. > > As the hardware can manage multicast forwarding itself, offload_fwd_mark > can be unconditionally set to true. > > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > --- > Changes since RFC: > Replaced BR_AUTO_MASK with BR_FLOOD | BR_LEARNING > > drivers/net/dsa/mt7530.c | 124 +++++++++++++++++++++++++++++++++++++-- > drivers/net/dsa/mt7530.h | 1 + > net/dsa/tag_mtk.c | 14 +---- > 3 files changed, 122 insertions(+), 17 deletions(-) > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > index 2342d4528b4c..f765984330c9 100644 > --- a/drivers/net/dsa/mt7530.c > +++ b/drivers/net/dsa/mt7530.c > @@ -1000,8 +1000,9 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port) > mt7530_write(priv, MT7530_PVC_P(port), > PORT_SPEC_TAG); > > - /* Unknown multicast frame forwarding to the cpu port */ > - mt7530_rmw(priv, MT7530_MFC, UNM_FFP_MASK, UNM_FFP(BIT(port))); > + /* Disable flooding by default */ > + mt7530_rmw(priv, MT7530_MFC, BC_FFP_MASK | UNM_FFP_MASK | UNU_FFP_MASK, > + BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) | UNU_FFP(BIT(port))); It's not clear to me why this is appropriate especially when the ports operated in standalone mode, can you expand a bit more on this?
On Mon, Mar 15, 2021 at 01:03:10PM -0700, Florian Fainelli wrote: > > > On 3/15/2021 10:09 AM, DENG Qingfang wrote: > > Support port MDB and bridge flag operations. > > > > As the hardware can manage multicast forwarding itself, offload_fwd_mark > > can be unconditionally set to true. > > > > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > > --- > > Changes since RFC: > > Replaced BR_AUTO_MASK with BR_FLOOD | BR_LEARNING > > > > drivers/net/dsa/mt7530.c | 124 +++++++++++++++++++++++++++++++++++++-- > > drivers/net/dsa/mt7530.h | 1 + > > net/dsa/tag_mtk.c | 14 +---- > > 3 files changed, 122 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > > index 2342d4528b4c..f765984330c9 100644 > > --- a/drivers/net/dsa/mt7530.c > > +++ b/drivers/net/dsa/mt7530.c > > @@ -1000,8 +1000,9 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port) > > mt7530_write(priv, MT7530_PVC_P(port), > > PORT_SPEC_TAG); > > > > - /* Unknown multicast frame forwarding to the cpu port */ > > - mt7530_rmw(priv, MT7530_MFC, UNM_FFP_MASK, UNM_FFP(BIT(port))); > > + /* Disable flooding by default */ > > + mt7530_rmw(priv, MT7530_MFC, BC_FFP_MASK | UNM_FFP_MASK | UNU_FFP_MASK, > > + BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) | UNU_FFP(BIT(port))); > > It's not clear to me why this is appropriate especially when the ports > operated in standalone mode, can you expand a bit more on this? We are in the function called "mt753x_cpu_port_enable" here. It's ok to apply this config for the CPU port.
On 3/15/2021 1:09 PM, Vladimir Oltean wrote: > On Mon, Mar 15, 2021 at 01:03:10PM -0700, Florian Fainelli wrote: >> >> >> On 3/15/2021 10:09 AM, DENG Qingfang wrote: >>> Support port MDB and bridge flag operations. >>> >>> As the hardware can manage multicast forwarding itself, offload_fwd_mark >>> can be unconditionally set to true. >>> >>> Signed-off-by: DENG Qingfang <dqfext@gmail.com> >>> --- >>> Changes since RFC: >>> Replaced BR_AUTO_MASK with BR_FLOOD | BR_LEARNING >>> >>> drivers/net/dsa/mt7530.c | 124 +++++++++++++++++++++++++++++++++++++-- >>> drivers/net/dsa/mt7530.h | 1 + >>> net/dsa/tag_mtk.c | 14 +---- >>> 3 files changed, 122 insertions(+), 17 deletions(-) >>> >>> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c >>> index 2342d4528b4c..f765984330c9 100644 >>> --- a/drivers/net/dsa/mt7530.c >>> +++ b/drivers/net/dsa/mt7530.c >>> @@ -1000,8 +1000,9 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port) >>> mt7530_write(priv, MT7530_PVC_P(port), >>> PORT_SPEC_TAG); >>> >>> - /* Unknown multicast frame forwarding to the cpu port */ >>> - mt7530_rmw(priv, MT7530_MFC, UNM_FFP_MASK, UNM_FFP(BIT(port))); >>> + /* Disable flooding by default */ >>> + mt7530_rmw(priv, MT7530_MFC, BC_FFP_MASK | UNM_FFP_MASK | UNU_FFP_MASK, >>> + BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) | UNU_FFP(BIT(port))); >> >> It's not clear to me why this is appropriate especially when the ports >> operated in standalone mode, can you expand a bit more on this? > > We are in the function called "mt753x_cpu_port_enable" here. It's ok to > apply this config for the CPU port. Because the user ports will flood unknown traffic and we have mediatek tags enabled presumably, so all traffic is copied to the CPU port, OK.
On Mon, Mar 15, 2021 at 01:44:02PM -0700, Florian Fainelli wrote: > On 3/15/2021 1:09 PM, Vladimir Oltean wrote: > > On Mon, Mar 15, 2021 at 01:03:10PM -0700, Florian Fainelli wrote: > >> > >> > >> On 3/15/2021 10:09 AM, DENG Qingfang wrote: > >>> Support port MDB and bridge flag operations. > >>> > >>> As the hardware can manage multicast forwarding itself, offload_fwd_mark > >>> can be unconditionally set to true. > >>> > >>> Signed-off-by: DENG Qingfang <dqfext@gmail.com> > >>> --- > >>> Changes since RFC: > >>> Replaced BR_AUTO_MASK with BR_FLOOD | BR_LEARNING > >>> > >>> drivers/net/dsa/mt7530.c | 124 +++++++++++++++++++++++++++++++++++++-- > >>> drivers/net/dsa/mt7530.h | 1 + > >>> net/dsa/tag_mtk.c | 14 +---- > >>> 3 files changed, 122 insertions(+), 17 deletions(-) > >>> > >>> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > >>> index 2342d4528b4c..f765984330c9 100644 > >>> --- a/drivers/net/dsa/mt7530.c > >>> +++ b/drivers/net/dsa/mt7530.c > >>> @@ -1000,8 +1000,9 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port) > >>> mt7530_write(priv, MT7530_PVC_P(port), > >>> PORT_SPEC_TAG); > >>> > >>> - /* Unknown multicast frame forwarding to the cpu port */ > >>> - mt7530_rmw(priv, MT7530_MFC, UNM_FFP_MASK, UNM_FFP(BIT(port))); > >>> + /* Disable flooding by default */ > >>> + mt7530_rmw(priv, MT7530_MFC, BC_FFP_MASK | UNM_FFP_MASK | UNU_FFP_MASK, > >>> + BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) | UNU_FFP(BIT(port))); > >> > >> It's not clear to me why this is appropriate especially when the ports > >> operated in standalone mode, can you expand a bit more on this? > > > > We are in the function called "mt753x_cpu_port_enable" here. It's ok to > > apply this config for the CPU port. > > Because the user ports will flood unknown traffic and we have mediatek > tags enabled presumably, so all traffic is copied to the CPU port, OK. Actually this is just how Qingfang explained it: https://patchwork.kernel.org/project/netdevbpf/patch/20210224081018.24719-1-dqfext@gmail.com/ I just assume that MT7530/7531 switches don't need to enable flooding on user ports when the only possible traffic source is the CPU port - the CPU port can inject traffic into any port regardless of egress flooding setting. If that's not true, I don't see how traffic in standalone ports mode would work after this patch.
On Tue, Mar 16, 2021 at 5:15 AM Vladimir Oltean <olteanv@gmail.com> wrote: > > Actually this is just how Qingfang explained it: > https://patchwork.kernel.org/project/netdevbpf/patch/20210224081018.24719-1-dqfext@gmail.com/ > > I just assume that MT7530/7531 switches don't need to enable flooding on > user ports when the only possible traffic source is the CPU port - the > CPU port can inject traffic into any port regardless of egress flooding > setting. If that's not true, I don't see how traffic in standalone ports > mode would work after this patch. Correct. Don't forget the earlier version of this driver (before my attempt to fix roaming) disabled unknown unicast flooding (trapped to CPU) in the same way.
On Tue, Mar 16, 2021 at 12:36:24PM +0800, DENG Qingfang wrote: > On Tue, Mar 16, 2021 at 5:15 AM Vladimir Oltean <olteanv@gmail.com> wrote: > > > > Actually this is just how Qingfang explained it: > > https://patchwork.kernel.org/project/netdevbpf/patch/20210224081018.24719-1-dqfext@gmail.com/ > > > > I just assume that MT7530/7531 switches don't need to enable flooding on > > user ports when the only possible traffic source is the CPU port - the > > CPU port can inject traffic into any port regardless of egress flooding > > setting. If that's not true, I don't see how traffic in standalone ports > > mode would work after this patch. > > Correct. Don't forget the earlier version of this driver (before my > attempt to fix roaming) disabled unknown unicast flooding (trapped to > CPU) in the same way. Ok, so in practice you don't really need to touch this register if it's already all ones.
On Tue, Mar 16, 2021 at 01:09:40AM +0800, DENG Qingfang wrote: > Support port MDB and bridge flag operations. > > As the hardware can manage multicast forwarding itself, offload_fwd_mark > can be unconditionally set to true. > > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > --- Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index 2342d4528b4c..f765984330c9 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -1000,8 +1000,9 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port) mt7530_write(priv, MT7530_PVC_P(port), PORT_SPEC_TAG); - /* Unknown multicast frame forwarding to the cpu port */ - mt7530_rmw(priv, MT7530_MFC, UNM_FFP_MASK, UNM_FFP(BIT(port))); + /* Disable flooding by default */ + mt7530_rmw(priv, MT7530_MFC, BC_FFP_MASK | UNM_FFP_MASK | UNU_FFP_MASK, + BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) | UNU_FFP(BIT(port))); /* Set CPU port number */ if (priv->id == ID_MT7621) @@ -1138,6 +1139,56 @@ mt7530_stp_state_set(struct dsa_switch *ds, int port, u8 state) mt7530_rmw(priv, MT7530_SSP_P(port), FID_PST_MASK, stp_state); } +static int +mt7530_port_pre_bridge_flags(struct dsa_switch *ds, int port, + struct switchdev_brport_flags flags, + struct netlink_ext_ack *extack) +{ + if (flags.mask & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD | + BR_BCAST_FLOOD)) + return -EINVAL; + + return 0; +} + +static int +mt7530_port_bridge_flags(struct dsa_switch *ds, int port, + struct switchdev_brport_flags flags, + struct netlink_ext_ack *extack) +{ + struct mt7530_priv *priv = ds->priv; + + if (flags.mask & BR_LEARNING) + mt7530_rmw(priv, MT7530_PSC_P(port), SA_DIS, + flags.val & BR_LEARNING ? 0 : SA_DIS); + + if (flags.mask & BR_FLOOD) + mt7530_rmw(priv, MT7530_MFC, UNU_FFP(BIT(port)), + flags.val & BR_FLOOD ? UNU_FFP(BIT(port)) : 0); + + if (flags.mask & BR_MCAST_FLOOD) + mt7530_rmw(priv, MT7530_MFC, UNM_FFP(BIT(port)), + flags.val & BR_MCAST_FLOOD ? UNM_FFP(BIT(port)) : 0); + + if (flags.mask & BR_BCAST_FLOOD) + mt7530_rmw(priv, MT7530_MFC, BC_FFP(BIT(port)), + flags.val & BR_BCAST_FLOOD ? BC_FFP(BIT(port)) : 0); + + return 0; +} + +static int +mt7530_port_set_mrouter(struct dsa_switch *ds, int port, bool mrouter, + struct netlink_ext_ack *extack) +{ + struct mt7530_priv *priv = ds->priv; + + mt7530_rmw(priv, MT7530_MFC, UNM_FFP(BIT(port)), + mrouter ? UNM_FFP(BIT(port)) : 0); + + return 0; +} + static int mt7530_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *bridge) @@ -1349,6 +1400,59 @@ mt7530_port_fdb_dump(struct dsa_switch *ds, int port, return 0; } +static int +mt7530_port_mdb_add(struct dsa_switch *ds, int port, + const struct switchdev_obj_port_mdb *mdb) +{ + struct mt7530_priv *priv = ds->priv; + const u8 *addr = mdb->addr; + u16 vid = mdb->vid; + u8 port_mask = 0; + int ret; + + mutex_lock(&priv->reg_mutex); + + mt7530_fdb_write(priv, vid, 0, addr, 0, STATIC_EMP); + if (!mt7530_fdb_cmd(priv, MT7530_FDB_READ, NULL)) + port_mask = (mt7530_read(priv, MT7530_ATRD) >> PORT_MAP) + & PORT_MAP_MASK; + + port_mask |= BIT(port); + mt7530_fdb_write(priv, vid, port_mask, addr, -1, STATIC_ENT); + ret = mt7530_fdb_cmd(priv, MT7530_FDB_WRITE, NULL); + + mutex_unlock(&priv->reg_mutex); + + return ret; +} + +static int +mt7530_port_mdb_del(struct dsa_switch *ds, int port, + const struct switchdev_obj_port_mdb *mdb) +{ + struct mt7530_priv *priv = ds->priv; + const u8 *addr = mdb->addr; + u16 vid = mdb->vid; + u8 port_mask = 0; + int ret; + + mutex_lock(&priv->reg_mutex); + + mt7530_fdb_write(priv, vid, 0, addr, 0, STATIC_EMP); + if (!mt7530_fdb_cmd(priv, MT7530_FDB_READ, NULL)) + port_mask = (mt7530_read(priv, MT7530_ATRD) >> PORT_MAP) + & PORT_MAP_MASK; + + port_mask &= ~BIT(port); + mt7530_fdb_write(priv, vid, port_mask, addr, -1, + port_mask ? STATIC_ENT : STATIC_EMP); + ret = mt7530_fdb_cmd(priv, MT7530_FDB_WRITE, NULL); + + mutex_unlock(&priv->reg_mutex); + + return ret; +} + static int mt7530_vlan_cmd(struct mt7530_priv *priv, enum mt7530_vlan_cmd cmd, u16 vid) { @@ -2492,9 +2596,13 @@ mt7530_setup(struct dsa_switch *ds) ret = mt753x_cpu_port_enable(ds, i); if (ret) return ret; - } else + } 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, PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT)); @@ -2656,9 +2764,12 @@ mt7531_setup(struct dsa_switch *ds) ret = mt753x_cpu_port_enable(ds, i); if (ret) return ret; - } else + } 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, PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT)); @@ -3385,11 +3496,16 @@ static const struct dsa_switch_ops mt7530_switch_ops = { .port_change_mtu = mt7530_port_change_mtu, .port_max_mtu = mt7530_port_max_mtu, .port_stp_state_set = mt7530_stp_state_set, + .port_pre_bridge_flags = mt7530_port_pre_bridge_flags, + .port_bridge_flags = mt7530_port_bridge_flags, + .port_set_mrouter = mt7530_port_set_mrouter, .port_bridge_join = mt7530_port_bridge_join, .port_bridge_leave = mt7530_port_bridge_leave, .port_fdb_add = mt7530_port_fdb_add, .port_fdb_del = mt7530_port_fdb_del, .port_fdb_dump = mt7530_port_fdb_dump, + .port_mdb_add = mt7530_port_mdb_add, + .port_mdb_del = mt7530_port_mdb_del, .port_vlan_filtering = mt7530_port_vlan_filtering, .port_vlan_add = mt7530_port_vlan_add, .port_vlan_del = mt7530_port_vlan_del, diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h index e4a4c8d1fbc8..fd7b66776c4e 100644 --- a/drivers/net/dsa/mt7530.h +++ b/drivers/net/dsa/mt7530.h @@ -34,6 +34,7 @@ enum mt753x_id { /* Registers to mac forward control for unknown frames */ #define MT7530_MFC 0x10 #define BC_FFP(x) (((x) & 0xff) << 24) +#define BC_FFP_MASK BC_FFP(~0) #define UNM_FFP(x) (((x) & 0xff) << 16) #define UNM_FFP_MASK UNM_FFP(~0) #define UNU_FFP(x) (((x) & 0xff) << 8) diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c index 59748487664f..f9b2966d1936 100644 --- a/net/dsa/tag_mtk.c +++ b/net/dsa/tag_mtk.c @@ -24,9 +24,6 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, struct dsa_port *dp = dsa_slave_to_port(dev); u8 xmit_tpid; u8 *mtk_tag; - 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 @@ -55,10 +52,6 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, mtk_tag[0] = xmit_tpid; 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 (xmit_tpid == MTK_HDR_XMIT_UNTAGGED) { mtk_tag[2] = 0; @@ -74,9 +67,6 @@ static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev, u16 hdr; int port; __be16 *phdr; - 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; @@ -102,9 +92,7 @@ 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; + skb->offload_fwd_mark = 1; return skb; }
Support port MDB and bridge flag operations. As the hardware can manage multicast forwarding itself, offload_fwd_mark can be unconditionally set to true. Signed-off-by: DENG Qingfang <dqfext@gmail.com> --- Changes since RFC: Replaced BR_AUTO_MASK with BR_FLOOD | BR_LEARNING drivers/net/dsa/mt7530.c | 124 +++++++++++++++++++++++++++++++++++++-- drivers/net/dsa/mt7530.h | 1 + net/dsa/tag_mtk.c | 14 +---- 3 files changed, 122 insertions(+), 17 deletions(-)