connman.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority
@ 2023-12-16  6:12 Grant Erickson
  2023-12-16  6:12 ` [PATCH 01/11] service: Add '__connman_service_get_route_metric' Grant Erickson
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Grant Erickson @ 2023-12-16  6:12 UTC (permalink / raw)
  To: connman

This add and leverages '__connman_service_get_route_metric' to select
an appropriate service-specific route metric/priority when adding or
deleting WISPr host routes.
    
This allows multiple such routes to coexist simultaneously, supporting
"continuous" mode online checks in which one or more services may be
conducting "online" WISPr-based Internet reachability checks as they
move in and out of reachability success/failure.

'__connman_service_get_route_metric' is a non-public service
interface, which attempts to get the route metric/priority for the
specified service based on the current service and services state.

If the service is the default or if it is the only service, then the
metric is zero (0). Otherwise, a low-priority metric (metric > 0)
unique to the service and its underlying network interface is computed
and returned.

Grant Erickson (11):
  service: Add '__connman_service_get_route_metric'.
  service: Document '__connman_service_get_route_metric'.
  wispr: Expand 'DBG' in 'wispr_portal_detect'.
  wispr: Add 'DBG' to '__connman_wispr_start' error path.
  wispr: Update 'DBG' in '__connman_wispr_start'.
  wispr: Refactor 'free_wispr_routes'.
  wispr: Simplify IPv4 vs. IPv6 host route management.
  wispr: Document 'free_wispr_route{,s}'.
  wispr: Document 'wispr_route'.
  wispr: Document 'wispr_portal_context_route_ops'.
  wispr: Leverage '__connman_service_get_route_metric'.

 src/connman.h |   2 +
 src/service.c |  98 ++++++++++++++++++++++++
 src/wispr.c   | 207 +++++++++++++++++++++++++++++++++++++++++---------
 3 files changed, 271 insertions(+), 36 deletions(-)

-- 
2.42.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 01/11] service: Add '__connman_service_get_route_metric'.
  2023-12-16  6:12 [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Grant Erickson
@ 2023-12-16  6:12 ` Grant Erickson
  2023-12-16  6:12 ` [PATCH 02/11] service: Document '__connman_service_get_route_metric' Grant Erickson
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Grant Erickson @ 2023-12-16  6:12 UTC (permalink / raw)
  To: connman

This adds a new non-public service interface,
'__connman_service_get_route_metric', which attempts to get the route
metric/priority for the specified service based on the current service
and services state.

If the service is the default or if it is the only service, then the
metric is zero (0). Otherwise, a low-priority metric (metric > 0)
unique toservice and its underlying network interface is computed and
returned.
---
 src/connman.h |  2 ++
 src/service.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/src/connman.h b/src/connman.h
index 24e667d8e1f4..90d5a34b8c58 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -733,6 +733,8 @@ void __connman_service_remove_from_network(struct connman_network *network);
 void __connman_service_read_ip4config(struct connman_service *service);
 void __connman_service_read_ip6config(struct connman_service *service);
 
+int __connman_service_get_route_metric(const struct connman_service *service,
+				uint32_t *metric);
 struct connman_ipconfig *__connman_service_get_ip4config(
 				struct connman_service *service);
 struct connman_ipconfig *__connman_service_get_ip6config(
diff --git a/src/service.c b/src/service.c
index 020e43fa4038..f0b37e9a0d41 100644
--- a/src/service.c
+++ b/src/service.c
@@ -6539,6 +6539,70 @@ __connman_service_get_network(struct connman_service *service)
 	return service->network;
 }
 
+static size_t service_get_count(void)
+{
+	return service_list ? g_list_length(service_list) : 0;
+}
+
+int __connman_service_get_route_metric(const struct connman_service *service,
+				uint32_t *metric)
+{
+	static const uint32_t metric_base = UINT32_MAX;
+	static const uint32_t metric_ceiling = (1 << 20);
+	static const uint32_t metric_index_step = (1 << 10);
+	int index;
+
+	DBG("");
+
+	if (!service || !metric)
+		return -EINVAL;
+
+	DBG("service %p (%s) metric %p",
+		service, connman_service_get_identifier(service),
+		metric);
+
+	index = __connman_service_get_index(service);
+	if (index < 0)
+		return -ENXIO;
+
+	/*
+	 * The algorithm uses the network interface index since it is
+	 * assumed to be stable for the uptime of the network interface
+	 * and, consequently, the potential maximum lifetime of the route.
+	 *
+	 * The algorithm establishes UINT32_MAX as the metric base (the
+	 * lowest possible priority) and a somewhat-arbitrary 2^20 as the
+	 * ceiling (to keep metrics out of a range that might be used by
+	 * other applications). The metric is then adjusted in increments
+	 * of 1,024 (2^10) from the base, but less than the ceiling, by
+	 * multiplying the increment by the network interface index. This
+	 * is easy and simple to compute and is invariant on service
+	 * order.
+	 *
+	 * In the fullness of time, the "rule of least astonishment" for
+	 * Connection Manager might be that low priority metrics follow
+	 * the service order with the default service always having metric
+	 * zero (0) and lowest priority metric assigned to the lowest
+	 * priority service, etc. Achieving this would require having
+	 * access to APIs (such as '__connman_service_get_count()' and
+	 * '__connman_service_get_order(service)') that expose a
+	 * strictly-in/decreasing service order with no duplicates. Today,
+	 * there is no such API nor is there such a durable service order
+	 * meeting that mathematical requirement.
+	 */
+
+	if (service_get_count() <= 1 || connman_service_is_default(service))
+		*metric = 0;
+	else
+		*metric = MAX(metric_ceiling,
+					metric_base -
+					(index * metric_index_step));
+
+	DBG("metric %u", *metric);
+
+	return 0;
+}
+
 struct connman_ipconfig *
 __connman_service_get_ip4config(struct connman_service *service)
 {
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 02/11] service: Document '__connman_service_get_route_metric'.
  2023-12-16  6:12 [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Grant Erickson
  2023-12-16  6:12 ` [PATCH 01/11] service: Add '__connman_service_get_route_metric' Grant Erickson
