2021-10-29 20:12:08

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 1/4] SUNRPC: Fix races when closing the socket

From: Trond Myklebust <[email protected]>

Ensure that we bump the xprt->connect_cookie when we set the
XPRT_CLOSE_WAIT flag so that another call to
xprt_conditional_disconnect() won't race with the reconnection.

Signed-off-by: Trond Myklebust <[email protected]>
---
net/sunrpc/xprt.c | 2 ++
net/sunrpc/xprtsock.c | 1 +
2 files changed, 3 insertions(+)

diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 48560188e84d..691fe5a682b6 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -735,6 +735,8 @@ static void xprt_autoclose(struct work_struct *work)
unsigned int pflags = memalloc_nofs_save();

trace_xprt_disconnect_auto(xprt);
+ xprt->connect_cookie++;
+ smp_mb__before_atomic();
clear_bit(XPRT_CLOSE_WAIT, &xprt->state);
xprt->ops->close(xprt);
xprt_release_write(xprt, NULL);
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 04f1b78bcbca..b18d13479104 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -1134,6 +1134,7 @@ static void xs_run_error_worker(struct sock_xprt *transport, unsigned int nr)

static void xs_sock_reset_connection_flags(struct rpc_xprt *xprt)
{
+ xprt->connect_cookie++;
smp_mb__before_atomic();
clear_bit(XPRT_CLOSE_WAIT, &xprt->state);
clear_bit(XPRT_CLOSING, &xprt->state);
--
2.31.1


2021-10-29 20:12:08

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 2/4] SUNRPC: Replace use of socket sk_callback_lock with sock_lock

From: Trond Myklebust <[email protected]>

Since we do things like setting flags, etc it really is more appropriate
to use sock_lock().

Signed-off-by: Trond Myklebust <[email protected]>
---
net/sunrpc/xprtsock.c | 38 +++++++++++---------------------------
1 file changed, 11 insertions(+), 27 deletions(-)

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index b18d13479104..291b63136c08 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -1154,14 +1154,13 @@ static void xs_error_report(struct sock *sk)
struct sock_xprt *transport;
struct rpc_xprt *xprt;

- read_lock_bh(&sk->sk_callback_lock);
if (!(xprt = xprt_from_sock(sk)))
- goto out;
+ return;

transport = container_of(xprt, struct sock_xprt, xprt);
transport->xprt_err = -sk->sk_err;
if (transport->xprt_err == 0)
- goto out;
+ return;
dprintk("RPC: xs_error_report client %p, error=%d...\n",
xprt, -transport->xprt_err);
trace_rpc_socket_error(xprt, sk->sk_socket, transport->xprt_err);
@@ -1169,8 +1168,6 @@ static void xs_error_report(struct sock *sk)
/* barrier ensures xprt_err is set before XPRT_SOCK_WAKE_ERROR */
smp_mb__before_atomic();
xs_run_error_worker(transport, XPRT_SOCK_WAKE_ERROR);
- out:
- read_unlock_bh(&sk->sk_callback_lock);
}

static void xs_reset_transport(struct sock_xprt *transport)
@@ -1189,7 +1186,7 @@ static void xs_reset_transport(struct sock_xprt *transport)
kernel_sock_shutdown(sock, SHUT_RDWR);

mutex_lock(&transport->recv_mutex);
- write_lock_bh(&sk->sk_callback_lock);
+ lock_sock(sk);
transport->inet = NULL;
transport->sock = NULL;
transport->file = NULL;
@@ -1198,10 +1195,10 @@ static void xs_reset_transport(struct sock_xprt *transport)

xs_restore_old_callbacks(transport, sk);
xprt_clear_connected(xprt);
- write_unlock_bh(&sk->sk_callback_lock);
xs_sock_reset_connection_flags(xprt);
/* Reset stream record info */
xs_stream_reset_connect(transport);
+ release_sock(sk);
mutex_unlock(&transport->recv_mutex);

trace_rpc_socket_close(xprt, sock);
@@ -1365,7 +1362,6 @@ static void xs_data_ready(struct sock *sk)
{
struct rpc_xprt *xprt;

- read_lock_bh(&sk->sk_callback_lock);
dprintk("RPC: xs_data_ready...\n");
xprt = xprt_from_sock(sk);
if (xprt != NULL) {
@@ -1380,7 +1376,6 @@ static void xs_data_ready(struct sock *sk)
if (!test_and_set_bit(XPRT_SOCK_DATA_READY, &transport->sock_state))
queue_work(xprtiod_workqueue, &transport->recv_worker);
}
- read_unlock_bh(&sk->sk_callback_lock);
}

