[PATCH net-next 0/3] net: dsa: Receive path simplifications

[PATCH net-next 0/3] net: dsa: Receive path simplifications

[Florian Fainelli]

Hi all,

This patch series does factor the common code found in all tag implementations
into dsa_switch_rcv(). The original motivation was to add GRO support, but this
may be a lot of work with unclear benefits at this point.

Florian Fainelli (3):
  net: dsa: Do not check for NULL dst in tag parsers
  net: dsa: Move skb_unshare() to dsa_switch_rcv()
  net: dsa: Factor bottom tag receive functions

 include/net/dsa.h     |  2 +-
 net/dsa/dsa.c         | 24 +++++++++++++++++++++++-
 net/dsa/dsa_priv.h    |  5 +++--
 net/dsa/tag_brcm.c    | 26 +++++---------------------
 net/dsa/tag_dsa.c     | 26 +++++---------------------
 net/dsa/tag_edsa.c    | 26 +++++---------------------
 net/dsa/tag_qca.c     | 26 +++++---------------------
 net/dsa/tag_trailer.c | 25 +++++--------------------
 8 files changed, 52 insertions(+), 108 deletions(-)

-- 
2.9.3

[PATCH net-next 1/3] net: dsa: Do not check for NULL dst in tag parsers

[Florian Fainelli]

dsa_switch_rcv() already tests for dst == NULL, so there is no need to duplicate
the same check within the tag receive functions.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/tag_brcm.c    | 3 ---
 net/dsa/tag_dsa.c     | 3 ---
 net/dsa/tag_edsa.c    | 3 ---
 net/dsa/tag_qca.c     | 3 ---
 net/dsa/tag_trailer.c | 2 --
 5 files changed, 14 deletions(-)

diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index e2ed6cf68261..68d4feef96d4 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -100,9 +100,6 @@ static int brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev,
 	int source_port;
 	u8 *brcm_tag;
 