@ 2023-12-16  6:12 ` Grant Erickson
  2023-12-16  6:12 ` [PATCH 03/11] wispr: Expand 'DBG' in 'wispr_portal_detect' Grant Erickson
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Grant Erickson @ 2023-12-16  6:12 UTC (permalink / raw)
  To: connman

This adds documentation to the '__connman_service_get_route_metric'
function.
---
 src/service.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/src/service.c b/src/service.c
index f0b37e9a0d41..1f2c9f54db9a 100644
--- a/src/service.c
+++ b/src/service.c
@@ -6539,11 +6539,45 @@ __connman_service_get_network(struct connman_service *service)
 	return service->network;
 }
 
+/**
+ *  @brief
+ *    Return the current service count.
+ *
+ *  @returns
+ *    The current service count.
+ *
+ */
 static size_t service_get_count(void)
 {
 	return service_list ? g_list_length(service_list) : 0;
 }
 
+/**
+ *  @brief
+ *    Get the route metric/priority for the specified service.
+ *
+ *  This attempts to get the route metric/priority for the specified
+ *  service based on the current service and services state.
+ *
+ *  If the service is the default or if it is the only service, then
+ *  the metric is zero (0). Otherwise, a low-priority metric (metric >
+ *  0) unique to @a service and its underlying network interface is
+ *  computed and returned.
+ *
+ *  @param[in]      service  A pointer to the immutable service for
+ *                           which to get the route metric/priority.
+ *  @param[in,out]  metric   A pointer to storage for the route
+ *                           metric/priority, populated with the route
+ *                           metric/priority on success.
+ *
+ *  @retval  0        If successful.
+ *  @retval  -EINVAL  If @a service or @a metric are null.
+ *  @retval  -ENXIO   If the network interface index associated with
+ *                    @a service is invalid.
+ *
+ *  @sa connman_service_is_default
+ *
+ */
 int __connman_service_get_route_metric(const struct connman_service *service,
 				uint32_t *metric)
 {
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 03/11] wispr: Expand 'DBG' in 'wispr_portal_detect'.
  2023-12-16  6:12 [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Grant Erickson
  2023-12-16  6:12 ` [PATCH 01/11] service: Add '__connman_service_get_route_metric' Grant Erickson
  2023-12-16  6:12 ` [PATCH 02/11] service: Document '__connman_service_get_route_metric' Grant Erickson
@ 2023-12-16  6:12 ` Grant Erickson
  2023-12-16  6:12 ` [PATCH 04/11] wispr: Add 'DBG' to '__connman_wispr_start' error path Grant Erickson
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Grant Erickson @ 2023-12-16  6:12 UTC (permalink / raw)
  To: connman

This includes both the network interface index and name in the 'DBG'
in 'wispr_portal_detect' to make it easier to identify on which
network interface the WISPr portal detection is being made.
---
 src/wispr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/wispr.c b/src/wispr.c
index 9cd1e7db501d..ba774a62dd48 100644
--- a/src/wispr.c
+++ b/src/wispr.c
@@ -1104,8 +1104,6 @@ static int wispr_portal_detect(struct connman_wispr_portal_context *wp_context,
 	if (!interface)
 		return -EINVAL;
 
-	DBG("interface %s", interface);
-
 	if_index = connman_inet_ifindex(interface);
 	if (if_index < 0) {
 		DBG("Could not get ifindex");
@@ -1113,6 +1111,8 @@ static int wispr_portal_detect(struct connman_wispr_portal_context *wp_context,
 		goto done;
 	}
 
+	DBG("index %d (%s)", if_index, interface);
+
 	nameservers = connman_service_get_nameservers(wp_context->service);
 	if (!nameservers) {
 		DBG("Could not get nameservers");
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 04/11] wispr: Add 'DBG' to '__connman_wispr_start' error path.
  2023-12-16  6:12 [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Grant Erickson
                   ` (2 preceding siblings ...)
  2023-12-16  6:12 ` [PATCH 03/11] wispr: Expand 'DBG' in 'wispr_portal_detect' Grant Erickson
@ 2023-12-16  6:12 ` Grant Erickson
  2023-12-16  6:12 ` [PATCH 05/11] wispr: Update 'DBG' in '__connman_wispr_start' Grant Erickson
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Grant Erickson @ 2023-12-16  6:12 UTC (permalink / raw)
  To: connman

This adds a 'DBG' to the '__connman_wispr_start' error path that
includes both the error and the WISPr portal context associated with
the error.
---
 src/wispr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/wispr.c b/src/wispr.c
index ba774a62dd48..19af1e7bede2 100644
--- a/src/wispr.c
+++ b/src/wispr.c
@@ -1338,6 +1338,8 @@ int __connman_wispr_start(struct connman_service *service,
 	return 0;
 
 free_wp:
+	DBG("err %d wp_context %p", err, wp_context);
+
 	wp_context->cb(wp_context->service, wp_context->type, false, err);
 
 	g_hash_table_remove(wispr_portal_hash, GINT_TO_POINTER(index));
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 05/11] wispr: Update 'DBG' in '__connman_wispr_start'.
  2023-12-16  6:12 [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Grant Erickson
                   ` (3 preceding siblings ...)
  2023-12-16  6:12 ` [PATCH 04/11] wispr: Add 'DBG' to '__connman_wispr_start' error path Grant Erickson
@ 2023-12-16  6:12 ` Grant Erickson
  2023-12-16  6:12 ` [PATCH 06/11] wispr: Refactor 'free_wispr_routes' Grant Erickson
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Grant Erickson @ 2023-12-16  6:12 UTC (permalink / raw)
  To: connman

This updates the initial 'DBG' in '__connman_wispr_start' to ensure
there is space after the connect timeout value and the "ms" unit
designation.
---
 src/wispr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/wispr.c b/src/wispr.c
index 19af1e7bede2..04229d045cab 100644
--- a/src/wispr.c
+++ b/src/wispr.c
@@ -1281,7 +1281,7 @@ int __connman_wispr_start(struct connman_service *service,
 	struct connman_wispr_portal *wispr_portal = NULL;
 	int index, err;
 
-	DBG("service %p (%s) type %d (%s) connect_timeout %ums callback %p",
+	DBG("service %p (%s) type %d (%s) connect_timeout %u ms callback %p",
 		service, connman_service_get_identifier(service),
 		type, __connman_ipconfig_type2string(type),
 		connect_timeout_ms, callback);
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 06/11] wispr: Refactor 'free_wispr_routes'.
  2023-12-16  6:12 [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Grant Erickson
                   ` (4 preceding siblings ...)
  2023-12-16  6:12 ` [PATCH 05/11] wispr: Update 'DBG' in '__connman_wispr_start' Grant Erickson
@ 2023-12-16  6:12 ` Grant Erickson
  2023-12-16  6:12 ` [PATCH 07/11] wispr: Simplify IPv4 vs. IPv6 host route management Grant Erickson
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Grant Erickson @ 2023-12-16  6:12 UTC (permalink / raw)
  To: connman

This refactors 'free_wispr_routes' into a second, helper function
'free_wispr_route' such that host route deallocation is separated from
container iteration.
---
 src/wispr.c | 45 ++++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/src/wispr.c b/src/wispr.c
index 04229d045cab..568b7f0fc8a7 100644
--- a/src/wispr.c
+++ b/src/wispr.c
@@ -135,30 +135,37 @@ static void connman_wispr_message_init(struct connman_wispr_message *msg)
 	msg->location_name = NULL;
 }
 
+static void free_wispr_route(
+		const struct connman_wispr_portal_context *wp_context,
+		struct wispr_route *route)
+{
+	DBG("free route to %s if %d type %d", route->address,
+			route->if_index, wp_context->type);
+
+	switch (wp_context->type) {
+	case CONNMAN_IPCONFIG_TYPE_IPV4:
+		connman_inet_del_host_route(route->if_index,
+				route->address);
+		break;
+	case CONNMAN_IPCONFIG_TYPE_IPV6:
+		connman_inet_del_ipv6_host_route(route->if_index,
+				route->address);
+		break;
+	case CONNMAN_IPCONFIG_TYPE_UNKNOWN:
+	case CONNMAN_IPCONFIG_TYPE_ALL:
+		break;
+	}
+
+	g_free(route->address);
+	route->address = NULL;
+}
+
 static void free_wispr_routes(struct connman_wispr_portal_context *wp_context)
 {
 	while (wp_context->route_list) {
 		struct wispr_route *route = wp_context->route_list->data;
 
-		DBG("free route to %s if %d type %d", route->address,
-				route->if_index, wp_context->type);
-
-		switch (wp_context->type) {
-		case CONNMAN_IPCONFIG_TYPE_IPV4:
-			connman_inet_del_host_route(route->if_index,
-					route->address);
-			break;
-		case CONNMAN_IPCONFIG_TYPE_IPV6:
-			connman_inet_del_ipv6_host_route(route->if_index,
-					route->address);
-			break;
-		case CONNMAN_IPCONFIG_TYPE_UNKNOWN:
-		case CONNMAN_IPCONFIG_TYPE_ALL:
-			break;
-		}
-
-		g_free(route->address);
-		route->address = NULL;
+		free_wispr_route(wp_context, route);
 
 		g_free(route);
 		wp_context->route_list->data = NULL;
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 07/11] wispr: Simplify IPv4 vs. IPv6 host route management.
  2023-12-16  6:12 [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Grant Erickson
                   ` (5 preceding siblings ...)
  2023-12-16  6:12 ` [PATCH 06/11] wispr: Refactor 'free_wispr_routes' Grant Erickson
@ 2023-12-16  6:12 ` Grant Erickson
  2023-12-16  6:12 ` [PATCH 08/11] wispr: Document 'free_wispr_route{,s}' Grant Erickson
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Grant Erickson @ 2023-12-16  6:12 UTC (permalink / raw)
  To: connman

This uses the recently-added, symmetric
'connman_inet_{add,del}_{,ipv6_}host_route_with_metric' functions and
route operations tables to simplify IPv4 vs. IPv6 host route
management for WISPr requests.
---
 src/wispr.c | 112 ++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 82 insertions(+), 30 deletions(-)

diff --git a/src/wispr.c b/src/wispr.c
index 568b7f0fc8a7..5ea573ca7769 100644
--- a/src/wispr.c
+++ b/src/wispr.c
@@ -53,12 +53,25 @@ enum connman_wispr_result {
 struct wispr_route {
 	char *address;
 	int if_index;
+	uint32_t metric;
+};
+
+struct wispr_portal_context_route_ops {
+	int (*add_host_route_with_metric)(int index,
+		const char *host,
+		const char *gateway,
+		uint32_t metric);
+	int (*del_host_route_with_metric)(int index,
+		const char *host,
+		const char *gateway,
+		uint32_t metric);
 };
 
 struct connman_wispr_portal_context {
 	int refcount;
 	struct connman_service *service;
 	enum connman_ipconfig_type type;
+	const struct wispr_portal_context_route_ops *ops;
 	__connman_wispr_cb_t cb;
 	struct connman_wispr_portal *wispr_portal;
 
@@ -103,6 +116,22 @@ static GHashTable *wispr_portal_hash = NULL;
 static char *online_check_ipv4_url = NULL;
 static char *online_check_ipv6_url = NULL;
 
+static const struct wispr_portal_context_route_ops
+	ipv4_wispr_portal_context_route_ops = {
+	.add_host_route_with_metric    =
+		connman_inet_add_host_route_with_metric,
+	.del_host_route_with_metric    =
+		connman_inet_del_host_route_with_metric
+};
+
+static const struct wispr_portal_context_route_ops
+	ipv6_wispr_portal_context_route_ops = {
+	.add_host_route_with_metric    =
+		connman_inet_add_ipv6_host_route_with_metric,
+	.del_host_route_with_metric    =
+		connman_inet_del_ipv6_host_route_with_metric
+};
+
 #define wispr_portal_context_ref(wp_context) \
 	wispr_portal_context_ref_debug(wp_context, __FILE__, __LINE__, __func__)
 #define wispr_portal_context_unref(wp_context) \
@@ -139,23 +168,39 @@ static void free_wispr_route(
 		const struct connman_wispr_portal_context *wp_context,
 		struct wispr_route *route)
 {
-	DBG("free route to %s if %d type %d", route->address,
-			route->if_index, wp_context->type);
+	g_autofree char *interface = NULL;
+	const char *gateway;
 
-	switch (wp_context->type) {
-	case CONNMAN_IPCONFIG_TYPE_IPV4:
-		connman_inet_del_host_route(route->if_index,
-				route->address);
-		break;
-	case CONNMAN_IPCONFIG_TYPE_IPV6:
-		connman_inet_del_ipv6_host_route(route->if_index,
-				route->address);
-		break;
-	case CONNMAN_IPCONFIG_TYPE_UNKNOWN:
-	case CONNMAN_IPCONFIG_TYPE_ALL:
-		break;
-	}
+	gateway = __connman_ipconfig_get_gateway_from_index(route->if_index,
+		wp_context->type);
+
+	/*
+	 * If gateway was null, as with wispr_route_request, there was no
+	 * host route created. Consequently, there is no host route to
+	 * delete. Simply free the address.
+	 */
+	if (!gateway)
+		goto free;
+
+	interface = connman_inet_ifname(route->if_index);
 
+	DBG("delete route to %s via %s dev %d (%s) metric %u type %d (%s) "
+		"ops %p",
+		route->address,
+		gateway,
+		route->if_index, interface,
+		route->metric,
+		wp_context->type,
+		__connman_ipconfig_type2string(wp_context->type),
+		wp_context->ops);
+
+	wp_context->ops->del_host_route_with_metric(
+				route->if_index,
+				route->address,
+				gateway,
+				route->metric);
+
+free:
 	g_free(route->address);
 	route->address = NULL;
 }
@@ -584,6 +629,7 @@ static bool wispr_route_request(const char *address, int ai_family,
 	struct connman_wispr_portal_context *wp_context = user_data;
 	const char *gateway;
 	struct wispr_route *route;
+	uint32_t metric = 0;
 
 	gateway = __connman_ipconfig_get_gateway_from_index(if_index,
 		wp_context->type);
@@ -599,20 +645,16 @@ static bool wispr_route_request(const char *address, int ai_family,
 		return false;
 	}
 
-	switch (wp_context->type) {
-	case CONNMAN_IPCONFIG_TYPE_IPV4:
-		result = connman_inet_add_host_route(if_index, address,
-				gateway);
-		break;
-	case CONNMAN_IPCONFIG_TYPE_IPV6:
-		result = connman_inet_add_ipv6_host_route(if_index, address,
-				gateway);
-		break;
-	case CONNMAN_IPCONFIG_TYPE_UNKNOWN:
-	case CONNMAN_IPCONFIG_TYPE_ALL:
-		break;
-	}
+	DBG("service %p (%s) metric %u",
+		wp_context->service,
+		connman_service_get_identifier(wp_context->service),
+		metric);
 
+	result = wp_context->ops->add_host_route_with_metric(
+					if_index,
+					address,
+					gateway,
+					metric);
 	if (result < 0) {
 		g_free(route);
 		return false;
@@ -620,6 +662,8 @@ static bool wispr_route_request(const char *address, int ai_family,
 
 	route->address = g_strdup(address);
 	route->if_index = if_index;
+	route->metric = metric;
+
 	wp_context->route_list = g_slist_prepend(wp_context->route_list, route);
 
 	return true;
@@ -1334,10 +1378,18 @@ int __connman_wispr_start(struct connman_service *service,
 	wp_context->cb = callback;
 	wp_context->wispr_portal = wispr_portal;
 
-	if (type == CONNMAN_IPCONFIG_TYPE_IPV4)
+	if (type == CONNMAN_IPCONFIG_TYPE_IPV4) {
 		wispr_portal->ipv4_context = wp_context;
-	else
+		wispr_portal->ipv4_context->ops =
+			&ipv4_wispr_portal_context_route_ops;
+	} else if (type == CONNMAN_IPCONFIG_TYPE_IPV6) {
 		wispr_portal->ipv6_context = wp_context;
+		wispr_portal->ipv6_context->ops =
+			&ipv6_wispr_portal_context_route_ops;
+	} else {
+		err = -EINVAL;
+		goto free_wp;
+	}
 
 	err = wispr_portal_detect(wp_context, connect_timeout_ms);
 	if (err)
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 08/11] wispr: Document 'free_wispr_route{,s}'.
  2023-12-16  6:12 [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Grant Erickson
                   ` (6 preceding siblings ...)
  2023-12-16  6:12 ` [PATCH 07/11] wispr: Simplify IPv4 vs. IPv6 host route management Grant Erickson
@ 2023-12-16  6:12 ` Grant Erickson
  2023-12-16  6:12 ` [PATCH 09/11] wispr: Document 'wispr_route' Grant Erickson
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Grant Erickson @ 2023-12-16  6:12 UTC (permalink / raw)
  To: connman

This adds documentation to the 'free_wispr_route{,s}' functions.
---
 src/wispr.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/src/wispr.c b/src/wispr.c
index 5ea573ca7769..581205c6662c 100644
--- a/src/wispr.c
+++ b/src/wispr.c
@@ -164,6 +164,27 @@ static void connman_wispr_message_init(struct connman_wispr_message *msg)
 	msg->location_name = NULL;
 }
 
+/**
+ *  @brief
+ *    Deallocate resources associated with the specified WISPr host
+ *    route.
+ *
+ *  This attempts to deallocate resources, including host routes,
+ *  associated with the specified WISPr host route belonging to the
+ *  specified WISPr portal context.
+ *
+ *  @param[in]      wp_context  A pointer to the immutable WISPr
+ *                              portal context associated with @a
+ *                              route.
+ *  @param[in,out]  route       A pointer to the mutable WISPr host
+ *                              route structure for which to delete an
+ *                              associated host route and to
+ *                              deallocate any dynamically-allocated
+ *                              resources associated with it.
+ *
+ *  @sa wispr_route_request
+ *
+ */
 static void free_wispr_route(
 		const struct connman_wispr_portal_context *wp_context,
 		struct wispr_route *route)
@@ -205,6 +226,19 @@ free:
 	route->address = NULL;
 }
 
+/**
+ *  @brief
+ *    Deallocate host route resources associated with the specified
+ *    WISPr portal context.
+ *
+ *  This attempts to deallocate host route resources associated with
+ *  the specified WISPr specified WISPr portal context.
+ *
+ *  @param[in]      wp_context  A pointer to the mutable WISPr
+ *                              portal context for which to deallocate
+ *                              resources associated with host routes.
+ *
+ */
 static void free_wispr_routes(struct connman_wispr_portal_context *wp_context)
 {
 	while (wp_context->route_list) {
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 09/11] wispr: Document 'wispr_route'.
  2023-12-16  6:12 [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Grant Erickson
                   ` (7 preceding siblings ...)
  2023-12-16  6:12 ` [PATCH 08/11] wispr: Document 'free_wispr_route{,s}' Grant Erickson
@ 2023-12-16  6:12 ` Grant Erickson
  2023-12-16  6:12 ` [PATCH 10/11] wispr: Document 'wispr_portal_context_route_ops' Grant Erickson
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Grant Erickson @ 2023-12-16  6:12 UTC (permalink / raw)
  To: connman

This adds documentation to the 'wispr_route' structure.
---
 src/wispr.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/src/wispr.c b/src/wispr.c
index 581205c6662c..317146af8cb8 100644
--- a/src/wispr.c
+++ b/src/wispr.c
@@ -50,9 +50,28 @@ enum connman_wispr_result {
 	CONNMAN_WISPR_RESULT_FAILED  = 3,
 };
 
+/**
+ *  State for host routes used for a WISPr request.
+ */
 struct wispr_route {
+	/**
+	 *	A pointer to a mutable, dynamically-allocated null-terminated
+	 *	C string containing the text-formatted address of the WISPr
+	 *	host.
+	 */
 	char *address;
+
+	/**
+	 *	The network interface index associated with the underlying
+	 *	network interface over which the WISPr request will sent and
+	 *	the reply received.
+	 */
 	int if_index;
+
+	/**
+	 *	The route metric/priority used for the host route created for
+	 *	the WISPr host.
+	 */
 	uint32_t metric;
 };
 
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 10/11] wispr: Document 'wispr_portal_context_route_ops'.
  2023-12-16  6:12 [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Grant Erickson
                   ` (8 preceding siblings ...)
  2023-12-16  6:12 ` [PATCH 09/11] wispr: Document 'wispr_route' Grant Erickson
@ 2023-12-16  6:12 ` Grant Erickson
  2023-12-16  6:12 ` [PATCH 11/11] wispr: Leverage '__connman_service_get_route_metric' Grant Erickson
  2023-12-16 18:46 ` [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Marcel Holtmann
  11 siblings, 0 replies; 13+ messages in thread
From: Grant Erickson @ 2023-12-16  6:12 UTC (permalink / raw)
  To: connman

This adds documentation to the 'wispr_portal_context_route_ops'
structure.
---
 src/wispr.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/wispr.c b/src/wispr.c
index 317146af8cb8..1af880118e14 100644
--- a/src/wispr.c
+++ b/src/wispr.c
@@ -75,6 +75,10 @@ struct wispr_route {
 	uint32_t metric;
 };
 
+/**
+ *  Gateway configuration function pointers for IP configuration
+ *  type-specific route set/clear/add/delete operations.
+ */
 struct wispr_portal_context_route_ops {
 	int (*add_host_route_with_metric)(int index,
 		const char *host,
@@ -90,7 +94,13 @@ struct connman_wispr_portal_context {
 	int refcount;
 	struct connman_service *service;
 	enum connman_ipconfig_type type;
+
+	/**
+	 *	A pointer to immutable function pointers for route
+	 *	set/clear/add/delete operations.
+	 */
 	const struct wispr_portal_context_route_ops *ops;
+
 	__connman_wispr_cb_t cb;
 	struct connman_wispr_portal *wispr_portal;
 
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 11/11] wispr: Leverage '__connman_service_get_route_metric'.
  2023-12-16  6:12 [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Grant Erickson
                   ` (9 preceding siblings ...)
  2023-12-16  6:12 ` [PATCH 10/11] wispr: Document 'wispr_portal_context_route_ops' Grant Erickson
@ 2023-12-16  6:12 ` Grant Erickson
  2023-12-16 18:46 ` [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Marcel Holtmann
  11 siblings, 0 replies; 13+ messages in thread
From: Grant Erickson @ 2023-12-16  6:12 UTC (permalink / raw)
  To: connman

This leverages '__connman_service_get_route_metric' to select an
appropriate service-specific route metric/priority when adding or
deleting WISPr host routes.

This allows multiple such routes to coexist simultaneously, supporting
"continuous" mode online checks in which one or more services may be
conducting "online" WISPr-based Internet reachability checks as they
move in and out of reachability success/failure.
---
 src/wispr.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/wispr.c b/src/wispr.c
index 1af880118e14..efc0ca2fbaa9 100644
--- a/src/wispr.c
+++ b/src/wispr.c
@@ -708,6 +708,17 @@ static bool wispr_route_request(const char *address, int ai_family,
 		return false;
 	}
 
+	result = __connman_service_get_route_metric(wp_context->service,
+				&metric);
+	if (result < 0) {
+		DBG("failed to get metric for service %p (%s): %s (%d)",
+			wp_context->service,
+			connman_service_get_identifier(wp_context->service),
+			strerror(-result), result);
+
+		return false;
+	}
+
 	DBG("service %p (%s) metric %u",
 		wp_context->service,
 		connman_service_get_identifier(wp_context->service),
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority
  2023-12-16  6:12 [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Grant Erickson
                   ` (10 preceding siblings ...)
  2023-12-16  6:12 ` [PATCH 11/11] wispr: Leverage '__connman_service_get_route_metric' Grant Erickson
@ 2023-12-16 18:46 ` Marcel Holtmann
  11 siblings, 0 replies; 13+ messages in thread
From: Marcel Holtmann @ 2023-12-16 18:46 UTC (permalink / raw)
  To: Grant Erickson; +Cc: connman

Hi Grant,

> This add and leverages '__connman_service_get_route_metric' to select
> an appropriate service-specific route metric/priority when adding or
> deleting WISPr host routes.
> 
> This allows multiple such routes to coexist simultaneously, supporting
> "continuous" mode online checks in which one or more services may be
> conducting "online" WISPr-based Internet reachability checks as they
> move in and out of reachability success/failure.
> 
> '__connman_service_get_route_metric' is a non-public service
> interface, which attempts to get the route metric/priority for the
> specified service based on the current service and services state.
> 
> If the service is the default or if it is the only service, then the
> metric is zero (0). Otherwise, a low-priority metric (metric > 0)
> unique to the service and its underlying network interface is computed
> and returned.
> 
> Grant Erickson (11):
>  service: Add '__connman_service_get_route_metric'.
>  service: Document '__connman_service_get_route_metric'.
>  wispr: Expand 'DBG' in 'wispr_portal_detect'.
>  wispr: Add 'DBG' to '__connman_wispr_start' error path.
>  wispr: Update 'DBG' in '__connman_wispr_start'.
>  wispr: Refactor 'free_wispr_routes'.
>  wispr: Simplify IPv4 vs. IPv6 host route management.
>  wispr: Document 'free_wispr_route{,s}'.
>  wispr: Document 'wispr_route'.
>  wispr: Document 'wispr_portal_context_route_ops'.
>  wispr: Leverage '__connman_service_get_route_metric'.
> 
> src/connman.h |   2 +
> src/service.c |  98 ++++++++++++++++++++++++
> src/wispr.c   | 207 +++++++++++++++++++++++++++++++++++++++++---------
> 3 files changed, 271 insertions(+), 36 deletions(-)

all 11 patches have been applied.

Regards

Marcel


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-12-16 18:47 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-16  6:12 [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Grant Erickson
2023-12-16  6:12 ` [PATCH 01/11] service: Add '__connman_service_get_route_metric' Grant Erickson
2023-12-16  6:12 ` [PATCH 02/11] service: Document '__connman_service_get_route_metric' Grant Erickson
2023-12-16  6:12 ` [PATCH 03/11] wispr: Expand 'DBG' in 'wispr_portal_detect' Grant Erickson
2023-12-16  6:12 ` [PATCH 04/11] wispr: Add 'DBG' to '__connman_wispr_start' error path Grant Erickson
2023-12-16  6:12 ` [PATCH 05/11] wispr: Update 'DBG' in '__connman_wispr_start' Grant Erickson
2023-12-16  6:12 ` [PATCH 06/11] wispr: Refactor 'free_wispr_routes' Grant Erickson
2023-12-16  6:12 ` [PATCH 07/11] wispr: Simplify IPv4 vs. IPv6 host route management Grant Erickson
2023-12-16  6:12 ` [PATCH 08/11] wispr: Document 'free_wispr_route{,s}' Grant Erickson
2023-12-16  6:12 ` [PATCH 09/11] wispr: Document 'wispr_route' Grant Erickson
2023-12-16  6:12 ` [PATCH 10/11] wispr: Document 'wispr_portal_context_route_ops' Grant Erickson
2023-12-16  6:12 ` [PATCH 11/11] wispr: Leverage '__connman_service_get_route_metric' Grant Erickson
2023-12-16 18:46 ` [PATCH 00/11] service/wispr: Add Per-service-unique WISPr Host Route Metric/Priority Marcel Holtmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).