From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Pirko Subject: Re: [PATCH 00/10] add flow_rule infrastructure Date: Mon, 19 Nov 2018 10:57:58 +0100 Message-ID: <20181119095758.GA2110@nanopsycho> References: <20181116014139.25438-1-pablo@netfilter.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Pablo Neira Ayuso , "netdev@vger.kernel.org" , "davem@davemloft.net" , "thomas.lendacky@amd.com" , "f.fainelli@gmail.com" , "Elior, Ariel" , "michael.chan@broadcom.com" , "santosh@chelsio.com" , "madalin.bucur@nxp.com" , "yisen.zhuang@huawei.com" , "salil.mehta@huawei.com" , "jeffrey.t.kirsher@intel.com" , "tariqt@mellanox.com" , "saeedm@mellanox.com" , "jiri@mellanox.com" , "idosch@mellanox.com" , To: "Chopra, Manish" Return-path: Received: from mail-wr1-f68.google.com ([209.85.221.68]:36110 "EHLO mail-wr1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726287AbeKSU1s (ORCPT ); Mon, 19 Nov 2018 15:27:48 -0500 Received: by mail-wr1-f68.google.com with SMTP id t3so1119994wrr.3 for ; Mon, 19 Nov 2018 02:04:38 -0800 (PST) Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Mon, Nov 19, 2018 at 10:19:28AM CET, Manish.Chopra@cavium.com wrote: >> -----Original Message----- >> From: netdev-owner@vger.kernel.org On >> Behalf Of Pablo Neira Ayuso >> Sent: Friday, November 16, 2018 7:11 AM >> To: netdev@vger.kernel.org >> Cc: davem@davemloft.net; thomas.lendacky@amd.com; >> f.fainelli@gmail.com; Elior, Ariel ; >> michael.chan@broadcom.com; santosh@chelsio.com; >> madalin.bucur@nxp.com; yisen.zhuang@huawei.com; >> salil.mehta@huawei.com; jeffrey.t.kirsher@intel.com; tariqt@mellanox.com; >> saeedm@mellanox.com; jiri@mellanox.com; idosch@mellanox.com; >> jakub.kicinski@netronome.com; peppe.cavallaro@st.com; >> grygorii.strashko@ti.com; andrew@lunn.ch; >> vivien.didelot@savoirfairelinux.com; alexandre.torgue@st.com; >> joabreu@synopsys.com; linux-net-drivers@solarflare.com; >> ganeshgr@chelsio.com >> Subject: [PATCH 00/10] add flow_rule infrastructure >> >> External Email >> >> This patchset introduces a kernel intermediate representation (IR) to express >> ACL hardware offloads, this is heavily based on the existing flow dissector >> infrastructure and the TC actions. This IR can be used by different frontend >> ACL interfaces such as ethtool_rxnfc and tc to represent ACL hardware >> offloads. Main goal is to simplify the development of ACL hardware offloads >> for the existing frontend interfaces, the idea is that driver developers do not >> need to add one specific parser for each ACL frontend, instead each frontend >> can just generate this flow_rule IR and pass it to drivers to populate the >> hardware IR. >> >> . ethtool_rxnfc tc >> | (ioctl) (netlink) >> | | | translate native >> Frontend | | | interface representation >> | | | to flow_rule IR >> | | | >> . \/ \/ >> . flow_rule IR >> | | >> Drivers | | parsing of flow_rule IR >> | | to populate hardware IR >> | \/ >> . hardware IR (driver) >> >> For design and implementation details, please have a look at: >> >> https://lwn.net/Articles/766695/ >> >> As an example, with this patchset, it should be possible to simplify the >> existing net/qede driver which already has two parsers to populate the >> hardware IR, one for ethtool_rxnfc interface and another for tc. >> >> This batch is composed of 10 patches: >> >> Patch #1 adds the flow_match structure, this includes the >> flow_rule_match_key() interface to check for existing selectors >> that are in used in the rule and the flow_rule_match_*() >> functions to fetch the selector value and the mask. This >> also introduces the initial flow_rule structure skeleton to >> avoid a follow up patch that would update the same LoCs. >> >> Patch #2 makes changes to packet edit parser of mlx5e driver, to prepare >> introduction of the new flow_action to mangle packets. >> >> Patch #3 Introduce flow_action infrastructure. This infrastructure is >> based on the TC actions. Patch #8 extends it so it also >> supports two new actions that are only available through the >> ethtool_rxnfc interface. >> >> Patch #4 Add function to translate TC action to flow_action from >> cls_flower. >> >> Patch #5 Add infrastructure to fetch statistics into container structure >> and synchronize them to TC actions from cls_flower. Another >> preparation patch before patch #7, so we can stop exposing the >> TC action native layout to the drivers. >> >> Patch #6 Use flow_action infrastructure from drivers. >> >> Patch #7 Do not expose TC actions to drivers anymore, now that drivers >> have been converted to use the flow_action infrastructure after >> patch #5. >> >> Patch #8 Support to wake-up-on-lan and queue actions for the flow_action >> infrastructure, two actions supported by NICs. This is used by >> the ethtool_rx_flow interface. >> >> Patch #9 Add a function to translate from ethtool_rx_flow_spec structure >> to the flow_action structure. This is a simple enough for its >> first client: the ethtool_rxnfc interface in the bcm_sf2 driver. >> >> Patch #10 Update bcm_sf2 to use this new translator function and >> update codebase to configure hardware IR using the >> flow_action representation. This will allow later development >> of cls_flower using the same codebase from the driver. >> >> This patchset has passed here functional tests of the codepath that generates >> the flow_rule structure and the functions to implement the parsers that >> populate the hardware IR. >> >> Thanks. >> >> Pablo Neira Ayuso (10): >> flow_dissector: add flow_rule and flow_match structures and use them >> net/mlx5e: support for two independent packet edit actions >> flow_dissector: add flow action infrastructure >> cls_api: add translator to flow_action representation >> cls_flower: add statistics retrieval infrastructure and use it >> drivers: net: use flow action infrastructure >> cls_flower: don't expose TC actions to drivers anymore >> flow_dissector: add wake-up-on-lan and queue to flow_action >> flow_dissector: add basic ethtool_rx_flow_spec to flow_rule structure >> translator >> dsa: bcm_sf2: use flow_rule infrastructure >> >> drivers/net/dsa/bcm_sf2_cfp.c | 103 ++- >> drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 252 +++---- >> .../net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 450 ++++++------- >> drivers/net/ethernet/intel/i40e/i40e_main.c | 178 ++--- >> drivers/net/ethernet/intel/iavf/iavf_main.c | 195 +++--- >> drivers/net/ethernet/intel/igb/igb_main.c | 64 +- >> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 743 ++++++++++-------- >> --- >> drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c | 2 +- >> .../net/ethernet/mellanox/mlxsw/spectrum_flower.c | 259 ++++--- >> drivers/net/ethernet/netronome/nfp/flower/action.c | 196 +++--- >> drivers/net/ethernet/netronome/nfp/flower/match.c | 416 ++++++------ >> .../net/ethernet/netronome/nfp/flower/offload.c | 151 ++--- >> drivers/net/ethernet/qlogic/qede/qede_filter.c | 93 ++- >> include/net/flow_dissector.h | 185 +++++ >> include/net/pkt_cls.h | 23 +- >> net/core/flow_dissector.c | 341 ++++++++++ >> net/sched/cls_api.c | 113 ++++ >> net/sched/cls_flower.c | 42 +- >> 18 files changed, 2120 insertions(+), 1686 deletions(-) >> >> -- >> 2.11.0 > >Today, we have different kernel interfaces(aRFS/ethtool/tc) to do flow offloading. >One big question arises in my mind that why we need these all different interfaces to do flow offloading and >why can't they just be combined into a single kernel interface somehow. >Having these flow offloading done through various hooks makes driver implementation really complex. > >Specially when looking at ethtool/tc flows most of the use cases seems common to me - > >1. Drop action can be done on tuples from both tc/ethtool flows. >2. Flow redirection is done from both ethtool/tc > (May be with just slight difference ? as Ethtool does steering on PF and VF/VF-queues > but tc doesn't have redirection on queues which can be extended over tc) >3. Could be more such common cases which could be the reason to combine them through single interface ? Sure you can make one interface feature complete. But that does not mean you can remove the others. At least not easily. You woul break UAPI. > >Thanks !! > > >