/*
@@ -1409,9 +1404,8 @@ static void xs_tcp_state_change(struct sock *sk)
struct rpc_xprt *xprt;
struct sock_xprt *transport;

- read_lock_bh(&sk->sk_callback_lock);
if (!(xprt = xprt_from_sock(sk)))
- goto out;
+ return;
dprintk("RPC: xs_tcp_state_change client %p...\n", xprt);
dprintk("RPC: state %x conn %d dead %d zapped %d sk_shutdown %d\n",
sk->sk_state, xprt_connected(xprt),
@@ -1472,8 +1466,6 @@ static void xs_tcp_state_change(struct sock *sk)
/* Trigger the socket release */
xs_run_error_worker(transport, XPRT_SOCK_WAKE_DISCONNECT);
}
- out:
- read_unlock_bh(&sk->sk_callback_lock);
}

static void xs_write_space(struct sock *sk)
@@ -1512,13 +1504,9 @@ static void xs_write_space(struct sock *sk)
*/
static void xs_udp_write_space(struct sock *sk)
{
- read_lock_bh(&sk->sk_callback_lock);
-
/* from net/core/sock.c:sock_def_write_space */
if (sock_writeable(sk))
xs_write_space(sk);
-
- read_unlock_bh(&sk->sk_callback_lock);
}

/**
@@ -1533,13 +1521,9 @@ static void xs_udp_write_space(struct sock *sk)
*/
static void xs_tcp_write_space(struct sock *sk)
{
- read_lock_bh(&sk->sk_callback_lock);
-
/* from net/core/stream.c:sk_stream_write_space */
if (sk_stream_is_writeable(sk))
xs_write_space(sk);
-
- read_unlock_bh(&sk->sk_callback_lock);
}

static void xs_udp_do_set_buffer_size(struct rpc_xprt *xprt)
@@ -1834,7 +1818,7 @@ static int xs_local_finish_connecting(struct rpc_xprt *xprt,
if (!transport->inet) {
struct sock *sk = sock->sk;

- write_lock_bh(&sk->sk_callback_lock);
+ lock_sock(sk);

xs_save_old_callbacks(transport, sk);

@@ -1850,7 +1834,7 @@ static int xs_local_finish_connecting(struct rpc_xprt *xprt,
transport->sock = sock;
transport->inet = sk;

- write_unlock_bh(&sk->sk_callback_lock);
+ release_sock(sk);
}

xs_stream_start_connect(transport);
@@ -2032,7 +2016,7 @@ static void xs_udp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
if (!transport->inet) {
struct sock *sk = sock->sk;

- write_lock_bh(&sk->sk_callback_lock);
+ lock_sock(sk);

xs_save_old_callbacks(transport, sk);

@@ -2049,7 +2033,7 @@ static void xs_udp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)

xs_set_memalloc(xprt);

- write_unlock_bh(&sk->sk_callback_lock);
+ release_sock(sk);
}
xs_udp_do_set_buffer_size(xprt);

@@ -2195,7 +2179,7 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
xs_tcp_set_socket_timeouts(xprt, sock);
tcp_sock_set_nodelay(sk);

- write_lock_bh(&sk->sk_callback_lock);
+ lock_sock(sk);

xs_save_old_callbacks(transport, sk);

@@ -2215,7 +2199,7 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
transport->sock = sock;
transport->inet = sk;

- write_unlock_bh(&sk->sk_callback_lock);
+ release_sock(sk);
}

if (!xprt_bound(xprt))
--
2.31.1

2021-10-29 20:13:04

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 3/4] SUNRPC: Clean up xs_tcp_setup_sock()

From: Trond Myklebust <[email protected]>

Move the error handling into a single switch statement for clarity.

Signed-off-by: Trond Myklebust <[email protected]>
---
net/sunrpc/xprtsock.c | 68 ++++++++++++++++++-------------------------
1 file changed, 28 insertions(+), 40 deletions(-)

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 291b63136c08..7fb302e202bc 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2159,7 +2159,6 @@ static void xs_tcp_set_connect_timeout(struct rpc_xprt *xprt,
static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
{
struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt);
- int ret = -ENOTCONN;

if (!transport->inet) {
struct sock *sk = sock->sk;
@@ -2203,7 +2202,7 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
}

if (!xprt_bound(xprt))
- goto out;
+ return -ENOTCONN;

xs_set_memalloc(xprt);

@@ -2211,22 +2210,7 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)

