Re: [PATCH net v2] L2TP:Adjust intf MTU,factor underlay L3,overlay L2

From: James Chapman
Date: Tue Oct 11 2016 - 03:48:02 EST


On 11/10/16 02:54, R Parameswaran wrote:
>
>
> Hi James,
>
> Please see inline:
>
> On Tue, Oct 4, 2016 at 12:53 AM, James Chapman <jchapman@xxxxxxxxxxx
> <mailto:jchapman@xxxxxxxxxxx>> wrote:
>
> On 04/10/16 04:12, R. Parameswaran wrote:
> >
> > Hi James,
> >
> > Please see inline, thanks for the reply:
> >
> > On Sat, 1 Oct 2016, James Chapman wrote:
> >
> >> On 30/09/16 03:39, R. Parameswaran wrote:
> >>>>> + /* Adjust MTU, factor overhead - underlay L3 hdr, overlay
> L2 hdr*/
> >>>>> + if (tunnel->sock->sk_family == AF_INET)
> >>>>> + overhead += (ETH_HLEN + sizeof(struct iphdr));
> >>>>> + else if (tunnel->sock->sk_family == AF_INET6)
> >>>>> + overhead += (ETH_HLEN + sizeof(struct ipv6hdr));
> >>>> What about options in the IP header? If certain options are
> set on the
> >>>> socket, the IP header may be larger.
> >>>>
> >>> Thanks for the reply - It looks like IP options can only be
> >>> enabled through setsockopt on an application's socket (if
> there's any
> >>> other way to turn on IP options, please let me know - didn't
> see any
> >>> sysctl setting for transmit). This scenario would come
> >>> into picture when an application opens a raw IP or UDP socket
> such that it
> >>> routes into the L2TP logical interface.
> >> No. An L2TP daemon (userspace) will open a socket for each
> tunnel that
> >> it creates. Control and data packets use the same socket, which
> is the
> >> socket used by this code. It may set any options on its
> sockets. L2TP
> >> tunnel sockets can be created either by an L2TP daemon (managed
> tunnels)
> >> or by ip l2tp commands (unmanaged tunnels).
> >>
> > One Q I have is whether it would be sufficient to solve this for the
> > common case (i.e no IP options) and have an expectation that the
> > administrator will explicitly provision the mtu using the 'ip
> link ...
> > mtu' command when dealing with infrequent occurences like IP
> options?
> >
> > But looking at the code, it looks to be possible to pick up whether
> > options are enabled and how long the options are, from the
> ip_options struct
> > embedded in the tunnel socket. If you want me to, I can repost
> the patch
> > with this change (will need a few days) - please let me know if
> this is
> > what you had in mind.
> >
> >
> Yes, that's what I had in mind. But my preference would be that this
> would be a new function in the ip core, for use by any encap protocol,
> where appropriate.
>
> Discussed this with Nachi (nprachan), we were thinking of a new
> function in ip_sockglue.c which would take the tunnel socket as
> parameter, derive the underlay device MTU and compute the underlay L3
> overhead (IPv4/IPv6 header, UDP header if it is a UDP socket, and IP
> option length if the ip_options struct exists in the socket). The
> function would be agnostic to the tunnel type (although we could
> provision tunnel-type and encap type as parameters). Callers would
> call it to figure out the cumulative underlay L3 overhead and the
> underlay MTU, and then use these numbers in the MTU calculation for
> their specific tunnel type. Let me know if that is different from what
> you had in mind, and/or if you have any suggestions on which file to
> place this in. I'll try and have this re-posted by the end of this
> week or by early next week.
>

I think keep it simple. A function to return the size of the IP header
associated with any IP socket, not necessarily a tunnel socket. Don't
mix in any MTU derivation logic or UDP header size etc.

Post code early as an RFC. You're more likely to get review feedback
from others.