netdev - Re: [PATCH net-next v2 0/5] net/sched: conditional notification of events for cls and act

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]

Message-ID: <e0724326-834f-4651-bb0b-d8fa9226b909@mojatatu.com>
Date: Tue, 5 Dec 2023 11:49:26 -0300
From: Pedro Tammela <pctammela@...atatu.com>
To: netdev@...r.kernel.org
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, jhs@...atatu.com, xiyou.wangcong@...il.com,
 jiri@...nulli.us, marcelo.leitner@...il.com, vladbu@...dia.com
Subject: Re: [PATCH net-next v2 0/5] net/sched: conditional notification of
 events for cls and act

On 04/12/2023 17:39, Pedro Tammela wrote:
> This is an optimization we have been leveraging on P4TC but we believe
> it will benefit rtnl users in general.
> 
> It's common to allocate an skb, build a notification message and then
> broadcast an event. In the absence of any user space listeners, these
> resources (cpu and memory operations) are wasted. In cases where the subsystem
> is lockless (such as in tc-flower) this waste is more prominent. For the
> scenarios where the rtnl_lock is held it is not as prominent.
> 
> The idea is simple. Build and send the notification iif:
>     - The user requests via NLM_F_ECHO or
>     - Someone is listening to the rtnl group (tc mon)
> 
> On a simple test with tc-flower adding 1M entries, using just a single core,
> there's already a noticeable difference in the cycles spent in tc_new_tfilter
> with this patchset.
> 
> before:
>     - 43.68% tc_new_tfilter
>        + 31.73% fl_change
>        + 6.35% tfilter_notify
>        + 1.62% nlmsg_notify
>          0.66% __tcf_qdisc_find.part.0
>          0.64% __tcf_chain_get
>          0.54% fl_get
>        + 0.53% tcf_proto_lookup_ops
> 
> after:
>     - 39.20% tc_new_tfilter
>        + 34.58% fl_change
>          0.69% __tcf_qdisc_find.part.0
>          0.67% __tcf_chain_get
>        + 0.61% tcf_proto_lookup_ops
> 
> Note, the above test is using iproute2:tc which execs a shell.
> We expect people using netlink directly to observe even greater
> reductions.
> 
> The qdisc side needs some refactoring of the notification routines to fit in
> this new model, so they will be sent in a later patchset.
> 
> v1->v2:
> - Address Jakub comments
> 
> Jamal Hadi Salim (1):
>    rtnl: add helper to check if rtnl group has listeners
> 
> Pedro Tammela (3):
>    rtnl: add helper to send if skb is not null
>    net/sched: act_api: conditional notification of events
>    net/sched: cls_api: conditional notification of events

I just noticed some commits are still referencing the now non-existent 
function 'tc_should_notify'.I will post a v3.
--
pw-bot: cr