Re: [PATCH mptcp-next 4/8] mptcp: set error messages for set_flags

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 21/02/2024 7:31 am, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> In addition to returning the error value, this patch also sets an error
> messages with GENL_SET_ERR_MSG or NL_SET_ERR_MSG_ATTR both for pm_netlink.c
> and pm_userspace.c.

Good idea!

Even if it might be obvious, please *always* explain the reason(s) why
having this is interesting. Here, just one sentence is enough:

  It will help the userspace to identify the issue.

(same for other patches in general: please *always* add the reason(s))

(...)

> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 7ef3b69852f0..09a60f440fef 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -563,13 +563,17 @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
>  	token_val = nla_get_u32(token);
>  
>  	msk = mptcp_token_get_sock(net, token_val);
> -	if (!msk)
> +	if (!msk) {
> +		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
>  		return ret;
> +	}
>  
>  	sk = (struct sock *)msk;
>  
> -	if (!mptcp_pm_is_userspace(msk))
> +	if (!mptcp_pm_is_userspace(msk)) {
> +		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");

Maybe just "userspace PM not selected"?

>  		goto set_flags_err;
> +	}
>  
>  	ret = mptcp_pm_parse_entry(attr, info, false, &loc);
>  	if (ret < 0)
> @@ -583,6 +587,7 @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
>  
>  	if (loc.addr.family == AF_UNSPEC ||
>  	    rem.addr.family == AF_UNSPEC) {
> +		GENL_SET_ERR_MSG(info, "address families do not match");

Maybe better with this?

  "invalid address families"

>  		ret = -EINVAL;
>  		goto set_flags_err;
>  	}

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