From patchwork Wed Nov 29 07:39:43 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grant Erickson X-Patchwork-Id: 13472372 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 BE42410A10 for ; Wed, 29 Nov 2023 07:39:49 +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 Authentication-Results: smtp.subspace.kernel.org; dkim=none Received: from mohas.pair.com (localhost [127.0.0.1]) by mohas.pair.com (Postfix) with ESMTP id CE3C573150 for ; Wed, 29 Nov 2023 02:39:48 -0500 (EST) Received: from localhost.localdomain (unknown [IPv6:2601:647:5a00:15c1:230d:b2c9:c388:f96b]) (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 9132673159 for ; Wed, 29 Nov 2023 02:39:48 -0500 (EST) From: Grant Erickson To: connman@lists.linux.dev Subject: [PATCH 1/4] connection: Refactor 'connection_newgateway'. Date: Tue, 28 Nov 2023 23:39:43 -0800 Message-ID: <20231129073947.1280705-2-gerickson@nuovations.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231129073947.1280705-1-gerickson@nuovations.com> References: <20231129073947.1280705-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 refactors the body tail of 'connection_newgateway' into a separate function, 'check_default_gateway', shortening the length of the former and making the latter an individually auditable and comprehensible function. --- src/connection.c | 86 +++++++++++++++++++++++++++++++----------------- 1 file changed, 55 insertions(+), 31 deletions(-) diff --git a/src/connection.c b/src/connection.c index ce38f35d68c7..747b744a295e 100644 --- a/src/connection.c +++ b/src/connection.c @@ -1051,6 +1051,57 @@ static bool choose_default_gateway(struct gateway_data *data, return downgraded; } +static void check_default_gateway(struct gateway_data *activated) +{ + GHashTableIter iter; + gpointer value, key; + bool found = false; + + DBG("activated %p", activated); + + GATEWAY_DATA_DBG("activated", activated); + + /* + * If we have already handled a Routing Netlink (rtnl) + * notification and checked the newly-activated gateway against + * the existing gateway / default routers, simply return. + * + * Otherwise, failure to use this 'default_checked' sentinel could + * lead into an infinite Routing Netlink (rntl) loop as changing + * the default gateway pushes a new route into the kernel and ends + * up back here again (via the .newgateway method). + */ + if (activated->default_checked) + return; + + g_hash_table_iter_init(&iter, gateway_hash); + + while (g_hash_table_iter_next(&iter, &key, &value)) { + struct gateway_data *existing = value; + + if (existing == activated) + continue; + + found = choose_default_gateway(activated, existing); + if (found) + break; + } + + DBG("found %u", found); + + if (!found) { + if (activated->ipv4_config) + set_default_gateway(activated, + CONNMAN_IPCONFIG_TYPE_IPV4); + + if (activated->ipv6_config) + set_default_gateway(activated, + CONNMAN_IPCONFIG_TYPE_IPV6); + } + + activated->default_checked = true; +} + /** * @brief * Handler for gateway, or default route, -specific routes newly @@ -1070,6 +1121,7 @@ static bool choose_default_gateway(struct gateway_data *data, * formatted address of the gateway, or default * router, that was added. * + * @sa check_default_gateway * @sa set_default_gateway * @sa connection_delgateway * @@ -1079,9 +1131,6 @@ static void connection_newgateway(int index, const char *gateway) g_autofree char *interface = NULL; struct gateway_config *config; struct gateway_data *data; - GHashTableIter iter; - gpointer value, key; - bool found = false; interface = connman_inet_ifname(index); @@ -1118,36 +1167,11 @@ static void connection_newgateway(int index, const char *gateway) GATEWAY_DATA_DBG("data", data); - if (data->default_checked) - return; - /* - * The next checks are only done once, otherwise setting - * the default gateway could lead into rtnl forever loop. + * Check whether this newly-activated gateway should yield or + * become the default. */ - - g_hash_table_iter_init(&iter, gateway_hash); - - while (g_hash_table_iter_next(&iter, &key, &value)) { - struct gateway_data *candidate = value; - - if (candidate == data) - continue; - - found = choose_default_gateway(data, candidate); - if (found) - break; - } - - if (!found) { - if (data->ipv4_config) - set_default_gateway(data, CONNMAN_IPCONFIG_TYPE_IPV4); - - if (data->ipv6_config) - set_default_gateway(data, CONNMAN_IPCONFIG_TYPE_IPV6); - } - - data->default_checked = true; + check_default_gateway(data); } static void remove_gateway(gpointer user_data) From patchwork Wed Nov 29 07:39:44 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grant Erickson X-Patchwork-Id: 13472373 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 68FD110A21 for ; Wed, 29 Nov 2023 07:39:50 +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 Authentication-Results: smtp.subspace.kernel.org; dkim=none Received: from mohas.pair.com (localhost [127.0.0.1]) by mohas.pair.com (Postfix) with ESMTP id 3A85873156 for ; Wed, 29 Nov 2023 02:39:49 -0500 (EST) Received: from localhost.localdomain (unknown [IPv6:2601:647:5a00:15c1:230d:b2c9:c388:f96b]) (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 ED3CC731A8 for ; Wed, 29 Nov 2023 02:39:48 -0500 (EST) From: Grant Erickson To: connman@lists.linux.dev Subject: [PATCH 2/4] connection: Document 'check_default_gateway'. Date: Tue, 28 Nov 2023 23:39:44 -0800 Message-ID: <20231129073947.1280705-3-gerickson@nuovations.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231129073947.1280705-1-gerickson@nuovations.com> References: <20231129073947.1280705-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 adds documentation to the 'check_default_gateway' function. --- src/connection.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/connection.c b/src/connection.c index 747b744a295e..8f6a08e12cf3 100644 --- a/src/connection.c +++ b/src/connection.c @@ -1051,6 +1051,26 @@ static bool choose_default_gateway(struct gateway_data *data, return downgraded; } +/** + * @brief + * Check whether the specified gateway should yield or become the + * default. + * + * This compares the specified, ostenisbly new, gateway data against + * all, known existing gateway data in the service-to-gateway hash + * and determines whether or not the default should be ceded from an + * existing gateway and given to the new, incoming gateway or vice + * versa. + * + * @param[in,out] activated A pointer to the mutable gateway data + * associated with a newly-activated + * gateway route which is to be checked + * against existing gateway data. + * + * @sa choose_default_gateway + * @sa connection_newgateway + * + */ static void check_default_gateway(struct gateway_data *activated) { GHashTableIter iter; From patchwork Wed Nov 29 07:39:45 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grant Erickson X-Patchwork-Id: 13472374 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 839B110A29 for ; Wed, 29 Nov 2023 07:39:50 +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 Authentication-Results: smtp.subspace.kernel.org; dkim=none Received: from mohas.pair.com (localhost [127.0.0.1]) by mohas.pair.com (Postfix) with ESMTP id 916A273180 for ; Wed, 29 Nov 2023 02:39:49 -0500 (EST) Received: from localhost.localdomain (unknown [IPv6:2601:647:5a00:15c1:230d:b2c9:c388:f96b]) (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 54AE9731F7 for ; Wed, 29 Nov 2023 02:39:49 -0500 (EST) From: Grant Erickson To: connman@lists.linux.dev Subject: [PATCH 3/4] connection: Rename 'choose_default_gateway'. Date: Tue, 28 Nov 2023 23:39:45 -0800 Message-ID: <20231129073947.1280705-4-gerickson@nuovations.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231129073947.1280705-1-gerickson@nuovations.com> References: <20231129073947.1280705-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 renames 'choose_default_gateway' as 'yield_default_gateway'. The latter more accurately reflects the action of the function which is determining which of two gateway data should cede or yield the default gateway and associated routes. The actual "choice" and acquisition of the default gateway and associated routes happens outside of the function. --- src/connection.c | 77 +++++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 34 deletions(-) diff --git a/src/connection.c b/src/connection.c index 8f6a08e12cf3..a8cc6d430467 100644 --- a/src/connection.c +++ b/src/connection.c @@ -1002,53 +1002,62 @@ static void unset_default_gateway(struct gateway_data *data, data->ipv4_config->gateway); } -static bool choose_default_gateway(struct gateway_data *data, - struct gateway_data *candidate) +static bool yield_default_gateway(struct gateway_data *activated, + struct gateway_data *existing) { - bool downgraded = false; + enum connman_ipconfig_type type; + bool yield_activated = false; - GATEWAY_DATA_DBG("data", data); - GATEWAY_DATA_DBG("candidate", candidate); + DBG("activated %p existing %p", activated, existing); + + GATEWAY_DATA_DBG("activated", activated); + GATEWAY_DATA_DBG("existing", existing); /* * If the current default is not active, then we mark * this one as default. If the other one is already active * we mark this one as non default. */ - if (data->ipv4_config && candidate->ipv4_config) { + if (activated->ipv4_config && existing->ipv4_config) { + type = CONNMAN_IPCONFIG_TYPE_IPV4; - if (!candidate->ipv4_config->active) { - DBG("ipv4 downgrading %p", candidate); - unset_default_gateway(candidate, - CONNMAN_IPCONFIG_TYPE_IPV4); + if (!existing->ipv4_config->active) { + DBG("ipv4 existing %p yielding default", existing); + + unset_default_gateway(existing, type); } - if (candidate->ipv4_config->active && - __connman_service_compare(candidate->service, - data->service) < 0) { - DBG("ipv4 downgrading this %p", data); - unset_default_gateway(data, CONNMAN_IPCONFIG_TYPE_IPV4); - downgraded = true; + if (existing->ipv4_config->active && + __connman_service_compare(existing->service, + activated->service) < 0) { + DBG("ipv4 activated %p yielding default", activated); + + unset_default_gateway(activated, type); + + yield_activated = true; } } - if (data->ipv6_config && candidate->ipv6_config) { - if (!candidate->ipv6_config->active) { - DBG("ipv6 downgrading %p", candidate); - unset_default_gateway(candidate, - CONNMAN_IPCONFIG_TYPE_IPV6); + if (activated->ipv6_config && existing->ipv6_config) { + type = CONNMAN_IPCONFIG_TYPE_IPV6; + if (!existing->ipv6_config->active) { + DBG("ipv6 existing %p yielding default", existing); + + unset_default_gateway(existing, type); } - if (candidate->ipv6_config->active && - __connman_service_compare(candidate->service, - data->service) < 0) { - DBG("ipv6 downgrading this %p", data); - unset_default_gateway(data, CONNMAN_IPCONFIG_TYPE_IPV6); - downgraded = true; + if (existing->ipv6_config->active && + __connman_service_compare(existing->service, + activated->service) < 0) { + DBG("ipv6 activated %p yielding default", activated); + + unset_default_gateway(activated, type); + + yield_activated = true; } } - return downgraded; + return yield_activated; } /** @@ -1067,7 +1076,7 @@ static bool choose_default_gateway(struct gateway_data *data, * gateway route which is to be checked * against existing gateway data. * - * @sa choose_default_gateway + * @sa yield_default_gateway * @sa connection_newgateway * */ @@ -1075,7 +1084,7 @@ static void check_default_gateway(struct gateway_data *activated) { GHashTableIter iter; gpointer value, key; - bool found = false; + bool yield_activated = false; DBG("activated %p", activated); @@ -1102,14 +1111,14 @@ static void check_default_gateway(struct gateway_data *activated) if (existing == activated) continue; - found = choose_default_gateway(activated, existing); - if (found) + yield_activated = yield_default_gateway(activated, existing); + if (yield_activated) break; } - DBG("found %u", found); + DBG("yield_activated %u", yield_activated); - if (!found) { + if (!yield_activated) { if (activated->ipv4_config) set_default_gateway(activated, CONNMAN_IPCONFIG_TYPE_IPV4); From patchwork Wed Nov 29 07:39:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grant Erickson X-Patchwork-Id: 13472375 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 CF0D210A2C for ; Wed, 29 Nov 2023 07:39:50 +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 Authentication-Results: smtp.subspace.kernel.org; dkim=none Received: from mohas.pair.com (localhost [127.0.0.1]) by mohas.pair.com (Postfix) with ESMTP id EF06D731BE for ; Wed, 29 Nov 2023 02:39:49 -0500 (EST) Received: from localhost.localdomain (unknown [IPv6:2601:647:5a00:15c1:230d:b2c9:c388:f96b]) (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 B06F4731FD for ; Wed, 29 Nov 2023 02:39:49 -0500 (EST) From: Grant Erickson To: connman@lists.linux.dev Subject: [PATCH 4/4] connection: Document 'yield_default_gateway'. Date: Tue, 28 Nov 2023 23:39:46 -0800 Message-ID: <20231129073947.1280705-5-gerickson@nuovations.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231129073947.1280705-1-gerickson@nuovations.com> References: <20231129073947.1280705-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 adds documentation to the 'yield_default_gateway' function. --- src/connection.c | 63 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 3 deletions(-) diff --git a/src/connection.c b/src/connection.c index a8cc6d430467..a4b98c7c7c5f 100644 --- a/src/connection.c +++ b/src/connection.c @@ -1002,6 +1002,31 @@ static void unset_default_gateway(struct gateway_data *data, data->ipv4_config->gateway); } +/** + * @brief + * Decide whether either of the specified gateways should yield the + * default gateway route. + * + * This determines whether either of the specified gateway data + * should yield the default gateway route via + * #unset_default_gateway. @a activated is a newly-activated gateway + * from a Routing Netlink (rtnl) notification. @a existing is an + * existing gateway from the services-to-gateway data hash. + * + * @param[in,out] activated A pointer to a mutable newly-activated + * gateway. + * @param[in,out] existing A pointer to a mutable existing + * gateway. + * + * @returns + * True of @a activated yielded the default gateway; otherwise, + * false. + * + * @sa check_default_gateway + * @sa __connman_service_compare + * @sa unset_default_gateway + * + */ static bool yield_default_gateway(struct gateway_data *activated, struct gateway_data *existing) { @@ -1014,19 +1039,32 @@ static bool yield_default_gateway(struct gateway_data *activated, GATEWAY_DATA_DBG("existing", existing); /* - * If the current default is not active, then we mark - * this one as default. If the other one is already active - * we mark this one as non default. + * There is only an IPv4 default gateway yield decision to be + * considered if there is an IPv4 gateway configuration for BOTH + * the activated and existing gateway data. */ if (activated->ipv4_config && existing->ipv4_config) { type = CONNMAN_IPCONFIG_TYPE_IPV4; + /* + * If the existing IPv4 gateway data IS NOT active (that is, + * HAS NOT made it to the RTNL notification phase of its + * lifecycle), then it yields the default gateway to the + * activated gateway data. + */ if (!existing->ipv4_config->active) { DBG("ipv4 existing %p yielding default", existing); unset_default_gateway(existing, type); } + /* + * If the existing IPv4 gateway data IS active (that is, HAS + * made it to the RTNL notification phase of its lifecycle) + * and if its associated service is more "senior" in the + * service sort order, then the activated gateway data yields + * the default gateway to the existing gateway data. + */ if (existing->ipv4_config->active && __connman_service_compare(existing->service, activated->service) < 0) { @@ -1038,14 +1076,33 @@ static bool yield_default_gateway(struct gateway_data *activated, } } + /* + * There is only an IPv6 default gateway yield decision to be + * considered if there is an IPv6 gateway configuration for BOTH + * the activated and existing gateway data. + */ if (activated->ipv6_config && existing->ipv6_config) { type = CONNMAN_IPCONFIG_TYPE_IPV6; + + /* + * If the existing IPv6 gateway data IS NOT active (that is, + * HAS NOT made it to the RTNL notification phase of its + * lifecycle), then it yields the default gateway to the + * activated gateway data. + */ if (!existing->ipv6_config->active) { DBG("ipv6 existing %p yielding default", existing); unset_default_gateway(existing, type); } + /* + * If the existing IPv6 gateway data IS active (that is, HAS + * made it to the RTNL notification phase of its lifecycle) + * and if its associated service is more "senior" in the + * service sort order, then the activated gateway data yields + * the default gateway to the existing gateway data. + */ if (existing->ipv6_config->active && __connman_service_compare(existing->service, activated->service) < 0) {