Skip to content

xapi/nm: Send non-empty dns to networkd when using IPv6 autoconf #6586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 20, 2025
+43 −41
Merged

xapi/nm: Send non-empty dns to networkd when using IPv6 autoconf #6586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
edb0d81
xapi/nm: Send non-empty dns to networkd when using IPv6 autoconf
psafont Jul 11, 2025
bdbd975
xapi-idl/network: Remove code duplication for DNS persistence decisions
psafont Jul 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions ocaml/networkd/bin/network_server.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,8 @@ module Interface = struct
let set_dns _ dbg ~name ~nameservers ~domains =
Debug.with_thread_associated dbg
(fun () ->
update_config name {(get_config name) with dns= (nameservers, domains)} ;
update_config name
{(get_config name) with dns= Some (nameservers, domains)} ;
debug "Configuring DNS for %s: nameservers: [%s]; domains: [%s]" name
(String.concat ", " (List.map Unix.string_of_inet_addr nameservers))
(String.concat ", " domains) ;
Expand Down Expand Up @@ -727,7 +728,7 @@ module Interface = struct
; ipv6_conf
; ipv6_gateway
; ipv4_routes
; dns= nameservers, domains
; dns
; mtu
; ethtool_settings
; ethtool_offload
Expand All @@ -736,15 +737,10 @@ module Interface = struct
) ) ->
update_config name c ;
exec (fun () ->
(* We only apply the DNS settings when not in a DHCP mode
to avoid conflicts. The `dns` field
should really be an option type so that we don't have to
derive the intention of the caller by looking at other
fields. *)
match (ipv4_conf, ipv6_conf) with
| Static4 _, _ | _, Static6 _ | _, Autoconf6 ->
match dns with
| Some (nameservers, domains) ->
set_dns () dbg ~name ~nameservers ~domains
| _ ->
| None ->
()
) ;
exec (fun () -> set_ipv4_conf dbg name ipv4_conf) ;
Expand Down
29 changes: 17 additions & 12 deletions ocaml/networkd/bin_db/networkd_db.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,20 +74,25 @@ let _ =
[("gateway", Unix.string_of_inet_addr addr)]
in
let dns =
let dns' =
List.map Unix.string_of_inet_addr (fst interface_config.dns)
in
if dns' = [] then
[]
else
[("dns", String.concat "," dns')]
interface_config.dns
|> Option.map fst
|> Option.map (List.map Unix.string_of_inet_addr)
|> Option.fold ~none:[] ~some:(function
| [] ->
[]
| dns' ->
[("dns", String.concat "," dns')]
)
in
let domains =
let domains' = snd interface_config.dns in
if domains' = [] then
[]
else
[("domain", String.concat "," domains')]
interface_config.dns
|> Option.map snd
|> Option.fold ~none:[] ~some:(function
| [] ->
[]
| domains' ->
[("domain", String.concat "," domains')]
)
in
mode @ addrs @ gateway @ dns @ domains
| None4 ->
Expand Down
15 changes: 8 additions & 7 deletions ocaml/networkd/lib/network_config.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ let bridge_naming_convention (device : string) =
let get_list_from ~sep ~key args =
List.assoc_opt key args
|> Option.map (fun v -> Astring.String.cuts ~empty:false ~sep v)
|> Option.value ~default:[]

let parse_ipv4_config args = function
| Some "static" ->
Expand Down Expand Up @@ -73,11 +72,13 @@ let parse_ipv6_config args = function
(None6, None)

let parse_dns_config args =
let nameservers =
get_list_from ~sep:"," ~key:"DNS" args |> List.map Unix.inet_addr_of_string
let ( let* ) = Option.bind in
let* nameservers =
get_list_from ~sep:"," ~key:"DNS" args
|> Option.map (List.map Unix.inet_addr_of_string)
in
let domains = get_list_from ~sep:" " ~key:"DOMAIN" args in
(nameservers, domains)
let* domains = get_list_from ~sep:" " ~key:"DOMAIN" args in
Some (nameservers, domains)

let read_management_conf () =
try
Expand All @@ -103,15 +104,15 @@ let read_management_conf () =
let device =
(* Take 1st member of bond *)
match (bond_mode, bond_members) with
| None, _ | _, [] -> (
| None, _ | _, (None | Some []) -> (
match List.assoc_opt "LABEL" args with
| Some x ->
x
| None ->
error "%s: missing LABEL in %s" __FUNCTION__ management_conf ;
raise Read_error
)
| _, hd :: _ ->
| _, Some (hd :: _) ->
hd
in
Inventory.reread_inventory () ;
Expand Down
7 changes: 5 additions & 2 deletions ocaml/xapi-idl/network/network_interface.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,10 @@ type interface_config_t = {
; ipv6_conf: ipv6 [@default None6]
; 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 •
edited
Loading

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 •
edited
Loading

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!

(** the list
of nameservers and domains to persist in /etc/resolv.conf. Must be None when
using a DHCP mode *)
; mtu: int [@default 1500]
; ethtool_settings: (string * string) list [@default []]
; ethtool_offload: (string * string) list [@default [("lro", "off")]]
Expand Down Expand Up @@ -200,7 +203,7 @@ let default_interface =
; ipv6_conf= None6
; ipv6_gateway= None
; ipv4_routes= []
; dns= ([], [])
; dns= None
; mtu= 1500
; ethtool_settings= []
; ethtool_offload= [("lro", "off")]
Expand Down
17 changes: 7 additions & 10 deletions ocaml/xapi/nm.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,28 +634,25 @@ let bring_pif_up ~__context ?(management_interface = false) (pif : API.ref_PIF)
rc.API.pIF_ip_configuration_mode = `Static
| `IPv6 ->
rc.API.pIF_ipv6_configuration_mode = `Static
|| rc.API.pIF_ipv6_configuration_mode = `Autoconf
in
let dns =
match (static, rc.API.pIF_DNS) with
| false, _ | true, "" ->
([], [])
None
| true, pif_dns ->
let nameservers =
List.map Unix.inet_addr_of_string
(String.split ',' pif_dns)
(String.split_on_char ',' pif_dns)
in
let domains =
match List.assoc_opt "domain" rc.API.pIF_other_config with
| None ->
| None | Some "" ->
[]
| Some domains -> (
try String.split ',' domains
with _ ->
warn "Invalid DNS search domains: %s" domains ;
[]
)
| Some domains ->
String.split_on_char ',' domains
in
(nameservers, domains)
Some (nameservers, domains)
in
let mtu = determine_mtu rc net_rc in
let ethtool_settings, ethtool_offload =
Expand Down
Loading