Skip to contentSkip to conversation

IPv4 tunnels: why IP-IP and SIT enforce DF bit, but GRE does not?

DISCUSSION Konstantin Shemyak 3 participants 7 messages Nov 26, 2015 – Dec 5, 2015

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

Konstantin Shemyak

author
#0 lore raw

The kernel has taken the decision to always enforce DF bit on IPv4 tunnels, which have fixed (not inherited) TTL (e.g. net/ipv4/ipip.c:ipip_tunnel_ioctl()). Commment by Alexey Kuznetsov in the head of ip_gre.c explains that the reason is attempting to avoid network loops.

But the commit c54419321455631 removed this enforcing from GRE tunnels, not changing this behavior for IP-IP (net/ipv4/ipip.c) and SIT (net/ipv6/sit.c).

It can be discussed whether such enforcing of DF bit is exactly the desired behavior, but shouldn't it at least be identical across IPv4 tunnels?

Konstantin Shemyak

#1 lore raw

Hello,

Quoted 6 lines · Konstantin Shemyak

The kernel has taken the decision to always enforce DF bit on IPv4 tunnels, which have fixed (not inherited) TTL (e.g. net/ipv4/ipip.c:ipip_tunnel_ioctl()). Commment by Alexey Kuznetsov in the head of ip_gre.c explains that the reason is attempting to avoid network loops.

Quoted 7 lines

But the commit c54419321455631 removed this enforcing from GRE tunnels, not changing this behavior for IP-IP (net/ipv4/ipip.c) and SIT (net/ipv6/sit.c).

It can be discussed whether such enforcing of DF bit is exactly the desired behavior, but shouldn't it at least be identical across IPv4 tunnels?

Very simple, I would like to see DF bit being enforced in case we have a static TTL. Parvin, any reasons you removed this code? It is currently the only way to make sure the network does not kill itself in an endless loop on configuration mistakes.

Thanks for noticing,
Hannes

author
#2 lore raw
Quoted 13 lines · Hannes Frederic Sowa

Hello,

Quoted 10 lines · Konstantin Shemyak

The kernel has taken the decision to always enforce DF bit on IPv4 tunnels, which have fixed (not inherited) TTL (e.g. net/ipv4/ipip.c:ipip_tunnel_ioctl()). Commment by Alexey Kuznetsov in the head of ip_gre.c explains that the reason is attempting to avoid network loops.

But the commit c54419321455631 removed this enforcing from GRE tunnels, not changing this behavior for IP-IP (net/ipv4/ipip.c) and SIT (net/ipv6/sit.c).

Quoted 8 lines
Quoted 3 lines

It can be discussed whether such enforcing of DF bit is exactly the desired behavior, but shouldn't it at least be identical across IPv4 tunnels?

Very simple, I would like to see DF bit being enforced in case we have a static TTL. Parvin, any reasons you removed this code? It is currently the only way to make sure the network does not kill itself in an endless loop on configuration mistakes.

This solution does not completely eliminate all network loops, as there may be other routers in the loop which do not set the DF bit. But it is true that this behavior is "we did all we could". My point was not to question its feasibility, but to make it similar across GRE, IP-IP and SIT tunnels.

Konstantin.

#3 lore raw
Quoted 18 lines · Konstantin Shemyak
Quoted 17 lines · Hannes Frederic Sowa

Hello,

Quoted 13 lines · Konstantin Shemyak

The kernel has taken the decision to always enforce DF bit on IPv4 tunnels, which have fixed (not inherited) TTL (e.g. net/ipv4/ipip.c:ipip_tunnel_ioctl()). Commment by Alexey Kuznetsov in the head of ip_gre.c explains that the reason is attempting to avoid network loops.

But the commit c54419321455631 removed this enforcing from GRE tunnels, not changing this behavior for IP-IP (net/ipv4/ipip.c) and SIT (net/ipv6/sit.c).

It can be discussed whether such enforcing of DF bit is exactly the desired behavior, but shouldn't it at least be identical across IPv4 tunnels?

Quoted 8 lines
Quoted 4 lines

