Date:   Tue, 23 Jun 2020 11:55:37 +0000
From:   Po Liu <po.liu@....com>
To:     Jamal Hadi Salim <jhs@...atatu.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "idosch@...sch.org" <idosch@...sch.org>
CC:     "jiri@...nulli.us" <jiri@...nulli.us>,
        "vinicius.gomes@...el.com" <vinicius.gomes@...el.com>,
        "vlad@...lov.dev" <vlad@...lov.dev>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Vladimir Oltean <vladimir.oltean@....com>,
        Alexandru Marginean <alexandru.marginean@....com>,
        "michael.chan@...adcom.com" <michael.chan@...adcom.com>,
        "vishal@...lsio.com" <vishal@...lsio.com>,
        "saeedm@...lanox.com" <saeedm@...lanox.com>,
        "leon@...nel.org" <leon@...nel.org>,
        "jiri@...lanox.com" <jiri@...lanox.com>,
        "idosch@...lanox.com" <idosch@...lanox.com>,
        "alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>,
        "simon.horman@...ronome.com" <simon.horman@...ronome.com>,
        "pablo@...filter.org" <pablo@...filter.org>,
        "moshe@...lanox.com" <moshe@...lanox.com>,
        "m-karicheri2@...com" <m-karicheri2@...com>,
        "andre.guedes@...ux.intel.com" <andre.guedes@...ux.intel.com>,
        "stephen@...workplumber.org" <stephen@...workplumber.org>,
        Edward Cree <ecree@...arflare.com>
Subject: 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@...atatu.com>
> Sent: 2020年6月23日 18:09
> To: Po Liu <po.liu@....com>; davem@...emloft.net; linux-
> kernel@...r.kernel.org; netdev@...r.kernel.org; idosch@...sch.org
> Cc: jiri@...nulli.us; vinicius.gomes@...el.com; vlad@...lov.dev; Claudiu
> Manoil <claudiu.manoil@....com>; Vladimir Oltean
> <vladimir.oltean@....com>; Alexandru Marginean
> <alexandru.marginean@....com>; michael.chan@...adcom.com;
> vishal@...lsio.com; saeedm@...lanox.com; leon@...nel.org;
> jiri@...lanox.com; idosch@...lanox.com;
> alexandre.belloni@...tlin.com; UNGLinuxDriver@...rochip.com;
> kuba@...nel.org; xiyou.wangcong@...il.com;
> simon.horman@...ronome.com; pablo@...filter.org;
> moshe@...lanox.com; m-karicheri2@...com;
> andre.guedes@...ux.intel.com; stephen@...workplumber.org; Edward
> Cree <ecree@...arflare.com>
> Subject: Re: [v1,net-next 3/4] net: qos: police action add index for tc
> flower offloading
> 
> 
> This certainly brings an interesting point which i brought up earlier when
> Jiri was doing offloading of stats.
> In this case the action index is being used as the offloaded policer index
> (note: there'd need to be a check whether the index is infact acceptable to
> the h/w etc unless there
> 2^32 meters available in the hardware).

Yes, device should report invalid if index is out of range which means hardware not support.

> 
> My question: Is this any different from how stats are structured?

I don't know I fully catch the question. Are you trying to get how many frames for each filter chain passing one index policing action? 
If one index police action bind to multiple tc filter(they should have differnt chain index ). All those filter should get same index police action stats value since they are sharing the same hardware entry. But I don't think this is the problem.

With index provide to device driver(map the s/w action index to a h/w table index ), user could list the police actions list by command:
# tc actions show action police
Shows the police action table by index.
Thanks!

> In this case you can map the s/w action index to a h/w table index (of
> meters).
> My comment then was: hardware i have encountered (and i pointed to P4
> model as well) assumes an indexed table of stats.
> 
> cheers,
> jamal
> 
> On 2020-06-23 2:34 a.m., Po Liu wrote:
> > From: Po Liu <Po.Liu@....com>
> >
> > Hardware may own many entries for police flow. So that make one(or
> >   multi) flow to be policed by one hardware entry. This patch add the
> > police action index provide to the driver side make it mapping the
> > driver hardware entry index.
> >
> > Signed-off-by: Po Liu <Po.Liu@....com>
> > ---
> >   include/net/flow_offload.h | 1 +
> >   net/sched/cls_api.c        | 1 +
> >   2 files changed, 2 insertions(+)
> >
> > diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
> > index c2ef19c6b27d..eed98075b1ae 100644
> > --- a/include/net/flow_offload.h
> > +++ b/include/net/flow_offload.h
> > @@ -232,6 +232,7 @@ struct flow_action_entry {
> >                       bool                    truncate;
> >               } sample;
> >               struct {                                /* FLOW_ACTION_POLICE */
> > +                     u32                     index;
> >                       s64                     burst;
> >                       u64                     rate_bytes_ps;
> >                       u32                     mtu;
> > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index
> > 6aba7d5ba1ec..fdc4c89ca1fa 100644
> > --- a/net/sched/cls_api.c
> > +++ b/net/sched/cls_api.c
> > @@ -3659,6 +3659,7 @@ int tc_setup_flow_action(struct flow_action
> *flow_action,
> >                       entry->police.rate_bytes_ps =
> >                               tcf_police_rate_bytes_ps(act);
> >                       entry->police.mtu = tcf_police_tcfp_mtu(act);
> > +                     entry->police.index = act->tcfa_index;
> >               } else if (is_tcf_ct(act)) {
> >                       entry->id = FLOW_ACTION_CT;
> >                       entry->ct.action = tcf_ct_action(act);
> >

Br,
Po Liu

Powered by blists - more mailing lists