git: 3f46be6acadd - main - tcp_hpts: let tcp_hpts_init() set a random CPU only once

From: Gleb Smirnoff <glebius_at_FreeBSD.org>
Date: Thu, 07 Dec 2023 22:42:14 UTC
The branch main has been updated by glebius:

URL: https://cgit.FreeBSD.org/src/commit/?id=3f46be6acadd5d660acde67d9d4c80137f424b70

commit 3f46be6acadd5d660acde67d9d4c80137f424b70
Author:     Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2023-12-07 22:41:43 +0000
Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2023-12-07 22:41:43 +0000

    tcp_hpts: let tcp_hpts_init() set a random CPU only once
    
    After d2ef52ef3dee the tcp_hpts_init() function can be called multiple
    times on a tcpcb if it is switched there and back between two TCP stacks.
    First, this makes existing assertion in tcp_hpts_init() incorrect.  Second,
    it creates possibility to change a randomly set t_hpts_cpu to a different
    random value, while a tcpcb is already in the HPTS wheel, triggering other
    assertions later in tcp_hptsi().
    
    The best approach here would be to work on the stacks to really clear a
    tcpcb out of HPTS wheel in tfb_tcp_fb_fini, draining the IHPTS_MOVING
    state.  But that's pretty intrusive change, so let's just get back to the
    old logic (pre d2ef52ef3dee) where t_hpts_cpu was set to a random value
    only once in a CPU lifetime and a newly switched stack inherits t_hpts_cpu
    from the previous stack.
    
    Reviewed by:            rrs, tuexen
    Differential Revision:  https://reviews.freebsd.org/D42946
    Reported-by:    syzbot+fab29fe1ab089c52998d@syzkaller.appspotmail.com
    Reported-by:    syzbot+ca5f2aa0fda15dcfe6d7@syzkaller.appspotmail.com
    Fixes:          2b3a77467dd3d74a7170f279fb25f9736b46ef8a
---
 sys/netinet/tcp_hpts.c | 16 ++++++++++++----
 sys/netinet/tcp_subr.c |  3 +++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/sys/netinet/tcp_hpts.c b/sys/netinet/tcp_hpts.c
index e88a3f24dfcc..f1b729c249c6 100644
--- a/sys/netinet/tcp_hpts.c
+++ b/sys/netinet/tcp_hpts.c
@@ -542,15 +542,23 @@ tcp_hpts_release(struct tcpcb *tp)
 }
 
 /*
- * Initialize newborn tcpcb to get ready for use with HPTS.
+ * Initialize tcpcb to get ready for use with HPTS.  We will know which CPU
+ * is preferred on the first incoming packet.  Before that avoid crowding
+ * a single CPU with newborn connections and use a random one.
+ * This initialization is normally called on a newborn tcpcb, but potentially
+ * can be called once again if stack is switched.  In that case we inherit CPU
+ * that the previous stack has set, be it random or not.  In extreme cases,
+ * e.g. syzkaller fuzzing, a tcpcb can already be in HPTS in IHPTS_MOVING state
+ * and has never received a first packet.
  */
 void
 tcp_hpts_init(struct tcpcb *tp)
 {
 
-	tp->t_hpts_cpu = hpts_random_cpu();
-	tp->t_lro_cpu = HPTS_CPU_NONE;
-	MPASS(!(tp->t_flags2 & TF2_HPTS_CPU_SET));
+	if (__predict_true(tp->t_hpts_cpu == HPTS_CPU_NONE)) {
+		tp->t_hpts_cpu = hpts_random_cpu();
+		MPASS(!(tp->t_flags2 & TF2_HPTS_CPU_SET));
+	}
 }
 
 /*
diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c
index c79cadd04944..be38280aef0a 100644
--- a/sys/netinet/tcp_subr.c
+++ b/sys/netinet/tcp_subr.c
@@ -2274,6 +2274,9 @@ tcp_newtcpcb(struct inpcb *inp)
 	/* All mbuf queue/ack compress flags should be off */
 	tcp_lro_features_off(tp);
 
+	tp->t_hpts_cpu = HPTS_CPU_NONE;
+	tp->t_lro_cpu = HPTS_CPU_NONE;
+
 	callout_init_rw(&tp->t_callout, &inp->inp_lock, CALLOUT_RETURNUNLOCKED);
 	for (int i = 0; i < TT_N; i++)
 		tp->t_timers[i] = SBT_MAX;