From: Geliang Tang <geliang@kernel.org>
To: "Matthieu Baerts (NGI0)" <matttbe@kernel.org>, mptcp@lists.linux.dev
Subject: Re: [PATCH mptcp-next v6 1/7] mptcp: pm: use NL_SET_ERR_MSG_ATTR when possible
Date: Mon, 06 Jan 2025 16:27:13 +0800 [thread overview]
Message-ID: <59dbcaf581144fafcc1085726e7e06cdabd244da.camel@kernel.org> (raw)
In-Reply-To: <20241230-genl_req_attr_check-v6-1-3ec9103559e7@kernel.org>
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;
> }
>
next prev parent reply other threads:[~2025-01-06 8:27 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-30 13:24 [PATCH mptcp-next v6 0/7] mptcp: use GENL_REQ_ATTR_CHECK in userspace pm Matthieu Baerts (NGI0)
2024-12-30 13:24 ` [PATCH mptcp-next v6 1/7] mptcp: pm: use NL_SET_ERR_MSG_ATTR when possible Matthieu Baerts (NGI0)
2025-01-06 8:27 ` Geliang Tang [this message]
2024-12-30 13:24 ` [PATCH mptcp-next v6 2/7] mptcp: pm: more precise error messages Matthieu Baerts (NGI0)
2025-01-06 8:38 ` Geliang Tang
2025-01-06 8:55 ` Matthieu Baerts
2024-12-30 13:24 ` [PATCH mptcp-next v6 3/7] mptcp: pm: improve " Matthieu Baerts (NGI0)
2025-01-06 8:41 ` Geliang Tang
2025-01-06 8:45 ` Matthieu Baerts
2025-01-06 8:55 ` Geliang Tang
2025-01-06 9:03 ` Matthieu Baerts
2024-12-30 13:24 ` [PATCH mptcp-next v6 4/7] mptcp: pm: userspace: flags: clearer msg if no remote addr Matthieu Baerts (NGI0)
2025-01-06 8:46 ` Geliang Tang
2024-12-30 13:24 ` [PATCH mptcp-next v6 5/7] mptcp: pm: userspace: use GENL_REQ_ATTR_CHECK Matthieu Baerts (NGI0)
2024-12-30 13:24 ` [PATCH mptcp-next v6 6/7] mptcp: pm: remove duplicated error messages Matthieu Baerts (NGI0)
2024-12-30 13:24 ` [PATCH mptcp-next v6 7/7] mptcp: pm: mark missing address attributes Matthieu Baerts (NGI0)
2024-12-30 14:21 ` [PATCH mptcp-next v6 0/7] mptcp: use GENL_REQ_ATTR_CHECK in userspace pm MPTCP CI
2025-01-06 8:24 ` Geliang Tang
2025-01-06 9:08 ` Matthieu Baerts
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=59dbcaf581144fafcc1085726e7e06cdabd244da.camel@kernel.org \
--to=geliang@kernel.org \
--cc=matttbe@kernel.org \
--cc=mptcp@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox