Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 16 Aug 2016 12:40:56 +0000 (UTC)
From:      Randall Stewart <rrs@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r304218 - head/sys/netinet
Message-ID:  <201608161240.u7GCeuWS082118@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: rrs
Date: Tue Aug 16 12:40:56 2016
New Revision: 304218
URL: https://svnweb.freebsd.org/changeset/base/304218

Log:
  This cleans up the timer code in TCP and also makes it so we do not
  take the INFO lock *unless* we are really going to delete the TCB.
  
  Differential Revision:	D7136

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	Tue Aug 16 12:13:12 2016	(r304217)
+++ head/sys/netinet/tcp_timer.c	Tue Aug 16 12:40:56 2016	(r304218)
@@ -294,11 +294,6 @@ tcp_timer_delack(void *xtp)
 		CURVNET_RESTORE();
 		return;
 	}
-	KASSERT((tp->t_timers->tt_flags & TT_STOPPED) == 0,
-		("%s: tp %p tcpcb can't be stopped here", __func__, tp));
-	KASSERT((tp->t_timers->tt_flags & TT_DELACK) != 0,
-		("%s: tp %p delack callout should be running", __func__, tp));
-
 	tp->t_flags |= TF_ACKNOW;
 	TCPSTAT_INC(tcps_delack);
 	(void) tp->t_fb->tfb_tcp_output(tp);
@@ -306,6 +301,39 @@ tcp_timer_delack(void *xtp)
 	CURVNET_RESTORE();
 }
 
+int
+tcp_inpinfo_lock_add(struct inpcb *inp)
+{
+	in_pcbref(inp);
+	INP_WUNLOCK(inp);
+	INP_INFO_RLOCK(&V_tcbinfo);
+	INP_WLOCK(inp);
+	if (inp->inp_flags & (INP_TIMEWAIT | INP_DROPPED)) {
+		return(1);
+	}
+	return(0);
+
+}
+
+void
+tcp_inpinfo_lock_del(struct inpcb *inp, struct tcpcb *tp)
+{
+	INP_INFO_RUNLOCK(&V_tcbinfo);
+	if (inp && (tp == NULL)) {
+		/*
+		 * If tcp_close/drop() gets called and tp
+		 * returns NULL, then the function dropped
+		 * the inp lock, we hold a reference keeping
+		 * this around, so we must re-aquire the 
+		 * INP_WLOCK() in order to proceed with
+		 * our dropping the inp reference.
+		 */
+		INP_WLOCK(inp);
+	}
+	if (inp && in_pcbrele_wlocked(inp) == 0)
+		INP_WUNLOCK(inp);
+}
+
 void
 tcp_timer_2msl(void *xtp)
 {
@@ -317,7 +345,6 @@ tcp_timer_2msl(void *xtp)
 
 	ostate = tp->t_state;
 #endif
-	INP_INFO_RLOCK(&V_tcbinfo);
 	inp = tp->t_inpcb;
 	KASSERT(inp != NULL, ("%s: tp %p tp->t_inpcb == NULL", __func__, tp));
 	INP_WLOCK(inp);
@@ -325,21 +352,17 @@ tcp_timer_2msl(void *xtp)
 	if (callout_pending(&tp->t_timers->tt_2msl) ||
 	    !callout_active(&tp->t_timers->tt_2msl)) {
 		INP_WUNLOCK(tp->t_inpcb);
-		INP_INFO_RUNLOCK(&V_tcbinfo);
 		CURVNET_RESTORE();
 		return;
 	}
 	callout_deactivate(&tp->t_timers->tt_2msl);
 	if ((inp->inp_flags & INP_DROPPED) != 0) {
 		INP_WUNLOCK(inp);
-		INP_INFO_RUNLOCK(&V_tcbinfo);
 		CURVNET_RESTORE();
 		return;
 	}
 	KASSERT((tp->t_timers->tt_flags & TT_STOPPED) == 0,
 		("%s: tp %p tcpcb can't be stopped here", __func__, tp));
-	KASSERT((tp->t_timers->tt_flags & TT_2MSL) != 0,
-		("%s: tp %p 2msl callout should be running", __func__, tp));
 	/*
 	 * 2 MSL timeout in shutdown went off.  If we're closed but
 	 * still waiting for peer to close and connection has been idle
@@ -355,7 +378,6 @@ tcp_timer_2msl(void *xtp)
 	 */
 	if ((inp->inp_flags & INP_TIMEWAIT) != 0) {
 		INP_WUNLOCK(inp);
-		INP_INFO_RUNLOCK(&V_tcbinfo);
 		CURVNET_RESTORE();
 		return;
 	}
@@ -363,15 +385,26 @@ tcp_timer_2msl(void *xtp)
 	    tp->t_inpcb && tp->t_inpcb->inp_socket && 
 	    (tp->t_inpcb->inp_socket->so_rcv.sb_state & SBS_CANTRCVMORE)) {
 		TCPSTAT_INC(tcps_finwait2_drops);
+		if (tcp_inpinfo_lock_add(inp)) {
+			tcp_inpinfo_lock_del(inp, tp);
+			goto out;
+		}
 		tp = tcp_close(tp);             
+		tcp_inpinfo_lock_del(inp, tp);
+		goto out;
 	} 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;
