lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 24 Mar 2020 13:04:21 +0000
From:   Po Liu <po.liu@....com>
To:     Jiri Pirko <jiri@...nulli.us>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "vinicius.gomes@...el.com" <vinicius.gomes@...el.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Vladimir Oltean <vladimir.oltean@....com>,
        Alexandru Marginean <alexandru.marginean@....com>,
        Xiaoliang Yang <xiaoliang.yang_1@....com>,
        Roy Zang <roy.zang@....com>, Mingkai Hu <mingkai.hu@....com>,
        Jerry Huang <jerry.huang@....com>, Leo Li <leoyang.li@....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>,
        "jhs@...atatu.com" <jhs@...atatu.com>,
        "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>
Subject: RE: [EXT] Re: [v1,net-next  1/5] net: qos offload add flow status
 with dropped count

Hi Jiri,

> -----Original Message-----
> From: Jiri Pirko <jiri@...nulli.us>
> Sent: 2020年3月24日 18:02
> To: Po Liu <po.liu@....com>
> Cc: davem@...emloft.net; linux-kernel@...r.kernel.org;
> netdev@...r.kernel.org; vinicius.gomes@...el.com; Claudiu Manoil
> <claudiu.manoil@....com>; Vladimir Oltean <vladimir.oltean@....com>;
> Alexandru Marginean <alexandru.marginean@....com>; Xiaoliang Yang
> <xiaoliang.yang_1@....com>; Roy Zang <roy.zang@....com>; Mingkai Hu
> <mingkai.hu@....com>; Jerry Huang <jerry.huang@....com>; Leo Li
> <leoyang.li@....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; jhs@...atatu.com;
> 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
> Subject: [EXT] Re: [v1,net-next 1/5] net: qos offload add flow status with
> dropped count
> 
> Caution: EXT Email
> 
> Tue, Mar 24, 2020 at 04:47:39AM CET, Po.Liu@....com wrote:
> >Add the hardware tc flower offloading with dropped frame counter for
> >status update. action ops->stats_update only loaded by the
> >tcf_exts_stats_update() and tcf_exts_stats_update() only loaded by
> >matchall and tc flower hardware filter. But the stats_update only set
> >the dropped count as default false in the ops->stats_update. This patch
> >add the dropped counter to action stats update. Its dropped counter
> >update by the hardware offloading driver.
> >This is changed by replacing the drop flag with dropped frame counter.
> 
> I just read this paragraph 3 times, I'm unable to decypher :(

Sorry to make you confusing. I would make a clear description.
Before that, I just try to explain what the patch do here so you can provide more suggestion.

To get the stats in the tc flower offloading(by flag FLOW_CLS_STATS), it saves in the 'struct flow_stats' in the ' struct flow_cls_offload '. But the ' struct flow_stats ' only include the packages numbers. Some actions like policing also this 0002/0003 patch introduce stream gate action would produce dropped frames which is important for user evaluation. The packages number(pkts in struct flow_stats) and dropped number relation should be 'pkts' is how many frames were filtered, and 'dropped' number is how many frames dropped in those 'pkts'.
Eventually, the stats updated by the struct tc_action 's operation stats_update(). To implement add dropped number, then add parameter 'dropped' in the related functions: ops->stats_update/tcf_exts_stats_update() and also the tcf_action_update_stats(). There is a bool parameter 'drop' which is using now, can be understand the stats updating is for update for 'drop' count or not. But this flag is not useless as I checked in current kernel code(correct me if it is not). So replace the bool 'drop' flag with 'dropped' number in tcf_action_update_stats(). Make the tcf_action_update_stats() shows how many ' packets' updated and how many 'dropped' number in these 'packets'.
Thanks!

> 
> 
> 
> >
> >Driver side should update how many "packets" it filtered and how many
> >"dropped" in those "packets".
> >
> 
> [...]
> 
> 
> >       return action;
> > }
> >
> >-static void tcf_gact_stats_update(struct tc_action *a, u64 bytes, u32
> packets,
> >-                                u64 lastuse, bool hw)
> >+static void tcf_gact_stats_update(struct tc_action *a, u64 bytes, u64
> packets,
> >+                                u64 lastuse, u64 dropped, bool hw)
> > {
> >       struct tcf_gact *gact = to_gact(a);
> >       int action = READ_ONCE(gact->tcf_action);
> >       struct tcf_t *tm = &gact->tcf_tm;
> >
> >-      tcf_action_update_stats(a, bytes, packets, action == TC_ACT_SHOT,
> hw);
> >+      tcf_action_update_stats(a, bytes, packets,
> >+                              (action == TC_ACT_SHOT) ? packets : 0,
> >+ hw);
> 
> Avoid ()s here.

Ok.

> 
> 
> >       tm->lastuse = max_t(u64, tm->lastuse, lastuse); }
> >


Br,
Po Liu

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