Re: [RFC PATCH net-next 2/3] flow_offload: restore ability to collect separate stats per action

From: Jamal Hadi Salim <jhs@mojatatu.com>
To: Edward Cree <ecree@solarflare.com>,
	Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Jiri Pirko <jiri@resnulli.us>,
	Pablo Neira Ayuso <pablo@netfilter.org>,
	David Miller <davem@davemloft.net>,
	netdev <netdev@vger.kernel.org>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Andy Gospodarek <andy@greyhouse.net>,
	Anjali Singhai Jain <anjali.singhai@intel.com>,
	Or Gerlitz <gerlitz.or@gmail.com>
Subject: Re: [RFC PATCH net-next 2/3] flow_offload: restore ability to collect separate stats per action
Date: Wed, 8 May 2019 10:02:21 -0400	[thread overview]
Message-ID: <ab1f179e-9a91-837b-28c8-81eecbd09e7f@mojatatu.com> (raw)
In-Reply-To: <1b37d659-5a2b-6130-e8d6-c15d6f57b55e@solarflare.com>

On 2019-05-07 8:27 a.m., Edward Cree wrote:
> On 06/05/2019 13:41, Jamal Hadi Salim wrote:
>> On 2019-05-04 2:27 a.m., Jakub Kicinski wrote:
>>> On Fri, 3 May 2019 16:06:55 +0100, Edward Cree wrote:

[..]

> I don't know much of anything about RTM_GETACTION, but it doesn't appear
>   to be part of the current "tc offload" world, which AIUI is very much
>   centred around cls_flower.  I'm just trying to make counters in
>   cls_flower offload do 'the right thing' (whatever that may be), anything
>   else is out of scope.
> 

The lazy thing most people have done is essentially assume that
there is a stat per filter rule. From tc semantics that is an implicit
"pipe" action (which then carries stats).
And most times they implement a single action per match. I wouldnt call
it the 'the right thing'...

>> Most H/W i have seen has a global indexed stats table which is
>> shared by different action types (droppers, accept, mirror etc).
>> The specific actions may also have their own tables which also
>> then refer to the 32 bit index used in the stats table[1].
>> So for this to work well, the action will need at minimal to have
>> two indices one that is used in hardware stats table
>> and another that is kernel mapped to identify the attributes. Of
>> course we'll need to have a skip_sw flag etc.
> I'm not sure I'm parsing this correctly, but are you saying that the
>   index namespace is per-action type?  I.e. a mirred and a drop action
>   could have the same index yet expect to have separate counters?  My
>   approach here has assumed that in such a case they would share their
>   counters.

Yes, the index at tc semantics level is per-action type.
So "mirred index 1" and "drop index 1" are not the same stats counter.
Filters on the other hand can share per-action type stats by virtue of
sharing the same index per-action. e.g

flower match foo action drop index 1
flower match bar action drop index 1

will share the same stats.

cheers,
jamal