+			callout_reset(&tp->t_timers->tt_2msl,
+				      TP_KEEPINTVL(tp), tcp_timer_2msl, tp);
+		} else {
+			if (tcp_inpinfo_lock_add(inp)) {
+				tcp_inpinfo_lock_del(inp, tp);
+				goto out;
 			}
-		} else
-		       tp = tcp_close(tp);
+			tp = tcp_close(tp);
+			tcp_inpinfo_lock_del(inp, tp);
+			goto out;
+		}
        }
 
 #ifdef TCPDEBUG
@@ -383,7 +416,7 @@ tcp_timer_2msl(void *xtp)
 
 	if (tp != NULL)
 		INP_WUNLOCK(inp);
-	INP_INFO_RUNLOCK(&V_tcbinfo);
+out:
 	CURVNET_RESTORE();
 }
 
@@ -399,28 +432,23 @@ tcp_timer_keep(void *xtp)
 
 	ostate = tp->t_state;
 #endif
-	INP_INFO_RLOCK(&V_tcbinfo);
 	inp = tp->t_inpcb;
 	KASSERT(inp != NULL, ("%s: tp %p tp->t_inpcb == NULL", __func__, tp));
 	INP_WLOCK(inp);
 	if (callout_pending(&tp->t_timers->tt_keep) ||
 	    !callout_active(&tp->t_timers->tt_keep)) {
 		INP_WUNLOCK(inp);
-		INP_INFO_RUNLOCK(&V_tcbinfo);
 		CURVNET_RESTORE();
 		return;
 	}
 	callout_deactivate(&tp->t_timers->tt_keep);
 	if ((inp->inp_flags & INP_DROPPED) != 0) {
 		INP_WUNLOCK(inp);
-		INP_INFO_RUNLOCK(&V_tcbinfo);
 		CURVNET_RESTORE();
 		return;
 	}
 	KASSERT((tp->t_timers->tt_flags & TT_STOPPED) == 0,
 		("%s: tp %p tcpcb can't be stopped here", __func__, tp));
