Message ID | 4078a76f5e18d55532f87e4d8a936876d356222c.1736150983.git.tanggeliang@kylinos.cn (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Matthieu Baerts |
Headers | show |
Series | mptcp: use GENL_REQ_ATTR_CHECK in userspace pm | expand |
Context | Check | Description |
---|---|---|
matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 42 lines checked |
matttbe/shellcheck | success | MPTCP selftests files have not been modified |
matttbe/build | success | Build and static analysis OK |
matttbe/KVM_Validation__normal | warning | Unstable: 1 failed test(s): selftest_userspace_pm - Critical: 1 Call Trace(s) ❌ |
matttbe/KVM_Validation__debug | warning | Unstable: 2 failed test(s): packetdrill_fastopen selftest_userspace_pm - Critical: 1 Call Trace(s) ❌ |
matttbe/KVM_Validation__btf-normal__only_bpftest_all_ | success | Success! ✅ |
matttbe/KVM_Validation__btf-debug__only_bpftest_all_ | success | Success! ✅ |
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
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);