Re: Examine route.c! -- two bugs fixed -- patch attached!

Wolfgang Walter (wolfgang.walter@stusta.mhn.de)
Thu, 4 Dec 1997 19:35:08 +0100


On Thu, Dec 04, 1997 at 07:11:34PM +0100, Ingo Molnar wrote:
>
> On Thu, 4 Dec 1997, Alan Cox wrote:
>
> > > > the reference count is exactly one. This is in the most cases correct.
> > > > If Philip found out all possible drops this may be a reason.
> > >
> > > Im beginning to suspect we have a kernel/interrupt context problem here and
> > > the reference counters arent safe
> >
> > Scratch that it seems.
>
> I've verified ipv4/*.c, wether we accidentally drop/overwrite a route
> somewhere, but apart from Daniel's fixes everything seemed to be just
> fine. One thing i was unable to verify, when we destroy a socket, do we
> always 'rt_put()' it's sk->ip_cached_route? [but this should make no
> difference as the activity done by the 'nc' script does not create as
> many sockets as there are leaks].
>
> What i suspect is a nontrivial race somewhere in the tcp+ip send/receive
> path, especially when adding cached routes. Since there are no crashes, it
> must be a 'clean' race, ie. some list element gets lost when adding a new
> element to the route hash, or similar?
>
> -- mingo
>

This maybe is a stupid remark and is probably not related to that problem, but I
think this is not ok:

in (2.0.32) in route.c ip_rt_put is

void ip_rt_put(struct rtable * rt)
{
if (rt)
atomic_dec(&rt->rt_refcnt);

/* If this rtable entry is not in the cache, we'd better free it once the
* refcnt goes to zero, because nobody else will... */
if ( rt && (rt->rt_flags & RTF_NOTCACHED) && (!rt->rt_refcnt) )
rt_free(rt);
}

whereas the inline function is (net/route.h) still is:

extern __inline__ void ip_rt_put(struct rtable * rt)
#ifndef MODULE
{
if (rt)
atomic_dec(&rt->rt_refcnt);
}
#else
;
#endif

I think there should be no difference if something using ip_rt_put is
compiled as module (not using the inline version) or into kernel.

Wolfgang Walter