Date: Wed, 18 Jul 2001 11:01:58 +0300 From: Ruslan Ermilov <ru@FreeBSD.org> To: JINMEI Tatuya <jinmei@isl.rdc.toshiba.co.jp> Cc: Garrett Wollman <wollman@FreeBSD.org>, net@FreeBSD.org Subject: Re: avoiding unnecessary route deletion in rt_fixchange() Message-ID: <20010718110158.A12171@sunbay.com> In-Reply-To: <y7vvgkqenz6.wl@condor.jinmei.org>; from jinmei@isl.rdc.toshiba.co.jp on Wed, Jul 18, 2001 at 03:43:41PM %2B0900 References: <y7vvgkqenz6.wl@condor.jinmei.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Yay, cool! Unless Garrett commits this in a few days, I will
on Friday. And yes, this fixes the "mysterious" problem when
gwroute was not allocated when adding the "default" route.
Thanks!
On Wed, Jul 18, 2001 at 03:43:41PM +0900, JINMEI Tatuya / ?$B?@L@C#:H?(B wrote:
> As commented in defined in sys/net/route.c, rt_fixchange() has a bad
> effect, which would cause unnecessary route deletion:
>
> * Unfortunately, this has the obnoxious
> * property of also triggering for insertion /above/ a pre-existing network
> * route and clones. Sigh. This may be fixed some day.
>
> The effect has been even worse, because recent versions of route.c set
> the parent rtentry for cloned routes from an interface-direct route.
> For example, suppose that we have an interface "ne0" that has an IPv4
> subnet "10.0.0.0/24". Then we may have a cloned route like 10.0.0.1
> on the interface, whose parent route is 10.0.0.0/24 (to the interface
> ne0). Now, when we add the default route (i.e. 0.0.0.0/0),
> rt_fixchange() will remove the cloned route 10.0.0.1. The (bad) effect
> also prevents rt_setgate from configuring rt_gwroute, which would not
> be an intended behavior.
>
> As suggested in the comments to rt_fixchange(), we need stricter check
> in the function, to prevent unintentional route deletion. The
> attached is a proposed fix to this problem (for FreeBSD4-STABLE).
> Please review it, and merge it to the repository if acceptable.
>
> This fix will also solve the "IPV6 panic?" problem which was recently
> reported in this list.
>
> Thanks,
>
> JINMEI, Tatuya
> Communication Platform Lab.
> Corporate R&D Center, Toshiba Corp.
> jinmei@isl.rdc.toshiba.co.jp
>
> *** route.c.orig Wed Jul 18 15:13:18 2001
> --- route.c Wed Jul 18 15:18:15 2001
> ***************
> *** 726,734 ****
> * network route and (cloned) host routes. For this reason, a simple check
> * of rt->rt_parent is insufficient; each candidate route must be tested
> * against the (mask, value) of the new route (passed as before in vp)
> ! * to see if the new route matches it. Unfortunately, this has the obnoxious
> ! * property of also triggering for insertion /above/ a pre-existing network
> ! * route and clones. Sigh. This may be fixed some day.
> *
> * XXX - it may be possible to do fixdelete() for changes and reserve this
> * routine just for adds. I'm not sure why I thought it was necessary to do
> --- 726,732 ----
> * network route and (cloned) host routes. For this reason, a simple check
> * of rt->rt_parent is insufficient; each candidate route must be tested
> * against the (mask, value) of the new route (passed as before in vp)
> ! * to see if the new route matches it.
> *
> * XXX - it may be possible to do fixdelete() for changes and reserve this
> * routine just for adds. I'm not sure why I thought it was necessary to do
> ***************
> *** 747,754 ****
> struct rtfc_arg *ap = vp;
> struct rtentry *rt0 = ap->rt0;
> struct radix_node_head *rnh = ap->rnh;
> ! u_char *xk1, *xm1, *xk2;
> ! int i, len;
>
> #ifdef DEBUG
> if (rtfcdebug)
> --- 745,752 ----
> struct rtfc_arg *ap = vp;
> struct rtentry *rt0 = ap->rt0;
> struct radix_node_head *rnh = ap->rnh;
> ! u_char *xk1, *xm1, *xk2, *xmp;
> ! int i, len, mlen;
>
> #ifdef DEBUG
> if (rtfcdebug)
> ***************
> *** 781,786 ****
> --- 779,806 ----
> xk1 = (u_char *)rt_key(rt0);
> xm1 = (u_char *)rt_mask(rt0);
> xk2 = (u_char *)rt_key(rt);
> +
> + /* avoid applying a less specific route */
> + xmp = (u_char *)rt_mask(rt->rt_parent);
> + mlen = ((struct sockaddr *)rt_key(rt->rt_parent))->sa_len;
> + if (mlen > ((struct sockaddr *)rt_key(rt0))->sa_len) {
> + #ifdef DEBUG
> + if (rtfcdebug)
> + printf("rt_fixchange: inserting a less "
> + "specific route\n");
> + #endif
> + return 0;
> + }
> + for (i = rnh->rnh_treetop->rn_offset; i < mlen; i++) {
> + if ((xmp[i] & ~(xmp[i] ^ xm1[i])) != xmp[i]) {
> + #ifdef DEBUG
> + if (rtfcdebug)
> + printf("rt_fixchange: inserting a less "
> + "specific route\n");
> + #endif
> + return 0;
> + }
> + }
>
> for (i = rnh->rnh_treetop->rn_offset; i < len; i++) {
> if ((xk2[i] & xm1[i]) != xk1[i]) {
--
Ruslan Ermilov Oracle Developer/DBA,
ru@sunbay.com Sunbay Software AG,
ru@FreeBSD.org FreeBSD committer,
+380.652.512.251 Simferopol, Ukraine
http://www.FreeBSD.org The Power To Serve
http://www.oracle.com Enabling The Information Age
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010718110158.A12171>