| Message ID | 1644913490-21594-1-git-send-email-alibuda@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net/smc: return ETIMEDOUT when smc_connect_clc() timeout | expand |
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()?
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.
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>
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!
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);