On Tue, Jun 04, 2002 at 10:24:49AM +0200, Andre Oppermann wrote: > Ruslan Ermilov wrote: > > > > On Tue, Jun 04, 2002 at 12:05:51AM +0200, Andre Oppermann wrote: > > > After reading this whole redirect stuff a couple of time I've come to > > > the conclusion that the function is right as it is there. There is no > > > such bug as I described it. The rtalloc1() in rtredirect() is incre- > > > menting the refcount on the route found, the two rtfree() after > > > "create:" and "done:" will decrement it as it is must happen (we don't > > > have a reference to that route because we don't clone here). > > > > > Right. There's just no point in calling rtfree() just to decrement > > the route's rt_refcnt (we were only looking up the route, we didn't > > need a reference to it). rtfree() does not free the route because > > it's still RTF_UP, and calling rtfree() also needlessly calls > > rnh_close() (in_clsroute() in the PF_INET case). Hence this would > > still be a slight optimization (tested this time): > > This breaks if someone hacks up the routing table. > What do you mean by "hacks"? > The vast majority > is using rtfree() (actually the RTFREE macro which checks ==0 first). > To be entirely correct and clean the macro RTFREE would be the right > thing to do here. > Well, rtinit() does the same decrementing.
> > %%% > > Index: net/route.c > > =================================================================== > > RCS file: /home/ncvs/src/sys/net/route.c,v > > retrieving revision 1.69 > > diff -u -p -r1.69 route.c > > --- net/route.c 19 Mar 2002 21:54:18 -0000 1.69 > > +++ net/route.c 4 Jun 2002 06:41:42 -0000 > > @@ -345,7 +345,7 @@ rtredirect(dst, gateway, netmask, flags, > > */ > > create: > > if (rt) > > - rtfree(rt); > > + rt->rt_refcnt--; > > flags |= RTF_GATEWAY | RTF_DYNAMIC; > > bzero((caddr_t)&info, sizeof(info)); > > info.rti_info[RTAX_DST] = dst; > > %%% > > > > > A bug is that host routes created by redirect are never being purged. > > > But that one has been present for a long (?) time. > > > > > > I'm still try to track a problem with the following situation: 1. I > > > get a tcp syn from somewhere, 2. a host route is created by tcp, 3. > > > the synack is sent back, 4. we get a redirect from the router to use > > > a different router, 5. host route created from tcp is updated and > > > replaced by icmp redirect route, 6. I see a RTM_LOSING message and > > > the redirect route is being purged. > > > > > This is handled by in_losing(). in_losing() has all the necessary > > comments explaining what's going on here. > > Yes, I see what and why in_losing() is doing it. What I'm wondering is > why are the packets lost so in_losing() is being triggered. In my net- > work such packet loss is not supposed to happen. Otherwise I could only > imagine getting a bugus redirect. That would be a problem on the router > (which is also FreeBSD based). Anyway, I'll investigate my problem here > further and see where that takes me... > OK, you're the best person to try realize that. :-) > > > This happens in, I think, some 5% of the cases. What I'm tracking > > > is why the rtfree() after "create:" is de-refcounting the default > > > route when we should have updated the host route created by tcp > > > before. Maybe this is a side-effect of the tcp syncache and the > > > flow has changed? I'll track what happens there. > > > > > There may be only one reason: there's no (yet) route created by tcp. > > I can't reproduce it here. > > In tcp_input.c is a route lookup (via tcp_rtlookup) in the function > tcp_mssopt(). tcp_mssopt is supposed to look up the outgoing inter- > face to find out the MSS this host supports. tcp_rtlookup rtalloc's > a route and clones it if neccessary. This is before any redirect can > happen because it's before we've sent out the packet which triggers > the redirect. syncache_respond is doing the same. So there should be > a route existing prior to the redirect?! > This TCP-cloned route is only installed after a connection is established. There's the packet flow taking place before it is established. Could it be that you're seeing these RTM_LOOSING during the connection establishment phase? What does tcpdump(1) show you? What does ``route -vn monitor'' tell you? > (UDP is not doing this, it does not request a host route but simply > takes whichever routes matches, usually default route). > I know, there's no per-destination metrics for UDP. :-) > > > > Heh, so you in fact tried to rtfree() "rt" in "done:" by adding > > > > "rtn". And how *rtp (if rtp != NULL) will become "rtn" then? > > > > What about this? > > > > > > No. No bug as I said, no need to patch. Sorry for this touble. > > > > > > For the expiration of redirects I'll port/adapt the NetBSD solution > > > and post the patch here. > > > > > We could treat RTF_DYNAMIC routes just like RTF_WASCLONED ones. > > Seems to work just fine here: > > Ah, an even smarter solution! I'll try this on my box this evening! > OK, I will also wait for what others think about it. > > %%% > > Index: netinet/in_rmx.c > > =================================================================== > > RCS file: /home/ncvs/src/sys/netinet/in_rmx.c,v > > retrieving revision 1.42 > > diff -u -p -r1.42 in_rmx.c > > --- netinet/in_rmx.c 19 Mar 2002 21:25:46 -0000 1.42 > > +++ netinet/in_rmx.c 4 Jun 2002 06:41:42 -0000 > > @@ -202,8 +202,10 @@ in_clsroute(struct radix_node *rn, struc > > if((rt->rt_flags & (RTF_LLINFO | RTF_HOST)) != RTF_HOST) > > return; > > > > - if((rt->rt_flags & (RTF_WASCLONED | RTPRF_OURS)) > > - != RTF_WASCLONED) > > + if((rt->rt_flags & RTPRF_OURS) != 0) > > + return; > > + > > + if((rt->rt_flags & (RTF_WASCLONED | RTF_DYNAMIC)) == 0) > > return; > > > > /* > > %%% > > > > On Mon, Jun 03, 2002 at 06:37:00PM -0400, Garrett Wollman wrote: > > > <<On Tue, 04 Jun 2002 00:05:51 +0200, Andre Oppermann <[EMAIL PROTECTED]> >said: > > > > > > > A bug is that host routes created by redirect are never being purged. > > > > But that one has been present for a long (?) time. > > > > > > You are expected to be running a routing process (like `routed' in > > > router-discovery mode) which will delete them for you. I agree that > > > this is not a reasonable expectation, but that was the design intent. > > > > > I think there's nothing wrong if dynamically created routes (by redirect) > > will be treated as "temporary" routes, just like protocol-cloned routes. > > The above patch for in_rmx.c implements this. Let me know what do you > > think. > > /me thinks this is fine. This reminds of a problem I had in August 2001 > with the .CH secondary nameserver ccTLD.tix.ch where I've got more than > 303'000 redirects in the routing table. The thread was named "303,000 > routes in kernel". Thankfully the network code is quite robust and > didn't fall over when it could no longer allocate memory for new routes. > And if you think about it, dynamically learned routes are just another way of "cloning". Your host learns them from a remote gateway, and that gateway's configuration may change as time goes by, and this route would become stale. Cheers, -- Ruslan Ermilov Sysadmin and DBA, [EMAIL PROTECTED] Sunbay Software AG, [EMAIL PROTECTED] FreeBSD committer, +380.652.512.251 Simferopol, Ukraine http://www.FreeBSD.org The Power To Serve http://www.oracle.com Enabling The Information Age
msg06229/pgp00000.pgp
Description: PGP signature
