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

Subsystems: networking [general], the rest

STALE3661d REVIEWED: 1 (0M)

1 review trailer.

12 messages, 4 authors, 2016-10-04 · open the first message on its own page

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

From: R. Parameswaran <hidden>
Date: 2016-09-22 20:52:41

From ed585bdd6d3d2b3dec58d414f514cd764d89159d Mon Sep 17 00:00:00 2001
From: "R. Parameswaran" <redacted>
Date: Thu, 22 Sep 2016 13:19:25 -0700
Subject: [PATCH] L2TP:Adjust intf MTU,factor underlay L3,overlay L2

Take into account all of the tunnel encapsulation headers when setting
up the MTU on the L2TP logical interface device. Otherwise, packets
created by the applications on top of the L2TP layer are larger
than they ought to be, relative to the underlay MTU, leading to
needless fragmentation once the outer IP encap is added.

Specifically, take into account the (outer, underlay) IP header
imposed on the encapsulated L2TP packet, and the Layer 2 header
imposed on the inner IP packet prior to L2TP encapsulation.

Do not assume an Ethernet (non-jumbo) underlay. Use the PMTU mechanism
and the dst entry in the L2TP tunnel socket to directly pull up
the underlay MTU (as the baseline number on top of which the
encapsulation headers are factored in).  Fall back to Ethernet MTU
if this fails.

Signed-off-by: R. Parameswaran <redacted>

Reviewed-by: "N. Prachanda" <redacted>,
Reviewed-by: "R. Shearman" <redacted>,
Reviewed-by: "D. Fawcus" <redacted>
---
 net/l2tp/l2tp_eth.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)
diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
index 57fc5a4..dbcd6bd 100644
--- a/net/l2tp/l2tp_eth.c
+++ b/net/l2tp/l2tp_eth.c
@@ -30,6 +30,9 @@
 #include <net/xfrm.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/udp.h>
 
 #include "l2tp_core.h"
 
@@ -206,6 +209,46 @@ static void l2tp_eth_show(struct seq_file *m, void *arg)
 }
 #endif
 
+static void l2tp_eth_adjust_mtu(struct l2tp_tunnel *tunnel,
+				struct l2tp_session *session,
+				struct net_device *dev)
+{
+	unsigned int overhead = 0;
+	struct dst_entry *dst;
+
+	if (session->mtu != 0) {
+		dev->mtu = session->mtu;
+		dev->needed_headroom += session->hdr_len;
+		if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
+			dev->needed_headroom += sizeof(struct udphdr);
+		return;
+	}
+	overhead = session->hdr_len;
+	/* 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));
+	/* Additionally, if the encap is UDP, account for UDP header size */
+	if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
+		overhead += sizeof(struct udphdr);
+	/* If PMTU discovery was enabled, use discovered MTU on L2TP device */
+	dst = sk_dst_get(tunnel->sock);
+	if (dst) {
+		u32 pmtu = dst_mtu(dst);
+
+		if (pmtu != 0)
+			dev->mtu = pmtu;
+		dst_release(dst);
+	}
+	/* else (no PMTUD) L2TP dev MTU defaulted to Ethernet MTU in caller */
+	session->mtu = dev->mtu - overhead;
+	dev->mtu = session->mtu;
+	dev->needed_headroom += session->hdr_len;
+	if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
+		dev->needed_headroom += sizeof(struct udphdr);
+}
+
 static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg)
 {
 	struct net_device *dev;
@@ -255,11 +298,8 @@ static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 p
 	}
 
 	dev_net_set(dev, net);
-	if (session->mtu == 0)
-		session->mtu = dev->mtu - session->hdr_len;
-	dev->mtu = session->mtu;
-	dev->needed_headroom += session->hdr_len;
 
+	l2tp_eth_adjust_mtu(tunnel, session, dev);
 	priv = netdev_priv(dev);
 	priv->dev = dev;
 	priv->session = session;
-- 
2.1.4

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

From: David Miller <davem@davemloft.net>
Date: 2016-09-27 07:32:02

From: "R. Parameswaran" <redacted>
Date: Thu, 22 Sep 2016 13:52:43 -0700 (PDT)
From ed585bdd6d3d2b3dec58d414f514cd764d89159d Mon Sep 17 00:00:00 2001
From: "R. Parameswaran" <redacted>
Date: Thu, 22 Sep 2016 13:19:25 -0700
Subject: [PATCH] L2TP:Adjust intf MTU,factor underlay L3,overlay L2