/* Tell the socket layer to start connecting... */
set_bit(XPRT_SOCK_CONNECTING, &transport->sock_state);
- ret = kernel_connect(sock, xs_addr(xprt), xprt->addrlen, O_NONBLOCK);
- switch (ret) {
- case 0:
- xs_set_srcport(transport, sock);
- fallthrough;
- case -EINPROGRESS:
- /* SYN_SENT! */
- if (xprt->reestablish_timeout < XS_TCP_INIT_REEST_TO)
- xprt->reestablish_timeout = XS_TCP_INIT_REEST_TO;
- break;
- case -EADDRNOTAVAIL:
- /* Source port number is unavailable. Try a new one! */
- transport->srcport = 0;
- }
-out:
- return ret;
+ return kernel_connect(sock, xs_addr(xprt), xprt->addrlen, O_NONBLOCK);
}

/**
@@ -2241,14 +2225,14 @@ static void xs_tcp_setup_socket(struct work_struct *work)
container_of(work, struct sock_xprt, connect_worker.work);
struct socket *sock = transport->sock;
struct rpc_xprt *xprt = &transport->xprt;
- int status = -EIO;
+ int status;

if (!sock) {
sock = xs_create_sock(xprt, transport,
xs_addr(xprt)->sa_family, SOCK_STREAM,
IPPROTO_TCP, true);
if (IS_ERR(sock)) {
- status = PTR_ERR(sock);
+ xprt_wake_pending_tasks(xprt, PTR_ERR(sock));
goto out;
}
}
@@ -2265,21 +2249,21 @@ static void xs_tcp_setup_socket(struct work_struct *work)
xprt, -status, xprt_connected(xprt),
sock->sk->sk_state);
switch (status) {
- default:
- printk("%s: connect returned unhandled error %d\n",
- __func__, status);
- fallthrough;
- case -EADDRNOTAVAIL:
- /* We're probably in TIME_WAIT. Get rid of existing socket,
- * and retry
- */
- xs_tcp_force_close(xprt);
- break;
case 0:
+ xs_set_srcport(transport, sock);
+ fallthrough;
case -EINPROGRESS:
+ /* SYN_SENT! */
+ if (xprt->reestablish_timeout < XS_TCP_INIT_REEST_TO)
+ xprt->reestablish_timeout = XS_TCP_INIT_REEST_TO;
+ fallthrough;
case -EALREADY:
- xprt_unlock_connect(xprt, transport);
- return;
+ goto out_unlock;
+ case -EADDRNOTAVAIL:
+ /* Source port number is unavailable. Try a new one! */
+ transport->srcport = 0;
+ status = -EAGAIN;
+ break;
case -EINVAL:
/* Happens, for instance, if the user specified a link
* local IPv6 address without a scope-id.
@@ -2291,18 +2275,22 @@ static void xs_tcp_setup_socket(struct work_struct *work)
case -EHOSTUNREACH:
case -EADDRINUSE:
case -ENOBUFS:
- /* xs_tcp_force_close() wakes tasks with a fixed error code.
- * We need to wake them first to ensure the correct error code.
- */
- xprt_wake_pending_tasks(xprt, status);
- xs_tcp_force_close(xprt);
- goto out;
+ break;
+ default:
+ printk("%s: connect returned unhandled error %d\n",
+ __func__, status);
+ status = -EAGAIN;
}
- status = -EAGAIN;
+
+ /* xs_tcp_force_close() wakes tasks with a fixed error code.
+ * We need to wake them first to ensure the correct error code.
+ */
+ xprt_wake_pending_tasks(xprt, status);
+ xs_tcp_force_close(xprt);
out:
xprt_clear_connecting(xprt);
+out_unlock:
xprt_unlock_connect(xprt, transport);
- xprt_wake_pending_tasks(xprt, status);
}

