All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] net-next: dsa: fix flow dissection
@ 2017-08-09 12:41 ` John Crispin
  0 siblings, 0 replies; 21+ messages in thread
From: John Crispin @ 2017-08-09 12:41 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S . Miller,
	Sean Wang
  Cc: netdev, linux-mediatek, linux-kernel, Muciri Gatimu,
	Shashidhar Lakkavalli, John Crispin

RPS and probably other kernel features are currently broken on some if not
all DSA devices. The root cause of this is that skb_hash will call the
flow_dissector. At this point the skb still contains the magic switch
header and the skb->protocol field is not set up to the correct 802.3
value yet. By the time the tag specific code is called, removing the header
and properly setting the protocol an invalid hash is already set. In the
case of the mt7530 this will result in all flows always having the same
hash.

Changes since RFC:
* use a callback instead of static values
* add cover letter

John Crispin (4):
  net-next: dsa: move struct dsa_device_ops to the global header file
  net-next: dsa: add flow_dissect callback to struct dsa_device_ops
  net-next: tag_mtk: add flow_dissect callback to the ops struct
  net-next: dsa: fix flow dissection

 include/net/dsa.h         |  9 +++++++++
 net/core/flow_dissector.c | 12 ++++++++++++
 net/dsa/dsa_priv.h        |  7 -------
 net/dsa/tag_mtk.c         | 14 ++++++++++++--
 4 files changed, 33 insertions(+), 9 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 0/4] net-next: dsa: fix flow dissection
@ 2017-08-09 12:41 ` John Crispin
  0 siblings, 0 replies; 21+ messages in thread
From: John Crispin @ 2017-08-09 12:41 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S . Miller,
	Sean Wang
  Cc: Muciri Gatimu, netdev-u79uwXL29TY76Z2rM5mHXA,
	Shashidhar Lakkavalli, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, John Crispin

RPS and probably other kernel features are currently broken on some if not
all DSA devices. The root cause of this is that skb_hash will call the
flow_dissector. At this point the skb still contains the magic switch
header and the skb->protocol field is not set up to the correct 802.3
value yet. By the time the tag specific code is called, removing the header
and properly setting the protocol an invalid hash is already set. In the
case of the mt7530 this will result in all flows always having the same
hash.

Changes since RFC:
* use a callback instead of static values
* add cover letter

John Crispin (4):
  net-next: dsa: move struct dsa_device_ops to the global header file
  net-next: dsa: add flow_dissect callback to struct dsa_device_ops
  net-next: tag_mtk: add flow_dissect callback to the ops struct
  net-next: dsa: fix flow dissection

 include/net/dsa.h         |  9 +++++++++
 net/core/flow_dissector.c | 12 ++++++++++++
 net/dsa/dsa_priv.h        |  7 -------
 net/dsa/tag_mtk.c         | 14 ++++++++++++--
 4 files changed, 33 insertions(+), 9 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/4] net-next: dsa: move struct dsa_device_ops to the global header file
  2017-08-09 12:41 ` John Crispin
  (?)
@ 2017-08-09 12:41 ` John Crispin
  2017-08-09 13:45     ` Andrew Lunn
  -1 siblings, 1 reply; 21+ messages in thread
From: John Crispin @ 2017-08-09 12:41 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S . Miller,
	Sean Wang
  Cc: netdev, linux-mediatek, linux-kernel, Muciri Gatimu,
	Shashidhar Lakkavalli, John Crispin

We need to access this struct from within the flow_dissector to fix
dissection for packets coming in on DSA devices.

Signed-off-by: Muciri Gatimu <muciri@openmesh.com>
Signed-off-by: Shashidhar Lakkavalli <shashidhar.lakkavalli@openmesh.com>
Signed-off-by: John Crispin <john@phrozen.org>
---
 include/net/dsa.h  | 7 +++++++
 net/dsa/dsa_priv.h | 7 -------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index a4f66dbb4b7c..65d7804c6f69 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -101,6 +101,13 @@ struct dsa_platform_data {
 
 struct packet_type;
 
+struct dsa_device_ops {
+	struct sk_buff *(*xmit)(struct sk_buff *skb, struct net_device *dev);
+	struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev,
+			       struct packet_type *pt,
+			       struct net_device *orig_dev);
+};
+
 struct dsa_switch_tree {
 	struct list_head	list;
 
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 68c63d4b989c..9fff7f4c8689 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -65,13 +65,6 @@ struct dsa_notifier_vlan_info {
 	int port;
 };
 
-struct dsa_device_ops {
-	struct sk_buff *(*xmit)(struct sk_buff *skb, struct net_device *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 {
 	/* Copy of dp->ds->dst->tag_ops->xmit for faster access in hot path */
 	struct sk_buff *	(*xmit)(struct sk_buff *skb,
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/4] net-next: dsa: add flow_dissect callback to struct dsa_device_ops
@ 2017-08-09 12:41   ` John Crispin
  0 siblings, 0 replies; 21+ messages in thread
