lkml.org 
[lkml]   [2022]   [May]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH net 2/2] net/smc: align the connect behaviour with TCP
From
Date
On Mon, 2022-05-09 at 19:58 +0800, Guangguan Wang wrote:
> Connect with O_NONBLOCK will not be completed immediately
> and returns -EINPROGRESS. It is possible to use selector/poll
> for completion by selecting the socket for writing. After select
> indicates writability, a second connect function call will return
> 0 to indicate connected successfully as TCP does, but smc returns
> -EISCONN. Use socket state for smc to indicate connect state, which
> can help smc aligning the connect behaviour with TCP.
>
> Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
> ---
> net/smc/af_smc.c | 53 ++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index fce16b9d6e1a..45f9f7c6e776 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -1544,9 +1544,32 @@ static int smc_connect(struct socket *sock, struct sockaddr *addr,
> goto out_err;
>
> lock_sock(sk);
> + switch (sock->state) {
> + default:
> + rc = -EINVAL;
> + goto out;
> + case SS_CONNECTED:
> + rc = sk->sk_state == SMC_ACTIVE ? -EISCONN : -EINVAL;
> + goto out;
> + case SS_CONNECTING:
> + if (sk->sk_state == SMC_ACTIVE) {
> + sock->state = SS_CONNECTED;
> + rc = 0;
> + goto out;
> + }
> + break;
> + case SS_UNCONNECTED:
> + sock->state = SS_CONNECTING;
> + break;
> + }
> +
> switch (sk->sk_state) {
> default:
> goto out;
> + case SMC_CLOSED:
> + rc = sock_error(sk) ? : -ECONNABORTED;
> + sock->state = SS_UNCONNECTED;
> + goto out;
> case SMC_ACTIVE:
> rc = -EISCONN;
> goto out;
> @@ -1565,18 +1588,22 @@ static int smc_connect(struct socket *sock, struct sockaddr *addr,
> goto out;
>
> sock_hold(&smc->sk); /* sock put in passive closing */
> - if (smc->use_fallback)
> + if (smc->use_fallback) {
> + sock->state = SS_CONNECTED;
> goto out;
> + }
> if (flags & O_NONBLOCK) {
> if (queue_work(smc_hs_wq, &smc->connect_work))
> smc->connect_nonblock = 1;
> rc = -EINPROGRESS;
> } else {
> rc = __smc_connect(smc);
> - if (rc < 0)
> + if (rc < 0) {
> goto out;
> - else
> + } else {
> rc = 0; /* success cases including fallback */
> + sock->state = SS_CONNECTED;

'else' is not needed here, you can keep the above 2 statements dropping
an indentation level.

> + }
> }
>

You can avoid a little code duplication adding here the following:

connected:
sock->state = SS_CONNECTED;

and using the new label where appropriate.

> out:
> @@ -1693,6 +1720,7 @@ struct sock *smc_accept_dequeue(struct sock *parent,
> }
> if (new_sock) {
> sock_graft(new_sk, new_sock);
> + new_sock->state = SS_CONNECTED;
> if (isk->use_fallback) {
> smc_sk(new_sk)->clcsock->file = new_sock->file;
> isk->clcsock->file->private_data = isk->clcsock;
> @@ -2424,7 +2452,7 @@ static int smc_listen(struct socket *sock, int backlog)
>
> rc = -EINVAL;
> if ((sk->sk_state != SMC_INIT && sk->sk_state != SMC_LISTEN) ||
> - smc->connect_nonblock)
> + smc->connect_nonblock || sock->state != SS_UNCONNECTED)
> goto out;
>
> rc = 0;
> @@ -2716,6 +2744,17 @@ static int smc_shutdown(struct socket *sock, int how)
>
> lock_sock(sk);
>
> + if (sock->state == SS_CONNECTING) {
> + if (sk->sk_state == SMC_ACTIVE)
> + sock->state = SS_CONNECTED;
> + else if (sk->sk_state == SMC_PEERCLOSEWAIT1 ||
> + sk->sk_state == SMC_PEERCLOSEWAIT2 ||
> + sk->sk_state == SMC_APPCLOSEWAIT1 ||
> + sk->sk_state == SMC_APPCLOSEWAIT2 ||
> + sk->sk_state == SMC_APPFINCLOSEWAIT)
> + sock->state = SS_DISCONNECTING;
> + }
> +
> rc = -ENOTCONN;
> if ((sk->sk_state != SMC_ACTIVE) &&
> (sk->sk_state != SMC_PEERCLOSEWAIT1) &&
> @@ -2729,6 +2768,7 @@ static int smc_shutdown(struct socket *sock, int how)
> sk->sk_shutdown = smc->clcsock->sk->sk_shutdown;
> if (sk->sk_shutdown == SHUTDOWN_MASK) {
> sk->sk_state = SMC_CLOSED;
> + sk->sk_socket->state = SS_UNCONNECTED;
> sock_put(sk);
> }
> goto out;
> @@ -2754,6 +2794,10 @@ static int smc_shutdown(struct socket *sock, int how)
> /* map sock_shutdown_cmd constants to sk_shutdown value range */
> sk->sk_shutdown |= how + 1;
>
> + if (sk->sk_state == SMC_CLOSED)
> + sock->state = SS_UNCONNECTED;
> + else
> + sock->state = SS_DISCONNECTING;
> out:
> release_sock(sk);
> return rc ? rc : rc1;
> @@ -3139,6 +3183,7 @@ static int __smc_create(struct net *net, struct socket *sock, int protocol,
>
> rc = -ENOBUFS;
> sock->ops = &smc_sock_ops;
> + sock->state = SS_UNCONNECTED;
> sk = smc_sock_alloc(net, sock, protocol);
> if (!sk)
> goto out;

\
 
 \ /
  Last update: 2022-05-10 13:07    [W:0.078 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site