Structure icmp6hdr (IPv6/ICMPv6) with bug in the bitfields!

First, let-me say sorry for the long text, i think is needed for context.

My name is Pedro Ribeiro and I’m the operations coordinator at IPLNet, a network that interconnects all the schools belonging to the Polytechnic Institute of Lisbon (IPL) and in addition I’m also a teacher of network engineering at ISEL (a local university).

In the majority of the network segments of our infrastructure, we have dual stack connectivity with IPv4/IPv6. In the last months, we have upgraded most of the infrastructure, specially targeting IPv6 performance leaving most of the segments with an “old/slow router” and a “new/fast” router as possible default gateways.
In the course of validating the changes, we have done some tests with IPv6 and some of them were really disappointing with no gains at all.

While investigating the cause of that, I have seen that our main Linux servers were using the “wrong” default gateway, the “old/slow” one. This behaviour is inconsistent with the configuration of the routers; the “new/fast” is sending the router advertisements announcing himself as default gateway with MEDIUM (default) preference and the “old/slow” announcing himself as LOW preference.

Deep analysis in the sources of Linux revealed that the value of “pref” passed to rt6_add_dflt_router(...) in the file net/ipv6/route.c isn’t consistent with the one sent by the routers and observed with wireshark. Seeking the roots of the problem I’ve detected a bug in the definitions of the bitfield that includes the router preference in the router advertisement message that are resulting in retrieving the wrong bits from the structure defined in “include/linux/icmpv6.h”. The struct is the base one from ICMPv6 (icmp6hdr) and was lacking the bit field “home_agent” between “router_pref” and “other” and the reserved bits are only 3, not 4 as in the structure (according to RFC4191)

Follows a “diff” with the changes I’ve made to correct this problem (I’ve made it against kernel 2.6.23, but I’ve confirmed that the problem still exists in 2.6.25)

