Date: Thu, 8 Feb 2024 17:01:07 GMT From: Gleb Smirnoff <glebius@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 0bd8eb3e08d9 - main - unix: retire LOCAL_CONNWAIT Message-ID: <202402081701.418H170V065823@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by glebius: URL: https://cgit.FreeBSD.org/src/commit/?id=0bd8eb3e08d9720ee3814b29da16a9fd0044c83f commit 0bd8eb3e08d9720ee3814b29da16a9fd0044c83f Author: Gleb Smirnoff <glebius@FreeBSD.org> AuthorDate: 2024-02-08 17:00:23 +0000 Commit: Gleb Smirnoff <glebius@FreeBSD.org> CommitDate: 2024-02-08 17:00:41 +0000 unix: retire LOCAL_CONNWAIT This socket option was added in 6a2989fd54a9 together with LOCAL_CREDS. Both options originate from NetBSD. The LOCAL_CREDS seems to be used by some software and is covered by our test suite. The main problem with LOCAL_CONNWAIT is that it doesn't work as documented. A basic test shows that connect(2) indeed blocks, but accept(2) on the other side does not wake it up. Indeed, I don't see what code in the accept(2) path would go into the peer socket of a unix/stream listener's child and would make wakeup(&so->so_timeo). I tried the test even on a FreeBSD 6.4-RELEASE and it produced the same results as on CURRENT. The other thing that puzzles me is why that option would be useful even if it worked? Because on unix/stream you can send(2) immediately after connect(2) and that would put data on the peer receive buffer even before listener had done accept(2). In other words, one side can do connect(2) then send(2), only after the remote side would make accept(2) and the remote would see the data sent before the accept(2). Again this undocumented feature of unix(4) is present on all versions from FreeBSD 6 to CURRENT. Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D43708 --- share/man/man4/unix.4 | 10 +--------- sys/kern/uipc_usrreq.c | 30 +++++------------------------- sys/sys/un.h | 1 - sys/sys/unpcb.h | 2 -- 4 files changed, 6 insertions(+), 37 deletions(-) diff --git a/share/man/man4/unix.4 b/share/man/man4/unix.4 index 24069ae4663c..5ac9ccd5514f 100644 --- a/share/man/man4/unix.4 +++ b/share/man/man4/unix.4 @@ -25,7 +25,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd June 24, 2022 +.Dd February 1, 2022 .Dt UNIX 4 .Os .Sh NAME @@ -332,14 +332,6 @@ The and .Dv LOCAL_CREDS_PERSISTENT options are mutually exclusive. -.It Dv LOCAL_CONNWAIT -Used with -.Dv SOCK_STREAM -sockets, this option causes the -.Xr connect 2 -function to block until -.Xr accept 2 -has been called on the listening socket. .It Dv LOCAL_PEERCRED Requested via .Xr getsockopt 2 diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c index 80458cd6a4fe..db226a16674e 100644 --- a/sys/kern/uipc_usrreq.c +++ b/sys/kern/uipc_usrreq.c @@ -291,8 +291,7 @@ static int unp_connect(struct socket *, struct sockaddr *, struct thread *); static int unp_connectat(int, struct socket *, struct sockaddr *, struct thread *, bool); -typedef enum { PRU_CONNECT, PRU_CONNECT2 } conn2_how; -static void unp_connect2(struct socket *so, struct socket *so2, conn2_how); +static void unp_connect2(struct socket *so, struct socket *so2); static void unp_disconnect(struct unpcb *unp, struct unpcb *unp2); static void unp_dispose(struct socket *so); static void unp_shutdown(struct unpcb *); @@ -704,7 +703,7 @@ uipc_connect2(struct socket *so1, struct socket *so2) unp2 = so2->so_pcb; KASSERT(unp2 != NULL, ("uipc_connect2: unp2 == NULL")); unp_pcb_lock_pair(unp, unp2); - unp_connect2(so1, so2, PRU_CONNECT2); + unp_connect2(so1, so2); unp_pcb_unlock_pair(unp, unp2); return (0); @@ -1784,12 +1783,6 @@ uipc_ctloutput(struct socket *so, struct sockopt *sopt) error = sooptcopyout(sopt, &optval, sizeof(optval)); break; - case LOCAL_CONNWAIT: - /* Unlocked read. */ - optval = unp->unp_flags & UNP_CONNWAIT ? 1 : 0; - error = sooptcopyout(sopt, &optval, sizeof(optval)); - break; - default: error = EOPNOTSUPP; break; @@ -1800,7 +1793,6 @@ uipc_ctloutput(struct socket *so, struct sockopt *sopt) switch (sopt->sopt_name) { case LOCAL_CREDS: case LOCAL_CREDS_PERSISTENT: - case LOCAL_CONNWAIT: error = sooptcopyin(sopt, &optval, sizeof(optval), sizeof(optval)); if (error) @@ -1829,10 +1821,6 @@ uipc_ctloutput(struct socket *so, struct sockopt *sopt) OPTSET(UNP_WANTCRED_ALWAYS, UNP_WANTCRED_ONESHOT); break; - case LOCAL_CONNWAIT: - OPTSET(UNP_CONNWAIT, 0); - break; - default: break; } @@ -2006,7 +1994,7 @@ unp_connectat(int fd, struct socket *so, struct sockaddr *nam, KASSERT(unp2 != NULL && so2 != NULL && unp2->unp_socket == so2 && sotounpcb(so2) == unp2, ("%s: unp2 %p so2 %p", __func__, unp2, so2)); - unp_connect2(so, so2, PRU_CONNECT); + unp_connect2(so, so2); KASSERT((unp->unp_flags & UNP_CONNECTING) != 0, ("%s: unp %p has UNP_CONNECTING clear", __func__, unp)); unp->unp_flags &= ~UNP_CONNECTING; @@ -2057,7 +2045,7 @@ unp_copy_peercred(struct thread *td, struct unpcb *client_unp, } static void -unp_connect2(struct socket *so, struct socket *so2, conn2_how req) +unp_connect2(struct socket *so, struct socket *so2) { struct unpcb *unp; struct unpcb *unp2; @@ -2089,11 +2077,7 @@ unp_connect2(struct socket *so, struct socket *so2, conn2_how req) KASSERT(unp2->unp_conn == NULL, ("%s: socket %p is already connected", __func__, unp2)); unp2->unp_conn = unp; - if (req == PRU_CONNECT && - ((unp->unp_flags | unp2->unp_flags) & UNP_CONNWAIT)) - soisconnecting(so); - else - soisconnected(so); + soisconnected(so); soisconnected(so2); break; @@ -3493,10 +3477,6 @@ db_print_unpflags(int unp_flags) db_printf("%sUNP_WANTCRED_ONESHOT", comma ? ", " : ""); comma = 1; } - if (unp_flags & UNP_CONNWAIT) { - db_printf("%sUNP_CONNWAIT", comma ? ", " : ""); - comma = 1; - } if (unp_flags & UNP_CONNECTING) { db_printf("%sUNP_CONNECTING", comma ? ", " : ""); comma = 1; diff --git a/sys/sys/un.h b/sys/sys/un.h index 640bd254d579..41bde1701900 100644 --- a/sys/sys/un.h +++ b/sys/sys/un.h @@ -65,7 +65,6 @@ struct sockaddr_un { #define LOCAL_PEERCRED 1 /* retrieve peer credentials */ #define LOCAL_CREDS 2 /* pass credentials to receiver */ #define LOCAL_CREDS_PERSISTENT 3 /* pass credentials to receiver */ -#define LOCAL_CONNWAIT 4 /* connects block until accepted */ /* Start of reserved space for third-party socket options. */ #define LOCAL_VENDOR SO_VENDOR diff --git a/sys/sys/unpcb.h b/sys/sys/unpcb.h index fe701d5d38b1..d22662fe83e5 100644 --- a/sys/sys/unpcb.h +++ b/sys/sys/unpcb.h @@ -107,8 +107,6 @@ struct unpcb { #define UNP_HAVEPC 0x001 #define UNP_WANTCRED_ALWAYS 0x002 /* credentials wanted always */ #define UNP_WANTCRED_ONESHOT 0x004 /* credentials wanted once */ -#define UNP_CONNWAIT 0x008 /* connect blocks until accepted */ - #define UNP_WANTCRED_MASK (UNP_WANTCRED_ONESHOT | UNP_WANTCRED_ALWAYS) /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202402081701.418H170V065823>