[RFC PATCH net-next 0/3] flow_offload: Re-add various features that disappeared

[RFC PATCH net-next 0/3] flow_offload: Re-add various features that disappeared

[Edward Cree]

When the flow_offload infrastructure was added, a couple of things that
 were previously possible for drivers to support in TC offload were not
 plumbed through, perhaps because the drivers in the tree did not fully
 or correctly implement them.
The main issue was with statistics; in TC (and in the previous offload
 API) statistics are per-action, though generally only on 'delivery'
 actions like mirred, ok and shot.  Actions also have an index, which
 may be supplied by the user, which allows the sharing of entities such
 as counters between multiple rules.  The existing driver implementations
 did not support this, however, instead allocating a single counter per
 rule.  The flow_offload API did not support this either, as (a) the
 action index never reached the driver, and (b) the TC_CLSFLOWER_STATS
 callback was only able to return a single set of stats which were added
 to all counters for actions on the rule.  Patch #1 of this series fixes
 (a) by storing tcfa_index in a new action_index member of struct
 flow_action_entry, while patch #2 fixes (b) by adding a new callback,
 TC_CLSFLOWER_STATS_BYINDEX, which retrieves statistics for a specified
 action_index rather than by rule (although the rule cookie is still   
 passed as well).
Patch #3 adds flow_rule_match_cvlan(), analogous to
 flow_rule_match_vlan() but accessing FLOW_DISSECTOR_KEY_CVLAN instead
 of FLOW_DISSECTOR_KEY_VLAN, to allow offloading inner VLAN matches.
This patch series does not include any users of these new interfaces;   
 the driver in which I hope to use them does not yet exist upstream as  
 it is for hardware which is still under development.  However I've CCed
 developers of various other drivers that implement TC offload, in case
 any of them want to implement support.  Otherwise I imagine that David
 won't be willing to take this without a user, in which case I'll save
 it to submit alongside the aforementioned unfinished driver (hence the
 RFC tags for now).

Edward Cree (3):
  flow_offload: copy tcfa_index into flow_action_entry
  flow_offload: restore ability to collect separate stats per action
  flow_offload: support CVLAN match

 include/net/flow_offload.h |  3 +++
 include/net/pkt_cls.h      |  2 ++
 net/core/flow_offload.c    |  7 +++++++
 net/sched/cls_api.c        |  1 +
 net/sched/cls_flower.c     | 30 ++++++++++++++++++++++++++++++
 5 files changed, 43 insertions(+)

Re: [RFC PATCH net-next 0/3] flow_offload: Re-add various features that disappeared

[Jiri Pirko]

Fri, May 03, 2019 at 04:59:28PM CEST, ecree@solarflare.com wrote:
>When the flow_offload infrastructure was added, a couple of things that
> were previously possible for drivers to support in TC offload were not
> plumbed through, perhaps because the drivers in the tree did not fully
> or correctly implement them.
>The main issue was with statistics; in TC (and in the previous offload
> API) statistics are per-action, though generally only on 'delivery'
> actions like mirred, ok and shot.  Actions also have an index, which
> may be supplied by the user, which allows the sharing of entities such
> as counters between multiple rules.  The existing driver implementations
> did not support this, however, instead allocating a single counter per
> rule.  The flow_offload API did not support this either, as (a) the
> action index never reached the driver, and (b) the TC_CLSFLOWER_STATS
> callback was only able to return a single set of stats which were added
> to all counters for actions on the rule.  Patch #1 of this series fixes
> (a) by storing tcfa_index in a new action_index member of struct
> flow_action_entry, while patch #2 fixes (b) by adding a new callback,
> TC_CLSFLOWER_STATS_BYINDEX, which retrieves statistics for a specified
> action_index rather than by rule (although the rule cookie is still   
> passed as well).
>Patch #3 adds flow_rule_match_cvlan(), analogous to
> flow_rule_match_vlan() but accessing FLOW_DISSECTOR_KEY_CVLAN instead
> of FLOW_DISSECTOR_KEY_VLAN, to allow offloading inner VLAN matches.
>This patch series does not include any users of these new interfaces;   
> the driver in which I hope to use them does not yet exist upstream as  
> it is for hardware which is still under development.  However I've CCed
> developers of various other drivers that implement TC offload, in case
> any of them want to implement support.  Otherwise I imagine that David
> won't be willing to take this without a user, in which case I'll save
> it to submit alongside the aforementioned unfinished driver (hence the
> RFC tags for now).

