diff mbox series

[net-next,05/14] gtp: Remove special mtu handling

Message ID 20170919003904.5124-6-tom@quantonium.net
State Changes Requested, archived
Delegated to: David Miller
Headers show
Series gtp: Additional feature support | expand

Commit Message

Tom Herbert Sept. 19, 2017, 12:38 a.m. UTC
Removes MTU handling in gtp_build_skb_ip4. This is non standard relative
to how other tunneling protocols handle MTU. The model espoused is that
the inner interface should set it's MTU to be less than the expected
path MTU on the overlay network. Path MTU discovery is not typically
used for modifying tunnel MTUs.

Signed-off-by: Tom Herbert <tom@quantonium.net>
---
 drivers/net/gtp.c | 30 ------------------------------
 1 file changed, 30 deletions(-)

Comments

Harald Welte Sept. 19, 2017, 11:42 a.m. UTC | #1
Hi Tom,

On Mon, Sep 18, 2017 at 05:38:55PM -0700, Tom Herbert wrote:
> Removes MTU handling in gtp_build_skb_ip4. This is non standard relative
> to how other tunneling protocols handle MTU. The model espoused is that
> the inner interface should set it's MTU to be less than the expected
> path MTU on the overlay network. Path MTU discovery is not typically
> used for modifying tunnel MTUs.

The point of the kernel GTP module is to interoperate with existing
other GTP implementations and the practises established by cellular
operators when operating GTP in their networks.

While what you describe (chose interface MTU to be less than the
expected path MTU) is generally best practise in the Linux IP/networking
world, this is not generally reflected in the cellular
universe. You see quite a bit of GTP fragmentation due to the fact
that the transport network simply has to deal with the MTU that has
been established via the control plane between SGSN and MS/UE, without
the GGSN even being part of that negotiation.

Also, you may very well have one "gtp0" tunnel device at the GGSN,
but you are establishing individual GTP tunnels to dozesn to hundreds of
different SGSNs at operators all over the world.  You cannot reliably
set the "gtp0" interface MTU to "the path MTU of the overlay network",
as the overlay network is in fact different for each of the SGSNs you're
talking to - and each may have a different path MTU.

So unless I'm missing something, I would currently vote for staying with
the current code, which uses the path MTU to the specific destination IP
address (the SGSN).

Regards,
	Harald
Tom Herbert Sept. 19, 2017, 6:12 p.m. UTC | #2
On Tue, Sep 19, 2017 at 4:42 AM, Harald Welte <laforge@gnumonks.org> wrote:
> Hi Tom,
>
> On Mon, Sep 18, 2017 at 05:38:55PM -0700, Tom Herbert wrote:
>> Removes MTU handling in gtp_build_skb_ip4. This is non standard relative
>> to how other tunneling protocols handle MTU. The model espoused is that
>> the inner interface should set it's MTU to be less than the expected
>> path MTU on the overlay network. Path MTU discovery is not typically
>> used for modifying tunnel MTUs.
>
> The point of the kernel GTP module is to interoperate with existing
> other GTP implementations and the practises established by cellular
> operators when operating GTP in their networks.
>
> While what you describe (chose interface MTU to be less than the
> expected path MTU) is generally best practise in the Linux IP/networking
> world, this is not generally reflected in the cellular
> universe. You see quite a bit of GTP fragmentation due to the fact
> that the transport network simply has to deal with the MTU that has
> been established via the control plane between SGSN and MS/UE, without
> the GGSN even being part of that negotiation.
>
> Also, you may very well have one "gtp0" tunnel device at the GGSN,
> but you are establishing individual GTP tunnels to dozesn to hundreds of
> different SGSNs at operators all over the world.  You cannot reliably
> set the "gtp0" interface MTU to "the path MTU of the overlay network",
> as the overlay network is in fact different for each of the SGSNs you're
> talking to - and each may have a different path MTU.
>
> So unless I'm missing something, I would currently vote for staying with
> the current code, which uses the path MTU to the specific destination IP
> address (the SGSN).
>
Okay, I'll modify tnl_update_pmtu so we can call it from GTP and not
have to replicate that function. I suspect VXLAN might also what this
at some point.

Tom

> Regards,
>         Harald
>
> --
> - Harald Welte <laforge@gnumonks.org>           http://laforge.gnumonks.org/
> ============================================================================
> "Privacy in residential applications is a desirable marketing option."
>                                                   (ETSI EN 300 175-7 Ch. A6)
diff mbox series

Patch

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 1de2ea6217ea..f2089fa4f004 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -466,8 +466,6 @@  static int gtp_build_skb_ip4(struct sk_buff *skb, struct net_device *dev,
 	struct iphdr *iph;
 	struct sock *sk;
 	__be32 saddr;
-	__be16 df;
-	int mtu;
 
 	/* Read the IP destination address and resolve the PDP context.
 	 * Prepend PDP header with TEI/TID from PDP ctx.
@@ -510,34 +508,6 @@  static int gtp_build_skb_ip4(struct sk_buff *skb, struct net_device *dev,
 
 	skb_dst_drop(skb);
 
-	/* This is similar to tnl_update_pmtu(). */
-	df = iph->frag_off;
-	if (df) {
-		mtu = dst_mtu(&rt->dst) - dev->hard_header_len -
-			sizeof(struct iphdr) - sizeof(struct udphdr);
-		switch (pctx->gtp_version) {
-		case GTP_V0:
-			mtu -= sizeof(struct gtp0_header);
-			break;
-		case GTP_V1:
-			mtu -= sizeof(struct gtp1_header);
-			break;
-		}
-	} else {
-		mtu = dst_mtu(&rt->dst);
-	}
-
-	rt->dst.ops->update_pmtu(&rt->dst, NULL, skb, mtu);
-
-	if (!skb_is_gso(skb) && (iph->frag_off & htons(IP_DF)) &&
-	    mtu < ntohs(iph->tot_len)) {
-		netdev_dbg(dev, "packet too big, fragmentation needed\n");
-		memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
-		icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
-			  htonl(mtu));
-		goto err_rt;
-	}
-
 	gtp_set_pktinfo_ipv4(pktinfo, sk, iph, pctx, rt, &fl4, dev);
 	gtp_push_header(skb, pktinfo);