From patchwork Wed Jan 8 01:44:02 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Geliang Tang X-Patchwork-Id: 13929938 X-Patchwork-Delegate: matthieu.baerts@tessares.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CACF6139D1B for ; Wed, 8 Jan 2025 01:44:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300660; cv=none; b=g7h55WHCJaeak9+XWfNZ60f8hKSgkESOjFiCVGfR26N4CD5oVFmlWRqsOk9X7bQdCvuO/xrYmno0m6iL7GRajx8GTCCcpRPb1Cyd+U6tzYplL+J9020k6pxYuH2TGf7zr9gw8AZSQL6XbQ5S0xWJOTUY1j88ICjemi+3+ihQYR8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300660; c=relaxed/simple; bh=5ZwuEaCyrh7/4JVc+sYIO1AdjSTMJWmfCVu+Im7GrHc=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Al6RyotYSZEHpsLALeoNkXuaOy5Hx+kpFQ3NmsgVjLqwqvZFMupJRfQDB6kD7MPMeFRpAS4gQeBkQ4QZxcqQ+XInr2I+xoPC4kNgvgm/XJtyaNsj/2qsyZeHMWkZDlR3WmnliKlPuMPIxHScIuFLMo6BDLSwoGYJmOlt+rAyvbg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=ZWaN2f/P; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="ZWaN2f/P" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0E8E5C4CED6; Wed, 8 Jan 2025 01:44:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736300659; bh=5ZwuEaCyrh7/4JVc+sYIO1AdjSTMJWmfCVu+Im7GrHc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ZWaN2f/Phx7IYST52WGUHWIrzLqa83OUyDbAVd4Pn4wiOzNkIuDL0RP6MWp5vkGYD 9wdpT8/cvBylJCHxuyt8XpLyHJ/qSM5lavovGmkotz1VAnoG1PQ7hflBmwgHepZTLm wnf6OAYAosdmI43siy+dG3iHluG6Davp9CELGYSo9bos5zmNHaj2bIdcxzOaZmuPgp vFeUAp6OUkm2JqdwPLIcSx50Be/jDOl71Y+4YDeWikVi6fHsMhGCp4UY/8XU8fRWjI Z56LiFpYWylTnGMZN8ysTyGceAogbhiOPHRL/0WQJce70fT1RG2C3WeyDHxfJfdeU0 KTR2SDeDX22ow== From: Geliang Tang To: mptcp@lists.linux.dev Cc: Geliang Tang Subject: [PATCH mptcp-next v8 1/8] mptcp: drop info of userspace_pm_remove_id_zero_address Date: Wed, 8 Jan 2025 09:44:02 +0800 Message-ID: <8eb1ce6b0fc86ce30746bdd0e309806494615ac6.1736299989.git.tanggeliang@kylinos.cn> X-Mailer: git-send-email 2.45.2 In-Reply-To: References: Precedence: bulk X-Mailing-List: mptcp@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Geliang Tang 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 --- 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 patchwork Wed Jan 8 01:44:03 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Geliang Tang X-Patchwork-Id: 13929939 X-Patchwork-Delegate: matthieu.baerts@tessares.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AE93A2C80 for ; Wed, 8 Jan 2025 01:44:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300661; cv=none; b=d1g2ddlksWolicGBEapBl1PihrAOZ+eVQGt8jb93ygF6EPFXXKFLlhoG0h1U735Pk2kp6/If+ONPA3OBt51qrNSnjt0V+bChGdeoHSkchnGEEGbQqnG0Kdv88X2GsU2BvDVHifR/2eTEZ76wfFzY15kbhMJvtZvNrD5wEhmyh0A= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300661; c=relaxed/simple; bh=cQ5/ht1MhEKJnGUiCvumlLsoSzZ7WkaoBBdCe30201g=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=OBDTjVyooS8fgKjaUTaVDjzcseB3K/UgFcrRjN6YiJxTozVkjtHoJUCworlCuHFhbMYhlVHHKXRwngVju2OQNn/xvt+OFEVJO/CY2wc4OQyLvHBPHGFWlGI9GNnG7A+v/4LVZWHd30CNvOBgDFtA4UJmpJbSb+qE4/F7z9Nqq5A= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=vO61Pv8H; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="vO61Pv8H" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 00877C4CEDF; Wed, 8 Jan 2025 01:44:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736300661; bh=cQ5/ht1MhEKJnGUiCvumlLsoSzZ7WkaoBBdCe30201g=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=vO61Pv8HwAJwwTD3bxc9kEO0k1T1YLyPktzSLtEN0prq6RPVIYOJ0bGgOC51vnOFY YQnyVY/90U1Q6hNdc5PFFzFSWdj1QFqkk4LS0u5Q/0CPNZK9tz4hSEwUHF/0p57Y8w HHHJIkmVZgqG0D+rF1J692x8lExsOwZBl2Gletr5ot3Zjp8tTOBwS1SwtQ+yocf8X8 5kyR6SbA8k5Edi75+Z3xcbI1hGePrihwZFGijRiEBXqsRC0Y7WO/OPY5/MHjmSACn4 qUEsGFlGmX85ZfCuFV3S/25c54EkaoIwMjb7FhjSifAwlho9IbgQFu4yvPGI33HiT5 TfhzZ34OWZayA== From: Geliang Tang To: mptcp@lists.linux.dev Cc: "Matthieu Baerts (NGI0)" Subject: [PATCH mptcp-next v8 2/8] mptcp: pm: userspace: flags: clearer msg if no remote addr Date: Wed, 8 Jan 2025 09:44:03 +0800 Message-ID: <0151403138cac7363208c011580d9c1e92b34a4d.1736299989.git.tanggeliang@kylinos.cn> X-Mailer: git-send-email 2.45.2 In-Reply-To: References: Precedence: bulk X-Mailing-List: mptcp@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Matthieu Baerts (NGI0)" 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. Signed-off-by: Matthieu Baerts (NGI0) --- 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 patchwork Wed Jan 8 01:44:04 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Geliang Tang X-Patchwork-Id: 13929940 X-Patchwork-Delegate: matthieu.baerts@tessares.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2A680139D1B for ; Wed, 8 Jan 2025 01:44:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300663; cv=none; b=CNSqoVihseQ4CAYGbwfGvwQxd6scfC7OpepBYbRXeqisteXofzFafrrAxa+AnbNLZQdttoLMyPB4v2q1xyLsyB+B/TZ+DPoUr+XwlNXjM1OHv3nTG9gRNhzQx0xaqgktAPTF/e8I9zV7IIi3LX7NX8rTJkDdlXyAbjo62AXOb/0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300663; c=relaxed/simple; bh=g+PDOpELuumD0L2THzgQhquEm3ukHOmoPKjaFUob+No=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=JAmZpV4AC8XPOH0q2QbWKU4LXjiZJqLdzx/gjTK5dXTjSUqrLEliieqQnWcwMI+UoAeN45vBjofy0zO1kQHjXXR/EUBtCcOUztiESRejGlDc2UPgvJyVAX6NVPLG8iT6372Wo9uAVNF5gUicSGBYbg/xXHgSpk4okRRrs1QM1+0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=DdunhB17; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="DdunhB17" Received: by smtp.kernel.org (Postfix) with ESMTPSA id CF755C4CEDF; Wed, 8 Jan 2025 01:44:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736300663; bh=g+PDOpELuumD0L2THzgQhquEm3ukHOmoPKjaFUob+No=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=DdunhB17LsVmZjmdsm/aEx9iPtdry9gIVckogKszxfQCX1D+P1Ltka2Q4dGKDssHa bTggqsp5CZnkDqhIHirTMZ/5f36pMFgE2bdq5n+QXmpSL0M6bUtTkJafc2ITPI2cYA YsuPKyL9xtk3qZ+Lqts4ei7Bjg8yCi/VPzGFZpRQq/BXJIQUwQsKgV5JDta+Q5JA+Q hHXL3hM9ngA6IQbWG7JPVOWYfFUFs92NiCaF+g4GNxCguPOhQ2wv5DNxc34VbWPMij DKHSD4VR6vmM9M/9K1FQS/q7VuKyhzkVuZwiCzBtsDANAGESLcQAYRa4uRRCsiURrt MfkB47yYR620Q== From: Geliang Tang To: mptcp@lists.linux.dev Cc: "Matthieu Baerts (NGI0)" Subject: [PATCH mptcp-next v8 3/8] mptcp: pm: more precise error messages Date: Wed, 8 Jan 2025 09:44:04 +0800 Message-ID: X-Mailer: git-send-email 2.45.2 In-Reply-To: References: Precedence: bulk X-Mailing-List: mptcp@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Matthieu Baerts (NGI0)" 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. By splitting some error messages, NL_SET_ERR_MSG_ATTR() can be used instead of GENL_SET_ERR_MSG() in order to give an additional hint to the userspace developers about which attribute is wrong. While at it, in mptcp_userspace_pm_set_flags() move the parsing after the check linked to the local attribute. Signed-off-by: Matthieu Baerts (NGI0) --- net/mptcp/pm_userspace.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index b6cf8ea1161d..2cb42091bd08 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) { + 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)) { + NL_SET_ERR_MSG_ATTR(info->extack, addr, "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) { + NL_SET_ERR_MSG_ATTR(info->extack, laddr, "missing local port"); + err = -EINVAL; + goto destroy_err; + } + + if (!addr_r.port) { + NL_SET_ERR_MSG_ATTR(info->extack, raddr, "missing remote port"); err = -EINVAL; goto destroy_err; } @@ -580,13 +592,20 @@ 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) { + NL_SET_ERR_MSG_ATTR(info->extack, attr, + "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) { + NL_SET_ERR_MSG_ATTR(info->extack, attr_rem, + "invalid remote address family"); ret = -EINVAL; goto set_flags_err; } From patchwork Wed Jan 8 01:44:05 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Geliang Tang X-Patchwork-Id: 13929941 X-Patchwork-Delegate: matthieu.baerts@tessares.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 46B68139CF2 for ; Wed, 8 Jan 2025 01:44:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300665; cv=none; b=ujUiM5MN0jv9THVIGOMLd1lfoFgNIlmymNh2D+4Z+jnsGceFFbV8MJVDq5G+dJBMdKvq3VInc1O/EkYO3ax/86MwQr1UEtBXZPyMrNb3bmE3gUNsuqLRRIoU0btO4KLF71vvQ/fxbZ/2JAzINwrwgkuknLzgpKsmhldFfGi59SI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300665; c=relaxed/simple; bh=/v+Lww7LFz6yGbabYhG0P65FdRLrBMBcX0CKiZajfGo=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=PA5rBfwNADXMlMNAHfQ6AM+Vu8h40kmwrU1elyaSPUbOjqJo/wn/9OSlP1zqIsiGK3Nt79cKzxq2O91YPX8VlTW+yvJgWYtRMU1SHCRiYI/9gfANYc9PVb7xT7RQCnoLbeIkp1cd8ZRXD3ACNdbp9PPMUUCukg4AKDV98LSYIZU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=BHSX9a0N; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="BHSX9a0N" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A802BC4CEE0; Wed, 8 Jan 2025 01:44:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736300664; bh=/v+Lww7LFz6yGbabYhG0P65FdRLrBMBcX0CKiZajfGo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=BHSX9a0N+5iyEMnAGoUqrBjFuZVCzuP822IwNyj1tnVoh25bXNYYKg4gzMrqDCVvy +b3UQ6Md7mj+z7c8RJ7Z96Q+Tn0Gq3JJ8PJu/J1ZU5S8aMtRHZRhUKZiJs15pkMJVv t46aZakAUZZOT4eUjhljkI4sFEaCHC6jiwqkpMCwWsuf5MW9/+IUbzUVCiYndk+DvO D10XZyMtcDhSAFtVGrRe93N+icRRWxVblZS9e9UgADsQ0FOKU5dvZeiLy6agWVULNv qW3D2zM8AWRQpFD1IoJ/qXWkyfoZVOcSDBPUfS+kRETGQjfRMsz/qxJ/ooK9V+xaLP 4QSi2Mv7I4DOg== From: Geliang Tang To: mptcp@lists.linux.dev Cc: "Matthieu Baerts (NGI0)" Subject: [PATCH mptcp-next v8 4/8] mptcp: pm: improve error messages Date: Wed, 8 Jan 2025 09:44:05 +0800 Message-ID: X-Mailer: git-send-email 2.45.2 In-Reply-To: References: Precedence: bulk X-Mailing-List: mptcp@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Matthieu Baerts (NGI0)" 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. v2: add a comment in mptcp_userspace_pm_set_flags(). Signed-off-by: Matthieu Baerts (NGI0) --- 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 98ac73938bd8..a60217faf95d 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 2cb42091bd08..f5372e0a0dd9 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; } @@ -627,6 +631,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 patchwork Wed Jan 8 01:44:06 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Geliang Tang X-Patchwork-Id: 13929942 X-Patchwork-Delegate: matthieu.baerts@tessares.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D5FB912CD88 for ; Wed, 8 Jan 2025 01:44:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300667; cv=none; b=izO61cRJEdmpV2sRs5nspV8/zAUt5JvFLgZxRX/Wgkl/iJx9z/mXD8sFNE3HiwVyf8ORu1T6F+lC0wMj7xaXA5Fp1GMt0sBhKNWKP2YKMVIzNeBKaSiuVjX9Al+B0N5+w2q4fpyhFECpRCQ7HERQCHsO5rFAL//BtDFvwF4HYMg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300667; c=relaxed/simple; bh=tAITuCpY9DClcp8iIfFaL+DngulYuZALomjNyDx+ziM=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=jcno7azox7moM+BYmDNed86CLSnnZVErkqqMV+jK+sb5LupYy3jfPU5ftDzAigA2AeffuaPk/MXlxyJvmqEwEv09+6+0ZbXamNUmPYOZdxfJtEI6d4T3rYobTypSha31PWcUfj5Qc+hUNQohb0lKfYZUwk6Yob3dkY+L9nLmxS8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=q8I61eyO; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="q8I61eyO" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 81EE5C4CEDF; Wed, 8 Jan 2025 01:44:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736300667; bh=tAITuCpY9DClcp8iIfFaL+DngulYuZALomjNyDx+ziM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=q8I61eyO+VGW+PI7n7LZSaK30Y5pemWyq2cuP6CCtKPsJK5rsIQXCNqiKHPQtq785 JgHVCoKrKWQGusjzkH12bNsMMyVlER/k6HXkMlmbf3EZz2VpGiHG+liRUlaKfQIvKl N2nAB8FE/N0YaibzTbXLxeY7u5bvnoBCkz7QuPkc7Q1L4OfIVvb1au8ACCRMJV4dZs hBFT44HxfcNmmmreuvi9S6IOtZauOs8AMNEeyc+goD4/z4/c1ZG0v4tM843EfxvoWZ bRukX9TiNfhK0FKPm/i08j+ZNCOqZ+KKbuQ4DgU1kBmoFUTQMoeKRIyddP3EjTAlTu oMy/WUqO+qjQw== From: Geliang Tang To: mptcp@lists.linux.dev Cc: Geliang Tang , Jakub Kicinski , Matthieu Baerts Subject: [PATCH mptcp-next v8 5/8] mptcp: pm: userspace: use GENL_REQ_ATTR_CHECK Date: Wed, 8 Jan 2025 09:44:06 +0800 Message-ID: X-Mailer: git-send-email 2.45.2 In-Reply-To: References: Precedence: bulk X-Mailing-List: mptcp@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Geliang Tang 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) --- 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 f5372e0a0dd9..90272cd99f7b 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 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info) 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 patchwork Wed Jan 8 01:44:07 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Geliang Tang X-Patchwork-Id: 13929943 X-Patchwork-Delegate: matthieu.baerts@tessares.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1FAD513792B for ; Wed, 8 Jan 2025 01:44:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300670; cv=none; b=UCfQKdrePLhFOnAKyRN1Q+6fqCagJdPzGetYZKOV1Cu0Kjqy+BsEEfofGJgng9az6aYC7ARO+yXmZo9uZ/pTy1xzF30cjFJ9y5Zh6rvnG6/Ult/R1AeF8bEan9edpa/3roO5xKtECi9+NXNShiBo+hBNF9vqUSfRnzbdva5RRkQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300670; c=relaxed/simple; bh=FnzzqbayxNIm8b3fUmAziev6iX9LYmHm31pnvEZbORQ=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=r/EO1Wo0/Iwr6ngjExiLW1L9pE0PUSn3x07PhSEc2mkxDjJNzHtRRXrVSxVYjy5zpOOiLis4wZveWaC3t1xG6ylSpJp9gA7ebv5nhB1/d5kUzZzcuMJbp/TrC7LElfp+weSgw2FkXKdraB7BmO9R+y8oFkQO9+WpFHETmdUwYl8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=RcsYd6MC; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="RcsYd6MC" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 93774C4CED6; Wed, 8 Jan 2025 01:44:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736300669; bh=FnzzqbayxNIm8b3fUmAziev6iX9LYmHm31pnvEZbORQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=RcsYd6MCH8BXxkLiIl4d9bvrILrTo2eSnQC260a+eQp6OEm62B20qILtqH4k0flIy NhF5US1lf61w4OyDoaDwk/D0AjAsrgaP7Vwh70RFafdTMLej6UP7GYeWox6+bwQNCl +VaHWmdcZIe8YnJa+hygb00OUCzmO/j4QJt9ewkUyJNKHbGXZgCQwbb5Y8icSg/NZY 7Wb1mvUBF9JyekU0OnSZWxlvjnemw5ekbvxHw3/cbSurORAln55Nl8m77URjj1DX5+ FRmRFrLqkqYj3Mgcjql50nEln7jcEinvQ+9Td1OL6ggzGjGVs6yvPvjPqKGmNWO0Kj 4hkDfVH9xLZtg== From: Geliang Tang To: mptcp@lists.linux.dev Cc: "Matthieu Baerts (NGI0)" Subject: [PATCH mptcp-next v8 6/8] mptcp: pm: remove duplicated error messages Date: Wed, 8 Jan 2025 09:44:07 +0800 Message-ID: X-Mailer: git-send-email 2.45.2 In-Reply-To: References: Precedence: bulk X-Mailing-List: mptcp@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Matthieu Baerts (NGI0)" 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". Signed-off-by: Matthieu Baerts (NGI0) --- 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 90272cd99f7b..7c6b1241664a 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) { NL_SET_ERR_MSG_ATTR(info->extack, addr, "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 patchwork Wed Jan 8 01:44:08 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Geliang Tang X-Patchwork-Id: 13929944 X-Patchwork-Delegate: matthieu.baerts@tessares.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E453E13BADF for ; Wed, 8 Jan 2025 01:44:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300672; cv=none; b=LWdOvUB/5OPUi0S3rWHVGYZYJxt/TsFej9WYdpS3KyFPB5rVRZRxTmWbLzDQJeqLafnocFbBg5AqgthBID4VjcRnmpIc/plDUE9esux5Eo9x4qNIDHkm7pw0G6cLRTJ9Al9HPijZJ0BDTBEhcv234u42V3fuJivbOlSXs/SJsqM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300672; c=relaxed/simple; bh=9CxQknm6b5hPwp/6SJpVRmrNspyR6bzDXro/7onkB4M=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=dh8W910mZ1SRXKBGeazsZonsqHZWeVaKExglh1vwA44Cfm+u+fQBZZSd98jnuZ20h/BEjCtlmmP05clv+feLnGfQotHEfr6Rb5KjgCYo/AYyI2SPb3EP/lQhO42I8uvwne3+Dp2xO1oa6I98fy4kFaafjkTDrWcEW1z9kvrIeIY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=UZ9FHQp7; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="UZ9FHQp7" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 54236C4CED6; Wed, 8 Jan 2025 01:44:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736300671; bh=9CxQknm6b5hPwp/6SJpVRmrNspyR6bzDXro/7onkB4M=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=UZ9FHQp7G+I6GCVyHjE8NVjb75W+HwwSq51v+QowuUj8CcIC7YIHXuQw1UsNbtodA mdNfI1lMf/OXScKOEayDFxIU0yUG+KNkoJx552n1mVY6Npv6hxpPJPkHSE1U/cvXrB ZAy5ZGOetiBPiIR5edk1eGKKEqC95i/hHV4nOyquTFVdxdLUWf5B6AHC+P6pfy3PmR u0iHacdkVhWPjCOmBXzstglhAiPFW8kBoedd9cvcpQ3OJleMsCyKnXGfjeZ1WlkjJg Jcwfki2gugeeJ5wtFqNZjATyXNXtFldb4U1cOQ2IhTsCZByb9YLKvDn5B+/VRu4kv+ cDAdIemL126uQ== From: Geliang Tang To: mptcp@lists.linux.dev Cc: "Matthieu Baerts (NGI0)" Subject: [PATCH mptcp-next v8 7/8] mptcp: pm: mark missing address attributes Date: Wed, 8 Jan 2025 09:44:08 +0800 Message-ID: <07fcafc9153f0386b0d7340e193bc73f806e7154.1736299989.git.tanggeliang@kylinos.cn> X-Mailer: git-send-email 2.45.2 In-Reply-To: References: Precedence: bulk X-Mailing-List: mptcp@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Matthieu Baerts (NGI0)" 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. v2: - keep "pernet = pm_nl_get_pernet(net);" at the beginning of mptcp_pm_nl_set_flags(). Signed-off-by: Matthieu Baerts (NGI0) --- 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 a60217faf95d..adef7f12914b 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 @@ static int mptcp_nl_remove_id_zero_address(struct net *net, 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 @@ int mptcp_nl_fill_addr(struct sk_buff *skb, 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 @@ static int mptcp_nl_set_flags(struct net *net, 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; pernet = pm_nl_get_pernet(net); + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR)) + return -EINVAL; + + 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 7c6b1241664a..1876b3b2cb2d 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; @@ -590,6 +594,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; @@ -679,20 +684,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 patchwork Wed Jan 8 01:44:09 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Geliang Tang X-Patchwork-Id: 13929945 X-Patchwork-Delegate: matthieu.baerts@tessares.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EEEF213792B for ; Wed, 8 Jan 2025 01:44:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300674; cv=none; b=uCyzVeAqGuYT/no3blLOJaQr3SVyrBKDfQ2QAkHg8MVYUtBpdCfIHk45L1FHYY/LTe3iR7i6TTKmgcENBqHgHlXWW5NIsTE4VdW0OaVY0dA44eNQf7BiCLni47v6buocRn8tFF6TM3FVUL5crzW6JBs0kk/ZE2JoE4t65dx8oWA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736300674; c=relaxed/simple; bh=pMN6zjOMl/nl6rz6Q31fwGDXcf4xv8aZwu393sat1TM=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=ged5FXsJyAHU8vYOm7OkGOS3GWZzgvdtKQZ6/wSRvpJqe334Dr54B56KeFLuN4zfiDxzIg89J+0taIpMUr7SZ5MZTVNTuX4SBPhJkoODfZChziWbC8yEww1Bxjv6+BE1iGrND9t0aJC0YGYh6ko0IE+nUo/JzNHMbftUa2nCDd4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=eTj8LSTs; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="eTj8LSTs" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2D91AC4CED6; Wed, 8 Jan 2025 01:44:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736300673; bh=pMN6zjOMl/nl6rz6Q31fwGDXcf4xv8aZwu393sat1TM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=eTj8LSTsVwpxyXfQmcZe3lfSgHu008m9Me4UkYoRGZj7ZeNsP7jEC5hQB6UvTO6Qh 5QiSph5u1AzeY0jWPl5sHLYNOVe0I5O/TLZAvBT2f8QIQ3zCL9v98cCNqMyY+QACIW rE9q0iL0rbhUAkvBrDWkJArHOpCZGYSDb/+yyGoXs0ktH2BEqtYhZ3y5e94TgNBhOK dMaCWsk3/5RxXnJ3RNvml85DBgOUZcu8kQrdFlvW2K6LL/VL9tym1Qyq26vjHri8Xb p6l5b4T0kXx9WDuj0KW8XVPG/1CY1F7d2P+sGZR8ZgXQki0SUb6N8lOO/wR5TquLWr xi7tNWUGNlF6w== From: Geliang Tang To: mptcp@lists.linux.dev Cc: "Matthieu Baerts (NGI0)" Subject: [PATCH mptcp-next v8 8/8] mptcp: pm: use NL_SET_ERR_MSG_ATTR when possible Date: Wed, 8 Jan 2025 09:44:09 +0800 Message-ID: <32317d5b2968ae381f700bd771e302531bddc292.1736299989.git.tanggeliang@kylinos.cn> X-Mailer: git-send-email 2.45.2 In-Reply-To: References: Precedence: bulk X-Mailing-List: mptcp@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Matthieu Baerts (NGI0)" 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. v2: - update the code related mptcp_userspace_pm_remove_id_zero_address() since a new patch to drop "info" parameter of this patch is added. v3: - not use NL_SET_ERR_MSG_ATTR in mptcp_pm_nl_set_flags(), since 'attr' will be removed in the commit "mptcp: add local & remote parameters for set_flags". Signed-off-by: Matthieu Baerts (NGI0) --- net/mptcp/pm_netlink.c | 13 ++++++++----- net/mptcp/pm_userspace.c | 13 ++++++++----- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index adef7f12914b..60418b8a6119 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; } diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 1876b3b2cb2d..16337d08186b 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; } @@ -233,7 +234,8 @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info) 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; } @@ -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; } @@ -724,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; }