From: Marcel Holtmann <marcel@holtmann.org>
To: Grant Erickson <gerickson@nuovations.com>
Cc: connman@lists.linux.dev
Subject: Re: RFC: __connman_service_get_route_metric
Date: Thu, 14 Dec 2023 12:35:07 +0100 [thread overview]
Message-ID: <AE15E661-4815-49A0-A9B8-1B2BF41FC2A8@holtmann.org> (raw)
In-Reply-To: <129F38C5-2B36-40FB-A240-EE0542FCB5D7@nuovations.com>
Hi Grant,
> Was out on my morning ride this morning and had this revelation: https://github.com/Nuovations/connman/issues/6:
>
> Addressing https://github.com/Nuovations/connman/issues/5 will "paper over it"; however, fundamentally, there is the same high- vs. low-priority routing metric at play here for online check failover. For the online check / WiSPr, host routes for the IPv4 or IPv6 URL host are installed and removed.
>
> INSTALLED
>
> static bool wispr_route_request(...)
> {
> 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;
>
> […]
>
> return true;
> }
>
> REMOVED
>
> static void free_wispr_routes(...)
> {
> 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;
>
> […]
> }
>
> However, note that there is not a metric parameter in connman_inet_{add,del}{_ipv6,}_host_route. So, in a failover scenario, there may be host routes installed for one or more of Ethernet, Wi-Fi, and Cellular as each tries to recover online connectivity. However, since the metric for all is implicitly zero, one and only one network interface index will get the host route and all the others will silently fail (because connman_inet_{add,del}{_ipv6,}_host_route silently swallows -EEXIST).
>
> In the recent set of patches to https://github.com/Nuovations/connman/issues/5, I'd commented in compute_low_priority_metric:
>
> static uint32_t compute_low_priority_metric(...)
> {
> […]
>
> /*
> * 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 1) caching
> * the computed metric in the gateway data since services may
> * re-sort by the time we are asked to recompute high- and
> * low-priority routes and we need a stable and matching metric to
> * successfully delete a previously-created route and 2) having
> * access to an API (such as
> * '__connman_service_get_order(data->service)') that exposes 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.
> */
>
> return MAX(metric_ceiling, metric_base - (data->index * metric_index_step));
> }
>
> However, because this routing metric is needed across multiple modules (right now, both wispr.c and connection.c) the right approach given this revelation might be an API:
>
> __connman_service_get_route_metric(const struct connman_service *service);
>
> where the default service returns zero (0) and the rest return a metric reflective of their service order, between some ceiling (say, 2^20) and some floor (say, UINT32_MAX).
>
> This could then be applied easily to both gateway network and on-link network routes as well, making routing determinant based on service order, not kernel preference, for all routes.
>
> Thoughts on the proposed:
>
> __connman_service_get_route_metric(const struct connman_service *service);
>
> API?
I would say go for it and try it out. See if it works.
Regards
Marcel
prev parent reply other threads:[~2023-12-14 11:35 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-07 17:04 RFC: __connman_service_get_route_metric Grant Erickson
2023-12-14 11:35 ` Marcel Holtmann [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AE15E661-4815-49A0-A9B8-1B2BF41FC2A8@holtmann.org \
--to=marcel@holtmann.org \
--cc=connman@lists.linux.dev \
--cc=gerickson@nuovations.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).