Re: [PATCH RFC net] net/smc: Ensure the active closing peer first closes clcsock

From: Karsten Graul <kgraul@linux.ibm.com>
To: Tony Lu <tonylu@linux.alibaba.com>
Cc: kuba@kernel.org, davem@davemloft.net, guwen@linux.alibaba.com,
	netdev@vger.kernel.org, linux-s390@vger.kernel.org,
	linux-rdma@vger.kernel.org
Subject: Re: [PATCH RFC net] net/smc: Ensure the active closing peer first closes clcsock
Date: Wed, 24 Nov 2021 11:08:23 +0100	[thread overview]
Message-ID: <1f67548e-cbf6-0dce-82b5-10288a4583bd@linux.ibm.com> (raw)
In-Reply-To: <YZ3+ihxIU5l8mvWY@TonyMac-Alibaba>

On 24/11/2021 09:57, Tony Lu wrote:
> IMHO, given that, it is better to not ignore smc_close_final(), and move 
> kernel_sock_shutdown() to __smc_release(), because smc_shutdown() also
> calls kernel_sock_shutdown() after smc_close_active() and
> smc_close_shutdown_write(), then enters SMC_PEERCLOSEWAIT1. It's no need
> to call it twice with SHUT_WR and SHUT_RDWR. 

Since the idea is to shutdown the socket before the remote peer shutdowns it
first, are you sure that this shutdown in smc_release() is not too late?
Is it sure that smc_release() is called in time for this processing?

Maybe its better to keep the shutdown in smc_close_active() and to use an rc1
just like shown in your proposal, and return either the rc of smc_close_final() 
or the rc of kernel_sock_shutdown().
I see the possibility of calling shutdown twice for the clcsocket, but does it
harm enough to give a reason to check it before in smc_shutdown()? I expect TCP
to handle this already.