Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 9 Apr 2004 04:27:20 -0700
From:      Luigi Rizzo <rizzo@icir.org>
To:        ume@freebsd.org, net@freebsd.org, ru@freebsd.org, andre@freebsd.org
Cc:        current@freebsd.org
Subject:   suggested patches for netinet6/
Message-ID:  <20040409042720.A99087@xorpc.icir.org>
next in thread | raw e-mail | index | archive | help 

While adapting to ipv6 the new arp table code I am developing
following Andre's ideas, i hit a few places that would deserve a
fix independently of that:

+ nd6_nud_hint() is only called as nd6_nud_hint(NULL, NULL, 0);
  in some cases from netinet/tcp_input.c
  With these args, the routine is a NOP. I propose to remove it
  (and the associated field, ln_byhint, in struct llinfo_nd6)

+ In a couple of places, the logic to compute 'olladdr' and 'llchange'
  is rather convoluted, and it could be greatly simplified (see below)

+ nd6_output() contains a tail-recursive call which certainly can be
  removed by using a 'goto' near the beginning of the function
  (or maybe in a better way!)

Also:
+ in nd6_na_input() we are responding to an nd6 message on interface 'ifp',
  so i guess in the routine rt->rt_ifp == ifp, and we can use the latter
  when needed instead of rt->rt_ifp ?
 
+ is it ok to remove the __P() from the header files, ANSIfy
  the function declarations and make them static as appropriate ?
  Of course this ought to be done as a separate step.

I am attaching the relevant snippets of the diff... Can anyone comment ?
Any objection if i commit the trivial fixes to the first 3 issues above ?
What about the style changes ?

	cheers
	luigi

Index: nd6.c
===================================================================
-	olladdr = (sdl->sdl_alen) ? 1 : 0;
-	if (olladdr && lladdr) {
-		if (bcmp(lladdr, LLADDR(sdl), ifp->if_addrlen))
-			llchange = 1;
-		else
-			llchange = 0;
-	} else
-		llchange = 0;
+	olladdr = ln->ln_state >= ND6_LLINFO_REACHABLE;
+	llchange = olladdr && lladdr &&	bcmp(lladdr, &ln->ll_addr, ifp->if_addrlen);
 
@@ -1844,11 +1836,8 @@ nd6_output(ifp, origifp, m0, dst, rt0)
+again:
        /*
         * next hop determination.  This routine is derived from ether_outpout.
         */
        if (rt) {
                if ((rt->rt_flags & RTF_UP) == 0) {
                        rt0 = rt = rtalloc1((struct sockaddr *)dst, 1, 0UL);
 			if (rt != NULL) {
 				RT_REMREF(rt);
 				RT_UNLOCK(rt);
-				if (rt->rt_ifp != ifp) {
-					/* XXX: loop care? */
-					return nd6_output(ifp, origifp, m0,
-					    dst, rt);
-				}
+				if (rt->rt_ifp != ifp)
+					goto again;	/* rt has changed. */
 			} else
 				senderr(EHOSTUNREACH);
 		}
 
Index: nd6_nbr.c
===================================================================
@@ -653,17 +650,9 @@ nd6_na_input(m, off, icmp6len)
 		/*
 		 * Check if the link-layer address has changed or not.
 		 */
-		if (!lladdr)
-			llchange = 0;
-		else {
-			if (sdl->sdl_alen) {
-				if (bcmp(lladdr, LLADDR(sdl), ifp->if_addrlen))
-					llchange = 1;
-				else
-					llchange = 0;
-			} else
-				llchange = 1;
-		}
+		llchange = lladdr &&
+			( (ln->ln_state < ND6_LLINFO_REACHABLE) ||
+				bcmp(lladdr, &ln->l3_addr, ifp->if_addrlen) );
 


@@ -744,7 +728,7 @@ nd6_na_input(m, off, icmp6len)
 			 * context.  However, we keep it just for safety.
 			 */
 			s = splnet();
-			dr = defrouter_lookup(in6, rt->rt_ifp);
+			dr = defrouter_lookup(&ln->l3_addr, ifp);
 			if (dr)
 				defrtrlist_del(dr);
 			else if (!ip6_forwarding && ip6_accept_rtadv) {
@@ -755,21 +739,25 @@ nd6_na_input(m, off, icmp6len)
 				 * (e.g. redirect case). So we must
 				 * call rt6_flush explicitly.
 				 */
-				rt6_flush(&ip6->ip6_src, rt->rt_ifp);
+				rt6_flush(&ip6->ip6_src, ifp);
 			}
 			splx(s);
 		}
 		ln->ln_router = is_router;
 	}

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040409042720.A99087>