On Tue, 5 May 2020 20:36:43 +0200 Jiri Pirko wrote: > Tue, May 05, 2020 at 07:47:36PM CEST, pablo@xxxxxxxxxxxxx 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> > > Looks great. Thanks! > > Reviewed-by: Jiri Pirko <jiri@xxxxxxxxxxxx> Is this going to "just work" for mlxsw? act = flow_action_first_entry_get(flow_action); if (act->hw_stats == FLOW_ACTION_HW_STATS_ANY || act->hw_stats == FLOW_ACTION_HW_STATS_IMMEDIATE) { /* Count action is inserted first */ err = mlxsw_sp_acl_rulei_act_count(mlxsw_sp, rulei, extack); if (err) return err; } else if (act->hw_stats != FLOW_ACTION_HW_STATS_DISABLED) { NL_SET_ERR_MSG_MOD(extack, "Unsupported action HW stats type"); return -EOPNOTSUPP; } if hw_stats is 0 we'll get into the else and bail. That doesn't deliver on the "don't care" promise, no?