-	if (unlikely(dst == NULL))
-		goto out_drop;
-
 	ds = dst->cpu_switch;
 
 	skb = skb_unshare(skb, GFP_ATOMIC);
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index e42ba906100c..377569c0e4f7 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -77,9 +77,6 @@ static int dsa_rcv(struct sk_buff *skb, struct net_device *dev,
 	int source_device;
 	int source_port;
 
-	if (unlikely(dst == NULL))
-		goto out_drop;
-
 	skb = skb_unshare(skb, GFP_ATOMIC);
 	if (skb == NULL)
 		goto out;
diff --git a/net/dsa/tag_edsa.c b/net/dsa/tag_edsa.c
index 6a9b7a9e4e15..30520ff9c9a2 100644
--- a/net/dsa/tag_edsa.c
+++ b/net/dsa/tag_edsa.c
@@ -90,9 +90,6 @@ static int edsa_rcv(struct sk_buff *skb, struct net_device *dev,
 	int source_device;
 	int source_port;
 
-	if (unlikely(dst == NULL))
-		goto out_drop;
-
 	skb = skb_unshare(skb, GFP_ATOMIC);
 	if (skb == NULL)
 		goto out;
diff --git a/net/dsa/tag_qca.c b/net/dsa/tag_qca.c
index 4e0dad759d04..6579d6db1bc6 100644
--- a/net/dsa/tag_qca.c
+++ b/net/dsa/tag_qca.c
@@ -75,9 +75,6 @@ static int qca_tag_rcv(struct sk_buff *skb, struct net_device *dev,
 	int port;
 	__be16 *phdr, hdr;
 
-	if (unlikely(!dst))
-		goto out_drop;
-
 	skb = skb_unshare(skb, GFP_ATOMIC);
 	if (!skb)
 		goto out;
diff --git a/net/dsa/tag_trailer.c b/net/dsa/tag_trailer.c
index 74c948512550..f5c764ee2968 100644
--- a/net/dsa/tag_trailer.c
+++ b/net/dsa/tag_trailer.c
@@ -66,8 +66,6 @@ static int trailer_rcv(struct sk_buff *skb, struct net_device *dev,
 	u8 *trailer;
 	int source_port;
 
-	if (unlikely(dst == NULL))
-		goto out_drop;
 	ds = dst->cpu_switch;
 
 	skb = skb_unshare(skb, GFP_ATOMIC);
-- 
2.9.3

[PATCH net-next 2/3] net: dsa: Move skb_unshare() to dsa_switch_rcv()

[Florian Fainelli]

All DSA tag receive functions need to unshare the skb before mangling it, move
this to the generic dsa_switch_rcv() function which will allow us to make the
tag receive function return their mangled skb without caring about freeing a
NULL skb.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa.c         | 4 ++++
 net/dsa/tag_brcm.c    | 5 -----
 net/dsa/tag_dsa.c     | 5 -----
 net/dsa/tag_edsa.c    | 5 -----
 net/dsa/tag_qca.c     | 5 -----
 net/dsa/tag_trailer.c | 5 -----
 6 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 95d1a756202c..c20da7bfc771 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -903,6 +903,10 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
 		return 0;
 	}
 
+	skb = skb_unshare(skb, GFP_ATOMIC);
+	if (!skb)
+		return 0;
+
 	return dst->rcv(skb, dev, pt, orig_dev);
 }
 
diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index 68d4feef96d4..263941769c88 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -102,10 +102,6 @@ static int brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev,
 
 	ds = dst->cpu_switch;
 
-	skb = skb_unshare(skb, GFP_ATOMIC);
-	if (skb == NULL)
-		goto out;
-
 	if (unlikely(!pskb_may_pull(skb, BRCM_TAG_LEN)))
 		goto out_drop;
 
@@ -151,7 +147,6 @@ static int brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev,
 
 out_drop:
 	kfree_skb(skb);
-out:
 	return 0;
 }
 
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index 377569c0e4f7..b7032699eaad 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -77,10 +77,6 @@ static int dsa_rcv(struct sk_buff *skb, struct net_device *dev,
 	int source_device;
 	int source_port;
 
-	skb = skb_unshare(skb, GFP_ATOMIC);
-	if (skb == NULL)
-		goto out;
-
 	if (unlikely(!pskb_may_pull(skb, DSA_HLEN)))
 		goto out_drop;
 
@@ -175,7 +171,6 @@ static int dsa_rcv(struct sk_buff *skb, struct net_device *dev,
 
 out_drop:
 	kfree_skb(skb);
-out:
 	return 0;
 }
 
diff --git a/net/dsa/tag_edsa.c b/net/dsa/tag_edsa.c
index 30520ff9c9a2..b87009672b40 100644
--- a/net/dsa/tag_edsa.c
+++ b/net/dsa/tag_edsa.c
@@ -90,10 +90,6 @@ static int edsa_rcv(struct sk_buff *skb, struct net_device *dev,
 	int source_device;
 	int source_port;
 
-	skb = skb_unshare(skb, GFP_ATOMIC);
-	if (skb == NULL)
-		goto out;
-
 	if (unlikely(!pskb_may_pull(skb, EDSA_HLEN)))
 		goto out_drop;
 
@@ -194,7 +190,6 @@ static int edsa_rcv(struct sk_buff *skb, struct net_device *dev,
 
 out_drop:
 	kfree_skb(skb);
-out:
 	return 0;
 }
 
diff --git a/net/dsa/tag_qca.c b/net/dsa/tag_qca.c
index 6579d6db1bc6..d1324649808c 100644
--- a/net/dsa/tag_qca.c
+++ b/net/dsa/tag_qca.c
@@ -75,10 +75,6 @@ static int qca_tag_rcv(struct sk_buff *skb, struct net_device *dev,
 	int port;
 	__be16 *phdr, hdr;
 
-	skb = skb_unshare(skb, GFP_ATOMIC);
-	if (!skb)
-		goto out;
-
 	if (unlikely(!pskb_may_pull(skb, QCA_HDR_LEN)))
 		goto out_drop;
 
@@ -126,7 +122,6 @@ static int qca_tag_rcv(struct sk_buff *skb, struct net_device *dev,
 
 out_drop:
 	kfree_skb(skb);
-out:
 	return 0;
 }
 
diff --git a/net/dsa/tag_trailer.c b/net/dsa/tag_trailer.c
index f5c764ee2968..1fc0b221a70f 100644
--- a/net/dsa/tag_trailer.c
+++ b/net/dsa/tag_trailer.c
@@ -68,10 +68,6 @@ static int trailer_rcv(struct sk_buff *skb, struct net_device *dev,
 
 	ds = dst->cpu_switch;
 
-	skb = skb_unshare(skb, GFP_ATOMIC);
-	if (skb == NULL)
-		goto out;
-
 	if (skb_linearize(skb))
 		goto out_drop;
 
@@ -100,7 +96,6 @@ static int trailer_rcv(struct sk_buff *skb, struct net_device *dev,
 
 out_drop:
 	kfree_skb(skb);
-out:
 	return 0;
 }
 
-- 
2.9.3

[PATCH net-next 3/3] net: dsa: Factor bottom tag receive functions

[Florian Fainelli]

All DSA tag receive functions do strictly the same thing after they have located
the originating source port from their tag specific protocol:

- push ETH_HLEN bytes
- set pkt_type to PACKET_HOST
- call eth_type_trans()
- bump up counters
- call netif_receive_skb()

Factor all of that into dsa_switch_rcv(). This also makes us return a pointer to
a sk_buff, which makes us symetric with the xmit function.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/net/dsa.h     |  2 +-
 net/dsa/dsa.c         | 20 +++++++++++++++++++-
 net/dsa/dsa_priv.h    |  5 +++--
 net/dsa/tag_brcm.c    | 18 +++++-------------
 net/dsa/tag_dsa.c     | 18 +++++-------------
 net/dsa/tag_edsa.c    | 18 +++++-------------
 net/dsa/tag_qca.c     | 18 +++++-------------
 net/dsa/tag_trailer.c | 18 +++++-------------
 8 files changed, 48 insertions(+), 69 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index ffe56cc338fe..671a00aa35de 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -123,7 +123,7 @@ struct dsa_switch_tree {
 	 * protocol to use.
 	 */
 	struct net_device	*master_netdev;
-	int			(*rcv)(struct sk_buff *skb,
+	struct sk_buff *	(*rcv)(struct sk_buff *skb,
 				       struct net_device *dev,
 				       struct packet_type *pt,
 				       struct net_device *orig_dev);
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index c20da7bfc771..21f960ef8138 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -23,6 +23,7 @@
 #include <linux/sysfs.h>
 #include <linux/phy_fixed.h>
 #include <linux/gpio/consumer.h>
+#include <linux/etherdevice.h>
 #include <net/dsa.h>
 #include "dsa_priv.h"
 
@@ -897,6 +898,7 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
 			  struct packet_type *pt, struct net_device *orig_dev)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct sk_buff *nskb = NULL;
 
 	if (unlikely(dst == NULL)) {
 		kfree_skb(skb);
@@ -907,7 +909,23 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
 	if (!skb)
 		return 0;
 
-	return dst->rcv(skb, dev, pt, orig_dev);
+	nskb = dst->rcv(skb, dev, pt, orig_dev);
+	if (!nskb) {
+		kfree_skb(skb);
+		return 0;
+	}
+
+	skb = nskb;
+	skb_push(skb, ETH_HLEN);
+	skb->pkt_type = PACKET_HOST;
+	skb->protocol = eth_type_trans(skb, skb->dev);
+
+	skb->dev->stats.rx_packets++;
+	skb->dev->stats.rx_bytes += skb->len;
+
+	netif_receive_skb(skb);
+
+	return 0;
 }
 
 static struct packet_type dsa_pack_type __read_mostly = {
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 0706a511244e..f4ac8d5fb8d5 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -17,8 +17,9 @@
 
 struct dsa_device_ops {
 	struct sk_buff *(*xmit)(struct sk_buff *skb, struct net_device *dev);
-	int (*rcv)(struct sk_buff *skb, struct net_device *dev,
-		   struct packet_type *pt, struct net_device *orig_dev);
+	struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev,
+			       struct packet_type *pt,
+			       struct net_device *orig_dev);
 };
 
 struct dsa_slave_priv {
diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index 263941769c88..2a9b52c5af86 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -92,8 +92,9 @@ static struct sk_buff *brcm_tag_xmit(struct sk_buff *skb, struct net_device *dev
 	return NULL;
 }
 
-static int brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev,
-			struct packet_type *pt, struct net_device *orig_dev)
+static struct sk_buff *brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev,
+				    struct packet_type *pt,
+				    struct net_device *orig_dev)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_switch *ds;
@@ -133,21 +134,12 @@ static int brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev,
 		skb->data - ETH_HLEN - BRCM_TAG_LEN,
 		2 * ETH_ALEN);
 
-	skb_push(skb, ETH_HLEN);
-	skb->pkt_type = PACKET_HOST;
 	skb->dev = ds->ports[source_port].netdev;
-	skb->protocol = eth_type_trans(skb, skb->dev);
 
-	skb->dev->stats.rx_packets++;
-	skb->dev->stats.rx_bytes += skb->len;
-
-	netif_receive_skb(skb);
-
-	return 0;
+	return skb;
 
 out_drop:
-	kfree_skb(skb);
-	return 0;
+	return NULL;
 }
 
 const struct dsa_device_ops brcm_netdev_ops = {
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index b7032699eaad..1c6633f0de01 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -68,8 +68,9 @@ static struct sk_buff *dsa_xmit(struct sk_buff *skb, struct net_device *dev)
 	return NULL;
 }
 
-static int dsa_rcv(struct sk_buff *skb, struct net_device *dev,
-		   struct packet_type *pt, struct net_device *orig_dev)
+static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev,
+			       struct packet_type *pt,
+			       struct net_device *orig_dev)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_switch *ds;
@@ -158,20 +159,11 @@ static int dsa_rcv(struct sk_buff *skb, struct net_device *dev,
 	}
 
 	skb->dev = ds->ports[source_port].netdev;
-	skb_push(skb, ETH_HLEN);
-	skb->pkt_type = PACKET_HOST;
-	skb->protocol = eth_type_trans(skb, skb->dev);
 
-	skb->dev->stats.rx_packets++;
-	skb->dev->stats.rx_bytes += skb->len;
-
-	netif_receive_skb(skb);
-
-	return 0;
+	return skb;
 
 out_drop:
-	kfree_skb(skb);
-	return 0;
+	return NULL;
 }
 
 const struct dsa_device_ops dsa_netdev_ops = {
diff --git a/net/dsa/tag_edsa.c b/net/dsa/tag_edsa.c
index b87009672b40..d9c668aa5e54 100644
--- a/net/dsa/tag_edsa.c
+++ b/net/dsa/tag_edsa.c
@@ -81,8 +81,9 @@ static struct sk_buff *edsa_xmit(struct sk_buff *skb, struct net_device *dev)
 	return NULL;
 }
 
-static int edsa_rcv(struct sk_buff *skb, struct net_device *dev,
-		    struct packet_type *pt, struct net_device *orig_dev)
+static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev,
+				struct packet_type *pt,
+				struct net_device *orig_dev)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_switch *ds;
@@ -177,20 +178,11 @@ static int edsa_rcv(struct sk_buff *skb, struct net_device *dev,
 	}
 
 	skb->dev = ds->ports[source_port].netdev;