From: John Crispin @ 2017-08-09 12:41 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S . Miller,
	Sean Wang
  Cc: netdev, linux-mediatek, linux-kernel, Muciri Gatimu,
	Shashidhar Lakkavalli, John Crispin

When the flow dissector first sees packets coming in on a DSA devices the
802.3 header wont be located where the code expects it to be as the tag
is still present. Adding this new callback allows a DSA device to provide a
new function that the flow_dissector can use to get the correct protocol
and offset of the network header.

Signed-off-by: Muciri Gatimu <muciri@openmesh.com>
Signed-off-by: Shashidhar Lakkavalli <shashidhar.lakkavalli@openmesh.com>
Signed-off-by: John Crispin <john@phrozen.org>
---
 include/net/dsa.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 65d7804c6f69..7f46b521313e 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -106,6 +106,8 @@ struct dsa_device_ops {
 	struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev,
 			       struct packet_type *pt,
 			       struct net_device *orig_dev);
+	int (*flow_dissect)(const struct sk_buff *skb, __be16 *proto,
+			    int *offset);
 };
 
 struct dsa_switch_tree {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/4] net-next: dsa: add flow_dissect callback to struct dsa_device_ops
@ 2017-08-09 12:41   ` John Crispin
  0 siblings, 0 replies; 21+ messages in thread
From: John Crispin @ 2017-08-09 12:41 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S . Miller,
	Sean Wang
  Cc: Muciri Gatimu, netdev-u79uwXL29TY76Z2rM5mHXA,
	Shashidhar Lakkavalli, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, John Crispin

When the flow dissector first sees packets coming in on a DSA devices the
802.3 header wont be located where the code expects it to be as the tag
is still present. Adding this new callback allows a DSA device to provide a
new function that the flow_dissector can use to get the correct protocol
and offset of the network header.

Signed-off-by: Muciri Gatimu <muciri-lv6y7wLVQPlWk0Htik3J/w@public.gmane.org>
Signed-off-by: Shashidhar Lakkavalli <shashidhar.lakkavalli-lv6y7wLVQPlWk0Htik3J/w@public.gmane.org>
Signed-off-by: John Crispin <john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org>
---
 include/net/dsa.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 65d7804c6f69..7f46b521313e 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -106,6 +106,8 @@ struct dsa_device_ops {
 	struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev,
 			       struct packet_type *pt,
 			       struct net_device *orig_dev);
+	int (*flow_dissect)(const struct sk_buff *skb, __be16 *proto,
+			    int *offset);
 };
 
 struct dsa_switch_tree {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 3/4] net-next: tag_mtk: add flow_dissect callback to the ops struct
  2017-08-09 12:41 ` John Crispin
                   ` (2 preceding siblings ...)
  (?)
@ 2017-08-09 12:41 ` John Crispin
  2017-08-09 13:54   ` Andrew Lunn
  -1 siblings, 1 reply; 21+ messages in thread
From: John Crispin @ 2017-08-09 12:41 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S . Miller,
	Sean Wang
  Cc: netdev, linux-mediatek, linux-kernel, Muciri Gatimu,
	Shashidhar Lakkavalli, John Crispin

The MT7530 inserts the 4 magic header in between the 802.3 address and
protocol field. The patch implements the callback that can be called by
the flow dissector to figure out the real protocol and offset of the
network header. With this patch applied we can properly parse the packet
and thus make hashing function properly.

Signed-off-by: Muciri Gatimu <muciri@openmesh.com>
Signed-off-by: Shashidhar Lakkavalli <shashidhar.lakkavalli@openmesh.com>
Signed-off-by: John Crispin <john@phrozen.org>
---
 net/dsa/tag_mtk.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c
index 2f32b7ea3365..02163c045a96 100644
--- a/net/dsa/tag_mtk.c
+++ b/net/dsa/tag_mtk.c
@@ -87,7 +87,17 @@ static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev,
 	return skb;
 }
 
+static int mtk_tag_flow_dissect(const struct sk_buff *skb, __be16 *proto,
+				int *offset)
+{
+	*offset = 4;
+	*proto = ((__be16 *)skb->data)[1];
+
+	return 0;
+}
+
 const struct dsa_device_ops mtk_netdev_ops = {
-	.xmit	= mtk_tag_xmit,
-	.rcv	= mtk_tag_rcv,
+	.xmit		= mtk_tag_xmit,
+	.rcv		= mtk_tag_rcv,
+	.flow_dissect	= mtk_tag_flow_dissect,
 };
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 4/4] net-next: dsa: fix flow dissection
@ 2017-08-09 12:41   ` John Crispin
  0 siblings, 0 replies; 21+ messages in thread
From: John Crispin @ 2017-08-09 12:41 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S . Miller,
	Sean Wang
  Cc: netdev, linux-mediatek, linux-kernel, Muciri Gatimu,
	Shashidhar Lakkavalli, John Crispin

RPS and probably other kernel features are currently broken on some if not
all DSA devices. The root cause of this is that skb_hash will call the
flow_dissector. At this point the skb still contains the magic switch
header and the skb->protocol field is not set up to the correct 802.3
value yet. By the time the tag specific code is called, removing the header
and properly setting the protocol an invalid hash is already set. In the
case of the mt7530 this will result in all flows always having the same
hash.

Signed-off-by: Muciri Gatimu <muciri@openmesh.com>
Signed-off-by: Shashidhar Lakkavalli <shashidhar.lakkavalli@openmesh.com>
Signed-off-by: John Crispin <john@phrozen.org>
---
 net/core/flow_dissector.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 0cc672aba1f0..5b5be9577257 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -4,6 +4,7 @@
 #include <linux/ip.h>
 #include <linux/ipv6.h>
 #include <linux/if_vlan.h>
+#include <net/dsa.h>
 #include <net/ip.h>
 #include <net/ipv6.h>
 #include <net/gre.h>
@@ -440,6 +441,17 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 			 skb->vlan_proto : skb->protocol;
 		nhoff = skb_network_offset(skb);
 		hlen = skb_headlen(skb);
