wait-online: add initial support for waiting for DNS #34640
Merged
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
|
I like the idea - do we need to bother with resolv.conf? For the other wait-online we only support networkd, and it's up to other implementations to provide a service that uses the wait-online target, we could just do the same here? |
yuwata
reviewed
Oct 7, 2024
There was a problem hiding this comment.
Please also check that a DNS server is accessible.
E.g. if DNS server 8.8.8.8 is configured, but the interface has only IPv4 link-local address, then we cannot access the server.
Also, please check if resolved already acquired the server. Even if networkd saves DNS servers to the state file, resolved may not read the file yet.
poettering
reviewed
Oct 7, 2024
poettering
reviewed
Oct 7, 2024
poettering
reviewed
Oct 7, 2024
poettering
reviewed
Oct 7, 2024
poettering
reviewed
Oct 7, 2024
poettering
reviewed
Oct 7, 2024
|
Hmm, the problem with this approach is that it is racy: afaics you are only subscribing to changes from networkd, but not resolved. Thus, it might happen that networkd gets a valid config, you then check resolved, but it hasn't processed the request yet, and you hence say "not online", and you'll never notice once resolved has caught up. I think the correct, race-free fix would be if we teach resolved's varlink interface a new call SubscribeDNS() which would be one of those methods that Varlink allows to have multiple replies. It's first reply would be the status qup, and then it would fire an event whenever the manager or per-link DNS server list changes (or search domain list). It would return a complete description of the DNS config, i.e. one object that contains subobjects for the global config and for each link config. wait-online could then issue that, and simply wait until it gets a non-empty reply. I think the patch for that wouldn't even be too involved at all. |
My understanding is that when resolved configures/modifies per-link DNS, this does end up recorded in networkd's link state, and hence an event is emitted. Testing locally seems to confirm this, too. On the other hand, @yuwata did mention above that just because networkd's link state records the DNS server, that does not mean resolved has picked it up yet (e.g. I guess DNS servers issued via DHCP would be reflected in the networkd state immediately).
That does sound like a more concrete approach this. And it's more generic/re-usable. I will work on something to that extent. Thanks for reviewing! |
Thanks for reviewing! Both of those points make sense, and I will address them in my next version. |
It's the other way round: networkd learns DNS from DHCP/IPv6LL and such like and then resolved picks it up from there. But this is asynchronous (it's actually implemented via inotify). resolved might take a bit until it see networkd's changes. |
Note that this is a really tough problem btw, because routing might make surprising stuff connectible, that the network configuration otherwise suggests wouldn't be allowed. i.e. you could route 8.8.8.8 or some subnet covering it to some ipv4ll host and it would work. It might be OK'ish as an approximation would be to allocate an UDP socket, then issue connect() towards the DNS server address. This forces a routing decision without actually sending any traffic. This should fail if there are no routes to that host, but succeed if there are. And that's the info you need. You never actually send any traffic over that socket, but immediately close it. |
poettering
added
resolve
reviewed/needs-rework 🔨
|
I pushed a new version with the following changes:
A couple things I am not sure of yet:
If we immediately check global DNS when a link does not its own DNS servers yet, and fallback DNS are configured, then we would usually just immediately use fallback to determine if a link is configured. That doesn't feel quite right, but I am not sure. |
yuwata
requested changes
Oct 9, 2024
enr0n
force-pushed
the
wait-online-dns
branch
from
c49f417 to
76b44c2
Compare
yuwata
requested changes
Oct 10, 2024
poettering
reviewed
Oct 15, 2024
poettering
reviewed
Oct 15, 2024
poettering
reviewed
Oct 15, 2024
poettering
reviewed
Oct 15, 2024
github-actions
bot
added
please-review
enr0n
force-pushed
the
wait-online-dns
branch
from
da4f07f to
cfa9712
Compare
|
@yuwata Besides the resolved serialization, I believe I addressed all other comments. |
yuwata
approved these changes
Jan 28, 2025
yuwata
added
good-to-merge/with-minor-suggestions
and removed
please-review
enr0n
force-pushed
the
wait-online-dns
branch
from
cfa9712 to
c3f760f
Compare
yuwata
added
good-to-merge/waiting-for-ci 👍
|
This looks related @enr0n can you double check? |
|
That backtrace is unsymbolized. The symbolized backtrace is Direct leak of 10 byte(s) in 2 object(s) allocated from:
#0 0x7f1a1fac2897 in malloc (/lib64/libasan.so.8+0xc2897) (BuildId: 0505b45e5a5d9a6c8ecb1d529aaaf13cd21fbe4e)
#1 0x7f1a1e6c8b1d in malloc_multiply ../src/src/basic/alloc-util.h:119
#2 0x7f1a1e6c9f95 in json_dispatch_byte_array_iovec ../src/src/libsystemd/sd-json/json-util.c:49
#3 0x7f1a1e707a9d in sd_json_dispatch_full ../src/src/libsystemd/sd-json/sd-json.c:5229
#4 0x7f1a1e70847b in sd_json_dispatch ../src/src/libsystemd/sd-json/sd-json.c:5301
#5 0x556be8c367b9 in dispatch_dns_server ../src/src/network/wait-online/dns-configuration.c:49
#6 0x556be8c36f26 in dispatch_dns_server_array ../src/src/network/wait-online/dns-configuration.c:72
#7 0x7f1a1e707a9d in sd_json_dispatch_full ../src/src/libsystemd/sd-json/sd-json.c:5229
#8 0x7f1a1e70847b in sd_json_dispatch ../src/src/libsystemd/sd-json/sd-json.c:5301
#9 0x556be8c3845c in dispatch_dns_configuration ../src/src/network/wait-online/dns-configuration.c:189
#10 0x556be8c386b6 in dns_configuration_from_json ../src/src/network/wait-online/dns-configuration.c:199
#11 0x556be8c45976 in on_dns_configuration_event ../src/src/network/wait-online/manager.c:440
#12 0x7f1a1e72525b in varlink_dispatch_reply ../src/src/libsystemd/sd-varlink/sd-varlink.c:1154
#13 0x7f1a1e728a00 in sd_varlink_process ../src/src/libsystemd/sd-varlink/sd-varlink.c:1455
#14 0x7f1a1e73fa67 in defer_callback ../src/src/libsystemd/sd-varlink/sd-varlink.c:2950
#15 0x7f1a1e52bccf in source_dispatch ../src/src/libsystemd/sd-event/sd-event.c:4241
#16 0x7f1a1e535121 in sd_event_dispatch ../src/src/libsystemd/sd-event/sd-event.c:4830
#17 0x7f1a1e53638e in sd_event_run ../src/src/libsystemd/sd-event/sd-event.c:4891
#18 0x7f1a1e5367b6 in sd_event_loop ../src/src/libsystemd/sd-event/sd-event.c:4912
#19 0x556be8c4af1e in run ../src/src/network/wait-online/wait-online.c:236
#20 0x556be8c4b3db in main ../src/src/network/wait-online/wait-online.c:248
#21 0x7f1a1d210247 in __libc_start_call_main (/lib64/libc.so.6+0x3247) (BuildId: 515c33a35f41020661fea8ac4eb995e26ccd6b00)
#22 0x7f1a1d21030a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x330a) (BuildId: 515c33a35f41020661fea8ac4eb995e26ccd6b00)
#23 0x556be8c35fd4 in _start (/usr/lib/systemd/systemd-networkd-wait-online+0x1fd4) (BuildId: a9ba5539dfad2ed308377349965131730fc74687)
Direct leak of 10 byte(s) in 2 object(s) allocated from:
#0 0x7f1a1fac2897 in malloc (/lib64/libasan.so.8+0xc2897) (BuildId: 0505b45e5a5d9a6c8ecb1d529aaaf13cd21fbe4e)
#1 0x7f1a1e6c8b1d in malloc_multiply ../src/src/basic/alloc-util.h:119
#2 0x7f1a1e6c9f95 in json_dispatch_byte_array_iovec ../src/src/libsystemd/sd-json/json-util.c:49
#3 0x7f1a1e707a9d in sd_json_dispatch_full ../src/src/libsystemd/sd-json/sd-json.c:5229
#4 0x7f1a1e70847b in sd_json_dispatch ../src/src/libsystemd/sd-json/sd-json.c:5301
#5 0x556be8c367b9 in dispatch_dns_server ../src/src/network/wait-online/dns-configuration.c:49
#6 0x7f1a1e707a9d in sd_json_dispatch_full ../src/src/libsystemd/sd-json/sd-json.c:5229
#7 0x7f1a1e70847b in sd_json_dispatch ../src/src/libsystemd/sd-json/sd-json.c:5301
#8 0x556be8c3845c in dispatch_dns_configuration ../src/src/network/wait-online/dns-configuration.c:189
#9 0x556be8c386b6 in dns_configuration_from_json ../src/src/network/wait-online/dns-configuration.c:199
#10 0x556be8c45976 in on_dns_configuration_event ../src/src/network/wait-online/manager.c:440
#11 0x7f1a1e72525b in varlink_dispatch_reply ../src/src/libsystemd/sd-varlink/sd-varlink.c:1154
#12 0x7f1a1e728a00 in sd_varlink_process ../src/src/libsystemd/sd-varlink/sd-varlink.c:1455
#13 0x7f1a1e73fa67 in defer_callback ../src/src/libsystemd/sd-varlink/sd-varlink.c:2950
#14 0x7f1a1e52bccf in source_dispatch ../src/src/libsystemd/sd-event/sd-event.c:4241
#15 0x7f1a1e535121 in sd_event_dispatch ../src/src/libsystemd/sd-event/sd-event.c:4830
#16 0x7f1a1e53638e in sd_event_run ../src/src/libsystemd/sd-event/sd-event.c:4891
#17 0x7f1a1e5367b6 in sd_event_loop ../src/src/libsystemd/sd-event/sd-event.c:4912
#18 0x556be8c4af1e in run ../src/src/network/wait-online/wait-online.c:236
#19 0x556be8c4b3db in main ../src/src/network/wait-online/wait-online.c:248
#20 0x7f1a1d210247 in __libc_start_call_main (/lib64/libc.so.6+0x3247) (BuildId: 515c33a35f41020661fea8ac4eb995e26ccd6b00)
#21 0x7f1a1d21030a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x330a) (BuildId: 515c33a35f41020661fea8ac4eb995e26ccd6b00)
#22 0x556be8c35fd4 in _start (/usr/lib/systemd/systemd-networkd-wait-online+0x1fd4) (BuildId: a9ba5539dfad2ed308377349965131730fc74687)
SUMMARY: AddressSanitizer: 20 byte(s) leaked in 4 allocation(s). |
yuwata
added
the
ci-fails/needs-rework 🔥
Add a new flag to systemd-networkd-wait-online, --dns, to allow waiting for DNS to be configured. DNS is considered configured when at least one DNS server is accessible. If a link has the property DefaultRoute=yes (either by explicit configuration, or because there are no routing-only domains), or if the search domain '.' is configured, wait for link-specific DNS to be configured. Otherwise, global DNS servers may be considered.
enr0n
force-pushed
the
wait-online-dns
branch
from
c3f760f to
d6608da
Compare
I was missing an |
12 tasks
github-actions
bot
removed
the
ci-fails/needs-rework 🔥
|
Okay, I believe the remaining failures are unrelated now. |
github-actions
bot
removed
the
good-to-merge/waiting-for-ci 👍
poettering
reviewed
Jan 29, 2025
| if (set_isempty(m->varlink_dns_configuration_subscription)) | ||
| manager_stop_dns_configuration_monitor(m); | ||
|
|
||
|
|
There was a problem hiding this comment.
spurious double empty line
slyon
added a commit
to slyon/netplan
that referenced
this pull request
Feb 3, 2025
As implemented in systemd (v258): systemd/systemd#34640
slyon
added a commit
to canonical/netplan
that referenced
this pull request
Mar 11, 2025
As implemented in systemd (v258): systemd/systemd#34640
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
/cc @yuwata
Opening this as a draft to get initial comments on the general approach. Is this an acceptable option to add to
systemd-networkd-wait-online? And, in the cases wheresystemd-resolvedis active, is checkingsd_network_link_get_dns()sufficient?Proposal:
Add a new flag,
--dns, to systemd-networkd-wait-online to allow waiting for DNS to be configured. The--dnsflag respects the--ipv4and--ipv6flags, as well as--interface=and--any.When
systemd-resolvedis running, we check for DNS by checking the link state file (i.e. by callingsd_network_link_get_dns()). Whensystemd-resolvedis not active, we fallback to parsing/etc/resolv.conf.