Skip to contentSkip to conversation

A problem about ICMP packet-too-big message handler

RFC PATCH In review Yang Yingliang 5 participants 16 messages Jan 27, 2015 – Mar 9, 2015

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

Hi,

My kernel is 3.10 LTS.

I got a problem here about handling ICMP packet-too-big message.

Before sending a packet-too-big packet :

# ip -6 route list table local
local ::1 dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80::1 dev lo  metric 0 	//It  does not have expire value
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:2 dev lo  metric 0 
local fe80::5054:ff:fe12:3456 dev lo  metric 0

After sending a packet-too-big packet

ip -6 route list table local
local ::1 dev lo metric 0

local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80::1 dev lo  metric 0  expires _597_  //It has expire value
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:2 dev lo  metric 0 

local fe80::5054:ff:fe12:3456 dev lo metric 0

When time is up, I can't ping fe80::1 .
Is it ok or a bug ?

Regards,
Yang

#1 lore raw
Quoted 7 lines · Yang Yingliang

Hi,

My kernel is 3.10 LTS.

I got a problem here about handling ICMP packet-too-big message.

Before sending a packet-too-big packet :

The expires should be set by the host _receiving_ the icmpv6 too-big.

Can you spell out some details of the outgoing icmpv6 too-big packet? like, the src/dst ip of the icmpv6 and the original ip packet that triggered the too-big.

#2 lore raw
Quoted 5 lines · Martin Lau
Quoted 4 lines · Yang Yingliang

Hi,

My kernel is 3.10 LTS.

Quoted 9 lines
Quoted 3 lines

I got a problem here about handling ICMP packet-too-big message.

Before sending a packet-too-big packet :

The expires should be set by the host _receiving_ the icmpv6 too-big.

Can you spell out some details of the outgoing icmpv6 too-big packet? like, the src/dst ip of the icmpv6 and the original ip packet that triggered the too-big.

I don't send too-big packet to trigger the err handling.

I just only send a ICMPv6 packet which type is ICMPV6_PKT_TOOBIG.

#6 lore raw
In reply to Hannes Frederic Sowa
Quoted 19 lines · Hannes Frederic Sowa
Quoted 8 lines · Yang Yingliang

Hi,

My kernel is 3.10 LTS.

I got a problem here about handling ICMP packet-too-big message.

Before sending a packet-too-big packet :

Quoted 9 lines
# ip -6 route list table local
local ::1 dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80::1 dev lo  metric 0 	//It  does not have expire value
Quoted 2 lines

Did you just add a local route or do you also have a corresponding IPv6 address bound to loopback?

I just set a IPv6 linklocal address and the route was added automatic. I do not add any other route by hand.

Regards,
Yang

#7 lore raw
In reply to Yang Yingliang
Quoted 29 lines · Yang Yingliang

Hi,

My kernel is 3.10 LTS.

I got a problem here about handling ICMP packet-too-big message.

Before sending a packet-too-big packet :

# ip -6 route list table local
local ::1 dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80::1 dev lo  metric 0 	//It  does not have expire value
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:2 dev lo  metric 0 
local fe80::5054:ff:fe12:3456 dev lo  metric 0

After sending a packet-too-big packet

ip -6 route list table local
local ::1 dev lo metric 0

Quoted 7 lines
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 

local fe80::1 dev lo metric 0 expires _597_ //It has expire value

Is this route still present after the expiration time is elapsed?

Quoted 9 lines
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:2 dev lo  metric 0 

local fe80::5054:ff:fe12:3456 dev lo metric 0

When time is up, I can't ping fe80::1 .
Is it ok or a bug ?

This looks pretty similar to a bug I discovered recently. In my case, a ipv6 host route dissapeared 10 minutes after a PMTU event. As a result, this host was not reachable anymore.

This happens because we don't clone host routes before we use them. If a PMTU event happens, the original route is marked with an expire value. After the expiration time is elapsed, the original route is deleted and we loose conectivity to the host.

I'm currently testing patches to fix this. With these patches the ipv6 host routes are cloned if they are gateway routes, i.e. if PMTU events can happen.

I fear it will not fix your case because PMTU events are not expected to happen at local fe80 routes. But you could change patch 1 to unconditionally clone the routes, if you want to check if this is your problem.

