Date: Fri, 26 Nov 1999 17:41:49 -0800 (PST) From: John Polstra <jdp@polstra.com> To: wollman@khavrinen.lcs.mit.edu Cc: current@freebsd.org Subject: Re: Route table leaks Message-ID: <199911270141.RAA29416@vashon.polstra.com> In-Reply-To: <199911221552.KAA84691@khavrinen.lcs.mit.edu> References: <199911220150.UAA78559@khavrinen.lcs.mit.edu> <XFMail.991121195840.jdp@polstra.com> <199911221552.KAA84691@khavrinen.lcs.mit.edu>
next in thread | previous in thread | raw e-mail | index | archive | help
I think I've finally found the route table leak. At least I found _a_
leak, and I think it's the one that's been plaguing cvsup-master. I
have a question or two (see below) before I commit the fix.
Here's how to leak a route table entry. Establish a TCP connection
with another machine so that you have a cloned route to that host.
With the connection idle, use "route delete" to remove the cloned
route. The route disappears from the routing table, but it is
not freed. (The Leak.) Now cause some packets to travel on the
connection. A new cloned route is created and added to the routing
table.
Each time you do that, you leak a struct rtentry and also a 32-byte
chunk that's used to hold a couple of address structures. Routed is
doing these route deletions regularly on cvsup-master. I haven't
tried to figure out why.
The leak is in rtalloc() and rtalloc_ign(), and here's the patch I'm
using to fix it:
Index: route.c
===================================================================
RCS file: /home/ncvs/src/sys/net/route.c,v
retrieving revision 1.53
diff -u -r1.53 route.c
--- route.c 1999/08/28 00:48:28 1.53
+++ route.c 1999/11/27 01:21:56
@@ -88,8 +88,16 @@
rtalloc(ro)
register struct route *ro;
{
- if (ro->ro_rt && ro->ro_rt->rt_ifp && (ro->ro_rt->rt_flags & RTF_UP))
- return; /* XXX */
+ struct rtentry *rt;
+ int s;
+
+ if ((rt = ro->ro_rt) != NULL) {
+ if (rt->rt_ifp != NULL && rt->rt_flags & RTF_UP)
+ return;
+ s = splnet();
+ RTFREE(rt);
+ splx(s);
+ }
ro->ro_rt = rtalloc1(&ro->ro_dst, 1, 0UL);
}
@@ -98,8 +106,16 @@
register struct route *ro;
u_long ignore;
{
- if (ro->ro_rt && ro->ro_rt->rt_ifp && (ro->ro_rt->rt_flags & RTF_UP))
- return; /* XXX */
+ struct rtentry *rt;
+ int s;
+
+ if ((rt = ro->ro_rt) != NULL) {
+ if (rt->rt_ifp != NULL && rt->rt_flags & RTF_UP)
+ return;
+ s = splnet();
+ RTFREE(rt);
+ splx(s);
+ }
ro->ro_rt = rtalloc1(&ro->ro_dst, 1, ignore);
}
The original code was jamming a new pointer into ro->ro_rt, but it
didn't free the old rtentry that was referenced there.
Now for my questions:
1. Do I really need the splnet calls around RTFREE?
2. To eliminate all the duplicated code, shall I make rtalloc just
call rtalloc_ign(ro, 0UL)? I assume that was avoided originally for
performance reasons, but now there's more code than before.
John
--
John Polstra jdp@polstra.com
John D. Polstra & Co., Inc. Seattle, Washington USA
"No matter how cynical I get, I just can't keep up." -- Nora Ephron
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199911270141.RAA29416>