[PATCH mptcp-next] mptcp: pm: in-kernel: remove mptcp_pm_has_addr_attr_id

[PATCH mptcp-next] mptcp: pm: in-kernel: remove mptcp_pm_has_addr_attr_id

[Matthieu Baerts (NGI0)]

There is no need to call this helper: it will check if the address ID
attribute is set, but this attribute has already been parsed previously.

Indeed, the value has been set in 'entry->addr.id' if it was set and
positive, which is what we were looking at. Then only looking at this
already parsed value is enough, not need to re-extract all Netlink
attributes again.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Seen when inspecting the code, related to ticket 612/615.
---
 net/mptcp/pm_kernel.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 87e37c729f81..ba15fbf2781f 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -711,7 +711,7 @@ static void __mptcp_pm_release_addr_entry(struct mptcp_pm_addr_entry *entry)
 
 static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 					     struct mptcp_pm_addr_entry *entry,
-					     bool needs_id, bool replace)
+					     bool replace)
 {
 	struct mptcp_pm_addr_entry *cur, *del_entry = NULL;
 	int ret = -EINVAL;
@@ -770,7 +770,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 		}
 	}
 
-	if (!entry->addr.id && needs_id) {
+	if (!entry->addr.id) {
 find_next:
 		entry->addr.id = find_next_zero_bit(pernet->id_bitmap,
 						    MPTCP_PM_MAX_ADDR_ID + 1,
@@ -781,7 +781,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 		}
 	}
 
-	if (!entry->addr.id && needs_id)
+	if (!entry->addr.id)
 		goto out;
 
 	__set_bit(entry->addr.id, pernet->id_bitmap);
@@ -914,7 +914,7 @@ static int mptcp_pm_kernel_get_local_id(struct mptcp_sock *msk,
 		return -ENOMEM;
 
 	entry->addr.port = 0;
-	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry, true, false);
+	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry, false);
 	if (ret < 0)
 		kfree(entry);
 
@@ -969,18 +969,6 @@ static int mptcp_nl_add_subflow_or_signal_addr(struct net *net,
 	return 0;
 }
 
-static bool mptcp_pm_has_addr_attr_id(const struct nlattr *attr,
-				      struct genl_info *info)
-{
-	struct nlattr *tb[MPTCP_PM_ADDR_ATTR_MAX + 1];
-
-	if (!nla_parse_nested_deprecated(tb, MPTCP_PM_ADDR_ATTR_MAX, attr,
-					 mptcp_pm_address_nl_policy, info->extack) &&
-	    tb[MPTCP_PM_ADDR_ATTR_ID])
-		return true;
-	return false;
-}
-
 /* Add an MPTCP endpoint */
 int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
 {
@@ -1029,9 +1017,7 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
 			goto out_free;
 		}
 	}
-	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry,
-						!mptcp_pm_has_addr_attr_id(attr, info),
-						true);
+	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry, true);
 	if (ret < 0) {
 		GENL_SET_ERR_MSG_FMT(info, "too many addresses or duplicate one: %d", ret);
 		goto out_free;

---
base-commit: db54d4a79a413b2702191c0dc1d855bc3c9e1a98
change-id: 20260223-mptcp_pm_has_addr_attr_id-6bb52e3bd89a

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>

Re: [PATCH mptcp-next] mptcp: pm: in-kernel: remove mptcp_pm_has_addr_attr_id

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/22318311107

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/6e8955d8b020
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1056692


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH mptcp-next] mptcp: pm: in-kernel: remove mptcp_pm_has_addr_attr_id

[Geliang Tang]

Hi Matt,

Thanks for this patch.

On Mon, 2026-02-23 at 18:35 +0100, Matthieu Baerts (NGI0) wrote:
> There is no need to call this helper: it will check if the address ID
> attribute is set, but this attribute has already been parsed
> previously.
> 
> Indeed, the value has been set in 'entry->addr.id' if it was set and
> positive, which is what we were looking at. Then only looking at this
> already parsed value is enough, not need to re-extract all Netlink
> attributes again.

Yes indeed.

    Reviewed-by: Geliang Tang <geliang@kernel.org>

-Geliang

> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> Seen when inspecting the code, related to ticket 612/615.
> ---
>  net/mptcp/pm_kernel.c | 24 +++++-------------------
>  1 file changed, 5 insertions(+), 19 deletions(-)
> 
> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index 87e37c729f81..ba15fbf2781f 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -711,7 +711,7 @@ static void __mptcp_pm_release_addr_entry(struct
> mptcp_pm_addr_entry *entry)
>  
>  static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet
> *pernet,
>  					     struct
> mptcp_pm_addr_entry *entry,
> -					     bool needs_id, bool
> replace)
> +					     bool replace)
>  {
>  	struct mptcp_pm_addr_entry *cur, *del_entry = NULL;
>  	int ret = -EINVAL;
> @@ -770,7 +770,7 @@ static int
> mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
>  		}
>  	}
>  
> -	if (!entry->addr.id && needs_id) {
> +	if (!entry->addr.id) {
>  find_next:
>  		entry->addr.id = find_next_zero_bit(pernet-
> >id_bitmap,
>  						   
> MPTCP_PM_MAX_ADDR_ID + 1,
> @@ -781,7 +781,7 @@ static int
> mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
>  		}
>  	}
>  
> -	if (!entry->addr.id && needs_id)
> +	if (!entry->addr.id)
>  		goto out;
>  
>  	__set_bit(entry->addr.id, pernet->id_bitmap);
> @@ -914,7 +914,7 @@ static int mptcp_pm_kernel_get_local_id(struct
> mptcp_sock *msk,
>  		return -ENOMEM;
>  
>  	entry->addr.port = 0;
> -	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry, true,
> false);
> +	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry,
> false);
>  	if (ret < 0)
>  		kfree(entry);
>  
> @@ -969,18 +969,6 @@ static int
> mptcp_nl_add_subflow_or_signal_addr(struct net *net,
>  	return 0;
>  }
>  
> -static bool mptcp_pm_has_addr_attr_id(const struct nlattr *attr,
> -				      struct genl_info *info)
> -{
> -	struct nlattr *tb[MPTCP_PM_ADDR_ATTR_MAX + 1];
> -
> -	if (!nla_parse_nested_deprecated(tb, MPTCP_PM_ADDR_ATTR_MAX,
> attr,
> -					 mptcp_pm_address_nl_policy,
> info->extack) &&
> -	    tb[MPTCP_PM_ADDR_ATTR_ID])
> -		return true;
> -	return false;
> -}
> -
>  /* Add an MPTCP endpoint */
>  int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info
> *info)
>  {
> @@ -1029,9 +1017,7 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff
> *skb, struct genl_info *info)
>  			goto out_free;
>  		}
>  	}
> -	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry,
> -
> 						!mptcp_pm_has_addr_attr_id(attr, info),
> -						true);
> +	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry,
> true);
>  	if (ret < 0) {
>  		GENL_SET_ERR_MSG_FMT(info, "too many addresses or
> duplicate one: %d", ret);
>  		goto out_free;
> 
> ---
> base-commit: db54d4a79a413b2702191c0dc1d855bc3c9e1a98
> change-id: 20260223-mptcp_pm_has_addr_attr_id-6bb52e3bd89a
> 
> Best regards,

Re: [PATCH mptcp-next] mptcp: pm: in-kernel: remove mptcp_pm_has_addr_attr_id

[Matthieu Baerts]

Hi Geliang,

On 26/02/2026 02:56, Geliang Tang wrote:
> Hi Matt,
> 
> Thanks for this patch.
> 
> On Mon, 2026-02-23 at 18:35 +0100, Matthieu Baerts (NGI0) wrote:
>> There is no need to call this helper: it will check if the address ID
>> attribute is set, but this attribute has already been parsed
>> previously.
>>
>> Indeed, the value has been set in 'entry->addr.id' if it was set and
>> positive, which is what we were looking at. Then only looking at this
>> already parsed value is enough, not need to re-extract all Netlink
>> attributes again.
> 
> Yes indeed.
> 
>     Reviewed-by: Geliang Tang <geliang@kernel.org>

Thank you for the review!

New patches for t/upstream:
- 34089fd1812f: mptcp: pm: in-kernel: remove mptcp_pm_has_addr_attr_id
- Results: c0858783271a..cc839747fb54 (export)

Tests are now in progress:

- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/0a6bf9713531722bee20a1f3d24ad42c68e18206/checks

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

Re: [PATCH mptcp-next] mptcp: pm: in-kernel: remove mptcp_pm_has_addr_attr_id

[Matthieu Baerts]

Hello,

On 23/02/2026 18:35, Matthieu Baerts (NGI0) wrote:
> There is no need to call this helper: it will check if the address ID
> attribute is set, but this attribute has already been parsed previously.
> 
> Indeed, the value has been set in 'entry->addr.id' if it was set and
> positive, which is what we were looking at. Then only looking at this
> already parsed value is enough, not need to re-extract all Netlink
> attributes again.
Linked to my message on the same patch sent to netdev [1] ...

 https://lore.kernel.org/070ec312-4836-4220-b03f-07c14f1f5470@kernel.org

... I suggest replacing the commit title and message here:

------------------------- 8< -------------------------
Revert "mptcp: add needs_id for netlink appending addr"

This commit was originally adding the ability to add MPTCP endpoints
with ID 0 by accident. The in-kernel PM, handling MPTCP endpoints at the
net namespace level, is not supposed to handle endpoints with such ID,
because this ID 0 is reserved to the initial subflow as mentioned in the
MPTCPv1 protocol, a per-connection setting.

Note that 'ip mptcp endpoint add id 0' stops early with an error, but
other tools might still ask to create MPTCP endpoint with the ID 0.

In other words, it was wrong to call the mptcp_pm_has_addr_attr_id
helper to check whether the address ID attribute is set: if it was set
to 0, a new MPTCP endpoint would be created with ID 0, which is not
expected, and might cause various issues later (even if in most cases,
such endpoint is ignored).

Fixes: 584f38942626 ("mptcp: add needs_id for netlink appending addr")
------------------------- 8< -------------------------

Any objections?

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