From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752683AbdG1TlB (ORCPT ); Fri, 28 Jul 2017 15:41:01 -0400 Received: from nbd.name ([46.4.11.11]:55100 "EHLO nbd.name" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752505AbdG1Tk7 (ORCPT ); Fri, 28 Jul 2017 15:40:59 -0400 Subject: Re: [PATCH V2 3/4] net-next: dsa: fix flow dissection To: Andrew Lunn References: <20170721085813.30789-1-john@phrozen.org> <20170721085813.30789-3-john@phrozen.org> <20170726151013.GD12049@lunn.ch> Cc: Vivien Didelot , Florian Fainelli , "David S . Miller" , netdev@vger.kernel.org, linux-kernel@vger.kernel.org From: John Crispin Message-ID: <82b46393-6c63-eedf-25e4-2bd2278c0d16@phrozen.org> Date: Fri, 28 Jul 2017 21:40:42 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.6.0 MIME-Version: 1.0 In-Reply-To: <20170726151013.GD12049@lunn.ch> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 26/07/17 17:10, Andrew Lunn wrote: > On Fri, Jul 21, 2017 at 10:58:12AM +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. >> >> This patch makes the flow dissector honour the nh and protocol offset >> defined by the dsa tag driver thus fixing dissection, hashing and RPS. >> >> Signed-off-by: John Crispin >> --- >> 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 fc5fc4594c90..1268ae75c3b3 100644 >> --- a/net/core/flow_dissector.c >> +++ b/net/core/flow_dissector.c >> @@ -4,6 +4,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -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; >> + u8 *p = (u8 *)data; >> + >> + ops = skb->dev->dsa_ptr->tag_ops; >> + if (ops->hash_proto_off) >> + proto = (u16)p[ops->hash_proto_off]; > Hi John > > Unfortunately, this is not generic enough to work for DSA and EDSA > tagging. With these tagging protocols, the size of the tag depends on > the presence or not of a VLAN header. > > To make this work for all tagging protocols, we are going to need to > add an a new op to tag_ops. > > Andrew Hi Andrew, thanks for the feedback. should I add 2 callbacks for each of the 2 parameters ? John