Date: Wed, 24 Feb 2021 04:52:39 GMT From: Kyle Evans <kevans@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 5e57a3806ef3 - stable/12 - 504ebd612ec: kern: sonewconn: set so_options before pru_attach() Message-ID: <202102240452.11O4qdbq008160@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=5e57a3806ef3dc9d04981b1bc06f69507cc67684 commit 5e57a3806ef3dc9d04981b1bc06f69507cc67684 Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2021-01-20 17:53:05 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2021-02-24 04:52:18 +0000 504ebd612ec: kern: sonewconn: set so_options before pru_attach() Protocol attachment has historically been able to observe and modify so->so_options as needed, and it still can for newly created sockets. 779f106aa169 moved this to after pru_attach() when we re-acquire the lock on the listening socket. Restore the historical behavior so that pru_attach implementations can consistently use it. Note that some pru_attach() do currently rely on this, though that may change in the future. D28265 contains a change to remove the use in TCP and IB/SDP bits, as resetting the requested linger time on incoming connections seems questionable at best. This does move the assignment out from under the head's listen lock, but glebius notes that head won't be going away and applications cannot assume any specific ordering with a race between a connection coming in and the application changing socket options anyways. 4c0bef07be0: kern: net: remove TCP_LINGERTIME TCP_LINGERTIME can be traced back to BSD 4.4 Lite and perhaps beyond, in exactly the same form that it appears here modulo slightly different context. It used to be the case that there was a single pr_usrreq method with requests dispatched to it; these exact two lines appeared in tcp_usrreq's PRU_ATTACH handling. The only purpose of this that I can find is to cause surprising behavior on accepted connections. Newly-created sockets will never hit these paths as one cannot set SO_LINGER prior to socket(2). If SO_LINGER is set on a listening socket and inherited, one would expect the timeout to be inherited rather than changed arbitrarily like this -- noting that SO_LINGER is nonsense on a listening socket beyond inheritance, since they cannot be 'connected' by definition. Neither Illumos nor Linux reset the timer like this based on testing and inspection of Illumos, and testing of Linux. (cherry picked from commit 504ebd612ec61165bb949cfce3a348b0d6f37008) (cherry picked from commit 4c0bef07be071a1633ebc86a653f9bd59d40796e) --- sys/kern/uipc_socket.c | 2 +- sys/netinet/tcp_timer.h | 2 -- sys/netinet/tcp_usrreq.c | 3 --- sys/ofed/drivers/infiniband/ulp/sdp/sdp_main.c | 2 -- 4 files changed, 1 insertion(+), 8 deletions(-) diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c index ba03de4c32fe..ab05da880e46 100644 --- a/sys/kern/uipc_socket.c +++ b/sys/kern/uipc_socket.c @@ -616,6 +616,7 @@ sonewconn(struct socket *head, int connstatus) } so->so_listen = head; so->so_type = head->so_type; + so->so_options = head->so_options & ~SO_ACCEPTCONN; so->so_linger = head->so_linger; so->so_state = head->so_state | SS_NOFDREF; so->so_fibnum = head->so_fibnum; @@ -652,7 +653,6 @@ sonewconn(struct socket *head, int connstatus) if (head->sol_accept_filter != NULL) connstatus = 0; so->so_state |= connstatus; - so->so_options = head->so_options & ~SO_ACCEPTCONN; soref(head); /* A socket on (in)complete queue refs head. */ if (connstatus) { TAILQ_INSERT_TAIL(&head->sol_comp, so, so_list); diff --git a/sys/netinet/tcp_timer.h b/sys/netinet/tcp_timer.h index 1f73a806eb7a..b95fcf52861b 100644 --- a/sys/netinet/tcp_timer.h +++ b/sys/netinet/tcp_timer.h @@ -115,8 +115,6 @@ #define TCPTV_TWTRUNC 8 /* RTO factor to truncate TW */ -#define TCP_LINGERTIME 120 /* linger at most 2 minutes */ - #define TCP_MAXRXTSHIFT 12 /* maximum retransmits */ #define TCPTV_DELACK ( hz/25 ) /* 40ms timeout */ diff --git a/sys/netinet/tcp_usrreq.c b/sys/netinet/tcp_usrreq.c index 71c35c1bed66..968b1f4c5e30 100644 --- a/sys/netinet/tcp_usrreq.c +++ b/sys/netinet/tcp_usrreq.c @@ -163,9 +163,6 @@ tcp_usr_attach(struct socket *so, int proto, struct thread *td) if (error) goto out; - if ((so->so_options & SO_LINGER) && so->so_linger == 0) - so->so_linger = TCP_LINGERTIME; - inp = sotoinpcb(so); tp = intotcpcb(inp); out: diff --git a/sys/ofed/drivers/infiniband/ulp/sdp/sdp_main.c b/sys/ofed/drivers/infiniband/ulp/sdp/sdp_main.c index 5fefa2929b12..ba6d822c38e8 100644 --- a/sys/ofed/drivers/infiniband/ulp/sdp/sdp_main.c +++ b/sys/ofed/drivers/infiniband/ulp/sdp/sdp_main.c @@ -481,8 +481,6 @@ sdp_attach(struct socket *so, int proto, struct thread *td) LIST_INSERT_HEAD(&sdp_list, ssk, list); sdp_count++; SDP_LIST_WUNLOCK(); - if ((so->so_options & SO_LINGER) && so->so_linger == 0) - so->so_linger = TCP_LINGERTIME; return (0); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202102240452.11O4qdbq008160>