Take into account all of the tunnel encapsulation headers when setting
up the MTU on the L2TP logical interface device. Otherwise, packets
created by the applications on top of the L2TP layer are larger
than they ought to be, relative to the underlay MTU, leading to
needless fragmentation once the outer IP encap is added.

Specifically, take into account the (outer, underlay) IP header
imposed on the encapsulated L2TP packet, and the Layer 2 header
imposed on the inner IP packet prior to L2TP encapsulation.

Do not assume an Ethernet (non-jumbo) underlay. Use the PMTU mechanism
and the dst entry in the L2TP tunnel socket to directly pull up
the underlay MTU (as the baseline number on top of which the
encapsulation headers are factored in).  Fall back to Ethernet MTU
if this fails.

Signed-off-by: R. Parameswaran <redacted>

Reviewed-by: "N. Prachanda" <redacted>,
Reviewed-by: "R. Shearman" <redacted>,
Reviewed-by: "D. Fawcus" <redacted>
I have to ask, how do other tunnels over UDP such as VXLAN handle
this problem?

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

From: R. Parameswaran <hidden>
Date: 2016-09-27 19:17:40

Hi David,

Thanks for the reply, please see inline:

On Tue, 27 Sep 2016, David Miller wrote:
From: "R. Parameswaran" <redacted>
Date: Thu, 22 Sep 2016 13:52:43 -0700 (PDT)
quoted
From ed585bdd6d3d2b3dec58d414f514cd764d89159d Mon Sep 17 00:00:00 2001
From: "R. Parameswaran" <redacted>
Date: Thu, 22 Sep 2016 13:19:25 -0700
Subject: [PATCH] L2TP:Adjust intf MTU,factor underlay L3,overlay L2

Take into account all of the tunnel encapsulation headers when setting
up the MTU on the L2TP logical interface device. Otherwise, packets
created by the applications on top of the L2TP layer are larger
than they ought to be, relative to the underlay MTU, leading to
needless fragmentation once the outer IP encap is added.

Specifically, take into account the (outer, underlay) IP header
imposed on the encapsulated L2TP packet, and the Layer 2 header
imposed on the inner IP packet prior to L2TP encapsulation.

Do not assume an Ethernet (non-jumbo) underlay. Use the PMTU mechanism
and the dst entry in the L2TP tunnel socket to directly pull up
the underlay MTU (as the baseline number on top of which the
encapsulation headers are factored in).  Fall back to Ethernet MTU
if this fails.

Signed-off-by: R. Parameswaran <redacted>

Reviewed-by: "N. Prachanda" <redacted>,
Reviewed-by: "R. Shearman" <redacted>,
Reviewed-by: "D. Fawcus" <redacted>
I have to ask, how do other tunnels over UDP such as VXLAN handle
this problem?
Specific to Vxlan, it appears to behave similarly.  I haven't functionally 
tested fragmentation on vxlan interfaces, but looking at the
code, it seems to account for the headers involved:


When the vxlan interface is created, from vxlan_dev_create(), in 
vxlan_setup(), it initially starts off with an ethernet MTU:

vxlan_setup(struct net_device *dev)
{
...
...
        ether_setup(dev); <<<<<<< Will set device MTU to 1500


Later, in vxlan_dev_configure(), called from vxlan_dev_create(), it gets 
adjusted to account for the headers:

vxlan_dev_configure():
...
 if (!conf->mtu)
                        dev->mtu = lowerdev->mtu - (use_ipv6 ? 
VXLAN6_HEADROOM : VXLAN_HEADROOM);


where VXLAN_HEADROOM is defined as follows: 

/* IP header + UDP + VXLAN + Ethernet header */
#define VXLAN_HEADROOM (20 + 8 + 8 + 14)
/* IPv6 header + UDP + VXLAN + Ethernet header */
#define VXLAN6_HEADROOM (40 + 8 + 8 + 14)


This seems to match what I see with hand config:

sudo ip link add vxlan0 type vxlan id 42 group 239.1.1.1 dev eth0 dstport 
4789 <<<< (eth0 has an MTU of 1500)


sudo ip -d link show vxlan0
36: vxlan0: <BROADCAST,MULTICAST> mtu 1450 qdisc noop state DOWN mode 
DEFAULT group default <<<< (1450 = 1500 -50)
    link/ether e2:b8:2d:f4:f7:ae brd ff:ff:ff:ff:ff:ff promiscuity 0
    vxlan id 42 group 239.1.1.1 dev eth0 srcport 32768 61000 dstport 4789 
ageing 300

thanks,

Ramkumar

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

From: David Miller <davem@davemloft.net>
Date: 2016-09-28 07:49:47

From: "R. Parameswaran" <redacted>
Date: Tue, 27 Sep 2016 12:17:21 -0700 (PDT)
Later, in vxlan_dev_configure(), called from vxlan_dev_create(), it gets 
adjusted to account for the headers:

vxlan_dev_configure():
...
 if (!conf->mtu)
                        dev->mtu = lowerdev->mtu - (use_ipv6 ? 
VXLAN6_HEADROOM : VXLAN_HEADROOM);


where VXLAN_HEADROOM is defined as follows: 

/* IP header + UDP + VXLAN + Ethernet header */
#define VXLAN_HEADROOM (20 + 8 + 8 + 14)
/* IPv6 header + UDP + VXLAN + Ethernet header */
#define VXLAN6_HEADROOM (40 + 8 + 8 + 14)
Right but I don't see it going through the effort to make use of the
PMTU like you are.

I have another strong concern related to this.  There seems to be no
mechanism used to propagate any PMTU events into the device's MTU.

Because if there is a limiting nexthop in the route to the other end
of the UDP tunnel, you won't learn the PMTU until you (or some other
entity on the machine) actually starts sending traffic to the tunnel's
endpoint.

