| 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 |
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 --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:
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(-)