Date: Tue, 04 Jun 2002 00:05:51 +0200 From: Andre Oppermann <oppermann@pipeline.ch> To: Ruslan Ermilov <ru@FreeBSD.ORG> Cc: freebsd-net@FreeBSD.ORG, silby@silby.com Subject: Re: Bug in net/route.c function rtredirect() Message-ID: <3CFBE83F.3E7025A4@pipeline.ch> References: <3CF81BC5.EAF0ACC@pipeline.ch> <20020601095707.GC50435@sunbay.com> <3CF8A6CF.6033679A@pipeline.ch> <20020602125310.GD58857@sunbay.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Ruslan Ermilov wrote: > > On Sat, Jun 01, 2002 at 12:49:51PM +0200, Andre Oppermann wrote: > > You're right but the refcount decrementation will be done, just in a > > different place. In the "done:" section the rtfree() will be done also > > in the case when an existing host route was already present. > > > No it won't be done in "done:", because a few lines later we assign > rt = NULL: > > create: > if (rt) > rt->rt_refcnt--; > flags |= RTF_GATEWAY | RTF_DYNAMIC; > bzero((caddr_t)&info, sizeof(info)); > info.rti_info[RTAX_DST] = dst; > info.rti_info[RTAX_GATEWAY] = gateway; > info.rti_info[RTAX_NETMASK] = netmask; > info.rti_ifa = ifa; > info.rti_flags = flags; > rt = NULL; > > > > > The solution is to remove the rtfree() in rtredirect. Diff against > > > > -STABLE is attached. Should apply to -CURRENT with some fuzz. > > > > > > > No, please try the below patch instead. > > > > The rtfree() I removed is one too many and still has to go. Your patch > > doesn't change the problem, it just makes it different. Routes could > > go below zero refcount in your version. > > > The second rt->rt_refcnt-- I added is indeed unnecessary, you're > right here. 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). 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 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. > 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. -- Andre 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?3CFBE83F.3E7025A4>