diff mbox series

[net-next,v8,3/6] net: dsa: Introduce dsa tagger data operation.

Message ID 20220909085138.3539952-4-mattias.forsblad@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: dsa: qca8k, mv88e6xxx: rmon: Add RMU support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 19 this patch: 19
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 13 this patch: 13
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
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: 19 this patch: 19
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 74 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Mattias Forsblad Sept. 9, 2022, 8:51 a.m. UTC
Support connecting dsa tagger for frame2reg decoding
with it's associated hookup functions.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Signed-off-by: Mattias Forsblad <mattias.forsblad@gmail.com>
---
 include/net/dsa.h |  5 +++++
 net/dsa/tag_dsa.c | 32 +++++++++++++++++++++++++++++---
 2 files changed, 34 insertions(+), 3 deletions(-)

Comments

Florian Fainelli Sept. 9, 2022, 5:35 p.m. UTC | #1
On 9/9/2022 1:51 AM, Mattias Forsblad wrote:
> Support connecting dsa tagger for frame2reg decoding
> with it's associated hookup functions.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
> Signed-off-by: Mattias Forsblad <mattias.forsblad@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Vladimir Oltean Sept. 11, 2022, 1:46 p.m. UTC | #2
A proper prefix for this patch would be "net: dsa: tag_dsa: ", since
"net: dsa:" refers to the whole subsystem.

On Fri, Sep 09, 2022 at 10:51:35AM +0200, Mattias Forsblad wrote:
> Support connecting dsa tagger for frame2reg decoding
> with it's associated hookup functions.

s/it's/its/

> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
> Signed-off-by: Mattias Forsblad <mattias.forsblad@gmail.com>
> ---
>  include/net/dsa.h |  5 +++++
>  net/dsa/tag_dsa.c | 32 +++++++++++++++++++++++++++++---
>  2 files changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 0e8a7ef17490..8510267d6188 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -130,6 +130,11 @@ struct dsa_lag {
>  	refcount_t refcount;
>  };
>  
> +struct dsa_tagger_data {
> +	void (*decode_frame2reg)(struct net_device *netdev,
> +				 struct sk_buff *skb);
> +};
> +

You probably mean to put this in include/linux/dsa/mv88e6xxx.h. Despite
the common naming, there is a big difference between DSA the framework
and DSA the Marvell implementation.