If the PMTU events aren't propagated into the tunnel's MTU or similar
I think this is an ad-hoc solution.

I would suggest that you either:

1) Do what VXLAN appears to do an ignore the PMTu

2) Add code to handle PMTU events that land on the UDP tunnel
   socket.

Thanks.

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

From: R. Parameswaran <hidden>
Date: 2016-09-29 02:37:30


Hi David,

Please see inline:

On Wed, 28 Sep 2016, David Miller wrote:
From: "R. Parameswaran" <redacted>
Date: Tue, 27 Sep 2016 12:17:21 -0700 (PDT)
quoted
Later, in vxlan_dev_configure(), called from vxlan_dev_create(), it gets 
adjusted to account for the headers:

vxlan_dev_configure():
...
 if (!conf->mtu)
                        dev->mtu = lowerdev->mtu - (use_ipv6 ? 
VXLAN6_HEADROOM : VXLAN_HEADROOM);


where VXLAN_HEADROOM is defined as follows: 

/* IP header + UDP + VXLAN + Ethernet header */
#define VXLAN_HEADROOM (20 + 8 + 8 + 14)
/* IPv6 header + UDP + VXLAN + Ethernet header */
#define VXLAN6_HEADROOM (40 + 8 + 8 + 14)
Right but I don't see it going through the effort to make use of the
PMTU like you are.

I have another strong concern related to this.  There seems to be no
mechanism used to propagate any PMTU events into the device's MTU.

Because if there is a limiting nexthop in the route to the other end
of the UDP tunnel, you won't learn the PMTU until you (or some other
entity on the machine) actually starts sending traffic to the tunnel's
endpoint.

If the PMTU events aren't propagated into the tunnel's MTU or similar
I think this is an ad-hoc solution.

I would suggest that you either:

1) Do what VXLAN appears to do an ignore the PMTu
I'd like to point out one difference with VXLAN - in VXLAN, the 
local physical interface is directly specified at the time of 
creation of the tunnel, and the data structure seems to have the ifindex 
of the local interface with which it is able to directly pull up the 
underlay interface device. Whereas in L2TP, we only have the IP
address of the remote tunnel end-point and thus only the socket and the 
dst from which we need to derive this. 

Also, dst_mtu references dst->ops->mtu, which if I followed the pointer
chain correctly, will dereference to ipv4_mtu() (for the IPv4 case, as
an example). The code in ipv4_mtu looks like the following:

ipv4_mtu():

        unsigned int mtu = rt->rt_pmtu;

        if (!mtu || time_after_eq(jiffies, rt->dst.expires))
                mtu = dst_metric_raw(dst, RTAX_MTU);

        if (mtu)
                return mtu;

        mtu = dst->dev->mtu;

        if (unlikely(dst_metric_locked(dst, RTAX_MTU))) {
                if (rt->rt_uses_gateway && mtu > 576)
                        mtu = 576;
        }

        return min_t(unsigned int, mtu, IP_MAX_MTU);

The code above does not depend on PMTU to be working. If no PMTU 
discovered MTU exists, it eventually falls back to the local 
underlay device MTU - and this is the mode in which I tested the fix - PMTU 
was off in my testbed, but it was picking up the local device MTU correctly.