-	skb_push(skb, ETH_HLEN);
-	skb->pkt_type = PACKET_HOST;
-	skb->protocol = eth_type_trans(skb, skb->dev);
 
-	skb->dev->stats.rx_packets++;
-	skb->dev->stats.rx_bytes += skb->len;
-
-	netif_receive_skb(skb);
-
-	return 0;
+	return skb;
 
 out_drop:
-	kfree_skb(skb);
-	return 0;
+	return NULL;
 }
 
 const struct dsa_device_ops edsa_netdev_ops = {
diff --git a/net/dsa/tag_qca.c b/net/dsa/tag_qca.c
index d1324649808c..3ba3f59f7a34 100644
--- a/net/dsa/tag_qca.c
+++ b/net/dsa/tag_qca.c
@@ -66,8 +66,9 @@ static struct sk_buff *qca_tag_xmit(struct sk_buff *skb, struct net_device *dev)
 	return NULL;
 }
 
-static int qca_tag_rcv(struct sk_buff *skb, struct net_device *dev,
-		       struct packet_type *pt, struct net_device *orig_dev)
+static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev,
+				   struct packet_type *pt,
+				   struct net_device *orig_dev)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_switch *ds;
@@ -108,21 +109,12 @@ static int qca_tag_rcv(struct sk_buff *skb, struct net_device *dev,
 		goto out_drop;
 
 	/* Update skb & forward the frame accordingly */
-	skb_push(skb, ETH_HLEN);
-	skb->pkt_type = PACKET_HOST;
 	skb->dev = ds->ports[port].netdev;
-	skb->protocol = eth_type_trans(skb, skb->dev);
 
-	skb->dev->stats.rx_packets++;
-	skb->dev->stats.rx_bytes += skb->len;
-
-	netif_receive_skb(skb);
-
-	return 0;
+	return skb;
 
 out_drop:
-	kfree_skb(skb);
-	return 0;
+	return NULL;
 }
 
 const struct dsa_device_ops qca_netdev_ops = {
diff --git a/net/dsa/tag_trailer.c b/net/dsa/tag_trailer.c
index 1fc0b221a70f..aafc2fc74c30 100644
--- a/net/dsa/tag_trailer.c
+++ b/net/dsa/tag_trailer.c
@@ -58,8 +58,9 @@ static struct sk_buff *trailer_xmit(struct sk_buff *skb, struct net_device *dev)
 	return nskb;
 }
 
-static int trailer_rcv(struct sk_buff *skb, struct net_device *dev,
-		       struct packet_type *pt, struct net_device *orig_dev)
+static struct sk_buff *trailer_rcv(struct sk_buff *skb, struct net_device *dev,
+				   struct packet_type *pt,
+				   struct net_device *orig_dev)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_switch *ds;
@@ -83,20 +84,11 @@ static int trailer_rcv(struct sk_buff *skb, struct net_device *dev,
 	pskb_trim_rcsum(skb, skb->len - 4);
 
 	skb->dev = ds->ports[source_port].netdev;
-	skb_push(skb, ETH_HLEN);
-	skb->pkt_type = PACKET_HOST;
-	skb->protocol = eth_type_trans(skb, skb->dev);
 
-	skb->dev->stats.rx_packets++;
-	skb->dev->stats.rx_bytes += skb->len;
-
-	netif_receive_skb(skb);
-
-	return 0;
+	return skb;
 
 out_drop:
-	kfree_skb(skb);
-	return 0;
+	return NULL;
 }
 
 const struct dsa_device_ops trailer_netdev_ops = {
-- 
2.9.3

Re: [PATCH net-next 0/3] net: dsa: Receive path simplifications

[Andrew Lunn]

On Fri, Apr 07, 2017 at 01:41:51PM -0700, Florian Fainelli wrote:
> Hi all,
> 
> This patch series does factor the common code found in all tag implementations
> into dsa_switch_rcv(). The original motivation was to add GRO support, but this
> may be a lot of work with unclear benefits at this point.
> 
> Florian Fainelli (3):
>   net: dsa: Do not check for NULL dst in tag parsers
>   net: dsa: Move skb_unshare() to dsa_switch_rcv()
>   net: dsa: Factor bottom tag receive functions

Hey, are you looking in my git repo and stealing my patches? I have a
pretty much identical set :-)

I was holding off for the microchip and mediatek drivers to land, so
to not cause conflicts.

       Andrew

Re: [PATCH net-next 1/3] net: dsa: Do not check for NULL dst in tag parsers

[Vivien Didelot]

Florian Fainelli <f.fainelli@gmail.com> writes:

> dsa_switch_rcv() already tests for dst == NULL, so there is no need to duplicate
> the same check within the tag receive functions.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Re: [PATCH net-next 2/3] net: dsa: Move skb_unshare() to dsa_switch_rcv()

[Vivien Didelot]

Florian Fainelli <f.fainelli@gmail.com> writes:

> All DSA tag receive functions need to unshare the skb before mangling it, move
> this to the generic dsa_switch_rcv() function which will allow us to make the
> tag receive function return their mangled skb without caring about freeing a
> NULL skb.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Re: [PATCH net-next 3/3] net: dsa: Factor bottom tag receive functions

[Vivien Didelot]

Florian Fainelli <f.fainelli@gmail.com> writes:

> All DSA tag receive functions do strictly the same thing after they have located
> the originating source port from their tag specific protocol:
>
> - push ETH_HLEN bytes
> - set pkt_type to PACKET_HOST
> - call eth_type_trans()
> - bump up counters
> - call netif_receive_skb()
>
> Factor all of that into dsa_switch_rcv(). This also makes us return a pointer to
> a sk_buff, which makes us symetric with the xmit function.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Re: [PATCH net-next 1/3] net: dsa: Do not check for NULL dst in tag parsers

[Andrew Lunn]

On Fri, Apr 07, 2017 at 01:41:52PM -0700, Florian Fainelli wrote:
> dsa_switch_rcv() already tests for dst == NULL, so there is no need to duplicate
> the same check within the tag receive functions.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

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

    Andrew

Re: [PATCH net-next 0/3] net: dsa: Receive path simplifications

[Florian Fainelli]

On 04/07/2017 01:53 PM, Andrew Lunn wrote:
> On Fri, Apr 07, 2017 at 01:41:51PM -0700, Florian Fainelli wrote:
>> Hi all,
>>
>> This patch series does factor the common code found in all tag implementations
>> into dsa_switch_rcv(). The original motivation was to add GRO support, but this
>> may be a lot of work with unclear benefits at this point.
>>
>> Florian Fainelli (3):
>>   net: dsa: Do not check for NULL dst in tag parsers
>>   net: dsa: Move skb_unshare() to dsa_switch_rcv()
>>   net: dsa: Factor bottom tag receive functions
> 
> Hey, are you looking in my git repo and stealing my patches? I have a
> pretty much identical set :-)

Actually, I did not at all, good thing we are at the same conference and
not talking about that :)

> 
> I was holding off for the microchip and mediatek drivers to land, so
> to not cause conflicts.

Yes, that's a good point, this just became slightly obsolete since the
mediatek tag code got included.
-- 
Florian

Re: [PATCH net-next 2/3] net: dsa: Move skb_unshare() to dsa_switch_rcv()

[Andrew Lunn]

On Fri, Apr 07, 2017 at 01:41:53PM -0700, Florian Fainelli wrote:
> All DSA tag receive functions need to unshare the skb before mangling it, move
> this to the generic dsa_switch_rcv() function which will allow us to make the
> tag receive function return their mangled skb without caring about freeing a
> NULL skb.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

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

    Andrew

Re: [PATCH net-next 0/3] net: dsa: Receive path simplifications

[Andrew Lunn]

> Yes, that's a good point, this just became slightly obsolete since the
> mediatek tag code got included.

Yes, rebase, add the received tags, and Dave can merge it before the
next talk ends :-)

     Andrew

Re: [PATCH net-next 0/3] net: dsa: Receive path simplifications

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri,  7 Apr 2017 13:41:51 -0700

> This patch series does factor the common code found in all tag implementations
> into dsa_switch_rcv(). The original motivation was to add GRO support, but this
> may be a lot of work with unclear benefits at this point.

Let's get the other new DSA driver merged and then you can respin this and
make sure all the new tag drivers get handled properly.

Re: [PATCH net-next 3/3] net: dsa: Factor bottom tag receive functions

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2173 bytes --]

Hi Florian,

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Florian-Fainelli/net-dsa-Receive-path-simplifications/20170408-074503
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

All errors (new ones prefixed by >>):

>> net//dsa/tag_mtk.c:117:9: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
     .rcv = mtk_tag_rcv,
            ^~~~~~~~~~~
   net//dsa/tag_mtk.c:117:9: note: (near initialization for 'mtk_netdev_ops.rcv')
   cc1: some warnings being treated as errors