We need in-tree users of the API.


>
>Edward Cree (3):
>  flow_offload: copy tcfa_index into flow_action_entry
>  flow_offload: restore ability to collect separate stats per action
>  flow_offload: support CVLAN match
>
> include/net/flow_offload.h |  3 +++
> include/net/pkt_cls.h      |  2 ++
> net/core/flow_offload.c    |  7 +++++++
> net/sched/cls_api.c        |  1 +
> net/sched/cls_flower.c     | 30 ++++++++++++++++++++++++++++++
> 5 files changed, 43 insertions(+)

Re: [RFC PATCH net-next 0/3] flow_offload: Re-add various features that disappeared

[Jakub Kicinski]

On Fri, 3 May 2019 15:59:28 +0100, Edward Cree wrote:
> When the flow_offload infrastructure was added, a couple of things that
>  were previously possible for drivers to support in TC offload were not
>  plumbed through, perhaps because the drivers in the tree did not fully
>  or correctly implement them.
> The main issue was with statistics; in TC (and in the previous offload
>  API) statistics are per-action, though generally only on 'delivery'
>  actions like mirred, ok and shot.  Actions also have an index, which
>  may be supplied by the user, which allows the sharing of entities such
>  as counters between multiple rules.  The existing driver implementations
>  did not support this, however, instead allocating a single counter per
>  rule.  The flow_offload API did not support this either, as (a) the
>  action index never reached the driver, and (b) the TC_CLSFLOWER_STATS
>  callback was only able to return a single set of stats which were added
>  to all counters for actions on the rule.  Patch #1 of this series fixes
>  (a) by storing tcfa_index in a new action_index member of struct
>  flow_action_entry, while patch #2 fixes (b) by adding a new callback,
>  TC_CLSFLOWER_STATS_BYINDEX, which retrieves statistics for a specified
>  action_index rather than by rule (although the rule cookie is still   
>  passed as well).
> Patch #3 adds flow_rule_match_cvlan(), analogous to
>  flow_rule_match_vlan() but accessing FLOW_DISSECTOR_KEY_CVLAN instead
>  of FLOW_DISSECTOR_KEY_VLAN, to allow offloading inner VLAN matches.
> This patch series does not include any users of these new interfaces;   
>  the driver in which I hope to use them does not yet exist upstream as  
>  it is for hardware which is still under development.  However I've CCed
>  developers of various other drivers that implement TC offload, in case
>  any of them want to implement support.  Otherwise I imagine that David
>  won't be willing to take this without a user, in which case I'll save
>  it to submit alongside the aforementioned unfinished driver (hence the
>  RFC tags for now).

Thanks for making this clear, I'd also like to make a disclaimer
that my comments on the patches are not an indication that I think 
these patches are worth merging right now :)

> Edward Cree (3):
>   flow_offload: copy tcfa_index into flow_action_entry
>   flow_offload: restore ability to collect separate stats per action
>   flow_offload: support CVLAN match
> 
>  include/net/flow_offload.h |  3 +++
>  include/net/pkt_cls.h      |  2 ++
>  net/core/flow_offload.c    |  7 +++++++
>  net/sched/cls_api.c        |  1 +
>  net/sched/cls_flower.c     | 30 ++++++++++++++++++++++++++++++
>  5 files changed, 43 insertions(+)