-	KASSERT((tp->t_timers->tt_flags & TT_KEEP) != 0,
-		("%s: tp %p keep callout should be running", __func__, tp));
 	/*
 	 * Keep-alive timer went off; send something
 	 * or drop connection if idle for too long.
@@ -452,14 +480,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)
@@ -468,12 +493,16 @@ tcp_timer_keep(void *xtp)
 #endif
 	TCP_PROBE2(debug__user, tp, PRU_SLOWTIMO);
 	INP_WUNLOCK(inp);
-	INP_INFO_RUNLOCK(&V_tcbinfo);
 	CURVNET_RESTORE();
 	return;
 
 dropit:
 	TCPSTAT_INC(tcps_keepdrops);
+
+	if (tcp_inpinfo_lock_add(inp)) {
+		tcp_inpinfo_lock_del(inp, tp);
+		goto out;
+	}
 	tp = tcp_drop(tp, ETIMEDOUT);
 
 #ifdef TCPDEBUG
@@ -482,9 +511,8 @@ dropit:
 			  PRU_SLOWTIMO);
 #endif
 	TCP_PROBE2(debug__user, tp, PRU_SLOWTIMO);
-	if (tp != NULL)
-		INP_WUNLOCK(tp->t_inpcb);
-	INP_INFO_RUNLOCK(&V_tcbinfo);
+	tcp_inpinfo_lock_del(inp, tp);
+out:
 	CURVNET_RESTORE();
 }
 
@@ -499,28 +527,23 @@ tcp_timer_persist(void *xtp)
 
 	ostate = tp->t_state;
 #endif
-	INP_INFO_RLOCK(&V_tcbinfo);
 	inp = tp->t_inpcb;
 	KASSERT(inp != NULL, ("%s: tp %p tp->t_inpcb == NULL", __func__, tp));
 	INP_WLOCK(inp);
 	if (callout_pending(&tp->t_timers->tt_persist) ||
 	    !callout_active(&tp->t_timers->tt_persist)) {
 		INP_WUNLOCK(inp);
-		INP_INFO_RUNLOCK(&V_tcbinfo);
 		CURVNET_RESTORE();
 		return;
 	}
 	callout_deactivate(&tp->t_timers->tt_persist);
 	if ((inp->inp_flags & INP_DROPPED) != 0) {
 		INP_WUNLOCK(inp);
-		INP_INFO_RUNLOCK(&V_tcbinfo);
 		CURVNET_RESTORE();
 		return;
 	}
 	KASSERT((tp->t_timers->tt_flags & TT_STOPPED) == 0,
 		("%s: tp %p tcpcb can't be stopped here", __func__, tp));
-	KASSERT((tp->t_timers->tt_flags & TT_PERSIST) != 0,
-		("%s: tp %p persist callout should be running", __func__, tp));
 	/*
 	 * Persistence timer into zero window.
 	 * Force a byte to be output, if possible.
@@ -537,7 +560,12 @@ tcp_timer_persist(void *xtp)
 	    (ticks - tp->t_rcvtime >= tcp_maxpersistidle ||
 	     ticks - tp->t_rcvtime >= TCP_REXMTVAL(tp) * tcp_totbackoff)) {
 		TCPSTAT_INC(tcps_persistdrop);
+		if (tcp_inpinfo_lock_add(inp)) {
+			tcp_inpinfo_lock_del(inp, tp);
+			goto out;
+		}
 		tp = tcp_drop(tp, ETIMEDOUT);
+		tcp_inpinfo_lock_del(inp, tp);
 		goto out;
 	}
 	/*
@@ -547,7 +575,12 @@ tcp_timer_persist(void *xtp)
 	if (tp->t_state > TCPS_CLOSE_WAIT &&
 	    (ticks - tp->t_rcvtime) >= TCPTV_PERSMAX) {
 		TCPSTAT_INC(tcps_persistdrop);
+		if (tcp_inpinfo_lock_add(inp)) {
+			tcp_inpinfo_lock_del(inp, tp);
+			goto out;
+		}
 		tp = tcp_drop(tp, ETIMEDOUT);
+		tcp_inpinfo_lock_del(inp, tp);
 		goto out;
 	}
 	tcp_setpersist(tp);
@@ -555,15 +588,13 @@ tcp_timer_persist(void *xtp)
 	(void) tp->t_fb->tfb_tcp_output(tp);
 	tp->t_flags &= ~TF_FORCEDATA;
 
-out:
 #ifdef TCPDEBUG
 	if (tp != NULL && tp->t_inpcb->inp_socket->so_options & SO_DEBUG)
 		tcp_trace(TA_USER, ostate, tp, NULL, NULL, PRU_SLOWTIMO);
 #endif
 	TCP_PROBE2(debug__user, tp, PRU_SLOWTIMO);
-	if (tp != NULL)
-		INP_WUNLOCK(inp);
-	INP_INFO_RUNLOCK(&V_tcbinfo);
+	INP_WUNLOCK(inp);
+out:
 	CURVNET_RESTORE();
 }
 
@@ -573,36 +604,29 @@ tcp_timer_rexmt(void * xtp)
 	struct tcpcb *tp = xtp;
 	CURVNET_SET(tp->t_vnet);
 	int rexmt;
-	int headlocked;
 	struct inpcb *inp;
 #ifdef TCPDEBUG
 	int ostate;
 
 	ostate = tp->t_state;
 #endif
-
-	INP_INFO_RLOCK(&V_tcbinfo);
 	inp = tp->t_inpcb;
 	KASSERT(inp != NULL, ("%s: tp %p tp->t_inpcb == NULL", __func__, tp));
 	INP_WLOCK(inp);
 	if (callout_pending(&tp->t_timers->tt_rexmt) ||
 	    !callout_active(&tp->t_timers->tt_rexmt)) {
 		INP_WUNLOCK(inp);
-		INP_INFO_RUNLOCK(&V_tcbinfo);
 		CURVNET_RESTORE();
 		return;
 	}
 	callout_deactivate(&tp->t_timers->tt_rexmt);
 	if ((inp->inp_flags & INP_DROPPED) != 0) {
 		INP_WUNLOCK(inp);
-		INP_INFO_RUNLOCK(&V_tcbinfo);
 		CURVNET_RESTORE();
 		return;
 	}
 	KASSERT((tp->t_timers->tt_flags & TT_STOPPED) == 0,
 		("%s: tp %p tcpcb can't be stopped here", __func__, tp));
-	KASSERT((tp->t_timers->tt_flags & TT_REXMT) != 0,
-		("%s: tp %p rexmt callout should be running", __func__, tp));
 	tcp_free_sackholes(tp);
 	if (tp->t_fb->tfb_tcp_rexmit_tmr) {
 		/* The stack has a timer action too. */
@@ -616,14 +640,15 @@ tcp_timer_rexmt(void * xtp)
 	if (++tp->t_rxtshift > TCP_MAXRXTSHIFT) {
 		tp->t_rxtshift = TCP_MAXRXTSHIFT;
 		TCPSTAT_INC(tcps_timeoutdrop);
-
+		if (tcp_inpinfo_lock_add(inp)) {
+			tcp_inpinfo_lock_del(inp, tp);
+			goto out;
+		}
 		tp = tcp_drop(tp, tp->t_softerror ?
 			      tp->t_softerror : ETIMEDOUT);
-		headlocked = 1;
+		tcp_inpinfo_lock_del(inp, tp);
 		goto out;
 	}
-	INP_INFO_RUNLOCK(&V_tcbinfo);
-	headlocked = 0;
 	if (tp->t_state == TCPS_SYN_SENT) {
 		/*
 		 * If the SYN was retransmitted, indicate CWND to be
@@ -811,17 +836,14 @@ tcp_timer_rexmt(void * xtp)
 
 	(void) tp->t_fb->tfb_tcp_output(tp);
 
-out:
 #ifdef TCPDEBUG
 	if (tp != NULL && (tp->t_inpcb->inp_socket->so_options & SO_DEBUG))
 		tcp_trace(TA_USER, ostate, tp, (void *)0, (struct tcphdr *)0,
 			  PRU_SLOWTIMO);
 #endif
 	TCP_PROBE2(debug__user, tp, PRU_SLOWTIMO);
-	if (tp != NULL)
-		INP_WUNLOCK(inp);
-	if (headlocked)
-		INP_INFO_RUNLOCK(&V_tcbinfo);
+	INP_WUNLOCK(inp);
+out:
 	CURVNET_RESTORE();
 }
 
@@ -832,7 +854,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)
@@ -846,27 +867,22 @@ 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:
 			if (tp->t_fb->tfb_tcp_timer_activate) {
@@ -876,24 +892,9 @@ tcp_timer_activate(struct tcpcb *tp, uin
 			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) > 0) &&
-		    (tp->t_timers->tt_flags & f_reset)) {
-			tp->t_timers->tt_flags &= ~(timer_type | f_reset);
-		}
+		callout_stop(t_callout);
 	} else {
-		if ((tp->t_timers->tt_flags & timer_type) == 0) {
-			tp->t_timers->tt_flags |= (timer_type | f_reset);
-			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_on(t_callout, delta, f_callout, tp, cpu);
 	}
 }
 
@@ -931,30 +932,23 @@ void
 tcp_timer_stop(struct tcpcb *tp, uint32_t timer_type)
 {
 	struct callout *t_callout;
-	uint32_t f_reset;
 
 	tp->t_timers->tt_flags |= TT_STOPPED;
-
 	switch (timer_type) {
 		case TT_DELACK:
 			t_callout = &tp->t_timers->tt_delack;
-			f_reset = TT_DELACK_RST;
 			break;
 		case TT_REXMT:
 			t_callout = &tp->t_timers->tt_rexmt;
-			f_reset = TT_REXMT_RST;
 			break;
 		case TT_PERSIST:
 			t_callout = &tp->t_timers->tt_persist;
-			f_reset = TT_PERSIST_RST;
 			break;
 		case TT_KEEP:
 			t_callout = &tp->t_timers->tt_keep;
-			f_reset = TT_KEEP_RST;
 			break;
 		case TT_2MSL:
 			t_callout = &tp->t_timers->tt_2msl;
-			f_reset = TT_2MSL_RST;
 			break;
 		default:
 			if (tp->t_fb->tfb_tcp_timer_stop) {
@@ -968,15 +962,13 @@ tcp_timer_stop(struct tcpcb *tp, uint32_
 			panic("tp %p bad timer_type %#x", tp, timer_type);
 		}
 
-	if (tp->t_timers->tt_flags & timer_type) {
-		if (callout_async_drain(t_callout, tcp_timer_discard) == 0) {
-			/*
-			 * Can't stop the callout, defer tcpcb actual deletion
-			 * to the last one. We do this using the async drain
-			 * function and incrementing the count in 
-			 */
-			tp->t_timers->tt_draincnt++;
-		}
+	if (callout_async_drain(t_callout, tcp_timer_discard) == 0) {
+		/*
+		 * Can't stop the callout, defer tcpcb actual deletion
+		 * to the last one. We do this using the async drain
+		 * function and incrementing the count in 
+		 */
+		tp->t_timers->tt_draincnt++;
 	}
 }
 

Modified: head/sys/netinet/tcp_timer.h
==============================================================================
--- head/sys/netinet/tcp_timer.h	Tue Aug 16 12:13:12 2016	(r304217)
+++ head/sys/netinet/tcp_timer.h	Tue Aug 16 12:40:56 2016	(r304218)
@@ -191,6 +191,9 @@ extern int tcp_syn_backoff[];
 extern int tcp_finwait2_timeout;
 extern int tcp_fast_finwait2_recycle;
 
+int tcp_inpinfo_lock_add(struct inpcb *inp);
+void tcp_inpinfo_lock_del(struct inpcb *inp, struct tcpcb *tp);
+
 void	tcp_timer_init(void);
 void	tcp_timer_2msl(void *xtp);
 void	tcp_timer_discard(void *);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201608161240.u7GCeuWS082118>