/**
--
2.31.1

2021-10-30 14:55:12

by David Wysochanski

[permalink] [raw]
Subject: Re: [PATCH 1/4] SUNRPC: Fix races when closing the socket

On Fri, Oct 29, 2021 at 4:11 PM <[email protected]> wrote:
>
> From: Trond Myklebust <[email protected]>
>
> Ensure that we bump the xprt->connect_cookie when we set the
> XPRT_CLOSE_WAIT flag so that another call to
> xprt_conditional_disconnect() won't race with the reconnection.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> net/sunrpc/xprt.c | 2 ++
> net/sunrpc/xprtsock.c | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> index 48560188e84d..691fe5a682b6 100644
> --- a/net/sunrpc/xprt.c
> +++ b/net/sunrpc/xprt.c
> @@ -735,6 +735,8 @@ static void xprt_autoclose(struct work_struct *work)
> unsigned int pflags = memalloc_nofs_save();
>
> trace_xprt_disconnect_auto(xprt);
> + xprt->connect_cookie++;
> + smp_mb__before_atomic();
> clear_bit(XPRT_CLOSE_WAIT, &xprt->state);
> xprt->ops->close(xprt);
> xprt_release_write(xprt, NULL);
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 04f1b78bcbca..b18d13479104 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -1134,6 +1134,7 @@ static void xs_run_error_worker(struct sock_xprt *transport, unsigned int nr)
>
> static void xs_sock_reset_connection_flags(struct rpc_xprt *xprt)
> {
> + xprt->connect_cookie++;
> smp_mb__before_atomic();
> clear_bit(XPRT_CLOSE_WAIT, &xprt->state);
> clear_bit(XPRT_CLOSING, &xprt->state);
> --
> 2.31.1
>

Hey Trond,

Are you working on this "double SYN" problem that Olga / Netapp found
or something else?

FWIW, late yesterday, I tested these patches on top of your "testing"
branch and still see "double SYNs". I have a simple reproducer I'm
using where I fire off a bunch of "flocks" in parallel then reboot the
server.

If I can help with testing or something similar, let me know. I also
noticed that the tracepoints that would be useful for these reconnect
problems do not have 'src' TCP port in them, which would be helpful.
I don't have a patch for that yet but started looking into it.

2021-12-03 16:10:55

by David Wysochanski

[permalink] [raw]
Subject: Re: [PATCH 3/4] SUNRPC: Clean up xs_tcp_setup_sock()

On Fri, Oct 29, 2021 at 4:11 PM <[email protected]> wrote:
>
> From: Trond Myklebust <[email protected]>
>
> Move the error handling into a single switch statement for clarity.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> net/sunrpc/xprtsock.c | 68 ++++++++++++++++++-------------------------
> 1 file changed, 28 insertions(+), 40 deletions(-)
>
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 291b63136c08..7fb302e202bc 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -2159,7 +2159,6 @@ static void xs_tcp_set_connect_timeout(struct rpc_xprt *xprt,
> static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
> {
> struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt);
> - int ret = -ENOTCONN;
>
> if (!transport->inet) {
> struct sock *sk = sock->sk;
> @@ -2203,7 +2202,7 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
> }
>
> if (!xprt_bound(xprt))
> - goto out;
> + return -ENOTCONN;
>
> xs_set_memalloc(xprt);
>
> @@ -2211,22 +2210,7 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
>
> /* Tell the socket layer to start connecting... */
> set_bit(XPRT_SOCK_CONNECTING, &transport->sock_state);
> - ret = kernel_connect(sock, xs_addr(xprt), xprt->addrlen, O_NONBLOCK);
> - switch (ret) {
> - case 0:
> - xs_set_srcport(transport, sock);
> - fallthrough;
> - case -EINPROGRESS:
> - /* SYN_SENT! */
> - if (xprt->reestablish_timeout < XS_TCP_INIT_REEST_TO)
> - xprt->reestablish_timeout = XS_TCP_INIT_REEST_TO;
> - break;
> - case -EADDRNOTAVAIL:
> - /* Source port number is unavailable. Try a new one! */
> - transport->srcport = 0;
> - }
> -out:
> - return ret;
> + return kernel_connect(sock, xs_addr(xprt), xprt->addrlen, O_NONBLOCK);
> }
>
> /**
> @@ -2241,14 +2225,14 @@ static void xs_tcp_setup_socket(struct work_struct *work)
> container_of(work, struct sock_xprt, connect_worker.work);
> struct socket *sock = transport->sock;
> struct rpc_xprt *xprt = &transport->xprt;
> - int status = -EIO;
> + int status;
>
> if (!sock) {
> sock = xs_create_sock(xprt, transport,
> xs_addr(xprt)->sa_family, SOCK_STREAM,
> IPPROTO_TCP, true);
> if (IS_ERR(sock)) {
> - status = PTR_ERR(sock);
> + xprt_wake_pending_tasks(xprt, PTR_ERR(sock));
> goto out;
> }
> }
> @@ -2265,21 +2249,21 @@ static void xs_tcp_setup_socket(struct work_struct *work)
> xprt, -status, xprt_connected(xprt),
> sock->sk->sk_state);
> switch (status) {
> - default:
> - printk("%s: connect returned unhandled error %d\n",
> - __func__, status);
> - fallthrough;
> - case -EADDRNOTAVAIL:
> - /* We're probably in TIME_WAIT. Get rid of existing socket,
> - * and retry
> - */
> - xs_tcp_force_close(xprt);
> - break;
> case 0:
> + xs_set_srcport(transport, sock);
> + fallthrough;
> case -EINPROGRESS:
> + /* SYN_SENT! */
> + if (xprt->reestablish_timeout < XS_TCP_INIT_REEST_TO)
> + xprt->reestablish_timeout = XS_TCP_INIT_REEST_TO;
> + fallthrough;
> case -EALREADY:
> - xprt_unlock_connect(xprt, transport);
> - return;
> + goto out_unlock;
> + case -EADDRNOTAVAIL:
> + /* Source port number is unavailable. Try a new one! */
> + transport->srcport = 0;
> + status = -EAGAIN;
> + break;
> case -EINVAL:
> /* Happens, for instance, if the user specified a link
> * local IPv6 address without a scope-id.
> @@ -2291,18 +2275,22 @@ static void xs_tcp_setup_socket(struct work_struct *work)
> case -EHOSTUNREACH:
> case -EADDRINUSE:
> case -ENOBUFS:
> - /* xs_tcp_force_close() wakes tasks with a fixed error code.
> - * We need to wake them first to ensure the correct error code.
> - */
> - xprt_wake_pending_tasks(xprt, status);
> - xs_tcp_force_close(xprt);
> - goto out;
> + break;
> + default:
> + printk("%s: connect returned unhandled error %d\n",
> + __func__, status);
> + status = -EAGAIN;
> }
> - status = -EAGAIN;
> +
> + /* xs_tcp_force_close() wakes tasks with a fixed error code.
> + * We need to wake them first to ensure the correct error code.
> + */
> + xprt_wake_pending_tasks(xprt, status);
> + xs_tcp_force_close(xprt);

Isn't this a problem to call xs_tcp_force_close() unconditionally at
the bottom of xs_tcp_setup_socket()?
Before this patch we only called this depending on certain returns
from kernel_connect().



> out:
> xprt_clear_connecting(xprt);
> +out_unlock:
> xprt_unlock_connect(xprt, transport);
> - xprt_wake_pending_tasks(xprt, status);
> }
>
> /**
> --
> 2.31.1
>


2021-12-03 16:36:45

by David Wysochanski

[permalink] [raw]
Subject: Re: [PATCH 3/4] SUNRPC: Clean up xs_tcp_setup_sock()

On Fri, Dec 3, 2021 at 11:10 AM David Wysochanski <[email protected]> wrote:
>
> On Fri, Oct 29, 2021 at 4:11 PM <[email protected]> wrote:
> >
> > From: Trond Myklebust <[email protected]>
> >
> > Move the error handling into a single switch statement for clarity.
> >
> > Signed-off-by: Trond Myklebust <[email protected]>
> > ---
> > net/sunrpc/xprtsock.c | 68 ++++++++++++++++++-------------------------
> > 1 file changed, 28 insertions(+), 40 deletions(-)
> >
> > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> > index 291b63136c08..7fb302e202bc 100644
> > --- a/net/sunrpc/xprtsock.c
> > +++ b/net/sunrpc/xprtsock.c
> > @@ -2159,7 +2159,6 @@ static void xs_tcp_set_connect_timeout(struct rpc_xprt *xprt,
> > static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
> > {
> > struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt);
> > - int ret = -ENOTCONN;
> >
> > if (!transport->inet) {
> > struct sock *sk = sock->sk;
> > @@ -2203,7 +2202,7 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
> > }
> >
> > if (!xprt_bound(xprt))
> > - goto out;
> > + return -ENOTCONN;
> >
> > xs_set_memalloc(xprt);
> >
> > @@ -2211,22 +2210,7 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
> >
> > /* Tell the socket layer to start connecting... */
> > set_bit(XPRT_SOCK_CONNECTING, &transport->sock_state);
> > - ret = kernel_connect(sock, xs_addr(xprt), xprt->addrlen, O_NONBLOCK);
> > - switch (ret) {
> > - case 0:
> > - xs_set_srcport(transport, sock);
> > - fallthrough;
> > - case -EINPROGRESS:
> > - /* SYN_SENT! */
> > - if (xprt->reestablish_timeout < XS_TCP_INIT_REEST_TO)
> > - xprt->reestablish_timeout = XS_TCP_INIT_REEST_TO;
> > - break;
> > - case -EADDRNOTAVAIL:
> > - /* Source port number is unavailable. Try a new one! */
> > - transport->srcport = 0;
> > - }
> > -out:
> > - return ret;
> > + return kernel_connect(sock, xs_addr(xprt), xprt->addrlen, O_NONBLOCK);
> > }
> >
> > /**
> > @@ -2241,14 +2225,14 @@ static void xs_tcp_setup_socket(struct work_struct *work)
> > container_of(work, struct sock_xprt, connect_worker.work);
> > struct socket *sock = transport->sock;
> > struct rpc_xprt *xprt = &transport->xprt;
> > - int status = -EIO;
> > + int status;
> >
> > if (!sock) {
> > sock = xs_create_sock(xprt, transport,
> > xs_addr(xprt)->sa_family, SOCK_STREAM,
> > IPPROTO_TCP, true);
> > if (IS_ERR(sock)) {
> > - status = PTR_ERR(sock);
> > + xprt_wake_pending_tasks(xprt, PTR_ERR(sock));
> > goto out;
> > }
> > }
> > @@ -2265,21 +2249,21 @@ static void xs_tcp_setup_socket(struct work_struct *work)
> > xprt, -status, xprt_connected(xprt),
> > sock->sk->sk_state);
> > switch (status) {
> > - default:
> > - printk("%s: connect returned unhandled error %d\n",
> > - __func__, status);
> > - fallthrough;
> > - case -EADDRNOTAVAIL:
> > - /* We're probably in TIME_WAIT. Get rid of existing socket,
> > - * and retry
> > - */
> > - xs_tcp_force_close(xprt);
> > - break;
> > case 0:
> > + xs_set_srcport(transport, sock);
> > + fallthrough;
> > case -EINPROGRESS:
> > + /* SYN_SENT! */
> > + if (xprt->reestablish_timeout < XS_TCP_INIT_REEST_TO)
> > + xprt->reestablish_timeout = XS_TCP_INIT_REEST_TO;
> > + fallthrough;
> > case -EALREADY:
> > - xprt_unlock_connect(xprt, transport);
> > - return;
> > + goto out_unlock;
> > + case -EADDRNOTAVAIL:
> > + /* Source port number is unavailable. Try a new one! */
> > + transport->srcport = 0;
> > + status = -EAGAIN;
> > + break;
> > case -EINVAL:
> > /* Happens, for instance, if the user specified a link
> > * local IPv6 address without a scope-id.
> > @@ -2291,18 +2275,22 @@ static void xs_tcp_setup_socket(struct work_struct *work)
> > case -EHOSTUNREACH:
> > case -EADDRINUSE:
> > case -ENOBUFS:
> > - /* xs_tcp_force_close() wakes tasks with a fixed error code.
> > - * We need to wake them first to ensure the correct error code.
> > - */
> > - xprt_wake_pending_tasks(xprt, status);
> > - xs_tcp_force_close(xprt);
> > - goto out;
> > + break;
> > + default:
> > + printk("%s: connect returned unhandled error %d\n",
> > + __func__, status);
> > + status = -EAGAIN;
> > }
> > - status = -EAGAIN;
> > +
> > + /* xs_tcp_force_close() wakes tasks with a fixed error code.
> > + * We need to wake them first to ensure the correct error code.
> > + */
> > + xprt_wake_pending_tasks(xprt, status);
> > + xs_tcp_force_close(xprt);
>
> Isn't this a problem to call xs_tcp_force_close() unconditionally at
> the bottom of xs_tcp_setup_socket()?
> Before this patch we only called this depending on certain returns
> from kernel_connect().
>
Nevermind - I see the fallthroughs and out_unlock

>
>
> > out:
> > xprt_clear_connecting(xprt);
> > +out_unlock:
> > xprt_unlock_connect(xprt, transport);
> > - xprt_wake_pending_tasks(xprt, status);
> > }
> >
> > /**
> > --
> > 2.31.1
> >