I'll sent the fixes marked as RFC in reply to this mail.

#8 lore raw

RFC PATCH 1/2 ipv6: Fix after pmtu events dissapearing host routes

We currently don't clone host routes before we use them. If a pmtu event is received on such a route, it gets an expires value. As soon as the expiration time is elapsed, the route is deleted. As a result, the host is not reachable any more.

We fix this by cloning host routes if they are gatewayed, i.e. if pmtu events can happen.

---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv6/route.c +1 -1
index c910831..3e864e7 100644
@@ -961,7 +961,7 @@ redo_rt6_select:
 
 	if (!(rt->rt6i_flags & (RTF_NONEXTHOP | RTF_GATEWAY)))
 		nrt = rt6_alloc_cow(rt, &fl6->daddr, &fl6->saddr);
-	else if (!(rt->dst.flags & DST_HOST))
+	else if (!(rt->dst.flags & DST_HOST) || (rt->rt6i_flags & RTF_GATEWAY))
 		nrt = rt6_alloc_clone(rt, &fl6->daddr);
 	else
 		goto out2;
Signature · 1 line
1.9.1
#14 lore raw

RFC PATCH 2/2 ipv6: Extend the route lookups to low priority metrics.

In reply to Steffen Klassert

We search only for routes with highest prioriy metric in find_rr_leaf(). However if one of these routes is marked as invalid, we may fail to find a route even if there is a appropriate route with lower priority. Then we loose connectivity until the garbage collector deletes the invalid route. This typically happens if a host route expires afer a pmtu event. Fix this by searching also for routes with a lower priority metric.

---
 net/ipv6/route.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)
