[PATCH net-next v2 0/2] net/smc: two fixes for using smc with io_uring

[PATCH net-next v2 0/2] net/smc: two fixes for using smc with io_uring

[Guangguan Wang]

This patch set includes two fixes for using smc with io_uring.

Guangguan Wang (2):
  net/smc: non blocking recvmsg() return -EAGAIN when no data and
    signal_pending
  net/smc: align the connect behaviour with TCP

 net/smc/af_smc.c | 50 ++++++++++++++++++++++++++++++++++++++++++++----
 net/smc/smc_rx.c |  4 ++--
 2 files changed, 48 insertions(+), 6 deletions(-)

-- 
2.24.3 (Apple Git-128)

[PATCH net-next v2 1/2] net/smc: non blocking recvmsg() return -EAGAIN when no data and signal_pending

[Guangguan Wang]

Non blocking sendmsg will return -EAGAIN when any signal pending
and no send space left, while non blocking recvmsg return -EINTR
when signal pending and no data received. This may makes confused.
As TCP returns -EAGAIN in the conditions described above. Align the
behavior of smc with TCP.

Fixes: 846e344eb722 ("net/smc: add receive timeout check")
Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>
---
 net/smc/smc_rx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/smc/smc_rx.c b/net/smc/smc_rx.c
index 51e8eb2933ff..338b9ef806e8 100644
--- a/net/smc/smc_rx.c
+++ b/net/smc/smc_rx.c
@@ -355,12 +355,12 @@ int smc_rx_recvmsg(struct smc_sock *smc, struct msghdr *msg,
 				}
 				break;
 			}
+			if (!timeo)
+				return -EAGAIN;
 			if (signal_pending(current)) {
 				read_done = sock_intr_errno(timeo);
 				break;
 			}
-			if (!timeo)
-				return -EAGAIN;
 		}
 
 		if (!smc_rx_data_available(conn)) {
-- 
2.24.3 (Apple Git-128)

Re: [PATCH net-next v2 1/2] net/smc: non blocking recvmsg() return -EAGAIN when no data and signal_pending

[Tony Lu]

On Thu, May 12, 2022 at 11:11:55AM +0800, Guangguan Wang wrote:
> Non blocking sendmsg will return -EAGAIN when any signal pending
> and no send space left, while non blocking recvmsg return -EINTR
> when signal pending and no data received. This may makes confused.
> As TCP returns -EAGAIN in the conditions described above. Align the
> behavior of smc with TCP.
> 
> Fixes: 846e344eb722 ("net/smc: add receive timeout check")
> Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
> Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>

I see that you have already sent this patch to net, so this patch is a
duplicate. There is no need to send it again to net-next.

Thanks,
Tony Lu

Re: [PATCH net-next v2 1/2] net/smc: non blocking recvmsg() return -EAGAIN when no data and signal_pending

[Guangguan Wang]

On 2022/5/12 11:43, Tony Lu wrote:
> On Thu, May 12, 2022 at 11:11:55AM +0800, Guangguan Wang wrote:
>> Non blocking sendmsg will return -EAGAIN when any signal pending
>> and no send space left, while non blocking recvmsg return -EINTR
>> when signal pending and no data received. This may makes confused.
>> As TCP returns -EAGAIN in the conditions described above. Align the
>> behavior of smc with TCP.
>>
>> Fixes: 846e344eb722 ("net/smc: add receive timeout check")
>> Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
>> Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>
> 
> I see that you have already sent this patch to net, so this patch is a
> duplicate. There is no need to send it again to net-next.
> 
> Thanks,
> Tony Lu

Ok, just ignore it. Thanks！

Re: [PATCH net-next v2 1/2] net/smc: non blocking recvmsg() return -EAGAIN when no data and signal_pending

[Jakub Kicinski]

On Thu, 12 May 2022 11:51:22 +0800 Guangguan Wang wrote:
> On 2022/5/12 11:43, Tony Lu wrote:
> > On Thu, May 12, 2022 at 11:11:55AM +0800, Guangguan Wang wrote:  
> >> Non blocking sendmsg will return -EAGAIN when any signal pending
> >> and no send space left, while non blocking recvmsg return -EINTR
> >> when signal pending and no data received. This may makes confused.
> >> As TCP returns -EAGAIN in the conditions described above. Align the
> >> behavior of smc with TCP.
> >>
> >> Fixes: 846e344eb722 ("net/smc: add receive timeout check")
> >> Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
> >> Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>  
> > 
> > I see that you have already sent this patch to net, so this patch is a
> > duplicate. There is no need to send it again to net-next.
> 
> Ok, just ignore it. Thanks！

You gotta repost just patch 2, then. Please wait until net and net-next
get merged before sending (or 12h if you don't know how to figure out if
that already happened ;))

[PATCH net-next v2 2/2] net/smc: align the connect behaviour with TCP

[Guangguan Wang]

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 | 50 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 46 insertions(+), 4 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index fce16b9d6e1a..5f70642a8044 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1544,9 +1544,29 @@ 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)
+			goto connected;
+		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,20 +1585,24 @@ 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 = rc ? SS_CONNECTING : SS_CONNECTED;
 		goto out;
+	}
 	if (flags & O_NONBLOCK) {
 		if (queue_work(smc_hs_wq, &smc->connect_work))
 			smc->connect_nonblock = 1;
 		rc = -EINPROGRESS;
+		goto out;
 	} else {
 		rc = __smc_connect(smc);
 		if (rc < 0)
 			goto out;
-		else
-			rc = 0; /* success cases including fallback */
 	}
 
+connected:
+	rc = 0;
+	sock->state = SS_CONNECTED;
 out:
 	release_sock(sk);
 out_err:
@@ -1693,6 +1717,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 +2449,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 +2741,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 +2765,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 +2791,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 +3180,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;
-- 
2.24.3 (Apple Git-128)

Re: [PATCH net-next v2 2/2] net/smc: align the connect behaviour with TCP

[Karsten Graul]

On 12/05/2022 05:11, 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>
> ---

Acked-by: Karsten Graul <kgraul@linux.ibm.com>

Thank you.