keep acquisition timestamp when route isn't updated

[Freax13]

RFC 4861 specifies that router advertisements should be sent no more than once every MIN_DELAY_BETWEEN_RAS (3 seconds). In practice though, there are several ISPs (Comcast, Spectrum, Vodafone) that send out router advertisements at higher rates.

Previously this caused dhcpcd to never update the route until it eventually expired and was removed by the kernel.

This happened because the rt_acquired timestamp was out of sync with the kernel expiry tracking. rt_acquired was updated every time a router advertisement was processed even when the route wasn't updated in the kernel. When router advertisements were sent out frequently, rt_acquired was updated frequently, the difference between the old and new rt_acquired values never became great enough for rt_cmp_lifetime to return 1 and the kernel's routing table was never updated.

To fix this, keep the old rt_acquired value iff the kernel's routing table wasn't updated. This ensures that rt_acquired stays in sync with the expiry value in the kernel's routing table.

Closes #681
Cc @squarooticus

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 783063ad-402a-4f5d-a3a0-ec14e54d6715

📥 Commits

Reviewing files that changed from the base of the PR and between b27efb1 and c59e940.

📒 Files selected for processing (1)

• .github/workflows/build.yml

💤 Files with no reviewable changes (1)

• .github/workflows/build.yml

---

Walkthrough

The route acquisition timestamp field is renamed from rt_aquired to rt_acquired. Route lifetime calculations, replacement, Linux expiration handling, and IPv6 route construction use the corrected field. The Ubuntu CI job refreshes apt indexes before dependency installation.

Changes

Route lifetime handling

Layer / File(s)	Summary
Route timestamp field src/route.h	Renames the acquisition timestamp field to rt_acquired and corrects its comment.
Route lifetime preservation src/route.c, src/if-linux.c	Updates lifetime comparisons, preserves the timestamp during unchanged route replacement, and uses the corrected field for expiration.
IPv6 route timestamp sources src/ipv6.c	Uses rt_acquired when constructing RA prefix, subnet, default, and DHCP routes.

CI package setup

Layer / File(s)	Summary
Ubuntu package refresh .github/workflows/build.yml	Refreshes package indexes before installing libudev-dev.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• NetworkConfiguration/dhcpcd#621: Both PRs modify route handling in struct rt and related route-processing files.
• NetworkConfiguration/dhcpcd#654: Both PRs modify the Ubuntu workflow dependency-installation step.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The workflow change that refreshes apt indexes is unrelated to the route renewal fix.	Move the apt index refresh to a separate pull request unless the build dependency issue is an explicit requirement.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the primary route timestamp preservation change.
Description check	✅ Passed	The description explains the router advertisement issue and the route timestamp fix addressed by the changes.
Linked Issues check	✅ Passed	The route timestamp changes address issue #681 by keeping route expiry tracking synchronized during frequent router advertisements.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[squarooticus]

It's very hard to follow the logic of how this code is supposed to work. It looks like:

• When rt_doroute() returns true, rt_build() replaces the original route (i.e., the one in dhcpcd's internal tables) with the one it acquired from the network (e.g., via DHCP or RA)
• rt_doroute() returns true when it indicates either (a) some change in the route (makes sense) and is successful in making that change or (b) no change (???).

I'm struggling to figure out why (b). The reason I'm trying to puzzle this out is that I'm not sure that modifying the newly-acquired route to match the original one is the right solution. I think it'll work, but AFAICT it's the only place where the newly-acquired route is synthetically modified, which feels like the wrong solution here.

It seems like the right class of solution here is to leave the original route in place when doroute thinks nothing has changed, whereupon eventually the acquisition time difference will exceed DEV_MAX, and the route will be replaced, both in the internal tables and the kernel. But to be confident about this, I first need to understand why (b) above. Can any of the core devs for dhcpcd explain that logic?

[rsmarples]

It's very hard to follow the logic of how this code is supposed to work. It looks like:

• When rt_doroute() returns true, rt_build() replaces the original route (i.e., the one in dhcpcd's internal tables) with the one it acquired from the network (e.g., via DHCP or RA)
• rt_doroute() returns true when it indicates either (a) some change in the route (makes sense) and is successful in making that change or (b) no change (???).

I'm struggling to figure out why (b).

We return true on no change so we can update our internal tables.
This is for the case when routes already exist and dhcpcd is just starting up.
We really don't want to touch them if we don't need to. If we do some OS might flush the ARP table and cause connectivity issues with in-use network applications.

The reason I'm trying to puzzle this out is that I'm not sure that modifying the newly-acquired route to match the original one is the right solution. I think it'll work, but AFAICT it's the only place where the newly-acquired route is synthetically modified, which feels like the wrong solution here.

It seems like the right class of solution here is to leave the original route in place when doroute thinks nothing has changed, whereupon eventually the acquisition time difference will exceed DEV_MAX, and the route will be replaced, both in the internal tables and the kernel. But to be confident about this, I first need to understand why (b) above. Can any of the core devs for dhcpcd explain that logic?

I think the change makes sense and I've added a supporting comment for it.
@squarooticus does this help?

[squarooticus]

It's your code. 🤷‍♂️ I'm mainly worried about adding another layer of complexity to the onion, in a place where there have been subtle logic problems in the past. But I do agree that this PR will likely fix the immediate issue.

[squarooticus]

I guess I can outline how I would manage routes if I were writing this from scratch. This is just me opining. Feel free to ignore. ;-)

Essentially, whenever receiving an RA, for the default route:

1. Compare the default route info to our internal state.

   • If there was no change, check the last time I updated this route in the kernel and if it exceeds some configurable interval, atomically replace/add the route (mostly to reset the lifetime) and reset the last-update time to now(). (This is to avoid replacing the route after literally every RA on networks that send them every 1-2 seconds. Maybe this isn't necessary, and it's cheap enough to do every time, but it feels like something worthy of a hack, especially if the expiration time is many multiples of the Adv interval, as it is on most networks.)

   • If there was a change (or we just started up and had no previous internal state), atomically replace/add the route with that from the RA, update the internal state to match the RA, and reset the last-update time to now().

2. Clean up any vestigial routes we added that might have been left over, e.g., because of a metric change in the config.

   • This is a little complicated because for proper compartmentalization you don't necessarily want to duplicate all of the kernel's logic for the key fields in a route that it uses to determine replace vs. add. You could instead define two protos and alternate between them for each update, flushing rules from the other proto after doing your atomic replaces. Having a custom proto also makes it easy to clean up all the routes you added on termination.

You can do the same basic thing for RIOs.

PIOs for SLAAC networks are a little more complicated, but the same general idea applies (don't replace too often on networks that flood RAs).

[rsmarples]

I guess I can outline how I would manage routes if I were writing this from scratch. This is just me opining. Feel free to ignore. ;-)

Essentially, whenever receiving an RA, for the default route:

1. Compare the default route info to our internal state.

   • If there was no change, check the last time I updated this route in the kernel and if it exceeds some configurable interval, atomically replace/add the route (mostly to reset the lifetime) and reset the last-update time to now(). (This is to avoid replacing the route after literally every RA on networks that send them every 1-2 seconds. Maybe this isn't necessary, and it's cheap enough to do every time, but it feels like something worthy of a hack, especially if the expiration time is many multiples of the Adv interval, as it is on most networks.)

This is essentially what we do now.

• If there was a change (or we just started up and had no previous internal state), atomically replace/add the route with that from the RA, update the internal state to match the RA, and reset the last-update time to now().

We actively want to avoid this on BSD as any route change will flush the internal ARP tables.

2. Clean up any vestigial routes we added that might have been left over, e.g., because of a metric change in the config.

   • This is a little complicated because for proper compartmentalization you don't necessarily want to duplicate all of the kernel's logic for the key fields in a route that it uses to determine replace vs. add. You could instead define two protos and alternate between them for each update, flushing rules from the other proto after doing your atomic replaces. Having a custom proto also makes it easy to clean up all the routes you added on termination.

Yes a metric change is complicated and is somewhat supported in dhcpcd.
Although I haven't changed it works still for a number of years.

You can do the same basic thing for RIOs.

PIOs for SLAAC networks are a little more complicated, but the same general idea applies (don't replace too often on networks that flood RAs).

For dhcpcd it's just a route. The route code is agnostic to its source and works for IPv4 and IPv6.

Thanks for your comments :)

[squarooticus]

• If there was a change (or we just started up and had no previous internal state), atomically replace/add the route with that from the RA, update the internal state to match the RA, and reset the last-update time to now().

We actively want to avoid this on BSD as any route change will flush the internal ARP tables.

It flushes the entire ARP table/ND table for all interfaces whenever any route changes? Wow.

You can do the same basic thing for RIOs.
PIOs for SLAAC networks are a little more complicated, but the same general idea applies (don't replace too often on networks that flood RAs).

For dhcpcd it's just a route. The route code is agnostic to its source and works for IPv4 and IPv6.

dhcpcd does set accept_ra=0 on Linux on startup, disabling kernel autoconfiguration, so it must be handling SLAAC itself for PIOs marked Autonomous. (I know it works because my Starlink-facing interface has accept_ra=0 and that interface still manages to get an address, and also the logs say it's doing that. This is with 10.1.0, at least, so maybe that behavior changed. I decided to downgrade from 10.3.2, but even though I have an exit hook workaround I am not foolish enough to attempt that upgrade from several hundred miles away.)

Anyway, thanks for the quick reply.

[coderabbitai]

🧹 Nitpick comments (1)

.github/workflows/build.yml (1)

54-55: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoid upgrading the entire CI runner.

apt upgrade -y changes every preinstalled package on both matrix jobs. This increases build time and reduces reproducibility. Refreshing the indexes before installing libudev-dev is sufficient.

Proposed fix

-    - name: Upgrade packages
-      run: sudo apt upgrade -y

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/build.yml around lines 54 - 55, Remove the broad package
upgrade step from the workflow and update the dependency-installation flow to
refresh apt indexes before installing libudev-dev. Keep the existing matrix jobs
and install only the required package to preserve CI reproducibility.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @.github/workflows/build.yml:
- Around line 54-55: Remove the broad package upgrade step from the workflow and
update the dependency-installation flow to refresh apt indexes before installing
libudev-dev. Keep the existing matrix jobs and install only the required package
to preserve CI reproducibility.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 695133c7-44e9-4dd4-9799-91373532fba7

📥 Commits

Reviewing files that changed from the base of the PR and between d7bdc8d and b27efb1.

📒 Files selected for processing (1)

• .github/workflows/build.yml

[rsmarples]

• If there was a change (or we just started up and had no previous internal state), atomically replace/add the route with that from the RA, update the internal state to match the RA, and reset the last-update time to now().

We actively want to avoid this on BSD as any route change will flush the internal ARP tables.

It flushes the entire ARP table/ND table for all interfaces whenever any route changes? Wow.

Only for entries attached to the route.
This makes sense when you think about it.

To get the same behaviour (which is important for multi-homed hosts) I need to delete and re-add the route.

You can do the same basic thing for RIOs.
PIOs for SLAAC networks are a little more complicated, but the same general idea applies (don't replace too often on networks that flood RAs).

For dhcpcd it's just a route. The route code is agnostic to its source and works for IPv4 and IPv6.

dhcpcd does set accept_ra=0 on Linux on startup, disabling kernel autoconfiguration, so it must be handling SLAAC itself for PIOs marked Autonomous. (I know it works because my Starlink-facing interface has accept_ra=0 and that interface still manages to get an address, and also the logs say it's doing that. This is with 10.1.0, at least, so maybe that behavior changed. I decided to downgrade from 10.3.2, but even though I have an exit hook workaround I am not foolish enough to attempt that upgrade from several hundred miles away.)

Anyway, thanks for the quick reply.

Ah let me rephrase. dhcpcd routing takes routes from many sources - RA PIO is just one, it could also be RA Route Information Option (RIO), or any number of DHCP options such as Classless Static Routing. The routing logic is agnostic to all of this.