[PATCH net-next v0] net: mvpp2: Add parser configuration for DSA tags

[PATCH net-next v0] net: mvpp2: Add parser configuration for DSA tags

[Aryan Srivastava]

Allow the header parser to consider DSA and EDSA tagging. Currently the
parser is always configured to use the MH tag, but this results in poor
traffic distribution across queues and sub-optimal performance (in the
case where DSA or EDSA tags are in the header).

Add mechanism to check for tag type in use and then configure the
parser correctly for this tag. This results in proper traffic
distribution and hash calculation.

Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  2 +
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 37 ++++++++++++++++++-
 .../net/ethernet/marvell/mvpp2/mvpp2_prs.c    |  4 ++
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 9e02e4367bec..6ebed21a7af3 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -59,6 +59,8 @@
 
 /* Top Registers */
 #define MVPP2_MH_REG(port)			(0x5040 + 4 * (port))
+#define MVPP2_MH_EN				BIT(0)
+#define MVPP2_DSA_NON_EXTENDED			BIT(4)
 #define MVPP2_DSA_EXTENDED			BIT(5)
 #define MVPP2_VER_ID_REG			0x50b0
 #define MVPP2_VER_PP22				0x10
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 103632ba78a2..8f865fddc8c1 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -38,6 +38,7 @@
 #include <net/page_pool/helpers.h>
 #include <net/tso.h>
 #include <linux/bpf_trace.h>
+#include <net/dsa.h>
 
 #include "mvpp2.h"
 #include "mvpp2_prs.h"
@@ -4782,6 +4783,36 @@ static bool mvpp22_rss_is_supported(struct mvpp2_port *port)
 		!(port->flags & MVPP2_F_LOOPBACK);
 }
 
+static int mvpp2_get_tag(struct net_device *dev)
+{
+	int tag;
+	int dsa_proto = DSA_TAG_PROTO_NONE;
+
+#if IS_ENABLED(CONFIG_NET_DSA)
+	if (netdev_uses_dsa(dev))
+		dsa_proto = dev->dsa_ptr->tag_ops->proto;
+#endif
+
+	switch (dsa_proto) {
+	case DSA_TAG_PROTO_DSA:
+		tag = MVPP2_TAG_TYPE_DSA;
+		break;
+	case DSA_TAG_PROTO_EDSA:
+	/**
+	 * DSA_TAG_PROTO_EDSA and MVPP2_TAG_TYPE_EDSA are
+	 * referring to separate things. MVPP2_TAG_TYPE_EDSA
+	 * refers to extended DSA, while DSA_TAG_PROTO_EDSA
+	 * refers to Ethertype DSA. Ethertype DSA requires no
+	 * setting in the parser.
+	 */
+	default:
+		tag = MVPP2_TAG_TYPE_MH;
+		break;
+	}
+
+	return tag;
+}
+
 static int mvpp2_open(struct net_device *dev)
 {
 	struct mvpp2_port *port = netdev_priv(dev);
@@ -4801,7 +4832,11 @@ static int mvpp2_open(struct net_device *dev)
 		netdev_err(dev, "mvpp2_prs_mac_da_accept own addr failed\n");
 		return err;
 	}
-	err = mvpp2_prs_tag_mode_set(port->priv, port->id, MVPP2_TAG_TYPE_MH);
+
+	if (netdev_uses_dsa(dev))
+		err = mvpp2_prs_tag_mode_set(port->priv, port->id, mvpp2_get_tag(dev));
+	else
+		err = mvpp2_prs_tag_mode_set(port->priv, port->id, MVPP2_TAG_TYPE_MH);
 	if (err) {
 		netdev_err(dev, "mvpp2_prs_tag_mode_set failed\n");
 		return err;
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c
index 9af22f497a40..7cc42e22ed92 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c
@@ -2393,6 +2393,8 @@ int mvpp2_prs_tag_mode_set(struct mvpp2 *priv, int port, int type)
 				      MVPP2_PRS_TAGGED, MVPP2_PRS_DSA);
 		mvpp2_prs_dsa_tag_set(priv, port, false,
 				      MVPP2_PRS_UNTAGGED, MVPP2_PRS_DSA);
