All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/2] Fixes for closing process and minor cleanup
@ 2021-11-23  8:25 Tony Lu
  2021-11-23  8:25 ` [PATCH net 1/2] net/smc: Clean up local struct sock variables Tony Lu
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Tony Lu @ 2021-11-23  8:25 UTC (permalink / raw)
  To: kgraul; +Cc: kuba, davem, guwen, netdev, linux-s390, linux-rdma

Patch 1 is a minor cleanup for local struct sock variables.

Patch 2 ensures the active closing side enters TIME_WAIT.

Tony Lu (2):
  net/smc: Clean up local struct sock variables
  net/smc: Ensure the active closing peer first closes clcsock

 net/smc/smc_close.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

-- 
2.32.0.3.g01195cf9f


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH net 1/2] net/smc: Clean up local struct sock variables
  2021-11-23  8:25 [PATCH net 0/2] Fixes for closing process and minor cleanup Tony Lu
@ 2021-11-23  8:25 ` Tony Lu
  2021-11-23  8:34   ` Karsten Graul
  2021-11-23  8:25 ` [PATCH net 2/2] net/smc: Ensure the active closing peer first closes clcsock Tony Lu
  2021-11-23 11:50 ` [PATCH net 0/2] Fixes for closing process and minor cleanup patchwork-bot+netdevbpf
  2 siblings, 1 reply; 6+ messages in thread
From: Tony Lu @ 2021-11-23  8:25 UTC (permalink / raw)
  To: kgraul; +Cc: kuba, davem, guwen, netdev, linux-s390, linux-rdma

There remains some variables to replace with local struct sock. So clean
them up all.

Fixes: 3163c5071f25 ("net/smc: use local struct sock variables consistently")
Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
Reviewed-by: Wen Gu <guwen@linux.alibaba.com>
---
 net/smc/smc_close.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index 0f9ffba07d26..9b235fbb089a 100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c
@@ -354,9 +354,9 @@ static void smc_close_passive_work(struct work_struct *work)
 	if (rxflags->peer_conn_abort) {
 		/* peer has not received all data */
 		smc_close_passive_abort_received(smc);
-		release_sock(&smc->sk);
+		release_sock(sk);
 		cancel_delayed_work_sync(&conn->tx_work);
-		lock_sock(&smc->sk);
+		lock_sock(sk);
 		goto wakeup;
 	}
 
-- 
2.32.0.3.g01195cf9f


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net 2/2] net/smc: Ensure the active closing peer first closes clcsock
  2021-11-23  8:25 [PATCH net 0/2] Fixes for closing process and minor cleanup Tony Lu
  2021-11-23  8:25 ` [PATCH net 1/2] net/smc: Clean up local struct sock variables Tony Lu
@ 2021-11-23  8:25 ` Tony Lu
  2021-11-23  8:36   ` Karsten Graul
  2021-11-23 11:50 ` [PATCH net 0/2] Fixes for closing process and minor cleanup patchwork-bot+netdevbpf
  2 siblings, 1 reply; 6+ messages in thread
From: Tony Lu @ 2021-11-23  8:25 UTC (permalink / raw)
  To: kgraul; +Cc: kuba, davem, guwen, netdev, linux-s390, linux-rdma

The side that actively closed socket, it's clcsock doesn't enter
TIME_WAIT state, but the passive side does it. It should show the same
behavior as TCP sockets.

Consider this, when client actively closes the socket, the clcsock in
server enters TIME_WAIT state, which means the address is occupied and
won't be reused before TIME_WAIT dismissing. If we restarted server, the
service would be unavailable for a long time.

To solve this issue, shutdown the clcsock in [A], perform the TCP active
close progress first, before the passive closed side closing it. So that
the actively closed side enters TIME_WAIT, not the passive one.

Client                                            |  Server
close() // client actively close                  |
  smc_release()                                   |
      smc_close_active() // PEERCLOSEWAIT1        |
          smc_close_final() // abort or closed = 1|
              smc_cdc_get_slot_and_msg_send()     |
          [A]                                     |
                                                  |smc_cdc_msg_recv_action() // ACTIVE
                                                  |  queue_work(smc_close_wq, &conn->close_work)
                                                  |    smc_close_passive_work() // PROCESSABORT or APPCLOSEWAIT1
                                                  |      smc_close_passive_abort_received() // only in abort
                                                  |
                                                  |close() // server recv zero, close
                                                  |  smc_release() // PROCESSABORT or APPCLOSEWAIT1
                                                  |    smc_close_active()
                                                  |      smc_close_abort() or smc_close_final() // CLOSED
                                                  |        smc_cdc_get_slot_and_msg_send() // abort or closed = 1
smc_cdc_msg_recv_action()                         |    smc_clcsock_release()
  queue_work(smc_close_wq, &conn->close_work)     |      sock_release(tcp) // actively close clc, enter TIME_WAIT
    smc_close_passive_work() // PEERCLOSEWAIT1    |    smc_conn_free()
      smc_close_passive_abort_received() // CLOSED|
      smc_conn_free()                             |
      smc_clcsock_release()                       |
        sock_release(tcp) // passive close clc    |

Link: https://www.spinics.net/lists/netdev/msg780407.html
Fixes: b38d732477e4 ("smc: socket closing and linkgroup cleanup")
Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
Reviewed-by: Wen Gu <guwen@linux.alibaba.com>
---
 net/smc/smc_close.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index 9b235fbb089a..3715d2f5ad55 100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c
@@ -228,6 +228,12 @@ int smc_close_active(struct smc_sock *smc)
 			/* send close request */
 			rc = smc_close_final(conn);
 			sk->sk_state = SMC_PEERCLOSEWAIT1;
+
+			/* 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);
 		} else {
 			/* peer event has changed the state */
 			goto again;
-- 
2.32.0.3.g01195cf9f


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net 1/2] net/smc: Clean up local struct sock variables
  2021-11-23  8:25 ` [PATCH net 1/2] net/smc: Clean up local struct sock variables Tony Lu
