[PATCH mptcp-next] mptcp: add do_check_data_fin to replace copied

[PATCH mptcp-next] mptcp: add do_check_data_fin to replace copied

[Geliang Tang]

This patch adds a new bool variable 'do_check_data_fin' to replace the
original int variable 'copied' in __mptcp_push_pending(), check it to
determine whether to call __mptcp_check_send_data_fin().

Suggested-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 Do a small cleanup as Mat suggested in [1].

[1]
https://patchwork.kernel.org/project/mptcp/patch/ef3d52ff98c97b3c3e5b1efd84a1d4bf3448df5d.1656578856.git.geliang.tang@suse.com/
---
 net/mptcp/protocol.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 74699bd47edf..fb574d135002 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1473,8 +1473,9 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
 	struct mptcp_sendmsg_info info = {
 				.flags = flags,
 	};
+	bool do_check_data_fin = false;
 	struct mptcp_data_frag *dfrag;
-	int len, copied = 0;
+	int len;
 
 	while ((dfrag = mptcp_send_head(sk))) {
 		info.sent = dfrag->already_sent;
@@ -1510,7 +1511,7 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
 			}
 
 			info.sent += ret;
-			copied += ret;
+			do_check_data_fin = true;
 			len -= ret;
 
 			mptcp_update_post_push(msk, dfrag, ret);
@@ -1526,7 +1527,7 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
 	/* ensure the rtx timer is running */
 	if (!mptcp_timer_pending(sk))
 		mptcp_reset_timer(sk);
-	if (copied)
+	if (do_check_data_fin)
 		__mptcp_check_send_data_fin(sk);
 }
 
-- 
2.35.3

Re: mptcp: add do_check_data_fin to replace copied: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Unstable: 2 failed test(s): selftest_mptcp_join selftest_simult_flows 🔴:
  - Task: https://cirrus-ci.com/task/4966947568746496
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4966947568746496/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 2 failed test(s): packetdrill_mp_join packetdrill_sockopts 🔴:
  - Task: https://cirrus-ci.com/task/6092847475589120
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6092847475589120/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/90bc7455e572


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)

Re: [PATCH mptcp-next] mptcp: add do_check_data_fin to replace copied

[Matthieu Baerts]

Hi Geliang,

On 23/08/2022 14:52, Geliang Tang wrote:
> This patch adds a new bool variable 'do_check_data_fin' to replace the
> original int variable 'copied' in __mptcp_push_pending(), check it to
> determine whether to call __mptcp_check_send_data_fin().

Thank you for this clean-up! (and good to see you around again!)
Sorry for the delay, I had a few days off but it is now in our tree
(feat. for net-next):

New patches for t/upstream:
- 695cc660c6c6: mptcp: add do_check_data_fin to replace copied
- Results: 276e5dcffc80..0838c359eb16 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220829T154355

> @@ -1510,7 +1511,7 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
>  			}
>  
>  			info.sent += ret;
> -			copied += ret;
> +			do_check_data_fin = true;

Note that I just moved this new line one line above to do the two
operations with 'ret' together.

>  			len -= ret;
>  
>  			mptcp_update_post_push(msk, dfrag, ret);

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