Re: [PATCH mptcp-next v7 06/11] mptcp: drop info of userspace_pm_remove_id_zero_address

[Matthieu Baerts <matttbe@kernel.org>]

On 06/01/2025 09:16, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> The only use of 'info' parameter of userspace_pm_remove_id_zero_address()
> is to set an error message into it.

Can you add:

Plus, this helper will only fail when it cannot find any subflows with a
local address ID 0.

(Because if there were different types of errors, you could not drop
'info').

> 
> This patch drops this parameter and sets the error message where this
> function is called in mptcp_pm_nl_remove_doit().
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm_userspace.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 0f703e06d771..a6580942e628 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -259,8 +259,7 @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
>  	return err;
>  }
>  
> -static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
> -						     struct genl_info *info)
> +static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk)
>  {
>  	struct mptcp_rm_list list = { .nr = 0 };
>  	struct mptcp_subflow_context *subflow;
> @@ -275,10 +274,8 @@ static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
>  			break;
>  		}
>  	}
> -	if (!has_id_0) {
> -		GENL_SET_ERR_MSG(info, "address with id 0 not found");
> +	if (!has_id_0)
>  		goto remove_err;
> -	}
>  
>  	list.ids[list.nr++] = 0;
>  
> @@ -336,7 +333,7 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
>  	sk = (struct sock *)msk;
>  
>  	if (id_val == 0) {
> -		err = mptcp_userspace_pm_remove_id_zero_address(msk, info);
> +		err = mptcp_userspace_pm_remove_id_zero_address(msk);
>  		goto out;
>  	}
>  
> @@ -345,7 +342,6 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
>  	spin_lock_bh(&msk->pm.lock);
>  	match = mptcp_userspace_pm_lookup_addr_by_id(msk, id_val);
>  	if (!match) {
> -		GENL_SET_ERR_MSG(info, "address with specified id not found");
>  		spin_unlock_bh(&msk->pm.lock);
>  		release_sock(sk);
>  		goto out;
> @@ -362,6 +358,11 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
>  
>  	err = 0;
>  out:
> +	if (err)
> +		GENL_SET_ERR_MSG_FMT(info,

Can you not use NL_SET_ERR_MSG_ATTR_FMT() here?

> +				     "address with id %u not found",
> +				     id_val);

Detail: in fact, I don't think it is needed to specify the ID in the
error message to send to the userspace: the ID has been specified by the
userspace, it knows what it has set. Up to you.

> +
>  	sock_put(sk);
>  	return err;
>  }

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