@ 2021-11-23  8:34   ` Karsten Graul
  0 siblings, 0 replies; 6+ messages in thread
From: Karsten Graul @ 2021-11-23  8:34 UTC (permalink / raw)
  To: Tony Lu; +Cc: kuba, davem, guwen, netdev, linux-s390, linux-rdma

On 23/11/2021 09:25, Tony Lu wrote:
> There remains some variables to replace with local struct sock. So clean
> them up all.
> 
> Fixes: 3163c5071f25 ("net/smc: use local struct sock variables consistently")
> Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
> Reviewed-by: Wen Gu <guwen@linux.alibaba.com>
> ---

This is a rather cosmetic change, I will pick it up for our next submission to the 
net-next tree.

Thank you.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net 2/2] net/smc: Ensure the active closing peer first closes clcsock
  2021-11-23  8:25 ` [PATCH net 2/2] net/smc: Ensure the active closing peer first closes clcsock Tony Lu
@ 2021-11-23  8:36   ` Karsten Graul
  0 siblings, 0 replies; 6+ messages in thread
From: Karsten Graul @ 2021-11-23  8:36 UTC (permalink / raw)
  To: Tony Lu; +Cc: kuba, davem, guwen, netdev, linux-s390, linux-rdma

On 23/11/2021 09:25, Tony Lu wrote:
> The side that actively closed socket, it's clcsock doesn't enter
> TIME_WAIT state, but the passive side does it. It should show the same
> behavior as TCP sockets.
> 
> Consider this, when client actively closes the socket, the clcsock in
> server enters TIME_WAIT state, which means the address is occupied and
> won't be reused before TIME_WAIT dismissing. If we restarted server, the
> service would be unavailable for a long time.
> 
> To solve this issue, shutdown the clcsock in [A], perform the TCP active
> close progress first, before the passive closed side closing it. So that
> the actively closed side enters TIME_WAIT, not the passive one.
> 

Thank you, I will pick this up for our next submission to the net tree.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net 0/2] Fixes for closing process and minor cleanup
  2021-11-23  8:25 [PATCH net 0/2] Fixes for closing process and minor cleanup Tony Lu
  2021-11-23  8:25 ` [PATCH net 1/2] net/smc: Clean up local struct sock variables Tony Lu
  2021-11-23  8:25 ` [PATCH net 2/2] net/smc: Ensure the active closing peer first closes clcsock Tony Lu
@ 2021-11-23 11:50 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-11-23 11:50 UTC (permalink / raw)
  To: Tony Lu; +Cc: kgraul, kuba, davem, guwen, netdev, linux-s390, linux-rdma

Hello:

This series was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Tue, 23 Nov 2021 16:25:14 +0800 you wrote:
> Patch 1 is a minor cleanup for local struct sock variables.
> 
> Patch 2 ensures the active closing side enters TIME_WAIT.
> 
> Tony Lu (2):
>   net/smc: Clean up local struct sock variables
>   net/smc: Ensure the active closing peer first closes clcsock
> 
> [...]

Here is the summary with links:
  - [net,1/2] net/smc: Clean up local struct sock variables
    https://git.kernel.org/netdev/net/c/45c3ff7a9ac1
  - [net,2/2] net/smc: Ensure the active closing peer first closes clcsock
    https://git.kernel.org/netdev/net/c/606a63c9783a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-23 11:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-23  8:25 [PATCH net 0/2] Fixes for closing process and minor cleanup Tony Lu
2021-11-23  8:25 ` [PATCH net 1/2] net/smc: Clean up local struct sock variables Tony Lu
2021-11-23  8:34   ` Karsten Graul
2021-11-23  8:25 ` [PATCH net 2/2] net/smc: Ensure the active closing peer first closes clcsock Tony Lu
2021-11-23  8:36   ` Karsten Graul
2021-11-23 11:50 ` [PATCH net 0/2] Fixes for closing process and minor cleanup patchwork-bot+netdevbpf

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.