[PATCH net-next] net/smc: return ETIMEDOUT when smc_connect_clc() timeout

[PATCH net-next] net/smc: return ETIMEDOUT when smc_connect_clc() timeout

[D. Wythe]

From: "D. Wythe" <alibuda@linux.alibaba.com>

When smc_connect_clc() times out, it will return -EAGAIN(tcp_recvmsg
retuns -EAGAIN while timeout), then this value will passed to the
application, which is quite confusing to the applications, makes
inconsistency with TCP.

From the manual of connect, ETIMEDOUT is more suitable, and this patch
try convert EAGAIN to ETIMEDOUT in that case.

Signed-off-by: D. Wythe <alibuda@linux.alibaba.com>
---
 net/smc/af_smc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 246c874..38a4064 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1372,8 +1372,14 @@ static int __smc_connect(struct smc_sock *smc)
 
 	/* perform CLC handshake */
 	rc = smc_connect_clc(smc, aclc2, ini);
-	if (rc)
+	if (rc) {
+		/* -EAGAIN on timeout, see tcp_recvmsg() */
+		if (rc == -EAGAIN) {
+			rc = -ETIMEDOUT;
+			smc->sk.sk_err = ETIMEDOUT;
+		}
 		goto vlan_cleanup;
+	}
 
 	/* check if smc modes and versions of CLC proposal and accept match */
 	rc = smc_connect_check_aclc(ini, aclc);
-- 
1.8.3.1

Re: [PATCH net-next] net/smc: return ETIMEDOUT when smc_connect_clc() timeout

[Karsten Graul]

On 15/02/2022 09:24, D. Wythe wrote:
> From: "D. Wythe" <alibuda@linux.alibaba.com>
> 
> When smc_connect_clc() times out, it will return -EAGAIN(tcp_recvmsg
> retuns -EAGAIN while timeout), then this value will passed to the
> application, which is quite confusing to the applications, makes
> inconsistency with TCP.
> 
> From the manual of connect, ETIMEDOUT is more suitable, and this patch
> try convert EAGAIN to ETIMEDOUT in that case.

You say that the sock_recvmsg() in smc_clc_wait_msg() returns -EAGAIN?
Is there a reason why you translate it in __smc_connect() and not already in
smc_clc_wait_msg() after the call to sock_recvmsg()?

Re: [PATCH net-next] net/smc: return ETIMEDOUT when smc_connect_clc() timeout

[D. Wythe]

On Tue, Feb 15, 2022 at 02:02:37PM +0100, Karsten Graul wrote:
> On 15/02/2022 09:24, D. Wythe wrote:
> > From: "D. Wythe" <alibuda@linux.alibaba.com>
> > 
> > When smc_connect_clc() times out, it will return -EAGAIN(tcp_recvmsg
> > retuns -EAGAIN while timeout), then this value will passed to the
> > application, which is quite confusing to the applications, makes
> > inconsistency with TCP.
> > 
> > From the manual of connect, ETIMEDOUT is more suitable, and this patch
> > try convert EAGAIN to ETIMEDOUT in that case.
> 
> You say that the sock_recvmsg() in smc_clc_wait_msg() returns -EAGAIN?
> Is there a reason why you translate it in __smc_connect() and not already in
> smc_clc_wait_msg() after the call to sock_recvmsg()?


Because other code that uses smc_clc_wait_msg() handles EAGAIN allready, 
and the only exception is smc_listen_work(), but it doesn't really matter for it. 

The most important thing is that this conversion needs to be determined according to 
the calling scene, convert in smc_clc_wait_msg() is not very suitable.

Thanks.

Re: [PATCH net-next] net/smc: return ETIMEDOUT when smc_connect_clc() timeout

[Karsten Graul]

On 16/02/2022 04:13, D. Wythe wrote:
> On Tue, Feb 15, 2022 at 02:02:37PM +0100, Karsten Graul wrote:
>> On 15/02/2022 09:24, D. Wythe wrote:
>>> From: "D. Wythe" <alibuda@linux.alibaba.com>
>>>
>>> When smc_connect_clc() times out, it will return -EAGAIN(tcp_recvmsg
>>> retuns -EAGAIN while timeout), then this value will passed to the
>>> application, which is quite confusing to the applications, makes
>>> inconsistency with TCP.
>>>
>>> From the manual of connect, ETIMEDOUT is more suitable, and this patch
>>> try convert EAGAIN to ETIMEDOUT in that case.
>>
>> You say that the sock_recvmsg() in smc_clc_wait_msg() returns -EAGAIN?
>> Is there a reason why you translate it in __smc_connect() and not already in
>> smc_clc_wait_msg() after the call to sock_recvmsg()?
> 
> 
> Because other code that uses smc_clc_wait_msg() handles EAGAIN allready, 
> and the only exception is smc_listen_work(), but it doesn't really matter for it. 
> 
> The most important thing is that this conversion needs to be determined according to 
> the calling scene, convert in smc_clc_wait_msg() is not very suitable.

Okay I understand, thank you.

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

Re: [PATCH net-next] net/smc: return ETIMEDOUT when smc_connect_clc() timeout

[Jakub Kicinski]

On Wed, 16 Feb 2022 11:23:12 +0100 Karsten Graul wrote:
> On 16/02/2022 04:13, D. Wythe wrote:
> > Because other code that uses smc_clc_wait_msg() handles EAGAIN allready, 
> > and the only exception is smc_listen_work(), but it doesn't really matter for it. 
> > 
> > The most important thing is that this conversion needs to be determined according to 
> > the calling scene, convert in smc_clc_wait_msg() is not very suitable.  
> 
> Okay I understand, thank you.
> 
> Reviewed-by: Karsten Graul <kgraul@linux.ibm.com>

Applied, thanks!