From 00c7e4855e20fe4fd4503be78ac1de348a9ec586 Mon Sep 17 00:00:00 2001 From: Wen Liang Date: Tue, 16 Jan 2024 16:18:10 -0500 Subject: [PATCH 1/3] l3cfg: add dhcp_enabled_4 and dhcp_enabled_6 properties to NML3ConfigData --- src/core/nm-l3-config-data.c | 39 ++++++++++++++++++++++++++++++++++++ src/core/nm-l3-config-data.h | 2 ++ 2 files changed, 41 insertions(+) diff --git a/src/core/nm-l3-config-data.c b/src/core/nm-l3-config-data.c index 030c1101f5..a4647116a9 100644 --- a/src/core/nm-l3-config-data.c +++ b/src/core/nm-l3-config-data.c @@ -157,6 +157,8 @@ struct _NML3ConfigData { bool has_routes_with_type_local_6_set : 1; bool has_routes_with_type_local_4_val : 1; bool has_routes_with_type_local_6_val : 1; + bool dhcp_enabled_4 : 1; + bool dhcp_enabled_6 : 1; bool ndisc_hop_limit_set : 1; bool ndisc_reachable_time_msec_set : 1; @@ -1933,6 +1935,19 @@ nm_l3_config_data_set_mptcp_flags(NML3ConfigData *self, NMMptcpFlags mptcp_flags return TRUE; } +gboolean +nm_l3_config_data_get_dhcp_enabled(const NML3ConfigData *self, int addr_family) +{ + const int IS_IPv4 = NM_IS_IPv4(addr_family); + + nm_assert(_NM_IS_L3_CONFIG_DATA(self, TRUE)); + if (IS_IPv4) { + return self->dhcp_enabled_4; + } else { + return self->dhcp_enabled_6; + } +} + NMProxyConfigMethod nm_l3_config_data_get_proxy_method(const NML3ConfigData *self) { @@ -2722,6 +2737,7 @@ _init_from_connection_ip(NML3ConfigData *self, int addr_family, NMConnection *co guint nnameservers; guint nsearches; const char *gateway_str; + const char *method; NMIPAddr gateway_bin; guint i; int idx; @@ -2739,6 +2755,24 @@ _init_from_connection_ip(NML3ConfigData *self, int addr_family, NMConnection *co never_default = nm_setting_ip_config_get_never_default(s_ip); + method = nm_setting_ip_config_get_method(s_ip); + if (IS_IPv4) { + if (nm_streq(method, NM_SETTING_IP4_CONFIG_METHOD_AUTO)) { + self->dhcp_enabled_4 = TRUE; + } else { + self->dhcp_enabled_4 = FALSE; + } + } else { + method = nm_setting_ip_config_get_method(s_ip); + if (NM_IN_STRSET(method, + NM_SETTING_IP6_CONFIG_METHOD_AUTO, + NM_SETTING_IP6_CONFIG_METHOD_DHCP)) { + self->dhcp_enabled_6 = TRUE; + } else { + self->dhcp_enabled_6 = FALSE; + } + } + nm_l3_config_data_set_never_default(self, addr_family, !!never_default); if (!never_default && (gateway_str = nm_setting_ip_config_get_gateway(s_ip)) @@ -3422,6 +3456,11 @@ nm_l3_config_data_merge(NML3ConfigData *self, self->dhcp_lease_x[0] = nm_dhcp_lease_ref(self->dhcp_lease_x[0]); self->dhcp_lease_x[1] = nm_dhcp_lease_ref(self->dhcp_lease_x[1]); } + if (src->dhcp_enabled_4) + self->dhcp_enabled_4 = TRUE; + + if (src->dhcp_enabled_6) + self->dhcp_enabled_6 = TRUE; } NML3ConfigData * diff --git a/src/core/nm-l3-config-data.h b/src/core/nm-l3-config-data.h index 80abb00d88..b55b2f4194 100644 --- a/src/core/nm-l3-config-data.h +++ b/src/core/nm-l3-config-data.h @@ -554,6 +554,8 @@ NMSettingIP6ConfigPrivacy nm_l3_config_data_get_ip6_privacy(const NML3ConfigData gboolean nm_l3_config_data_set_ip6_privacy(NML3ConfigData *self, NMSettingIP6ConfigPrivacy ip6_privacy); +gboolean nm_l3_config_data_get_dhcp_enabled(const NML3ConfigData *self, int addr_family); + NMProxyConfigMethod nm_l3_config_data_get_proxy_method(const NML3ConfigData *self); gboolean nm_l3_config_data_set_proxy_method(NML3ConfigData *self, NMProxyConfigMethod value); From cf28660b6a5dd5177de8d56280e3fbec8a884ae6 Mon Sep 17 00:00:00 2001 From: Wen Liang Date: Mon, 22 Jan 2024 15:37:49 -0500 Subject: [PATCH 2/3] core: check the dhcp enabled flag in l3cfg The decision to configure or not configure routes without addresses only related to what method is configured - DHCP and non-DHCP cases. For DHCP case, the deamon waits until addresses appear first before configuring the static routes to preserve the behavior mentioned in https://bugzilla.redhat.com/show_bug.cgi?id=2102212, otherwise, the daemon can configure the routes immediately for non-DHCP case. --- src/core/nm-l3cfg.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index eb13968196..f428d04cc6 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -1301,6 +1301,7 @@ _commit_collect_routes(NML3Cfg *self, const int IS_IPv4 = NM_IS_IPv4(addr_family); const NMDedupMultiHeadEntry *head_entry; const NMDedupMultiEntry *entry; + gboolean is_dhcp_enabled; nm_assert(routes && !*routes); nm_assert(routes_nodev && !*routes_nodev); @@ -1320,17 +1321,21 @@ _commit_collect_routes(NML3Cfg *self, else { nm_assert(NMP_OBJECT_CAST_IP_ROUTE(obj)->ifindex == self->priv.ifindex); - if (!any_addrs) { + is_dhcp_enabled = + nm_l3_config_data_get_dhcp_enabled(self->priv.p->combined_l3cd_commited, + addr_family); + if (!any_addrs && is_dhcp_enabled) { /* This is a unicast route (or a similar route, which has an * ifindex). * * However, during this commit we don't plan to configure any - * IP addresses. With `ipvx.method=manual` that should not be - * possible. More likely, this is because the profile has - * `ipvx.method=auto` and static routes. + * IP addresses when the profile has `ipvx.method=auto` and + * static routes. * * Don't configure any such routes before we also have at least - * one IP address. + * one IP address except for `ipvx.method=manual` where static + * routes are allowed to configure even if the connection has + * no addresses * * This code applies to IPv4 and IPv6, however for IPv6 we * early on configure a link local address, so in practice the From 0e7bda8ad80cebb57c337c5deda8bdee2ac23941 Mon Sep 17 00:00:00 2001 From: Wen Liang Date: Sun, 21 Jan 2024 17:53:52 -0500 Subject: [PATCH 3/3] libnm: allow configuring static route when address is empty OpenShift MetalLB team requests to configure additional routes whenever the nodes does not have a configured IP address or route for the subnet in which MetalLB issues addresses. Note in linux network stack, it does not matter what interface you add the address on a node (for example, loopback), the kernel is always processing arp-requests and sending arp-replies to any of them, this behavior is considered correct and, moreover, it is widely used in the a dynamic environment as Kubernetes. https://issues.redhat.com/browse/RHEL-5098 https://gitlab.freedesktop.org/NetworkManager/NetworkManager-ci/-/merge_requests/1587 --- src/libnm-core-impl/nm-setting-ip4-config.c | 8 ++-- src/libnm-core-impl/nm-setting-ip6-config.c | 8 ++-- src/libnm-core-impl/tests/test-setting.c | 46 +++++++++++++++++++++ 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-ip4-config.c b/src/libnm-core-impl/nm-setting-ip4-config.c index 92495125ea..c79d0fdfa3 100644 --- a/src/libnm-core-impl/nm-setting-ip4-config.c +++ b/src/libnm-core-impl/nm-setting-ip4-config.c @@ -163,17 +163,17 @@ verify(NMSetting *setting, NMConnection *connection, GError **error) g_assert(method); if (!strcmp(method, NM_SETTING_IP4_CONFIG_METHOD_MANUAL)) { - if (nm_setting_ip_config_get_num_addresses(s_ip) == 0) { + if (nm_setting_ip_config_get_num_addresses(s_ip) == 0 + && nm_setting_ip_config_get_num_routes(s_ip) == 0) { g_set_error(error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_MISSING_PROPERTY, - _("this property cannot be empty for '%s=%s'"), - NM_SETTING_IP_CONFIG_METHOD, + _("method '%s' requires at least an address or a route"), method); g_prefix_error(error, "%s.%s: ", NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_ADDRESSES); + NM_SETTING_IP_CONFIG_METHOD); return FALSE; } } else if (!strcmp(method, NM_SETTING_IP4_CONFIG_METHOD_LINK_LOCAL) diff --git a/src/libnm-core-impl/nm-setting-ip6-config.c b/src/libnm-core-impl/nm-setting-ip6-config.c index 9d607264fa..4ad0993201 100644 --- a/src/libnm-core-impl/nm-setting-ip6-config.c +++ b/src/libnm-core-impl/nm-setting-ip6-config.c @@ -228,17 +228,17 @@ verify(NMSetting *setting, NMConnection *connection, GError **error) g_assert(method); if (nm_streq(method, NM_SETTING_IP6_CONFIG_METHOD_MANUAL)) { - if (nm_setting_ip_config_get_num_addresses(s_ip) == 0) { + if (nm_setting_ip_config_get_num_addresses(s_ip) == 0 + && nm_setting_ip_config_get_num_routes(s_ip) == 0) { g_set_error(error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_MISSING_PROPERTY, - _("this property cannot be empty for '%s=%s'"), - NM_SETTING_IP_CONFIG_METHOD, + _("method '%s' requires at least an address or a route"), method); g_prefix_error(error, "%s.%s: ", NM_SETTING_IP6_CONFIG_SETTING_NAME, - NM_SETTING_IP_CONFIG_ADDRESSES); + NM_SETTING_IP_CONFIG_METHOD); return FALSE; } } else if (NM_IN_STRSET(method, diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c index e4f3ab9fc1..4b1aa2c111 100644 --- a/src/libnm-core-impl/tests/test-setting.c +++ b/src/libnm-core-impl/tests/test-setting.c @@ -5112,6 +5112,50 @@ check_done:; /*****************************************************************************/ +static void +test_setting_connection_empty_address_and_route(void) +{ + NMSettingIPConfig *s_ip4; + NMIPRoute *route; + NMIPAddress *addr; + gs_unref_object NMConnection *con = NULL; + gs_free_error GError *error = NULL; + gboolean success; + + /* IP4 setting */ + con = nmtst_create_minimal_connection("wired", NULL, NM_SETTING_WIRED_SETTING_NAME, NULL); + s_ip4 = (NMSettingIPConfig *) nm_setting_ip4_config_new(); + nm_connection_add_setting(con, NM_SETTING(s_ip4)); + g_object_set(s_ip4, NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_MANUAL, NULL); + g_assert(s_ip4 != NULL); + g_assert(NM_IS_SETTING_IP4_CONFIG(s_ip4)); + success = nm_setting_verify((NMSetting *) s_ip4, con, &error); + nmtst_assert_no_success(success, error); + nm_clear_error(&error); + + route = nm_ip_route_new(AF_INET, "192.168.12.0", 24, NULL, 0, NULL); + nm_setting_ip_config_add_route(s_ip4, route); + success = nm_setting_verify((NMSetting *) s_ip4, con, &error); + nmtst_assert_success(success, error); + nm_clear_error(&error); + + nm_setting_ip_config_clear_routes(s_ip4); + addr = nm_ip_address_new(AF_INET, "1.1.1.3", 24, NULL); + nm_setting_ip_config_add_address(s_ip4, addr); + success = nm_setting_verify((NMSetting *) s_ip4, con, &error); + nmtst_assert_success(success, error); + nm_clear_error(&error); + + nm_setting_ip_config_add_route(s_ip4, route); + success = nm_setting_verify((NMSetting *) s_ip4, con, &error); + nmtst_assert_success(success, error); + nm_ip_address_unref(addr); + nm_ip_route_unref(route); + nm_clear_error(&error); +} + +/*****************************************************************************/ + static void test_setting_connection_secondaries_verify(void) { @@ -5420,6 +5464,8 @@ main(int argc, char **argv) test_8021x); g_test_add_data_func("/libnm/setting-8021x/pkcs12", "test-cert.p12, test", test_8021x); + g_test_add_func("/libnm/settings/test_setting_connection_empty_address_and_route", + test_setting_connection_empty_address_and_route); g_test_add_func("/libnm/settings/test_setting_connection_secondaries_verify", test_setting_connection_secondaries_verify);