From 7a33870bf13527186a2e9ed5fdfd5486b9fc15d1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 23 Jun 2022 07:35:13 +0200 Subject: [PATCH 1/8] libnm: assert nm_utils_ip4_prefix_to_netmask() for valid IPv4 prefix length There was already an nm_assert() assertion. Upgrade this to a g_return_val_if_fail(). This function is public API, so this is potentially an API break. But it should highlight a bug in the caller. --- src/libnm-core-impl/nm-utils.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/libnm-core-impl/nm-utils.c b/src/libnm-core-impl/nm-utils.c index 0d3f6a6fda..d1590a7a54 100644 --- a/src/libnm-core-impl/nm-utils.c +++ b/src/libnm-core-impl/nm-utils.c @@ -1558,7 +1558,13 @@ nm_utils_ip4_routes_from_variant(GVariant *value) /** * nm_utils_ip4_netmask_to_prefix: - * @netmask: an IPv4 netmask in network byte order + * @netmask: an IPv4 netmask in network byte order. + * Usually the netmask has all leading bits up to the prefix + * set so that the netmask is identical to having the first + * prefix bits of the address set. + * If that is not the case and there are "holes" in the + * mask, the prefix is determined based on the lowest bit + * set. * * Returns: the CIDR prefix represented by the netmask **/ @@ -1567,6 +1573,7 @@ nm_utils_ip4_netmask_to_prefix(guint32 netmask) { G_STATIC_ASSERT_EXPR(__SIZEOF_INT__ == 4); G_STATIC_ASSERT_EXPR(sizeof(int) == 4); + G_STATIC_ASSERT_EXPR(sizeof(guint) == 4); G_STATIC_ASSERT_EXPR(sizeof(netmask) == 4); return ((netmask != 0u) ? (guint32) (32 - __builtin_ctz(ntohl(netmask))) : 0u); @@ -1574,13 +1581,15 @@ nm_utils_ip4_netmask_to_prefix(guint32 netmask) /** * nm_utils_ip4_prefix_to_netmask: - * @prefix: a CIDR prefix + * @prefix: a CIDR prefix, must be not larger than 32. * * Returns: the netmask represented by the prefix, in network byte order **/ guint32 nm_utils_ip4_prefix_to_netmask(guint32 prefix) { + g_return_val_if_fail(prefix <= 32, 0xffffffffu); + return _nm_utils_ip4_prefix_to_netmask(prefix); } From 05014b328f35db39e7de3f64497ef49b42fe7902 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 23 Jun 2022 07:39:59 +0200 Subject: [PATCH 2/8] glib-aux: add _nm_utils_ip4_netmask_to_prefix() helper nm_utils_ip4_netmask_to_prefix() and nm_utils_ip4_prefix_to_netmask() are public API in libnm. We thus already have an internal implementation _nm_utils_ip4_prefix_to_netmask(), for non-libnm users. Internally, we should never use the libnm variant. For consistency and so that we have the helper available in libnm-glib-aux, add _nm_utils_ip4_netmask_to_prefix(). --- src/libnm-core-impl/nm-utils.c | 7 +------ src/libnm-glib-aux/nm-shared-utils.h | 11 +++++++++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/libnm-core-impl/nm-utils.c b/src/libnm-core-impl/nm-utils.c index d1590a7a54..3c4cbc651e 100644 --- a/src/libnm-core-impl/nm-utils.c +++ b/src/libnm-core-impl/nm-utils.c @@ -1571,12 +1571,7 @@ nm_utils_ip4_routes_from_variant(GVariant *value) guint32 nm_utils_ip4_netmask_to_prefix(guint32 netmask) { - G_STATIC_ASSERT_EXPR(__SIZEOF_INT__ == 4); - G_STATIC_ASSERT_EXPR(sizeof(int) == 4); - G_STATIC_ASSERT_EXPR(sizeof(guint) == 4); - G_STATIC_ASSERT_EXPR(sizeof(netmask) == 4); - - return ((netmask != 0u) ? (guint32) (32 - __builtin_ctz(ntohl(netmask))) : 0u); + return _nm_utils_ip4_netmask_to_prefix(netmask); } /** diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index fddcc8fe26..6cdc8875ef 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -389,6 +389,17 @@ gboolean nm_utils_get_ipv6_interface_identifier(NMLinkType link_type, /*****************************************************************************/ +static inline guint32 +_nm_utils_ip4_netmask_to_prefix(in_addr_t subnetmask) +{ + G_STATIC_ASSERT_EXPR(__SIZEOF_INT__ == 4); + G_STATIC_ASSERT_EXPR(sizeof(int) == 4); + G_STATIC_ASSERT_EXPR(sizeof(guint) == 4); + G_STATIC_ASSERT_EXPR(sizeof(subnetmask) == 4); + + return ((subnetmask != 0u) ? (guint32) (32 - __builtin_ctz(ntohl(subnetmask))) : 0u); +} + /** * _nm_utils_ip4_prefix_to_netmask: * @prefix: a CIDR prefix From 863b71a8fe35a08955320d7db99c3c21c5f85635 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 23 Jun 2022 07:43:20 +0200 Subject: [PATCH 3/8] all: use internal _nm_utils_ip4_netmask_to_prefix() We have two variants of the function: nm_utils_ip4_netmask_to_prefix() and _nm_utils_ip4_netmask_to_prefix(). The former only exists because it is public API in libnm. Internally, only use the latter. --- src/core/devices/wwan/nm-modem-ofono.c | 2 +- src/core/dhcp/nm-dhcp-nettools.c | 2 +- src/core/dhcp/nm-dhcp-utils.c | 2 +- src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 4 ++-- src/core/settings/plugins/ifupdown/nms-ifupdown-parser.c | 2 +- src/core/supplicant/nm-supplicant-interface.c | 2 +- src/libnm-core-impl/tests/test-general.c | 2 +- src/nm-cloud-setup/nmcs-provider-aliyun.c | 2 +- src/nm-initrd-generator/nmi-cmdline-reader.c | 2 +- src/nm-initrd-generator/nmi-dt-reader.c | 2 +- 10 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/core/devices/wwan/nm-modem-ofono.c b/src/core/devices/wwan/nm-modem-ofono.c index 0db3004656..c003880e6c 100644 --- a/src/core/devices/wwan/nm-modem-ofono.c +++ b/src/core/devices/wwan/nm-modem-ofono.c @@ -805,7 +805,7 @@ handle_settings(GVariant *v_dict, gpointer user_data) _LOGW("invalid 'Netmask': %s", s ?: ""); goto out; } - address.plen = nm_utils_ip4_netmask_to_prefix(address_network); + address.plen = _nm_utils_ip4_netmask_to_prefix(address_network); _LOGI("Address: %s", nm_platform_ip4_address_to_string(&address, sbuf, sizeof(sbuf))); nm_l3_config_data_add_address_4(priv->l3cd_4, &address); diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 0f9c998929..224f96d370 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -249,7 +249,7 @@ lease_parse_address(NDhcp4ClientLease *lease, return FALSE; } - a_plen = nm_utils_ip4_netmask_to_prefix(a_netmask); + a_plen = _nm_utils_ip4_netmask_to_prefix(a_netmask); nm_dhcp_option_add_option_in_addr(options, AF_INET, diff --git a/src/core/dhcp/nm-dhcp-utils.c b/src/core/dhcp/nm-dhcp-utils.c index 71813594a0..160ae189b1 100644 --- a/src/core/dhcp/nm-dhcp-utils.c +++ b/src/core/dhcp/nm-dhcp-utils.c @@ -423,7 +423,7 @@ nm_dhcp_utils_ip4_config_from_options(NMDedupMultiIndex *multi_idx, str = g_hash_table_lookup(options, "subnet_mask"); if (str && (inet_pton(AF_INET, str, &tmp_addr) > 0)) { - plen = nm_utils_ip4_netmask_to_prefix(tmp_addr); + plen = _nm_utils_ip4_netmask_to_prefix(tmp_addr); _LOG2I(LOGD_DHCP4, iface, " plen %d (%s)", plen, str); } else { /* Get default netmask for the IP according to appropriate class. */ diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index c62e3495fc..78b65c7730 100644 --- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -777,7 +777,7 @@ read_full_ip4_address(shvarFile *ifcfg, if (!read_ip4_address(ifcfg, numbered_tag(tag, "NETMASK", which), &has_key, &a, error)) return FALSE; if (has_key) - prefix = nm_utils_ip4_netmask_to_prefix(a); + prefix = _nm_utils_ip4_netmask_to_prefix(a); else { if (base_addr) prefix = nm_ip_address_get_prefix(base_addr); @@ -1450,7 +1450,7 @@ read_one_ip4_route(shvarFile *ifcfg, guint32 which, NMIPRoute **out_route, GErro error)) return FALSE; if (has_key) { - prefix = nm_utils_ip4_netmask_to_prefix(netmask); + prefix = _nm_utils_ip4_netmask_to_prefix(netmask); if (netmask != _nm_utils_ip4_prefix_to_netmask(prefix)) { g_set_error(error, NM_SETTINGS_ERROR, diff --git a/src/core/settings/plugins/ifupdown/nms-ifupdown-parser.c b/src/core/settings/plugins/ifupdown/nms-ifupdown-parser.c index c079266e03..a1d6781b41 100644 --- a/src/core/settings/plugins/ifupdown/nms-ifupdown-parser.c +++ b/src/core/settings/plugins/ifupdown/nms-ifupdown-parser.c @@ -451,7 +451,7 @@ update_ip4_setting_from_if_block(NMConnection *connection, if_block *block, GErr netmask_v); return FALSE; } else { - netmask_int = nm_utils_ip4_netmask_to_prefix(tmp_mask); + netmask_int = _nm_utils_ip4_netmask_to_prefix(tmp_mask); } } diff --git a/src/core/supplicant/nm-supplicant-interface.c b/src/core/supplicant/nm-supplicant-interface.c index e502ae8556..ad5595cac5 100644 --- a/src/core/supplicant/nm-supplicant-interface.c +++ b/src/core/supplicant/nm-supplicant-interface.c @@ -3131,7 +3131,7 @@ _signal_handle(NMSupplicantInterface *self, _set_p2p_assigned_addr(iface, addr_data, - nm_utils_ip4_netmask_to_prefix(netmask)); + _nm_utils_ip4_netmask_to_prefix(netmask)); } else { _LOGW("P2P: GroupStarted signaled invalid IP Address information"); } diff --git a/src/libnm-core-impl/tests/test-general.c b/src/libnm-core-impl/tests/test-general.c index eb939ba15c..30be50b92c 100644 --- a/src/libnm-core-impl/tests/test-general.c +++ b/src/libnm-core-impl/tests/test-general.c @@ -4925,7 +4925,7 @@ _netmask_to_prefix(guint32 netmask) /* we re-implemented the netmask-to-prefix code differently. Check * that they agree. */ - g_assert_cmpint(prefix, ==, nm_utils_ip4_netmask_to_prefix(netmask)); + g_assert_cmpint(prefix, ==, _nm_utils_ip4_netmask_to_prefix(netmask)); return prefix; } diff --git a/src/nm-cloud-setup/nmcs-provider-aliyun.c b/src/nm-cloud-setup/nmcs-provider-aliyun.c index 1a5e5459e4..34ab5ecc87 100644 --- a/src/nm-cloud-setup/nmcs-provider-aliyun.c +++ b/src/nm-cloud-setup/nmcs-provider-aliyun.c @@ -203,7 +203,7 @@ _get_config_fetch_done_cb(NMHttpClient *http_client, g_bytes_get_data(response, NULL), NULL, &netmask_bin)) { - config_iface_data->cidr_prefix = nm_utils_ip4_netmask_to_prefix(netmask_bin); + config_iface_data->cidr_prefix = _nm_utils_ip4_netmask_to_prefix(netmask_bin); }; break; diff --git a/src/nm-initrd-generator/nmi-cmdline-reader.c b/src/nm-initrd-generator/nmi-cmdline-reader.c index 88760f7574..43d9eefc99 100644 --- a/src/nm-initrd-generator/nmi-cmdline-reader.c +++ b/src/nm-initrd-generator/nmi-cmdline-reader.c @@ -633,7 +633,7 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) NMIPAddr addr; if (is_ipv4 && nm_utils_parse_inaddr_bin(AF_INET, netmask, NULL, &addr)) - client_ip_prefix = nm_utils_ip4_netmask_to_prefix(addr.addr4); + client_ip_prefix = _nm_utils_ip4_netmask_to_prefix(addr.addr4); else client_ip_prefix = _nm_utils_ascii_str_to_int64(netmask, 10, 0, is_ipv4 ? 32 : 128, -1); diff --git a/src/nm-initrd-generator/nmi-dt-reader.c b/src/nm-initrd-generator/nmi-dt-reader.c index 8bd591a803..f140e7a30e 100644 --- a/src/nm-initrd-generator/nmi-dt-reader.c +++ b/src/nm-initrd-generator/nmi-dt-reader.c @@ -288,7 +288,7 @@ nmi_dt_reader_parse(const char *sysfs_dir) guint32 netmask_v4; nm_ip_address_get_address_binary(netmask, &netmask_v4); - prefix = nm_utils_ip4_netmask_to_prefix(netmask_v4); + prefix = _nm_utils_ip4_netmask_to_prefix(netmask_v4); } if (prefix == -1) From 57dfa999f77d84a184960d3c2095c6c1d28c8a4e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 21 Jun 2022 10:31:02 +0200 Subject: [PATCH 4/8] dhcp/nettools: accept missing "subnet mask" (option 1) in DHCP lease Do the same as dhclient plugin in nm_dhcp_utils_ip4_config_from_options(). https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1037 --- src/core/dhcp/nm-dhcp-nettools.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 224f96d370..0a25f90d27 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -237,20 +237,26 @@ lease_parse_address(NDhcp4ClientLease *lease, } r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_SUBNET_MASK, &l_data, &l_data_len); - if (r != 0 - || !nm_dhcp_lease_data_parse_in_addr(l_data, - l_data_len, - &a_netmask, - iface, - NM_DHCP_OPTION_DHCP4_SUBNET_MASK)) { - nm_utils_error_set_literal(error, - NM_UTILS_ERROR_UNKNOWN, - "could not get netmask from lease"); - return FALSE; + if (r == N_DHCP4_E_UNSET) { + /* Some DHCP servers may not set the subnet-mask (issue#1037). + * Do the same as the dhclient plugin and use a default. */ + a_plen = _nm_utils_ip4_get_default_prefix(a_address.s_addr); + a_netmask = _nm_utils_ip4_prefix_to_netmask(a_plen); + } else { + if (r != 0 + || !nm_dhcp_lease_data_parse_in_addr(l_data, + l_data_len, + &a_netmask, + iface, + NM_DHCP_OPTION_DHCP4_SUBNET_MASK)) { + nm_utils_error_set_literal(error, + NM_UTILS_ERROR_UNKNOWN, + "could not get netmask from lease"); + return FALSE; + } + a_plen = _nm_utils_ip4_netmask_to_prefix(a_netmask); } - a_plen = _nm_utils_ip4_netmask_to_prefix(a_netmask); - nm_dhcp_option_add_option_in_addr(options, AF_INET, NM_DHCP_OPTION_DHCP4_NM_IP_ADDRESS, From c06e6390a48d45cc13a225a1ea99165f4cb933fd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 21 Jun 2022 11:26:23 +0200 Subject: [PATCH 5/8] dhcp/nettools: normalize subnet netmask in nettools client For an IPv4 subnet mask we expect that all the leading bits are set (no "holes"). But _nm_utils_ip4_netmask_to_prefix() does not enforce that, and tries to make the best of it. In face of a netmask with holes, normalize the mask. --- src/core/dhcp/nm-dhcp-nettools.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 0a25f90d27..4fe0a9394d 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -254,7 +254,8 @@ lease_parse_address(NDhcp4ClientLease *lease, "could not get netmask from lease"); return FALSE; } - a_plen = _nm_utils_ip4_netmask_to_prefix(a_netmask); + a_plen = _nm_utils_ip4_netmask_to_prefix(a_netmask); + a_netmask = _nm_utils_ip4_prefix_to_netmask(a_plen); } nm_dhcp_option_add_option_in_addr(options, From f0d132bda96a1ce6ca4f6be4b8523773c483dc28 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 23 Jun 2022 12:07:16 +0200 Subject: [PATCH 6/8] dhcp: add nm_dhcp_client_create_l3cd() helper --- src/core/dhcp/nm-dhcp-client.c | 10 ++++++++++ src/core/dhcp/nm-dhcp-client.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c index 976fec4ecc..113a2c904c 100644 --- a/src/core/dhcp/nm-dhcp-client.c +++ b/src/core/dhcp/nm-dhcp-client.c @@ -228,6 +228,16 @@ nm_dhcp_client_get_effective_client_id(NMDhcpClient *self) return priv->effective_client_id; } +NML3ConfigData * +nm_dhcp_client_create_l3cd(NMDhcpClient *self) +{ + NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); + + return nm_l3_config_data_new(nm_l3cfg_get_multi_idx(priv->config.l3cfg), + nm_l3cfg_get_ifindex(priv->config.l3cfg), + NM_IP_CONFIG_SOURCE_DHCP); +} + /*****************************************************************************/ void diff --git a/src/core/dhcp/nm-dhcp-client.h b/src/core/dhcp/nm-dhcp-client.h index 72444c3fc0..51c6bc048f 100644 --- a/src/core/dhcp/nm-dhcp-client.h +++ b/src/core/dhcp/nm-dhcp-client.h @@ -288,6 +288,8 @@ int nm_dhcp_client_get_ifindex(NMDhcpClient *self); void nm_dhcp_client_set_effective_client_id(NMDhcpClient *self, GBytes *client_id); GBytes *nm_dhcp_client_get_effective_client_id(NMDhcpClient *self); +NML3ConfigData *nm_dhcp_client_create_l3cd(NMDhcpClient *self); + /***************************************************************************** * Client data *****************************************************************************/ From c7c57e8fb9e1d295f13a3959f3eebbfb05a88588 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 23 Jun 2022 11:55:52 +0200 Subject: [PATCH 7/8] dhcp/nettools: log message about guessing subnet mask for IPv4 --- src/core/dhcp/nm-dhcp-nettools.c | 35 ++++++++++++++++---------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 4fe0a9394d..134c884e58 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -164,7 +164,8 @@ lease_option_consume_route(const uint8_t **datap, /*****************************************************************************/ static gboolean -lease_parse_address(NDhcp4ClientLease *lease, +lease_parse_address(NMDhcpNettools *self /* for logging context only */, + NDhcp4ClientLease *lease, NML3ConfigData *l3cd, const char *iface, GHashTable *options, @@ -238,10 +239,16 @@ lease_parse_address(NDhcp4ClientLease *lease, r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_SUBNET_MASK, &l_data, &l_data_len); if (r == N_DHCP4_E_UNSET) { + char str1[NM_UTILS_INET_ADDRSTRLEN]; + char str2[NM_UTILS_INET_ADDRSTRLEN]; + /* Some DHCP servers may not set the subnet-mask (issue#1037). * Do the same as the dhclient plugin and use a default. */ a_plen = _nm_utils_ip4_get_default_prefix(a_address.s_addr); a_netmask = _nm_utils_ip4_prefix_to_netmask(a_plen); + _LOGT("missing subnet mask (option 1). Guess %s based on IP address %s", + _nm_utils_inet4_ntop(a_netmask, str1), + _nm_utils_inet4_ntop(a_address.s_addr, str2)); } else { if (r != 0 || !nm_dhcp_lease_data_parse_in_addr(l_data, @@ -596,12 +603,9 @@ lease_parse_private_options(NDhcp4ClientLease *lease, GHashTable *options) } static NML3ConfigData * -lease_to_ip4_config(NMDedupMultiIndex *multi_idx, - const char *iface, - int ifindex, - NDhcp4ClientLease *lease, - GError **error) +lease_to_ip4_config(NMDhcpNettools *self, NDhcp4ClientLease *lease, GError **error) { + const char *iface; nm_auto_str_buf NMStrBuf sbuf = NM_STR_BUF_INIT(0, FALSE); nm_auto_unref_l3cd_init NML3ConfigData *l3cd = NULL; gs_unref_hashtable GHashTable *options = NULL; @@ -614,13 +618,15 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, struct in_addr v_inaddr_s; int r; - g_return_val_if_fail(lease != NULL, NULL); + nm_assert(lease); - l3cd = nm_l3_config_data_new(multi_idx, ifindex, NM_IP_CONFIG_SOURCE_DHCP); + iface = nm_dhcp_client_get_iface(NM_DHCP_CLIENT(self)); + + l3cd = nm_dhcp_client_create_l3cd(NM_DHCP_CLIENT(self)); options = nm_dhcp_option_create_options_dict(); - if (!lease_parse_address(lease, l3cd, iface, options, &lease_address, error)) + if (!lease_parse_address(self, lease, l3cd, iface, options, &lease_address, error)) return NULL; r = n_dhcp4_client_lease_get_server_identifier(lease, &v_inaddr_s); @@ -884,9 +890,7 @@ lease_save(NMDhcpNettools *self, NDhcp4ClientLease *lease, const char *lease_fil static void bound4_handle(NMDhcpNettools *self, guint event, NDhcp4ClientLease *lease) { - NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self); - NMDhcpClient *client = NM_DHCP_CLIENT(self); - const NMDhcpClientConfig *client_config; + NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self); nm_auto_unref_l3cd_init NML3ConfigData *l3cd = NULL; gs_free_error GError *error = NULL; @@ -895,12 +899,7 @@ bound4_handle(NMDhcpNettools *self, guint event, NDhcp4ClientLease *lease) _LOGT("lease available (%s)", (event == N_DHCP4_CLIENT_EVENT_GRANTED) ? "granted" : "extended"); - client_config = nm_dhcp_client_get_config(client); - l3cd = lease_to_ip4_config(nm_dhcp_client_get_multi_idx(client), - client_config->iface, - nm_dhcp_client_get_ifindex(client), - lease, - &error); + l3cd = lease_to_ip4_config(self, lease, &error); if (!l3cd) { _LOGW("failure to parse lease: %s", error->message); From ff694f42a7c86ef5d6a9d1355b1fb2209604edb6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 23 Jun 2022 12:13:51 +0200 Subject: [PATCH 8/8] dhcp/systemd: pass client instance to lease_to_ip6_config() This makes it more consistent with nettools' lease_to_ip4_config(). The benefit of having a self pointer, is that it provides the necessary context for logging. Without it, these functions cannot correctly log. At this point, it's clearer to get the necessary data directly from the DHCP client instance, instead of having the caller passing them on (redundantly). --- src/core/dhcp/nm-dhcp-systemd.c | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-systemd.c b/src/core/dhcp/nm-dhcp-systemd.c index b106db2c60..49e21d97e5 100644 --- a/src/core/dhcp/nm-dhcp-systemd.c +++ b/src/core/dhcp/nm-dhcp-systemd.c @@ -68,13 +68,7 @@ G_DEFINE_TYPE(NMDhcpSystemd, nm_dhcp_systemd, NM_TYPE_DHCP_CLIENT) /*****************************************************************************/ static NML3ConfigData * -lease_to_ip6_config(NMDedupMultiIndex *multi_idx, - const char *iface, - int ifindex, - sd_dhcp6_lease *lease, - gboolean info_only, - gint32 ts, - GError **error) +lease_to_ip6_config(NMDhcpSystemd *self, sd_dhcp6_lease *lease, gint32 ts, GError **error) { nm_auto_unref_l3cd_init NML3ConfigData *l3cd = NULL; gs_unref_hashtable GHashTable *options = NULL; @@ -90,11 +84,11 @@ lease_to_ip6_config(NMDedupMultiIndex *multi_idx, nm_assert(lease); - l3cd = nm_l3_config_data_new(multi_idx, ifindex, NM_IP_CONFIG_SOURCE_DHCP); + l3cd = nm_dhcp_client_create_l3cd(NM_DHCP_CLIENT(self)); options = nm_dhcp_option_create_options_dict(); - if (!info_only) { + if (!nm_dhcp_client_get_config(NM_DHCP_CLIENT(self))->v6.info_only) { gboolean has_any_addresses = FALSE; uint32_t lft_pref; uint32_t lft_valid; @@ -192,17 +186,13 @@ lease_to_ip6_config(NMDedupMultiIndex *multi_idx, static void bound6_handle(NMDhcpSystemd *self) { - NMDhcpSystemdPrivate *priv = NM_DHCP_SYSTEMD_GET_PRIVATE(self); - const gint32 ts = nm_utils_get_monotonic_timestamp_sec(); - const char *iface = nm_dhcp_client_get_iface(NM_DHCP_CLIENT(self)); - const NMDhcpClientConfig *client_config; + NMDhcpSystemdPrivate *priv = NM_DHCP_SYSTEMD_GET_PRIVATE(self); + const gint32 ts = nm_utils_get_monotonic_timestamp_sec(); nm_auto_unref_l3cd_init NML3ConfigData *l3cd = NULL; gs_free_error GError *error = NULL; NMPlatformIP6Address prefix = {0}; sd_dhcp6_lease *lease = NULL; - client_config = nm_dhcp_client_get_config(NM_DHCP_CLIENT(self)); - if (sd_dhcp6_client_get_lease(priv->client6, &lease) < 0 || !lease) { _LOGW(" no lease!"); _nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_FAIL, NULL); @@ -211,13 +201,7 @@ bound6_handle(NMDhcpSystemd *self) _LOGD("lease available"); - l3cd = lease_to_ip6_config(nm_dhcp_client_get_multi_idx(NM_DHCP_CLIENT(self)), - iface, - nm_dhcp_client_get_ifindex(NM_DHCP_CLIENT(self)), - lease, - client_config->v6.info_only, - ts, - &error); + l3cd = lease_to_ip6_config(self, lease, ts, &error); if (!l3cd) { _LOGW("%s", error->message);