>  struct dsa_switch_tree {
>  	struct list_head	list;
>  
> diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
> index e4b6e3f2a3db..3dd1dcddaf05 100644
> --- a/net/dsa/tag_dsa.c
> +++ b/net/dsa/tag_dsa.c
> @@ -198,7 +198,10 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
>  static struct sk_buff *dsa_rcv_ll(struct sk_buff *skb, struct net_device *dev,
>  				  u8 extra)
>  {
> +	struct dsa_tagger_data *tagger_data;
> +	struct dsa_port *dp = dev->dsa_ptr;
>  	bool trap = false, trunk = false;
> +	struct dsa_switch *ds = dp->ds;
>  	int source_device, source_port;
>  	enum dsa_code code;
>  	enum dsa_cmd cmd;
> @@ -218,9 +221,9 @@ static struct sk_buff *dsa_rcv_ll(struct sk_buff *skb, struct net_device *dev,
>  
>  		switch (code) {
>  		case DSA_CODE_FRAME2REG:
> -			/* Remote management is not implemented yet,
> -			 * drop.
> -			 */
> +			tagger_data = ds->tagger_data;

You allocate one ds->tagger_data structure for each switch in the tree,
but you always use the tagger_data of the upstream-most "ds", the one
associated with the cpu_dp behind master->dsa_ptr.

How about minimally parsing the skb within the tagger, to figure out the
proper destination switch, and pass to tagger_data->decode_frame2reg()
the actual correct ds, plus a pointer to the skb so you can take a
reference on it?

> +			if (likely(tagger_data->decode_frame2reg))
> +				tagger_data->decode_frame2reg(dev, skb);
>  			return NULL;
>  		case DSA_CODE_ARP_MIRROR:
>  		case DSA_CODE_POLICY_MIRROR:
> @@ -323,6 +326,25 @@ static struct sk_buff *dsa_rcv_ll(struct sk_buff *skb, struct net_device *dev,
>  	return skb;
>  }
>  
> +static int dsa_tag_connect(struct dsa_switch *ds)
> +{
> +	struct dsa_tagger_data *tagger_data;
> +
> +	tagger_data = kzalloc(sizeof(*tagger_data), GFP_KERNEL);
> +	if (!tagger_data)
> +		return -ENOMEM;
> +
> +	ds->tagger_data = tagger_data;
> +
> +	return 0;
> +}
> +
> +static void dsa_tag_disconnect(struct dsa_switch *ds)
> +{
> +	kfree(ds->tagger_data);
> +	ds->tagger_data = NULL;
> +}
> +
>  #if IS_ENABLED(CONFIG_NET_DSA_TAG_DSA)
>  
>  static struct sk_buff *dsa_xmit(struct sk_buff *skb, struct net_device *dev)
> @@ -343,6 +365,8 @@ static const struct dsa_device_ops dsa_netdev_ops = {
>  	.proto	  = DSA_TAG_PROTO_DSA,
>  	.xmit	  = dsa_xmit,
>  	.rcv	  = dsa_rcv,
> +	.connect  = dsa_tag_connect,
> +	.disconnect = dsa_tag_disconnect,
>  	.needed_headroom = DSA_HLEN,
>  };
>  
> @@ -385,6 +409,8 @@ static const struct dsa_device_ops edsa_netdev_ops = {
>  	.proto	  = DSA_TAG_PROTO_EDSA,
>  	.xmit	  = edsa_xmit,
>  	.rcv	  = edsa_rcv,
> +	.connect  = dsa_tag_connect,
> +	.disconnect = dsa_tag_disconnect,
>  	.needed_headroom = EDSA_HLEN,
>  };
>  
> -- 
> 2.25.1
>
diff mbox series

Patch

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 0e8a7ef17490..8510267d6188 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -130,6 +130,11 @@  struct dsa_lag {
 	refcount_t refcount;
 };
 
+struct dsa_tagger_data {
+	void (*decode_frame2reg)(struct net_device *netdev,
+				 struct sk_buff *skb);
+};
+
 struct dsa_switch_tree {
 	struct list_head	list;
 
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index e4b6e3f2a3db..3dd1dcddaf05 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -198,7 +198,10 @@  static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
 static struct sk_buff *dsa_rcv_ll(struct sk_buff *skb, struct net_device *dev,
 				  u8 extra)
 {
+	struct dsa_tagger_data *tagger_data;
+	struct dsa_port *dp = dev->dsa_ptr;
 	bool trap = false, trunk = false;
+	struct dsa_switch *ds = dp->ds;
 	int source_device, source_port;
 	enum dsa_code code;
 	enum dsa_cmd cmd;
@@ -218,9 +221,9 @@  static struct sk_buff *dsa_rcv_ll(struct sk_buff *skb, struct net_device *dev,
 
 		switch (code) {
 		case DSA_CODE_FRAME2REG:
-			/* Remote management is not implemented yet,
-			 * drop.
-			 */
+			tagger_data = ds->tagger_data;
+			if (likely(tagger_data->decode_frame2reg))
+				tagger_data->decode_frame2reg(dev, skb);
 			return NULL;
 		case DSA_CODE_ARP_MIRROR:
 		case DSA_CODE_POLICY_MIRROR:
@@ -323,6 +326,25 @@  static struct sk_buff *dsa_rcv_ll(struct sk_buff *skb, struct net_device *dev,
 	return skb;
 }
 
+static int dsa_tag_connect(struct dsa_switch *ds)
+{
+	struct dsa_tagger_data *tagger_data;
+
+	tagger_data = kzalloc(sizeof(*tagger_data), GFP_KERNEL);
+	if (!tagger_data)
+		return -ENOMEM;
+
+	ds->tagger_data = tagger_data;
+
+	return 0;
+}
+
+static void dsa_tag_disconnect(struct dsa_switch *ds)
+{
+	kfree(ds->tagger_data);
+	ds->tagger_data = NULL;
+}
+
 #if IS_ENABLED(CONFIG_NET_DSA_TAG_DSA)
 
 static struct sk_buff *dsa_xmit(struct sk_buff *skb, struct net_device *dev)
@@ -343,6 +365,8 @@  static const struct dsa_device_ops dsa_netdev_ops = {
 	.proto	  = DSA_TAG_PROTO_DSA,
 	.xmit	  = dsa_xmit,
 	.rcv	  = dsa_rcv,
+	.connect  = dsa_tag_connect,
+	.disconnect = dsa_tag_disconnect,
 	.needed_headroom = DSA_HLEN,
 };
 
@@ -385,6 +409,8 @@  static const struct dsa_device_ops edsa_netdev_ops = {
 	.proto	  = DSA_TAG_PROTO_EDSA,
 	.xmit	  = edsa_xmit,
 	.rcv	  = edsa_rcv,
+	.connect  = dsa_tag_connect,
+	.disconnect = dsa_tag_disconnect,
 	.needed_headroom = EDSA_HLEN,
 };