net/ipv6/route.c +23 -5
index 3e864e7..ce354c9 100644
@@ -654,15 +654,33 @@ static struct rt6_info *find_rr_leaf(struct fib6_node *fn,
 				     u32 metric, int oif, int strict,
 				     bool *do_rr)
 {
-	struct rt6_info *rt, *match;
+	struct rt6_info *rt, *match, *cont;
 	int mpri = -1;
 
 	match = NULL;
-	for (rt = rr_head; rt && rt->rt6i_metric == metric;
-	     rt = rt->dst.rt6_next)
+	cont = NULL;
+	for (rt = rr_head; rt; rt = rt->dst.rt6_next) {
+		if (rt->rt6i_metric != metric) {
+			cont = rt;
+			break;
+		}
+
+		match = find_match(rt, oif, strict, &mpri, match, do_rr);
+	}
+
+	for (rt = fn->leaf; rt && rt != rr_head; rt = rt->dst.rt6_next) {
+		if (rt->rt6i_metric != metric) {
+			cont = rt;
+			break;
+		}
+
 		match = find_match(rt, oif, strict, &mpri, match, do_rr);
-	for (rt = fn->leaf; rt && rt != rr_head && rt->rt6i_metric == metric;
-	     rt = rt->dst.rt6_next)
+	}
+
+	if (match || !cont)
+		return match;
+
+	for (rt = cont; rt; rt = rt->dst.rt6_next)
 		match = find_match(rt, oif, strict, &mpri, match, do_rr);
 
 	return match;
Signature · 1 line
1.9.1
#4 lore raw
In reply to Eric Dumazet
Quoted 17 lines · Eric Dumazet
Quoted 16 lines · Yang Yingliang
Quoted 14 lines · Martin Lau
Quoted 7 lines · Yang Yingliang

Hi,

My kernel is 3.10 LTS.

I got a problem here about handling ICMP packet-too-big message.

Before sending a packet-too-big packet :

The expires should be set by the host _receiving_ the icmpv6 too-big.

Can you spell out some details of the outgoing icmpv6 too-big packet? like, the src/dst ip of the icmpv6 and the original ip packet that triggered the too-big.

Quoted 5 lines
Quoted 3 lines

I don't send too-big packet to trigger the err handling.

I just only send a ICMPv6 packet which type is ICMPV6_PKT_TOOBIG.

You meant : you received an ICMPv6 packet, or you send it ?

Received an ICMPv6 packet.

Regards,
Yang

#5 lore raw
In reply to Yang Yingliang
Quoted 8 lines · Yang Yingliang

Hi,

My kernel is 3.10 LTS.

I got a problem here about handling ICMP packet-too-big message.

Before sending a packet-too-big packet :

Quoted 9 lines
# ip -6 route list table local
local ::1 dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80::1 dev lo  metric 0 	//It  does not have expire value

Did you just add a local route or do you also have a corresponding IPv6 address bound to loopback?

Bye,
Hannes

#3 lore raw
In reply to Yang Yingliang
Quoted 11 lines · Yang Yingliang
Quoted 10 lines · Martin Lau
Quoted 7 lines · Yang Yingliang

Hi,

My kernel is 3.10 LTS.

I got a problem here about handling ICMP packet-too-big message.

Before sending a packet-too-big packet :

The expires should be set by the host _receiving_ the icmpv6 too-big.

Quoted 8 lines
Quoted 4 lines

Can you spell out some details of the outgoing icmpv6 too-big packet? like, the src/dst ip of the icmpv6 and the original ip packet that triggered the too-big.

I don't send too-big packet to trigger the err handling.

I just only send a ICMPv6 packet which type is ICMPV6_PKT_TOOBIG.

You meant : you received an ICMPv6 packet, or you send it ?

#15 lore raw
In reply to Steffen Klassert
Quoted 61 lines · Steffen Klassert
Quoted 29 lines · Yang Yingliang

Hi,

My kernel is 3.10 LTS.

I got a problem here about handling ICMP packet-too-big message.

Before sending a packet-too-big packet :

# ip -6 route list table local
local ::1 dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80::1 dev lo  metric 0 	//It  does not have expire value
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:2 dev lo  metric 0 
local fe80::5054:ff:fe12:3456 dev lo  metric 0

After sending a packet-too-big packet

ip -6 route list table local
local ::1 dev lo metric 0

Quoted 7 lines
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 
local fe80:: dev lo  metric 0 

local fe80::1 dev lo metric 0 expires _597_ //It has expire value

Is this route still present after the expiration time is elapsed?

Quoted 9 lines
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:0 dev lo  metric 0 
local fe80::200:ff:fe00:2 dev lo  metric 0 

local fe80::5054:ff:fe12:3456 dev lo metric 0

When time is up, I can't ping fe80::1 .
Is it ok or a bug ?

This looks pretty similar to a bug I discovered recently. In my case, a ipv6 host route dissapeared 10 minutes after a PMTU event. As a result, this host was not reachable anymore.

This happens because we don't clone host routes before we use them. If a PMTU event happens, the original route is marked with an expire value. After the expiration time is elapsed, the original route is deleted and we loose conectivity to the host.

Quoted 6 lines

I'm currently testing patches to fix this. With these patches the ipv6 host routes are cloned if they are gateway routes, i.e. if PMTU events can happen.

I fear it will not fix your case because PMTU events are not expected to happen at local fe80 routes.

I found current kernel will do ip6_update_pmtu() anyway after receiving an ICMPV6_PKT_TOOBIG type packet in icmpv6_err().

Does it need some more condition ? Like this:

net/ipv6/icmp.c +3 -1
index a5e95199585e..7c0a28add109 100644
@@ -89,8 +89,10 @@ static void icmpv6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	/* icmpv6_notify checks 8 bytes can be pulled, icmp6hdr is 8 bytes */
 	struct icmp6hdr *icmp6 = (struct icmp6hdr *) (skb->data + offset);
 	struct net *net = dev_net(skb->dev);
+	const struct ipv6hdr *iph = (struct ipv6hdr *) skb->data;
 
-	if (type == ICMPV6_PKT_TOOBIG)
+	if (type == ICMPV6_PKT_TOOBIG &&
+	    !(ipv6_addr_type(&iph->daddr) & IPV6_ADDR_LINKLOCAL))
 		ip6_update_pmtu(skb, net, info, 0, 0);
 	else if (type == NDISC_REDIRECT)
 		ip6_redirect(skb, net, skb->dev->ifindex, 0);

Regards,
Yang

#9 lore raw

Re: [PATCH RFC 1/2] ipv6: Fix after pmtu events dissapearing host routes

In reply to Steffen Klassert
Quoted 19 lines · Steffen Klassert

We currently don't clone host routes before we use them. If a pmtu event is received on such a route, it gets an expires value. As soon as the expiration time is elapsed, the route is deleted. As a result, the host is not reachable any more.

We fix this by cloning host routes if they are gatewayed, i.e. if pmtu events can happen.

---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv6/route.c +0 -0
index c910831..3e864e7 100644

net/ipv6/route.c redo_rt6_select: · line 967

Quoted 8 lines
net/ipv6/route.c +1 -1
@@ -961,7 +961,7 @@ redo_rt6_select:
 
 	if (!(rt->rt6i_flags & (RTF_NONEXTHOP | RTF_GATEWAY)))
 		nrt = rt6_alloc_cow(rt, &fl6->daddr, &fl6->saddr);
-	else if (!(rt->dst.flags & DST_HOST))
+	else if (!(rt->dst.flags & DST_HOST) || (rt->rt6i_flags & RTF_GATEWAY))
 		nrt = rt6_alloc_clone(rt, &fl6->daddr);
 	else
 		goto out2;

My approach was to suppress mtu updates on the loopback interface. Hmm...

Bye,
Hannes

#10 lore raw

Re: [PATCH RFC 1/2] ipv6: Fix after pmtu events dissapearing host routes

Quoted 21 lines · Hannes Frederic Sowa
Quoted 20 lines · Steffen Klassert

We currently don't clone host routes before we use them. If a pmtu event is received on such a route, it gets an expires value. As soon as the expiration time is elapsed, the route is deleted. As a result, the host is not reachable any more.

We fix this by cloning host routes if they are gatewayed, i.e. if pmtu events can happen.

---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv6/route.c +0 -0
index c910831..3e864e7 100644
@@ -961,7 +961,7 @@ redo_rt6_select:
 
Quoted 10 lines

net/ipv6/route.c redo_rt6_select: · line 967

Quoted 7 lines
net/ipv6/route.c +1 -1
 	if (!(rt->rt6i_flags & (RTF_NONEXTHOP | RTF_GATEWAY)))
 		nrt = rt6_alloc_cow(rt, &fl6->daddr, &fl6->saddr);
-	else if (!(rt->dst.flags & DST_HOST))
+	else if (!(rt->dst.flags & DST_HOST) || (rt->rt6i_flags & RTF_GATEWAY))
 		nrt = rt6_alloc_clone(rt, &fl6->daddr);
 	else
 		goto out2;

My approach was to suppress mtu updates on the loopback interface. Hmm...

Maybe we need to do this too. My patch fixes just the case where we get a valid pmtu update from a remote host.

#11 lore raw

Re: [PATCH RFC 1/2] ipv6: Fix after pmtu events dissapearing host routes

In reply to Steffen Klassert
Quoted 19 lines · Steffen Klassert

We currently don't clone host routes before we use them. If a pmtu event is received on such a route, it gets an expires value. As soon as the expiration time is elapsed, the route is deleted. As a result, the host is not reachable any more.

We fix this by cloning host routes if they are gatewayed, i.e. if pmtu events can happen.

---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv6/route.c +0 -0
index c910831..3e864e7 100644

net/ipv6/route.c redo_rt6_select: · line 965

Quoted 6 lines
net/ipv6/route.c +1 -1
@@ -961,7 +961,7 @@ redo_rt6_select:
 
 	if (!(rt->rt6i_flags & (RTF_NONEXTHOP | RTF_GATEWAY)))
 		nrt = rt6_alloc_cow(rt, &fl6->daddr, &fl6->saddr);
-	else if (!(rt->dst.flags & DST_HOST))
+	else if (!(rt->dst.flags & DST_HOST) || (rt->rt6i_flags & RTF_GATEWAY))
 		nrt = rt6_alloc_clone(rt, &fl6->daddr);

The del path may also require changes. I am thinking:
1. Create a /128 via gateway route
2. Send some traffic and RTF_CACHE rt is created
3. Delete the /128 route by ip route del. I suspect the RTF_CACHE route may be deleted and the route added in (1) stays.

Thanks

#12 lore raw

Re: [PATCH RFC 1/2] ipv6: Fix after pmtu events dissapearing host routes

Quoted 21 lines · Martin Lau
Quoted 20 lines · Steffen Klassert

We currently don't clone host routes before we use them. If a pmtu event is received on such a route, it gets an expires value. As soon as the expiration time is elapsed, the route is deleted. As a result, the host is not reachable any more.

We fix this by cloning host routes if they are gatewayed, i.e. if pmtu events can happen.

---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv6/route.c +0 -0
index c910831..3e864e7 100644
@@ -961,7 +961,7 @@ redo_rt6_select:
 
Quoted 10 lines

net/ipv6/route.c redo_rt6_select: · line 965

Quoted 5 lines
net/ipv6/route.c +1 -1
 	if (!(rt->rt6i_flags & (RTF_NONEXTHOP | RTF_GATEWAY)))
 		nrt = rt6_alloc_cow(rt, &fl6->daddr, &fl6->saddr);
-	else if (!(rt->dst.flags & DST_HOST))
+	else if (!(rt->dst.flags & DST_HOST) || (rt->rt6i_flags & RTF_GATEWAY))
 		nrt = rt6_alloc_clone(rt, &fl6->daddr);

The del path may also require changes. I am thinking:
1. Create a /128 via gateway route
2. Send some traffic and RTF_CACHE rt is created
3. Delete the /128 route by ip route del. I suspect the RTF_CACHE route may be deleted and the route added in (1) stays.

Good point.

Both routes are on the same fib node. The cached one has the better metric, so I guess this one will be found and deleted.

I'll check this. Thanks for the hint!

#13 lore raw

Re: [PATCH RFC 1/2] ipv6: Fix after pmtu events dissapearing host routes

Quoted 33 lines · Steffen Klassert
Quoted 21 lines · Martin Lau
Quoted 20 lines · Steffen Klassert

We currently don't clone host routes before we use them. If a pmtu event is received on such a route, it gets an expires value. As soon as the expiration time is elapsed, the route is deleted. As a result, the host is not reachable any more.

We fix this by cloning host routes if they are gatewayed, i.e. if pmtu events can happen.

---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv6/route.c +0 -0
index c910831..3e864e7 100644
@@ -961,7 +961,7 @@ redo_rt6_select:
 
Quoted 10 lines

net/ipv6/route.c redo_rt6_select: · line 965

Quoted 5 lines
net/ipv6/route.c +1 -1
 	if (!(rt->rt6i_flags & (RTF_NONEXTHOP | RTF_GATEWAY)))
 		nrt = rt6_alloc_cow(rt, &fl6->daddr, &fl6->saddr);
-	else if (!(rt->dst.flags & DST_HOST))
+	else if (!(rt->dst.flags & DST_HOST) || (rt->rt6i_flags & RTF_GATEWAY))
 		nrt = rt6_alloc_clone(rt, &fl6->daddr);

The del path may also require changes. I am thinking:
1. Create a /128 via gateway route
2. Send some traffic and RTF_CACHE rt is created
3. Delete the /128 route by ip route del. I suspect the RTF_CACHE route may be deleted and the route added in (1) stays.

Quoted 6 lines

Good point.

Both routes are on the same fib node. The cached one has the better metric, so I guess this one will be found and deleted.

I'll check this. Thanks for the hint!

I finally got around to test this. It is as you say, we delete the clone but not the original route.

I was able to fix this by adding the following:

---
 net/ipv6/route.c | 2 ++
 1 file changed, 2 insertions(+)
net/ipv6/route.c +2 -0
index c20186f..e4f6a56 100644
@@ -1814,6 +1814,8 @@ static int ip6_route_del(struct fib6_config *cfg)
 				continue;
 			if (cfg->fc_metric && cfg->fc_metric != rt->rt6i_metric)
 				continue;
+			if (rt->rt6i_flags & RTF_CACHE)
+				continue;
 			dst_hold(&rt->dst);
 			read_unlock_bh(&table->tb6_lock);
 

It should be ok to ignore cached routes here because they are removed with the original route. With this patch applied both routes, the cached and the original go away when deleting.

I'll send an updated version of my patchset if this survives some advanced testing.