Re: [PATCH mptcp-next v1 6/6] mptcp: pm: in-kernel: drop changed parameter of set_flags

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 27/02/2025 07:43, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> To drop the additional "changed" parameter of mptcp_nl_set_flags(),
> store "entry->flags" to "remote->flags" before modifying it in
> mptcp_pm_nl_set_flags(), so that "changed" value can be obtained by
> comparing "local->flags" and "remote->flags" in mptcp_nl_set_flags().

Here as well, no need to change the code: it doesn't make sense to deal
with 'remote' as a workaround to have the same interface that is not
needed from what I understood. No?

> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm_netlink.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 053f2bec9042..2f22c3bf88e2 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1916,13 +1916,16 @@ static void mptcp_pm_nl_fullmesh(struct mptcp_sock *msk,
>  
>  static void mptcp_nl_set_flags(struct net *net,
>  			       struct mptcp_pm_addr_entry *local,
> -			       u8 changed)
> +			       struct mptcp_pm_addr_entry *remote)
>  {
>  	u8 is_subflow = !!(local->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW);
>  	u8 bkup = !!(local->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
> +	u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
> +			   MPTCP_PM_ADDR_FLAG_FULLMESH;
>  	long s_slot = 0, s_num = 0;
>  	struct mptcp_sock *msk;
>  
> +	changed = (local->flags ^ remote->flags) & mask;
>  	if (changed == MPTCP_PM_ADDR_FLAG_FULLMESH && !is_subflow)
>  		return;
>  
> @@ -1987,12 +1990,13 @@ int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
>  		return -EINVAL;
>  	}
>  
> +	remote->flags = entry->flags;

(I guess you wanted to use "remote->flags = changed" or something like
that, but this code should not be modified I think, it doesn't make
sense to deal with a "remote" here)

>  	changed = (local->flags ^ entry->flags) & mask;
>  	entry->flags = (entry->flags & ~mask) | (local->flags & mask);
>  	*local = *entry;
>  	spin_unlock_bh(&pernet->lock);
>  
> -	mptcp_nl_set_flags(net, local, changed);
> +	mptcp_nl_set_flags(net, local, remote);
>  	return 0;
>  }
>  
Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.