From owner-freebsd-net@FreeBSD.ORG Fri Apr 9 04:27:22 2004 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 90ABF16A4CE; Fri, 9 Apr 2004 04:27:22 -0700 (PDT) Received: from xorpc.icir.org (xorpc.icir.org [192.150.187.68]) by mx1.FreeBSD.org (Postfix) with ESMTP id 61E6543D53; Fri, 9 Apr 2004 04:27:22 -0700 (PDT) (envelope-from rizzo@icir.org) Received: from xorpc.icir.org (localhost [127.0.0.1]) by xorpc.icir.org (8.12.9p1/8.12.8) with ESMTP id i39BRKgd008244; Fri, 9 Apr 2004 04:27:20 -0700 (PDT) (envelope-from rizzo@xorpc.icir.org) Received: (from rizzo@localhost) by xorpc.icir.org (8.12.9p1/8.12.3/Submit) id i39BRKZx008241; Fri, 9 Apr 2004 04:27:20 -0700 (PDT) (envelope-from rizzo) Date: Fri, 9 Apr 2004 04:27:20 -0700 From: Luigi Rizzo To: ume@freebsd.org, net@freebsd.org, ru@freebsd.org, andre@freebsd.org Message-ID: <20040409042720.A99087@xorpc.icir.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i cc: current@freebsd.org Subject: suggested patches for netinet6/ X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 09 Apr 2004 11:27:22 -0000 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; }