diff mbox series

[mptcp-next,v6,1/7] mptcp: pm: use NL_SET_ERR_MSG_ATTR when possible

Message ID 20241230-genl_req_attr_check-v6-1-3ec9103559e7@kernel.org (mailing list archive)
State Superseded
Delegated to: Geliang Tang
Headers show
Series mptcp: use GENL_REQ_ATTR_CHECK in userspace pm | expand

Checks

Context Check Description
matttbe/checkpatch success total: 0 errors, 0 warnings, 0 checks, 160 lines checked
matttbe/shellcheck success MPTCP selftests files have not been modified
matttbe/build success Build and static analysis OK
matttbe/KVM_Validation__normal success Success! ✅
matttbe/KVM_Validation__debug success Success! ✅
matttbe/KVM_Validation__btf-normal__only_bpftest_all_ success Success! ✅
matttbe/KVM_Validation__btf-debug__only_bpftest_all_ warning Unstable: 1 failed test(s): bpftest_test_progs-no_alu32_mptcp

Commit Message

Matthieu Baerts Dec. 30, 2024, 1:24 p.m. UTC 
Instead of only returning a text message with GENL_SET_ERR_MSG(),
NL_SET_ERR_MSG_ATTR() can help the userspace developers by also
reporting which attribute is faulty.

When the error is specific to an attribute, NL_SET_ERR_MSG_ATTR() is now
used. The error messages have not been modified in this commit.

mptcp_userspace_pm_remove_id_zero_address() has been modified to set the
missing attribute.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_netlink.c   | 23 ++++++++++++++---------
 net/mptcp/pm_userspace.c | 27 +++++++++++++++++----------
 2 files changed, 31 insertions(+), 19 deletions(-)

Comments

Geliang Tang Jan. 6, 2025, 8:27 a.m. UTC | #1 
On Mon, 2024-12-30 at 14:24 +0100, Matthieu Baerts (NGI0) wrote:
> Instead of only returning a text message with GENL_SET_ERR_MSG(),
> NL_SET_ERR_MSG_ATTR() can help the userspace developers by also
> reporting which attribute is faulty.
> 
> When the error is specific to an attribute, NL_SET_ERR_MSG_ATTR() is
> now
> used. The error messages have not been modified in this commit.
> 
> mptcp_userspace_pm_remove_id_zero_address() has been modified to set

A new patch to drop "info" parameter of this function is added in v7.

