* [PATCH net 0/2] Fixes for clcsock shutdown behaviors
@ 2021-11-25 6:19 Tony Lu
2021-11-25 6:19 ` [PATCH net 1/2] net/smc: Keep smc_close_final rc during active close Tony Lu
2021-11-25 6:19 ` [PATCH net 2/2] net/smc: Don't call clcsock shutdown twice when smc shutdown Tony Lu
0 siblings, 2 replies; 6+ messages in thread
From: Tony Lu @ 2021-11-25 6:19 UTC (permalink / raw)
To: kgraul; +Cc: kuba, davem, netdev, linux-s390, linux-rdma
These patches fix issues when calling kernel_sock_shutdown(). To make
the solution clear, I split it into two patches.
Patch 1 keeps the return code of smc_close_final() in
smc_close_active(), which is more important than kernel_sock_shutdown().
Patch 2 doesn't call clcsock shutdown twice when applications call
smc_shutdown(). It should be okay to call kernel_sock_shutdown() twice,
I decide to avoid it for slightly speed up releasing socket.
Tony Lu (2):
net/smc: Keep smc_close_final rc during active close
net/smc: Don't call clcsock shutdown twice when smc shutdown
net/smc/af_smc.c | 8 ++++++++
net/smc/smc_close.c | 8 ++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
--
2.32.0.3.g01195cf9f
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net 1/2] net/smc: Keep smc_close_final rc during active close
2021-11-25 6:19 [PATCH net 0/2] Fixes for clcsock shutdown behaviors Tony Lu
@ 2021-11-25 6:19 ` Tony Lu
2021-11-25 10:51 ` Karsten Graul
2021-11-25 6:19 ` [PATCH net 2/2] net/smc: Don't call clcsock shutdown twice when smc shutdown Tony Lu
1 sibling, 1 reply; 6+ messages in thread
From: Tony Lu @ 2021-11-25 6:19 UTC (permalink / raw)
To: kgraul; +Cc: kuba, davem, netdev, linux-s390, linux-rdma
When smc_close_final() returns error, the return code overwrites by
kernel_sock_shutdown() in smc_close_active(). The return code of
smc_close_final() is more important than kernel_sock_shutdown(), and it
will pass to userspace directly.
Fix it by keeping both return codes, if smc_close_final() raises an
error, return it or kernel_sock_shutdown()'s.
Link: https://lore.kernel.org/linux-s390/1f67548e-cbf6-0dce-82b5-10288a4583bd@linux.ibm.com/
Fixes: 606a63c9783a ("net/smc: Ensure the active closing peer first closes clcsock")
Suggested-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
Reviewed-by: Wen Gu <guwen@linux.alibaba.com>
---
net/smc/smc_close.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index 3715d2f5ad55..292e4d904ab6 100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c
@@ -195,6 +195,7 @@ int smc_close_active(struct smc_sock *smc)
int old_state;
long timeout;
int rc = 0;
+ int rc1 = 0;
timeout = current->flags & PF_EXITING ?
0 : sock_flag(sk, SOCK_LINGER) ?
@@ -232,8 +233,11 @@ int smc_close_active(struct smc_sock *smc)
/* actively shutdown clcsock before peer close it,
* prevent peer from entering TIME_WAIT state.
*/
- if (smc->clcsock && smc->clcsock->sk)
- rc = kernel_sock_shutdown(smc->clcsock, SHUT_RDWR);
+ if (smc->clcsock && smc->clcsock->sk) {
+ rc1 = kernel_sock_shutdown(smc->clcsock,
+ SHUT_RDWR);
+ rc = rc ? rc : rc1;
+ }
} else {
/* peer event has changed the state */
goto again;
--
2.32.0.3.g01195cf9f
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 2/2] net/smc: Don't call clcsock shutdown twice when smc shutdown
2021-11-25 6:19 [PATCH net 0/2] Fixes for clcsock shutdown behaviors Tony Lu
2021-11-25 6:19 ` [PATCH net 1/2] net/smc: Keep smc_close_final rc during active close Tony Lu
@ 2021-11-25 6:19 ` Tony Lu
2021-11-25 11:02 ` Karsten Graul
1 sibling, 1 reply; 6+ messages in thread
From: Tony Lu @ 2021-11-25 6:19 UTC (permalink / raw)
To: kgraul; +Cc: kuba, davem, netdev, linux-s390, linux-rdma
When applications call shutdown() with SHUT_RDWR in userspace,
smc_close_active() calls kernel_sock_shutdown(), and it is called
twice in smc_shutdown().
This fixes this by checking sk_state before do clcsock shutdown, and
avoids missing the application's call of smc_shutdown().
Link: https://lore.kernel.org/linux-s390/1f67548e-cbf6-0dce-82b5-10288a4583bd@linux.ibm.com/
Fixes: 606a63c9783a ("net/smc: Ensure the active closing peer first closes clcsock")
Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
Reviewed-by: Wen Gu <guwen@linux.alibaba.com>
---
net/smc/af_smc.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 4b62c925a13e..7b04cb4d15f4 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -2373,6 +2373,7 @@ static int smc_shutdown(struct socket *sock, int how)
struct smc_sock *smc;
int rc = -EINVAL;
int rc1 = 0;
+ int old_state;
smc = smc_sk(sk);
@@ -2398,7 +2399,12 @@ static int smc_shutdown(struct socket *sock, int how)
}
switch (how) {
case SHUT_RDWR: /* shutdown in both directions */
+ old_state = sk->sk_state;
rc = smc_close_active(smc);
+ if (old_state == SMC_ACTIVE &&
+ sk->sk_state == SMC_PEERCLOSEWAIT1)
+ goto out_no_shutdown;
+
break;
case SHUT_WR:
rc = smc_close_shutdown_write(smc);
@@ -2410,6 +2416,8 @@ static int smc_shutdown(struct socket *sock, int how)
}
if (smc->clcsock)
rc1 = kernel_sock_shutdown(smc->clcsock, how);
+
+out_no_shutdown:
/* map sock_shutdown_cmd constants to sk_shutdown value range */
sk->sk_shutdown |= how + 1;
--
2.32.0.3.g01195cf9f
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] net/smc: Keep smc_close_final rc during active close
2021-11-25 6:19 ` [PATCH net 1/2] net/smc: Keep smc_close_final rc during active close Tony Lu
@ 2021-11-25 10:51 ` Karsten Graul
0 siblings, 0 replies; 6+ messages in thread
From: Karsten Graul @ 2021-11-25 10:51 UTC (permalink / raw)
To: netdev; +Cc: kuba, davem, linux-s390, linux-rdma, Tony Lu
On 25/11/2021 07:19, Tony Lu wrote:
> When smc_close_final() returns error, the return code overwrites by
> kernel_sock_shutdown() in smc_close_active(). The return code of
> smc_close_final() is more important than kernel_sock_shutdown(), and it
> will pass to userspace directly.
>
> Fix it by keeping both return codes, if smc_close_final() raises an
> error, return it or kernel_sock_shutdown()'s.
>
> Link: https://lore.kernel.org/linux-s390/1f67548e-cbf6-0dce-82b5-10288a4583bd@linux.ibm.com/
> Fixes: 606a63c9783a ("net/smc: Ensure the active closing peer first closes clcsock")
> Suggested-by: Karsten Graul <kgraul@linux.ibm.com>
> Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
> Reviewed-by: Wen Gu <guwen@linux.alibaba.com>
> ---
Acked-by: Karsten Graul <kgraul@linux.ibm.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] net/smc: Don't call clcsock shutdown twice when smc shutdown
2021-11-25 6:19 ` [PATCH net 2/2] net/smc: Don't call clcsock shutdown twice when smc shutdown Tony Lu
@ 2021-11-25 11:02 ` Karsten Graul
2021-11-25 12:41 ` Tony Lu
0 siblings, 1 reply; 6+ messages in thread
From: Karsten Graul @ 2021-11-25 11:02 UTC (permalink / raw)
To: Tony Lu; +Cc: kuba, davem, netdev, linux-s390, linux-rdma
On 25/11/2021 07:19, Tony Lu wrote:
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 4b62c925a13e..7b04cb4d15f4 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -2373,6 +2373,7 @@ static int smc_shutdown(struct socket *sock, int how)
> struct smc_sock *smc;
> int rc = -EINVAL;
> int rc1 = 0;
> + int old_state;
Reverse Christmas tree formatting, please.
>
> smc = smc_sk(sk);
>
> @@ -2398,7 +2399,12 @@ static int smc_shutdown(struct socket *sock, int how)
> }
> switch (how) {
> case SHUT_RDWR: /* shutdown in both directions */
> + old_state = sk->sk_state;
> rc = smc_close_active(smc);
> + if (old_state == SMC_ACTIVE &&
> + sk->sk_state == SMC_PEERCLOSEWAIT1)
> + goto out_no_shutdown;
> +
I would prefer a new "bool do_shutdown" instead of a goto for this skip
of the shutdown. What do you think?
> break;
> case SHUT_WR:
> rc = smc_close_shutdown_write(smc);
> @@ -2410,6 +2416,8 @@ static int smc_shutdown(struct socket *sock, int how)
> }
> if (smc->clcsock)
> rc1 = kernel_sock_shutdown(smc->clcsock, how);
> +
> +out_no_shutdown:
> /* map sock_shutdown_cmd constants to sk_shutdown value range */
> sk->sk_shutdown |= how + 1;
>
>
--
Karsten
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] net/smc: Don't call clcsock shutdown twice when smc shutdown
2021-11-25 11:02 ` Karsten Graul
@ 2021-11-25 12:41 ` Tony Lu
0 siblings, 0 replies; 6+ messages in thread
From: Tony Lu @ 2021-11-25 12:41 UTC (permalink / raw)
To: Karsten Graul; +Cc: kuba, davem, netdev, linux-s390, linux-rdma
On Thu, Nov 25, 2021 at 12:02:06PM +0100, Karsten Graul wrote:
> On 25/11/2021 07:19, Tony Lu wrote:
> > diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> > index 4b62c925a13e..7b04cb4d15f4 100644
> > --- a/net/smc/af_smc.c
> > +++ b/net/smc/af_smc.c
> > @@ -2373,6 +2373,7 @@ static int smc_shutdown(struct socket *sock, int how)
> > struct smc_sock *smc;
> > int rc = -EINVAL;
> > int rc1 = 0;
> > + int old_state;
>
> Reverse Christmas tree formatting, please.
Sorry for that, I will fix it in the next patch.
>
> >
> > smc = smc_sk(sk);
> >
> > @@ -2398,7 +2399,12 @@ static int smc_shutdown(struct socket *sock, int how)
> > }
> > switch (how) {
> > case SHUT_RDWR: /* shutdown in both directions */
> > + old_state = sk->sk_state;
> > rc = smc_close_active(smc);
> > + if (old_state == SMC_ACTIVE &&
> > + sk->sk_state == SMC_PEERCLOSEWAIT1)
> > + goto out_no_shutdown;
> > +
>
> I would prefer a new "bool do_shutdown" instead of a goto for this skip
> of the shutdown. What do you think?
I agree with you, I'd like bool condition rather than goto, which will
disturb the continuity of reading code.
I will fix it soon. Thank you.
Tony Lu
>
> > break;
> > case SHUT_WR:
> > rc = smc_close_shutdown_write(smc);
> > @@ -2410,6 +2416,8 @@ static int smc_shutdown(struct socket *sock, int how)
> > }
> > if (smc->clcsock)
> > rc1 = kernel_sock_shutdown(smc->clcsock, how);
> > +
> > +out_no_shutdown:
> > /* map sock_shutdown_cmd constants to sk_shutdown value range */
> > sk->sk_shutdown |= how + 1;
> >
> >
>
> --
> Karsten
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-25 12:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-25 6:19 [PATCH net 0/2] Fixes for clcsock shutdown behaviors Tony Lu
2021-11-25 6:19 ` [PATCH net 1/2] net/smc: Keep smc_close_final rc during active close Tony Lu
2021-11-25 10:51 ` Karsten Graul
2021-11-25 6:19 ` [PATCH net 2/2] net/smc: Don't call clcsock shutdown twice when smc shutdown Tony Lu
2021-11-25 11:02 ` Karsten Graul
2021-11-25 12:41 ` Tony Lu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.