From 936c7c9a78a3ff03f2681a2017dda509e8072f84 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 12 Feb 2021 09:50:48 +0100 Subject: [PATCH 1/3] initrd: set autoconnect-retries=1 and increase default DHCP timeout By default a connection is retried 4 times before it is blocked from autoconnecting. This means that if a user specifies an explicit DHCP timeout in the initrd command line, NM will wait up to 4 times more. Instead, set the "connection.autoconnect-retries" property of connections always to 1, so that NM only waits for the time specified. Before this commit a default DHCP connection would take at most (45 x 4) seconds. Since the multiplier is now only 1, also increase the DHCP timeout to have a total time of (90 x 1) seconds, which is the half than before. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/559 (cherry picked from commit 7e126fe898f130f53f3e5cb2f87eca2169978b4d) --- src/core/initrd/nmi-cmdline-reader.c | 6 ++- src/core/initrd/tests/test-cmdline-reader.c | 59 +++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/core/initrd/nmi-cmdline-reader.c b/src/core/initrd/nmi-cmdline-reader.c index d95b0f9bef..7773b30f11 100644 --- a/src/core/initrd/nmi-cmdline-reader.c +++ b/src/core/initrd/nmi-cmdline-reader.c @@ -53,6 +53,7 @@ reader_new(void) g_hash_table_new_full(nm_direct_hash, NULL, g_object_unref, NULL), .vlan_parents = g_ptr_array_new_with_free_func(g_free), .array = g_ptr_array_new(), + .dhcp_timeout = 90, }; return reader; @@ -147,6 +148,8 @@ reader_create_connection(Reader * reader, type_name, NM_SETTING_CONNECTION_MULTI_CONNECT, multi_connect, + NM_SETTING_CONNECTION_AUTOCONNECT_RETRIES, + 1, NULL); if (nm_streq0(type_name, NM_SETTING_INFINIBAND_SETTING_NAME)) { @@ -1082,7 +1085,8 @@ nmi_cmdline_reader_parse(const char * sysfs_dir, else if (nm_streq(tag, "rd.peerdns")) reader->ignore_auto_dns = !_nm_utils_ascii_str_to_bool(argument, TRUE); else if (nm_streq(tag, "rd.net.timeout.dhcp")) { - reader->dhcp_timeout = _nm_utils_ascii_str_to_int64(argument, 10, 0, G_MAXINT32, 0); + reader->dhcp_timeout = + _nm_utils_ascii_str_to_int64(argument, 10, 1, G_MAXINT32, reader->dhcp_timeout); } else if (nm_streq(tag, "rd.net.dhcp.vendor-class")) { if (nm_utils_validate_dhcp4_vendor_class_id(argument, NULL)) nm_utils_strdup_reset(&reader->dhcp4_vci, argument); diff --git a/src/core/initrd/tests/test-cmdline-reader.c b/src/core/initrd/tests/test-cmdline-reader.c index cb65cd33b1..6f3dd1a504 100644 --- a/src/core/initrd/tests/test-cmdline-reader.c +++ b/src/core/initrd/tests/test-cmdline-reader.c @@ -229,11 +229,64 @@ test_dhcp_with_mtu(void) } } +static void +test_dhcp_timeout(void) +{ + struct { + const char *const *cmdline; + int timeout; + } data[] = { + {NM_MAKE_STRV("ip=dhcp"), 90}, + {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=0"), 90}, + {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=foobar"), 90}, + {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=42"), 42}, + }; + guint i; + + for (i = 0; i < G_N_ELEMENTS(data); i++) { + gs_unref_object NMConnection *connection = NULL; + NMSettingConnection * s_con; + NMSettingIPConfig * s_ip4; + NMSettingIPConfig * s_ip6; + + connection = _parse_con(data[i].cmdline, "default_connection"); + + s_con = nm_connection_get_setting_connection(connection); + g_assert(s_con); + g_assert_cmpstr(nm_setting_connection_get_connection_type(s_con), + ==, + NM_SETTING_WIRED_SETTING_NAME); + g_assert_cmpstr(nm_setting_connection_get_id(s_con), ==, "Wired Connection"); + g_assert_cmpint(nm_setting_connection_get_timestamp(s_con), ==, 0); + g_assert_cmpint(nm_setting_connection_get_multi_connect(s_con), + ==, + NM_CONNECTION_MULTI_CONNECT_MULTIPLE); + g_assert_cmpint(nm_setting_connection_get_wait_device_timeout(s_con), ==, -1); + g_assert_cmpint(nm_setting_connection_get_autoconnect_retries(s_con), ==, 1); + g_assert(nm_setting_connection_get_autoconnect(s_con)); + + s_ip4 = nm_connection_get_setting_ip4_config(connection); + g_assert(s_ip4); + g_assert_cmpstr(nm_setting_ip_config_get_method(s_ip4), + ==, + NM_SETTING_IP4_CONFIG_METHOD_AUTO); + g_assert_cmpint(nm_setting_ip_config_get_dhcp_timeout(s_ip4), ==, data[i].timeout); + + s_ip6 = nm_connection_get_setting_ip6_config(connection); + g_assert(s_ip6); + g_assert_cmpstr(nm_setting_ip_config_get_method(s_ip6), + ==, + NM_SETTING_IP6_CONFIG_METHOD_AUTO); + g_assert_cmpint(nm_setting_ip_config_get_dhcp_timeout(s_ip6), ==, data[i].timeout); + } +} + static void test_if_auto_with_mtu(void) { const char *const *ARGV = NM_MAKE_STRV("ip=eth0:auto:1666"); gs_unref_object NMConnection *connection = NULL; + NMSettingConnection * s_con; NMSettingWired * s_wired; NMSettingIPConfig * s_ip4; NMSettingIPConfig * s_ip6; @@ -242,6 +295,10 @@ test_if_auto_with_mtu(void) g_assert_cmpstr(nm_connection_get_id(connection), ==, "eth0"); + s_con = nm_connection_get_setting_connection(connection); + g_assert(s_con); + g_assert_cmpint(nm_setting_connection_get_autoconnect_retries(s_con), ==, 1); + s_wired = nm_connection_get_setting_wired(connection); g_assert(s_wired); g_assert_cmpint(nm_setting_wired_get_mtu(s_wired), ==, 1666); @@ -250,6 +307,7 @@ test_if_auto_with_mtu(void) g_assert(s_ip4); g_assert_cmpstr(nm_setting_ip_config_get_method(s_ip4), ==, NM_SETTING_IP4_CONFIG_METHOD_AUTO); g_assert(!nm_setting_ip_config_get_ignore_auto_dns(s_ip4)); + g_assert_cmpint(nm_setting_ip_config_get_dhcp_timeout(s_ip4), ==, 90); s_ip6 = nm_connection_get_setting_ip6_config(connection); g_assert(s_ip6); @@ -2074,6 +2132,7 @@ main(int argc, char **argv) g_test_add_func("/initrd/cmdline/auto", test_auto); g_test_add_func("/initrd/cmdline/dhcp_with_hostname", test_dhcp_with_hostname); g_test_add_func("/initrd/cmdline/dhcp_with_mtu", test_dhcp_with_mtu); + g_test_add_func("/initrd/cmdline/dhcp_timeout", test_dhcp_timeout); g_test_add_func("/initrd/cmdline/if_auto_with_mtu", test_if_auto_with_mtu); g_test_add_func("/initrd/cmdline/if_dhcp6", test_if_dhcp6); g_test_add_func("/initrd/cmdline/if_auto_with_mtu_and_mac", test_if_auto_with_mtu_and_mac); From 772b2133211b956c0c39a69f3d5c32a28fb87cf9 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 12 Feb 2021 09:53:30 +0100 Subject: [PATCH 2/3] initrd: accept 'infinity' as argument to rd.net.timeout.dhcp (cherry picked from commit 97833237bf38347c75022eb380208d99e1df9d5f) --- src/core/initrd/nmi-cmdline-reader.c | 8 ++++++-- src/core/initrd/tests/test-cmdline-reader.c | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/core/initrd/nmi-cmdline-reader.c b/src/core/initrd/nmi-cmdline-reader.c index 7773b30f11..3e636b7acd 100644 --- a/src/core/initrd/nmi-cmdline-reader.c +++ b/src/core/initrd/nmi-cmdline-reader.c @@ -1085,8 +1085,12 @@ nmi_cmdline_reader_parse(const char * sysfs_dir, else if (nm_streq(tag, "rd.peerdns")) reader->ignore_auto_dns = !_nm_utils_ascii_str_to_bool(argument, TRUE); else if (nm_streq(tag, "rd.net.timeout.dhcp")) { - reader->dhcp_timeout = - _nm_utils_ascii_str_to_int64(argument, 10, 1, G_MAXINT32, reader->dhcp_timeout); + if (nm_streq0(argument, "infinity")) { + reader->dhcp_timeout = G_MAXINT32; + } else { + reader->dhcp_timeout = + _nm_utils_ascii_str_to_int64(argument, 10, 1, G_MAXINT32, reader->dhcp_timeout); + } } else if (nm_streq(tag, "rd.net.dhcp.vendor-class")) { if (nm_utils_validate_dhcp4_vendor_class_id(argument, NULL)) nm_utils_strdup_reset(&reader->dhcp4_vci, argument); diff --git a/src/core/initrd/tests/test-cmdline-reader.c b/src/core/initrd/tests/test-cmdline-reader.c index 6f3dd1a504..6795e32d37 100644 --- a/src/core/initrd/tests/test-cmdline-reader.c +++ b/src/core/initrd/tests/test-cmdline-reader.c @@ -240,6 +240,7 @@ test_dhcp_timeout(void) {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=0"), 90}, {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=foobar"), 90}, {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=42"), 42}, + {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=infinity"), G_MAXINT32}, }; guint i; From 329902339e66998f36d62d0f3a7ea99e4be21843 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 12 Feb 2021 09:56:36 +0100 Subject: [PATCH 3/3] initrd: support the rd.net.dhcp.retry argument Since we always set autoconnect-retries=1, use the value of rd.net.dhcp.retry as a multiplier for the DHCP timeout. (cherry picked from commit 099ce63888013e82c0f369b02a8f27a0b31813a6) --- src/core/initrd/nmi-cmdline-reader.c | 14 ++++++++++---- src/core/initrd/tests/test-cmdline-reader.c | 4 ++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/core/initrd/nmi-cmdline-reader.c b/src/core/initrd/nmi-cmdline-reader.c index 3e636b7acd..508ef2b25c 100644 --- a/src/core/initrd/nmi-cmdline-reader.c +++ b/src/core/initrd/nmi-cmdline-reader.c @@ -53,7 +53,6 @@ reader_new(void) g_hash_table_new_full(nm_direct_hash, NULL, g_object_unref, NULL), .vlan_parents = g_ptr_array_new_with_free_func(g_free), .array = g_ptr_array_new(), - .dhcp_timeout = 90, }; return reader; @@ -1068,6 +1067,8 @@ nmi_cmdline_reader_parse(const char * sysfs_dir, gs_unref_ptrarray GPtrArray *routes = NULL; gs_unref_ptrarray GPtrArray *znets = NULL; int i; + guint64 dhcp_timeout = 90; + guint64 dhcp_num_tries = 1; reader = reader_new(); @@ -1086,11 +1087,14 @@ nmi_cmdline_reader_parse(const char * sysfs_dir, reader->ignore_auto_dns = !_nm_utils_ascii_str_to_bool(argument, TRUE); else if (nm_streq(tag, "rd.net.timeout.dhcp")) { if (nm_streq0(argument, "infinity")) { - reader->dhcp_timeout = G_MAXINT32; + dhcp_timeout = G_MAXINT32; } else { - reader->dhcp_timeout = - _nm_utils_ascii_str_to_int64(argument, 10, 1, G_MAXINT32, reader->dhcp_timeout); + dhcp_timeout = + _nm_utils_ascii_str_to_int64(argument, 10, 1, G_MAXINT32, dhcp_timeout); } + } else if (nm_streq(tag, "rd.net.dhcp.retry")) { + dhcp_num_tries = + _nm_utils_ascii_str_to_int64(argument, 10, 1, G_MAXINT32, dhcp_num_tries); } else if (nm_streq(tag, "rd.net.dhcp.vendor-class")) { if (nm_utils_validate_dhcp4_vendor_class_id(argument, NULL)) nm_utils_strdup_reset(&reader->dhcp4_vci, argument); @@ -1100,6 +1104,8 @@ nmi_cmdline_reader_parse(const char * sysfs_dir, } } + reader->dhcp_timeout = NM_CLAMP(dhcp_timeout * dhcp_num_tries, 1, G_MAXINT32); + for (i = 0; argv[i]; i++) { gs_free char *argument_clone = NULL; char * argument; diff --git a/src/core/initrd/tests/test-cmdline-reader.c b/src/core/initrd/tests/test-cmdline-reader.c index 6795e32d37..33fb22d364 100644 --- a/src/core/initrd/tests/test-cmdline-reader.c +++ b/src/core/initrd/tests/test-cmdline-reader.c @@ -240,7 +240,11 @@ test_dhcp_timeout(void) {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=0"), 90}, {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=foobar"), 90}, {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=42"), 42}, + {NM_MAKE_STRV("ip=dhcp", "rd.net.dhcp.retry=2"), 180}, + {NM_MAKE_STRV("ip=dhcp", "rd.net.dhcp.retry=3", "rd.net.timeout.dhcp=40"), 120}, {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=infinity"), G_MAXINT32}, + {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=infinity", "rd.net.dhcp.retry=100"), + G_MAXINT32}, }; guint i;