+		if (unlikely(netdev_uses_dsa(skb->dev))) {
+			const struct dsa_device_ops *ops;
+			int offset;
+
+			ops = skb->dev->dsa_ptr->tag_ops;
+			if (ops->flow_dissect &&
+			    !ops->flow_dissect(skb, &proto, &offset)) {
+				hlen -= offset;
+				nhoff += offset;
+			}
+		}
 	}
 
 	/* It is ensured by skb_flow_dissector_init() that control key will
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 4/4] net-next: dsa: fix flow dissection
@ 2017-08-09 12:41   ` John Crispin
  0 siblings, 0 replies; 21+ messages in thread
From: John Crispin @ 2017-08-09 12:41 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S . Miller,
	Sean Wang
  Cc: Muciri Gatimu, netdev-u79uwXL29TY76Z2rM5mHXA,
	Shashidhar Lakkavalli, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, John Crispin

RPS and probably other kernel features are currently broken on some if not
all DSA devices. The root cause of this is that skb_hash will call the
flow_dissector. At this point the skb still contains the magic switch
header and the skb->protocol field is not set up to the correct 802.3
value yet. By the time the tag specific code is called, removing the header
and properly setting the protocol an invalid hash is already set. In the
case of the mt7530 this will result in all flows always having the same
hash.

Signed-off-by: Muciri Gatimu <muciri-lv6y7wLVQPlWk0Htik3J/w@public.gmane.org>
Signed-off-by: Shashidhar Lakkavalli <shashidhar.lakkavalli-lv6y7wLVQPlWk0Htik3J/w@public.gmane.org>
Signed-off-by: John Crispin <john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org>
---
 net/core/flow_dissector.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 0cc672aba1f0..5b5be9577257 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -4,6 +4,7 @@
 #include <linux/ip.h>
 #include <linux/ipv6.h>
 #include <linux/if_vlan.h>
+#include <net/dsa.h>
 #include <net/ip.h>
 #include <net/ipv6.h>
 #include <net/gre.h>
@@ -440,6 +441,17 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 			 skb->vlan_proto : skb->protocol;
 		nhoff = skb_network_offset(skb);
 		hlen = skb_headlen(skb);
+		if (unlikely(netdev_uses_dsa(skb->dev))) {
+			const struct dsa_device_ops *ops;
+			int offset;
+
+			ops = skb->dev->dsa_ptr->tag_ops;
+			if (ops->flow_dissect &&
+			    !ops->flow_dissect(skb, &proto, &offset)) {
+				hlen -= offset;
+				nhoff += offset;
+			}
+		}
 	}
 
 	/* It is ensured by skb_flow_dissector_init() that control key will
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/4] net-next: dsa: move struct dsa_device_ops to the global header file
@ 2017-08-09 13:45     ` Andrew Lunn
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Lunn @ 2017-08-09 13:45 UTC (permalink / raw)
  To: John Crispin
  Cc: Vivien Didelot, Florian Fainelli, David S . Miller, Sean Wang,
	netdev, linux-mediatek, linux-kernel, Muciri Gatimu,
	Shashidhar Lakkavalli

On Wed, Aug 09, 2017 at 02:41:16PM +0200, John Crispin wrote:
> We need to access this struct from within the flow_dissector to fix
> dissection for packets coming in on DSA devices.
> 
> Signed-off-by: Muciri Gatimu <muciri@openmesh.com>
> Signed-off-by: Shashidhar Lakkavalli <shashidhar.lakkavalli@openmesh.com>
> Signed-off-by: John Crispin <john@phrozen.org>

Hi John

Please add the Reviewed-by: tags you receive from previous
versions. It makes it easier for reviewers to see what actually needs
reviewing.

Thanks
	Andrew

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/4] net-next: dsa: move struct dsa_device_ops to the global header file
@ 2017-08-09 13:45     ` Andrew Lunn
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Lunn @ 2017-08-09 13:45 UTC (permalink / raw)
  To: John Crispin
  Cc: Florian Fainelli, Muciri Gatimu, Vivien Didelot,
	netdev-u79uwXL29TY76Z2rM5mHXA, Sean Wang,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Shashidhar Lakkavalli, David S . Miller

On Wed, Aug 09, 2017 at 02:41:16PM +0200, John Crispin wrote:
> We need to access this struct from within the flow_dissector to fix
> dissection for packets coming in on DSA devices.
> 
> Signed-off-by: Muciri Gatimu <muciri-lv6y7wLVQPlWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Shashidhar Lakkavalli <shashidhar.lakkavalli-lv6y7wLVQPlWk0Htik3J/w@public.gmane.org>
> Signed-off-by: John Crispin <john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org>

Hi John

Please add the Reviewed-by: tags you receive from previous
versions. It makes it easier for reviewers to see what actually needs
reviewing.

Thanks
	Andrew

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/4] net-next: dsa: add flow_dissect callback to struct dsa_device_ops
@ 2017-08-09 13:51     ` Andrew Lunn
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Lunn @ 2017-08-09 13:51 UTC (permalink / raw)
  To: John Crispin
  Cc: Vivien Didelot, Florian Fainelli, David S . Miller, Sean Wang,
	netdev, linux-mediatek, linux-kernel, Muciri Gatimu,
	Shashidhar Lakkavalli

On Wed, Aug 09, 2017 at 02:41:17PM +0200, John Crispin wrote:
> When the flow dissector first sees packets coming in on a DSA devices the
> 802.3 header wont be located where the code expects it to be as the tag
> is still present. Adding this new callback allows a DSA device to provide a
> new function that the flow_dissector can use to get the correct protocol
> and offset of the network header.
> 
> Signed-off-by: Muciri Gatimu <muciri@openmesh.com>
> Signed-off-by: Shashidhar Lakkavalli <shashidhar.lakkavalli@openmesh.com>
> Signed-off-by: John Crispin <john@phrozen.org>

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

    Andrew

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/4] net-next: dsa: add flow_dissect callback to struct dsa_device_ops
@ 2017-08-09 13:51     ` Andrew Lunn
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Lunn @ 2017-08-09 13:51 UTC (permalink / raw)
  To: John Crispin
  Cc: Florian Fainelli, Muciri Gatimu, Vivien Didelot,
	netdev-u79uwXL29TY76Z2rM5mHXA, Sean Wang,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Shashidhar Lakkavalli, David S . Miller

On Wed, Aug 09, 2017 at 02:41:17PM +0200, John Crispin wrote:
> When the flow dissector first sees packets coming in on a DSA devices the
> 802.3 header wont be located where the code expects it to be as the tag
> is still present. Adding this new callback allows a DSA device to provide a
> new function that the flow_dissector can use to get the correct protocol
> and offset of the network header.
> 
> Signed-off-by: Muciri Gatimu <muciri-lv6y7wLVQPlWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Shashidhar Lakkavalli <shashidhar.lakkavalli-lv6y7wLVQPlWk0Htik3J/w@public.gmane.org>
> Signed-off-by: John Crispin <john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org>

Reviewed-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>

    Andrew

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/4] net-next: tag_mtk: add flow_dissect callback to the ops struct
  2017-08-09 12:41 ` [PATCH 3/4] net-next: tag_mtk: add flow_dissect callback to the ops struct John Crispin
@ 2017-08-09 13:54   ` Andrew Lunn
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Lunn @ 2017-08-09 13:54 UTC (permalink / raw)
  To: John Crispin
  Cc: Vivien Didelot, Florian Fainelli, David S . Miller, Sean Wang,
	netdev, linux-mediatek, linux-kernel, Muciri Gatimu,
	Shashidhar Lakkavalli

On Wed, Aug 09, 2017 at 02:41:18PM +0200, John Crispin wrote:
> The MT7530 inserts the 4 magic header in between the 802.3 address and
> protocol field. The patch implements the callback that can be called by
> the flow dissector to figure out the real protocol and offset of the
> network header. With this patch applied we can properly parse the packet
> and thus make hashing function properly.
> 
> Signed-off-by: Muciri Gatimu <muciri@openmesh.com>
> Signed-off-by: Shashidhar Lakkavalli <shashidhar.lakkavalli@openmesh.com>
> Signed-off-by: John Crispin <john@phrozen.org>

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

    Andrew

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/4] net-next: dsa: fix flow dissection
@ 2017-08-09 13:55     ` Andrew Lunn
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Lunn @ 2017-08-09 13:55 UTC (permalink / raw)
  To: John Crispin
  Cc: Vivien Didelot, Florian Fainelli, David S . Miller, Sean Wang,
	netdev, linux-mediatek, linux-kernel, Muciri Gatimu,
	Shashidhar Lakkavalli

On Wed, Aug 09, 2017 at 02:41:19PM +0200, John Crispin wrote:
> RPS and probably other kernel features are currently broken on some if not
> all DSA devices. The root cause of this is that skb_hash will call the
> flow_dissector. At this point the skb still contains the magic switch
> header and the skb->protocol field is not set up to the correct 802.3
> value yet. By the time the tag specific code is called, removing the header
> and properly setting the protocol an invalid hash is already set. In the
> case of the mt7530 this will result in all flows always having the same
> hash.
> 
> Signed-off-by: Muciri Gatimu <muciri@openmesh.com>
> Signed-off-by: Shashidhar Lakkavalli <shashidhar.lakkavalli@openmesh.com>
> Signed-off-by: John Crispin <john@phrozen.org>

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

    Andrew

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/4] net-next: dsa: fix flow dissection
@ 2017-08-09 13:55     ` Andrew Lunn
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Lunn @ 2017-08-09 13:55 UTC (permalink / raw)
  To: John Crispin
  Cc: Florian Fainelli, Muciri Gatimu, Vivien Didelot,
	netdev-u79uwXL29TY76Z2rM5mHXA, Sean Wang,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Shashidhar Lakkavalli, David S . Miller

On Wed, Aug 09, 2017 at 02:41:19PM +0200, John Crispin wrote:
> RPS and probably other kernel features are currently broken on some if not
> all DSA devices. The root cause of this is that skb_hash will call the
> flow_dissector. At this point the skb still contains the magic switch
> header and the skb->protocol field is not set up to the correct 802.3
> value yet. By the time the tag specific code is called, removing the header
> and properly setting the protocol an invalid hash is already set. In the
> case of the mt7530 this will result in all flows always having the same
> hash.
> 
> Signed-off-by: Muciri Gatimu <muciri-lv6y7wLVQPlWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Shashidhar Lakkavalli <shashidhar.lakkavalli-lv6y7wLVQPlWk0Htik3J/w@public.gmane.org>
> Signed-off-by: John Crispin <john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org>

Reviewed-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>

    Andrew

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] net-next: dsa: fix flow dissection
  2017-08-09 12:41 ` John Crispin
@ 2017-08-09 15:07   ` Vivien Didelot
  -1 siblings, 0 replies; 21+ messages in thread
From: Vivien Didelot @ 2017-08-09 15:07 UTC (permalink / raw)
  To: John Crispin, Andrew Lunn, Florian Fainelli, David S . Miller, Sean Wang
  Cc: netdev, linux-mediatek, linux-kernel, Muciri Gatimu,
	Shashidhar Lakkavalli, John Crispin

Hi John,

John Crispin <john@phrozen.org> writes:

> RPS and probably other kernel features are currently broken on some if not
> all DSA devices. The root cause of this is that skb_hash will call the
> flow_dissector. At this point the skb still contains the magic switch
> header and the skb->protocol field is not set up to the correct 802.3
> value yet. By the time the tag specific code is called, removing the header
> and properly setting the protocol an invalid hash is already set. In the
> case of the mt7530 this will result in all flows always having the same
> hash.
>
> Changes since RFC:
> * use a callback instead of static values
> * add cover letter
>
> John Crispin (4):
>   net-next: dsa: move struct dsa_device_ops to the global header file
>   net-next: dsa: add flow_dissect callback to struct dsa_device_ops
>   net-next: tag_mtk: add flow_dissect callback to the ops struct
>   net-next: dsa: fix flow dissection

The "net-next" tag goes into the subject-prefix, i.e.
"[PATCH net-next v3 0/4]" (this is the result of git format-patch
--subject-prefix="PATCH net-next" -v3 --cover-letter.)

The commit title prefix represents the most impacted subsystem, here
"net: dsa: xxx" would map the net/dsa/ or drivers/net/dsa directories.

Other than that, the whole patchset LGTM:

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


Thanks,

        Vivien

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] net-next: dsa: fix flow dissection
@ 2017-08-09 15:07   ` Vivien Didelot
  0 siblings, 0 replies; 21+ messages in thread
From: Vivien Didelot @ 2017-08-09 15:07 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David S . Miller, Sean Wang
  Cc: netdev, linux-mediatek, linux-kernel, Muciri Gatimu,
	Shashidhar Lakkavalli, John Crispin

Hi John,

John Crispin <john@phrozen.org> writes:

> RPS and probably other kernel features are currently broken on some if not
> all DSA devices. The root cause of this is that skb_hash will call the
> flow_dissector. At this point the skb still contains the magic switch
> header and the skb->protocol field is not set up to the correct 802.3
> value yet. By the time the tag specific code is called, removing the header
> and properly setting the protocol an invalid hash is already set. In the
> case of the mt7530 this will result in all flows always having the same
> hash.
>
> Changes since RFC:
> * use a callback instead of static values
> * add cover letter
>
> John Crispin (4):
>   net-next: dsa: move struct dsa_device_ops to the global header file
>   net-next: dsa: add flow_dissect callback to struct dsa_device_ops
>   net-next: tag_mtk: add flow_dissect callback to the ops struct
>   net-next: dsa: fix flow dissection

The "net-next" tag goes into the subject-prefix, i.e.
"[PATCH net-next v3 0/4]" (this is the result of git format-patch
--subject-prefix="PATCH net-next" -v3 --cover-letter.)

The commit title prefix represents the most impacted subsystem, here
"net: dsa: xxx" would map the net/dsa/ or drivers/net/dsa directories.

Other than that, the whole patchset LGTM:

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


Thanks,

        Vivien

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] net-next: dsa: fix flow dissection
  2017-08-09 12:41 ` John Crispin
                   ` (5 preceding siblings ...)
  (?)
@ 2017-08-10  5:52 ` David Miller
  2017-08-10  6:42   ` Eric Dumazet
  -1 siblings, 1 reply; 21+ messages in thread
From: David Miller @ 2017-08-10  5:52 UTC (permalink / raw)
  To: john
  Cc: andrew, vivien.didelot, f.fainelli, sean.wang, netdev,
	linux-mediatek, linux-kernel, muciri, shashidhar.lakkavalli

From: John Crispin <john@phrozen.org>
Date: Wed,  9 Aug 2017 14:41:15 +0200

> RPS and probably other kernel features are currently broken on some if not
> all DSA devices. The root cause of this is that skb_hash will call the
> flow_dissector. At this point the skb still contains the magic switch
> header and the skb->protocol field is not set up to the correct 802.3
> value yet. By the time the tag specific code is called, removing the header
> and properly setting the protocol an invalid hash is already set. In the
> case of the mt7530 this will result in all flows always having the same
> hash.
> 
> Changes since RFC:
> * use a callback instead of static values
> * add cover letter

Series applied, thanks.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] net-next: dsa: fix flow dissection
  2017-08-10  5:52 ` David Miller
@ 2017-08-10  6:42   ` Eric Dumazet
  2017-08-10  7:40     ` John Crispin
  2017-08-10  8:10     ` John Crispin
  0 siblings, 2 replies; 21+ messages in thread
From: Eric Dumazet @ 2017-08-10  6:42 UTC (permalink / raw)
  To: David Miller
  Cc: john, andrew, vivien.didelot, f.fainelli, sean.wang, netdev,
	linux-mediatek, linux-kernel, muciri, shashidhar.lakkavalli

On Wed, 2017-08-09 at 22:52 -0700, David Miller wrote:
> From: John Crispin <john@phrozen.org>
> Date: Wed,  9 Aug 2017 14:41:15 +0200
> 
> > RPS and probably other kernel features are currently broken on some if not
> > all DSA devices. The root cause of this is that skb_hash will call the
> > flow_dissector. At this point the skb still contains the magic switch
> > header and the skb->protocol field is not set up to the correct 802.3
> > value yet. By the time the tag specific code is called, removing the header
> > and properly setting the protocol an invalid hash is already set. In the
> > case of the mt7530 this will result in all flows always having the same
> > hash.
> > 
> > Changes since RFC:
> > * use a callback instead of static values
> > * add cover letter
> 
> Series applied, thanks.

Is this related ?

net/core/flow_dissector.c: In function '__skb_flow_dissect':
net/core/flow_dissector.c:448:18: error: 'struct net_device' has no member named 'dsa_ptr'
    ops = skb->dev->dsa_ptr->tag_ops;
                  ^
make[3]: *** [net/core/flow_dissector.o] Error 1

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] net-next: dsa: fix flow dissection
  2017-08-10  6:42   ` Eric Dumazet
@ 2017-08-10  7:40     ` John Crispin
  2017-08-10  8:10     ` John Crispin
  1 sibling, 0 replies; 21+ messages in thread
From: John Crispin @ 2017-08-10  7:40 UTC (permalink / raw)
  To: Eric Dumazet, David Miller
  Cc: andrew, f.fainelli, muciri, vivien.didelot, netdev, sean.wang,
	linux-kernel, shashidhar.lakkavalli, linux-mediatek



On 10/08/17 08:42, Eric Dumazet wrote:
> On Wed, 2017-08-09 at 22:52 -0700, David Miller wrote:
>> From: John Crispin <john@phrozen.org>
>> Date: Wed,  9 Aug 2017 14:41:15 +0200
>>
>>> RPS and probably other kernel features are currently broken on some if not
>>> all DSA devices. The root cause of this is that skb_hash will call the
>>> flow_dissector. At this point the skb still contains the magic switch
>>> header and the skb->protocol field is not set up to the correct 802.3
>>> value yet. By the time the tag specific code is called, removing the header
>>> and properly setting the protocol an invalid hash is already set. In the
>>> case of the mt7530 this will result in all flows always having the same
>>> hash.
>>>
>>> Changes since RFC:
>>> * use a callback instead of static values
>>> * add cover letter
>> Series applied, thanks.
> Is this related ?
>
> net/core/flow_dissector.c: In function '__skb_flow_dissect':
> net/core/flow_dissector.c:448:18: error: 'struct net_device' has no member named 'dsa_ptr'
>      ops = skb->dev->dsa_ptr->tag_ops;
>                    ^
> make[3]: *** [net/core/flow_dissector.o] Error 1
>
>

looks like it, I did test the patches against net-next from 24 hours 
ago, let me do a test build just now.
     John


> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/4] net-next: dsa: fix flow dissection
  2017-08-10  6:42   ` Eric Dumazet
  2017-08-10  7:40     ` John Crispin
@ 2017-08-10  8:10     ` John Crispin
  1 sibling, 0 replies; 21+ messages in thread
From: John Crispin @ 2017-08-10  8:10 UTC (permalink / raw)
  To: Eric Dumazet, David Miller
  Cc: andrew, f.fainelli, muciri, vivien.didelot, netdev, sean.wang,
	linux-kernel, shashidhar.lakkavalli, linux-mediatek



On 10/08/17 08:42, Eric Dumazet wrote:
> On Wed, 2017-08-09 at 22:52 -0700, David Miller wrote:
>> From: John Crispin <john@phrozen.org>
>> Date: Wed,  9 Aug 2017 14:41:15 +0200
>>
>>> RPS and probably other kernel features are currently broken on some if not
>>> all DSA devices. The root cause of this is that skb_hash will call the
>>> flow_dissector. At this point the skb still contains the magic switch
>>> header and the skb->protocol field is not set up to the correct 802.3
>>> value yet. By the time the tag specific code is called, removing the header
>>> and properly setting the protocol an invalid hash is already set. In the
>>> case of the mt7530 this will result in all flows always having the same
>>> hash.
>>>
>>> Changes since RFC:
>>> * use a callback instead of static values
>>> * add cover letter
>> Series applied, thanks.
> Is this related ?
>
> net/core/flow_dissector.c: In function '__skb_flow_dissect':
> net/core/flow_dissector.c:448:18: error: 'struct net_device' has no member named 'dsa_ptr'
>      ops = skb->dev->dsa_ptr->tag_ops;
>                    ^
> make[3]: *** [net/core/flow_dissector.o] Error 1
>
>
Hi Eric,

I have just sent the fix for this compile error

     John

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2017-08-10  8:11 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-09 12:41 [PATCH 0/4] net-next: dsa: fix flow dissection John Crispin
2017-08-09 12:41 ` John Crispin
2017-08-09 12:41 ` [PATCH 1/4] net-next: dsa: move struct dsa_device_ops to the global header file John Crispin
2017-08-09 13:45   ` Andrew Lunn
2017-08-09 13:45     ` Andrew Lunn
2017-08-09 12:41 ` [PATCH 2/4] net-next: dsa: add flow_dissect callback to struct dsa_device_ops John Crispin
2017-08-09 12:41   ` John Crispin
2017-08-09 13:51   ` Andrew Lunn
2017-08-09 13:51     ` Andrew Lunn
2017-08-09 12:41 ` [PATCH 3/4] net-next: tag_mtk: add flow_dissect callback to the ops struct John Crispin
2017-08-09 13:54   ` Andrew Lunn
2017-08-09 12:41 ` [PATCH 4/4] net-next: dsa: fix flow dissection John Crispin
2017-08-09 12:41   ` John Crispin
2017-08-09 13:55   ` Andrew Lunn
2017-08-09 13:55     ` Andrew Lunn
2017-08-09 15:07 ` [PATCH 0/4] " Vivien Didelot
2017-08-09 15:07   ` Vivien Didelot
2017-08-10  5:52 ` David Miller
2017-08-10  6:42   ` Eric Dumazet
2017-08-10  7:40     ` John Crispin
2017-08-10  8:10     ` John Crispin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.