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.

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..277f81f38134d07918143331746a50bc316d81ca 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);
 

-- 
2.51.0

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);
>  

Hi Geliang,

On 19/09/2025 05:18, Geliang Tang wrote:
> Hi Matt,
> 
> Thanks for this patch.

Thank you for the review!

> 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.

We have a restriction not to have "fullmesh" with "signal" (even if I
think we should remove it: I think there is no technical reason to
forbid), but we can have "fullmesh" without "subflow".

> So this patch looks good to me.

Thanks!

> 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.

I would prefer not to.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.