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 <jchap...@katalix.com > <mailto:jchap...@katalix.com>> 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.
