netdev - Re: [PATCH net-next] net: sched: refactor flower walk to iterate over idr

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: <vbfo9ffqb1p.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>
Date:   Tue, 10 Jul 2018 20:02:10 +0300
From:   Vlad Buslov <vladbu@...lanox.com>
To:     Simon Horman <simon.horman@...ronome.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, jhs@...atatu.com,
        xiyou.wangcong@...il.com, jiri@...nulli.us
Subject: Re: [PATCH net-next] net: sched: refactor flower walk to iterate over idr


On Tue 10 Jul 2018 at 13:55, Simon Horman <simon.horman@...ronome.com> wrote:
> On Mon, Jul 09, 2018 at 01:29:11PM +0300, Vlad Buslov wrote:
>> Extend struct tcf_walker with additional 'cookie' field. It is intended to
>> be used by classifier walk implementations to continue iteration directly
>> from particular filter, instead of iterating 'skip' number of times.
>> 
>> Change flower walk implementation to save filter handle in 'cookie'. Each
>> time flower walk is called, it looks up filter with saved handle directly
>> with idr, instead of iterating over filter linked list 'skip' number of
>> times. This change improves complexity of dumping flower classifier from
>> quadratic to linearithmic. (assuming idr lookup has logarithmic complexity)
>> 
>> Reviewed-by: Jiri Pirko <jiri@...lanox.com>
>> Signed-off-by: Vlad Buslov <vladbu@...lanox.com>
>
> Reported-by: Simon Horman <simon.horman@...ronome.com>
>
> Thanks, I'm very pleased to see this change. I would appreciate it if
> we could have a little time to test its impact on performance
> thoroughly.

For me it reduced time needed to dump 5m flows to ~50 seconds. Not a
very thorough benchmark, but performance improvement was so dramatic
that I decided to not investigate further.

>
> One question: will this work as expected (i.e. be at least backwards
> compatible) with existing user-space code?

I considered that and didn't find any reason why it would break
compatibility. Basically with current flower flows are dumped in
arbitrary order, and with this change dump(accidentally) outputs flows
in sorted ascending order. User space shouldn't expect any ordering in
dumped flows anyway, right?