Skip to content

Conversation

psafont
Copy link
Member

@psafont psafont commented Jul 11, 2025

Because Autoconf is not DHCP, networkd uses the dns value to write to
resolv.conf. This is done on ocaml/networkd/bin/network_server.ml line 745
This allows to have non-empty resolv.conf when using IPv6 autoconf.

This mismatch was caused byt technical debt that cause this check to be duplicated, the second removes it so
xapi decides the instances where these values are written and networkd follows that decision.

I've tested new isntallations using IPv4 and couldn't see any regression, two users have tested the patch and now the DNS entries don't get overriden when using IPv6 in Autoconf mode: xcp-ng/xcp#641 (comment)

I'd like Rob to review this approach in any case

psafont added 2 commits July 11, 2025 11:28
Because Autoconf is not DHCP, networkd uses the dns value to write to
resolv.conf. This is done on ocaml/networkd/bin/network_server.ml line 745

This allows to have non-empty resolv.conf when using IPv6 autoconf.

Signed-off-by: Pau Ruiz Safont <[email protected]>
Previously both xapi and networkd had to inspect the IP configuration to decide
whether the DNS values should be persistend into /etc/resolv.conf. This
actually lead to a mismatch in them. Instead use an option value for DNS that
simply means that if there's a value, it must be persisted.

Now xapi decides the instances where these values are written.

Signed-off-by: Pau Ruiz Safont <[email protected]>
@psafont psafont requested a review from robhoes July 11, 2025 11:12
psafont added a commit to psafont/xapi that referenced this pull request Jul 11, 2025
psafont added a commit to psafont/xapi that referenced this pull request Jul 11, 2025
psafont added a commit to psafont/xapi that referenced this pull request Jul 11, 2025
@psafont psafont marked this pull request as ready for review July 21, 2025 08:40
@minglumlu
Copy link
Member

From the discussion, especially xcp-ng/xcp#641 (comment)
I understand we still need to handle a case of SLAAC + RDNSS. The networkd will need to capture the DNS server addresses from the Router Advertisements messages and write them into /etc/resolv.conf file.

@psafont
Copy link
Member Author

psafont commented Jul 30, 2025

I understand we still need to handle a case of SLAAC + RDNSS. The networkd will need to capture the DNS server addresses from the Router Advertisements messages and write them into /etc/resolv.conf file.

Yes, this patch fixes the case where DNS is configured using IPv4 mechanisms, but not written to /etc/resolv.conf because IPv6 is configured in Autoconf mode.

I'm working on a design to make configuration of IP and DNS independent from each other under IPv6 mode, but that will take longer to get done. In any case RDNSS needs to be implemented independently

@minglumlu
Copy link
Member

I'd like Rob to review this approach in any case

Rob is in vacation. He will be back in 2 weeks.

but not written to /etc/resolv.conf because IPv6 is configured in Autoconf mode

Without these DNS server addresses, would the applications not be able to find DNS server to resolve their DNS name? XS doesn't have resolved running in dom0? Or I'm missing something. Anyway this could be resolved in your design.

@psafont
Copy link
Member Author

psafont commented Aug 1, 2025

Without these DNS server addresses, would the applications not be able to find DNS server to resolve their DNS name? XS doesn't have resolved running in dom0? Or I'm missing something. Anyway this could be resolved in your design.

The glibc function gethostbynameuses the resolv.conf file to know what IP to send the DNS requests to. So having the empty file means that some processes are unsable to send DNS requests if the file is empty.

This is an independent matter from the new design, the new design is meant to configure the IP addresses (yes, plural) of each PIF independently from its DNS configuration.

@0lini
Copy link

0lini commented Aug 1, 2025

An example for something that doesn't work when DNS is empty, is yum update.

; ipv6_gateway: Unix.inet_addr option [@default None]
; ipv4_routes: ipv4_route_t list [@default []]
; dns: Unix.inet_addr list * string list [@default [], []]
; dns: (Unix.inet_addr list * string list) option [@default None]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we assume that changing to an option type here does not change the expected JSON format? That is, existing dns fields in networkd.db (written before applying this patch) will be parsed correctly?

