[PATCH mptcp-next v4 02/11] mptcp: add new argument ssk_first

Geliang Tang posted 11 patches 3 years, 2 months ago
Maintainers: Mat Martineau <mathew.j.martineau@linux.intel.com>, Matthieu Baerts <matthieu.baerts@tessares.net>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
There is a newer version of this series
[PATCH mptcp-next v4 02/11] mptcp: add new argument ssk_first
Posted by Geliang Tang 3 years, 2 months ago
The function mptcp_subflow_process_delegated() uses the input ssk first,
while __mptcp_check_push() invokes the packet scheduler first.

So this patch adds a new argument named ssk_first for the function
__mptcp_subflow_push_pending() to deal with these two cases separately.

With this change, the code that invokes the packet scheduler in the
fuction __mptcp_check_push() can be removed, and replaced by invoking
__mptcp_subflow_push_pending() directly.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/protocol.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index cc8e67543e76..bbc43212a20f 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1593,16 +1593,17 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
 		__mptcp_check_send_data_fin(sk);
 }
 
-static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk)
+static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk,
+					 bool ssk_first)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
 	struct mptcp_sendmsg_info info = {
 		.data_lock_held = true,
 	};
 	struct mptcp_data_frag *dfrag;
+	bool first = ssk_first;
 	struct sock *xmit_ssk;
 	int len, copied = 0;
-	bool first = true;
 
 	info.flags = 0;
 	while ((dfrag = mptcp_send_head(sk))) {
@@ -1612,8 +1613,7 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk)
 		while (len > 0) {
 			int ret = 0;
 
-			/* the caller already invoked the packet scheduler,
-			 * check for a different subflow usage only after
+			/* check for a different subflow usage only after
 			 * spooling the first chunk of data
 			 */
 			xmit_ssk = first ? ssk : mptcp_subflow_get_send(mptcp_sk(sk));
@@ -3195,16 +3195,10 @@ void __mptcp_check_push(struct sock *sk, struct sock *ssk)
 	if (!mptcp_send_head(sk))
 		return;
 
-	if (!sock_owned_by_user(sk)) {
-		struct sock *xmit_ssk = mptcp_subflow_get_send(mptcp_sk(sk));
-
-		if (xmit_ssk == ssk)
-			__mptcp_subflow_push_pending(sk, ssk);
-		else if (xmit_ssk)
-			mptcp_subflow_delegate(mptcp_subflow_ctx(xmit_ssk), MPTCP_DELEGATE_SEND);
-	} else {
+	if (!sock_owned_by_user(sk))
+		__mptcp_subflow_push_pending(sk, ssk, false);
+	else
 		__set_bit(MPTCP_PUSH_PENDING, &mptcp_sk(sk)->cb_flags);
-	}
 }
 
 #define MPTCP_FLAGS_PROCESS_CTX_NEED (BIT(MPTCP_PUSH_PENDING) | \
@@ -3295,7 +3289,7 @@ void mptcp_subflow_process_delegated(struct sock *ssk)
 	if (test_bit(MPTCP_DELEGATE_SEND, &subflow->delegated_status)) {
 		mptcp_data_lock(sk);
 		if (!sock_owned_by_user(sk))
-			__mptcp_subflow_push_pending(sk, ssk);
+			__mptcp_subflow_push_pending(sk, ssk, true);
 		else
 			__set_bit(MPTCP_PUSH_PENDING, &mptcp_sk(sk)->cb_flags);
 		mptcp_data_unlock(sk);
-- 
2.35.3
Re: [PATCH mptcp-next v4 02/11] mptcp: add new argument ssk_first
Posted by Mat Martineau 3 years, 2 months ago
On Sun, 2 Oct 2022, Geliang Tang wrote:

> The function mptcp_subflow_process_delegated() uses the input ssk first,
> while __mptcp_check_push() invokes the packet scheduler first.
>
> So this patch adds a new argument named ssk_first for the function
> __mptcp_subflow_push_pending() to deal with these two cases separately.
>
> With this change, the code that invokes the packet scheduler in the
> fuction __mptcp_check_push() can be removed, and replaced by invoking
> __mptcp_subflow_push_pending() directly.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> net/mptcp/protocol.c | 22 ++++++++--------------
> 1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index cc8e67543e76..bbc43212a20f 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1593,16 +1593,17 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
> 		__mptcp_check_send_data_fin(sk);
> }
>
> -static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk)
> +static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk,
> +					 bool ssk_first)
> {
> 	struct mptcp_sock *msk = mptcp_sk(sk);
> 	struct mptcp_sendmsg_info info = {
> 		.data_lock_held = true,
> 	};
> 	struct mptcp_data_frag *dfrag;
> +	bool first = ssk_first;

Hi Geliang -

Thanks for the v4.

If you add 'first' as the new argument instead of ssk_first, then you can 
remove this line and use 'first' everywhere.


- Mat


> 	struct sock *xmit_ssk;
> 	int len, copied = 0;
> -	bool first = true;
>
> 	info.flags = 0;
> 	while ((dfrag = mptcp_send_head(sk))) {
> @@ -1612,8 +1613,7 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk)
> 		while (len > 0) {
> 			int ret = 0;
>
> -			/* the caller already invoked the packet scheduler,
> -			 * check for a different subflow usage only after
> +			/* check for a different subflow usage only after
> 			 * spooling the first chunk of data
> 			 */
> 			xmit_ssk = first ? ssk : mptcp_subflow_get_send(mptcp_sk(sk));
> @@ -3195,16 +3195,10 @@ void __mptcp_check_push(struct sock *sk, struct sock *ssk)
> 	if (!mptcp_send_head(sk))
> 		return;
>
> -	if (!sock_owned_by_user(sk)) {
> -		struct sock *xmit_ssk = mptcp_subflow_get_send(mptcp_sk(sk));
> -
> -		if (xmit_ssk == ssk)
> -			__mptcp_subflow_push_pending(sk, ssk);
> -		else if (xmit_ssk)
> -			mptcp_subflow_delegate(mptcp_subflow_ctx(xmit_ssk), MPTCP_DELEGATE_SEND);
> -	} else {
> +	if (!sock_owned_by_user(sk))
> +		__mptcp_subflow_push_pending(sk, ssk, false);
> +	else
> 		__set_bit(MPTCP_PUSH_PENDING, &mptcp_sk(sk)->cb_flags);
> -	}
> }
>
> #define MPTCP_FLAGS_PROCESS_CTX_NEED (BIT(MPTCP_PUSH_PENDING) | \
> @@ -3295,7 +3289,7 @@ void mptcp_subflow_process_delegated(struct sock *ssk)
> 	if (test_bit(MPTCP_DELEGATE_SEND, &subflow->delegated_status)) {
> 		mptcp_data_lock(sk);
> 		if (!sock_owned_by_user(sk))
> -			__mptcp_subflow_push_pending(sk, ssk);
> +			__mptcp_subflow_push_pending(sk, ssk, true);
> 		else
> 			__set_bit(MPTCP_PUSH_PENDING, &mptcp_sk(sk)->cb_flags);
> 		mptcp_data_unlock(sk);
> -- 
> 2.35.3
>
>
>

--
Mat Martineau
Intel