+		/* Set Marvell Header register for Ext. DSA tag */
+		mvpp2_write(priv, MVPP2_MH_REG(port), MVPP2_DSA_EXTENDED);
 		break;
 
 	case MVPP2_TAG_TYPE_DSA:
@@ -2406,6 +2408,8 @@ int mvpp2_prs_tag_mode_set(struct mvpp2 *priv, int port, int type)
 				      MVPP2_PRS_TAGGED, MVPP2_PRS_EDSA);
 		mvpp2_prs_dsa_tag_set(priv, port, false,
 				      MVPP2_PRS_UNTAGGED, MVPP2_PRS_EDSA);
+		/* Set Marvell Header register for DSA tag */
+		mvpp2_write(priv, MVPP2_MH_REG(port), MVPP2_DSA_NON_EXTENDED);
 		break;
 
 	case MVPP2_TAG_TYPE_MH:
-- 
2.46.0

Re: [PATCH net-next v0] net: mvpp2: Add parser configuration for DSA tags

[Maxime Chevallier]

Hello,

On Thu, 10 Oct 2024 14:51:04 +1300
Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz> wrote:

> Allow the header parser to consider DSA and EDSA tagging. Currently the
> parser is always configured to use the MH tag, but this results in poor
> traffic distribution across queues and sub-optimal performance (in the
> case where DSA or EDSA tags are in the header).
> 
> Add mechanism to check for tag type in use and then configure the
> parser correctly for this tag. This results in proper traffic
> distribution and hash calculation.
> 
> Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>

[...]

>  static int mvpp2_open(struct net_device *dev)
>  {
>  	struct mvpp2_port *port = netdev_priv(dev);
> @@ -4801,7 +4832,11 @@ static int mvpp2_open(struct net_device *dev)
>  		netdev_err(dev, "mvpp2_prs_mac_da_accept own addr failed\n");
>  		return err;
>  	}
> -	err = mvpp2_prs_tag_mode_set(port->priv, port->id, MVPP2_TAG_TYPE_MH);
> +
> +	if (netdev_uses_dsa(dev))
> +		err = mvpp2_prs_tag_mode_set(port->priv, port->id, mvpp2_get_tag(dev));
> +	else
> +		err = mvpp2_prs_tag_mode_set(port->priv, port->id, MVPP2_TAG_TYPE_MH);

This could unfortunately break VLAN filtering. If you look at the code
for mvpp2_prs_vid_entry_add() and mvpp2_prs_vid_enable_filtering(), the
value of the tag type set in MVPP2_MH_REG is used to compute the offset
at which the VLAN tag will be located.

It might be possible that users would :

 - Enable vlan filtering with :

ethtool -K ethX rx-vlan-filter on

 - Add vlan interfaces with :

ip link add link ethX name ethX.Y type vlan id Y

 - Set the interface up

ip link set ethX up => triggers a change in the DSA header size register

In that situation, the offset for the VLAN interface ethX.Y's header
will be incorrect, if the DSA tag type gets updated at .open() time.

So I think a solution would be to replace the read from the
MVPP2_MH_REG in the vlan filtering functions with a call your
newly-introduced mvpp2_get_tag, making sure that we use the correct tag
length for these parser entries as well.

Thanks,

Maxime

Re: [PATCH net-next v0] net: mvpp2: Add parser configuration for DSA tags

[Aryan Srivastava]

Hi Maxime,

Thank you for your review.
On Thu, 2024-10-10 at 16:17 +0200, Maxime Chevallier wrote:
> ip link set ethX up => triggers a change in the DSA header size
> register
> 
> In that situation, the offset for the VLAN interface ethX.Y's header
> will be incorrect, if the DSA tag type gets updated at .open() time.
> 
> So I think a solution would be to replace the read from the
> MVPP2_MH_REG in the vlan filtering functions with a call your
> newly-introduced mvpp2_get_tag, making sure that we use the correct
> tag
> length for these parser entries as well.
> 
> Thanks,
Yes I agree this can be a potential issue, and I will update the patch
according to your suggestion above.
> 
> Maxime

Thanks, Aryan