Re: Regression for ip6-in-ip4 IPsec tunnel in 4.14.16

From: Yves-Alexis Perez
Date: Wed Feb 07 2018 - 14:46:40 EST


On Wed, 2018-02-07 at 13:50 -0500, Mike Maloney wrote:
> On Wed, Feb 7, 2018 at 12:23 PM, Yves-Alexis Perez <corsac@xxxxxxxxxx>
>
> Hi Yves-Alexis -
>
> I apologize for the problem. It seems to me that tunneling with an
> outer MTU that causes the inner MTU to be smaller than the min, is
> potentially problematic in other ways as well.

Maybe. I tried with removing the MTU setting, and I get (on ping again)

fÃvr. 07 20:44:01 scapa kernel: mtu: 1266

which means I would get -EINVAL on standards kernels, which is not really good
either.

> But also it could seem unfortunate that the code with my fix does not
> look at actual packet size, but instead only looks at the MTU and then
> fails, even if no packet was going to be so large. The intention of
> my patch was to prevent a negative number while calculating the
> maxfraglen in __ip6_append_data(). An alternative fix maybe to
> instead return an error only if the mtu is less than or equal to the
> fragheaderlen. Something like:
>
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 3763dc01e374..5d912a289b95 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1214,8 +1214,6 @@ static int ip6_setup_cork(struct sock *sk,
> struct inet_cork_full *cork,
> if (np->frag_size)
> mtu = np->frag_size;
> }
> - if (mtu < IPV6_MIN_MTU)
> - return -EINVAL;
> cork->base.fragsize = mtu;
> if (dst_allfrag(rt->dst.path))
> cork->base.flags |= IPCORK_ALLFRAG;
> @@ -1264,6 +1262,8 @@ static int __ip6_append_data(struct sock *sk,
>
> fragheaderlen = sizeof(struct ipv6hdr) + rt->rt6i_nfheader_len +
> (opt ? opt->opt_nflen : 0);
> + if (mtu < fragheaderlen + 8)
> + return -EINVAL;
> maxfraglen = ((mtu - fragheaderlen) & ~7) + fragheaderlen -
> sizeof(struct frag_hdr);
> (opt ? opt->opt_nflen : 0);
>
> But then we also have to convince ourselves that maxfraglen can never
> be <= 0. I'd have to think about that.
>
> I am not sure if others have thoughts on supporting MTUs configured
> below the min in the spec.
>
Here, the MTU is not below, so I'm not sure what happens.

Regards,
--
Yves-Alexis

Attachment: signature.asc
Description: This is a digitally signed message part