Copy link
Member Author

@psafont psafont Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will check, interestingly I didn't spot any issue, nor users reported it.

Copy link
Member Author

@psafont psafont Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified the network_db executable to parse a custom networkdb file, on top of this PR, and extracted the networkd.db from a couple hosts to test this. I couldn't find any issue parsing the file:

$ dune exec ocaml/networkd/bin_db/networkd_db.exe -- -db networkd.old_dhcp.db -bridge xenbr1 -iface xenbr1
interfaces=eth1
mode=dhcp
$ dune exec ocaml/networkd/bin_db/networkd_db.exe -- -db networkd.old_static.db -bridge xenbr0 -iface xenbr0
interfaces=eth0
mode=static
ipaddr=192.168.178.14
netmask=255.255.255.0
gateway=192.168.178.1
dns=192.168.178.64

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks for checking!

@psafont psafont added this pull request to the merge queue Aug 20, 2025
Merged via the queue into xapi-project:master with commit 05e6317 Aug 20, 2025
16 checks passed
@psafont psafont deleted the dev/pau/host-ipv6 branch August 20, 2025 10:07
GabrielBuica added a commit to GabrielBuica/xen-api that referenced this pull request Aug 29, 2025
@robhoes
Copy link
Member

robhoes commented Sep 1, 2025

Unfortunately, this introduced a problem on update when the mode is DHCP. In that case, the old db file has

  "dns": [
    [],
    []
  ],

which the updated xcp-networkd reads a Some []. And because xcp-networkd no longer conditionally applies the dns setting based it on the mode, it clears resolv.conf even though the mode is DHCP. And it does that after starting dhclient, removing any DNS config set through DHCP.

@psafont
Copy link
Member Author

psafont commented Sep 1, 2025

Ah, that's unfortunate. we can revert the second commit, Can Gaby open the PR? I'll be back to work next week

GabrielBuica added a commit to GabrielBuica/xen-api that referenced this pull request Sep 1, 2025
GabrielBuica added a commit to GabrielBuica/xen-api that referenced this pull request Sep 1, 2025
…onf (xapi-project#6586)"

This reverts commit 05e6317, reversing
changes made to 1fbdaae.

Signed-off-by: Gabriel Buica <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Sep 1, 2025
This reverts @psafont's PR, #6586, this causing the `/etc/resolve.conf`
to be overwritten during update, after the dhclient writes it. This
happens because `set_dns` no longer checks the mode before applying the
DNS config,

This reverts commit 05e6317, reversing
changes made to 1fbdaae.
psafont added a commit to xcp-ng-rpms/xapi that referenced this pull request Sep 10, 2025
psafont added a commit to xcp-ng-rpms/xapi that referenced this pull request Sep 11, 2025
github-merge-queue bot pushed a commit that referenced this pull request Sep 16, 2025
Because Autoconf is not DHCP, networkd uses the dns value to write to
resolv.conf. This is done on ocaml/networkd/bin/network_server.ml line
745

This allows to have non-empty resolv.conf when using IPv6 autoconf.

xapi-idl/network: Remove code duplication for DNS persistence decisions:

Previously both xapi and networkd had to inspect the IP configuration to
decide
whether the DNS values should be persistend into /etc/resolv.conf. This
actually lead to a mismatch in them. Instead use an option value for DNS
that
simply means that if there's a value, it must be persisted.

Now xapi decides the instances where these values are written.

Treat a couple of empty lists as a lack of value to avoid writing empty
resolv.conf files. This can happen when updating a host from previous
versions, which use empty lists when using DHCP.

Tested manually by installing a version with this change and restarting
the toolstack. The file is kept intact, unlike the previous version of
the change that did not take into account the update behaviour.

This is PR fixed version of #6586
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants