From 13d60758bcae643e794d925a8d67aca2c9b160cb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 11 May 2022 08:23:14 +0200 Subject: [PATCH 1/3] dhcp: always explicitly set request/information-request flags for internal DHCPv6 client It seems clearer to explicitly set this always, and not rely on the defaults. --- src/core/dhcp/nm-dhcp-systemd.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-systemd.c b/src/core/dhcp/nm-dhcp-systemd.c index e70bc23216..2c72353109 100644 --- a/src/core/dhcp/nm-dhcp-systemd.c +++ b/src/core/dhcp/nm-dhcp-systemd.c @@ -294,11 +294,10 @@ ip6_start(NMDhcpClient *client, const struct in6_addr *ll_addr, GError **error) _LOGT("dhcp-client6: set %p", sd_client); - if (client_config->v6.info_only) { - sd_dhcp6_client_set_address_request(sd_client, 0); - if (client_config->v6.needed_prefixes == 0) - sd_dhcp6_client_set_information_request(sd_client, 1); - } + sd_dhcp6_client_set_address_request(sd_client, !client_config->v6.info_only); + sd_dhcp6_client_set_information_request(sd_client, + client_config->v6.info_only + && client_config->v6.needed_prefixes == 0); r = sd_dhcp6_client_set_iaid(sd_client, client_config->v6.iaid); if (r < 0) { -- GitLab From f7a773438262e0cdaa18a3a9955d5f2fb18be782 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 11 May 2022 10:54:35 +0200 Subject: [PATCH 2/3] dhcp: fix setting "-S" flag for dhclient info-only requests Fixes: 58287cbcc0c8 ('core: rework IP configuration in NetworkManager using layer 3 configuration') --- src/core/dhcp/nm-dhcp-dhclient.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-dhclient.c b/src/core/dhcp/nm-dhcp-dhclient.c index 28d40c075b..d0cd5ebdc7 100644 --- a/src/core/dhcp/nm-dhcp-dhclient.c +++ b/src/core/dhcp/nm-dhcp-dhclient.c @@ -330,7 +330,7 @@ create_dhclient_config(NMDhcpDhclient *self, static gboolean dhclient_start(NMDhcpClient *client, - const char *mode_opt, + gboolean set_mode, gboolean release, pid_t *out_pid, GError **error) @@ -439,14 +439,19 @@ dhclient_start(NMDhcpClient *client, } if (addr_family == AF_INET6) { - guint prefixes = client_config->v6.needed_prefixes; + guint prefixes = client_config->v6.needed_prefixes; + const char *mode_opt; g_ptr_array_add(argv, (gpointer) "-6"); - if (prefixes > 0 && nm_streq0(mode_opt, "-S")) { - /* -S is incompatible with -P, only use the latter */ + if (!set_mode) + mode_opt = NULL; + else if (!client_config->v6.info_only) + mode_opt = "-N"; + else if (prefixes == 0) + mode_opt = "-S"; + else mode_opt = NULL; - } if (mode_opt) g_ptr_array_add(argv, (gpointer) mode_opt); @@ -546,7 +551,7 @@ ip4_start(NMDhcpClient *client, GError **error) nm_assert(!client_config->client_id); nm_dhcp_client_set_effective_client_id(client, new_client_id); } - return dhclient_start(client, NULL, FALSE, NULL, error); + return dhclient_start(client, FALSE, FALSE, NULL, error); } static gboolean @@ -581,7 +586,7 @@ ip6_start(NMDhcpClient *client, const struct in6_addr *ll_addr, GError **error) return FALSE; } - return dhclient_start(client, config->v6.needed_prefixes ? "-S" : "-N", FALSE, NULL, error); + return dhclient_start(client, TRUE, FALSE, NULL, error); } static void @@ -615,7 +620,7 @@ stop(NMDhcpClient *client, gboolean release) if (release) { pid_t rpid = -1; - if (dhclient_start(client, NULL, TRUE, &rpid, NULL)) { + if (dhclient_start(client, FALSE, TRUE, &rpid, NULL)) { /* Wait a few seconds for the release to happen */ nm_dhcp_client_stop_pid(rpid, nm_dhcp_client_get_iface(client)); } -- GitLab From ea891485c7625ef2d12dfadf0768d2a46842f02f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 11 May 2022 08:26:24 +0200 Subject: [PATCH 3/3] dhcp: fix ignoring addresses with DHCPv6 otherconf (O flag) With O flag (otherconf mode), don't add the IPv6 addresses to the collected lease. An alternative would be to add it initially, but ignore it when merging the configuration in NML3Cfg. The idea of that would be that if the mode switches from otherconf to managed, that we already have the address. However, depending on the mode we already make different DHCPv6 requests (at least for internal client). That means, if the mode changes we anyway cannot just use the previous lease, because it might not contain all the information. So it seems better to ignore the address early. Fixes: 58287cbcc0c8 ('core: rework IP configuration in NetworkManager using layer 3 configuration') https://bugzilla.redhat.com/show_bug.cgi?id=2083968 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/ ## 953 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/ ## 1220 --- src/core/dhcp/nm-dhcp-systemd.c | 69 ++++++++++++++++++--------------- src/core/dhcp/nm-dhcp-utils.c | 12 +++--- 2 files changed, 45 insertions(+), 36 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-systemd.c b/src/core/dhcp/nm-dhcp-systemd.c index 2c72353109..045d528791 100644 --- a/src/core/dhcp/nm-dhcp-systemd.c +++ b/src/core/dhcp/nm-dhcp-systemd.c @@ -80,14 +80,12 @@ lease_to_ip6_config(NMDedupMultiIndex *multi_idx, gs_unref_hashtable GHashTable *options = NULL; struct in6_addr tmp_addr; const struct in6_addr *dns; - uint32_t lft_pref, lft_valid; char addr_str[NM_UTILS_INET_ADDRSTRLEN]; char **domains; char **ntp_fqdns; const struct in6_addr *ntp_addrs; const char *s; - nm_auto_free_gstring GString *str = NULL; - gboolean has_any_addresses = FALSE; + nm_auto_free_gstring GString *str = NULL; int num, i; nm_assert(lease); @@ -96,36 +94,45 @@ lease_to_ip6_config(NMDedupMultiIndex *multi_idx, options = nm_dhcp_option_create_options_dict(); - sd_dhcp6_lease_reset_address_iter(lease); - nm_gstring_prepare(&str); - while (sd_dhcp6_lease_get_address(lease, &tmp_addr, &lft_pref, &lft_valid) >= 0) { - const NMPlatformIP6Address address = { - .plen = 128, - .address = tmp_addr, - .timestamp = ts, - .lifetime = lft_valid, - .preferred = lft_pref, - .addr_source = NM_IP_CONFIG_SOURCE_DHCP, - }; - - nm_l3_config_data_add_address_6(l3cd, &address); - - _nm_utils_inet6_ntop(&tmp_addr, addr_str); - g_string_append(nm_gstring_add_space_delimiter(str), addr_str); - - has_any_addresses = TRUE; - } + if (!info_only) { + gboolean has_any_addresses = FALSE; + uint32_t lft_pref; + uint32_t lft_valid; - if (str->len) { - nm_dhcp_option_add_option(options, AF_INET6, NM_DHCP_OPTION_DHCP6_NM_IP_ADDRESS, str->str); - } + sd_dhcp6_lease_reset_address_iter(lease); + nm_gstring_prepare(&str); + while (sd_dhcp6_lease_get_address(lease, &tmp_addr, &lft_pref, &lft_valid) >= 0) { + const NMPlatformIP6Address address = { + .plen = 128, + .address = tmp_addr, + .timestamp = ts, + .lifetime = lft_valid, + .preferred = lft_pref, + .addr_source = NM_IP_CONFIG_SOURCE_DHCP, + }; + + nm_l3_config_data_add_address_6(l3cd, &address); + + _nm_utils_inet6_ntop(&tmp_addr, addr_str); + g_string_append(nm_gstring_add_space_delimiter(str), addr_str); - if (!info_only && !has_any_addresses) { - g_set_error_literal(error, - NM_MANAGER_ERROR, - NM_MANAGER_ERROR_FAILED, - "no address received in managed mode"); - return NULL; + has_any_addresses = TRUE; + } + + if (str->len) { + nm_dhcp_option_add_option(options, + AF_INET6, + NM_DHCP_OPTION_DHCP6_NM_IP_ADDRESS, + str->str); + } + + if (!has_any_addresses) { + g_set_error_literal(error, + NM_MANAGER_ERROR, + NM_MANAGER_ERROR_FAILED, + "no address received in managed mode"); + return NULL; + } } num = sd_dhcp6_lease_get_dns(lease, &dns); diff --git a/src/core/dhcp/nm-dhcp-utils.c b/src/core/dhcp/nm-dhcp-utils.c index ecb6eea67a..3ae7e6b71c 100644 --- a/src/core/dhcp/nm-dhcp-utils.c +++ b/src/core/dhcp/nm-dhcp-utils.c @@ -670,8 +670,13 @@ nm_dhcp_utils_ip6_config_from_options(NMDedupMultiIndex *multi_idx, _LOG2I(LOGD_DHCP6, iface, " preferred_lft %u", address.preferred); } - str = g_hash_table_lookup(options, "ip6_address"); - if (str) { + if (!info_only) { + str = g_hash_table_lookup(options, "ip6_address"); + if (!str) { + /* No address in Managed mode is a hard error */ + return NULL; + } + if (!inet_pton(AF_INET6, str, &tmp_addr)) { _LOG2W(LOGD_DHCP6, iface, "(%s): DHCP returned invalid address '%s'", iface, str); return NULL; @@ -681,9 +686,6 @@ nm_dhcp_utils_ip6_config_from_options(NMDedupMultiIndex *multi_idx, address.addr_source = NM_IP_CONFIG_SOURCE_DHCP; nm_l3_config_data_add_address_6(l3cd, &address); _LOG2I(LOGD_DHCP6, iface, " address %s", str); - } else if (info_only == FALSE) { - /* No address in Managed mode is a hard error */ - return NULL; } str = g_hash_table_lookup(options, "host_name"); -- GitLab