Re: KASSERT() in kern_timeout.c

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

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 <bouyer%antioche.eu.org@localhost>
     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);
 }