From patchwork Wed Dec 20 05:56:09 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grant Erickson X-Patchwork-Id: 13499566 Received: from mohas.pair.com (mohas.pair.com [209.68.5.112]) (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 D4D321D528 for ; Wed, 20 Dec 2023 05:56:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=nuovations.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=nuovations.com Received: from mohas.pair.com (localhost [127.0.0.1]) by mohas.pair.com (Postfix) with ESMTP id C9E8F73100 for ; Wed, 20 Dec 2023 00:56:15 -0500 (EST) Received: from localhost.localdomain (unknown [IPv6:2601:647:5a00:15c1:ed73:7d5e:cff0:c0f5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mohas.pair.com (Postfix) with ESMTPSA id 8D79E7312C for ; Wed, 20 Dec 2023 00:56:15 -0500 (EST) From: Grant Erickson To: connman@lists.linux.dev Subject: [PATCH 1/5] inet: Use '__connman_inet_rtnl_recv' for RTNL default route transaction. Date: Tue, 19 Dec 2023 21:56:09 -0800 Message-ID: <20231220055613.2287074-2-gerickson@nuovations.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231220055613.2287074-1-gerickson@nuovations.com> References: <20231220055613.2287074-1-gerickson@nuovations.com> Precedence: bulk X-Mailing-List: connman@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: mailmunge 3.11 on 209.68.5.112 This leverages '__connman_inet_rtnl_recv', introduced at commit 97967b4aae40 ("inet: Add '__connman_inet_rtnl_recv'."), to complete the Routing Netlink (rtnl) default route addition or deletion transaction in 'iproute_default_modify'. Every rtnl route addition or deletion transaction consists of a request and a response phase. The request phase contains the details of the route to be added or deleted. The response phase contains the status of whether the request succeeded. Unfortunately, to date, 'iproute_default_modify' did not include the response phase. Therefore, it did not complete the route request transaction, rendering the returns status incomplete in that it only reflected the success or failure of the request phase. Unfortunately, for callers of interfaces that, in turn, called 'iproute_default_modify', this made all routing requests appear to artificially succeed even when, in fact, they were failing and elided response status that callers need to conditionally act in the face of the transaction failure. By adding '__connman_inet_rtnl_recv' to complete the response phase of the rtnl route transaction, the return status of 'iproute_default_modify', and those interfaces that call it, now accurately reflect both the request and response status of the transaction. --- src/inet.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/inet.c b/src/inet.c index e0f927a12dcb..8a1e34235230 100644 --- a/src/inet.c +++ b/src/inet.c @@ -4748,6 +4748,12 @@ static int iproute_default_modify(int cmd, uint32_t table_id, uint32_t metric, goto done; ret = __connman_inet_rtnl_send(&rth, &rth.req.n); + if (ret < 0) + goto done; + + ret = __connman_inet_rtnl_recv(&rth, NULL); + if (ret < 0) + goto done; done: __connman_inet_rtnl_close(&rth); From patchwork Wed Dec 20 05:56:10 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grant Erickson X-Patchwork-Id: 13499569 Received: from mohas.pair.com (mohas.pair.com [209.68.5.112]) (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 0A2EF1D698 for ; Wed, 20 Dec 2023 05:56:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=nuovations.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=nuovations.com Received: from mohas.pair.com (localhost [127.0.0.1]) by mohas.pair.com (Postfix) with ESMTP id 3217773128 for ; Wed, 20 Dec 2023 00:56:16 -0500 (EST) Received: from localhost.localdomain (unknown [IPv6:2601:647:5a00:15c1:ed73:7d5e:cff0:c0f5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mohas.pair.com (Postfix) with ESMTPSA id E966373147 for ; Wed, 20 Dec 2023 00:56:15 -0500 (EST) From: Grant Erickson To: connman@lists.linux.dev Subject: [PATCH 2/5] gateway: Handle -ESRCH in 'unset_default_gateway_route_common'. Date: Tue, 19 Dec 2023 21:56:10 -0800 Message-ID: <20231220055613.2287074-3-gerickson@nuovations.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231220055613.2287074-1-gerickson@nuovations.com> References: <20231220055613.2287074-1-gerickson@nuovations.com> Precedence: bulk X-Mailing-List: connman@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: mailmunge 3.11 on 209.68.5.112 This maps the error '-ESRCH' in 'unset_default_gateway_route_common' to 0 ("Success"). Generally, we mandate that gateway routes follow the documented lifecycle and finite state machine, using events and down- and upcalls to drive the lifecycle. There is one exception, however. When the Linux kernel recognizes that the next hop (that is, the "via" or RTA_GATEWAY portion of the route) for a route becomes unreachable, it is automatically purged from the routing table with no RTM_DELROUTE RTNL notification. Consequently, routes so purged will return -ESRCH when we attempt to delete them here in the mistaken belief they are still there. By mapping -ESRCH to 0 ("Success") we ensure that gateway configuration for such routes is not indefinitely stuck in the "active" or "added" states but rather is correctly advanced to the "removed" state. --- src/gateway.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/gateway.c b/src/gateway.c index 29115250d243..db8636eb20ef 100644 --- a/src/gateway.c +++ b/src/gateway.c @@ -221,6 +221,11 @@ * interface basis and is computed by * 'compute_low_priority_metric'. * + * There is one exception to the above. When the Linux kernel + * recognizes that the next hop for a route becomes unreachable, it + * is automatically purged from the routing table with no + * RTM_DELROUTE RTNL notification. + * * Historically, this file started life as "connection.c". However, * it was renamed to "gateway.c" since its primary focus is gateway * routes and gateway route management. @@ -1864,8 +1869,6 @@ done: * @retval -EPERM If the current process does not have the * credentials or capabilities to unset, or * clear, routes. - * @retval -ESRCH A request was made to unset, or clear a - * non-existing routing entry. * * @sa gateway_config_state_set * @sa is_gateway_config_state @@ -1892,8 +1895,27 @@ static int unset_default_gateway_route_common(struct gateway_data *data, if (is_gateway_config_state_inactive(config)) return -EALREADY; + /* + * Generally, we mandate that gateway routes follow the documented + * lifecycle and state machine, using events and down- and upcalls + * to drive the lifecycle. + * + * There is one exception, however. When the Linux kernel + * recognizes that the next hop (that is, the "via" or RTA_GATEWAY + * part of the route) for a route becomes unreachable, it is + * automatically purged from the routing table with no + * RTM_DELROUTE RTNL notification. Consequently, routes so purged + * will return -ESRCH when we attempt to delete them here in the + * mistaken belief they are still there. + * + * Map -ESRCH to success such that gateway configuration for such + * routes is not indefinitely stuck in the "active" or "added" + * states. + */ err = cb(data, config); - if (err < 0) + if (err == -ESRCH) + err = 0; + else if (err < 0) goto done; gateway_config_state_set(config, From patchwork Wed Dec 20 05:56:11 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grant Erickson X-Patchwork-Id: 13499567 Received: from mohas.pair.com (mohas.pair.com [209.68.5.112]) (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 61A301D69C for ; Wed, 20 Dec 2023 05:56:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=nuovations.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=nuovations.com Received: from mohas.pair.com (localhost [127.0.0.1]) by mohas.pair.com (Postfix) with ESMTP id 8E39273145 for ; Wed, 20 Dec 2023 00:56:16 -0500 (EST) Received: from localhost.localdomain (unknown [IPv6:2601:647:5a00:15c1:ed73:7d5e:cff0:c0f5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mohas.pair.com (Postfix) with ESMTPSA id 5217E73150 for ; Wed, 20 Dec 2023 00:56:16 -0500 (EST) From: Grant Erickson To: connman@lists.linux.dev Subject: [PATCH 3/5] gateway: Address unhandled gateway lifecycle events/transitions. Date: Tue, 19 Dec 2023 21:56:11 -0800 Message-ID: <20231220055613.2287074-4-gerickson@nuovations.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231220055613.2287074-1-gerickson@nuovations.com> References: <20231220055613.2287074-1-gerickson@nuovations.com> Precedence: bulk X-Mailing-List: connman@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: mailmunge 3.11 on 209.68.5.112 There were a number of "escapes" or unhandled events and transitions that did not adhere to the documented gateway configuration lifecycle / state machine. This handles those events and transitions. Failure to handle these was resulting in duplicate default routes, no default routes, and incorrect default route priorities for some services and their underlying network interfaces. --- src/gateway.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/gateway.c b/src/gateway.c index db8636eb20ef..b5a1e2ddbf19 100644 --- a/src/gateway.c +++ b/src/gateway.c @@ -787,12 +787,6 @@ static bool is_gateway_config_type(const struct gateway_config *config, return config->type == type; } -static bool is_gateway_config_type_none(const struct gateway_config *config) -{ - return is_gateway_config_type(config, - CONNMAN_GATEWAY_CONFIG_TYPE_NONE); -} - /** * @brief * Conditionally log the specified gateway configuration. @@ -1809,7 +1803,8 @@ static int set_default_gateway_route_common(struct gateway_data *data, if (!data || !config || !cb) return -EINVAL; - if (!is_gateway_config_type_none(config) && + if ((is_gateway_config_state_added(config) || + is_gateway_config_state_active(config)) && !is_gateway_config_type(config, type)) return -EINVAL; @@ -1886,7 +1881,8 @@ static int unset_default_gateway_route_common(struct gateway_data *data, if (!data || !config || !cb) return -EINVAL; - if (!is_gateway_config_type(config, type)) + if (!is_gateway_config_state_inactive(config) && + !is_gateway_config_type(config, type)) return -EINVAL; if (is_gateway_config_state_removed(config)) @@ -3359,6 +3355,12 @@ static void gateway_rtnl_new(int index, const char *gateway) return; } + if (is_gateway_config_state_inactive(config)) { + DBG("ignoring inactive gateway activation"); + + return; + } + /* * Otherwise, this is a gateway default route we added, or set, * and it is now acknowledged by the kernel. Consequently, mark it @@ -3469,11 +3471,18 @@ static void gateway_rtnl_del(int index, const char *gateway) if (config) { GATEWAY_CONFIG_DBG("config", config); - gateway_config_state_set(config, - CONNMAN_GATEWAY_CONFIG_STATE_INACTIVE); + if (is_gateway_config_state_removed(config)) { + gateway_config_state_set(config, + CONNMAN_GATEWAY_CONFIG_STATE_INACTIVE); - gateway_config_type_set(config, - CONNMAN_GATEWAY_CONFIG_TYPE_NONE); + gateway_config_type_set(config, + CONNMAN_GATEWAY_CONFIG_TYPE_NONE); + } else { + DBG("ignoring gateway stale removed activation; " + "probably added before removed activation completed"); + + return; + } } else DBG("no matching gateway config"); From patchwork Wed Dec 20 05:56:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grant Erickson X-Patchwork-Id: 13499568 Received: from mohas.pair.com (mohas.pair.com [209.68.5.112]) (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 BC5F51D6A2 for ; Wed, 20 Dec 2023 05:56:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=nuovations.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=nuovations.com Received: from mohas.pair.com (localhost [127.0.0.1]) by mohas.pair.com (Postfix) with ESMTP id EA80773144 for ; Wed, 20 Dec 2023 00:56:16 -0500 (EST) Received: from localhost.localdomain (unknown [IPv6:2601:647:5a00:15c1:ed73:7d5e:cff0:c0f5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mohas.pair.com (Postfix) with ESMTPSA id AE14C7315A for ; Wed, 20 Dec 2023 00:56:16 -0500 (EST) From: Grant Erickson To: connman@lists.linux.dev Subject: [PATCH 4/5] gateway: Add and leveage 'gateway_config_set_{,in}active'. Date: Tue, 19 Dec 2023 21:56:12 -0800 Message-ID: <20231220055613.2287074-5-gerickson@nuovations.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231220055613.2287074-1-gerickson@nuovations.com> References: <20231220055613.2287074-1-gerickson@nuovations.com> Precedence: bulk X-Mailing-List: connman@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: mailmunge 3.11 on 209.68.5.112 This encapsulates and makes symmetric the actions at the "terminal ends" of the gateway configuration lifecycle / state machine by adding and leveraging 'gateway_config_set_{,in}active' to set the gateway configuration state and type appropriately. --- src/gateway.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/gateway.c b/src/gateway.c index b5a1e2ddbf19..42a8da72642c 100644 --- a/src/gateway.c +++ b/src/gateway.c @@ -787,6 +787,20 @@ static bool is_gateway_config_type(const struct gateway_config *config, return config->type == type; } +static void gateway_config_set_active(struct gateway_config *config) +{ + gateway_config_state_set(config, CONNMAN_GATEWAY_CONFIG_STATE_ACTIVE); +} + +static void gateway_config_set_inactive(struct gateway_config *config) +{ + gateway_config_state_set(config, + CONNMAN_GATEWAY_CONFIG_STATE_INACTIVE); + + gateway_config_type_set(config, + CONNMAN_GATEWAY_CONFIG_TYPE_NONE); +} + /** * @brief * Conditionally log the specified gateway configuration. @@ -3366,8 +3380,7 @@ static void gateway_rtnl_new(int index, const char *gateway) * and it is now acknowledged by the kernel. Consequently, mark it * as active. */ - gateway_config_state_set(config, - CONNMAN_GATEWAY_CONFIG_STATE_ACTIVE); + gateway_config_set_active(config); /* * It is possible that we have two default routes atm @@ -3471,13 +3484,9 @@ static void gateway_rtnl_del(int index, const char *gateway) if (config) { GATEWAY_CONFIG_DBG("config", config); - if (is_gateway_config_state_removed(config)) { - gateway_config_state_set(config, - CONNMAN_GATEWAY_CONFIG_STATE_INACTIVE); - - gateway_config_type_set(config, - CONNMAN_GATEWAY_CONFIG_TYPE_NONE); - } else { + if (is_gateway_config_state_removed(config)) + gateway_config_set_inactive(config); + else { DBG("ignoring gateway stale removed activation; " "probably added before removed activation completed"); From patchwork Wed Dec 20 05:56:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grant Erickson X-Patchwork-Id: 13499570 Received: from mohas.pair.com (mohas.pair.com [209.68.5.112]) (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 766B0FC07 for ; Wed, 20 Dec 2023 05:56:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=nuovations.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=nuovations.com Received: from mohas.pair.com (localhost [127.0.0.1]) by mohas.pair.com (Postfix) with ESMTP id 54AF473146 for ; Wed, 20 Dec 2023 00:56:17 -0500 (EST) Received: from localhost.localdomain (unknown [IPv6:2601:647:5a00:15c1:ed73:7d5e:cff0:c0f5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mohas.pair.com (Postfix) with ESMTPSA id 15E0473171 for ; Wed, 20 Dec 2023 00:56:17 -0500 (EST) From: Grant Erickson To: connman@lists.linux.dev Subject: [PATCH 5/5] gateway: Expand the @param documentation for 'yield_default_gateway{,_for_type}'. Date: Tue, 19 Dec 2023 21:56:13 -0800 Message-ID: <20231220055613.2287074-6-gerickson@nuovations.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231220055613.2287074-1-gerickson@nuovations.com> References: <20231220055613.2287074-1-gerickson@nuovations.com> Precedence: bulk X-Mailing-List: connman@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: mailmunge 3.11 on 209.68.5.112 This expands on the @param documentation for the 'yield_default_gateway{,_for_type}' functions by being more clear about the source of the parameters. --- src/gateway.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/gateway.c b/src/gateway.c index 42a8da72642c..16c87f83b4d6 100644 --- a/src/gateway.c +++ b/src/gateway.c @@ -3102,9 +3102,11 @@ static int promote_default_gateway(struct gateway_data *data, * existing gateway from the services-to-gateway data hash. * * @param[in,out] activated A pointer to a mutable newly-activated - * gateway. + * gateway from a Routing Netlink (rtnl) + * notification. * @param[in,out] existing A pointer to a mutable existing - * gateway. + * gateway from the services-to-gateway + * hash. * @param[in] type The IP configuration type for which * gateway, or default router, is to be * yielded. @@ -3200,9 +3202,11 @@ done: * existing gateway from the services-to-gateway data hash. * * @param[in,out] activated A pointer to a mutable newly-activated - * gateway. + * gateway from a Routing Netlink (rtnl) + * notification. * @param[in,out] existing A pointer to a mutable existing - * gateway. + * gateway from the services-to-gateway + * hash. * * @returns * True if @a activated yielded the default gateway; otherwise,