On Mon, Apr 19, 2010 at 02:09:43PM +0200, Manuel Bouyer wrote: > On Mon, Apr 19, 2010 at 12:55:21PM +0200, Manuel Bouyer wrote: > > > There are multiple paths calling syn_cache_put() and not only SYN cache, > > > but also upper TCP layers and IP would need to be inspected. Unlikely to > > > be safe and unlikely to be trivial to re-structure to be safe.. there we > > > shall start a big MP-safe network stack revamp. > > > > > > For a quick fix (apart from horrible hack, like having another softint > > > or kthread), it seems that we can use SCF_DEAD with syn_cache_timer() > > > invoked via our callout, which would destroy... itself. :) > > > > You mean, something like the attached diff ? > > That won't work well, because syn_cache_put() is called from > syn_cache_timer(). In addition syn_cache_rm() which does a callout_stop > is always called before syn_cache_put(), or syn_cache_put() is called > before the timer is armed. > would the attacched diff be OK ?
Forgot to free sc_route and sc_ipopts in the dropit: case. Updated patch attached. > > BTW, there is a comment in syn_cache_timer() saying "calls pool_put but > see spl above", but syn_cache_timer() has no spl calls. Should it raise > the IPL to splsoftnet() ? This remains ... -- Manuel Bouyer <bou...@antioche.eu.org> NetBSD: 26 ans d'experience feront toujours la difference --
Index: tcp_input.c =================================================================== RCS file: /cvsroot/src/sys/netinet/tcp_input.c,v retrieving revision 1.291.4.2 diff -u -p -u -r1.291.4.2 tcp_input.c --- tcp_input.c 26 Sep 2009 18:34:29 -0000 1.291.4.2 +++ tcp_input.c 19 Apr 2010 12:40:37 -0000 @@ -3343,12 +3343,7 @@ syn_cache_put(struct syn_cache *sc) if (sc->sc_ipopts) (void) m_free(sc->sc_ipopts); rtcache_free(&sc->sc_route); - if (callout_invoking(&sc->sc_timer)) - sc->sc_flags |= SCF_DEAD; - else { - callout_destroy(&sc->sc_timer); - pool_put(&syn_cache_pool, sc); - } + sc->sc_flags |= SCF_DEAD; } void @@ -3509,7 +3504,11 @@ syn_cache_timer(void *arg) dropit: TCP_STATINC(TCP_STAT_SC_TIMED_OUT); syn_cache_rm(sc); - syn_cache_put(sc); /* calls pool_put but see spl above */ + if (sc->sc_ipopts) + (void) m_free(sc->sc_ipopts); + rtcache_free(&sc->sc_route); + callout_destroy(&sc->sc_timer); + pool_put(&syn_cache_pool, sc); KERNEL_UNLOCK_ONE(NULL); mutex_exit(softnet_lock); }