Basically, this looks better than the VXLAN handling as far as I can 
tell - at least it will pick up the existing discovered PMTU on a best 
effort basis, while falling back to the underlay device if all else fails. 

I agree that something like 2. below would be needed in the long run (it 
will need some effort and redesign -e.g. how do I lookup the parent tunnel 
from the socket when receiving a PMTU update, existing pointer chain runs 
from tunnel to socket).  

But since the existing (Ethernet over L2TP) MTU derivation is incorrect, I am 
hoping this may be acceptable as an interim solution. 

thanks,

Ramkumar

2) Add code to handle PMTU events that land on the UDP tunnel
   socket.

Thanks.

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

From: Jiri Benc <hidden>
Date: 2016-09-29 12:22:09

On Wed, 28 Sep 2016 19:36:45 -0700 (PDT), R. Parameswaran wrote:
I'd like to point out one difference with VXLAN - in VXLAN, the 
local physical interface is directly specified at the time of 
creation of the tunnel, and the data structure seems to have the ifindex 
of the local interface with which it is able to directly pull up the 
underlay interface device. Whereas in L2TP, we only have the IP
address of the remote tunnel end-point and thus only the socket and the 
dst from which we need to derive this. 
Strictly speaking, VXLAN *may* know the underlying interface. It can
also be set up with just local and remote IP address, or even worse, in
metadata mode where we don't know the address nor the interface until
we get a packet (and each packet may have those different).

MTU wise, those cases are not accommodated for in the kernel. The vxlan
interface gets MTU of 1500 and it's up to the administrator to set it
correctly.

Btw, PMTU events won't help with the metadata mode. And even in
"normal" mode, it's not clear what should be done - the tunnel
interface may be in a bridge, thus there may be other interfaces that
depend on the same MTU, up to inside VMs.

 Jiri

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

From: James Chapman <jchapman@katalix.com>
Date: 2016-09-29 15:28:53

On 22/09/16 21:52, R. Parameswaran wrote:
quoted hunk
From ed585bdd6d3d2b3dec58d414f514cd764d89159d Mon Sep 17 00:00:00 2001
From: "R. Parameswaran" <redacted>
Date: Thu, 22 Sep 2016 13:19:25 -0700
Subject: [PATCH] L2TP:Adjust intf MTU,factor underlay L3,overlay L2

Take into account all of the tunnel encapsulation headers when setting
up the MTU on the L2TP logical interface device. Otherwise, packets
created by the applications on top of the L2TP layer are larger
than they ought to be, relative to the underlay MTU, leading to
needless fragmentation once the outer IP encap is added.

Specifically, take into account the (outer, underlay) IP header
imposed on the encapsulated L2TP packet, and the Layer 2 header
imposed on the inner IP packet prior to L2TP encapsulation.

Do not assume an Ethernet (non-jumbo) underlay. Use the PMTU mechanism
and the dst entry in the L2TP tunnel socket to directly pull up
the underlay MTU (as the baseline number on top of which the
encapsulation headers are factored in).  Fall back to Ethernet MTU
if this fails.

Signed-off-by: R. Parameswaran <redacted>

Reviewed-by: "N. Prachanda" <redacted>,
Reviewed-by: "R. Shearman" <redacted>,
Reviewed-by: "D. Fawcus" <redacted>
---
 net/l2tp/l2tp_eth.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)
diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
index 57fc5a4..dbcd6bd 100644
--- a/net/l2tp/l2tp_eth.c
+++ b/net/l2tp/l2tp_eth.c
@@ -30,6 +30,9 @@
 #include <net/xfrm.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/udp.h>
 
 #include "l2tp_core.h"
 
@@ -206,6 +209,46 @@ static void l2tp_eth_show(struct seq_file *m, void *arg)
 }
 #endif
 
