Our IPv6CP implementation has never handled IFID collisions. The old
code (from before r1.112 of if_spppsubr.c) punted and left the address
unchanged. The new code (as of r1.112) is supposed to handle collisions,
but instead goes into an endless conf-nak loop with the peer.

The problem is that I made wrong assumptions about in6_update_ifa().
It doesn't change existing addresses. It only updates parameters of
existing addresses (such as the destination address on the p2p link).
IPv6CP currently keeps suggesting the existing address which collides
with the peer's address.

Additionally, we need to update several routing table entries when
changing the link-local address (e.g. pinging the peer doesn't work
if we don't do this).

With the diff below we change our link-local address if it collides
with the address used by the peer.

In this logged session, I start out using the link-local address
used by the peer (fe80::218:82ff:fe22:a563) and switch over to
an address suggested by the peer.

Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp open(initial)
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp initial->starting
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp up(starting)
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp starting->req-sent
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp input(req-sent): <conf-req id=0x1 
len=14 
01-0a-02-18-82-ff-fe-22-a5-63-28-29-2a-2b-2c-2d-2e-2f-30-31-32-33-34-35-36-37-00-00-00-00-00-00-00-00>
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp parse opts: ifid
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp parse opt values:  ifid 
fe80::218:82ff:fe22:a563 [conf-nak] send conf-nak suggest 
fe80::18:82ff:fe22:722f
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp input(req-sent): <conf-nak id=0x19 
len=14 
01-0a-02-0a-e4-65-99-3e-f1-4e-9f-c0-00-03-00-7f-00-1a-3f-00-00-00-03-10-00-00-00-00-00-00-00-00-00-00>
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp nak opts: ifid  [suggestaddr 
fe80::20a:e465:993e:f14e] [agree]
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp input(req-sent): <conf-req id=0x2 
len=14 
01-0a-02-18-82-ff-fe-22-a5-63-dd-de-df-e0-e1-e2-e3-e4-e5-e6-e7-e8-e9-ea-eb-ec-00-00-00-00-00-00-00-00>
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp parse opts: ifid
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp parse opt values:  ifid 
fe80::218:82ff:fe22:a563 [conf-ack] send conf-ack
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp req-sent->ack-sent
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp input(ack-sent): <conf-ack id=0x1a 
len=14 
01-0a-02-0a-e4-65-99-3e-f1-4e-00-04-00-01-ff-ff-ff-ff-ff-ff-ff-ff-80-00-00-1f-00-00-00-00-00-00-00-00>
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp ack-sent->opened
Dec 17 16:01:38 jack /bsd: pppoe0: ipv6cp tlu

This shows the corresponding routing table changes: 

 Destination                        Gateway                        Flags   Refs 
     Use   Mtu  Prio Iface
 ::/104                             ::1                            UGRS       0 
       0     -     8 lo0  
 ::/96                              ::1                            UGRS       0 
       0     -     8 lo0  
+default                            fe80::218:82ff:fe22:a563%pppoe0 UG         
0        0     -    56 pppoe0
 ::1                                ::1                            UH        14 
       4 33192     4 lo0  
 ::127.0.0.0/104                    ::1                            UGRS       0 
       0     -     8 lo0  
 ::224.0.0.0/100                    ::1                            UGRS       0 
       0     -     8 lo0  
 ::255.0.0.0/104                    ::1                            UGRS       0 
       0     -     8 lo0  
 ::ffff:0.0.0.0/96                  ::1                            UGRS       0 
       0     -     8 lo0  
 2002::/24                          ::1                            UGRS       0 
       0     -     8 lo0  
 2002:7f00::/24                     ::1                            UGRS       0 
       0     -     8 lo0  
 2002:e000::/20                     ::1                            UGRS       0 
       0     -     8 lo0  
 2002:ff00::/24                     ::1                            UGRS       0 
       0     -     8 lo0  
-fe80::/10                          ::1                            UGRS       0 
       0     -     8 lo0  
+fe80::/10                          ::1                            UGRS       2 
       0     -     8 lo0  
 fe80::%em0/64                      link#1                         UC         0 
       0     -     4 em0  
 fe80::20a:e4ff:fe3e:f14e%em0       00:0a:e4:3e:f1:4e              HL         0 
       0     -     4 lo0  
 fe80::%lo0/64                      fe80::1%lo0                    U          0 
       0     -     4 lo0  
 fe80::1%lo0                        link#4                         UHL        0 
       0     -     4 lo0  
-fe80::%pppoe0/64                   fe80::218:82ff:fe22:a563%pppoe0            
0        0     -     4 pppoe0
-fe80::218:82ff:fe22:a563%pppoe0    link#6                         HL         0 
       0     -     4 lo0  
+fe80::%pppoe0/64                   fe80::20a:e468:123e:f14e%pppoe0 U          
1        0     -     4 pppoe0
+fe80::20a:e468:123e:f14e%pppoe0    link#6                         UHL        0 
       0     -     4 lo0  
+fe80::218:82ff:fe22:a563%pppoe0    link#6                         UHL        0 
       0     -     4 pppoe0
 fec0::/10                          ::1                            UGRS       0 
       0     -     8 lo0  
 ff01::/16                          ::1                            UGRS       1 
       0     -     8 lo0  
 ff01::%em0/32                      link#1                         UC         0 
       0     -     4 em0  
 ff01::%lo0/32                      fe80::1%lo0                    UC         0 
       0     -     4 lo0  
-ff01::%pppoe0/32                   fe80::218:82ff:fe22:a563%pppoe0 C          
0        0     -     4 pppoe0
+ff01::%pppoe0/32                   fe80::20a:e468:123e:f14e%pppoe0 UC         
0        0     -     4 pppoe0
 ff02::/16                          ::1                            UGRS       1 
       0     -     8 lo0  
 ff02::%em0/32                      link#1                         UC         0 
       0     -     4 em0  
 ff02::%lo0/32                      fe80::1%lo0                    UC         0 
       0     -     4 lo0  
-ff02::%pppoe0/32                   fe80::218:82ff:fe22:a563%pppoe0 C          
0        0     -     4 pppoe0
+ff02::%pppoe0/32                   fe80::20a:e468:123e:f14e%pppoe0 UC         
0        0     -     4 pppoe0

To make the netinet6 code use the IFID determined by IPv6CP, I need to
have in6_ifattach_linklocal() accept a provided IFID. I decided to replace
the altifp parameter, which wasn't used anywhere, with a new ifid parameter
that has the same purpose but suits the new sppp caller better (it's an
address, not an ifp).

Other bug fixes contained in here are:

 - Make IPv6CP always use the latest suggested address,
   (ipv6cp.req_ifid.ifra_addr.sin6_addr), so we use it even if the task
   that configures the new address on the interface didn't run yet.

 - Clear the ifindex (KAME hack) in addresses suggested for IPv6CP.
   An ifindex in the suggested address might confuse the peer.

 - Make in6_ifdetach() remove route to interface local allnodes
   (also here: http://marc.info/?l=openbsd-tech&m=138729124402857&w=2)
   This makes the above routing table changes work properly.

I'm considering committing the above fixes separately.

Ok?

Index: net/if_spppsubr.c
===================================================================
RCS file: /cvs/src/sys/net/if_spppsubr.c,v
retrieving revision 1.113
diff -u -p -r1.113 if_spppsubr.c
--- net/if_spppsubr.c   20 Nov 2013 08:21:33 -0000      1.113
+++ net/if_spppsubr.c   17 Dec 2013 14:56:06 -0000
@@ -69,6 +69,10 @@
 #include <netinet/if_ether.h>
 #endif
 
+#ifdef INET6
+#include <netinet6/in6_ifattach.h>
+#endif
+
 #include <net/if_sppp.h>
 
 # define UNTIMEOUT(fun, arg, handle)   \
@@ -3222,7 +3226,10 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
                addlog("\n");
 
        /* pass 2: parse option values */
-       sppp_get_ip6_addrs(sp, &myaddr, NULL, NULL);
+       if (sp->ipv6cp.flags & IPV6CP_MYIFID_DYN)
+               myaddr = sp->ipv6cp.req_ifid.ifra_addr.sin6_addr;
+       else
+               sppp_get_ip6_addrs(sp, &myaddr, NULL, NULL);
        if (debug)
                log(LOG_DEBUG, "%s: ipv6cp parse opt values: ",
                       SPP_ARGS(ifp));
@@ -3455,7 +3462,10 @@ sppp_ipv6cp_scr(struct sppp *sp)
        int i = 0;
 
        if (sp->ipv6cp.opts & (1 << IPV6CP_OPT_IFID)) {
-               sppp_get_ip6_addrs(sp, &ouraddr, NULL, NULL);
+               if (sp->ipv6cp.flags & IPV6CP_MYIFID_DYN)
+                       ouraddr = sp->ipv6cp.req_ifid.ifra_addr.sin6_addr;
+               else
+                       sppp_get_ip6_addrs(sp, &ouraddr, NULL, NULL);
                opt[i++] = IPV6CP_OPT_IFID;
                opt[i++] = 10;
                bcopy(&ouraddr.s6_addr[8], &opt[i], 8);
@@ -4768,6 +4778,25 @@ sppp_update_ip6_addr(void *arg1, void *a
                return;
        }
 
+       /* 
+        * Changing the link-local address requires purging all
+        * existing addresses and routes for the interface first.
+        */
+       if (sp->ipv6cp.flags & IPV6CP_MYIFID_DYN) {
+               in6_ifdetach(ifp);
+               error = in6_ifattach_linklocal(ifp, &ifra->ifra_addr.sin6_addr);
+               if (error)
+                       log(LOG_ERR, SPP_FMT
+                           "could not update IPv6 address (error %d)\n",
+                           SPP_ARGS(ifp), error);
+               splx(s);
+               return;
+       }
+
+       /* 
+        * Code below changes address parameters only, not the address itself.
+        */
+
        /* Destination address can only be set for /128. */
        if (!in6_are_prefix_equal(&ia->ia_prefixmask.sin6_addr, &mask, 128)) {
                ifra->ifra_dstaddr.sin6_len = 0;
@@ -4843,6 +4872,7 @@ sppp_suggest_ip6_addr(struct sppp *sp, s
                myaddr.s6_addr[14] ^= (random & 0xff);
                myaddr.s6_addr[15] ^= ((random & 0xff00) >> 8);
        }
+       myaddr.s6_addr16[1] = 0; /* KAME hack: clear ifindex */
        bcopy(&myaddr, suggest, sizeof(myaddr));
 }
 #endif /*INET6*/
Index: netinet6/in6.c
===================================================================
RCS file: /cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.126
diff -u -p -r1.126 in6.c
--- netinet6/in6.c      28 Nov 2013 10:16:44 -0000      1.126
+++ netinet6/in6.c      17 Dec 2013 14:24:05 -0000
@@ -2220,7 +2220,7 @@ in6_if_up(struct ifnet *ifp)
        /*
         * special cases, like 6to4, are handled in in6_ifattach
         */
-       in6_ifattach(ifp, NULL);
+       in6_ifattach(ifp);
 
        dad_delay = 0;
        TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
Index: netinet6/in6_ifattach.c
===================================================================
RCS file: /cvs/src/sys/netinet6/in6_ifattach.c,v
retrieving revision 1.64
diff -u -p -r1.64 in6_ifattach.c
--- netinet6/in6_ifattach.c     28 Nov 2013 10:16:44 -0000      1.64
+++ netinet6/in6_ifattach.c     17 Dec 2013 14:27:28 -0000
@@ -67,7 +67,7 @@ int ip6_auto_linklocal = 1;   /* enable by
 
 int get_last_resort_ifid(struct ifnet *, struct in6_addr *);
 int get_hw_ifid(struct ifnet *, struct in6_addr *);
-int get_ifid(struct ifnet *, struct ifnet *, struct in6_addr *);
+int get_ifid(struct ifnet *, struct in6_addr *);
 int in6_ifattach_loopback(struct ifnet *);
 
 #define EUI64_GBIT     0x01
@@ -257,11 +257,9 @@ found:
  * Get interface identifier for the specified interface.  If it is not
  * available on ifp0, borrow interface identifier from other information
  * sources.
- *
- * altifp - secondary EUI64 source
  */
 int
-get_ifid(struct ifnet *ifp0, struct ifnet *altifp, struct in6_addr *in6)
+get_ifid(struct ifnet *ifp0, struct in6_addr *in6)
 {
        struct ifnet *ifp;
 
@@ -272,13 +270,6 @@ get_ifid(struct ifnet *ifp0, struct ifne
                goto success;
        }
 
-       /* try secondary EUI64 source. this basically is for ATM PVC */
-       if (altifp && get_hw_ifid(altifp, in6) == 0) {
-               nd6log((LOG_DEBUG, "%s: got interface identifier from %s\n",
-                   ifp0->if_xname, altifp->if_xname));
-               goto success;
-       }
-
        /* next, try to get it from some other hardware interface */
        TAILQ_FOREACH(ifp, &ifnet, if_list) {
                if (ifp == ifp0)
@@ -318,11 +309,11 @@ success:
 }
 
 /*
- * altifp - secondary EUI64 source
+ * ifid - used as EUI64 if not NULL, overrides other EUI64 sources
  */
 
 int
-in6_ifattach_linklocal(struct ifnet *ifp, struct ifnet *altifp)
+in6_ifattach_linklocal(struct ifnet *ifp, struct in6_addr *ifid)
 {
        struct in6_ifaddr *ia;
        struct in6_aliasreq ifra;
@@ -348,8 +339,15 @@ in6_ifattach_linklocal(struct ifnet *ifp
        if ((ifp->if_flags & IFF_LOOPBACK) != 0) {
                ifra.ifra_addr.sin6_addr.s6_addr32[2] = 0;
                ifra.ifra_addr.sin6_addr.s6_addr32[3] = htonl(1);
+       } else if (ifid) {
+               ifra.ifra_addr.sin6_addr = *ifid;
+               ifra.ifra_addr.sin6_addr.s6_addr16[0] = htons(0xfe80);
+               ifra.ifra_addr.sin6_addr.s6_addr16[1] = htons(ifp->if_index);
+               ifra.ifra_addr.sin6_addr.s6_addr32[1] = 0;
+               ifra.ifra_addr.sin6_addr.s6_addr[8] &= ~EUI64_GBIT;
+               ifra.ifra_addr.sin6_addr.s6_addr[8] |= EUI64_UBIT;
        } else {
-               if (get_ifid(ifp, altifp, &ifra.ifra_addr.sin6_addr) != 0) {
+               if (get_ifid(ifp, &ifra.ifra_addr.sin6_addr) != 0) {
                        nd6log((LOG_ERR,
                            "%s: no ifid available\n", ifp->if_xname));
                        return (-1);
@@ -565,11 +563,9 @@ in6_nigroup(struct ifnet *ifp, const cha
  * XXX multiple loopback interface needs more care.  for instance,
  * nodelocal address needs to be configured onto only one of them.
  * XXX multiple link-local address case
- *
- * altifp - secondary EUI64 source
  */
 void
-in6_ifattach(struct ifnet *ifp, struct ifnet *altifp)
+in6_ifattach(struct ifnet *ifp)
 {
        struct in6_ifaddr *ia;
        struct in6_addr in6;
@@ -634,7 +630,7 @@ in6_ifattach(struct ifnet *ifp, struct i
        if (ip6_auto_linklocal) {
                ia = in6ifa_ifpforlinklocal(ifp, 0);
                if (ia == NULL) {
-                       if (in6_ifattach_linklocal(ifp, altifp) == 0) {
+                       if (in6_ifattach_linklocal(ifp, NULL) == 0) {
                                /* linklocal address assigned */
                        } else {
                                /* failed to assign linklocal address. bark? */
@@ -687,6 +683,26 @@ in6_ifdetach(struct ifnet *ifp)
         * (Or can we just delay calling nd6_purge until at this point?)
         */
        nd6_purge(ifp);
+
+       /* remove route to interface local allnodes multicast (ff01::1) */
+       bzero(&sin6, sizeof(sin6));
+       sin6.sin6_len = sizeof(struct sockaddr_in6);
+       sin6.sin6_family = AF_INET6;
+       sin6.sin6_addr = in6addr_intfacelocal_allnodes;
+       sin6.sin6_addr.s6_addr16[1] = htons(ifp->if_index);
+       rt = rtalloc1(sin6tosa(&sin6), 0, ifp->if_rdomain);
+       if (rt && rt->rt_ifp == ifp) {
+               struct rt_addrinfo info;
+
+               bzero(&info, sizeof(info));
+               info.rti_flags = rt->rt_flags;
+               info.rti_info[RTAX_DST] = rt_key(rt);
+               info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
+               info.rti_info[RTAX_NETMASK] = rt_mask(rt);
+               rtrequest1(RTM_DELETE, &info, rt->rt_priority, NULL,
+                   ifp->if_rdomain);
+               rtfree(rt);
+       }
 
        /* remove route to link-local allnodes multicast (ff02::1) */
        bzero(&sin6, sizeof(sin6));
Index: netinet6/in6_ifattach.h
===================================================================
RCS file: /cvs/src/sys/netinet6/in6_ifattach.h,v
retrieving revision 1.5
diff -u -p -r1.5 in6_ifattach.h
--- netinet6/in6_ifattach.h     31 Aug 2006 12:37:31 -0000      1.5
+++ netinet6/in6_ifattach.h     9 Dec 2013 19:33:15 -0000
@@ -34,10 +34,10 @@
 #define _NETINET6_IN6_IFATTACH_H_
 
 #ifdef _KERNEL
-void in6_ifattach(struct ifnet *, struct ifnet *);
+void in6_ifattach(struct ifnet *);
 void in6_ifdetach(struct ifnet *);
 int in6_nigroup(struct ifnet *, const char *, int, struct sockaddr_in6 *);
-int in6_ifattach_linklocal(struct ifnet *, struct ifnet *);
+int in6_ifattach_linklocal(struct ifnet *, struct in6_addr *);
 #endif /* _KERNEL */
 
 #endif /* _NETINET6_IN6_IFATTACH_H_ */

Reply via email to