netdev - Re: [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6 encapsulation

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]

Date:   Fri, 1 Oct 2021 08:06:33 -0600
From:   David Ahern <dsahern@...il.com>
To:     Justin Iurman <justin.iurman@...ege.be>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        yoshfuji@...ux-ipv6.org, dsahern@...nel.org
Subject: Re: [PATCH net-next 1/2] ipv6: ioam: Add support for the ip6ip6
 encapsulation

On 10/1/21 5:38 AM, Justin Iurman wrote:
>>>>>  static const struct nla_policy ioam6_iptunnel_policy[IOAM6_IPTUNNEL_MAX + 1] = {
>>>>> -	[IOAM6_IPTUNNEL_TRACE]	= NLA_POLICY_EXACT_LEN(sizeof(struct ioam6_trace_hdr)),
>>>>> +	[IOAM6_IPTUNNEL_TRACE]	= NLA_POLICY_EXACT_LEN(sizeof(struct
>>>>> ioam6_iptunnel_trace)),
>>>>
>>>> you can't do that. Once a kernel is released with a given UAPI, it can
>>>> not be changed. You could go the other way and handle
>>>>
>>>> struct ioam6_iptunnel_trace {
>>>> +	struct ioam6_trace_hdr trace;
>>>> +	__u8 mode;
>>>> +	struct in6_addr tundst;	/* unused for inline mode */
>>>> +};
>>>
>>> Makes sense. But I'm not sure what you mean by "go the other way". Should I
>>> handle ioam6_iptunnel_trace as well, in addition to ioam6_trace_hdr, so that
>>> the uapi is backward compatible?
>>
>> by "the other way" I meant let ioam6_trace_hdr be the top element in the
>> new ioam6_iptunnel_trace struct. If the IOAM6_IPTUNNEL_TRACE size ==
>> ioam6_trace_hdr then you know it is the legacy argument vs sizeof
>> ioam6_iptunnel_trace which is the new.
> 
> OK, I see. The problem is ioam6_trace_hdr must be the last entry because of its last field, which is "__u8 data[0]". But, anyway, I could still apply the same kind of logic with the size.

ok, forgot about the data field.

Why not make the new data separate attributes then? Avoids the alignment
problem.