> the
> missing attribute.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/pm_netlink.c   | 23 ++++++++++++++---------
>  net/mptcp/pm_userspace.c | 27 +++++++++++++++++----------
>  2 files changed, 31 insertions(+), 19 deletions(-)
> 
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index
> 98ac73938bd8196e196d5ee8c264784ba8d37645..49c34494e156ff6c3277b107c38
> 40ee29dd3afbb 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1403,18 +1403,21 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff
> *skb, struct genl_info *info)
>  		return ret;
>  
>  	if (addr.addr.port && !address_use_port(&addr)) {
> -		GENL_SET_ERR_MSG(info, "flags must have signal and
> not subflow when using port");
> +		NL_SET_ERR_MSG_ATTR(info->extack, attr,
> +				    "flags must have signal and not
> subflow when using port");
>  		return -EINVAL;
>  	}
>  
>  	if (addr.flags & MPTCP_PM_ADDR_FLAG_SIGNAL &&
>  	    addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) {
> -		GENL_SET_ERR_MSG(info, "flags mustn't have both
> signal and fullmesh");
> +		NL_SET_ERR_MSG_ATTR(info->extack, attr,
> +				    "flags mustn't have both signal
> and fullmesh");
>  		return -EINVAL;
>  	}
>  
>  	if (addr.flags & MPTCP_PM_ADDR_FLAG_IMPLICIT) {
> -		GENL_SET_ERR_MSG(info, "can't create IMPLICIT
> endpoint");
> +		NL_SET_ERR_MSG_ATTR(info->extack, attr,
> +				    "can't create IMPLICIT
> endpoint");
>  		return -EINVAL;
>  	}
>  
> @@ -1608,7 +1611,7 @@ int mptcp_pm_nl_del_addr_doit(struct sk_buff
> *skb, struct genl_info *info)
>  	spin_lock_bh(&pernet->lock);
>  	entry = __lookup_addr_by_id(pernet, addr.addr.id);
>  	if (!entry) {
> -		GENL_SET_ERR_MSG(info, "address not found");
> +		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not
> found");
>  		spin_unlock_bh(&pernet->lock);
>  		return -EINVAL;
>  	}
> @@ -1790,7 +1793,7 @@ int mptcp_pm_nl_get_addr(struct sk_buff *skb,
> struct genl_info *info)
>  	rcu_read_lock();
>  	entry = __lookup_addr_by_id(pernet, addr.addr.id);
>  	if (!entry) {
> -		GENL_SET_ERR_MSG(info, "address not found");
> +		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not
> found");
>  		ret = -EINVAL;
>  		goto unlock_fail;
>  	}
> @@ -1875,7 +1878,8 @@ static int parse_limit(struct genl_info *info,
> int id, unsigned int *limit)
>  
>  	*limit = nla_get_u32(attr);
>  	if (*limit > MPTCP_PM_ADDR_MAX) {
> -		GENL_SET_ERR_MSG(info, "limit greater than
> maximum");
> +		NL_SET_ERR_MSG_ATTR(info->extack, attr,
> +				    "limit greater than maximum");
>  		return -EINVAL;
>  	}
>  	return 0;
> @@ -2003,7 +2007,8 @@ int mptcp_pm_nl_set_flags(struct sk_buff *skb,
> struct genl_info *info)
>  	if (addr.addr.family == AF_UNSPEC) {
>  		lookup_by_id = 1;
>  		if (!addr.addr.id) {
> -			GENL_SET_ERR_MSG(info, "missing required
> inputs");
> +			NL_SET_ERR_MSG_ATTR(info->extack, attr,
> +					    "missing required
> inputs");
>  			return -EOPNOTSUPP;
>  		}
>  	}
> @@ -2016,13 +2021,13 @@ int mptcp_pm_nl_set_flags(struct sk_buff
> *skb, struct genl_info *info)
>  			       __lookup_addr(pernet, &addr.addr);
>  	if (!entry) {
>  		spin_unlock_bh(&pernet->lock);
> -		GENL_SET_ERR_MSG(info, "address not found");
> +		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not
> found");
>  		return -EINVAL;
>  	}
>  	if ((addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
>  	    (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
>  		spin_unlock_bh(&pernet->lock);
> -		GENL_SET_ERR_MSG(info, "invalid addr flags");
> +		NL_SET_ERR_MSG_ATTR(info->extack, attr, "invalid
> addr flags");
>  		return -EINVAL;
>  	}
>  
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index
> a3d477059b11c3a5618dbb6256434a8e55845995..72d4861e497eef5a516fc1c3ec1
> 659807ffedd53 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -190,7 +190,8 @@ static struct mptcp_sock
> *mptcp_userspace_pm_get_sock(const struct genl_info *in
>  	}
>  
>  	if (!mptcp_pm_is_userspace(msk)) {
> -		GENL_SET_ERR_MSG(info, "invalid request; userspace
> PM not selected");
> +		NL_SET_ERR_MSG_ATTR(info->extack, token,
> +				    "invalid request; userspace PM
> not selected");
>  		sock_put((struct sock *)msk);
>  		return NULL;
>  	}
> @@ -224,14 +225,16 @@ int mptcp_pm_nl_announce_doit(struct sk_buff
> *skb, struct genl_info *info)
>  	}
>  
>  	if (addr_val.addr.id == 0 || !(addr_val.flags &
> MPTCP_PM_ADDR_FLAG_SIGNAL)) {
> -		GENL_SET_ERR_MSG(info, "invalid addr id or flags");
> +		NL_SET_ERR_MSG_ATTR(info->extack, addr,
> +				    "invalid addr id or flags");
>  		err = -EINVAL;
>  		goto announce_err;
>  	}
>  
>  	err = mptcp_userspace_pm_append_new_local_addr(msk,
> &addr_val, false);
>  	if (err < 0) {
> -		GENL_SET_ERR_MSG(info, "did not match address and
> id");
> +		NL_SET_ERR_MSG_ATTR(info->extack, addr,
> +				    "did not match address and id");
>  		goto announce_err;
>  	}
>  
> @@ -254,7 +257,8 @@ int mptcp_pm_nl_announce_doit(struct sk_buff
> *skb, struct genl_info *info)
>  }
>  
>  static int mptcp_userspace_pm_remove_id_zero_address(struct
> mptcp_sock *msk,
> -						     struct
> genl_info *info)
> +						     struct
> genl_info *info,
> +						     struct nlattr
> *attr)
>  {
>  	struct mptcp_rm_list list = { .nr = 0 };
>  	struct mptcp_subflow_context *subflow;
> @@ -270,7 +274,8 @@ static int
> mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
>  		}
>  	}
>  	if (!has_id_0) {
> -		GENL_SET_ERR_MSG(info, "address with id 0 not
> found");
> +		NL_SET_ERR_MSG_ATTR(info->extack, attr,
> +				    "address with id 0 not found");
>  		goto remove_err;
>  	}
>  
> @@ -330,7 +335,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,
> info, id);
>  		goto out;
>  	}
>  
> @@ -339,7 +344,8 @@ 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");
> +		NL_SET_ERR_MSG_ATTR(info->extack, id,
> +				    "address with specified id not
> found");
>  		spin_unlock_bh(&msk->pm.lock);
>  		release_sock(sk);
>  		goto out;
> @@ -389,7 +395,7 @@ int mptcp_pm_nl_subflow_create_doit(struct
> sk_buff *skb, struct genl_info *info)
>  	}
>  
>  	if (entry.flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
> -		GENL_SET_ERR_MSG(info, "invalid addr flags");
> +		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "invalid
> addr flags");
>  		err = -EINVAL;
>  		goto create_err;
>  	}
> @@ -409,7 +415,8 @@ int mptcp_pm_nl_subflow_create_doit(struct
> sk_buff *skb, struct genl_info *info)
>  
>  	err = mptcp_userspace_pm_append_new_local_addr(msk, &entry,
> false);
>  	if (err < 0) {
> -		GENL_SET_ERR_MSG(info, "did not match address and
> id");
> +		NL_SET_ERR_MSG_ATTR(info->extack, laddr,
> +				    "did not match address and id");
>  		goto create_err;
>  	}
>  
> @@ -702,7 +709,7 @@ int mptcp_userspace_pm_get_addr(struct sk_buff
> *skb,
>  	spin_lock_bh(&msk->pm.lock);
>  	entry = mptcp_userspace_pm_lookup_addr_by_id(msk,
> addr.addr.id);
>  	if (!entry) {
> -		GENL_SET_ERR_MSG(info, "address not found");
> +		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not
> found");
>  		ret = -EINVAL;
>  		goto unlock_fail;
>  	}
>
diff mbox series

Patch

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 98ac73938bd8196e196d5ee8c264784ba8d37645..49c34494e156ff6c3277b107c3840ee29dd3afbb 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1403,18 +1403,21 @@  int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
 		return ret;
 
 	if (addr.addr.port && !address_use_port(&addr)) {
-		GENL_SET_ERR_MSG(info, "flags must have signal and not subflow when using port");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr,
+				    "flags must have signal and not subflow when using port");
 		return -EINVAL;
 	}
 
 	if (addr.flags & MPTCP_PM_ADDR_FLAG_SIGNAL &&
 	    addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) {
-		GENL_SET_ERR_MSG(info, "flags mustn't have both signal and fullmesh");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr,
+				    "flags mustn't have both signal and fullmesh");
 		return -EINVAL;
 	}
 
 	if (addr.flags & MPTCP_PM_ADDR_FLAG_IMPLICIT) {
-		GENL_SET_ERR_MSG(info, "can't create IMPLICIT endpoint");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr,
+				    "can't create IMPLICIT endpoint");
 		return -EINVAL;
 	}
 
@@ -1608,7 +1611,7 @@  int mptcp_pm_nl_del_addr_doit(struct sk_buff *skb, struct genl_info *info)
 	spin_lock_bh(&pernet->lock);
 	entry = __lookup_addr_by_id(pernet, addr.addr.id);
 	if (!entry) {
-		GENL_SET_ERR_MSG(info, "address not found");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		spin_unlock_bh(&pernet->lock);
 		return -EINVAL;
 	}
@@ -1790,7 +1793,7 @@  int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info)
 	rcu_read_lock();
 	entry = __lookup_addr_by_id(pernet, addr.addr.id);
 	if (!entry) {
-		GENL_SET_ERR_MSG(info, "address not found");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		ret = -EINVAL;
 		goto unlock_fail;
 	}
@@ -1875,7 +1878,8 @@  static int parse_limit(struct genl_info *info, int id, unsigned int *limit)
 
 	*limit = nla_get_u32(attr);
 	if (*limit > MPTCP_PM_ADDR_MAX) {
-		GENL_SET_ERR_MSG(info, "limit greater than maximum");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr,
+				    "limit greater than maximum");
 		return -EINVAL;
 	}
 	return 0;
@@ -2003,7 +2007,8 @@  int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
 	if (addr.addr.family == AF_UNSPEC) {
 		lookup_by_id = 1;
 		if (!addr.addr.id) {
-			GENL_SET_ERR_MSG(info, "missing required inputs");
+			NL_SET_ERR_MSG_ATTR(info->extack, attr,
+					    "missing required inputs");
 			return -EOPNOTSUPP;
 		}
 	}
@@ -2016,13 +2021,13 @@  int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
 			       __lookup_addr(pernet, &addr.addr);
 	if (!entry) {
 		spin_unlock_bh(&pernet->lock);
-		GENL_SET_ERR_MSG(info, "address not found");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		return -EINVAL;
 	}
 	if ((addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
 	    (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
 		spin_unlock_bh(&pernet->lock);
-		GENL_SET_ERR_MSG(info, "invalid addr flags");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "invalid addr flags");
 		return -EINVAL;
 	}
 
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index a3d477059b11c3a5618dbb6256434a8e55845995..72d4861e497eef5a516fc1c3ec1659807ffedd53 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -190,7 +190,8 @@  static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *in
 	}
 
 	if (!mptcp_pm_is_userspace(msk)) {
-		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
+		NL_SET_ERR_MSG_ATTR(info->extack, token,
+				    "invalid request; userspace PM not selected");
 		sock_put((struct sock *)msk);
 		return NULL;
 	}
@@ -224,14 +225,16 @@  int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	if (addr_val.addr.id == 0 || !(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
-		GENL_SET_ERR_MSG(info, "invalid addr id or flags");
+		NL_SET_ERR_MSG_ATTR(info->extack, addr,
+				    "invalid addr id or flags");
 		err = -EINVAL;
 		goto announce_err;
 	}
 
 	err = mptcp_userspace_pm_append_new_local_addr(msk, &addr_val, false);
 	if (err < 0) {
-		GENL_SET_ERR_MSG(info, "did not match address and id");
+		NL_SET_ERR_MSG_ATTR(info->extack, addr,
+				    "did not match address and id");
 		goto announce_err;
 	}
 
@@ -254,7 +257,8 @@  int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 }
 
 static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
-						     struct genl_info *info)
+						     struct genl_info *info,
+						     struct nlattr *attr)
 {
 	struct mptcp_rm_list list = { .nr = 0 };
 	struct mptcp_subflow_context *subflow;
@@ -270,7 +274,8 @@  static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
 		}
 	}
 	if (!has_id_0) {
-		GENL_SET_ERR_MSG(info, "address with id 0 not found");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr,
+				    "address with id 0 not found");
 		goto remove_err;
 	}
 
@@ -330,7 +335,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, info, id);
 		goto out;
 	}
 
@@ -339,7 +344,8 @@  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");
+		NL_SET_ERR_MSG_ATTR(info->extack, id,
+				    "address with specified id not found");
 		spin_unlock_bh(&msk->pm.lock);
 		release_sock(sk);
 		goto out;
@@ -389,7 +395,7 @@  int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	if (entry.flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
-		GENL_SET_ERR_MSG(info, "invalid addr flags");
+		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "invalid addr flags");
 		err = -EINVAL;
 		goto create_err;
 	}
@@ -409,7 +415,8 @@  int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 
 	err = mptcp_userspace_pm_append_new_local_addr(msk, &entry, false);
 	if (err < 0) {
-		GENL_SET_ERR_MSG(info, "did not match address and id");
+		NL_SET_ERR_MSG_ATTR(info->extack, laddr,
+				    "did not match address and id");
 		goto create_err;
 	}
 
@@ -702,7 +709,7 @@  int mptcp_userspace_pm_get_addr(struct sk_buff *skb,
 	spin_lock_bh(&msk->pm.lock);
 	entry = mptcp_userspace_pm_lookup_addr_by_id(msk, addr.addr.id);
 	if (!entry) {
-		GENL_SET_ERR_MSG(info, "address not found");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		ret = -EINVAL;
 		goto unlock_fail;
 	}