netdev - Re: [PATCH net-next 00/13] Control action percpu counters allocation by netlink flag

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <vbfimogvu84.fsf@mellanox.com>
Date:   Wed, 23 Oct 2019 06:42:06 +0000
From:   Vlad Buslov <vladbu@...lanox.com>
To:     Marcelo Ricardo Leitner <mleitner@...hat.com>
CC:     Vlad Buslov <vladbu@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "jhs@...atatu.com" <jhs@...atatu.com>,
        "xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>,
        "jiri@...nulli.us" <jiri@...nulli.us>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "dcaratti@...hat.com" <dcaratti@...hat.com>
Subject: Re: [PATCH net-next 00/13] Control action percpu counters allocation
 by netlink flag


On Tue 22 Oct 2019 at 20:09, Marcelo Ricardo Leitner <mleitner@...hat.com> wrote:
> On Tue, Oct 22, 2019 at 03:52:31PM +0000, Vlad Buslov wrote:
>>
>> On Tue 22 Oct 2019 at 18:15, Marcelo Ricardo Leitner <mleitner@...hat.com> wrote:
>> > On Tue, Oct 22, 2019 at 05:17:51PM +0300, Vlad Buslov wrote:
>> >> - Extend actions that are used for hardware offloads with optional
>> >>   netlink 32bit flags field. Add TCA_ACT_FLAGS_FAST_INIT action flag and
>> >>   update affected actions to not allocate percpu counters when the flag
>> >>   is set.
>> >
>> > I just went over all the patches and they mostly make sense to me. So
>> > far the only point I'm uncertain of is the naming of the flag,
>> > "fast_init".  That is not clear on what it does and can be overloaded
>> > with other stuff later and we probably don't want that.
>>
>> I intentionally named it like that because I do want to overload it with
>> other stuff in future, instead of adding new flag value for every single
>> small optimization we might come up with :)
>
> Hah :-)
>
>>
>> Also, I didn't want to hardcode implementation details into UAPI that we
>> will have to maintain for long time after percpu allocator in kernel is
>> potentially replaced with something new and better (like idr is being
>> replaced with xarray now, for example)
>
> I see. OTOH, this also means that the UAPI here would be unstable
> (different meanings over time for the same call), and hopefully new
> behaviors would always be backwards compatible.

This flag doesn't change any userland-visible behavior, just suggests
different performance trade off (insertion rate for sw packet processing
speed). Counters continue to work with or without the flag. Any
optimization that actually modifies cls or act API behavior will have to
use dedicated flag value.

>
>>
>> Anyway, lets see what other people think. I'm open to changing it.
>>
>> >
>> > Say, for example, we want percpu counters but to disable allocating
>> > the stats for hw, to make the counter in 28169abadb08 ("net/sched: Add
>> > hardware specific counters to TC actions") optional.
>> >
>> > So what about:
>> > TCA_ACT_FLAGS_NO_PERCPU_STATS
>> > TCA_ACT_FLAGS_NO_HW_STATS (this one to be done on a subsequent patchset, yes)
>> > ?
>> >
>> >   Marcelo
>>