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
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 <[email protected]>
Signed-off-by: Shashidhar Lakkavalli <[email protected]>
Signed-off-by: John Crispin <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Shashidhar Lakkavalli <[email protected]>
Signed-off-by: John Crispin <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Shashidhar Lakkavalli <[email protected]>
Signed-off-by: John Crispin <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Shashidhar Lakkavalli <[email protected]>
Signed-off-by: John Crispin <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Shashidhar Lakkavalli <[email protected]>
> Signed-off-by: John Crispin <[email protected]>
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
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 <[email protected]>
> Signed-off-by: Shashidhar Lakkavalli <[email protected]>
> Signed-off-by: John Crispin <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
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 <[email protected]>
> Signed-off-by: Shashidhar Lakkavalli <[email protected]>
> Signed-off-by: John Crispin <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
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 <[email protected]>
> Signed-off-by: Shashidhar Lakkavalli <[email protected]>
> Signed-off-by: John Crispin <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
Hi John,
John Crispin <[email protected]> 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 <[email protected]>
Thanks,
Vivien
From: John Crispin <[email protected]>
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.
On Wed, 2017-08-09 at 22:52 -0700, David Miller wrote:
> From: John Crispin <[email protected]>
> 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
On 10/08/17 08:42, Eric Dumazet wrote:
> On Wed, 2017-08-09 at 22:52 -0700, David Miller wrote:
>> From: John Crispin <[email protected]>
>> 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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/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 <[email protected]>
>> 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