On Sun, 1 Mar 2020 10:00:09 +0100 Jiri Pirko wrote:

Sat, Feb 29, 2020 at 08:18:48PM CET, kuba@kernel.org wrote:

quoted

On Sat, 29 Feb 2020 08:40:04 +0100 Jiri Pirko wrote:  

quoted

Fri, Feb 28, 2020 at 08:40:56PM CET, kuba@kernel.org wrote:  

quoted

On Fri, 28 Feb 2020 18:24:56 +0100 Jiri Pirko wrote:    

quoted

@@ -299,6 +300,9 @@ static int bnxt_tc_parse_actions(struct bnxt *bp,
 		return -EINVAL;
 	}
 
+	if (!flow_action_basic_hw_stats_types_check(flow_action, extack))
+		return -EOPNOTSUPP;    

Could we have this helper take one stat type? To let drivers pass the
stat type they support?     

That would be always "any" as "any" is supported by all drivers.
And that is exactly what the helper checks..  

I'd think most drivers implement some form of DELAYED today, 'cause for
the number of flows things like OvS need that's the only practical one.
I was thinking to let drivers pass DELAYED here.

I agree that your patch would most likely pass ANY in almost all cases
as you shouldn't be expected to know all the drivers, but at least the
maintainers can easily just tweak the parameter.

Does that make sense? Maybe I'm missing something.  

Well, I guess. mlx5 only supports "delayed". It would work for it.
How about having flow_action_basic_hw_stats_types_check() as is and
add flow_action_basic_hw_stats_types_check_ext() that would accept extra
arg with enum?

SGTM, perhaps with a more concise name? 
Just flow_basic_hw_stats_check()?