netdev - Re: [PATCH v4 net-next] net:sched: add action inheritdsfield to skbedit

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: <007b4e8c-48ee-0811-7af7-7a92db82e8b0@digirati.com.br>
Date:   Wed, 20 Jun 2018 08:42:15 -0400
From:   Michel Machado <michel@...irati.com.br>
To:     Jamal Hadi Salim <jhs@...atatu.com>,
        "Fu, Qiaobin" <qiaobinf@...edu>,
        "davem@...emloft.net" <davem@...emloft.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        "xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>
Subject: Re: [PATCH v4 net-next] net:sched: add action inheritdsfield to
 skbedit

On 06/20/2018 07:53 AM, Jamal Hadi Salim wrote:
> On 19/06/18 08:39 AM, Michel Machado wrote:
> 
>>  Notice that, different from skbmod, there's no field parm->flags in 
>> skbedit. Skbedit infers the flags in d->flags from the presence of the 
>> parameters of each of its actions. But SKBEDIT_F_INHERITDSFIELD has no 
>> parameter and adding field parm->flags breaks backward compatibility 
>> with user space as pointed out by Marcelo Ricardo Leitner. Our 
>> solution was to add TCA_SKBEDIT_FLAGS, so SKBEDIT_F_INHERITDSFIELD and 
>> future flag-only actions can be added.
> 
> Ok, that makes sense - thanks. I am not so sure about using
> 64 bits (32 bits would have been fine to match the size of
> the kernel flags), but other than that LGTM.

    The choice for the u64 is meant to keep the interface between kernel 
and user space the same for hopefully a longer time than it would be 
with a u32. Changing from u32 to u64 in the kernel, when the need 
arrives, won't impact applications. This interface choice was motivated 
by the backward compatibility issue mentioned above.

    Thank you for the review, Jamal.

[ ]'s
Michel Machado