Re: [PATCH 0/4] net-next: dsa: fix flow dissection

[John Crispin]

On 10/08/17 08:42, Eric Dumazet wrote:

On Wed, 2017-08-09 at 22:52 -0700, David Miller wrote:

From: John Crispin 
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

Re: [PATCH 0/4] net-next: dsa: fix flow dissection

[John Crispin]

On 10/08/17 08:42, Eric Dumazet wrote:

On Wed, 2017-08-09 at 22:52 -0700, David Miller wrote:

From: John Crispin 
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-media...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 0/4] net-next: dsa: fix flow dissection

[Eric Dumazet]

On Wed, 2017-08-09 at 22:52 -0700, David Miller wrote:
> From: John Crispin 
> 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

Re: [PATCH 0/4] net-next: dsa: fix flow dissection

[David Miller]

From: John Crispin 
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.

Re: [PATCH 0/4] net-next: dsa: fix flow dissection

[Vivien Didelot]

Hi John,

John Crispin  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 


Thanks,

Vivien

[PATCH 0/4] net-next: dsa: fix flow dissection

[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