On Tue, May 05, 2020 at 11:40:10AM -0700, Jakub Kicinski wrote:
> On Tue, 5 May 2020 19:47:36 +0200 Pablo Neira Ayuso wrote:
> > This patch adds FLOW_ACTION_HW_STATS_DONT_CARE which tells the driver
> > that the frontend does not need counters, this hw stats type request
> > never fails. The FLOW_ACTION_HW_STATS_DISABLED type explicitly requests
> > the driver to disable the stats, however, if the driver cannot disable
> > counters, it bails out.
> >
> > TCA_ACT_HW_STATS_* maintains the 1:1 mapping with FLOW_ACTION_HW_STATS_*
> > except by disabled which is mapped to FLOW_ACTION_HW_STATS_DISABLED
> > (this is 0 in tc). Add tc_act_hw_stats() to perform the mapping between
> > TCA_ACT_HW_STATS_* and FLOW_ACTION_HW_STATS_*.
> >
> > Fixes: 319a1d19471e ("flow_offload: check for basic action hw stats type")
> > Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> > ---
> > v2: define FLOW_ACTION_HW_STATS_DISABLED at the end of the enumeration
> > as Jiri suggested. Keep the 1:1 mapping between TCA_ACT_HW_STATS_*
> > and FLOW_ACTION_HW_STATS_* except by the disabled case.
> >
> > include/net/flow_offload.h | 9 ++++++++-
> > net/sched/cls_api.c | 14 ++++++++++++--
> > 2 files changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
> > index 3619c6acf60f..efc8350b42fb 100644
> > --- a/include/net/flow_offload.h
> > +++ b/include/net/flow_offload.h
> > @@ -166,15 +166,18 @@ enum flow_action_mangle_base {
> > enum flow_action_hw_stats_bit {
> > FLOW_ACTION_HW_STATS_IMMEDIATE_BIT,
> > FLOW_ACTION_HW_STATS_DELAYED_BIT,
> > + FLOW_ACTION_HW_STATS_DISABLED_BIT,
> > };
> >
> > enum flow_action_hw_stats {
> > - FLOW_ACTION_HW_STATS_DISABLED = 0,
> > + FLOW_ACTION_HW_STATS_DONT_CARE = 0,
>
> Why not ~0? Or ANY | DISABLED?
> Otherwise you may confuse drivers which check bit by bit.
I'm confused, you agreed with this behaviour:
https://lore.kernel.org/netfilter-devel/20200427111220.7b07aae1@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/T/#m6091486b2b0ddac512fe6c17f5508f280f630b60
