Mon, Feb 24, 2020 at 12:38:20PM CET, ecree@solarflare.com wrote:

On 22/02/2020 06:38, Jiri Pirko wrote:

quoted

Fri, Feb 21, 2020 at 07:22:00PM CET, kuba@kernel.org wrote:

quoted

Hmm, but the statistics are on actions, it feels a little bit like we
are perpetuating the mistake of counting on filters here.

You are right, the stats in tc are per-action. However, in both mlxsw
and mlx5, the stats are per-filter. What hw_stats does is that it
basically allows the user to set the type for all the actions in the
filter at once.

Could you imagine a usecase that would use different HW stats type for
different actions in one action-chain?

Potentially a user could only want stats on one action and disable them
on another.  For instance, if their action chain does delivery to the
'customer' and also mirrors the packets for monitoring, they might only
want stats on the first delivery (e.g. for billing) and not want to
waste a counter on the mirror.

Okay.

quoted

Plus, if the fine grained setup is ever needed, the hw_stats could be in
future easilyt extended to be specified per-action too overriding
the per-filter setup only for specific action. What do you think?

I think this is improper; the stats type should be defined per-action in
the uapi, even if userland initially only has UI to set it across the
entire filter.  (I imagine `tc action` would grow the corresponding UI
pretty quickly.)  Then on the driver side, you must determine whether
your hardware can support the user's request, and if not, return an
appropriate error code.

Let's try not to encourage people (users, future HW & driver developers)
to keep thinking of stats as per-filter entities.

Okay, but in that case, it does not make sense to have "UI to set it
across the entire filter". The UI would have to set it per-action too.
Othewise it would make people think it is per-filter entity.
But I guess the tc cmdline is chatty already an it can take other
repetitive cmdline options.

What do you think?


Thanks!