netdev - Re: [PATCH net-next v4 4/5] ipv6: ioam: Support for IOAM injection with lwtunnels

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]

Message-ID: <20210529140632.57b434b4@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Date:   Sat, 29 May 2021 14:06:32 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Justin Iurman <justin.iurman@...ege.be>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, tom@...bertland.com
Subject: Re: [PATCH net-next v4 4/5] ipv6: ioam: Support for IOAM injection
 with lwtunnels

On Thu, 27 May 2021 17:16:51 +0200 Justin Iurman wrote:
> Add support for the IOAM inline insertion (only for the host-to-host use case)
> which is per-route configured with lightweight tunnels. The target is iproute2
> and the patch is ready. It will be posted as soon as this patchset is merged.
> Here is an overview:
> 
> $ ip -6 ro ad fc00::1/128 encap ioam6 trace type 0x800000 ns 1 size 12 dev eth0
> 
> This example configures an IOAM Pre-allocated Trace option attached to the
> fc00::1/128 prefix. The IOAM namespace (ns) is 1, the size of the pre-allocated
> trace data block is 12 octets (size) and only the first IOAM data (bit 0:
> hop_limit + node id) is included in the trace (type) represented as a bitfield.
> 
> The reason why the in-transit (IPv6-in-IPv6 encapsulation) use case is not
> implemented is explained on the patchset cover.
> 
> Signed-off-by: Justin Iurman <justin.iurman@...ege.be>

> +static const struct nla_policy ioam6_iptunnel_policy[IOAM6_IPTUNNEL_MAX + 1] = {
> +	[IOAM6_IPTUNNEL_TRACE]	= { .type = NLA_BINARY },

Please use NLA_POLICY_EXACT_LEN(sizeof(..)), that should avoid the need
for explicit check in the code.

> +};

> +static int ioam6_build_state(struct net *net, struct nlattr *nla,
> +			     unsigned int family, const void *cfg,
> +			     struct lwtunnel_state **ts,
> +			     struct netlink_ext_ack *extack)
> +{
> +	struct nlattr *tb[IOAM6_IPTUNNEL_MAX + 1];
> +	struct ioam6_lwt_encap *tuninfo;
> +	struct ioam6_trace_hdr *trace;
> +	struct lwtunnel_state *s;
> +	int len_aligned;
> +	int len, err;
> +
> +	if (family != AF_INET6)
> +		return -EINVAL;
> +
> +	err = nla_parse_nested(tb, IOAM6_IPTUNNEL_MAX, nla,
> +			       ioam6_iptunnel_policy, extack);
> +	if (err < 0)
> +		return err;
> +
> +	if (!tb[IOAM6_IPTUNNEL_TRACE])
> +		return -EINVAL;
> +
> +	trace = nla_data(tb[IOAM6_IPTUNNEL_TRACE]);
> +	if (nla_len(tb[IOAM6_IPTUNNEL_TRACE]) != sizeof(*trace))
> +		return -EINVAL;
> +
> +	if (!ioam6_validate_trace_hdr(trace))
> +		return -EINVAL;

It'd be good to set the extack message and attribute here. And perhaps
a message for the case of trace missing.

> +	len = sizeof(*tuninfo) + trace->remlen * 4;
> +	len_aligned = ALIGN(len, 8);