@@ -40,16 +40,18 @@
                 struct icmpv6_nd_ra {
                        __u8            hop_limit;
 #if defined(__LITTLE_ENDIAN_BITFIELD)
-                       __u8            reserved:4,
+                       __u8            reserved:3,
                                        router_pref:2,
+                                       home_agent:1,
                                        other:1,
                                        managed:1;
 
 #elif defined(__BIG_ENDIAN_BITFIELD)
                        __u8            managed:1,
                                        other:1,
+                                       home_agent:1,
                                        router_pref:2,
-                                       reserved:4;
+                                       reserved:3;
 #else
 #error "Please fix <asm/byteorder.h>"
 #endif

Even after this fix, the problem of deterministic and preference aware default route selection, remains. I have not figured until now how the preference affects the selection, maybe it is a missing feature.

I am sorry to say it (I am not a Windows fan), but the behaviour observed in Windows Vista is in this matter consistent with the one I was expecting from the reading of the RFCs.

Developers on this area please clarify me this subject; does the preference present in the RAs is supposed to influence the decision? In addition, what is the criterion for selecting the default route in the presence of multiple candidates with the same preference? Best Regards to all the Linux community!

Best regards,

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Pedro Ribeiro
IPLNet - Rede de dados e comunicações
Instituto Politécnico de Lisboa (IPL)
Mail: mailto:pribeiro AT net.ipl.pt
VoIP: sip:pribeiro AT net.ipl.pt
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Hi,

+1

can you confirm that you already activated CONFIG_IPV6_ROUTER_PREF in your kernel configuration?

What are the values of following parameters on your system?:

/proc/sys/net/ipv6/conf/*/accept_ra_rtr_pref
/proc/sys/net/ipv6/conf/*/accept_ra_pinfo
/proc/sys/net/ipv6/conf/*/accept_ra
/proc/sys/net/ipv6/conf/*/autoconf

Looking at the code in ndisc.c (ndisc_router_discovery()), it should, but I may have missed something.

Don't know.

Funny it was not detected sooner. In patches I have for UMIP (userland MIPv6 Daemon for Linux), access to router preferences field is done by shifting bits (correctly, AFAICT) and routes are set from userspace using that info. radvd also fills its RA that way, i.e. by shifting bits (it uses struct nd_router_advert from netinet/icmp6.h, which does not have anything for router preference). That would explain why I never hit the bug.

Cheers,

a+

ps: added YOSHIFUJI Hideaki in CC.

Hello Arnaud,

Thanks for your quick answer ...

Here goes the information solicited.

TIA.

Tuesday, October 14, 2008, 1:58:41 PM, you wrote:

...

host ~ # gzip -d -c /proc/config.gz | grep -F CONFIG_IPV6_ROUTER_PREF CONFIG_IPV6_ROUTER_PREF=y

All of them have value "1"

/proc/sys/net/ipv6/conf/all/accept_ra:1 /proc/sys/net/ipv6/conf/all/accept_ra_defrtr:1 /proc/sys/net/ipv6/conf/all/accept_ra_pinfo:1 /proc/sys/net/ipv6/conf/all/accept_ra_rtr_pref:1 /proc/sys/net/ipv6/conf/default/accept_ra:1 /proc/sys/net/ipv6/conf/default/accept_ra_defrtr:1 /proc/sys/net/ipv6/conf/default/accept_ra_pinfo:1 /proc/sys/net/ipv6/conf/default/accept_ra_rtr_pref:1 /proc/sys/net/ipv6/conf/eth0/accept_ra:1 /proc/sys/net/ipv6/conf/eth0/accept_ra_defrtr:1 /proc/sys/net/ipv6/conf/eth0/accept_ra_pinfo:1 /proc/sys/net/ipv6/conf/eth0/accept_ra_rtr_pref:1 /proc/sys/net/ipv6/conf/lo/accept_ra:1 /proc/sys/net/ipv6/conf/lo/accept_ra_defrtr:1 /proc/sys/net/ipv6/conf/lo/accept_ra_pinfo:1 /proc/sys/net/ipv6/conf/lo/accept_ra_rtr_pref:1 /proc/sys/net/ipv6/conf/all/autoconf:1 /proc/sys/net/ipv6/conf/default/autoconf:1 /proc/sys/net/ipv6/conf/eth0/autoconf:1 /proc/sys/net/ipv6/conf/lo/autoconf:1

Best regards.

Best regards,
 Pedro                            mailto:[email protected]

Hello Arnaud and all,

I’ve found the other part of the problem. ndisc_router_discovery () at ndisc.c weren’t correctly updating the flags in the route table, the problem is now fixed with the correct default router being updated as soon as the next RA with the new preference is received.

Follows the two patches made against 2.6.23 (this one and the first I’ve sent) but I’ve seen in 2.6.26 the problem also exists and the fix should apply without problems.

Can any of the maintainers give-me some expectations of when (and if) this fix will be added to a kernel release?

Should I patch all of our ~60 Linux machines or wait for a general upgrade with a new kernel soon?

Thanks for all the support in this subject.

Best regards.

#######################################################################

@@ -1101,7 +1101,7 @@
                }
                neigh->flags |= NTF_ROUTER;
        } else if (rt) {
-               rt->rt6i_flags |= (rt->rt6i_flags & ~RTF_PREF_MASK) | RTF_PREF(pref);
+               rt->rt6i_flags = (rt->rt6i_flags & ~RTF_PREF_MASK) | RTF_PREF(pref);
        }
 
        if (rt)

(this part is a "repost" just to keep both patches together)

