| Message ID | 169336886172.5133.14231863738996844866@noble.neil.brown.name (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PATCH/RFC] SUNRPC: always clear XPRT_SOCK_CONNECTING before xprt_clear_connecting on TCP xprt | expand |
On Wed, Aug 30, 2023 at 02:14:21PM +1000, NeilBrown wrote: > > sunrpc/xprtsock.c has an XPRT_SOCK_CONNECTING flag which is used only on > TCP xprts while connecting. > The purpose appears to be to protect against delayed TCP_CLOSE > transitions from calling xprt_clear_connecting() asynchronously. > > Normally it is set after calling xprt_set_connecting() and before > calling kernel_connect(), and cleared before calling > xprt_clear_connecting(). It is only tested on a state change to > TCP_CLOSE. > > Unfortunately there is one time that it is *not* explicitly cleared > before calling xprt_clear_connecting(). I don't know what all to > consequences of this are. It may well relate to the underlying problem > that resulted in > Commit 3be232f11a3c ("SUNRPC: Prevent immediate close+reconnect") > That close/reconnect pattern can happen if a TCP_CLOSE is seen > while XPRT_SOCK_CONNECTING is set but shouldn't be. > > local and udp connections don't use XPRT_SOCK_CONNECTING. > > tcp_tls connection don't *appear* to set the flag but do sometimes clear > it. Note: At this early stage, don't assume that code is 100% complete and consistent. ;-) > I don't understand all of the tls code, so I added what might be a > missing clear of XPRT_SOCK_CONNECTING there. > > Signed-off-by: NeilBrown <neilb@suse.de> > --- > net/sunrpc/xprtsock.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c > index 268a2cc61acd..9426b1051b09 100644 > --- a/net/sunrpc/xprtsock.c > +++ b/net/sunrpc/xprtsock.c > @@ -2425,6 +2425,7 @@ static void xs_tcp_setup_socket(struct work_struct *work) > */ > xprt_wake_pending_tasks(xprt, status); > xs_tcp_force_close(xprt); > + clear_bit(XPRT_SOCK_CONNECTING, &transport->sock_state); > out: > xprt_clear_connecting(xprt); > out_unlock: > @@ -2689,6 +2690,7 @@ static void xs_tcp_tls_setup_socket(struct work_struct *work) > */ > xprt_wake_pending_tasks(upper_xprt, status); > xs_tcp_force_close(upper_xprt); > + clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state); > xprt_clear_connecting(upper_xprt); > goto out_unlock; > } > -- > 2.41.0 >
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c index 268a2cc61acd..9426b1051b09 100644 --- a/net/sunrpc/xprtsock.c +++ b/net/sunrpc/xprtsock.c @@ -2425,6 +2425,7 @@ static void xs_tcp_setup_socket(struct work_struct *work) */ xprt_wake_pending_tasks(xprt, status); xs_tcp_force_close(xprt); + clear_bit(XPRT_SOCK_CONNECTING, &transport->sock_state); out: xprt_clear_connecting(xprt); out_unlock: @@ -2689,6 +2690,7 @@ static void xs_tcp_tls_setup_socket(struct work_struct *work) */ xprt_wake_pending_tasks(upper_xprt, status); xs_tcp_force_close(upper_xprt); + clear_bit(XPRT_SOCK_CONNECTING, &upper_transport->sock_state); xprt_clear_connecting(upper_xprt); goto out_unlock; }
sunrpc/xprtsock.c has an XPRT_SOCK_CONNECTING flag which is used only on TCP xprts while connecting. The purpose appears to be to protect against delayed TCP_CLOSE transitions from calling xprt_clear_connecting() asynchronously. Normally it is set after calling xprt_set_connecting() and before calling kernel_connect(), and cleared before calling xprt_clear_connecting(). It is only tested on a state change to TCP_CLOSE. Unfortunately there is one time that it is *not* explicitly cleared before calling xprt_clear_connecting(). I don't know what all to consequences of this are. It may well relate to the underlying problem that resulted in Commit 3be232f11a3c ("SUNRPC: Prevent immediate close+reconnect") That close/reconnect pattern can happen if a TCP_CLOSE is seen while XPRT_SOCK_CONNECTING is set but shouldn't be. local and udp connections don't use XPRT_SOCK_CONNECTING. tcp_tls connection don't *appear* to set the flag but do sometimes clear it. I don't understand all of the tls code, so I added what might be a missing clear of XPRT_SOCK_CONNECTING there. Signed-off-by: NeilBrown <neilb@suse.de> --- net/sunrpc/xprtsock.c | 2 ++ 1 file changed, 2 insertions(+)