2022-03-28 19:55:08

by Wen Gu

[permalink] [raw]
Subject: [PATCH net] net/smc: Send out the remaining data in sndbuf before close

The current autocork algorithms will delay the data transmission
in BH context to smc_release_cb() when sock_lock is hold by user.

So there is a possibility that when connection is being actively
closed (sock_lock is hold by user now), some corked data still
remains in sndbuf, waiting to be sent by smc_release_cb(). This
will cause:

- smc_close_stream_wait(), which is called under the sock_lock,
has a high probability of timeout because data transmission is
delayed until sock_lock is released.

- Unexpected data sends may happen after connction closed and use
the rtoken which has been deleted by remote peer through
LLC_DELETE_RKEY messages.

So this patch will try to send out the remaining corked data in
sndbuf before active close process, to ensure data integrity and
avoid unexpected data transmission after close.

Reported-by: Guangguan Wang <[email protected]>
Fixes: 6b88af839d20 ("net/smc: don't send in the BH context if sock_owned_by_user")
Signed-off-by: Wen Gu <[email protected]>
---
net/smc/smc_close.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index 292e4d9..676cb23 100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c
@@ -57,6 +57,9 @@ static void smc_close_stream_wait(struct smc_sock *smc, long timeout)
if (!smc_tx_prepared_sends(&smc->conn))
return;

+ /* Send out corked data remaining in sndbuf */
+ smc_tx_pending(&smc->conn);
+
smc->wait_close_tx_prepared = 1;
add_wait_queue(sk_sleep(sk), &wait);
while (!signal_pending(current) && timeout) {
--
1.8.3.1


2022-03-28 21:47:06

by Dust Li

[permalink] [raw]
Subject: Re: [PATCH net] net/smc: Send out the remaining data in sndbuf before close

On Mon, Mar 28, 2022 at 02:10:36PM +0800, Wen Gu wrote:
>The current autocork algorithms will delay the data transmission
>in BH context to smc_release_cb() when sock_lock is hold by user.
>
>So there is a possibility that when connection is being actively
>closed (sock_lock is hold by user now), some corked data still
>remains in sndbuf, waiting to be sent by smc_release_cb(). This
>will cause:
>
>- smc_close_stream_wait(), which is called under the sock_lock,
> has a high probability of timeout because data transmission is
> delayed until sock_lock is released.
>
>- Unexpected data sends may happen after connction closed and use
> the rtoken which has been deleted by remote peer through
> LLC_DELETE_RKEY messages.
>
>So this patch will try to send out the remaining corked data in
>sndbuf before active close process, to ensure data integrity and
>avoid unexpected data transmission after close.

I think this issue should also happen if TCP_CORK is set and
autocorking is not enabled ?

Autocorking and delaying the TX from BH to smc_release_cb() greatly
increased the probability of this problem.

>
>Reported-by: Guangguan Wang <[email protected]>
>Fixes: 6b88af839d20 ("net/smc: don't send in the BH context if sock_owned_by_user")
>Signed-off-by: Wen Gu <[email protected]>
>---
> net/smc/smc_close.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
>index 292e4d9..676cb23 100644
>--- a/net/smc/smc_close.c
>+++ b/net/smc/smc_close.c
>@@ -57,6 +57,9 @@ static void smc_close_stream_wait(struct smc_sock *smc, long timeout)
> if (!smc_tx_prepared_sends(&smc->conn))
> return;
>
>+ /* Send out corked data remaining in sndbuf */
>+ smc_tx_pending(&smc->conn);
>+
> smc->wait_close_tx_prepared = 1;
> add_wait_queue(sk_sleep(sk), &wait);
> while (!signal_pending(current) && timeout) {
>--
>1.8.3.1

2022-03-28 22:43:48

by Karsten Graul

[permalink] [raw]
Subject: Re: [PATCH net] net/smc: Send out the remaining data in sndbuf before close

On 28/03/2022 08:10, Wen Gu wrote:
> The current autocork algorithms will delay the data transmission
> in BH context to smc_release_cb() when sock_lock is hold by user.
>
> So there is a possibility that when connection is being actively
> closed (sock_lock is hold by user now), some corked data still
> remains in sndbuf, waiting to be sent by smc_release_cb(). This
> will cause:
>
> - smc_close_stream_wait(), which is called under the sock_lock,
> has a high probability of timeout because data transmission is
> delayed until sock_lock is released.
>
> - Unexpected data sends may happen after connction closed and use
> the rtoken which has been deleted by remote peer through
> LLC_DELETE_RKEY messages.
>
> So this patch will try to send out the remaining corked data in
> sndbuf before active close process, to ensure data integrity and
> avoid unexpected data transmission after close.
>
> Reported-by: Guangguan Wang <[email protected]>
> Fixes: 6b88af839d20 ("net/smc: don't send in the BH context if sock_owned_by_user")
> Signed-off-by: Wen Gu <[email protected]>
> ---

Thank you,

Acked-by: Karsten Graul <[email protected]>

2022-03-29 00:00:55

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net] net/smc: Send out the remaining data in sndbuf before close

Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <[email protected]>:

On Mon, 28 Mar 2022 14:10:36 +0800 you wrote:
> The current autocork algorithms will delay the data transmission
> in BH context to smc_release_cb() when sock_lock is hold by user.
>
> So there is a possibility that when connection is being actively
> closed (sock_lock is hold by user now), some corked data still
> remains in sndbuf, waiting to be sent by smc_release_cb(). This
> will cause:
>
> [...]

Here is the summary with links:
- [net] net/smc: Send out the remaining data in sndbuf before close
https://git.kernel.org/netdev/net/c/906b3d64913c

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