From owner-freebsd-net Wed Jul 18 1: 3:10 2001 Delivered-To: freebsd-net@freebsd.org Received: from whale.sunbay.crimea.ua (whale.sunbay.crimea.ua [212.110.138.65]) by hub.freebsd.org (Postfix) with ESMTP id 1213C37B407; Wed, 18 Jul 2001 01:03:01 -0700 (PDT) (envelope-from ru@whale.sunbay.crimea.ua) Received: (from ru@localhost) by whale.sunbay.crimea.ua (8.11.2/8.11.2) id f6I81wk12805; Wed, 18 Jul 2001 11:01:58 +0300 (EEST) (envelope-from ru) Date: Wed, 18 Jul 2001 11:01:58 +0300 From: Ruslan Ermilov To: JINMEI Tatuya Cc: Garrett Wollman , net@FreeBSD.org Subject: Re: avoiding unnecessary route deletion in rt_fixchange() Message-ID: <20010718110158.A12171@sunbay.com> Mail-Followup-To: JINMEI Tatuya , Garrett Wollman , net@FreeBSD.org References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: ; from jinmei@isl.rdc.toshiba.co.jp on Wed, Jul 18, 2001 at 03:43:41PM +0900 Sender: owner-freebsd-net@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org 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