Re: [patch net-next 00/10] net: allow user specify TC filter HW stats type

[Jamal Hadi Salim <jhs@mojatatu.com>]

On 2020-02-25 11:22 a.m., Jiri Pirko wrote:
> Tue, Feb 25, 2020 at 05:01:05PM CET, jhs@mojatatu.com wrote:
>> +Cc Marian.
>>



>> So for the shared mirror action the counter is shared
>> by virtue of specifying index 111.
>>
>> What tc _doesnt allow_ is to re-use the same
>> counter index across different types of actions (example
>> mirror index 111 is not the same instance as drop 111).
>> Thats why i was asking if you are exposing the hw index.
> 
> User does not care about any "hw index". That should be abstracted out
> by the driver.
> 

My main motivation is proper accounting (which is important
for the billing and debugging of course). Example:
if i say "get stats" I should know it is the sum of both
h/w + s/w stats or the rules are clear in regards to how
to retrieve each and sum them or differentiate them.
If your patch takes care of summing up things etc, then i agree.
Or if the rules for accounting are consistent then we are fine
as well.

>> So i am guessing the hw cant support "branching" i.e based on in
>> some action state sometime you may execute action foo and other times
>> action bar. Those kind of scenarios would need multiple counters.
> 
> We don't and when/if we do, we need to put another counter to the
> branch point.
>

Ok, that would work.
> 
>>> and we report stats from action_counter for all the_actual_actionX.
>>
>> This may not be accurate if you are branching - for example
>> a policer or quota enforcer which either accepts or drops or sends next
>> to a marker action etc .
>> IMO, this was fine in the old days when you had one action per match.
>> Best is to leave it to whoever creates the policy to decide what to
>> count. IOW, I think modelling it as a pipe or ok or drop or continue
>> and be placed anywhere in the policy graph instead of the begining.
> 
> Eh, that is not that simple. The existing users are used to the fact
> that the actions are providing counters by themselves. Having and
> explicit counter action like this would break that expectation.
 >
> Also, I think it should be up to the driver implementation. Some HW
> might only support stats per rule, not the actions. Driver should fit
> into the existing abstraction, I think it is fine.
>

Reasonable point.
So "count" action is only useful for h/w?

>>> Note that I don't want to share, there is still separate "last_hit"
>>> record in hw I expose in "used X sec". Interestingly enough, in
>>> Spectrum-1 this is per rule, in Spectrum-2,3 this is per action block :)
>>
>> I didnt understand this one..
> 
> It's not "stats", it's an information about how long ago the act was
> used.

ah. Given tc has one of those per action, are you looking to introduce
a new "last used" action?

cheers,
jamal