Re: [PATCH mptcp-next v4 1/2] mptcp: update mptcp_push_release

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

On Sun, 11 Dec 2022, Geliang Tang wrote:

> This patch moves the NULL pointer check into mptcp_push_release(). Also
> add a new parameter 'push' for it to set whether to invoke tcp_push in
> it.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> net/mptcp/protocol.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index b3e5d30adbe1..2342b9469181 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1470,9 +1470,17 @@ struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
> 	return ssk;
> }
>
> -static void mptcp_push_release(struct sock *ssk, struct mptcp_sendmsg_info *info)
> +static void mptcp_push_release(struct sock *ssk,
> +			       struct mptcp_sendmsg_info *info,
> +			       bool push)
> {
> -	tcp_push(ssk, 0, info->mss_now, tcp_sk(ssk)->nonagle, info->size_goal);
> +	if (!ssk)
> +		return;
> +
> +	if (push) {
> +		tcp_push(ssk, 0, info->mss_now, tcp_sk(ssk)->nonagle,
> +			 info->size_goal);
> +	}
> 	release_sock(ssk);
> }
>
> @@ -1578,8 +1586,7 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
> 					/* First check. If the ssk has changed since
> 					 * the last round, release prev_ssk
> 					 */
> -					if (prev_ssk)
> -						mptcp_push_release(prev_ssk, &info);
> +					mptcp_push_release(prev_ssk, &info, do_check_data_fin);
>
> 					/* Need to lock the new subflow only if different
> 					 * from the previous one, otherwise we are still
> @@ -1605,8 +1612,7 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
> 	}
>
> 	/* at this point we held the socket lock for the last subflow we used */
> -	if (ssk)
> -		mptcp_push_release(ssk, &info);
> +	mptcp_push_release(ssk, &info, do_check_data_fin);

If do_check_data_fin is 'true', that means __subflow_push_pending() 
succeeded on some subflow (not necessarily ssk).

Seems like the intent is to only call tcp_push() within 
mptcp_push_release() if data was sent using _that_ ssk. So I think that 
needs to be tracked differently.

>
> 	/* ensure the rtx timer is running */
> 	if (!mptcp_timer_pending(sk))
> -- 
> 2.35.3
>
>
>

--
Mat Martineau
Intel