Skip to content

CFE-4723: Fixed default route selection to pick the lowest-metric route - #6302

Open
djbclark wants to merge 1 commit into
cfengine:masterfrom
djbclark:fix/default-route-lowest-metric
Open

CFE-4723: Fixed default route selection to pick the lowest-metric route#6302
djbclark wants to merge 1 commit into
cfengine:masterfrom
djbclark:fix/default-route-lowest-metric

Conversation

@djbclark

@djbclark djbclark commented Aug 17, 2026

Copy link
Copy Markdown

GetNetworkingInfo() in libenv/unix_iface.c declared long lowest_metric = 0;
and never assigned it again, so the guard
JsonPrimitiveGetAsInteger(metric) < lowest_metric compared every later route
against 0 and could not be true for the non-negative metrics the kernel prints.
The first active default route in /proc/net/route always won. Present since
the loop was introduced (CFE-1991, 3.9.0).

The fix records the selected route's metric so a strictly lower one replaces it;
equal metrics keep the earlier entry, as before.

Scope

Smaller than it sounds. The kernel emits default routes in ascending metric
order, so on most hosts the first route is the lowest and the reported
sys.inet.default_route / sys.inet.default_gateway do not change. It bites
where a lower-metric default route appears after a higher-metric one.

What this does not fix

fib_priority is u32 but net/ipv4/fib_trie.c prints it with %d, so a
metric ≥ 2^31 renders with a leading -, which the [[:xdigit:]]+ capture
cannot match. Those route lines are dropped before selection ever runs.
Pre-existing, orthogonal to this change, and deliberately not addressed here.

(Relatedly, metric and irtt are decimal fields captured as [[:xdigit:]]+
while refcnt/use/mtu/window use \d+. Over-permissive rather than
wrong — decimal is a subset of xdigit — so it is left alone.)

Tests

The selection loop moved into a static FindLowestMetricDefaultRoute() so
tests/unit/unix_iface_test.c can drive it with constructed route data, via the
#include <unix_iface.c> pattern sysinfo_test already uses, guarded by
if !NT. 7 cases.

Discrimination: deleting only the line lowest_metric = metric_value; builds
clean and fails exactly test_lowest_metric_last
("192.168.0.1" != "192.168.0.3" — first route winning instead of lowest).
test_lowest_metric_first is a deliberate control and passes against the
unfixed code too. The file restores byte-identical (sha256 4e6bd587…9e843)
and all 7 pass again.

This is a Linux /proc/net/route path and it was developed on macOS 26.6.1
arm64.
The unit tests feed constructed route data, so they run anywhere; there
is no end-to-end test crossing a real /proc/net/route. Worth a Linux eye.

Cut from master 17eb78e6d. Tracked as
CFE-4723.

GetNetworkingInfo() declared `long lowest_metric = 0;` and never
assigned it again, so once any candidate had been chosen the guard
`JsonPrimitiveGetAsInteger(metric) < lowest_metric` compared each later
route's metric against 0 and could never be true for the non-negative
metrics the kernel prints. The first active default route in
/proc/net/route therefore always won, regardless of metric, ever since
the loop was introduced (CFE-1991, first released in 3.9.0).

The metric of the selected route is now recorded, so a later route with
a strictly lower metric replaces the earlier choice; equal metrics keep
the earlier entry, as before. On hosts with several active default
routes where a lower-metric route appears after a higher-metric one,
this changes the reported sys.inet.default_route and
sys.inet.default_gateway to the lower-metric route.

The selection loop moved into FindLowestMetricDefaultRoute() so that
the new tests/unit/unix_iface_test.c can drive it with constructed
route data; its lowest-metric-last case fails against the previous
logic.

Ticket: CFE-4723
Changelog: Title
@cf-bottom

Copy link
Copy Markdown

Thanks for submitting a pull request! Maybe @larsewi can review this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants