Date: Mon, 24 Aug 2015 09:30:28 +0000 (UTC) From: Julien Charbon <jch@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r287101 - head/sys/netinet Message-ID: <201508240930.t7O9US2T028246@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jch Date: Mon Aug 24 09:30:27 2015 New Revision: 287101 URL: https://svnweb.freebsd.org/changeset/base/287101 Log: Revert r284245: "Fix a callout race condition introduced in TCP timers callouts with r281599." r281599 fixed a TCP timer race condition, but due a callout(9) bug it also introduced another race condition workaround-ed with r284245. The callout(9) bug being fixed with r286880, we can now revert the workaround (r284245). Differential Revision: https://reviews.freebsd.org/D2079 (Initial change) Differential Revision: https://reviews.freebsd.org/D2763 (Workaround) Differential Revision: https://reviews.freebsd.org/D3078 (Fix) Sponsored by: Verisign, Inc. MFC after: 2 weeks Modified: head/sys/netinet/tcp_timer.c head/sys/netinet/tcp_timer.h Modified: head/sys/netinet/tcp_timer.c ============================================================================== --- head/sys/netinet/tcp_timer.c Mon Aug 24 08:44:44 2015 (r287100) +++ head/sys/netinet/tcp_timer.c Mon Aug 24 09:30:27 2015 (r287101) @@ -355,12 +355,10 @@ tcp_timer_2msl(void *xtp) TCPSTAT_INC(tcps_finwait2_drops); tp = tcp_close(tp); } else { - if (ticks - tp->t_rcvtime <= TP_MAXIDLE(tp)) { - if (!callout_reset(&tp->t_timers->tt_2msl, - TP_KEEPINTVL(tp), tcp_timer_2msl, tp)) { - tp->t_timers->tt_flags &= ~TT_2MSL_RST; - } - } else + if (ticks - tp->t_rcvtime <= TP_MAXIDLE(tp)) + callout_reset(&tp->t_timers->tt_2msl, + TP_KEEPINTVL(tp), tcp_timer_2msl, tp); + else tp = tcp_close(tp); } @@ -440,14 +438,11 @@ tcp_timer_keep(void *xtp) tp->rcv_nxt, tp->snd_una - 1, 0); free(t_template, M_TEMP); } - if (!callout_reset(&tp->t_timers->tt_keep, TP_KEEPINTVL(tp), - tcp_timer_keep, tp)) { - tp->t_timers->tt_flags &= ~TT_KEEP_RST; - } - } else if (!callout_reset(&tp->t_timers->tt_keep, TP_KEEPIDLE(tp), - tcp_timer_keep, tp)) { - tp->t_timers->tt_flags &= ~TT_KEEP_RST; - } + callout_reset(&tp->t_timers->tt_keep, TP_KEEPINTVL(tp), + tcp_timer_keep, tp); + } else + callout_reset(&tp->t_timers->tt_keep, TP_KEEPIDLE(tp), + tcp_timer_keep, tp); #ifdef TCPDEBUG if (inp->inp_socket->so_options & SO_DEBUG) @@ -806,7 +801,6 @@ tcp_timer_activate(struct tcpcb *tp, uin timeout_t *f_callout; struct inpcb *inp = tp->t_inpcb; int cpu = inp_to_cpuid(inp); - uint32_t f_reset; #ifdef TCP_OFFLOAD if (tp->t_flags & TF_TOE) @@ -820,49 +814,38 @@ tcp_timer_activate(struct tcpcb *tp, uin case TT_DELACK: t_callout = &tp->t_timers->tt_delack; f_callout = tcp_timer_delack; - f_reset = TT_DELACK_RST; break; case TT_REXMT: t_callout = &tp->t_timers->tt_rexmt; f_callout = tcp_timer_rexmt; - f_reset = TT_REXMT_RST; break; case TT_PERSIST: t_callout = &tp->t_timers->tt_persist; f_callout = tcp_timer_persist; - f_reset = TT_PERSIST_RST; break; case TT_KEEP: t_callout = &tp->t_timers->tt_keep; f_callout = tcp_timer_keep; - f_reset = TT_KEEP_RST; break; case TT_2MSL: t_callout = &tp->t_timers->tt_2msl; f_callout = tcp_timer_2msl; - f_reset = TT_2MSL_RST; break; default: panic("tp %p bad timer_type %#x", tp, timer_type); } if (delta == 0) { if ((tp->t_timers->tt_flags & timer_type) && - callout_stop(t_callout) && - (tp->t_timers->tt_flags & f_reset)) { - tp->t_timers->tt_flags &= ~(timer_type | f_reset); + callout_stop(t_callout)) { + tp->t_timers->tt_flags &= ~timer_type; } } else { if ((tp->t_timers->tt_flags & timer_type) == 0) { - tp->t_timers->tt_flags |= (timer_type | f_reset); + tp->t_timers->tt_flags |= timer_type; callout_reset_on(t_callout, delta, f_callout, tp, cpu); } else { /* Reset already running callout on the same CPU. */ - if (!callout_reset(t_callout, delta, f_callout, tp)) { - /* - * Callout not cancelled, consider it as not - * properly restarted. */ - tp->t_timers->tt_flags &= ~f_reset; - } + callout_reset(t_callout, delta, f_callout, tp); } } } @@ -899,7 +882,6 @@ tcp_timer_stop(struct tcpcb *tp, uint32_ { struct callout *t_callout; timeout_t *f_callout; - uint32_t f_reset; tp->t_timers->tt_flags |= TT_STOPPED; @@ -907,36 +889,30 @@ tcp_timer_stop(struct tcpcb *tp, uint32_ case TT_DELACK: t_callout = &tp->t_timers->tt_delack; f_callout = tcp_timer_delack_discard; - f_reset = TT_DELACK_RST; break; case TT_REXMT: t_callout = &tp->t_timers->tt_rexmt; f_callout = tcp_timer_rexmt_discard; - f_reset = TT_REXMT_RST; break; case TT_PERSIST: t_callout = &tp->t_timers->tt_persist; f_callout = tcp_timer_persist_discard; - f_reset = TT_PERSIST_RST; break; case TT_KEEP: t_callout = &tp->t_timers->tt_keep; f_callout = tcp_timer_keep_discard; - f_reset = TT_KEEP_RST; break; case TT_2MSL: t_callout = &tp->t_timers->tt_2msl; f_callout = tcp_timer_2msl_discard; - f_reset = TT_2MSL_RST; break; default: panic("tp %p bad timer_type %#x", tp, timer_type); } if (tp->t_timers->tt_flags & timer_type) { - if (callout_stop(t_callout) && - (tp->t_timers->tt_flags & f_reset)) { - tp->t_timers->tt_flags &= ~(timer_type | f_reset); + if (callout_stop(t_callout)) { + tp->t_timers->tt_flags &= ~timer_type; } else { /* * Can't stop the callout, defer tcpcb actual deletion Modified: head/sys/netinet/tcp_timer.h ============================================================================== --- head/sys/netinet/tcp_timer.h Mon Aug 24 08:44:44 2015 (r287100) +++ head/sys/netinet/tcp_timer.h Mon Aug 24 09:30:27 2015 (r287101) @@ -159,12 +159,6 @@ struct tcp_timer { #define TT_2MSL 0x0010 #define TT_MASK (TT_DELACK|TT_REXMT|TT_PERSIST|TT_KEEP|TT_2MSL) -#define TT_DELACK_RST 0x0100 -#define TT_REXMT_RST 0x0200 -#define TT_PERSIST_RST 0x0400 -#define TT_KEEP_RST 0x0800 -#define TT_2MSL_RST 0x1000 - #define TT_STOPPED 0x00010000 #define TP_KEEPINIT(tp) ((tp)->t_keepinit ? (tp)->t_keepinit : tcp_keepinit)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201508240930.t7O9US2T028246>