diff --git a/src/dhcp/nm-dhcp-dhclient-utils.c b/src/dhcp/nm-dhcp-dhclient-utils.c index 28c290cf94..6a1b686523 100644 --- a/src/dhcp/nm-dhcp-dhclient-utils.c +++ b/src/dhcp/nm-dhcp-dhclient-utils.c @@ -31,7 +31,9 @@ #include "platform/nm-platform.h" #include "NetworkManagerUtils.h" -#define CLIENTID_TAG "send dhcp-client-identifier" +#define TIMEOUT_TAG "timeout " +#define RETRY_TAG "retry " +#define CLIENTID_TAG "send dhcp-client-identifier" #define HOSTNAME4_TAG "send host-name" #define HOSTNAME4_FORMAT HOSTNAME4_TAG " \"%s\"; # added by NetworkManager" @@ -263,6 +265,7 @@ nm_dhcp_dhclient_create_config (const char *interface, GBytes *client_id, const char *anycast_addr, const char *hostname, + guint32 timeout, gboolean use_fqdn, const char *orig_path, const char *orig_contents, @@ -310,6 +313,17 @@ nm_dhcp_dhclient_create_config (const char *interface, if (intf[0] && !nm_streq (intf, interface)) continue; + /* Some timing parameters in dhclient should not be imported (timeout, retry). + * The retry parameter will be simply not used as we will exit on first failure. + * The timeout one instead may affect NetworkManager behavior: if the timeout + * elapses before dhcp-timeout dhclient will report failure and cause NM to + * fail the dhcp process before dhcp-timeout. So, always skip importing timeout + * as we will need to add one greater than dhcp-timeout. + */ + if ( !strncmp (p, TIMEOUT_TAG, strlen (TIMEOUT_TAG)) + || !strncmp (p, RETRY_TAG, strlen (RETRY_TAG))) + continue; + if (!strncmp (p, CLIENTID_TAG, strlen (CLIENTID_TAG))) { /* Override config file "dhcp-client-id" and use one from the connection */ if (client_id) @@ -375,6 +389,14 @@ nm_dhcp_dhclient_create_config (const char *interface, } else g_string_append_c (new_contents, '\n'); + /* ensure dhclient timeout is greater than dhcp-timeout: as dhclient timeout default value is + * 60 seconds, we need this only if dhcp-timeout is greater than 60. + */ + if (timeout >= 60) { + timeout = timeout < G_MAXINT32 ? timeout + 1 : G_MAXINT32; + g_string_append_printf (new_contents, "timeout %u;\n", timeout); + } + if (is_ip6) { add_hostname6 (new_contents, hostname); add_request (reqs, "dhcp6.name-servers"); diff --git a/src/dhcp/nm-dhcp-dhclient-utils.h b/src/dhcp/nm-dhcp-dhclient-utils.h index 994b1b9f02..2268890bb5 100644 --- a/src/dhcp/nm-dhcp-dhclient-utils.h +++ b/src/dhcp/nm-dhcp-dhclient-utils.h @@ -27,6 +27,7 @@ char *nm_dhcp_dhclient_create_config (const char *interface, GBytes *client_id, const char *anycast_addr, const char *hostname, + guint32 timeout, gboolean use_fqdn, const char *orig_path, const char *orig_contents, diff --git a/src/dhcp/nm-dhcp-dhclient.c b/src/dhcp/nm-dhcp-dhclient.c index 5a07d34766..f20158c64d 100644 --- a/src/dhcp/nm-dhcp-dhclient.c +++ b/src/dhcp/nm-dhcp-dhclient.c @@ -182,6 +182,7 @@ merge_dhclient_config (NMDhcpDhclient *self, GBytes *client_id, const char *anycast_addr, const char *hostname, + guint32 timeout, gboolean use_fqdn, const char *orig_path, GBytes **out_new_client_id, @@ -206,7 +207,8 @@ merge_dhclient_config (NMDhcpDhclient *self, if (is_ip6 && hostname && !strchr (hostname, '.')) _LOGW ("hostname is not a FQDN, it will be ignored"); - new = nm_dhcp_dhclient_create_config (iface, is_ip6, client_id, anycast_addr, hostname, use_fqdn, orig_path, orig, out_new_client_id); + new = nm_dhcp_dhclient_create_config (iface, is_ip6, client_id, anycast_addr, hostname, timeout, + use_fqdn, orig_path, orig, out_new_client_id); g_assert (new); success = g_file_set_contents (conf_file, new, -1, error); g_free (new); @@ -294,6 +296,7 @@ create_dhclient_config (NMDhcpDhclient *self, GBytes *client_id, const char *dhcp_anycast_addr, const char *hostname, + guint32 timeout, gboolean use_fqdn, GBytes **out_new_client_id) { @@ -314,7 +317,7 @@ create_dhclient_config (NMDhcpDhclient *self, error = NULL; success = merge_dhclient_config (self, iface, new, is_ip6, client_id, dhcp_anycast_addr, - hostname, use_fqdn, orig, out_new_client_id, &error); + hostname, timeout, use_fqdn, orig, out_new_client_id, &error); if (!success) { _LOGW ("error creating dhclient configuration: %s", error->message); g_error_free (error); @@ -342,8 +345,6 @@ dhclient_start (NMDhcpClient *client, char *binary_name, *cmd_str, *pid_file = NULL, *system_bus_address_env = NULL; gboolean ipv6, success; char *escaped, *preferred_leasefile_path = NULL; - guint32 timeout; - char timeout_str[64]; g_return_val_if_fail (priv->pid_file == NULL, FALSE); @@ -446,17 +447,6 @@ dhclient_start (NMDhcpClient *client, g_ptr_array_add (argv, (gpointer) priv->conf_file); } - /* Specify a timeout longer than configuration's one, - * so that dhclient doesn't send back a FAIL event before - * that time. - */ - timeout = nm_dhcp_client_get_timeout (client); - if (timeout >= 60) { - timeout = timeout < G_MAXINT32 ? timeout + 1 : G_MAXINT32; - g_ptr_array_add (argv, (gpointer) "--timeout"); - g_ptr_array_add (argv, (gpointer) nm_sprintf_buf (timeout_str, "%u", (unsigned) timeout)); - } - /* Usually the system bus address is well-known; but if it's supposed * to be something else, we need to push it to dhclient, since dhclient * sanitizes the environment it gives the action scripts. @@ -506,6 +496,7 @@ ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last GBytes *client_id; gs_unref_bytes GBytes *new_client_id = NULL; const char *iface, *uuid, *hostname; + guint32 timeout; gboolean success = FALSE; gboolean use_fqdn; @@ -513,10 +504,11 @@ ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last uuid = nm_dhcp_client_get_uuid (client); client_id = nm_dhcp_client_get_client_id (client); hostname = nm_dhcp_client_get_hostname (client); + timeout = nm_dhcp_client_get_timeout (client); use_fqdn = nm_dhcp_client_get_use_fqdn (client); priv->conf_file = create_dhclient_config (self, iface, FALSE, uuid, client_id, dhcp_anycast_addr, - hostname, use_fqdn, &new_client_id); + hostname, timeout, use_fqdn, &new_client_id); if (priv->conf_file) { if (new_client_id) nm_dhcp_client_set_client_id (client, new_client_id); @@ -539,12 +531,15 @@ ip6_start (NMDhcpClient *client, NMDhcpDhclient *self = NM_DHCP_DHCLIENT (client); NMDhcpDhclientPrivate *priv = NM_DHCP_DHCLIENT_GET_PRIVATE (self); const char *iface, *uuid, *hostname; + guint32 timeout; iface = nm_dhcp_client_get_iface (client); uuid = nm_dhcp_client_get_uuid (client); hostname = nm_dhcp_client_get_hostname (client); + timeout = nm_dhcp_client_get_timeout (client); - priv->conf_file = create_dhclient_config (self, iface, TRUE, uuid, NULL, dhcp_anycast_addr, hostname, TRUE, NULL); + priv->conf_file = create_dhclient_config (self, iface, TRUE, uuid, NULL, dhcp_anycast_addr, + hostname, timeout, TRUE, NULL); if (!priv->conf_file) { _LOGW ("error creating dhclient configuration file"); return FALSE; diff --git a/src/dhcp/tests/test-dhcp-dhclient.c b/src/dhcp/tests/test-dhcp-dhclient.c index 3c9760ad67..5816932b3a 100644 --- a/src/dhcp/tests/test-dhcp-dhclient.c +++ b/src/dhcp/tests/test-dhcp-dhclient.c @@ -40,6 +40,7 @@ test_config (const char *orig, const char *expected, gboolean ipv6, const char *hostname, + guint32 timeout, gboolean use_fqdn, const char *dhcp_client_id, GBytes *expected_new_client_id, @@ -60,6 +61,7 @@ test_config (const char *orig, client_id, anycast_addr, hostname, + timeout, use_fqdn, "/path/to/dhclient.conf", orig, @@ -104,7 +106,7 @@ static const char *orig_missing_expected = \ static void test_orig_missing (void) { - test_config (NULL, orig_missing_expected, FALSE, NULL, FALSE, NULL, NULL, "eth0", NULL); + test_config (NULL, orig_missing_expected, FALSE, NULL, 0, FALSE, NULL, NULL, "eth0", NULL); } /*****************************************************************************/ @@ -133,7 +135,7 @@ static void test_override_client_id (void) { test_config (override_client_id_orig, override_client_id_expected, - FALSE, NULL, FALSE, + FALSE, NULL, 0, FALSE, "11:22:33:44:55:66", NULL, "eth0", @@ -162,7 +164,7 @@ static void test_quote_client_id (void) { test_config (NULL, quote_client_id_expected, - FALSE, NULL, FALSE, + FALSE, NULL, 0, FALSE, "1234", NULL, "eth0", @@ -191,7 +193,7 @@ static void test_ascii_client_id (void) { test_config (NULL, ascii_client_id_expected, - FALSE, NULL, FALSE, + FALSE, NULL, 0, FALSE, "qb:cd:ef:12:34:56", NULL, "eth0", @@ -220,7 +222,7 @@ static void test_hex_single_client_id (void) { test_config (NULL, hex_single_client_id_expected, - FALSE, NULL, FALSE, + FALSE, NULL, 0, FALSE, "ab:cd:e:12:34:56", NULL, "eth0", @@ -257,7 +259,7 @@ test_existing_hex_client_id (void) new_client_id = g_bytes_new (bytes, sizeof (bytes)); test_config (existing_hex_client_id_orig, existing_hex_client_id_expected, - FALSE, NULL, FALSE, + FALSE, NULL, 0, FALSE, NULL, new_client_id, "eth0", @@ -297,7 +299,7 @@ test_existing_ascii_client_id (void) memcpy (buf + 1, EACID, NM_STRLEN (EACID)); new_client_id = g_bytes_new (buf, sizeof (buf)); test_config (existing_ascii_client_id_orig, existing_ascii_client_id_expected, - FALSE, NULL, FALSE, + FALSE, NULL, 0, FALSE, NULL, new_client_id, "eth0", @@ -326,7 +328,7 @@ static void test_fqdn (void) { test_config (NULL, fqdn_expected, - FALSE, "foo.bar.com", + FALSE, "foo.bar.com", 0, TRUE, NULL, NULL, "eth0", @@ -366,7 +368,7 @@ test_fqdn_options_override (void) { test_config (fqdn_options_override_orig, fqdn_options_override_expected, - FALSE, "example2.com", + FALSE, "example2.com", 0, TRUE, NULL, NULL, "eth0", @@ -399,7 +401,7 @@ static void test_override_hostname (void) { test_config (override_hostname_orig, override_hostname_expected, - FALSE, "blahblah", FALSE, + FALSE, "blahblah", 0, FALSE, NULL, NULL, "eth0", @@ -428,7 +430,7 @@ static void test_override_hostname6 (void) { test_config (override_hostname6_orig, override_hostname6_expected, - TRUE, "blahblah.local", TRUE, + TRUE, "blahblah.local", 0, TRUE, NULL, NULL, "eth0", @@ -450,8 +452,8 @@ test_nonfqdn_hostname6 (void) { /* Non-FQDN hostname can't be used with dhclient */ test_config (NULL, nonfqdn_hostname6_expected, - TRUE, "blahblah", - TRUE, NULL, + TRUE, "blahblah", 0, TRUE, + NULL, NULL, "eth0", NULL); @@ -485,8 +487,7 @@ static void test_existing_alsoreq (void) { test_config (existing_alsoreq_orig, existing_alsoreq_expected, - FALSE, NULL, - FALSE, + FALSE, NULL, 0, FALSE, NULL, NULL, "eth0", @@ -524,8 +525,7 @@ static void test_existing_req (void) { test_config (existing_req_orig, existing_req_expected, - FALSE, NULL, - FALSE, + FALSE, NULL, 0, FALSE, NULL, NULL, "eth0", @@ -564,7 +564,7 @@ static void test_existing_multiline_alsoreq (void) { test_config (existing_multiline_alsoreq_orig, existing_multiline_alsoreq_expected, - FALSE, NULL, FALSE, + FALSE, NULL, 0, FALSE, NULL, NULL, "eth0", @@ -778,7 +778,7 @@ static void test_interface1 (void) { test_config (interface1_orig, interface1_expected, - FALSE, NULL, FALSE, + FALSE, NULL, 0, FALSE, NULL, NULL, "eth0", @@ -823,7 +823,7 @@ static void test_interface2 (void) { test_config (interface2_orig, interface2_expected, - FALSE, NULL, FALSE, + FALSE, NULL, 0, FALSE, NULL, NULL, "eth1", @@ -877,7 +877,7 @@ test_config_req_intf (void) "\n"; test_config (orig, expected, - FALSE, NULL, FALSE, + FALSE, NULL, 0, FALSE, NULL, NULL, "eth0",