Re: [PATCH mptcp-next] mptcp: pm: drop redundant mptcp_pm_send_ack

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 05/04/2025 16:33, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> mptcp_pm_send_ack() is called twice in __mptcp_pm_addr_send_ack(), which
> makes the code a bit redundant. The first call to mptcp_pm_send_ack() when
> a non-stale subflow is found in the loop can be removed. Instead, we can
> break the loop to use the second mptcp_pm_send_ack().
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index be1e27ee393e..a4f9cb113018 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -225,8 +225,8 @@ void mptcp_pm_addr_send_ack(struct mptcp_sock *msk)
>  	mptcp_for_each_subflow(msk, subflow) {
>  		if (__mptcp_subflow_active(subflow)) {
>  			if (!subflow->stale) {
> -				mptcp_pm_send_ack(msk, subflow, false, false);
> -				return;
> +				alt = subflow;
> +				break;

Is this patch going to simplify another one later on? Because the
modification sounds strange: here 'alt' stands for 'alternative'. So to
be complete, better to rename the variable as well.

But... is this really worth it? If you need to modify this helper later
on, maybe. If not, I guess it is fine to keep 'mptcp_pm_send_ack()'
duplicated for the moment. WDYT?

This is also to be aligned with:

 https://docs.kernel.org/process/maintainer-netdev.html#clean-up-patches

(trying to avoid cleanup patches "alone", except if it is part of
another set of changes related to that)

>  			}
>  
>  			if (!alt)
Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.