+static void l2tp_eth_adjust_mtu(struct l2tp_tunnel *tunnel,
+				struct l2tp_session *session,
+				struct net_device *dev)
+{
+	unsigned int overhead = 0;
+	struct dst_entry *dst;
+
+	if (session->mtu != 0) {
+		dev->mtu = session->mtu;
+		dev->needed_headroom += session->hdr_len;
+		if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
+			dev->needed_headroom += sizeof(struct udphdr);
+		return;
+	}
+	overhead = session->hdr_len;
+	/* 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.
quoted hunk
+	/* Additionally, if the encap is UDP, account for UDP header size */
+	if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
+		overhead += sizeof(struct udphdr);
+	/* If PMTU discovery was enabled, use discovered MTU on L2TP device */
+	dst = sk_dst_get(tunnel->sock);
+	if (dst) {
+		u32 pmtu = dst_mtu(dst);
+
+		if (pmtu != 0)
+			dev->mtu = pmtu;
+		dst_release(dst);
+	}
+	/* else (no PMTUD) L2TP dev MTU defaulted to Ethernet MTU in caller */
+	session->mtu = dev->mtu - overhead;
+	dev->mtu = session->mtu;
+	dev->needed_headroom += session->hdr_len;
+	if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
+		dev->needed_headroom += sizeof(struct udphdr);
+}
+
 static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg)
 {
 	struct net_device *dev;
@@ -255,11 +298,8 @@ static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 p
 	}
 
 	dev_net_set(dev, net);
-	if (session->mtu == 0)
-		session->mtu = dev->mtu - session->hdr_len;
-	dev->mtu = session->mtu;
-	dev->needed_headroom += session->hdr_len;
 
+	l2tp_eth_adjust_mtu(tunnel, session, dev);
 	priv = netdev_priv(dev);
 	priv->dev = dev;
 	priv->session = session;

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

From: James Chapman <jchapman@katalix.com>
Date: 2016-09-29 15:39:40

On 29/09/16 03:36, R. Parameswaran wrote:
I agree that something like 2. below would be needed in the long run (it 
will need some effort and redesign -e.g. how do I lookup the parent tunnel 
from the socket when receiving a PMTU update, existing pointer chain runs 
from tunnel to socket).
quoted
2) Add code to handle PMTU events that land on the UDP tunnel
   socket.
Another function pointer could be added to struct udp_sock, similar to
encap_rcv,  such that the pmtu event could be handled by the UDP encap
protocol implementation.

James

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

From: R. Parameswaran <hidden>
Date: 2016-09-30 02:40:40

Hi James,

On Thu, 29 Sep 2016, James Chapman wrote:
On 22/09/16 21:52, R. Parameswaran wrote:
quoted
From ed585bdd6d3d2b3dec58d414f514cd764d89159d Mon Sep 17 00:00:00 2001
From: "R. Parameswaran" <redacted>
Date: Thu, 22 Sep 2016 13:19:25 -0700
Subject: [PATCH] L2TP:Adjust intf MTU,factor underlay L3,overlay L2

Take into account all of the tunnel encapsulation headers when setting
up the MTU on the L2TP logical interface device. Otherwise, packets
created by the applications on top of the L2TP layer are larger
than they ought to be, relative to the underlay MTU, leading to
needless fragmentation once the outer IP encap is added.

Specifically, take into account the (outer, underlay) IP header
imposed on the encapsulated L2TP packet, and the Layer 2 header
imposed on the inner IP packet prior to L2TP encapsulation.

Do not assume an Ethernet (non-jumbo) underlay. Use the PMTU mechanism
and the dst entry in the L2TP tunnel socket to directly pull up
the underlay MTU (as the baseline number on top of which the
encapsulation headers are factored in).  Fall back to Ethernet MTU
if this fails.

Signed-off-by: R. Parameswaran <redacted>

Reviewed-by: "N. Prachanda" <redacted>,
Reviewed-by: "R. Shearman" <redacted>,
Reviewed-by: "D. Fawcus" <redacted>
---
 net/l2tp/l2tp_eth.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)
diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
index 57fc5a4..dbcd6bd 100644
--- a/net/l2tp/l2tp_eth.c
+++ b/net/l2tp/l2tp_eth.c
@@ -30,6 +30,9 @@
 #include <net/xfrm.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/udp.h>
 
 #include "l2tp_core.h"
 
@@ -206,6 +209,46 @@ static void l2tp_eth_show(struct seq_file *m, void *arg)
 }
 #endif
 
