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

Date:   Fri, 1 Oct 2021 16:10:55 +0200 (CEST)
From:   Justin Iurman <justin.iurman@...ege.be>
To:     David Ahern <dsahern@...il.com>
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

>>>>>>  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.

Great idea, I'll do that.