diff mbox series

[v1] net: mvpp2: Add parser configuration for DSA tags

Message ID 20250121021804.1302042-1-aryan.srivastava@alliedtelesis.co.nz (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [v1] net: mvpp2: Add parser configuration for DSA tags | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 5 this patch: 5
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 6 this patch: 6
netdev/checkpatch warning WARNING: line length of 86 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2025-01-21--03-00 (tests: 885)

Commit Message

Aryan Srivastava Jan. 21, 2025, 2:18 a.m. UTC
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.

Use mvpp2_get_tag instead of reading the MH register to determine tag
type. As the MH register is set during mvpp2_open it is subject to
change and not a proper reflection of the tagging type in use.

Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
---
Changes in v1:
- use mvpp2_get_tag to ascertain tagging type in use.

 drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  3 ++
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 37 ++++++++++++++++++-
 .../net/ethernet/marvell/mvpp2/mvpp2_prs.c    | 16 +++++---
 3 files changed, 49 insertions(+), 7 deletions(-)

Comments

Michal Swiatkowski Jan. 21, 2025, 7:05 a.m. UTC | #1
On Tue, Jan 21, 2025 at 03:18:04PM +1300, Aryan Srivastava 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.
> 
> Use mvpp2_get_tag instead of reading the MH register to determine tag
> type. As the MH register is set during mvpp2_open it is subject to
> change and not a proper reflection of the tagging type in use.
> 
> Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>

You didn't specify the tree, but it look like a feature implementation,
not a fix; net-next is closed now [1].

[1] https://lore.kernel.org/netdev/20250120163446.3bd93e30@kernel.org/#t

> ---
> Changes in v1:
> - use mvpp2_get_tag to ascertain tagging type in use.
> 
>  drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  3 ++
>  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 37 ++++++++++++++++++-
>  .../net/ethernet/marvell/mvpp2/mvpp2_prs.c    | 16 +++++---
>  3 files changed, 49 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> index 44fe9b68d1c2..456f9aeb4d82 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)
Defined, but not used.

> +#define MVPP2_DSA_NON_EXTENDED			BIT(4)
>  #define MVPP2_DSA_EXTENDED			BIT(5)
>  #define MVPP2_VER_ID_REG			0x50b0
>  #define MVPP2_VER_PP22				0x10
> @@ -1538,6 +1540,7 @@ void mvpp2_dbgfs_cleanup(struct mvpp2 *priv);
>  void mvpp2_dbgfs_exit(void);
>  
>  void mvpp23_rx_fifo_fc_en(struct mvpp2 *priv, int port, bool en);
> +int mvpp2_get_tag(struct net_device *dev);
>  
>  #ifdef CONFIG_MVPP2_PTP
>  int mvpp22_tai_probe(struct device *dev, struct mvpp2 *priv);
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index dd76c1b7ed3a..3a954da7660f 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>

Nit, can be in alphabetic order (rest is, so it will be cleaner to add
new following alphabetic order).
>  
>  #include "mvpp2.h"
>  #include "mvpp2_prs.h"
> @@ -4769,6 +4770,36 @@ static bool mvpp22_rss_is_supported(struct mvpp2_port *port)
>  		!(port->flags & MVPP2_F_LOOPBACK);
>  }
>  
> +int mvpp2_get_tag(struct net_device *dev)
> +{
> +	int tag;
> +	int dsa_proto = DSA_TAG_PROTO_NONE;

RCT (int dsa_proto before int tag)

> +
> +#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;

if (dsa_proto == DSA_TAG_PROTO_DSA)
	return MVPP2_TAG_TYPE_DSA

resturn MVPP2_TAG_TYPE_MH;

looks simpler, you can move comment above the if.

> +	}
> +
> +	return tag;
> +}
> +
>  static int mvpp2_open(struct net_device *dev)
>  {
>  	struct mvpp2_port *port = netdev_priv(dev);
> @@ -4788,7 +4819,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));

In mvpp2_get_tag() netdev_uses_dsa() is guarded by CONFIG_NET_DSA,
shouldn't it be here too? Or better, check for it is already in
mvpp2_get_tag() you can simple call

err = mvpp2_prs_tag_mode_set(port->priv, port->id, mvpp2_get_tag(dev));

as mvpp2_get_tag(dev) returns MVPP2_TAG_TYPE_MH in case
netdev_uses_dsa() returns 0.

