From owner-freebsd-net@FreeBSD.ORG Tue Sep 9 22:00:19 2008 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6E1B21065673 for ; Tue, 9 Sep 2008 22:00:19 +0000 (UTC) (envelope-from keramida@freebsd.org) Received: from igloo.linux.gr (igloo.linux.gr [62.1.205.36]) by mx1.freebsd.org (Postfix) with ESMTP id 008EB8FC15 for ; Tue, 9 Sep 2008 22:00:18 +0000 (UTC) (envelope-from keramida@freebsd.org) Received: from kobe.laptop (adsl21-99.kln.forthnet.gr [77.49.148.99]) (authenticated bits=128) by igloo.linux.gr (8.14.3/8.14.3/Debian-5) with ESMTP id m89LjPqH011623 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Wed, 10 Sep 2008 00:45:33 +0300 Received: from kobe.laptop (kobe.laptop [127.0.0.1]) by kobe.laptop (8.14.3/8.14.3) with ESMTP id m89LjPp8002591; Wed, 10 Sep 2008 00:45:25 +0300 (EEST) (envelope-from keramida@freebsd.org) Received: (from keramida@localhost) by kobe.laptop (8.14.3/8.14.3/Submit) id m89LjMPv002590; Wed, 10 Sep 2008 00:45:22 +0300 (EEST) (envelope-from keramida@freebsd.org) From: Giorgos Keramidas To: Julian Elischer In-Reply-To: <48C1BD31.6090804@elischer.org> (Julian Elischer's message of "Fri, 05 Sep 2008 16:13:53 -0700") Date: Tue, 09 Sep 2008 21:38:13 +0300 Message-ID: <87y721go6i.fsf@kobe.laptop> References: <48C19568.807@elischer.org> <48C1B774.2020405@elischer.org> <48C1B83C.9000404@elischer.org> <48C1BD31.6090804@elischer.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (berkeley-unix) MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" X-MailScanner-ID: m89LjPqH011623 X-Hellug-MailScanner: Found to be clean X-Hellug-MailScanner-SpamCheck: not spam, SpamAssassin (not cached, score=-4.267, required 5, ALL_TRUSTED -1.80, AWL 0.09, BAYES_00 -2.60, DATE_IN_PAST_03_06 0.04) X-Hellug-MailScanner-From: keramida@freebsd.org X-Spam-Status: No Cc: freebsd-net@freebsd.org Subject: Re: rewrite of rt_check() (now rt_check_fib()) X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Sep 2008 22:00:19 -0000 --=-=-= Content-Transfer-Encoding: quoted-printable Hi Julian. Has anyone else tested this patch? I'm going to have a bit of time to try reproducing this again in the following days. Is this patch version the last one you have written? Should I patch with this one and give it a try? FWIW, reading through this version of rt_check_fib() is nicer, and I really liked the comment that explains how it works :-) On Fri, 05 Sep 2008 16:13:53 -0700, Julian Elischer w= rote: > this time with less (I hope) bugs... > > new macros... > > #define RT_TEMP_UNLOCK(_rt) do { \ > RT_ADDREF(_rt); \ > RT_UNLOCK(_rt); \ > } while (0) > > #define RT_RELOCK(_rt) do { \ > RT_LOCK(_rt) \ > if ((_rt)->rt_refcnt <=3D 1) \ > rtfree(_rt); \ > _rt =3D 0; /* signal that it went away */ \ > else { \ > RT_REMREF(_rt); \ > /* note that _rt is still valid */ \ > } \ > } while (0) > > > with (better) code attached: > > /* > * rt_check() is invoked on each layer 2 output path, prior to > * encapsulating outbound packets. > * > * The function is mostly used to find a routing entry for the gateway, > * which in some protocol families could also point to the link-level > * address for the gateway itself (the side effect of revalidating the > * route to the destination is rather pointless at this stage, we did it > * already a moment before in the pr_output() routine to locate the ifp > * and gateway to use). > * > * When we remove the layer-3 to layer-2 mapping tables from the > * routing table, this function can be removed. > * > * =3D=3D=3D On input =3D=3D=3D > * *dst is the address of the NEXT HOP (which coincides with the > * final destination if directly reachable); > * *lrt0 points to the cached route to the final destination; > * *lrt is not meaningful; > * fibnum is the index to the correct network fib for this packet > * (*lrt0 has not ref held on it so REMREF is not needed ) > * > * =3D=3D=3D Operation =3D=3D=3D > * If the route is marked down try to find a new route. If the route > * to the gateway is gone, try to setup a new route. Otherwise, > * if the route is marked for packets to be rejected, enforce that. > * Note that rtalloc returns an rtentry with an extra REF that we need to= lose. > * > * =3D=3D=3D On return =3D=3D=3D > * *dst is unchanged; > * *lrt0 points to the (possibly new) route to the final destination > * *lrt points to the route to the next hop [LOCKED] > * > * Their values are meaningful ONLY if no error is returned. > * > * To follow this you have to remember that: > * RT_REMREF reduces the reference count by 1 but doesn't check it for 0 = (!) > * RTFREE_LOCKED includes an RT_REMREF (or an rtfree if refs =3D=3D 1) > * and an RT_UNLOCK > * RTFREE does an RT_LOCK and an RTFREE_LOCKED > * The gwroute pointer counts as a reference on the rtentry to which it p= oints. > * so when we add it we use the ref that rtalloc gives us and when we los= e it > * we need to remove the reference. > */ > int > rt_check(struct rtentry **lrt, struct rtentry **lrt0, struct sockaddr *ds= t) > { > return (rt_check_fib(lrt, lrt0, dst, 0)); > } > > int > rt_check_fib(struct rtentry **lrt, struct rtentry **lrt0, struct sockaddr= *dst, > u_int fibnum) > { > struct rtentry *rt; > struct rtentry *rt0; > int error; > > KASSERT(*lrt0 !=3D NULL, ("rt_check")); > rt0 =3D *lrt0; > rt =3D NULL; > > /* NB: the locking here is tortuous... */ > RT_LOCK(rt0); > retry: > if (rt0 && (rt0->rt_flags & RTF_UP) =3D=3D 0) { > /* Current rt0 is useless, try get a replacement. */ > RT_UNLOCK(rt0); > rt0 =3D NULL; > } > if (rt0 =3D=3D NULL) { > rt0 =3D rtalloc1_fib(dst, 1, 0UL, fibnum); > if (rt0 =3D=3D NULL) { > return (EHOSTUNREACH); > } > RT_REMREF(rt0); /* don't need the reference. */ > } > > if (rt0->rt_flags & RTF_GATEWAY) { > if ((rt =3D rt0->rt_gwroute) !=3D NULL) { > RT_LOCK(rt); /* NB: gwroute */ > if ((rt->rt_flags & RTF_UP) =3D=3D 0) { > /* gw route is dud. ignore/lose it */ > RTFREE_LOCKED(rt); /* unref (&unlock) gwroute */ > rt =3D rt0->rt_gwroute =3D NULL; > } > } >=20=09=09 > if (rt =3D=3D NULL) { /* NOT AN ELSE CLAUSE */ > RT_TEMP_UNLOCK(rt0); /* MUST return to undo this */ > rt =3D rtalloc1_fib(rt0->rt_gateway, 1, 0UL, fibnum); > if ((rt =3D=3D rt0) || (rt =3D=3D NULL)) { > /* the best we can do is not good enough */ > if (rt) { > RT_REMREF(rt); /* assumes ref > 0 */ > RT_UNLOCK(rt); > } > RT_FREE(rt0); /* lock, unref, (unlock) */ > return (ENETUNREACH); > } > /* > * Relock it and lose the added reference. > * All sorts of things could have happenned while we > * had no lock on it, so check for them. > */ > RT_RELOCK(rt0); > if (rt0 =3D=3D NULL || ((rt0->rt_flags & RTF_UP) =3D=3D 0)) > /* Ru-roh.. what we had is no longer any good */ > goto retry; > /*=20 > * While we were away, someone replaced the gateway. > * Since a reference count is involved we can't just > * overwrite it. > */ > if (rt0->rt_gwroute) { > if (rt0->rt_gwroute !=3D rt) { > RT_FREE_LOCKED(rt); > goto retry; > } > } else { > rt0->rt_gwroute =3D rt; > } > } > RT_LOCK_ASSERT(rt); > RT_UNLOCK(rt0); > } else { > /* think of rt as having the lock from now on.. */ > rt =3D rt0; > } > /* XXX why are we inspecting rmx_expire? */ > if ((rt->rt_flags & RTF_REJECT) && > (rt->rt_rmx.rmx_expire =3D=3D 0 || > time_uptime < rt->rt_rmx.rmx_expire)) { > RT_UNLOCK(rt); > return (rt =3D=3D rt0 ? EHOSTDOWN : EHOSTUNREACH); > } > > *lrt =3D rt; > *lrt0 =3D rt0; > return (0); > } --=-=-= Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (FreeBSD) iEYEARECAAYFAkjGwqAACgkQ1g+UGjGGA7YxYgCeLIxaFENkLPluvhICMOIjfsxP xHcAoMEJ99l6gQV7lsQKpOPs9/G6EMAT =/10u -----END PGP SIGNATURE----- --=-=-=--