From nobody Thu Jun 6 06:33:00 2024 X-Original-To: dev-commits-src-main@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 4Vvvf50fZyz5LPPK; Thu, 06 Jun 2024 06:33:01 +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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Vvvf5088dz43B3; Thu, 6 Jun 2024 06:33:01 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1717655581; 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=O9G1YZszObCKa8zsgIMqzJNuWsc7VTPajixynjstB+A=; b=RqPnwFR4H6VRO5Jqa8l+gThRNqaQOpQH0jENHmAX0pPdIT3qYkJBXDAjM/fIVlRAIEWZg5 b3ZN4mzlDvEctEUsoadLsS5wn0YDzh+2dERhVgB7gGV3gRRD5pFsPwF69nuceNQx/KIl+c J3D/oCYIpqQxPiXY0OEnlNpMjru9wkqrLg0Av/3IolkHpuMGWdXnrbu8bNryMkYO7Tg/tR Ma29pnOG20k4YmCrgLMfKr7H6Ce48kAOuM95IYGWPtOCOWLXAS9+LzyWd1zZANHHenVKWY FJBD2SQ2fntiZuCgUre23DQyHnbNgBJNaeRpYry2dwKaLafYORgoyrtR939xVg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1717655581; a=rsa-sha256; cv=none; b=UmbvkIdPVHp4fEhWBz5u8Z3Fx1U5mO75MjTzSICH0wd1gCcfRkaeoteZy7K0VNRQTsHtOL ikFB3MYs41Jlb7qJ1bREd0FFuhYtPwWqNS93C2hTdjUeoNPU41lTpTo+nk6LvAysMm9lnw /bH+VOfYHCmLnrCF+ShDwyJck+L2xY+y3b7J0vchFse3gpayPdZZyRntYATs5hWM1zp6WD d2n8wHxvJyy3rxF4u6YCqfz8Ocmz/Vz/vSdNdPv0hj3uA27/qKJ+9Ve3v7stpUxoE0aQNM qeCZp0yGGQAH8vJmbY7ly8/XQKQjRyEVP4Gqyp9hFORCrF1X5gu6kvts30FEeQ== 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=1717655581; 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=O9G1YZszObCKa8zsgIMqzJNuWsc7VTPajixynjstB+A=; b=bOqw21KI+jUrVEBEcmX/IQ7v5DQb9BFfqc8N5MP/kjz/hezlRKBev7uBSemAWRBnp+z4cY ptdii/OMapnYaizI+zEcxcyfbRYxanpnZG6ZZzccrmd434yKMqlJpcMjsCVra0gkADEJo+ xzCF9ck4yCMsOimB/8cCDuop0BhLGL14VVWEgMPH9eqp4F0t+WheI8Cmt7nxceLHsEN5cH 2wgRGRECJ+Z2lyVN3FFlKrxt9+vZWdEMjh5fpL4YTX75h2I/2R0a9xW+uRBMSRf968oBWR j7eUNT5eVFC0fpoz/WFpr/vNah2SqDYIKcNPcoa2i66F+hAsfHZyr9FliNahvw== 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 4Vvvf46s1CzbYd; Thu, 6 Jun 2024 06:33:00 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 4566X0pH077020; Thu, 6 Jun 2024 06:33:00 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 4566X0ZB077017; Thu, 6 Jun 2024 06:33:00 GMT (envelope-from git) Date: Thu, 6 Jun 2024 06:33:00 GMT Message-Id: <202406060633.4566X0ZB077017@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Michael Tuexen Subject: git: 86c9325d341f - main - tcp: simplify stack switching protocol List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@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/main X-Git-Reftype: branch X-Git-Commit: 86c9325d341fc3f39543bcfaf7c3bb3ceeacbe5d Auto-Submitted: auto-generated The branch main has been updated by tuexen: URL: https://cgit.FreeBSD.org/src/commit/?id=86c9325d341fc3f39543bcfaf7c3bb3ceeacbe5d commit 86c9325d341fc3f39543bcfaf7c3bb3ceeacbe5d Author: Michael Tuexen AuthorDate: 2024-06-06 06:29:05 +0000 Commit: Michael Tuexen CommitDate: 2024-06-06 06:29:05 +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 MFC after: 1 week Sponsored by: Netflix, Inc. Differential Revision: https://reviews.freebsd.org/D45253 --- 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 7259d3607869..b871d8416b19 100644 --- a/sys/netinet/tcp_subr.c +++ b/sys/netinet/tcp_subr.c @@ -516,8 +516,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; } @@ -551,11 +550,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)) { @@ -1186,6 +1183,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 f768f42114d4..3bc283c5a9db 100644 --- a/sys/netinet/tcp_usrreq.c +++ b/sys/netinet/tcp_usrreq.c @@ -1731,32 +1731,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 3fdc1f4a9d74..e81ebf301c8e 100644 --- a/sys/netinet/tcp_var.h +++ b/sys/netinet/tcp_var.h @@ -542,13 +542,10 @@ typedef enum { #define TCP_FUNC_OUTPUT_CANDROP 0x02 /* tfb_tcp_output may ask tcp_drop */ /** - * 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