netdev - Re: [PATCH net] net: openvswitch: fix TTL decrement action netlink message format

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] [day] [month] [year] [list]

Date:   Tue, 24 Nov 2020 12:20:49 +0100
From:   "Eelco Chaudron" <echaudro@...hat.com>
To:     "Jakub Kicinski" <kuba@...nel.org>
Cc:     "Matteo Croce" <mcroce@...ux.microsoft.com>,
        netdev@...r.kernel.org, "David Miller" <davem@...emloft.net>,
        dev@...nvswitch.org, "Pravin B Shelar" <pshelar@....org>,
        bindiyakurle@...il.com, "Ilya Maximets" <i.maximets@....org>
Subject: Re: [PATCH net] net: openvswitch: fix TTL decrement action netlink
 message format



On 24 Nov 2020, at 2:57, Jakub Kicinski wrote:

> On Mon, 23 Nov 2020 20:36:39 +0100 Matteo Croce wrote:
>> On Fri, Nov 20, 2020 at 10:12 PM Jakub Kicinski <kuba@...nel.org> 
>> wrote:
>>> On Thu, 19 Nov 2020 04:04:04 -0500 Eelco Chaudron wrote:
>>>> Currently, the openvswitch module is not accepting the correctly 
>>>> formated
>>>> netlink message for the TTL decrement action. For both setting and 
>>>> getting
>>>> the dec_ttl action, the actions should be nested in the
>>>> OVS_DEC_TTL_ATTR_ACTION attribute as mentioned in the openvswitch.h 
>>>> uapi.
>>>
>>> IOW this change will not break any known user space, correct?
>>>
>>> But existing OvS user space already expects it to work like you
>>> make it work now?
>>>
>>> What's the harm in leaving it as is?
>>>
>>>> Fixes: 744676e77720 ("openvswitch: add TTL decrement action")
>>>> Signed-off-by: Eelco Chaudron <echaudro@...hat.com>
>>>
>>> Can we get a review from OvS folks? Matteo looks good to you (as the
>>> original author)?
>>
>> I think that the userspace still has to implement the dec_ttl action;
>> by now dec_ttl is implemented with set_ttl().
>> So there is no breakage yet.
>>
>> Eelco, with this fix we will encode the netlink attribute in the same
>> way for the kernel and netdev datapath?
>
> We don't allow breaking uAPI. Sounds like the user space never
> implemented this and perhaps the nesting is just inconvenient
> but not necessarily broken? If it is broken and unusable that
> has to be clearly explained in the commit message. I'm dropping
> v1 from patchwork.

Thanks, I will add some explaining comments to the V2, and sent it out.