> +	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..f14b9e8c301e 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c
> @@ -1963,7 +1963,7 @@ int mvpp2_prs_vid_entry_add(struct mvpp2_port *port, u16 vid)
>  {
>  	unsigned int vid_start = MVPP2_PE_VID_FILT_RANGE_START +
>  				 port->id * MVPP2_PRS_VLAN_FILT_MAX;
> -	unsigned int mask = 0xfff, reg_val, shift;
> +	unsigned int mask = 0xfff, tag, shift;
>  	struct mvpp2 *priv = port->priv;
>  	struct mvpp2_prs_entry pe;
>  	int tid;
> @@ -1973,8 +1973,8 @@ int mvpp2_prs_vid_entry_add(struct mvpp2_port *port, u16 vid)
>  	/* Scan TCAM and see if entry with this <vid,port> already exist */
>  	tid = mvpp2_prs_vid_range_find(port, vid, mask);
>  
> -	reg_val = mvpp2_read(priv, MVPP2_MH_REG(port->id));
> -	if (reg_val & MVPP2_DSA_EXTENDED)
> +	tag = mvpp2_get_tag(port->dev);
> +	if (tag & MVPP2_DSA_EXTENDED)

I will drop tag and use mvpp2_get_tag() directly (it isn't used anywhere
else in this function). This is only preference, ignore it if you prefer
to store it somewhere.

Am I right that code under if is unreachable? tag can be 1 or 2
(MVPP2_TAG_TYPE_MH or MVPP2_TAG_TYPE_DSA) and MVPP2_DSA_EXTENDED is
BIT(5). It looks like sth is missing between getting the tag and
checking it with this value.

>  		shift = MVPP2_VLAN_TAG_EDSA_LEN;
>  	else
>  		shift = MVPP2_VLAN_TAG_LEN;
> @@ -2071,7 +2071,7 @@ void mvpp2_prs_vid_enable_filtering(struct mvpp2_port *port)
>  {
>  	unsigned int tid = MVPP2_PRS_VID_PORT_DFLT(port->id);
>  	struct mvpp2 *priv = port->priv;
> -	unsigned int reg_val, shift;
> +	unsigned int tag, shift;
>  	struct mvpp2_prs_entry pe;
>  
>  	if (priv->prs_shadow[tid].valid)
> @@ -2081,8 +2081,8 @@ void mvpp2_prs_vid_enable_filtering(struct mvpp2_port *port)
>  
>  	pe.index = tid;
>  
> -	reg_val = mvpp2_read(priv, MVPP2_MH_REG(port->id));
> -	if (reg_val & MVPP2_DSA_EXTENDED)
> +	tag = mvpp2_get_tag(port->dev);
> +	if (tag & MVPP2_DSA_EXTENDED)

The same here, unreachable.

>  		shift = MVPP2_VLAN_TAG_EDSA_LEN;
>  	else
>  		shift = MVPP2_VLAN_TAG_LEN;
> @@ -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.47.1
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 44fe9b68d1c2..456f9aeb4d82 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
@@ -1538,6 +1540,7 @@  void mvpp2_dbgfs_cleanup(struct mvpp2 *priv);
 void mvpp2_dbgfs_exit(void);
 
 void mvpp23_rx_fifo_fc_en(struct mvpp2 *priv, int port, bool en);
+int mvpp2_get_tag(struct net_device *dev);
 
 #ifdef CONFIG_MVPP2_PTP
 int mvpp22_tai_probe(struct device *dev, struct mvpp2 *priv);
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index dd76c1b7ed3a..3a954da7660f 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"
@@ -4769,6 +4770,36 @@  static bool mvpp22_rss_is_supported(struct mvpp2_port *port)
 		!(port->flags & MVPP2_F_LOOPBACK);
 }
 
+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);
@@ -4788,7 +4819,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..f14b9e8c301e 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c
@@ -1963,7 +1963,7 @@  int mvpp2_prs_vid_entry_add(struct mvpp2_port *port, u16 vid)
 {
 	unsigned int vid_start = MVPP2_PE_VID_FILT_RANGE_START +
 				 port->id * MVPP2_PRS_VLAN_FILT_MAX;
-	unsigned int mask = 0xfff, reg_val, shift;
+	unsigned int mask = 0xfff, tag, shift;
 	struct mvpp2 *priv = port->priv;
 	struct mvpp2_prs_entry pe;
 	int tid;
@@ -1973,8 +1973,8 @@  int mvpp2_prs_vid_entry_add(struct mvpp2_port *port, u16 vid)
 	/* Scan TCAM and see if entry with this <vid,port> already exist */
 	tid = mvpp2_prs_vid_range_find(port, vid, mask);
 
-	reg_val = mvpp2_read(priv, MVPP2_MH_REG(port->id));
-	if (reg_val & MVPP2_DSA_EXTENDED)
+	tag = mvpp2_get_tag(port->dev);
+	if (tag & MVPP2_DSA_EXTENDED)
 		shift = MVPP2_VLAN_TAG_EDSA_LEN;
 	else
 		shift = MVPP2_VLAN_TAG_LEN;
@@ -2071,7 +2071,7 @@  void mvpp2_prs_vid_enable_filtering(struct mvpp2_port *port)
 {
 	unsigned int tid = MVPP2_PRS_VID_PORT_DFLT(port->id);
 	struct mvpp2 *priv = port->priv;
-	unsigned int reg_val, shift;
+	unsigned int tag, shift;
 	struct mvpp2_prs_entry pe;
 
 	if (priv->prs_shadow[tid].valid)
@@ -2081,8 +2081,8 @@  void mvpp2_prs_vid_enable_filtering(struct mvpp2_port *port)
 
 	pe.index = tid;
 
-	reg_val = mvpp2_read(priv, MVPP2_MH_REG(port->id));
-	if (reg_val & MVPP2_DSA_EXTENDED)
+	tag = mvpp2_get_tag(port->dev);
+	if (tag & MVPP2_DSA_EXTENDED)
 		shift = MVPP2_VLAN_TAG_EDSA_LEN;
 	else
 		shift = MVPP2_VLAN_TAG_LEN;
@@ -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: