panic in netinet/tcp_syncache.c: syncache_timer
I'd like a review of the following fix I'd like to commit.
The syncache_timer() function seems to have a locking problem causing
panics, even after yesterday's patches.
This apparently occurs when there are unexpired syncache entries
while the corresponding listening socket is closed. tcp_close()
destroys the relevant lock in the inpcb structure, which causes
INP_LOCK() on that structure in the next syncache_timer() call to
panic.
I'm testing the patch below, which simply removes the inpcb locking
and avoids the panic. It seems safe to me since we're running splnet,
but I'm not sure it's correct since I suppose the locking is there
for a reason...
--- tcp_syncache.c.old Sat Dec 21 03:03:22 2002
+++ tcp_syncache.c Sat Dec 21 17:50:10 2002
@@ -384,14 +384,12 @@
break;
sc = nsc;
inp = sc->sc_tp->t_inpcb;
- INP_LOCK(inp);
if (slot == SYNCACHE_MAXREXMTS ||
slot >= tcp_syncache.rexmt_limit ||
inp->inp_gencnt != sc->sc_inp_gencnt) {
nsc = TAILQ_NEXT(sc, sc_timerq);
syncache_drop(sc, NULL);
tcpstat.tcps_sc_stale++;
- INP_UNLOCK(inp);
continue;
}
/*
@@ -400,7 +398,6 @@
* entry on the timer chain until it has completed.
*/
(void) syncache_respond(sc, NULL);
- INP_UNLOCK(inp);
nsc = TAILQ_NEXT(sc, sc_timerq);
tcpstat.tcps_sc_retransmitted++;
TAILQ_REMOVE(&tcp_syncache.timerq[slot], sc, sc_timerq);
--
Pierre Beyssac [EMAIL PROTECTED] [EMAIL PROTECTED]
Free domains: http://www.eu.org/ or mail [EMAIL PROTECTED]
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: panic in netinet/tcp_syncache.c: syncache_timer
Can you try upgrading to rev 1.29 of tcp_syncache.c which I committed yesterday? I suspect that should fix this problem. Jeffrey To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: panic in netinet/tcp_syncache.c: syncache_timer
On Sat, Dec 21, 2002 at 10:57:35AM -0800, Jeffrey Hsu wrote: > Can you try upgrading to rev 1.29 of tcp_syncache.c which I committed > yesterday? I suspect that should fix this problem. No, I believed that too when I saw your patch, but it didn't solve my problem. -- Pierre Beyssac [EMAIL PROTECTED] [EMAIL PROTECTED] Free domains: http://www.eu.org/ or mail [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: panic in netinet/tcp_syncache.c: syncache_timer
> I'm testing the patch below, which simply removes the inpcb locking > and avoids the panic. It seems safe to me since we're running splnet, > but I'm not sure it's correct since I suppose the locking is there > for a reason... It's safe to remove those inp locks. We only use the generation count to check to see if the inp has been deleted. Jeffrey To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: panic in netinet/tcp_syncache.c: syncache_timer
Pierre, can you see if this patch fixes your problem? Thanks.
Jeffrey
Index: tcp_syncache.c
===
RCS file: /home/ncvs/src/sys/netinet/tcp_syncache.c,v
retrieving revision 1.30
diff -u -r1.30 tcp_syncache.c
--- tcp_syncache.c 20 Dec 2002 11:24:02 - 1.30
+++ tcp_syncache.c 21 Dec 2002 19:52:18 -
@@ -384,14 +384,12 @@
break;
sc = nsc;
inp = sc->sc_tp->t_inpcb;
- INP_LOCK(inp);
if (slot == SYNCACHE_MAXREXMTS ||
slot >= tcp_syncache.rexmt_limit ||
inp->inp_gencnt != sc->sc_inp_gencnt) {
nsc = TAILQ_NEXT(sc, sc_timerq);
syncache_drop(sc, NULL);
tcpstat.tcps_sc_stale++;
- INP_UNLOCK(inp);
continue;
}
/*
@@ -399,6 +397,7 @@
* to modify another entry, so do not obtain the next
* entry on the timer chain until it has completed.
*/
+ INP_LOCK(inp);
(void) syncache_respond(sc, NULL);
INP_UNLOCK(inp);
nsc = TAILQ_NEXT(sc, sc_timerq);
Re: panic in netinet/tcp_syncache.c: syncache_timer
On Sat, Dec 21, 2002 at 12:03:24PM -0800, Jeffrey Hsu wrote: > Pierre, can you see if this patch fixes your problem? Thanks. Yes, it does. Actually I tried that before, but then I thought locking at this place was probably unnecessary because it seemed to apply to the generation count only. As matter of fact I just committed the previous patch I sent before I saw your mail... probably we should commit yours instead? -- Pierre Beyssac [EMAIL PROTECTED] [EMAIL PROTECTED] Free domains: http://www.eu.org/ or mail [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