@@ -40,16 +40,18 @@
                 struct icmpv6_nd_ra {
                        __u8            hop_limit;
 #if defined(__LITTLE_ENDIAN_BITFIELD)
-                       __u8            reserved:4,
+                       __u8            reserved:3,
                                        router_pref:2,
+                                       home_agent:1,
                                        other:1,
                                        managed:1;
 
 #elif defined(__BIG_ENDIAN_BITFIELD)
                        __u8            managed:1,
                                        other:1,
+                                       home_agent:1,
                                        router_pref:2,
-                                       reserved:4;
+                                       reserved:3;
 #else
 #error "Please fix <asm/byteorder.h>"
 

Best regards,

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Pedro Ribeiro
IPLNet - Rede de dados e comunicações
Instituto Politécnico de Lisboa (IPL)
Mail: mailto:[email protected]
VoIP: sip:[email protected]
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

From: Pedro Ribeiro <[email protected]>
Date: Tue, 14 Oct 2008 21:07:32 +0100

Well, first of all, your email client corrupted the patch (turning tab characters into spaces etc.) so that is the first thing you need to fix before we'll even look at this.

From: Pedro Ribeiro <[email protected]>
Date: Tue, 14 Oct 2008 12:06:20 +0100

Please make your patch against current sources, it does not apply to the current tree.

Hello David,

Thanks for your time on this, follows a new patch made against 2.6.27, this time I minimized the cut&paste to avoid corrupting the patch.

diff -wurNbB linux-2.6.27/include/linux/icmpv6.h linux-2.6.27fix/include/linux/icmpv6.h
@@ -40,16 +40,18 @@
                 struct icmpv6_nd_ra {
                        __u8            hop_limit;
 #if defined(__LITTLE_ENDIAN_BITFIELD)
-                       __u8            reserved:4,
+                       __u8            reserved:3,
                                        router_pref:2,
+                                       home_agent:1,
                                        other:1,
                                        managed:1;
 
 #elif defined(__BIG_ENDIAN_BITFIELD)
                        __u8            managed:1,
                                        other:1,
+                                       home_agent:1,
                                        router_pref:2,
-                                       reserved:4;
+                                       reserved:3;
 #else
 #error "Please fix <asm/byteorder.h>"
 #endif

diff -wurNbB linux-2.6.27/net/ipv6/ndisc.c linux-2.6.27fix/net/ipv6/ndisc.c
@@ -1199,7 +1199,7 @@
                }
                neigh->flags |= NTF_ROUTER;
        } else if (rt) {
-               rt->rt6i_flags |= (rt->rt6i_flags & ~RTF_PREF_MASK) | RTF_PREF(pref);
+               rt->rt6i_flags = (rt->rt6i_flags & ~RTF_PREF_MASK) | RTF_PREF(pref);
        }
 
        if (rt)

Best regards.

Best regards,

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Pedro Ribeiro
IPLNet - Rede de dados e comunicações
Instituto Politécnico de Lisboa (IPL)
Mail: mailto:[email protected]
VoIP: sip:[email protected]
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

From: Pedro Ribeiro <[email protected]>
Date: Tue, 14 Oct 2008 23:38:11 +0100

It's still corrupted, the tab characters in the patch were transformed into spaces.

It's not about cut&paste, your email client is reformatting the content (no matter how you compose it) before sending it out.

You need to configure your email client explicitly to not reformat the content and to use plain ASCII text.

Also, you need to provide a proper "Signed-off-by: " line.

All of this is detailed in the following kernel source doc files:

linux/Documentation/email-clients.txt
linux/Documentation/SubmittingPatches

Thanks.

Problem observed:
In IPv6, in the presence of multiple routers candidates to default gateway in one segment, each sending a different value of preference, the Linux hosts connected to the segment weren't selecting the right one in all the combinations possible of LOW/MEDIUM/HIGH preference.

This patch changes two files:
include/linux/icmpv6.h
Get the "router_pref" bitfield in the right place (as RFC4191 says), named the bit left with this fix as "home_agent" (RFC3775 say that's his function)

net/ipv6/ndisc.c
Corrects the binary logic behind the updating of the router preference in the flags of the routing table

Result:
With this two fixes applied, the default route used by the system was to consistent with the rules mentioned in RFC4191 in case of changes in the value of preference in router advertisements

diff -uprN linux-2.6.27/include/linux/icmpv6.h linux-2.6.27fix/include/linux/icmpv6.h
@@ -40,16 +40,18 @@ struct icmp6hdr {
                 struct icmpv6_nd_ra {
 			__u8		hop_limit;
 #if defined(__LITTLE_ENDIAN_BITFIELD)
-			__u8		reserved:4,
+			__u8		reserved:3,
 					router_pref:2,
+					home_agent:1,
 					other:1,
 					managed:1;
 
 #elif defined(__BIG_ENDIAN_BITFIELD)
 			__u8		managed:1,
 					other:1,
+					home_agent:1,
 					router_pref:2,
-					reserved:4;
+					reserved:3;
 #else
 #error	"Please fix <asm/byteorder.h>"
 #endif

diff -uprN linux-2.6.27/net/ipv6/ndisc.c linux-2.6.27fix/net/ipv6/ndisc.c
@@ -1199,7 +1199,7 @@ static void ndisc_router_discovery(struc
 		}
 		neigh->flags |= NTF_ROUTER;
 	} else if (rt) {
-		rt->rt6i_flags |= (rt->rt6i_flags & ~RTF_PREF_MASK) | RTF_PREF(pref);
+		rt->rt6i_flags = (rt->rt6i_flags & ~RTF_PREF_MASK) | RTF_PREF(pref);
 	}
 
 	if (rt)

Best regards,

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Pedro Ribeiro
IPLNet - Rede de dados e comunicações
Instituto Politécnico de Lisboa (IPL)
Mail: mailto:pribeiro AT net.ipl.pt
VoIP: sip:pribeiro AT net.ipl.pt
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

In article <[email protected]> (at Wed, 15 Oct 2008 16:55:49 +0100), Pedro Ribeiro <[email protected]> says:

--yoshfuji

From: YOSHIFUJI Hideaki / 吉藤英明 <[email protected]>
Date: Thu, 16 Oct 2008 01:11:43 +0900 (JST)

Applied, thanks everyone.

Problem observed:
In IPv6, in the presence of multiple routers candidates to default gateway in one segment, each sending a different value of preference, the Linux hosts connected to the segment weren't selecting the right one in all the combinations possible of LOW/MEDIUM/HIGH preference.

This patch changes two files:
include/linux/icmpv6.h
Get the "router_pref" bitfield in the right place (as RFC4191 says), named the bit left with this fix as "home_agent" (RFC3775 say that's his function)

net/ipv6/ndisc.c
Corrects the binary logic behind the updating of the router preference in the flags of the routing table

Result:
With this two fixes applied, the default route used by the system was to consistent with the rules mentioned in RFC4191 in case of changes in the value of preference in router advertisements

diff -uprN linux-2.6.27/include/linux/icmpv6.h linux-2.6.27fix/include/linux/icmpv6.h
@@ -40,16 +40,18 @@ struct icmp6hdr {
                 struct icmpv6_nd_ra {
 			__u8		hop_limit;
 #if defined(__LITTLE_ENDIAN_BITFIELD)
-			__u8		reserved:4,
+			__u8		reserved:3,
 					router_pref:2,
+					home_agent:1,
 					other:1,
 					managed:1;
 
 #elif defined(__BIG_ENDIAN_BITFIELD)
 			__u8		managed:1,
 					other:1,
+					home_agent:1,
 					router_pref:2,
-					reserved:4;
+					reserved:3;
 #else
 #error	"Please fix <asm/byteorder.h>"
 #endif

diff -uprN linux-2.6.27/net/ipv6/ndisc.c linux-2.6.27fix/net/ipv6/ndisc.c
@@ -1199,7 +1199,7 @@ static void ndisc_router_discovery(struc
 		}
 		neigh->flags |= NTF_ROUTER;
 	} else if (rt) {
-		rt->rt6i_flags |= (rt->rt6i_flags & ~RTF_PREF_MASK) | RTF_PREF(pref);
+		rt->rt6i_flags = (rt->rt6i_flags & ~RTF_PREF_MASK) | RTF_PREF(pref);
 	}
 
 	if (rt)