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

[Geliang Tang <geliang@kernel.org>]

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,