Very simple, I would like to see DF bit being enforced in case we have a static TTL. Parvin, any reasons you removed this code? It is currently the only way to make sure the network does not kill itself in an endless loop on configuration mistakes.

This solution does not completely eliminate all network loops, as there may be other routers in the loop which do not set the DF bit. But it is true that this behavior is "we did all we could".

That is what I meant. We cannot care about other routers, at least Linux shouldn't be pinpointed down to be the problem. :)

Quoted 2 lines

My point was not to question its feasibility, but to make it similar across GRE, IP-IP and SIT tunnels.

I would send a patch to add it again if Parvin didn't have good reasons to remove it.

Thanks,
Hannes

maintainer
#4 lore raw

From: Hannes Frederic Sowa <[email protected]>
Date: Tue, 01 Dec 2015 14:30:55 +0100

Quoted 6 lines · Konstantin Shemyak
Quoted 2 lines

My point was not to question its feasibility, but to make it similar across GRE, IP-IP and SIT tunnels.

I would send a patch to add it again if Parvin didn't have good reasons to remove it.

The tunnel code consolidation created a lot of regressions and subtle unintended changes in behavior between the different tunnel types.

This DF bit issue is just yet another example of that.

I'm really disappointed at how many bugs and problems were introduced by those changes.

author
#5 lore raw
Quoted 7 lines · David Miller

From: Hannes Frederic Sowa <[email protected]>
Date: Tue, 01 Dec 2015 14:30:55 +0100

Quoted 4 lines · Konstantin Shemyak
Quoted 2 lines

My point was not to question its feasibility, but to make it similar across GRE, IP-IP and SIT tunnels.

Quoted 7 lines
Quoted 2 lines

I would send a patch to add it again if Parvin didn't have good reasons to remove it.

The tunnel code consolidation created a lot of regressions and subtle unintended changes in behavior between the different tunnel types.

This DF bit issue is just yet another example of that.

The patch is rather trivial; attached.

Konstantin.

Quoted 1 line

From b8a901a0d970907a2dc162744536b1ef07e5d45a Mon Sep 17 00:00:00 2001

From: Konstantin Shemyak <[email protected]>
Date: Sat, 5 Dec 2015 17:15:55 +0200
Subject: [PATCH] Set DF bit to GRE tunnels with fixed TTL, similarly to IP-IP
and SIT

The DF bit on IPv4 tunnels with fixed TTL is enforced in order to prevent networking loops. This was the original behavior of GRE tunnels, lost in the refactoring.

---
 net/ipv4/ip_gre.c | 4 ++++
 1 file changed, 4 insertions(+)
net/ipv4/ip_gre.c +4 -0
index 6145214..485bf27 100644
@@ -679,6 +679,10 @@ static int ipgre_tunnel_ioctl(struct net_device *dev,
 		    ((p.i_flags|p.o_flags)&(GRE_VERSION|GRE_ROUTING)))
 			return -EINVAL;
 	}
+
+        if (p.iph.ttl)
+                p.iph.frag_off |= htons(IP_DF);
+
 	p.i_flags = gre_flags_to_tnl_flags(p.i_flags);
 	p.o_flags = gre_flags_to_tnl_flags(p.o_flags);
Signature · 1 line
1.9.1
maintainer
#6 lore raw

From: Konstantin Shemyak <[email protected]>
Date: Sat, 5 Dec 2015 17:24:39 +0200

Quoted 12 lines · David Miller
Quoted 11 lines

From: Hannes Frederic Sowa <[email protected]>
Date: Tue, 01 Dec 2015 14:30:55 +0100

Quoted 7 lines · Konstantin Shemyak
Quoted 2 lines

My point was not to question its feasibility, but to make it similar across GRE, IP-IP and SIT tunnels.

I would send a patch to add it again if Parvin didn't have good reasons
to remove it.

Quoted 6 lines
Quoted 4 lines

The tunnel code consolidation created a lot of regressions and subtle unintended changes in behavior between the different tunnel types.

This DF bit issue is just yet another example of that.

The patch is rather trivial; attached.

No signoff, so I can't apply this.

Also patches should be submitted as fresh mailing list postings, with the patch inline rather than as an attachment.

Thanks.