+static void l2tp_eth_adjust_mtu(struct l2tp_tunnel *tunnel,
+				struct l2tp_session *session,
+				struct net_device *dev)
+{
+	unsigned int overhead = 0;
+	struct dst_entry *dst;
+
+	if (session->mtu != 0) {
+		dev->mtu = session->mtu;
+		dev->needed_headroom += session->hdr_len;
+		if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
+			dev->needed_headroom += sizeof(struct udphdr);
+		return;
+	}
+	overhead = session->hdr_len;
+	/* 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.

If you take the case of a plain IP (ethernet) interface, even if an
application opened a socket turning on IP options, it would not change
the MTU of the underlying interface, and it would not affect other 
applications transacting packets on the same interface. I know its not an 
exact parallel to this case, but since the IP option control is per 
application, we probably should not factor it into the L2TP logical interface?
We cannot affect other applications/processes running on the same L2TP 
tunnel. Also, since the application  using IP options knows that it has turned 
on IP options, maybe we can count on it to factor the size of the options 
into the size of the payload it sends into the socket, or set the mtu on the 
L2TP interface through config? 

Other than this, I don't see keepalives or anything else in which the 
kernel will source its own packet into the L2TP interface, outside of 
an application injected packet - if there is something like that, please
let me know. The user space L2TP daemon would probably fall in the 
category of applications.

thanks,

Ramkumar 

quoted
+	/* Additionally, if the encap is UDP, account for UDP header size */
+	if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
+		overhead += sizeof(struct udphdr);
+	/* If PMTU discovery was enabled, use discovered MTU on L2TP device */
+	dst = sk_dst_get(tunnel->sock);
+	if (dst) {
+		u32 pmtu = dst_mtu(dst);
+
+		if (pmtu != 0)
+			dev->mtu = pmtu;
+		dst_release(dst);
+	}
+	/* else (no PMTUD) L2TP dev MTU defaulted to Ethernet MTU in caller */
+	session->mtu = dev->mtu - overhead;
+	dev->mtu = session->mtu;
+	dev->needed_headroom += session->hdr_len;
+	if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
+		dev->needed_headroom += sizeof(struct udphdr);
+}
+
 static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg)
 {
 	struct net_device *dev;
@@ -255,11 +298,8 @@ static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 p
 	}
 
 	dev_net_set(dev, net);
-	if (session->mtu == 0)
-		session->mtu = dev->mtu - session->hdr_len;
-	dev->mtu = session->mtu;
-	dev->needed_headroom += session->hdr_len;
 
+	l2tp_eth_adjust_mtu(tunnel, session, dev);
 	priv = netdev_priv(dev);
 	priv->dev = dev;
 	priv->session = session;

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

From: James Chapman <jchapman@katalix.com>
Date: 2016-10-01 16:50:56

On 30/09/16 03:39, R. Parameswaran wrote:
quoted
quoted
+	/* 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).
If you take the case of a plain IP (ethernet) interface, even if an
application opened a socket turning on IP options, it would not change
the MTU of the underlying interface, and it would not affect other 
applications transacting packets on the same interface. I know its not an 
exact parallel to this case, but since the IP option control is per 
application, we probably should not factor it into the L2TP logical interface?
We cannot affect other applications/processes running on the same L2TP 
tunnel. Also, since the application  using IP options knows that it has turned 
on IP options, maybe we can count on it to factor the size of the options 
into the size of the payload it sends into the socket, or set the mtu on the 
L2TP interface through config? 
No. See above.
Other than this, I don't see keepalives or anything else in which the 
kernel will source its own packet into the L2TP interface, outside of 
an application injected packet - if there is something like that, please
let me know. The user space L2TP daemon would probably fall in the 
category of applications.

thanks,

Ramkumar 

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

From: R. Parameswaran <hidden>
Date: 2016-10-04 03:12:45


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:
quoted
quoted
quoted
+	/* 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.

thanks,

Ramkumar


quoted
If you take the case of a plain IP (ethernet) interface, even if an
application opened a socket turning on IP options, it would not change
the MTU of the underlying interface, and it would not affect other 
applications transacting packets on the same interface. I know its not an 
exact parallel to this case, but since the IP option control is per 
application, we probably should not factor it into the L2TP logical interface?
We cannot affect other applications/processes running on the same L2TP 
tunnel. Also, since the application  using IP options knows that it has turned 
on IP options, maybe we can count on it to factor the size of the options 
into the size of the payload it sends into the socket, or set the mtu on the 
L2TP interface through config? 
No. See above.
quoted
Other than this, I don't see keepalives or anything else in which the 
kernel will source its own packet into the L2TP interface, outside of 
an application injected packet - if there is something like that, please
let me know. The user space L2TP daemon would probably fall in the 
category of applications.

thanks,

Ramkumar 

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

From: James Chapman <jchapman@katalix.com>
Date: 2016-10-04 07:53:29

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:
quoted
On 30/09/16 03:39, R. Parameswaran wrote:
quoted
quoted
quoted
+	/* 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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help