[PATCH mptcp-next 0/2] mptcp: some smaller cleanup

[PATCH mptcp-next 0/2] mptcp: some smaller cleanup

[Paolo Abeni]

This are 2 smallish pre-req for the RX path refactor.

I think they can live of their own merit, so sending them
out now.

Paolo Abeni (2):
  mptcp: don't clear MPTCP_DATA_READY in sk_wait_event()
  mptcp: drop rendundant test in move_skbs_to_msk()

 net/mptcp/protocol.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

-- 
2.26.3

[PATCH mptcp-next 1/2] mptcp: don't clear MPTCP_DATA_READY in sk_wait_event()

[Paolo Abeni]

If we don't flush entirely the receive queue, we need set
again such bit later. We can simply avoid clearing it.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 1e77d2defd28..a08ea8867716 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1711,7 +1711,7 @@ static void mptcp_wait_data(struct sock *sk, long *timeo)
 	sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
 
 	sk_wait_event(sk, timeo,
-		      test_and_clear_bit(MPTCP_DATA_READY, &msk->flags), &wait);
+		      test_bit(MPTCP_DATA_READY, &msk->flags), &wait);
 
 	sk_clear_bit(SOCKWQ_ASYNC_WAITDATA, sk);
 	remove_wait_queue(sk_sleep(sk), &wait);
@@ -2032,10 +2032,8 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 		 */
 		if (unlikely(__mptcp_move_skbs(msk)))
 			set_bit(MPTCP_DATA_READY, &msk->flags);
-	} else if (unlikely(!test_bit(MPTCP_DATA_READY, &msk->flags))) {
-		/* data to read but mptcp_wait_data() cleared DATA_READY */
-		set_bit(MPTCP_DATA_READY, &msk->flags);
 	}
+
 out_err:
 	if (cmsg_flags && copied >= 0) {
 		if (cmsg_flags & MPTCP_CMSG_TS)
-- 
2.26.3

[PATCH mptcp-next 2/2] mptcp: drop rendundant test in move_skbs_to_msk()

[Paolo Abeni]

Currently we check the msk state to avoid enqueuing new
skbs at msk shutdown time.

Such test is racy - as we can't acquire the msk socket lock -
and useless, as the caller already checked the subflow
field 'disposable', covering the same scenario in a race
free manner - read and updated under the ssk socket lock.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index a08ea8867716..4ac55e696f52 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -686,9 +686,6 @@ static bool move_skbs_to_msk(struct mptcp_sock *msk, struct sock *ssk)
 	struct sock *sk = (struct sock *)msk;
 	unsigned int moved = 0;
 
-	if (inet_sk_state_load(sk) == TCP_CLOSE)
-		return false;
-
 	mptcp_data_lock(sk);
 
 	__mptcp_move_skbs_from_subflow(msk, ssk, &moved);
-- 
2.26.3

Re: [PATCH mptcp-next 0/2] mptcp: some smaller cleanup

[Mat Martineau]

On Thu, 27 May 2021, Paolo Abeni wrote:

> This are 2 smallish pre-req for the RX path refactor.
>
> I think they can live of their own merit, so sending them
> out now.
>
> Paolo Abeni (2):
>  mptcp: don't clear MPTCP_DATA_READY in sk_wait_event()
>  mptcp: drop rendundant test in move_skbs_to_msk()
>
> net/mptcp/protocol.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> -- 
> 2.26.3

Series looks good to me.

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next 0/2] mptcp: some smaller cleanup

[Matthieu Baerts]

Hi Paolo, Mat,

On 27/05/2021 18:53, Paolo Abeni wrote:
> This are 2 smallish pre-req for the RX path refactor.
> 
> I think they can live of their own merit, so sending them
> out now.
> 
> Paolo Abeni (2):
>   mptcp: don't clear MPTCP_DATA_READY in sk_wait_event()
>   mptcp: drop rendundant test in move_skbs_to_msk()

Thank you for the patches and the review!

Now in our tree:

- 8fefe3b1f9ec: mptcp: don't clear MPTCP_DATA_READY in sk_wait_event()
- 61f6d7b1a174: mptcp: drop rendundant test in move_skbs_to_msk()
- Results: e6629296aed2..a4f45379c78f

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20210607T151231
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20210607T151231

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