interface-ip: overwrite resolv.conf file instead of renaming #62
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently, the change tracking of resolv.conf works as follows: 1. Create a temporary resolv.conf with current entries. 2. Calculate CRC32 of both the temporary and the existing (old) resolv.conf file. 3. If the CRC checksum differs, the temporary file is renamed to replace the old resolv.conf. However, this causes issues with ujail environments where the jail has bind-mounted the resolv.conf from the host. In this case, the new file would not be visible inside the jail until the jail was restarted. This is particularly troublesome for jailed ntpd deamon running on a device without a local DNS server, relying on a DNS info supplied from DHCP (e.g. dumb AP). Very often, the DHCP lease comes later than the ntpd is started, causing time synchronization to fail until the process is manually restarted. The proposed solution modifies the step 3 in the resolv.conf change tracking above: 3. If the CRC checksum differs, the existing resolv.conf is overwritten in place with the new (current) entries (ones from the temporary file). This approach, however, has a drawback - rename is atomic while overwrite is not. To mitigate this issue, a file locking mechanism is added (`flock`). Fixes: - openwrt/openwrt#10389 - openwrt/openwrt#10843 Signed-off-by: Dávid Benko <davidbenko@davidbenko.dev>
|
wont you get more flash wear is you overwrite, instead of rename? |
Author
|
Author
|
Kind ping @dangowrt, @nbd168 or @systemcrash for review. I backported this patch to OpenWrt 24.10.5, deployed it to multiple ramips and filogic APs and recorded no more issues with time synchronization ever since. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, the change tracking of resolv.conf works as follows:
However, this causes issues with ujail environments where the jail has bind-mounted the resolv.conf from the host. In this case, the new file would not be visible inside the jail until the jail was restarted.
This is particularly troublesome for jailed ntpd deamon running on a device without a local DNS server, relying on a DNS info supplied from DHCP (e.g. dumb AP). Very often, the DHCP lease comes later than the ntpd is started, causing time synchronization to fail until the process is manually restarted.
The proposed solution modifies the step 3 in the resolv.conf change tracking above:
This approach, however, has a drawback - rename is atomic while overwrite is not. To mitigate the issue, a file locking mechanism is added (
flock).The modified implementation includes more comments and restructured log messages. Major failures regarding writing to the resolv.conf file are now on the
WARNsyslog level.Tested on todays (2025-12-17) snapshot x86/64 build.
Fixes long standing issues in OpenWrt main repository:
As a side note, there are definitely alternative solutions/workarounds for the problem described in the issues above. The simplest one would be to disable ujail for ntpd altogether. This has some security implications, but should be OK for NTP client (and has been done as a workaround in other projects FreifunkFranken/firmware@f48e53c). What about server or client+server tho? Another possibility would be to watch for changes of
/etc/resolv.conf(or/tmp/resolv.conf.d/resolv.conf.autoreally) and restart the ujail when needed. This can be (probably) implemented in the ujail itself (inotify API?) or externally. Internal watchdog could be OK, but possibly a lot of work. External watchdog is not a clean solution, but it's exactly what some users are doing. Ujail could also mount the whole/etcfolder instead of/etc/resolv.conf, but this also has security implications (leaks of unrelated credentials). The list can go on; however, I consider overwriting resolv.conf instead of rename replacement as the most viable solution to the ntpd on dumb AP problem.