Re: [PATCH 00/10] add flow_rule infrastructure

2018-11-19 Thread Florian Fainelli
On 11/19/18 1:57 AM, Jiri Pirko wrote:
> Mon, Nov 19, 2018 at 10:19:28AM CET, manish.cho...@cavium.com wrote:
>>> -Original Message-
>>> From: netdev-ow...@vger.kernel.org  On
>>> Behalf Of Pablo Neira Ayuso
>>> Sent: Friday, November 16, 2018 7:11 AM
>>> To: netdev@vger.kernel.org
>>> Cc: da...@davemloft.net; thomas.lenda...@amd.com;
>>> f.faine...@gmail.com; Elior, Ariel ;
>>> michael.c...@broadcom.com; sant...@chelsio.com;
>>> madalin.bu...@nxp.com; yisen.zhu...@huawei.com;
>>> salil.me...@huawei.com; jeffrey.t.kirs...@intel.com; tar...@mellanox.com;
>>> sae...@mellanox.com; j...@mellanox.com; ido...@mellanox.com;
>>> jakub.kicin...@netronome.com; peppe.cavall...@st.com;
>>> grygorii.stras...@ti.com; and...@lunn.ch;
>>> vivien.dide...@savoirfairelinux.com; alexandre.tor...@st.com;
>>> joab...@synopsys.com; linux-net-driv...@solarflare.com;
>>> ganes...@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 action

Re: [PATCH 00/10] add flow_rule infrastructure

2018-11-19 Thread Jiri Pirko
Mon, Nov 19, 2018 at 10:19:28AM CET, manish.cho...@cavium.com wrote:
>> -Original Message-
>> From: netdev-ow...@vger.kernel.org  On
>> Behalf Of Pablo Neira Ayuso
>> Sent: Friday, November 16, 2018 7:11 AM
>> To: netdev@vger.kernel.org
>> Cc: da...@davemloft.net; thomas.lenda...@amd.com;
>> f.faine...@gmail.com; Elior, Ariel ;
>> michael.c...@broadcom.com; sant...@chelsio.com;
>> madalin.bu...@nxp.com; yisen.zhu...@huawei.com;
>> salil.me...@huawei.com; jeffrey.t.kirs...@intel.com; tar...@mellanox.com;
>> sae...@mellanox.com; j...@mellanox.com; ido...@mellanox.com;
>> jakub.kicin...@netronome.com; peppe.cavall...@st.com;
>> grygorii.stras...@ti.com; and...@lunn.ch;
>> vivien.dide...@savoirfairelinux.com; alexandre.tor...@st.com;
>> joab...@synopsys.com; linux-net-driv...@solarflare.com;
>> ganes...@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

RE: [PATCH 00/10] add flow_rule infrastructure

2018-11-19 Thread Chopra, Manish
> -Original Message-
> From: netdev-ow...@vger.kernel.org  On
> Behalf Of Pablo Neira Ayuso
> Sent: Friday, November 16, 2018 7:11 AM
> To: netdev@vger.kernel.org
> Cc: da...@davemloft.net; thomas.lenda...@amd.com;
> f.faine...@gmail.com; Elior, Ariel ;
> michael.c...@broadcom.com; sant...@chelsio.com;
> madalin.bu...@nxp.com; yisen.zhu...@huawei.com;
> salil.me...@huawei.com; jeffrey.t.kirs...@intel.com; tar...@mellanox.com;
> sae...@mellanox.com; j...@mellanox.com; ido...@mellanox.com;
> jakub.kicin...@netronome.com; peppe.cavall...@st.com;
> grygorii.stras...@ti.com; and...@lunn.ch;
> vivien.dide...@savoirfairelinux.com; alexandre.tor...@st.com;
> joab...@synopsys.com; linux-net-driv...@solarflare.com;
> ganes...@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

Re: [PATCH 00/10] add flow_rule infrastructure

2018-11-17 Thread David Miller
From: Or Gerlitz 
Date: Fri, 16 Nov 2018 12:29:29 +0200

> I think it would be fair to ask for one such driver porting to see
> the impact/benefit.

This is an absolute requirement, otherwise the claim that it helps is
pure theory.


Re: [PATCH 00/10] add flow_rule infrastructure

2018-11-16 Thread Or Gerlitz
On Fri, Nov 16, 2018 at 3:43 AM Pablo Neira Ayuso  wrote:
> 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

any reason to keep aRFS out?

> 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.

yeah, but we are adding one more chain (IR), today we have

kernel frontend U/API X --> kernel parser Y --> driver --> HW API
kernel frontend U/API Z --> kernel parser W --> driver --> HW API

and we move to

kernel frontend U/API X --> kernel parser Y --> IR --> driver --> HW API
kernel frontend U/API Z --> kernel parser W --> IR --> driver --> HW API

So instead of

Y --> HW
W --> HW

we have IR --> HW while loosing the TC semantics and spirit in the drivers.

We could have normalize all the U/APIs to use the flow dissectors and
the TC actions and then have the drivers to deal with TC only.

IMHO flow dissectors and TC actions are the right approach to deal with ACLs
HW offloading. They properly reflect how ACLs work in modern HW pipelines.

>
> .   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/

I will further look next week, but as this is not marked as RFC (and
not with net-next
on the title), can we consider this still a discussion and not final/review?

> 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.

I think it would be fair to ask for one such driver porting to see the
impact/benefit.

Or.


[PATCH 00/10] add flow_rule infrastructure

2018-11-15 Thread Pablo Neira Ayuso
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  |