RE: Re: [v1,net-next 3/4] net: qos: police action add index for tc flower offloading

Hi Jamal,

-----Original Message-----
From: Jamal Hadi Salim <jhs@mojatatu.com>
Sent: 2020年6月26日 21:28
To: Po Liu <redacted>; davem@davemloft.net; linux-
kernel@vger.kernel.org; netdev@vger.kernel.org; idosch@idosch.org
Cc: jiri@resnulli.us; vinicius.gomes@intel.com; vlad@buslov.dev; Claudiu
Manoil [off-list ref]; Vladimir Oltean
[off-list ref]; Alexandru Marginean
[off-list ref]; michael.chan@broadcom.com;
vishal@chelsio.com; saeedm@mellanox.com; leon@kernel.org;
jiri@mellanox.com; idosch@mellanox.com;
alexandre.belloni@bootlin.com; UNGLinuxDriver@microchip.com;
kuba@kernel.org; xiyou.wangcong@gmail.com;
simon.horman@netronome.com; pablo@netfilter.org;
moshe@mellanox.com; m-karicheri2@ti.com;
andre.guedes@linux.intel.com; stephen@networkplumber.org; Edward
Cree [off-list ref]
Subject: Re: [v1,net-next 3/4] net: qos: police action add index for tc
flower offloading

On 2020-06-24 8:34 p.m., Po Liu wrote:

quoted

quoted

-----Original Message-----

quoted

quoted

That is the point i was trying to get to. Basically:
You have a counter table which is referenced by "index"
You also have a meter/policer table which is referenced by "index".

They should be one same group and same meaning.

Didnt follow. You mean the index is the same for both the stat and policer?

Sorry, just ignore this reply line, hardware has this police index counter, but wasn't use in this tc command, just focus on below psfp_streamfilter_counters. 
I thought you thought in this way.

quoted

quoted

For policers, they maintain their own stats. So when i say:
tc ... flower ... action police ... index 5 The index referred to is
in the policer table

Sure. Means police with No. 5 entry.

quoted

But for other actions, example when i say:
tc ... flower ... action drop index 10

Still the question, does gact action drop could bind with index? It

doesn't meanful.

quoted

Depends on your hardware. From this discussion i am trying to understand
where the constraint is for your case.
Whether it is your h/w or the TSN spec.
For a sample counting which is flexible see here:
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp4.o
rg%2Fp4-spec%2Fdocs%2FPSA.html%23sec-
counters&data=02%7C01%7Cpo.liu%40nxp.com%7C02dc8f3f60714afd
3dab08d819d4c66e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7
C637287748941041353&sdata=9%2FS%2BAMbHV09H5VrJMwdEeiSPzg
4w%2FmW5FxQr4ecuze4%3D&reserved=0

That concept is not specific to P4 but rather to newer flow-based
hardware.

More context:
The assumption these days is we can have a _lot_ of flows with a lot of
actions.
Then you want to be able to collect the stats separately, possibly one
counter entry for each action of interest.
Why is this important?f For analytics uses cases, when you are retrieving
the stats you want to reduce the amount of data being retrieved. Typically
these stats are polled every X seconds.
For starters, you dont dump filters (which in your case seems to be the
only way to get the stats).
In current tc, you dump the actions. But that could be improved so you
can just dump the stats. The mapping of stats index to actions is known to
the entity doing the dump.

Does that make sense?

quoted

quoted

The index is in the counter/stats table.
It is not exactly "10" in hardware, the driver magically hides it
from the user - so it could be hw counter index 1234

Not exactly. Current flower offloading stats means get the chain index

for that flow filter. The other actions should bind to that chain index.
 >

So if i read correctly: You have an index per filter pointing to the counter
table.
Is this something _you_ decided to do in software or is it how the
hardware works? (note i referred to this as "legacy ACL" approach earlier.
It worked like that in old hardware because the main use case was to have
one action on a match (drop/accept kind).

It is the hardware works and all registers according to the IEEE802.1Qci spec.

quoted

Like IEEE802.1Qci, what I am doing is bind gate action to filter

chain(mandatory). And also police action as optional.

I cant seem to find this spec online. Is it freely available?

Maybe need a register count on http://www.ieee802.org/

Also, if i understand you correctly you are saying according to this spec
you can only have the following type of policy:
tc .. filter match-spec-here .. \
action gate gate-action-attributes \
action police ...

That "action gate" MUST always be present but "action police" is optional?

Yes. That is what I trying to do: map stream gate to gate action and flow metering entry to action police. And a flow filter to a stream filter entry. Each stream filter entry in hardware bind with stream filter entry.

quoted

There is stream counter table which summary the counters pass gate

action entry and police action entry for that chain index(there is a bit
different if two chain sharing same action list).

quoted

One chain counter which tc show stats get counter source:
struct psfp_streamfilter_counters {
         u64 matching_frames_count;
         u64 passing_frames_count;
         u64 not_passing_frames_count;
         u64 passing_sdu_count;
         u64 not_passing_sdu_count;
         u64 red_frames_count;
};

Assuming psfp is something defined in IEEE802.1Qci and the spec will
describe these?

Yes.

Is the filter  "index" pointing to one of those in some counter table?

Filter 'index' together with a counter 'index' as statistics.

quoted

When pass to the user space, summarize as:
         stats.pkts = counters.matching_frames_count +
counters.not_passing_sdu_count - filter->stats.pkts;

 >

quoted

         stats.drops = counters.not_passing_frames_count +

counters.not_passing_sdu_count +   counters.red_frames_count - filter-

quoted

stats.drops;

Thanks for the explanation.
What is filter->stats?

Filter->stats buffer the counters last time read stats. Flower stats get the increasing counters.

The rest of those counters seem related to the gate action.
How do you account for policing actions?

counters.red_frames_count is the policing action counters.

cheers,
jamal

Br,
Po Liu