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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <vbfmudou5qp.fsf@mellanox.com>
Date:   Fri, 25 Oct 2019 16:53:05 +0000
From:   Vlad Buslov <vladbu@...lanox.com>
To:     Jamal Hadi Salim <jhs@...atatu.com>
CC:     Vlad Buslov <vladbu@...lanox.com>, Jiri Pirko <jiri@...nulli.us>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "mleitner@...hat.com" <mleitner@...hat.com>,
        "dcaratti@...hat.com" <dcaratti@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH net-next 00/13] Control action percpu counters allocation
 by netlink flag


On Fri 25 Oct 2019 at 19:29, Jamal Hadi Salim <jhs@...atatu.com> wrote:
> On 2019-10-25 12:10 p.m., Vlad Buslov wrote:
>>
>
>>> Hold on. Looking more at the code, direct call for tcf_action_init_1()
>>> from the cls code path is for backward compat of old policer approach.
>>> I think even modern iproute2 doesnt support that kind of call
>>> anymore. So you can pass NULL there for the *flags.
>>
>> But having the FAST_INIT flag set when creating actions through cls API
>> is my main use case. Are you suggesting to only have flags when actions
>> created through act API?
>>
>
> Not at all. Here's my thinking...
>
> I didnt see your iproute2 change; however, in user space - i think all
> the classifiers eventually call parse_action()? You can stick the flags
> there under TCA_ACT_ROOT_FLAGS
>
> In the kernel, tcf_exts_validate() - two spots:
> tcf_action_init_1() pass NULL
> tcf_action_init() before invocation extract the TCA_ACT_ROOT_FLAGS
> (similar to the act_api approach).
>
> Am i missing something? Sorry - dont have much cycles right now
> but i could do a prototype later.
>
> cheers,
> jamal

I don't exactly follow. In case of act API we have following call chain:
tc_ctl_action()->tcf_action_add()->tcf_action_init(). In this case
TCA_ROOT is parsed in tc_ctl_action() and tcf_action_init() is called
with one of its nested attributes - tca[TCA_ACT_TAB]. When
tcf_action_init() is called TCA_ROOT is already "parsed out", but it is
easy to pass it as an argument.

For cls API lets take flower as an example: fl_change() parses TCA_FLOWER, and calls
fl_set_parms()->tcf_exts_validate()->tcf_action_init() with
TCA_FLOWER_ACT nested attribute. No TCA_ROOT is expected, TCA_FLOWER_ACT
contains up to TCA_ACT_MAX_PRIO nested TCA_ACT attributes. So where can
I include it without breaking backward compatibility?

Regards,
Vlad