Re: [PATCH mptcp-next 01/10] Squash to "mptcp: pm: in-kernel: usable client side with C-flag"

[Geliang Tang <geliang@kernel.org>]

Hi Matt,

Thanks for this patch.

On Thu, 2025-09-18 at 19:42 +0200, Matthieu Baerts (NGI0) wrote:
> The id_avail_bitmap is only used when either the 'subflow' or
> 'signal'
> flag is used, but not with 'fullmesh' only. Here, it is replacing the
> 'subflow' action, so check if this flag is set.

I recall that 'fullmesh' can only be used together with 'subflow'. If
it has already been determined earlier that the flags contain
'fullmesh', shouldn't the flags here already include 'subflow'? I'm not
entirely certain, but directly checking for 'subflow' as done in this
patch seems like a better approach.

So this patch looks good to me.

As mentioned in my previous comment, if possible, the refactoring of
the fill_local_addresses_vec_c_flag helper in patch 2 could also be
added to this squash-to patch.

Thanks,
-Geliang

> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/pm_kernel.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index
> d7cd89fa6a11a1ea7703edbfbdf2bbe86a6a3054..277f81f38134d07918143331746
> a50bc316d81ca 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -411,7 +411,8 @@ static unsigned int
> fill_local_addresses_vec(struct mptcp_sock *msk,
>  			locals[i].flags = entry->flags;
>  			locals[i].ifindex = entry->ifindex;
>  
> -			if (c_flag_case)
> +			if (c_flag_case &&
> +			    (entry->flags &
> MPTCP_PM_ADDR_FLAG_SUBFLOW))
>  				__clear_bit(locals[i].addr.id,
>  					    msk-
> >pm.id_avail_bitmap);
>