From nobody Sat Aug 03 22:47:15 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4WbyWR4sdPz5RTNj; Sat, 03 Aug 2024 22:47:15 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4WbyWR3z91z4Jkl; Sat, 3 Aug 2024 22:47:15 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1722725235; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Evwz4XfSh6Jexlg8AxIImyIJMmdNdgNO2XtQmWivWpM=; b=rG1ZHz7QK/vd6TjB7Rk/zsjLqZ9R8aIAM+3/E2fhankOIZW6okYAordo9uiQ+9rHhEAOO7 waobbeerBsaEBdDFyhQsMoB2raQOgUtWOUULRQMepkA2dW+rolLcD5VM/VYhhrgDPiFagt vEyXAg3663Os6YinVHGN7/0UaSeYmP496GV4mbeVhRvk7fhOxqdKBE6AdGNoCvsPtvzeb8 G9+9o272PE7CHT0FnFWBZatCvOAgYZlc+FVcfjm778CTJErxUVTsl/A37zCAd9aCdCkyxr 3nAB/MX3kXtd19T+brRicmZg/4P7nZVMjFG/X6aDUSiNV4HdwIVa60Y5fx0v/Q== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1722725235; a=rsa-sha256; cv=none; b=IwJOFEs/XLVxXyZjqEg+AHvVG9AegDl7mLn8HqFreqk3bxJQM0Psl3DaETmFtqhsAfOY3H 32evT4a8Jf0Cr6ixeLyoeNjVhpFEKuPsUDW+wrl/LMrEG5iw3RQ6TQtu7T2UwR9AYN23fp Xc2yRlvKkE7gVr07XA091yy2hHAtSlpIa8x29KdJ3ESsVaunsjLkwmV9nTayclusFf5p6g Qnky29KeAw30wHP2GMvAlWLIZEZWTkL8JTjIvNcxyJBO6i7dVoFS3S58OEnzuLCsZAZYri nFoiYOSVql5+Xa+X1aTjMeuUGl9bPmbkLQHlxOhFIxLH1C871Lb0k2UlrkXx7Q== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1722725235; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Evwz4XfSh6Jexlg8AxIImyIJMmdNdgNO2XtQmWivWpM=; b=VwrPr/uV37DCYmY63GflUwAli00OTJsW0mnaoDiZeXmsrFg6uN1FZOVQP/BLfe+qxWJT9+ K5+kRLhlx7vewWG5BBxd0bwipjquAm4Sd8CDBJnN5mgcYIB9CFRFkaFcUJWPGeaGb5a21S X6542RFICEVtDDWzw60WSl6HDFLpjJIAu9sk6jUtansKMPApjtf6LrxruvaNdZCmK8eKgr xlhvoBfvPT+e10EAGW2VP+2XmZIYeid4XgdCOWJ5xFj2hkA4SaY4CyqO/kjnQhqqil6YQo ezsPkcKMNw7Vd5bbr6tXqhKfvWwSMmZXcT4yriDEjP+jbh8PFOkN9wjBa6BbkQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4WbyWR3YXNzWCl; Sat, 3 Aug 2024 22:47:15 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 473MlFc4060351; Sat, 3 Aug 2024 22:47:15 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 473MlFpX060348; Sat, 3 Aug 2024 22:47:15 GMT (envelope-from git) Date: Sat, 3 Aug 2024 22:47:15 GMT Message-Id: <202408032247.473MlFpX060348@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Michael Tuexen Subject: git: 808fb2711f0c - stable/14 - tcp: simplify stack switching protocol List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: tuexen X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: 808fb2711f0cad41decf5b18419cb9294c5ac062 Auto-Submitted: auto-generated The branch stable/14 has been updated by tuexen: URL: https://cgit.FreeBSD.org/src/commit/?id=808fb2711f0cad41decf5b18419cb9294c5ac062 commit 808fb2711f0cad41decf5b18419cb9294c5ac062 Author: Michael Tuexen AuthorDate: 2024-06-06 06:29:05 +0000 Commit: Michael Tuexen CommitDate: 2024-08-03 22:45:32 +0000 tcp: simplify stack switching protocol Before this patch, a stack (tfb) accepts a tcpcb (tp), if the tp->t_state is TCPS_CLOSED or tfb->tfb_tcp_handoff_ok is not NULL and tfb->tfb_tcp_handoff_ok(tp) returns 0. After this patch, the only check is tfb->tfb_tcp_handoff_ok(tp) returns 0. tfb->tfb_tcp_handoff_ok must always be provided. For existing TCP stacks (FreeBSD, RACK and BBR) there is no functional change. However, the logic is simpler. Reviewed by: lstewart, peter_lei_ieee_.org, rrs Sponsored by: Netflix, Inc. Differential Revision: https://reviews.freebsd.org/D45253 (cherry picked from commit 86c9325d341fc3f39543bcfaf7c3bb3ceeacbe5d) --- share/man/man9/tcp_functions.9 | 45 ++++++++++++++---------------------------- sys/netinet/tcp_subr.c | 12 +++++------ sys/netinet/tcp_usrreq.c | 27 ++++++------------------- sys/netinet/tcp_var.h | 11 ++++------- 4 files changed, 30 insertions(+), 65 deletions(-) diff --git a/share/man/man9/tcp_functions.9 b/share/man/man9/tcp_functions.9 index eb9b299eae9e..1e0616e03a9f 100644 --- a/share/man/man9/tcp_functions.9 +++ b/share/man/man9/tcp_functions.9 @@ -23,7 +23,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd March 10, 2017 +.Dd June 6, 2024 .Dt TCP_FUNCTIONS 9 .Os .Sh NAME @@ -176,9 +176,10 @@ struct tcp_function_block { uint32_t, u_int); int (*tfb_tcp_timer_active)(struct tcpcb *, uint32_t); void (*tfb_tcp_timer_stop)(struct tcpcb *, uint32_t); - /* Optional functions */ + /* Optional function */ void (*tfb_tcp_rexmit_tmr)(struct tcpcb *); - void (*tfb_tcp_handoff_ok)(struct tcpcb *); + /* Mandatory function */ + int (*tfb_tcp_handoff_ok)(struct tcpcb *); /* System use */ volatile uint32_t tfb_refcnt; uint32_t tfb_flags; @@ -261,37 +262,21 @@ However, care must be taken to ensure the retransmit timer leaves the TCP control block in a valid state for the remainder of the retransmit timer logic. .Pp -A user may select a new TCP stack before calling -.Xr connect 2 -or -.Xr listen 2 . -Optionally, a TCP stack may also allow a user to begin using the TCP stack for -a connection that is in a later state by setting a non-NULL function pointer in -the +A user may select a new TCP stack before calling at any time. +Therefore, the function pointer .Va tfb_tcp_handoff_ok -field. -If this field is non-NULL and a user attempts to select that TCP stack after -calling -.Xr connect 2 -or -.Xr listen 2 -for that socket, the kernel will call the function pointed to by the +field must be non-NULL. +If a user attempts to select that TCP stack, the kernel will call the function +pointed to by the .Va tfb_tcp_handoff_ok field. The function should return 0 if the user is allowed to switch the socket to use -the TCP stack. -Otherwise, the function should return an error code, which will be returned to -the user. -If the -.Va tfb_tcp_handoff_ok -field is -.Dv NULL -and a user attempts to select the TCP stack after calling -.Xr connect 2 -or -.Xr listen 2 -for that socket, the operation will fail and the kernel will return -.Er EINVAL . +the TCP stack. In this case, the kernel will call the function pointed to by +.Va tfb_tcp_fb_init +if this function pointer is non-NULL and finally perform the stack switch. +If the user is not allowed to switch the socket, the function should undo any +changes it made to the connection state configuration and return an error code, +which will be returned to the user. .Pp The .Va tfb_refcnt diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c index 0ba35ea13130..afa2dc2a3a95 100644 --- a/sys/netinet/tcp_subr.c +++ b/sys/netinet/tcp_subr.c @@ -559,8 +559,7 @@ tcp_switch_back_to_default(struct tcpcb *tp) tfb = NULL; } /* Does the stack accept this connection? */ - if (tfb != NULL && tfb->tfb_tcp_handoff_ok != NULL && - (*tfb->tfb_tcp_handoff_ok)(tp)) { + if (tfb != NULL && (*tfb->tfb_tcp_handoff_ok)(tp)) { refcount_release(&tfb->tfb_refcnt); tfb = NULL; } @@ -594,11 +593,9 @@ tcp_switch_back_to_default(struct tcpcb *tp) /* there always should be a default */ panic("Can't refer to tcp_def_funcblk"); } - if (tfb->tfb_tcp_handoff_ok != NULL) { - if ((*tfb->tfb_tcp_handoff_ok) (tp)) { - /* The default stack cannot say no */ - panic("Default stack rejects a new session?"); - } + if ((*tfb->tfb_tcp_handoff_ok)(tp)) { + /* The default stack cannot say no */ + panic("Default stack rejects a new session?"); } if (tfb->tfb_tcp_fb_init != NULL && (*tfb->tfb_tcp_fb_init)(tp, &ptr)) { @@ -1229,6 +1226,7 @@ register_tcp_functions_as_names(struct tcp_function_block *blk, int wait, if ((blk->tfb_tcp_output == NULL) || (blk->tfb_tcp_do_segment == NULL) || (blk->tfb_tcp_ctloutput == NULL) || + (blk->tfb_tcp_handoff_ok == NULL) || (strlen(blk->tfb_tcp_block_name) == 0)) { /* * These functions are required and you diff --git a/sys/netinet/tcp_usrreq.c b/sys/netinet/tcp_usrreq.c index a76a185474df..4e754965487b 100644 --- a/sys/netinet/tcp_usrreq.c +++ b/sys/netinet/tcp_usrreq.c @@ -1724,32 +1724,17 @@ tcp_ctloutput_set(struct inpcb *inp, struct sockopt *sopt) INP_WUNLOCK(inp); return (0); } - if (tp->t_state != TCPS_CLOSED) { - /* - * The user has advanced the state - * past the initial point, we may not - * be able to switch. - */ - if (blk->tfb_tcp_handoff_ok != NULL) { - /* - * Does the stack provide a - * query mechanism, if so it may - * still be possible? - */ - error = (*blk->tfb_tcp_handoff_ok)(tp); - } else - error = EINVAL; - if (error) { - refcount_release(&blk->tfb_refcnt); - INP_WUNLOCK(inp); - return(error); - } - } if (blk->tfb_flags & TCP_FUNC_BEING_REMOVED) { refcount_release(&blk->tfb_refcnt); INP_WUNLOCK(inp); return (ENOENT); } + error = (*blk->tfb_tcp_handoff_ok)(tp); + if (error) { + refcount_release(&blk->tfb_refcnt); + INP_WUNLOCK(inp); + return (error); + } /* * Ensure the new stack takes ownership with a * clean slate on peak rate threshold. diff --git a/sys/netinet/tcp_var.h b/sys/netinet/tcp_var.h index 1dc40c790b3e..b8278bbf2644 100644 --- a/sys/netinet/tcp_var.h +++ b/sys/netinet/tcp_var.h @@ -548,13 +548,10 @@ typedef enum { * stop_all function that loops through and calls * tcp_timer_stop() with each of your defined timers. * - * Adding a tfb_tcp_handoff_ok function allows the socket - * option to change stacks to query you even if the - * connection is in a later stage. You return 0 to - * say you can take over and run your stack, you return - * non-zero (an error number) to say no you can't. - * If the function is undefined you can only change - * in the early states (before connect or listen). + * tfb_tcp_handoff_ok is a mandatory function allowing + * to query a stack, if it can take over a tcpcb. + * You return 0 to say you can take over and run your stack, + * you return non-zero (an error number) to say no you can't. * * tfb_tcp_fb_init is used to allow the new stack to * setup its control block. Among the things it must