[MPTCP] Re: [PATCH v2 mptcp-net] mptcp: fix missing wakeup

[MPTCP] Re: [PATCH v2 mptcp-net] mptcp: fix missing wakeup

[Matthieu Baerts]

[-- Attachment #1: Type: text/plain, Size: 921 bytes --]

Hi Paolo,

On 24/02/2021 16:58, Paolo Abeni wrote:
> __mptcp_clean_una() can free write memory and should wake-up
> user-space processes when needed.
> 
> When such function is invoked by the MPTCP receive path, the wakeup
> is not needed, as the TCP stack will later trigger subflow_write_space
> which will do the wakeup as needed.
> 
> Other __mptcp_clean_una() call sites need an additional wakeup check
> Let's bundle the relevant code in a new helper and use it.

Thank you for the patch!

> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/165

After a few iterations of mptcp_join.sh with only the test #11 ("signal 
addresses" - after having modified mptcp_join.sh to do only that), 
everything seems good, thanks for the fix! :-)

Tested-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

[MPTCP] Re: [PATCH v2 mptcp-net] mptcp: fix missing wakeup

[Matthieu Baerts]

[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]

On 24/02/2021 17:20, Matthieu Baerts wrote:
> Hi Paolo,
> 
> On 24/02/2021 16:58, Paolo Abeni wrote:
>> __mptcp_clean_una() can free write memory and should wake-up
>> user-space processes when needed.
>>
>> When such function is invoked by the MPTCP receive path, the wakeup
>> is not needed, as the TCP stack will later trigger subflow_write_space
>> which will do the wakeup as needed.
>>
>> Other __mptcp_clean_una() call sites need an additional wakeup check
>> Let's bundle the relevant code in a new helper and use it.
> 
> Thank you for the patch!
> 
>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/165
> 
> After a few iterations of mptcp_join.sh with only the test #11 ("signal 
> addresses" - after having modified mptcp_join.sh to do only that), 
> everything seems good, thanks for the fix! :-)
> 
> Tested-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>

All good even with the real test #11! ("multiple flows, signal, link 
failure")

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

[MPTCP] Re: [PATCH v2 mptcp-net] mptcp: fix missing wakeup

[Mat Martineau]

[-- Attachment #1: Type: text/plain, Size: 2103 bytes --]

On Wed, 24 Feb 2021, Paolo Abeni wrote:

> __mptcp_clean_una() can free write memory and should wake-up
> user-space processes when needed.
>
> When such function is invoked by the MPTCP receive path, the wakeup
> is not needed, as the TCP stack will later trigger subflow_write_space
> which will do the wakeup as needed.
>
> Other __mptcp_clean_una() call sites need an additional wakeup check
> Let's bundle the relevant code in a new helper and use it.
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/165
> Fixes: 6e628cd3a8f7 ("mptcp: use mptcp release_cb for delayed tasks")
> Fixes: 64b9cea7a0af ("mptcp: fix spurious retransmissions")
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
> ---
> v1 -> v2:
> - drop self-tests changes, slipped here due to PEBKAC

Fix looks good to me too, thanks Paolo.

Reviewed-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>



>
> net/mptcp/protocol.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 2859ad2c6990c..87f62fe66317d 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1061,6 +1061,12 @@ static void __mptcp_clean_una(struct sock *sk)
> 	}
> }
>
> +static void __mptcp_clean_una_wakeup(struct sock *sk)
> +{
> +	__mptcp_clean_una(sk);
> +	mptcp_write_space(sk);
> +}
> +
> static void mptcp_enter_memory_pressure(struct sock *sk)
> {
> 	struct mptcp_subflow_context *subflow;
> @@ -2263,7 +2269,7 @@ static void __mptcp_retrans(struct sock *sk)
> 	struct sock *ssk;
> 	int ret;
>
> -	__mptcp_clean_una(sk);
> +	__mptcp_clean_una_wakeup(sk);
> 	dfrag = mptcp_rtx_head(sk);
> 	if (!dfrag)
> 		return;
> @@ -2983,7 +2989,7 @@ static void mptcp_release_cb(struct sock *sk)
> 	}
>
> 	if (test_and_clear_bit(MPTCP_CLEAN_UNA, &mptcp_sk(sk)->flags))
> -		__mptcp_clean_una(sk);
> +		__mptcp_clean_una_wakeup(sk);
> 	if (test_and_clear_bit(MPTCP_ERROR_REPORT, &mptcp_sk(sk)->flags))
> 		__mptcp_error_report(sk);
>
> -- 
> 2.26.2

--
Mat Martineau
Intel

[MPTCP] Re: [PATCH v2 mptcp-net] mptcp: fix missing wakeup

[Matthieu Baerts]

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

Hi Paolo, Mat,

On 24/02/2021 16:58, Paolo Abeni wrote:
> __mptcp_clean_una() can free write memory and should wake-up
> user-space processes when needed.
> 
> When such function is invoked by the MPTCP receive path, the wakeup
> is not needed, as the TCP stack will later trigger subflow_write_space
> which will do the wakeup as needed.
> 
> Other __mptcp_clean_una() call sites need an additional wakeup check
> Let's bundle the relevant code in a new helper and use it.

Thank you for the patch and the review! Now in our tree with Mat's RvB tag:

- 72f44322c161: mptcp: fix missing wakeup
- Results: 12ba6e545134..de59595c18b3

Tests + export are in progress!

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net