From: "Matthieu Baerts (NGI0)" <matttbe@kernel.org>

Some error messages were:

 - too generic: "missing input", "invalid request"

 - not precise enough: "limit greater than maximum" but what's the max?

 - missing: subflow not found, or connect error.

This can be easily improved by being more precise, or adding new error
messages.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_netlink.c   | 5 +++--
 net/mptcp/pm_userspace.c | 7 ++++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 46a97a3bf319..528718cb70ec 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1878,8 +1878,9 @@ static int parse_limit(struct genl_info *info, int id, unsigned int *limit)
 
 	*limit = nla_get_u32(attr);
 	if (*limit > MPTCP_PM_ADDR_MAX) {
-		NL_SET_ERR_MSG_ATTR(info->extack, attr,
-				    "limit greater than maximum");
+		NL_SET_ERR_MSG_ATTR_FMT(info->extack, attr,
+					"limit greater than maximum (%u)",
+					MPTCP_PM_ADDR_MAX);
 		return -EINVAL;
 	}
 	return 0;
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 066fd89b1691..341db3bc191b 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -191,7 +191,7 @@ static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *in
 
 	if (!mptcp_pm_is_userspace(msk)) {
 		NL_SET_ERR_MSG_ATTR(info->extack, token,
-				    "invalid request; userspace PM not selected");
+				    "userspace PM not selected");
 		sock_put((struct sock *)msk);
 		return NULL;
 	}
@@ -430,6 +430,8 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 	lock_sock(sk);
 	err = __mptcp_subflow_connect(sk, &local, &addr_r);
 	release_sock(sk);
+	if (err)
+		GENL_SET_ERR_MSG_FMT(info, "connect error: %d", err);
 
 	spin_lock_bh(&msk->pm.lock);
 	if (err)
@@ -555,6 +557,7 @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 	lock_sock(sk);
 	ssk = mptcp_nl_find_ssk(msk, &addr_l.addr, &addr_r);
 	if (!ssk) {
+		GENL_SET_ERR_MSG(info, "subflow not found");
 		err = -ESRCH;
 		goto release_sock;
 	}
@@ -613,6 +616,8 @@ int mptcp_userspace_pm_set_flags(struct mptcp_pm_addr_entry *local,
 	lock_sock(sk);
 	ret = mptcp_pm_nl_mp_prio_send_ack(msk, &entry->addr, remote, bkup);
 	release_sock(sk);
+	if (ret)
+		GENL_SET_ERR_MSG(info, "mp_prio send ack failed");
 
 set_flags_err:
 	sock_put(sk);
-- 
2.45.2

Hi Geliang,

On 06/01/2025 09:16, Geliang Tang wrote:
> From: "Matthieu Baerts (NGI0)" <matttbe@kernel.org>
> 
> Some error messages were:
> 
>  - too generic: "missing input", "invalid request"
> 
>  - not precise enough: "limit greater than maximum" but what's the max?
> 
>  - missing: subflow not found, or connect error.
> 
> This can be easily improved by being more precise, or adding new error
> messages.

(...)

> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 066fd89b1691..341db3bc191b 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c

(...)

> @@ -613,6 +616,8 @@ int mptcp_userspace_pm_set_flags(struct mptcp_pm_addr_entry *local,
>  	lock_sock(sk);
>  	ret = mptcp_pm_nl_mp_prio_send_ack(msk, &entry->addr, remote, bkup);
>  	release_sock(sk);
> +	if (ret)
> +		GENL_SET_ERR_MSG(info, "mp_prio send ack failed");

As mentioned on my comment on the v6, it might be better to have:

  /* mptcp_pm_nl_mp_prio_send_ack() only fails in one case */
  if (ret < 0)
          GENL_SET_ERR_MSG(info, "subflow not found");

>  
>  set_flags_err:
>  	sock_put(sk);

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