git: ff94500855c1 - main - Add tcp_freecb() - single place to free tcpcb.
Date: Fri, 19 Nov 2021 04:28:14 UTC
The branch main has been updated by glebius:
URL: https://cgit.FreeBSD.org/src/commit/?id=ff94500855c16d0d9cc18aa8b0ba73ea94020c56
commit ff94500855c16d0d9cc18aa8b0ba73ea94020c56
Author: Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2021-11-19 04:26:09 +0000
Commit: Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2021-11-19 04:27:45 +0000
Add tcp_freecb() - single place to free tcpcb.
Until this change there were two places where we would free tcpcb -
tcp_discardcb() in case if all timers are drained and tcp_timer_discard()
otherwise. They were pretty much copy-n-paste, except that in the
default case we would run tcp_hc_update(). Merge this into single
function tcp_freecb() and move new short version of tcp_timer_discard()
to tcp_timer.c and make it static.
Reviewed by: rrs, hselasky
Differential revision: https://reviews.freebsd.org/D32965
---
sys/netinet/tcp_subr.c | 180 +++++++++++++++++++++---------------------------
sys/netinet/tcp_timer.c | 23 +++++++
sys/netinet/tcp_timer.h | 1 -
sys/netinet/tcp_var.h | 1 +
4 files changed, 101 insertions(+), 104 deletions(-)
diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c
index 5563148a52ad..5fa16f43f28b 100644
--- a/sys/netinet/tcp_subr.c
+++ b/sys/netinet/tcp_subr.c
@@ -2392,11 +2392,6 @@ void
tcp_discardcb(struct tcpcb *tp)
{
struct inpcb *inp = tp->t_inpcb;
- struct socket *so = inp->inp_socket;
-#ifdef INET6
- int isipv6 = (inp->inp_vflag & INP_IPV6) != 0;
-#endif /* INET6 */
- int released __unused;
INP_WLOCK_ASSERT(inp);
@@ -2458,121 +2453,100 @@ tcp_discardcb(struct tcpcb *tp)
CC_ALGO(tp) = NULL;
inp->inp_ppcb = NULL;
if (tp->t_timers->tt_draincnt == 0) {
- /* We own the last reference on tcpcb, let's free it. */
+ bool released __diagused;
+
+ released = tcp_freecb(tp);
+ KASSERT(!released, ("%s: inp %p should not have been released "
+ "here", __func__, inp));
+ }
+}
+
+bool
+tcp_freecb(struct tcpcb *tp)
+{
+ struct inpcb *inp = tp->t_inpcb;
+ struct socket *so = inp->inp_socket;
+#ifdef INET6
+ bool isipv6 = (inp->inp_vflag & INP_IPV6) != 0;
+#endif
+
+ INP_WLOCK_ASSERT(inp);
+ MPASS(tp->t_timers->tt_draincnt == 0);
+
+ /* We own the last reference on tcpcb, let's free it. */
#ifdef TCP_BLACKBOX
- tcp_log_tcpcbfini(tp);
+ tcp_log_tcpcbfini(tp);
#endif
- TCPSTATES_DEC(tp->t_state);
- if (tp->t_fb->tfb_tcp_fb_fini)
- (*tp->t_fb->tfb_tcp_fb_fini)(tp, 1);
+ TCPSTATES_DEC(tp->t_state);
+ if (tp->t_fb->tfb_tcp_fb_fini)
+ (*tp->t_fb->tfb_tcp_fb_fini)(tp, 1);
+ /*
+ * If we got enough samples through the srtt filter,
+ * save the rtt and rttvar in the routing entry.
+ * 'Enough' is arbitrarily defined as 4 rtt samples.
+ * 4 samples is enough for the srtt filter to converge
+ * to within enough % of the correct value; fewer samples
+ * and we could save a bogus rtt. The danger is not high
+ * as tcp quickly recovers from everything.
+ * XXX: Works very well but needs some more statistics!
+ *
+ * XXXRRS: Updating must be after the stack fini() since
+ * that may be converting some internal representation of
+ * say srtt etc into the general one used by other stacks.
+ * Lets also at least protect against the so being NULL
+ * as RW stated below.
+ */
+ if ((tp->t_rttupdated >= 4) && (so != NULL)) {
+ struct hc_metrics_lite metrics;
+ uint32_t ssthresh;
+
+ bzero(&metrics, sizeof(metrics));
/*
- * If we got enough samples through the srtt filter,
- * save the rtt and rttvar in the routing entry.
- * 'Enough' is arbitrarily defined as 4 rtt samples.
- * 4 samples is enough for the srtt filter to converge
- * to within enough % of the correct value; fewer samples
- * and we could save a bogus rtt. The danger is not high
- * as tcp quickly recovers from everything.
- * XXX: Works very well but needs some more statistics!
+ * Update the ssthresh always when the conditions below
+ * are satisfied. This gives us better new start value
+ * for the congestion avoidance for new connections.
+ * ssthresh is only set if packet loss occurred on a session.
*
- * XXXRRS: Updating must be after the stack fini() since
- * that may be converting some internal representation of
- * say srtt etc into the general one used by other stacks.
- * Lets also at least protect against the so being NULL
- * as RW stated below.
+ * XXXRW: 'so' may be NULL here, and/or socket buffer may be
+ * being torn down. Ideally this code would not use 'so'.
*/
- if ((tp->t_rttupdated >= 4) && (so != NULL)) {
- struct hc_metrics_lite metrics;
- uint32_t ssthresh;
-
- bzero(&metrics, sizeof(metrics));
+ ssthresh = tp->snd_ssthresh;
+ if (ssthresh != 0 && ssthresh < so->so_snd.sb_hiwat / 2) {
/*
- * Update the ssthresh always when the conditions below
- * are satisfied. This gives us better new start value
- * for the congestion avoidance for new connections.
- * ssthresh is only set if packet loss occurred on a session.
- *
- * XXXRW: 'so' may be NULL here, and/or socket buffer may be
- * being torn down. Ideally this code would not use 'so'.
+ * convert the limit from user data bytes to
+ * packets then to packet data bytes.
*/
- ssthresh = tp->snd_ssthresh;
- if (ssthresh != 0 && ssthresh < so->so_snd.sb_hiwat / 2) {
- /*
- * convert the limit from user data bytes to
- * packets then to packet data bytes.
- */
- ssthresh = (ssthresh + tp->t_maxseg / 2) / tp->t_maxseg;
- if (ssthresh < 2)
- ssthresh = 2;
- ssthresh *= (tp->t_maxseg +
+ ssthresh = (ssthresh + tp->t_maxseg / 2) / tp->t_maxseg;
+ if (ssthresh < 2)
+ ssthresh = 2;
+ ssthresh *= (tp->t_maxseg +
#ifdef INET6
- (isipv6 ? sizeof (struct ip6_hdr) +
- sizeof (struct tcphdr) :
+ (isipv6 ? sizeof (struct ip6_hdr) +
+ sizeof (struct tcphdr) :
#endif
- sizeof (struct tcpiphdr)
+ sizeof (struct tcpiphdr)
#ifdef INET6
- )
+ )
#endif
- );
- } else
- ssthresh = 0;
- metrics.rmx_ssthresh = ssthresh;
+ );
+ } else
+ ssthresh = 0;
+ metrics.rmx_ssthresh = ssthresh;
- metrics.rmx_rtt = tp->t_srtt;
- metrics.rmx_rttvar = tp->t_rttvar;
- metrics.rmx_cwnd = tp->snd_cwnd;
- metrics.rmx_sendpipe = 0;
- metrics.rmx_recvpipe = 0;
+ metrics.rmx_rtt = tp->t_srtt;
+ metrics.rmx_rttvar = tp->t_rttvar;
+ metrics.rmx_cwnd = tp->snd_cwnd;
+ metrics.rmx_sendpipe = 0;
+ metrics.rmx_recvpipe = 0;
- tcp_hc_update(&inp->inp_inc, &metrics);
- }
- refcount_release(&tp->t_fb->tfb_refcnt);
- tp->t_inpcb = NULL;
- uma_zfree(V_tcpcb_zone, tp);
- released = in_pcbrele_wlocked(inp);
- KASSERT(!released, ("%s: inp %p should not have been released "
- "here", __func__, inp));
+ tcp_hc_update(&inp->inp_inc, &metrics);
}
-}
-void
-tcp_timer_discard(void *ptp)
-{
- struct inpcb *inp;
- struct tcpcb *tp;
- struct epoch_tracker et;
+ refcount_release(&tp->t_fb->tfb_refcnt);
+ uma_zfree(V_tcpcb_zone, tp);
- tp = (struct tcpcb *)ptp;
- CURVNET_SET(tp->t_vnet);
- NET_EPOCH_ENTER(et);
- inp = tp->t_inpcb;
- KASSERT(inp != NULL, ("%s: tp %p tp->t_inpcb == NULL",
- __func__, tp));
- INP_WLOCK(inp);
- KASSERT((tp->t_timers->tt_flags & TT_STOPPED) != 0,
- ("%s: tcpcb has to be stopped here", __func__));
- tp->t_timers->tt_draincnt--;
- if (tp->t_timers->tt_draincnt == 0) {
- /* We own the last reference on this tcpcb, let's free it. */
-#ifdef TCP_BLACKBOX
- tcp_log_tcpcbfini(tp);
-#endif
- TCPSTATES_DEC(tp->t_state);
- if (tp->t_fb->tfb_tcp_fb_fini)
- (*tp->t_fb->tfb_tcp_fb_fini)(tp, 1);
- refcount_release(&tp->t_fb->tfb_refcnt);
- tp->t_inpcb = NULL;
- uma_zfree(V_tcpcb_zone, tp);
- if (in_pcbrele_wlocked(inp)) {
- NET_EPOCH_EXIT(et);
- CURVNET_RESTORE();
- return;
- }
- }
- INP_WUNLOCK(inp);
- NET_EPOCH_EXIT(et);
- CURVNET_RESTORE();
+ return (in_pcbrele_wlocked(inp));
}
/*
diff --git a/sys/netinet/tcp_timer.c b/sys/netinet/tcp_timer.c
index feea3765821c..67e550b83bce 100644
--- a/sys/netinet/tcp_timer.c
+++ b/sys/netinet/tcp_timer.c
@@ -1042,6 +1042,29 @@ tcp_timers_unsuspend(struct tcpcb *tp, uint32_t timer_type)
}
}
+static void
+tcp_timer_discard(void *ptp)
+{
+ struct inpcb *inp;
+ struct tcpcb *tp;
+ struct epoch_tracker et;
+
+ tp = (struct tcpcb *)ptp;
+ CURVNET_SET(tp->t_vnet);
+ NET_EPOCH_ENTER(et);
+ inp = tp->t_inpcb;
+ KASSERT(inp != NULL, ("%s: tp %p tp->t_inpcb == NULL",
+ __func__, tp));
+ INP_WLOCK(inp);
+ KASSERT((tp->t_timers->tt_flags & TT_STOPPED) != 0,
+ ("%s: tcpcb has to be stopped here", __func__));
+ if (--tp->t_timers->tt_draincnt > 0 ||
+ tcp_freecb(tp) == false)
+ INP_WUNLOCK(inp);
+ NET_EPOCH_EXIT(et);
+ CURVNET_RESTORE();
+}
+
void
tcp_timer_stop(struct tcpcb *tp, uint32_t timer_type)
{
diff --git a/sys/netinet/tcp_timer.h b/sys/netinet/tcp_timer.h
index 9a711d173386..c5317d1a4155 100644
--- a/sys/netinet/tcp_timer.h
+++ b/sys/netinet/tcp_timer.h
@@ -217,7 +217,6 @@ 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 *);
struct tcptw *
tcp_tw_2msl_scan(int reuse); /* XXX temporary? */
void tcp_timer_keep(void *xtp);
diff --git a/sys/netinet/tcp_var.h b/sys/netinet/tcp_var.h
index 1511da3c70fd..46d00914354f 100644
--- a/sys/netinet/tcp_var.h
+++ b/sys/netinet/tcp_var.h
@@ -967,6 +967,7 @@ int tcp_ccalgounload(struct cc_algo *unload_algo);
struct tcpcb *
tcp_close(struct tcpcb *);
void tcp_discardcb(struct tcpcb *);
+bool tcp_freecb(struct tcpcb *);
void tcp_twstart(struct tcpcb *);
void tcp_twclose(struct tcptw *, int);
void tcp_ctlinput(int, struct sockaddr *, void *);