Re: [PATCH 00/12 net-next,v2] add flow_rule infrastructure
Tue, Nov 20, 2018 at 06:16:40PM CET, da...@davemloft.net wrote: >From: Jiri Pirko >Date: Tue, 20 Nov 2018 08:39:12 +0100 > >> If later on the netfilter code will use it, through another >> ndo/notifier/whatever, that is side a nice side-effect in my >> opinion. > >Netfilter HW offloading is the main motivation of these changes. > >You can try to spin it any way you like, but I think this is pretty >clear. > >Would the author of these changes be even be remotely interested in >this "cleanup" in areas of code he has never been involved in if that >were not the case? No, but of course. I'm just saying that the cleanup is nice and handy even if the code would never be used by netfilter. Therefore I think the info is irrelevant for the review. Anyway, I get your point. > >I think it is very dishonest to portray the situation differently. > >Thank you.
Re: [PATCH 00/12 net-next,v2] add flow_rule infrastructure
From: Jiri Pirko Date: Tue, 20 Nov 2018 08:39:12 +0100 > If later on the netfilter code will use it, through another > ndo/notifier/whatever, that is side a nice side-effect in my > opinion. Netfilter HW offloading is the main motivation of these changes. You can try to spin it any way you like, but I think this is pretty clear. Would the author of these changes be even be remotely interested in this "cleanup" in areas of code he has never been involved in if that were not the case? I think it is very dishonest to portray the situation differently. Thank you.
Re: [PATCH 00/12 net-next,v2] add flow_rule infrastructure
Mon, Nov 19, 2018 at 09:12:29PM CET, da...@davemloft.net wrote: >From: Pablo Neira Ayuso >Date: Mon, 19 Nov 2018 01:15:07 +0100 > >> This patchset introduces a kernel intermediate representation (IR) to >> express ACL hardware offloads, as already described in previous RFC and >> v1 patchset [1] [2]. The idea is to normalize the frontend U/APIs to use >> the flow dissectors and the flow actions so drivers can reuse the >> existing TC offload driver codebase - that has been converted to use the >> flow_rule infrastructure. > >I'm go to bring up the elephant in the room. > >I think the real motivation here is to offload netfilter rules to HW, >and you should be completely honest about that. Sure, but this patchset is mainly about making the parsing code in drivers common no matter from where the "flow rule" comes. If later on the netfilter code will use it, through another ndo/notifier/whatever, that is side a nice side-effect in my opinion.
Re: [PATCH 00/12 net-next,v2] add flow_rule infrastructure
On Mon, Nov 19, 2018 at 10:14 PM David Miller wrote: > From: Pablo Neira Ayuso > Date: Mon, 19 Nov 2018 01:15:07 +0100 > > > This patchset introduces a kernel intermediate representation (IR) to > > express ACL hardware offloads, as already described in previous RFC and > > v1 patchset [1] [2]. The idea is to normalize the frontend U/APIs to use > > the flow dissectors and the flow actions so drivers can reuse the > > existing TC offload driver codebase - that has been converted to use the > > flow_rule infrastructure. > > I'm go to bring up the elephant in the room. > I think the real motivation here is to offload netfilter rules to HW, > and you should be completely honest about that. Thanks Dave for clarifying. So.. (A) why the TC path isn't enough for CT offloading? if we could have just do it, that would have sound really cool. (B) why do we have to deal with EIRs (Elephants In Rooms)? (C) who can address A && B? Or.
Re: [PATCH 00/12 net-next,v2] add flow_rule infrastructure
From: Pablo Neira Ayuso Date: Mon, 19 Nov 2018 01:15:07 +0100 > This patchset introduces a kernel intermediate representation (IR) to > express ACL hardware offloads, as already described in previous RFC and > v1 patchset [1] [2]. The idea is to normalize the frontend U/APIs to use > the flow dissectors and the flow actions so drivers can reuse the > existing TC offload driver codebase - that has been converted to use the > flow_rule infrastructure. I'm go to bring up the elephant in the room. I think the real motivation here is to offload netfilter rules to HW, and you should be completely honest about that.
Re: [PATCH 00/12 net-next,v2] add flow_rule infrastructure
On Mon, Nov 19, 2018 at 09:20:43AM +, Jose Abreu wrote: [...] > Although I was cc'ed in the thread I'm not seeing stmmac driver > in this conversion. My intention was to attract the attention of driver maintainers that are using tc offloads in some way in their infrastructure. That's why you've been Cc. > Can you please add it ? stmmac is using cls_u32, and this patchset only converts cls_flower at this stage.
Re: [PATCH 00/12 net-next,v2] add flow_rule infrastructure
Hello Pablo, On 19-11-2018 00:15, Pablo Neira Ayuso wrote: > Hi, > > This patchset introduces a kernel intermediate representation (IR) to > express ACL hardware offloads, as already described in previous RFC and > v1 patchset [1] [2]. The idea is to normalize the frontend U/APIs to use > the flow dissectors and the flow actions so drivers can reuse the > existing TC offload driver codebase - that has been converted to use the > flow_rule infrastructure. > > After this patch, as Or previously described, there is one extra layer: > > kernel frontend U/API X --> kernel parser Y --> IR --> driver --> HW API > kernel frontend U/API Z --> kernel parser W --> IR --> driver --> HW API > > However, cost of this layer is very small, adding 1 million rules via > tc -batch, perf shows: > > 0.06% tc [kernel.vmlinux][k] tc_setup_flow_action > > at position 187 in the call graph, far from the top ten. The flow_match > representation uses the flow dissector infrastructure, just like > cls_flower, therefore, there is no need for conversion of the rule match > side. > > The flow_action representation is very similar to the TC action plus > this includes wake-up-on-lan and queue to CPU actions that are needed > for the ethtool_rx_flow_spec interface in the bcm_sf2 driver, that is > converted in this patchset to use it. It is now possible to add tc > cls_flower support for bcm_sf2 and reuse the existing parser that was > originally designed for the ethtool_rx_flow_spec interface. > > As requested, this new patchset also converts qlogic/qede to use this > new infrastructure (see patch 12/12). This driver currently has two > parsers, one for ethtool_rx_flow_spec and another for tc cls_flower. > This driver supports for simple 5-tuple matching and available actions > are packet drop and queue. This patch updates the driver code to use one > single parser to populate HW IR. > > Thanks. > > [1] > https://urldefense.proofpoint.com/v2/url?u=https-3A__lwn.net_Articles_766695_&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=yaVFU4TjGY0gVF8El1uKcisy6TPsyCl9uN7Wsis-qhY&m=tB4p4lWxJv3Np8Tg05scy415_MEU7RQO3Q6KGdcTMCg&s=EUCsiqp58Et5K2u9hHsxXyF6ep1yfhnASEBdXur33oQ&e= > [2] > https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3Dlinux-2Dnetdev-26m-3D154233253114506-26w-3D2&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=yaVFU4TjGY0gVF8El1uKcisy6TPsyCl9uN7Wsis-qhY&m=tB4p4lWxJv3Np8Tg05scy415_MEU7RQO3Q6KGdcTMCg&s=1XOZeTYyELUCyM8yS76_bRQOVGOy19pnV9cH937FjkY&e= > > Pablo Neira Ayuso (12): > 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 > qede: place ethtool_rx_flow_spec after code after TC flower codebase > qede: use ethtool_rx_flow_rule() to remove duplicated parser code > > drivers/net/dsa/bcm_sf2_cfp.c | 108 +-- > 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 | 417 ++-- > .../net/ethernet/netronome/nfp/flower/offload.c| 151 ++--- > drivers/net/ethernet/qlogic/qede/qede_filter.c | 537 ++- > include/net/flow_dissector.h | 185 + > include/net/pkt_cls.h | 29 +- > net/core/flow_dissector.c | 341 ++ > net/sched/cls_api.c| 113 > net/sched/cls_flower.c | 42 +- > 18 files changed, 2279 insertions(+), 1983 deletions(-) > Although I was cc'ed in the thread I'm not seeing stmmac driver in this conversion. Can you please add it ? Thanks and Best Regards, Jose Miguel Abreu
[PATCH 00/12 net-next,v2] add flow_rule infrastructure
Hi, This patchset introduces a kernel intermediate representation (IR) to express ACL hardware offloads, as already described in previous RFC and v1 patchset [1] [2]. The idea is to normalize the frontend U/APIs to use the flow dissectors and the flow actions so drivers can reuse the existing TC offload driver codebase - that has been converted to use the flow_rule infrastructure. After this patch, as Or previously described, there is one extra layer: kernel frontend U/API X --> kernel parser Y --> IR --> driver --> HW API kernel frontend U/API Z --> kernel parser W --> IR --> driver --> HW API However, cost of this layer is very small, adding 1 million rules via tc -batch, perf shows: 0.06% tc [kernel.vmlinux][k] tc_setup_flow_action at position 187 in the call graph, far from the top ten. The flow_match representation uses the flow dissector infrastructure, just like cls_flower, therefore, there is no need for conversion of the rule match side. The flow_action representation is very similar to the TC action plus this includes wake-up-on-lan and queue to CPU actions that are needed for the ethtool_rx_flow_spec interface in the bcm_sf2 driver, that is converted in this patchset to use it. It is now possible to add tc cls_flower support for bcm_sf2 and reuse the existing parser that was originally designed for the ethtool_rx_flow_spec interface. As requested, this new patchset also converts qlogic/qede to use this new infrastructure (see patch 12/12). This driver currently has two parsers, one for ethtool_rx_flow_spec and another for tc cls_flower. This driver supports for simple 5-tuple matching and available actions are packet drop and queue. This patch updates the driver code to use one single parser to populate HW IR. Thanks. [1] https://lwn.net/Articles/766695/ [2] https://marc.info/?l=linux-netdev&m=154233253114506&w=2 Pablo Neira Ayuso (12): 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 qede: place ethtool_rx_flow_spec after code after TC flower codebase qede: use ethtool_rx_flow_rule() to remove duplicated parser code drivers/net/dsa/bcm_sf2_cfp.c | 108 +-- 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 | 417 ++-- .../net/ethernet/netronome/nfp/flower/offload.c| 151 ++--- drivers/net/ethernet/qlogic/qede/qede_filter.c | 537 ++- include/net/flow_dissector.h | 185 + include/net/pkt_cls.h | 29 +- net/core/flow_dissector.c | 341 ++ net/sched/cls_api.c| 113 net/sched/cls_flower.c | 42 +- 18 files changed, 2279 insertions(+), 1983 deletions(-) -- 2.11.0