vim +117 net//dsa/tag_mtk.c

5cd8985a Sean Wang 2017-04-07  101  
5cd8985a Sean Wang 2017-04-07  102  	skb->dev->stats.rx_packets++;
5cd8985a Sean Wang 2017-04-07  103  	skb->dev->stats.rx_bytes += skb->len;
5cd8985a Sean Wang 2017-04-07  104  
5cd8985a Sean Wang 2017-04-07  105  	netif_receive_skb(skb);
5cd8985a Sean Wang 2017-04-07  106  
5cd8985a Sean Wang 2017-04-07  107  	return 0;
5cd8985a Sean Wang 2017-04-07  108  
5cd8985a Sean Wang 2017-04-07  109  out_drop:
5cd8985a Sean Wang 2017-04-07  110  	kfree_skb(skb);
5cd8985a Sean Wang 2017-04-07  111  out:
5cd8985a Sean Wang 2017-04-07  112  	return 0;
5cd8985a Sean Wang 2017-04-07  113  }
5cd8985a Sean Wang 2017-04-07  114  
5cd8985a Sean Wang 2017-04-07  115  const struct dsa_device_ops mtk_netdev_ops = {
5cd8985a Sean Wang 2017-04-07  116  	.xmit	= mtk_tag_xmit,
5cd8985a Sean Wang 2017-04-07 @117  	.rcv	= mtk_tag_rcv,
5cd8985a Sean Wang 2017-04-07  118  };

:::::: The code at line 117 was first introduced by commit
:::::: 5cd8985a19090f2b0ce8700ae3ec19e06a4fc5e9 net-next: dsa: add Mediatek tag RX/TX handler

:::::: TO: Sean Wang <sean.wang@mediatek.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 46816 bytes --]

Re: [PATCH net-next 3/3] net: dsa: Factor bottom tag receive functions

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2094 bytes --]

Hi Florian,

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Florian-Fainelli/net-dsa-Receive-path-simplifications/20170408-074503
config: tile-allmodconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 4.6.2
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=tile 

All warnings (new ones prefixed by >>):

>> net//dsa/tag_mtk.c:117:2: warning: initialization from incompatible pointer type [enabled by default]
   net//dsa/tag_mtk.c:117:2: warning: (near initialization for 'mtk_netdev_ops.rcv') [enabled by default]

vim +117 net//dsa/tag_mtk.c

5cd8985a Sean Wang 2017-04-07  101  
5cd8985a Sean Wang 2017-04-07  102  	skb->dev->stats.rx_packets++;
5cd8985a Sean Wang 2017-04-07  103  	skb->dev->stats.rx_bytes += skb->len;
5cd8985a Sean Wang 2017-04-07  104  
5cd8985a Sean Wang 2017-04-07  105  	netif_receive_skb(skb);
5cd8985a Sean Wang 2017-04-07  106  
5cd8985a Sean Wang 2017-04-07  107  	return 0;
5cd8985a Sean Wang 2017-04-07  108  
5cd8985a Sean Wang 2017-04-07  109  out_drop:
5cd8985a Sean Wang 2017-04-07  110  	kfree_skb(skb);
5cd8985a Sean Wang 2017-04-07  111  out:
5cd8985a Sean Wang 2017-04-07  112  	return 0;
5cd8985a Sean Wang 2017-04-07  113  }
5cd8985a Sean Wang 2017-04-07  114  
5cd8985a Sean Wang 2017-04-07  115  const struct dsa_device_ops mtk_netdev_ops = {
5cd8985a Sean Wang 2017-04-07  116  	.xmit	= mtk_tag_xmit,
5cd8985a Sean Wang 2017-04-07 @117  	.rcv	= mtk_tag_rcv,
5cd8985a Sean Wang 2017-04-07  118  };

:::::: The code at line 117 was first introduced by commit
:::::: 5cd8985a19090f2b0ce8700ae3ec19e06a4fc5e9 net-next: dsa: add Mediatek tag RX/TX handler

:::::: TO: Sean Wang <sean.wang@mediatek.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48092 bytes --]