diff mbox series

[net-next] net: dsa: mt7530: support MDB and bridge flag operations

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

Commit Message

DENG Qingfang March 15, 2021, 5:09 p.m. UTC
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(-)

Comments

Florian Fainelli March 15, 2021, 8:03 p.m. UTC | #1
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?
Vladimir Oltean March 15, 2021, 8:09 p.m. UTC | #2
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.
Florian Fainelli March 15, 2021, 8:44 p.m. UTC | #3
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.
Vladimir Oltean March 15, 2021, 9:15 p.m. UTC | #4
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.
DENG Qingfang March 16, 2021, 4:36 a.m. UTC | #5
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.
Vladimir Oltean March 16, 2021, 9:55 a.m. UTC | #6
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.
Vladimir Oltean March 16, 2021, 9:56 a.m. UTC | #7
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 mbox series

Patch

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;
 }