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

[David Ahern <dsahern@gmail.com>]

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.