From a9d71b5de76ccc50318d13b828f1a753d8e6a63f Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Fri, 7 Feb 2025 14:59:19 +0100 Subject: [PATCH 01/15] mptcp: pm: drop info of userspace_pm_remove_id_zero_address The only use of 'info' parameter of userspace_pm_remove_id_zero_address() is to set an error message into it. Plus, this helper will only fail when it cannot find any subflows with a local address ID 0. This patch drops this parameter and sets the error message where this function is called in mptcp_pm_nl_remove_doit(). Signed-off-by: Geliang Tang Reviewed-by: Matthieu Baerts (NGI0) Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm_userspace.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index a3d477059b11..4de38bc03ab8 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -253,8 +253,7 @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info) return err; } -static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk, - struct genl_info *info) +static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk) { struct mptcp_rm_list list = { .nr = 0 }; struct mptcp_subflow_context *subflow; @@ -269,10 +268,8 @@ static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk, break; } } - if (!has_id_0) { - GENL_SET_ERR_MSG(info, "address with id 0 not found"); + if (!has_id_0) goto remove_err; - } list.ids[list.nr++] = 0; @@ -330,7 +327,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); goto out; } @@ -339,7 +336,6 @@ 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"); spin_unlock_bh(&msk->pm.lock); release_sock(sk); goto out; @@ -356,6 +352,11 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info) err = 0; out: + if (err) + GENL_SET_ERR_MSG_FMT(info, + "address with id %u not found", + id_val); + sock_put(sk); return err; } From 58b21309f97b08b6b9814d1ee1419249eba9ef08 Mon Sep 17 00:00:00 2001 From: "Matthieu Baerts (NGI0)" Date: Fri, 7 Feb 2025 14:59:20 +0100 Subject: [PATCH 02/15] mptcp: pm: userspace: flags: clearer msg if no remote addr Since its introduction in commit 892f396c8e68 ("mptcp: netlink: issue MP_PRIO signals from userspace PMs"), it was mandatory to specify the remote address, because of the 'if (rem->addr.family == AF_UNSPEC)' check done later one. In theory, this attribute can be optional, but it sounds better to be precise to avoid sending the MP_PRIO on the wrong subflow, e.g. if there are multiple subflows attached to the same local ID. This can be relaxed later on if there is a need to act on multiple subflows with one command. For the moment, the check to see if attr_rem is NULL can be removed, because mptcp_pm_parse_entry() will do this check as well, no need to do that differently here. Reviewed-by: Geliang Tang Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm_userspace.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 4de38bc03ab8..b6cf8ea1161d 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -580,11 +580,9 @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info) if (ret < 0) goto set_flags_err; - if (attr_rem) { - ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem); - if (ret < 0) - goto set_flags_err; - } + ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem); + if (ret < 0) + goto set_flags_err; if (loc.addr.family == AF_UNSPEC || rem.addr.family == AF_UNSPEC) { From 891a87f7a76c77f8afde876e08b55a2af9819709 Mon Sep 17 00:00:00 2001 From: "Matthieu Baerts (NGI0)" Date: Fri, 7 Feb 2025 14:59:21 +0100 Subject: [PATCH 03/15] mptcp: pm: more precise error messages Some errors reported by the userspace PM were vague: "this or that is invalid". It is easier for the userspace to know which part is wrong, instead of having to guess that. While at it, in mptcp_userspace_pm_set_flags() move the parsing after the check linked to the local attribute. Reviewed-by: Geliang Tang Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm_userspace.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index b6cf8ea1161d..cdc83fabb7c2 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -223,8 +223,14 @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info) goto announce_err; } - if (addr_val.addr.id == 0 || !(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) { - GENL_SET_ERR_MSG(info, "invalid addr id or flags"); + if (addr_val.addr.id == 0) { + GENL_SET_ERR_MSG(info, "invalid addr id"); + err = -EINVAL; + goto announce_err; + } + + if (!(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) { + GENL_SET_ERR_MSG(info, "invalid addr flags"); err = -EINVAL; goto announce_err; } @@ -531,8 +537,14 @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info goto destroy_err; } - if (!addr_l.addr.port || !addr_r.port) { - GENL_SET_ERR_MSG(info, "missing local or remote port"); + if (!addr_l.addr.port) { + GENL_SET_ERR_MSG(info, "missing local port"); + err = -EINVAL; + goto destroy_err; + } + + if (!addr_r.port) { + GENL_SET_ERR_MSG(info, "missing remote port"); err = -EINVAL; goto destroy_err; } @@ -580,13 +592,18 @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info) if (ret < 0) goto set_flags_err; + if (loc.addr.family == AF_UNSPEC) { + GENL_SET_ERR_MSG(info, "invalid local address family"); + ret = -EINVAL; + goto set_flags_err; + } + ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem); if (ret < 0) goto set_flags_err; - if (loc.addr.family == AF_UNSPEC || - rem.addr.family == AF_UNSPEC) { - GENL_SET_ERR_MSG(info, "invalid address families"); + if (rem.addr.family == AF_UNSPEC) { + GENL_SET_ERR_MSG(info, "invalid remote address family"); ret = -EINVAL; goto set_flags_err; } From b2bdec19beecf9351ff9f7ca3131f5f61aacfc9d Mon Sep 17 00:00:00 2001 From: "Matthieu Baerts (NGI0)" Date: Fri, 7 Feb 2025 14:59:22 +0100 Subject: [PATCH 04/15] mptcp: pm: improve error messages 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. Reviewed-by: Geliang Tang Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm_netlink.c | 6 ++++-- net/mptcp/pm_userspace.c | 10 +++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 572d160edca3..1afa2bd89862 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -1875,7 +1875,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) { - GENL_SET_ERR_MSG(info, "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; @@ -2003,7 +2005,7 @@ 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"); + GENL_SET_ERR_MSG(info, "missing address ID"); return -EOPNOTSUPP; } } diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index cdc83fabb7c2..e350d6cc23bf 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -190,7 +190,7 @@ 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"); + GENL_SET_ERR_MSG(info, "userspace PM not selected"); sock_put((struct sock *)msk); return NULL; } @@ -428,6 +428,9 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info) 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) mptcp_userspace_pm_delete_local_addr(msk, &entry); @@ -552,6 +555,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; } @@ -625,6 +629,10 @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info) ret = mptcp_pm_nl_mp_prio_send_ack(msk, &loc.addr, &rem.addr, bkup); release_sock(sk); + /* 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); return ret; From 07bfabf8407b4c4cc010e0c41feeebed75594bd0 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Fri, 7 Feb 2025 14:59:23 +0100 Subject: [PATCH 05/15] mptcp: pm: userspace: use GENL_REQ_ATTR_CHECK A more general way to check if MPTCP_PM_ATTR_* exists in 'info' is to use GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_*) instead of directly reading info->attrs[MPTCP_PM_ATTR_*] and then checking if it's NULL. So this patch uses GENL_REQ_ATTR_CHECK() for userspace PM in mptcp_pm_nl_announce_doit(), mptcp_pm_nl_remove_doit(), mptcp_pm_nl_subflow_create_doit(), mptcp_pm_nl_subflow_destroy_doit() and mptcp_userspace_pm_get_sock(). Suggested-by: Jakub Kicinski Signed-off-by: Geliang Tang Reviewed-by: Matthieu Baerts (NGI0) Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm_userspace.c | 41 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index e350d6cc23bf..4cbd234e2670 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -175,14 +175,13 @@ bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk, static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *info) { - struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN]; struct mptcp_sock *msk; + struct nlattr *token; - if (!token) { - GENL_SET_ERR_MSG(info, "missing required token"); + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN)) return NULL; - } + token = info->attrs[MPTCP_PM_ATTR_TOKEN]; msk = mptcp_token_get_sock(genl_info_net(info), nla_get_u32(token)); if (!msk) { NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token"); @@ -200,16 +199,14 @@ static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *in int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info) { - struct nlattr *addr = info->attrs[MPTCP_PM_ATTR_ADDR]; struct mptcp_pm_addr_entry addr_val; struct mptcp_sock *msk; + struct nlattr *addr; int err = -EINVAL; struct sock *sk; - if (!addr) { - GENL_SET_ERR_MSG(info, "missing required address"); + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR)) return err; - } msk = mptcp_userspace_pm_get_sock(info); if (!msk) @@ -217,6 +214,7 @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info) sk = (struct sock *)msk; + addr = info->attrs[MPTCP_PM_ATTR_ADDR]; err = mptcp_pm_parse_entry(addr, info, true, &addr_val); if (err < 0) { GENL_SET_ERR_MSG(info, "error parsing local address"); @@ -312,18 +310,17 @@ void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk, int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info) { - struct nlattr *id = info->attrs[MPTCP_PM_ATTR_LOC_ID]; struct mptcp_pm_addr_entry *match; struct mptcp_sock *msk; + struct nlattr *id; int err = -EINVAL; struct sock *sk; u8 id_val; - if (!id) { - GENL_SET_ERR_MSG(info, "missing required ID"); + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_LOC_ID)) return err; - } + id = info->attrs[MPTCP_PM_ATTR_LOC_ID]; id_val = nla_get_u8(id); msk = mptcp_userspace_pm_get_sock(info); @@ -369,19 +366,17 @@ out: int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info) { - struct nlattr *raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE]; - struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR]; struct mptcp_pm_addr_entry entry = { 0 }; struct mptcp_addr_info addr_r; + struct nlattr *raddr, *laddr; struct mptcp_pm_local local; struct mptcp_sock *msk; int err = -EINVAL; struct sock *sk; - if (!laddr || !raddr) { - GENL_SET_ERR_MSG(info, "missing required address(es)"); + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) || + GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE)) return err; - } msk = mptcp_userspace_pm_get_sock(info); if (!msk) @@ -389,6 +384,7 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info) sk = (struct sock *)msk; + laddr = info->attrs[MPTCP_PM_ATTR_ADDR]; err = mptcp_pm_parse_entry(laddr, info, true, &entry); if (err < 0) { NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr"); @@ -402,6 +398,7 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info) } entry.flags |= MPTCP_PM_ADDR_FLAG_SUBFLOW; + raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE]; err = mptcp_pm_parse_addr(raddr, info, &addr_r); if (err < 0) { NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr"); @@ -493,18 +490,16 @@ static struct sock *mptcp_nl_find_ssk(struct mptcp_sock *msk, int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info) { - struct nlattr *raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE]; - struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR]; struct mptcp_pm_addr_entry addr_l; struct mptcp_addr_info addr_r; + struct nlattr *raddr, *laddr; struct mptcp_sock *msk; struct sock *sk, *ssk; int err = -EINVAL; - if (!laddr || !raddr) { - GENL_SET_ERR_MSG(info, "missing required address(es)"); + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) || + GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE)) return err; - } msk = mptcp_userspace_pm_get_sock(info); if (!msk) @@ -512,12 +507,14 @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info sk = (struct sock *)msk; + laddr = info->attrs[MPTCP_PM_ATTR_ADDR]; err = mptcp_pm_parse_entry(laddr, info, true, &addr_l); if (err < 0) { NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr"); goto destroy_err; } + raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE]; err = mptcp_pm_parse_addr(raddr, info, &addr_r); if (err < 0) { NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr"); From 60097f03fc7a96fecb4ea9a993397032fe586778 Mon Sep 17 00:00:00 2001 From: "Matthieu Baerts (NGI0)" Date: Fri, 7 Feb 2025 14:59:24 +0100 Subject: [PATCH 06/15] mptcp: pm: remove duplicated error messages mptcp_pm_parse_entry() and mptcp_pm_parse_addr() will already set a error message in case of parsing issue. Then, no need to override this error message with another less precise one: "error parsing address". Reviewed-by: Geliang Tang Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm_userspace.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 4cbd234e2670..ab915716ed41 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -216,10 +216,8 @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info) addr = info->attrs[MPTCP_PM_ATTR_ADDR]; err = mptcp_pm_parse_entry(addr, info, true, &addr_val); - if (err < 0) { - GENL_SET_ERR_MSG(info, "error parsing local address"); + if (err < 0) goto announce_err; - } if (addr_val.addr.id == 0) { GENL_SET_ERR_MSG(info, "invalid addr id"); @@ -386,10 +384,8 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info) laddr = info->attrs[MPTCP_PM_ATTR_ADDR]; err = mptcp_pm_parse_entry(laddr, info, true, &entry); - if (err < 0) { - NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr"); + if (err < 0) goto create_err; - } if (entry.flags & MPTCP_PM_ADDR_FLAG_SIGNAL) { GENL_SET_ERR_MSG(info, "invalid addr flags"); @@ -400,10 +396,8 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info) raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE]; err = mptcp_pm_parse_addr(raddr, info, &addr_r); - if (err < 0) { - NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr"); + if (err < 0) goto create_err; - } if (!mptcp_pm_addr_families_match(sk, &entry.addr, &addr_r)) { GENL_SET_ERR_MSG(info, "families mismatch"); @@ -509,17 +503,13 @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info laddr = info->attrs[MPTCP_PM_ATTR_ADDR]; err = mptcp_pm_parse_entry(laddr, info, true, &addr_l); - if (err < 0) { - NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr"); + if (err < 0) goto destroy_err; - } raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE]; err = mptcp_pm_parse_addr(raddr, info, &addr_r); - if (err < 0) { - NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr"); + if (err < 0) goto destroy_err; - } #if IS_ENABLED(CONFIG_MPTCP_IPV6) if (addr_l.addr.family == AF_INET && ipv6_addr_v4mapped(&addr_r.addr6)) { From 8cdc56f99e6c33abc855891dbd4d802be06d1404 Mon Sep 17 00:00:00 2001 From: "Matthieu Baerts (NGI0)" Date: Fri, 7 Feb 2025 14:59:25 +0100 Subject: [PATCH 07/15] mptcp: pm: mark missing address attributes mptcp_pm_parse_entry() will check if the given attribute is defined. If not, it will return a generic error: "missing address info". It might then not be clear for the userspace developer which attribute is missing, especially when the command takes multiple addresses. By using GENL_REQ_ATTR_CHECK(), the userspace will get a hint about which attribute is missing, making thing clearer. Note that this is what was already done for most of the other MPTCP NL commands, this patch simply adds the missing ones. Reviewed-by: Geliang Tang Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm_netlink.c | 24 ++++++++++++++++++++---- net/mptcp/pm_userspace.c | 15 ++++++++++++--- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 1afa2bd89862..4a1ba2bbb54d 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -1393,11 +1393,15 @@ static bool mptcp_pm_has_addr_attr_id(const struct nlattr *attr, int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info) { - struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; struct pm_nl_pernet *pernet = genl_info_pm_nl(info); struct mptcp_pm_addr_entry addr, *entry; + struct nlattr *attr; int ret; + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR)) + return -EINVAL; + + attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; ret = mptcp_pm_parse_entry(attr, info, true, &addr); if (ret < 0) return ret; @@ -1587,12 +1591,16 @@ next: int mptcp_pm_nl_del_addr_doit(struct sk_buff *skb, struct genl_info *info) { - struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; struct pm_nl_pernet *pernet = genl_info_pm_nl(info); struct mptcp_pm_addr_entry addr, *entry; unsigned int addr_max; + struct nlattr *attr; int ret; + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR)) + return -EINVAL; + + attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; ret = mptcp_pm_parse_entry(attr, info, false, &addr); if (ret < 0) return ret; @@ -1764,13 +1772,17 @@ nla_put_failure: int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info) { - struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; struct pm_nl_pernet *pernet = genl_info_pm_nl(info); struct mptcp_pm_addr_entry addr, *entry; struct sk_buff *msg; + struct nlattr *attr; void *reply; int ret; + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR)) + return -EINVAL; + + attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; ret = mptcp_pm_parse_entry(attr, info, false, &addr); if (ret < 0) return ret; @@ -1986,18 +1998,22 @@ next: int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info) { struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, }; - struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR]; u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP | MPTCP_PM_ADDR_FLAG_FULLMESH; struct net *net = sock_net(skb->sk); struct mptcp_pm_addr_entry *entry; struct pm_nl_pernet *pernet; + struct nlattr *attr; u8 lookup_by_id = 0; u8 bkup = 0; int ret; + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR)) + return -EINVAL; + pernet = pm_nl_get_pernet(net); + attr = info->attrs[MPTCP_PM_ATTR_ADDR]; ret = mptcp_pm_parse_entry(attr, info, false, &addr); if (ret < 0) return ret; diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index ab915716ed41..525dcb84353f 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -565,20 +565,24 @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info) { struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, }; struct mptcp_pm_addr_entry rem = { .addr = { .family = AF_UNSPEC }, }; - struct nlattr *attr_rem = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE]; - struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR]; struct mptcp_pm_addr_entry *entry; + struct nlattr *attr, *attr_rem; struct mptcp_sock *msk; int ret = -EINVAL; struct sock *sk; u8 bkup = 0; + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) || + GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE)) + return ret; + msk = mptcp_userspace_pm_get_sock(info); if (!msk) return ret; sk = (struct sock *)msk; + attr = info->attrs[MPTCP_PM_ATTR_ADDR]; ret = mptcp_pm_parse_entry(attr, info, false, &loc); if (ret < 0) goto set_flags_err; @@ -589,6 +593,7 @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info) goto set_flags_err; } + attr_rem = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE]; ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem); if (ret < 0) goto set_flags_err; @@ -677,20 +682,24 @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg, int mptcp_userspace_pm_get_addr(struct sk_buff *skb, struct genl_info *info) { - struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; struct mptcp_pm_addr_entry addr, *entry; struct mptcp_sock *msk; struct sk_buff *msg; + struct nlattr *attr; int ret = -EINVAL; struct sock *sk; void *reply; + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR)) + return ret; + msk = mptcp_userspace_pm_get_sock(info); if (!msk) return ret; sk = (struct sock *)msk; + attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; ret = mptcp_pm_parse_entry(attr, info, false, &addr); if (ret < 0) goto out; From a25a8b10491bb94e905fdbef06f0f2b80fd9f74f Mon Sep 17 00:00:00 2001 From: "Matthieu Baerts (NGI0)" Date: Fri, 7 Feb 2025 14:59:26 +0100 Subject: [PATCH 08/15] mptcp: pm: use NL_SET_ERR_MSG_ATTR when possible 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. Reviewed-by: Geliang Tang Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm_netlink.c | 20 ++++++++++++-------- net/mptcp/pm_userspace.c | 33 +++++++++++++++++++-------------- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 4a1ba2bbb54d..bbdfb7700538 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -1407,18 +1407,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; } @@ -1616,7 +1619,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; } @@ -1802,7 +1805,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; } @@ -2021,7 +2024,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 address ID"); + NL_SET_ERR_MSG_ATTR(info->extack, attr, + "missing address ID"); return -EOPNOTSUPP; } } @@ -2034,14 +2038,14 @@ 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 | MPTCP_PM_ADDR_FLAG_IMPLICIT))) { 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 525dcb84353f..8dddb1624736 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -189,7 +189,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, "userspace PM not selected"); + NL_SET_ERR_MSG_ATTR(info->extack, token, + "userspace PM not selected"); sock_put((struct sock *)msk); return NULL; } @@ -220,20 +221,21 @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info) goto announce_err; if (addr_val.addr.id == 0) { - GENL_SET_ERR_MSG(info, "invalid addr id"); + NL_SET_ERR_MSG_ATTR(info->extack, addr, "invalid addr id"); err = -EINVAL; goto announce_err; } if (!(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) { - GENL_SET_ERR_MSG(info, "invalid addr flags"); + NL_SET_ERR_MSG_ATTR(info->extack, addr, "invalid addr 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; } @@ -354,9 +356,9 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info) err = 0; out: if (err) - GENL_SET_ERR_MSG_FMT(info, - "address with id %u not found", - id_val); + NL_SET_ERR_MSG_ATTR_FMT(info->extack, id, + "address with id %u not found", + id_val); sock_put(sk); return err; @@ -388,7 +390,7 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info) goto create_err; 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; } @@ -407,7 +409,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; } @@ -528,13 +531,13 @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info } if (!addr_l.addr.port) { - GENL_SET_ERR_MSG(info, "missing local port"); + NL_SET_ERR_MSG_ATTR(info->extack, laddr, "missing local port"); err = -EINVAL; goto destroy_err; } if (!addr_r.port) { - GENL_SET_ERR_MSG(info, "missing remote port"); + NL_SET_ERR_MSG_ATTR(info->extack, raddr, "missing remote port"); err = -EINVAL; goto destroy_err; } @@ -588,7 +591,8 @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info) goto set_flags_err; if (loc.addr.family == AF_UNSPEC) { - GENL_SET_ERR_MSG(info, "invalid local address family"); + NL_SET_ERR_MSG_ATTR(info->extack, attr, + "invalid local address family"); ret = -EINVAL; goto set_flags_err; } @@ -599,7 +603,8 @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info) goto set_flags_err; if (rem.addr.family == AF_UNSPEC) { - GENL_SET_ERR_MSG(info, "invalid remote address family"); + NL_SET_ERR_MSG_ATTR(info->extack, attr_rem, + "invalid remote address family"); ret = -EINVAL; goto set_flags_err; } @@ -722,7 +727,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; } From 7aeab89b090fe40bc96e4af68c2b57f019d654d7 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Fri, 7 Feb 2025 14:59:27 +0100 Subject: [PATCH 09/15] mptcp: pm: make three pm wrappers static Three netlink functions: mptcp_pm_nl_get_addr_doit() mptcp_pm_nl_get_addr_dumpit() mptcp_pm_nl_set_flags_doit() are generic, implemented for each PM, in-kernel PM and userspace PM. It's clearer to move them from pm_netlink.c to pm.c. And the linked three path manager wrappers mptcp_pm_get_addr() mptcp_pm_dump_addr() mptcp_pm_set_flags() can be changed as static functions, no need to export them in protocol.h. Signed-off-by: Geliang Tang Reviewed-by: Matthieu Baerts (NGI0) Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm.c | 23 ++++++++++++++++++++--- net/mptcp/pm_netlink.c | 16 ---------------- net/mptcp/protocol.h | 3 --- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index 16c336c51940..a29be5ff73a6 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -10,6 +10,7 @@ #include "protocol.h" #include "mib.h" +#include "mptcp_pm_gen.h" /* path manager command handlers */ @@ -433,14 +434,19 @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc) return mptcp_pm_nl_is_backup(msk, &skc_local); } -int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info) +static int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info) { if (info->attrs[MPTCP_PM_ATTR_TOKEN]) return mptcp_userspace_pm_get_addr(skb, info); return mptcp_pm_nl_get_addr(skb, info); } -int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb) +int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info) +{ + return mptcp_pm_get_addr(skb, info); +} + +static int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb) { const struct genl_info *info = genl_info_dump(cb); @@ -449,13 +455,24 @@ int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb) return mptcp_pm_nl_dump_addr(msg, cb); } -int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info) +int mptcp_pm_nl_get_addr_dumpit(struct sk_buff *msg, + struct netlink_callback *cb) +{ + return mptcp_pm_dump_addr(msg, cb); +} + +static int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info) { if (info->attrs[MPTCP_PM_ATTR_TOKEN]) return mptcp_userspace_pm_set_flags(skb, info); return mptcp_pm_nl_set_flags(skb, info); } +int mptcp_pm_nl_set_flags_doit(struct sk_buff *skb, struct genl_info *info) +{ + return mptcp_pm_set_flags(skb, info); +} + void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk) { struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk); diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index bbdfb7700538..eb8f68ee24cf 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -1827,11 +1827,6 @@ fail: return ret; } -int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info) -{ - return mptcp_pm_get_addr(skb, info); -} - int mptcp_pm_nl_dump_addr(struct sk_buff *msg, struct netlink_callback *cb) { @@ -1875,12 +1870,6 @@ int mptcp_pm_nl_dump_addr(struct sk_buff *msg, return msg->len; } -int mptcp_pm_nl_get_addr_dumpit(struct sk_buff *msg, - struct netlink_callback *cb) -{ - return mptcp_pm_dump_addr(msg, cb); -} - static int parse_limit(struct genl_info *info, int id, unsigned int *limit) { struct nlattr *attr = info->attrs[id]; @@ -2058,11 +2047,6 @@ int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info) return 0; } -int mptcp_pm_nl_set_flags_doit(struct sk_buff *skb, struct genl_info *info) -{ - return mptcp_pm_set_flags(skb, info); -} - static void mptcp_nl_mcast_send(struct net *net, struct sk_buff *nlskb, gfp_t gfp) { genlmsg_multicast_netns(&mptcp_genl_family, net, diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index f6a207958459..6bfcde68d915 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -1038,7 +1038,6 @@ bool mptcp_lookup_subflow_by_saddr(const struct list_head *list, const struct mptcp_addr_info *saddr); bool mptcp_remove_anno_list_by_saddr(struct mptcp_sock *msk, const struct mptcp_addr_info *addr); -int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info); int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info); int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info); int mptcp_pm_announce_addr(struct mptcp_sock *msk, @@ -1131,12 +1130,10 @@ int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_in bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc); bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc); bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc); -int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb); int mptcp_pm_nl_dump_addr(struct sk_buff *msg, struct netlink_callback *cb); int mptcp_userspace_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb); -int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info); int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info); int mptcp_userspace_pm_get_addr(struct sk_buff *skb, struct genl_info *info); From 67dcf659254406596a6ff8600763244c3f4d75b0 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Fri, 7 Feb 2025 14:59:28 +0100 Subject: [PATCH 10/15] mptcp: pm: drop skb parameter of get_addr The first parameters 'skb' of get_addr() interfaces are now useless since mptcp_userspace_pm_get_sock() helper is used. This patch drops these useless parameters of them. Signed-off-by: Geliang Tang Reviewed-by: Matthieu Baerts (NGI0) Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm.c | 8 ++++---- net/mptcp/pm_netlink.c | 2 +- net/mptcp/pm_userspace.c | 3 +-- net/mptcp/protocol.h | 5 ++--- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index a29be5ff73a6..526e5bca1fa1 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -434,16 +434,16 @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc) return mptcp_pm_nl_is_backup(msk, &skc_local); } -static int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info) +static int mptcp_pm_get_addr(struct genl_info *info) { if (info->attrs[MPTCP_PM_ATTR_TOKEN]) - return mptcp_userspace_pm_get_addr(skb, info); - return mptcp_pm_nl_get_addr(skb, info); + return mptcp_userspace_pm_get_addr(info); + return mptcp_pm_nl_get_addr(info); } int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info) { - return mptcp_pm_get_addr(skb, info); + return mptcp_pm_get_addr(info); } static int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index eb8f68ee24cf..8185697044e2 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -1773,7 +1773,7 @@ nla_put_failure: return -EMSGSIZE; } -int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info) +int mptcp_pm_nl_get_addr(struct genl_info *info) { struct pm_nl_pernet *pernet = genl_info_pm_nl(info); struct mptcp_pm_addr_entry addr, *entry; diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 8dddb1624736..1246063598c8 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -684,8 +684,7 @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg, return ret; } -int mptcp_userspace_pm_get_addr(struct sk_buff *skb, - struct genl_info *info) +int mptcp_userspace_pm_get_addr(struct genl_info *info) { struct mptcp_pm_addr_entry addr, *entry; struct mptcp_sock *msk; diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 6bfcde68d915..a4c799ecceff 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -1134,9 +1134,8 @@ int mptcp_pm_nl_dump_addr(struct sk_buff *msg, struct netlink_callback *cb); int mptcp_userspace_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb); -int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info); -int mptcp_userspace_pm_get_addr(struct sk_buff *skb, - struct genl_info *info); +int mptcp_pm_nl_get_addr(struct genl_info *info); +int mptcp_userspace_pm_get_addr(struct genl_info *info); static inline u8 subflow_get_local_id(const struct mptcp_subflow_context *subflow) { From d47b80758f4c5d72a82cd8416cbf34617ded5c0b Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Fri, 7 Feb 2025 14:59:29 +0100 Subject: [PATCH 11/15] mptcp: pm: add id parameter for get_addr The address id is parsed both in mptcp_pm_nl_get_addr() and mptcp_userspace_pm_get_addr(), this makes the code somewhat repetitive. So this patch adds a new parameter 'id' for all get_addr() interfaces. The address id is only parsed in mptcp_pm_nl_get_addr_doit(), then pass it to both mptcp_pm_nl_get_addr() and mptcp_userspace_pm_get_addr(). Signed-off-by: Geliang Tang Reviewed-by: Matthieu Baerts (NGI0) Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm.c | 20 ++++++++++++++++---- net/mptcp/pm_netlink.c | 16 ++++------------ net/mptcp/pm_userspace.c | 16 ++++------------ net/mptcp/protocol.h | 4 ++-- 4 files changed, 26 insertions(+), 30 deletions(-) diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index 526e5bca1fa1..caf5bfc3cd1d 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -434,16 +434,28 @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc) return mptcp_pm_nl_is_backup(msk, &skc_local); } -static int mptcp_pm_get_addr(struct genl_info *info) +static int mptcp_pm_get_addr(u8 id, struct genl_info *info) { if (info->attrs[MPTCP_PM_ATTR_TOKEN]) - return mptcp_userspace_pm_get_addr(info); - return mptcp_pm_nl_get_addr(info); + return mptcp_userspace_pm_get_addr(id, info); + return mptcp_pm_nl_get_addr(id, info); } int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info) { - return mptcp_pm_get_addr(info); + struct mptcp_pm_addr_entry addr; + struct nlattr *attr; + int ret; + + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR)) + return -EINVAL; + + attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; + ret = mptcp_pm_parse_entry(attr, info, false, &addr); + if (ret < 0) + return ret; + + return mptcp_pm_get_addr(addr.addr.id, info); } static int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 8185697044e2..5a6c33d0063d 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -1773,23 +1773,15 @@ nla_put_failure: return -EMSGSIZE; } -int mptcp_pm_nl_get_addr(struct genl_info *info) +int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info) { + struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; struct pm_nl_pernet *pernet = genl_info_pm_nl(info); - struct mptcp_pm_addr_entry addr, *entry; + struct mptcp_pm_addr_entry *entry; struct sk_buff *msg; - struct nlattr *attr; void *reply; int ret; - if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR)) - return -EINVAL; - - attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; - ret = mptcp_pm_parse_entry(attr, info, false, &addr); - if (ret < 0) - return ret; - msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); if (!msg) return -ENOMEM; @@ -1803,7 +1795,7 @@ int mptcp_pm_nl_get_addr(struct genl_info *info) } rcu_read_lock(); - entry = __lookup_addr_by_id(pernet, addr.addr.id); + entry = __lookup_addr_by_id(pernet, id); if (!entry) { NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found"); ret = -EINVAL; diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 1246063598c8..99e882a5a671 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -684,30 +684,22 @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg, return ret; } -int mptcp_userspace_pm_get_addr(struct genl_info *info) +int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info) { - struct mptcp_pm_addr_entry addr, *entry; + struct nlattr *attr = attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; + struct mptcp_pm_addr_entry *entry; struct mptcp_sock *msk; struct sk_buff *msg; - struct nlattr *attr; int ret = -EINVAL; struct sock *sk; void *reply; - if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR)) - return ret; - msk = mptcp_userspace_pm_get_sock(info); if (!msk) return ret; sk = (struct sock *)msk; - attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; - ret = mptcp_pm_parse_entry(attr, info, false, &addr); - if (ret < 0) - goto out; - msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); if (!msg) { ret = -ENOMEM; @@ -724,7 +716,7 @@ int mptcp_userspace_pm_get_addr(struct genl_info *info) lock_sock(sk); spin_lock_bh(&msk->pm.lock); - entry = mptcp_userspace_pm_lookup_addr_by_id(msk, addr.addr.id); + entry = mptcp_userspace_pm_lookup_addr_by_id(msk, id); if (!entry) { NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found"); ret = -EINVAL; diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index a4c799ecceff..ffe370245ec5 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -1134,8 +1134,8 @@ int mptcp_pm_nl_dump_addr(struct sk_buff *msg, struct netlink_callback *cb); int mptcp_userspace_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb); -int mptcp_pm_nl_get_addr(struct genl_info *info); -int mptcp_userspace_pm_get_addr(struct genl_info *info); +int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info); +int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info); static inline u8 subflow_get_local_id(const struct mptcp_subflow_context *subflow) { From 8556f4aecc9a0620c1e411d6da1fe232d1c14138 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Fri, 7 Feb 2025 14:59:30 +0100 Subject: [PATCH 12/15] mptcp: pm: reuse sending nlmsg code in get_addr The netlink messages are sent both in mptcp_pm_nl_get_addr() and mptcp_userspace_pm_get_addr(), this makes the code somewhat repetitive. This is because the netlink PM and userspace PM use different locks to protect the address entry that needs to be sent via the netlink message. The former uses rcu read lock, and the latter uses msk->pm.lock. The current get_addr() flow looks like this: lock(); entry = get_entry(); send_nlmsg(entry); unlock(); After holding the lock, get the entry from the list, send the entry, and finally release the lock. This patch changes the process by getting the entry while holding the lock, then making a copy of the entry so that the lock can be released. Finally, the copy of the entry is sent without locking: lock(); entry = get_entry(); *copy = *entry; unlock(); send_nlmsg(copy); This way we can reuse the send_nlmsg() code in get_addr() interfaces between the netlink PM and userspace PM. They only need to implement their own get_addr() interfaces to hold the different locks, get the entry from the different lists, then release the locks. Signed-off-by: Geliang Tang Reviewed-by: Matthieu Baerts (NGI0) Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm.c | 39 +++++++++++++++++++++++++++++++++---- net/mptcp/pm_netlink.c | 40 ++++++-------------------------------- net/mptcp/pm_userspace.c | 42 +++++----------------------------------- net/mptcp/protocol.h | 6 ++++-- 4 files changed, 50 insertions(+), 77 deletions(-) diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index caf5bfc3cd1d..ba22d17c1451 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -434,17 +434,20 @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc) return mptcp_pm_nl_is_backup(msk, &skc_local); } -static int mptcp_pm_get_addr(u8 id, struct genl_info *info) +static int mptcp_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr, + struct genl_info *info) { if (info->attrs[MPTCP_PM_ATTR_TOKEN]) - return mptcp_userspace_pm_get_addr(id, info); - return mptcp_pm_nl_get_addr(id, info); + return mptcp_userspace_pm_get_addr(id, addr, info); + return mptcp_pm_nl_get_addr(id, addr, info); } int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info) { struct mptcp_pm_addr_entry addr; struct nlattr *attr; + struct sk_buff *msg; + void *reply; int ret; if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR)) @@ -455,7 +458,35 @@ int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info) if (ret < 0) return ret; - return mptcp_pm_get_addr(addr.addr.id, info); + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); + if (!msg) + return -ENOMEM; + + reply = genlmsg_put_reply(msg, info, &mptcp_genl_family, 0, + info->genlhdr->cmd); + if (!reply) { + GENL_SET_ERR_MSG(info, "not enough space in Netlink message"); + ret = -EMSGSIZE; + goto fail; + } + + ret = mptcp_pm_get_addr(addr.addr.id, &addr, info); + if (ret) { + NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found"); + goto fail; + } + + ret = mptcp_nl_fill_addr(msg, &addr); + if (ret) + goto fail; + + genlmsg_end(msg, reply); + ret = genlmsg_reply(msg, info); + return ret; + +fail: + nlmsg_free(msg); + return ret; } static int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 5a6c33d0063d..25b66674171f 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -1773,49 +1773,21 @@ nla_put_failure: return -EMSGSIZE; } -int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info) +int mptcp_pm_nl_get_addr(u8 id, struct mptcp_pm_addr_entry *addr, + struct genl_info *info) { - struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; struct pm_nl_pernet *pernet = genl_info_pm_nl(info); struct mptcp_pm_addr_entry *entry; - struct sk_buff *msg; - void *reply; - int ret; - - msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); - if (!msg) - return -ENOMEM; - - reply = genlmsg_put_reply(msg, info, &mptcp_genl_family, 0, - info->genlhdr->cmd); - if (!reply) { - GENL_SET_ERR_MSG(info, "not enough space in Netlink message"); - ret = -EMSGSIZE; - goto fail; - } + int ret = -EINVAL; rcu_read_lock(); entry = __lookup_addr_by_id(pernet, id); - if (!entry) { - NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found"); - ret = -EINVAL; - goto unlock_fail; + if (entry) { + *addr = *entry; + ret = 0; } - - ret = mptcp_nl_fill_addr(msg, entry); - if (ret) - goto unlock_fail; - - genlmsg_end(msg, reply); - ret = genlmsg_reply(msg, info); - rcu_read_unlock(); - return ret; - -unlock_fail: rcu_read_unlock(); -fail: - nlmsg_free(msg); return ret; } diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 99e882a5a671..80d75df18b03 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -684,15 +684,13 @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg, return ret; } -int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info) +int mptcp_userspace_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr, + struct genl_info *info) { - struct nlattr *attr = attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; struct mptcp_pm_addr_entry *entry; struct mptcp_sock *msk; - struct sk_buff *msg; int ret = -EINVAL; struct sock *sk; - void *reply; msk = mptcp_userspace_pm_get_sock(info); if (!msk) @@ -700,46 +698,16 @@ int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info) sk = (struct sock *)msk; - msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); - if (!msg) { - ret = -ENOMEM; - goto out; - } - - reply = genlmsg_put_reply(msg, info, &mptcp_genl_family, 0, - info->genlhdr->cmd); - if (!reply) { - GENL_SET_ERR_MSG(info, "not enough space in Netlink message"); - ret = -EMSGSIZE; - goto fail; - } - lock_sock(sk); spin_lock_bh(&msk->pm.lock); entry = mptcp_userspace_pm_lookup_addr_by_id(msk, id); - if (!entry) { - NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found"); - ret = -EINVAL; - goto unlock_fail; + if (entry) { + *addr = *entry; + ret = 0; } - - ret = mptcp_nl_fill_addr(msg, entry); - if (ret) - goto unlock_fail; - - genlmsg_end(msg, reply); - ret = genlmsg_reply(msg, info); spin_unlock_bh(&msk->pm.lock); release_sock(sk); - sock_put(sk); - return ret; -unlock_fail: - spin_unlock_bh(&msk->pm.lock); - release_sock(sk); -fail: - nlmsg_free(msg); -out: sock_put(sk); return ret; } diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index ffe370245ec5..e18ecd77a7f7 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -1134,8 +1134,10 @@ int mptcp_pm_nl_dump_addr(struct sk_buff *msg, struct netlink_callback *cb); int mptcp_userspace_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb); -int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info); -int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info); +int mptcp_pm_nl_get_addr(u8 id, struct mptcp_pm_addr_entry *addr, + struct genl_info *info); +int mptcp_userspace_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr, + struct genl_info *info); static inline u8 subflow_get_local_id(const struct mptcp_subflow_context *subflow) { From 2c8971c04f745de69814ae9cfd2273e8a1fefca9 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Fri, 7 Feb 2025 14:59:31 +0100 Subject: [PATCH 13/15] mptcp: pm: drop skb parameter of set_flags The first parameter 'skb' in mptcp_pm_nl_set_flags() is only used to obtained the network namespace, which can also be obtained through the second parameters 'info' by using genl_info_net() helper. This patch drops these useless parameters 'skb' in all three set_flags() interfaces. Signed-off-by: Geliang Tang Reviewed-by: Matthieu Baerts (NGI0) Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm.c | 8 ++++---- net/mptcp/pm_netlink.c | 4 ++-- net/mptcp/pm_userspace.c | 2 +- net/mptcp/protocol.h | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index ba22d17c1451..c213f06bc702 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -504,16 +504,16 @@ int mptcp_pm_nl_get_addr_dumpit(struct sk_buff *msg, return mptcp_pm_dump_addr(msg, cb); } -static int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info) +static int mptcp_pm_set_flags(struct genl_info *info) { if (info->attrs[MPTCP_PM_ATTR_TOKEN]) - return mptcp_userspace_pm_set_flags(skb, info); - return mptcp_pm_nl_set_flags(skb, info); + return mptcp_userspace_pm_set_flags(info); + return mptcp_pm_nl_set_flags(info); } int mptcp_pm_nl_set_flags_doit(struct sk_buff *skb, struct genl_info *info) { - return mptcp_pm_set_flags(skb, info); + return mptcp_pm_set_flags(info); } void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 25b66674171f..172ddb04e349 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -1951,12 +1951,12 @@ next: return ret; } -int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info) +int mptcp_pm_nl_set_flags(struct genl_info *info) { struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, }; u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP | MPTCP_PM_ADDR_FLAG_FULLMESH; - struct net *net = sock_net(skb->sk); + struct net *net = genl_info_net(info); struct mptcp_pm_addr_entry *entry; struct pm_nl_pernet *pernet; struct nlattr *attr; diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 80d75df18b03..4fa3935c5b47 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -564,7 +564,7 @@ destroy_err: return err; } -int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info) +int mptcp_userspace_pm_set_flags(struct genl_info *info) { struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, }; struct mptcp_pm_addr_entry rem = { .addr = { .family = AF_UNSPEC }, }; diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index e18ecd77a7f7..6e7dc5375e29 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -1038,8 +1038,8 @@ bool mptcp_lookup_subflow_by_saddr(const struct list_head *list, const struct mptcp_addr_info *saddr); bool mptcp_remove_anno_list_by_saddr(struct mptcp_sock *msk, const struct mptcp_addr_info *addr); -int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info); -int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info); +int mptcp_pm_nl_set_flags(struct genl_info *info); +int mptcp_userspace_pm_set_flags(struct genl_info *info); int mptcp_pm_announce_addr(struct mptcp_sock *msk, const struct mptcp_addr_info *addr, bool echo); From ab5723599cfd00a1898b11c9d67a770384a7f51b Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Fri, 7 Feb 2025 14:59:32 +0100 Subject: [PATCH 14/15] mptcp: pm: change rem type of set_flags Generally, in the path manager interfaces, the local address is defined as an mptcp_pm_addr_entry type address, while the remote address is defined as an mptcp_addr_info type one: (struct mptcp_pm_addr_entry *local, struct mptcp_addr_info *remote) But the set_flags() interface uses two mptcp_pm_addr_entry type parameters. This patch changes the second one to mptcp_addr_info type and use helper mptcp_pm_parse_addr() to parse it instead of using mptcp_pm_parse_entry(). Signed-off-by: Geliang Tang Reviewed-by: Matthieu Baerts (NGI0) Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm_userspace.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 4fa3935c5b47..1af70828c03c 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -567,7 +567,7 @@ destroy_err: int mptcp_userspace_pm_set_flags(struct genl_info *info) { struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, }; - struct mptcp_pm_addr_entry rem = { .addr = { .family = AF_UNSPEC }, }; + struct mptcp_addr_info rem = { .family = AF_UNSPEC, }; struct mptcp_pm_addr_entry *entry; struct nlattr *attr, *attr_rem; struct mptcp_sock *msk; @@ -598,11 +598,11 @@ int mptcp_userspace_pm_set_flags(struct genl_info *info) } attr_rem = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE]; - ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem); + ret = mptcp_pm_parse_addr(attr_rem, info, &rem); if (ret < 0) goto set_flags_err; - if (rem.addr.family == AF_UNSPEC) { + if (rem.family == AF_UNSPEC) { NL_SET_ERR_MSG_ATTR(info->extack, attr_rem, "invalid remote address family"); ret = -EINVAL; @@ -623,7 +623,7 @@ int mptcp_userspace_pm_set_flags(struct genl_info *info) spin_unlock_bh(&msk->pm.lock); lock_sock(sk); - ret = mptcp_pm_nl_mp_prio_send_ack(msk, &loc.addr, &rem.addr, bkup); + ret = mptcp_pm_nl_mp_prio_send_ack(msk, &loc.addr, &rem, bkup); release_sock(sk); /* mptcp_pm_nl_mp_prio_send_ack() only fails in one case */ From c7f25f7987c060b43b926c7c03c8ef8f0054a182 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Fri, 7 Feb 2025 14:59:33 +0100 Subject: [PATCH 15/15] mptcp: pm: add local parameter for set_flags This patch updates the interfaces set_flags to reduce repetitive code, adds a new parameter 'local' for them. The local address is parsed in public helper mptcp_pm_nl_set_flags_doit(), then pass it to mptcp_pm_nl_set_flags() and mptcp_userspace_pm_set_flags(). Signed-off-by: Geliang Tang Reviewed-by: Matthieu Baerts (NGI0) Signed-off-by: Matthieu Baerts (NGI0) Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/mptcp/pm.c | 16 ++++++++++++++-- net/mptcp/pm_netlink.c | 35 +++++++++++++---------------------- net/mptcp/pm_userspace.c | 19 +++++++------------ net/mptcp/protocol.h | 6 ++++-- 4 files changed, 38 insertions(+), 38 deletions(-) diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index c213f06bc702..b1f36dc1a091 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -506,9 +506,21 @@ int mptcp_pm_nl_get_addr_dumpit(struct sk_buff *msg, static int mptcp_pm_set_flags(struct genl_info *info) { + struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, }; + struct nlattr *attr_loc; + int ret = -EINVAL; + + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR)) + return ret; + + attr_loc = info->attrs[MPTCP_PM_ATTR_ADDR]; + ret = mptcp_pm_parse_entry(attr_loc, info, false, &loc); + if (ret < 0) + return ret; + if (info->attrs[MPTCP_PM_ATTR_TOKEN]) - return mptcp_userspace_pm_set_flags(info); - return mptcp_pm_nl_set_flags(info); + return mptcp_userspace_pm_set_flags(&loc, info); + return mptcp_pm_nl_set_flags(&loc, info); } int mptcp_pm_nl_set_flags_doit(struct sk_buff *skb, struct genl_info *info) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 172ddb04e349..99705a9c2238 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -1951,50 +1951,41 @@ next: return ret; } -int mptcp_pm_nl_set_flags(struct genl_info *info) +int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local, + struct genl_info *info) { - struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, }; + struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR]; u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP | MPTCP_PM_ADDR_FLAG_FULLMESH; struct net *net = genl_info_net(info); struct mptcp_pm_addr_entry *entry; struct pm_nl_pernet *pernet; - struct nlattr *attr; u8 lookup_by_id = 0; u8 bkup = 0; - int ret; - - if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR)) - return -EINVAL; pernet = pm_nl_get_pernet(net); - attr = info->attrs[MPTCP_PM_ATTR_ADDR]; - ret = mptcp_pm_parse_entry(attr, info, false, &addr); - if (ret < 0) - return ret; - - if (addr.addr.family == AF_UNSPEC) { + if (local->addr.family == AF_UNSPEC) { lookup_by_id = 1; - if (!addr.addr.id) { + if (!local->addr.id) { NL_SET_ERR_MSG_ATTR(info->extack, attr, "missing address ID"); return -EOPNOTSUPP; } } - if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP) + if (local->flags & MPTCP_PM_ADDR_FLAG_BACKUP) bkup = 1; spin_lock_bh(&pernet->lock); - entry = lookup_by_id ? __lookup_addr_by_id(pernet, addr.addr.id) : - __lookup_addr(pernet, &addr.addr); + entry = lookup_by_id ? __lookup_addr_by_id(pernet, local->addr.id) : + __lookup_addr(pernet, &local->addr); if (!entry) { spin_unlock_bh(&pernet->lock); NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found"); return -EINVAL; } - if ((addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) && + if ((local->flags & MPTCP_PM_ADDR_FLAG_FULLMESH) && (entry->flags & (MPTCP_PM_ADDR_FLAG_SIGNAL | MPTCP_PM_ADDR_FLAG_IMPLICIT))) { spin_unlock_bh(&pernet->lock); @@ -2002,12 +1993,12 @@ int mptcp_pm_nl_set_flags(struct genl_info *info) return -EINVAL; } - changed = (addr.flags ^ entry->flags) & mask; - entry->flags = (entry->flags & ~mask) | (addr.flags & mask); - addr = *entry; + changed = (local->flags ^ entry->flags) & mask; + entry->flags = (entry->flags & ~mask) | (local->flags & mask); + *local = *entry; spin_unlock_bh(&pernet->lock); - mptcp_nl_set_flags(net, &addr.addr, bkup, changed); + mptcp_nl_set_flags(net, &local->addr, bkup, changed); return 0; } diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 1af70828c03c..277cf092a870 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -564,9 +564,9 @@ destroy_err: return err; } -int mptcp_userspace_pm_set_flags(struct genl_info *info) +int mptcp_userspace_pm_set_flags(struct mptcp_pm_addr_entry *local, + struct genl_info *info) { - struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, }; struct mptcp_addr_info rem = { .family = AF_UNSPEC, }; struct mptcp_pm_addr_entry *entry; struct nlattr *attr, *attr_rem; @@ -575,8 +575,7 @@ int mptcp_userspace_pm_set_flags(struct genl_info *info) struct sock *sk; u8 bkup = 0; - if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) || - GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE)) + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE)) return ret; msk = mptcp_userspace_pm_get_sock(info); @@ -586,11 +585,7 @@ int mptcp_userspace_pm_set_flags(struct genl_info *info) sk = (struct sock *)msk; attr = info->attrs[MPTCP_PM_ATTR_ADDR]; - ret = mptcp_pm_parse_entry(attr, info, false, &loc); - if (ret < 0) - goto set_flags_err; - - if (loc.addr.family == AF_UNSPEC) { + if (local->addr.family == AF_UNSPEC) { NL_SET_ERR_MSG_ATTR(info->extack, attr, "invalid local address family"); ret = -EINVAL; @@ -609,11 +604,11 @@ int mptcp_userspace_pm_set_flags(struct genl_info *info) goto set_flags_err; } - if (loc.flags & MPTCP_PM_ADDR_FLAG_BACKUP) + if (local->flags & MPTCP_PM_ADDR_FLAG_BACKUP) bkup = 1; spin_lock_bh(&msk->pm.lock); - entry = mptcp_userspace_pm_lookup_addr(msk, &loc.addr); + entry = mptcp_userspace_pm_lookup_addr(msk, &local->addr); if (entry) { if (bkup) entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP; @@ -623,7 +618,7 @@ int mptcp_userspace_pm_set_flags(struct genl_info *info) spin_unlock_bh(&msk->pm.lock); lock_sock(sk); - ret = mptcp_pm_nl_mp_prio_send_ack(msk, &loc.addr, &rem, bkup); + ret = mptcp_pm_nl_mp_prio_send_ack(msk, &local->addr, &rem, bkup); release_sock(sk); /* mptcp_pm_nl_mp_prio_send_ack() only fails in one case */ diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 6e7dc5375e29..37226cdd9e37 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -1038,8 +1038,10 @@ bool mptcp_lookup_subflow_by_saddr(const struct list_head *list, const struct mptcp_addr_info *saddr); bool mptcp_remove_anno_list_by_saddr(struct mptcp_sock *msk, const struct mptcp_addr_info *addr); -int mptcp_pm_nl_set_flags(struct genl_info *info); -int mptcp_userspace_pm_set_flags(struct genl_info *info); +int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local, + struct genl_info *info); +int mptcp_userspace_pm_set_flags(struct mptcp_pm_addr_entry *local, + struct genl_info *info); int mptcp_pm_announce_addr(struct mptcp_sock *msk, const struct mptcp_addr_info *addr, bool echo);