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:   Wed, 29 Sep 2021 21:26:23 -0600
From:   David Ahern <dsahern@...il.com>
To:     Justin Iurman <justin.iurman@...ege.be>, netdev@...r.kernel.org
Cc:     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 9/28/21 1:03 PM, Justin Iurman wrote:
> @@ -42,34 +49,15 @@ static struct ioam6_lwt_encap *ioam6_lwt_info(struct lwtunnel_state *lwt)
>  	return &ioam6_lwt_state(lwt)->tuninfo;
>  }
>  
> -static struct ioam6_trace_hdr *ioam6_trace(struct lwtunnel_state *lwt)
> +static struct ioam6_trace_hdr *ioam6_lwt_trace(struct lwtunnel_state *lwt)
>  {
>  	return &(ioam6_lwt_state(lwt)->tuninfo.traceh);
>  }
>  
>  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 */
+};

Also, no gaps in uapi. Make sure all holes are stated; an anonymous
entry is best.


>  };
>  
> -static int nla_put_ioam6_trace(struct sk_buff *skb, int attrtype,
> -			       struct ioam6_trace_hdr *trace)
> -{
> -	struct ioam6_trace_hdr *data;
> -	struct nlattr *nla;
> -	int len;
> -
> -	len = sizeof(*trace);
> -
> -	nla = nla_reserve(skb, attrtype, len);
> -	if (!nla)
> -		return -EMSGSIZE;
> -
> -	data = nla_data(nla);
> -	memcpy(data, trace, len);
> -
> -	return 0;
> -}
> -

quite a bit of the change seems like refactoring from existing feature
to allow the new ones. Please submit refactoring changes as a
prerequisite patch. The patch that introduces your new feature should be
focused solely on what is needed to implement that feature.