Message-ID: <9cd1e555-6253-1856-f21d-43323eb77788@mojatatu.com>
Date:   Mon, 24 Feb 2020 10:45:57 -0500
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Jiri Pirko <jiri@...nulli.us>, Edward Cree <ecree@...arflare.com>
Cc:     Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        davem@...emloft.net, saeedm@...lanox.com, leon@...nel.org,
        michael.chan@...adcom.com, vishal@...lsio.com,
        jeffrey.t.kirsher@...el.com, idosch@...lanox.com,
        aelior@...vell.com, peppe.cavallaro@...com,
        alexandre.torgue@...com, xiyou.wangcong@...il.com,
        pablo@...filter.org, mlxsw@...lanox.com
Subject: Re: [patch net-next 00/10] net: allow user specify TC filter HW stats
 type

On 2020-02-24 8:11 a.m., Jiri Pirko wrote:
> Mon, Feb 24, 2020 at 12:38:20PM CET, ecree@...arflare.com wrote:
>> On 22/02/2020 06:38, Jiri Pirko wrote:

[..]
>> 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.
> 

+1  very important for telco billing use cases i am familiar with.

Ancient ACL implementations that had only one filter and
one action (drop/accept) didnt need more than one counter.
But not anymore in my opinion.

There's also a requirement for the concept of "sharing" - think
"family plans" or "small bussiness plan".
Counters may be shared across multiple filter-action chains for example.

> 
>>
>>> 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.
> 

I normally visualize policy as a dag composed of filter(s) +
actions. The UI and uAPI has to be able to express that.

I am not sure if mlxsw works this way, but:
Most hardware i have encountered tends to have a separate
stats/counter table. The stats table is indexed.

Going backwards and looking at your example in this stanza:
---
   in_hw in_hw_count 2
   hw_stats immediate
         action order 1: gact action drop
          random type none pass val 0
          index 1 ref 1 bind 1 installed 14 sec used 7 sec
         Action statistics:
----

Guessing from "in_hw in_hw_count 2" - 2 is a hw stats table index?
If you have enough counters in hardware, the "stats table index"
essentially can be directly mapped to the action "index" attribute.

Sharing then becomes a matter of specifying the same drop action
with the correct index across multiple filters.

If you dont have enough hw counters - then perhaps a scheme to show
separate hardware counter index and software counter index (aka action
index) is needed.

cheers,
jamal

> What do you think?
> 
> 
> Thanks!
> 

Powered by blists - more mailing lists