diff options
author | Francesco Giudici <fgiudici@redhat.com> | 2017-09-14 15:06:03 +0200 |
---|---|---|
committer | Francesco Giudici <fgiudici@redhat.com> | 2017-09-14 17:13:14 +0200 |
commit | e965a96349c46ccd25a77e1a1410b8e6ad06aab3 (patch) | |
tree | b18b7d5c561817acbffdf3767cc6f8374fbe9f5d | |
parent | 8198a3abed0155bc8405058a31fddf2d9617b558 (diff) | |
download | NetworkManager-e965a96349c46ccd25a77e1a1410b8e6ad06aab3.tar.gz |
dhcp: dhclient: remove the --timeout argument from the command linefg/dhcp_timeout_rh1491243
the --timeout command line option is a custom feature added in some
linux distributions (fedora). Passing that command line argument will
make dhclient fail if the binary does not support it, preventing
activation of dhcp based connections.
Worse, the option has just been recently changed from "-timeout", so
that we are currently incompatibile with Centos, RedHat and older
versions of Fedora too.
Leverage the "timeout" option in dhclient config file: it will produce
the expected behavior and will be universally supported.
Fixes test: dhcp-timeout
Fixes: fa46736013fa1e3df1508b1f67b495ce45daf94a
https://bugzilla.redhat.com/show_bug.cgi?id=1491243
-rw-r--r-- | src/dhcp/nm-dhcp-dhclient-utils.c | 24 | ||||
-rw-r--r-- | src/dhcp/nm-dhcp-dhclient-utils.h | 1 | ||||
-rw-r--r-- | src/dhcp/nm-dhcp-dhclient.c | 29 | ||||
-rw-r--r-- | src/dhcp/tests/test-dhcp-dhclient.c | 42 |
4 files changed, 57 insertions, 39 deletions
diff --git a/src/dhcp/nm-dhcp-dhclient-utils.c b/src/dhcp/nm-dhcp-dhclient-utils.c index 9ae355ece1..309dbc6cee 100644 --- a/src/dhcp/nm-dhcp-dhclient-utils.c +++ b/src/dhcp/nm-dhcp-dhclient-utils.c @@ -33,7 +33,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" @@ -261,6 +263,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, @@ -309,6 +312,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) @@ -374,6 +388,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 (addr_family == AF_INET) { add_ip4_config (new_contents, client_id, hostname, use_fqdn); add_request (reqs, "rfc3442-classless-static-routes"); diff --git a/src/dhcp/nm-dhcp-dhclient-utils.h b/src/dhcp/nm-dhcp-dhclient-utils.h index de69fb297c..a5dc9c2b62 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 39aaaabfc9..f04c1862b7 100644 --- a/src/dhcp/nm-dhcp-dhclient.c +++ b/src/dhcp/nm-dhcp-dhclient.c @@ -194,6 +194,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, @@ -215,7 +216,8 @@ merge_dhclient_config (NMDhcpDhclient *self, } } - new = nm_dhcp_dhclient_create_config (iface, addr_family, client_id, anycast_addr, hostname, use_fqdn, orig_path, orig, out_new_client_id); + new = nm_dhcp_dhclient_create_config (iface, addr_family, 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); @@ -303,6 +305,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) { @@ -323,7 +326,7 @@ create_dhclient_config (NMDhcpDhclient *self, error = NULL; success = merge_dhclient_config (self, addr_family, iface, new, 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); @@ -352,8 +355,6 @@ dhclient_start (NMDhcpClient *client, int addr_family; gboolean success; char *escaped, *preferred_leasefile_path = NULL; - guint32 timeout; - char timeout_str[64]; g_return_val_if_fail (priv->pid_file == NULL, FALSE); @@ -456,17 +457,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. @@ -516,6 +506,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; @@ -523,10 +514,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, AF_INET, iface, 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); @@ -549,12 +541,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, AF_INET6, iface, uuid, NULL, dhcp_anycast_addr, hostname, TRUE, NULL); + priv->conf_file = create_dhclient_config (self, AF_INET6, iface, 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 879aa317e7..a338c13394 100644 --- a/src/dhcp/tests/test-dhcp-dhclient.c +++ b/src/dhcp/tests/test-dhcp-dhclient.c @@ -44,6 +44,7 @@ test_config (const char *orig, const char *expected, int addr_family, const char *hostname, + guint32 timeout, gboolean use_fqdn, const char *dhcp_client_id, GBytes *expected_new_client_id, @@ -64,6 +65,7 @@ test_config (const char *orig, client_id, anycast_addr, hostname, + timeout, use_fqdn, "/path/to/dhclient.conf", orig, @@ -108,7 +110,7 @@ static const char *orig_missing_expected = \ static void test_orig_missing (void) { - test_config (NULL, orig_missing_expected, AF_INET, NULL, FALSE, NULL, NULL, "eth0", NULL); + test_config (NULL, orig_missing_expected, AF_INET, NULL, 0, FALSE, NULL, NULL, "eth0", NULL); } /*****************************************************************************/ @@ -137,7 +139,7 @@ static void test_override_client_id (void) { test_config (override_client_id_orig, override_client_id_expected, - AF_INET, NULL, FALSE, + AF_INET, NULL, 0, FALSE, "11:22:33:44:55:66", NULL, "eth0", @@ -166,7 +168,7 @@ static void test_quote_client_id (void) { test_config (NULL, quote_client_id_expected, - AF_INET, NULL, FALSE, + AF_INET, NULL, 0, FALSE, "1234", NULL, "eth0", @@ -195,7 +197,7 @@ static void test_ascii_client_id (void) { test_config (NULL, ascii_client_id_expected, - AF_INET, NULL, FALSE, + AF_INET, NULL, 0, FALSE, "qb:cd:ef:12:34:56", NULL, "eth0", @@ -224,7 +226,7 @@ static void test_hex_single_client_id (void) { test_config (NULL, hex_single_client_id_expected, - AF_INET, NULL, FALSE, + AF_INET, NULL, 0, FALSE, "ab:cd:e:12:34:56", NULL, "eth0", @@ -261,7 +263,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, - AF_INET, NULL, FALSE, + AF_INET, NULL, 0, FALSE, NULL, new_client_id, "eth0", @@ -301,7 +303,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, - AF_INET, NULL, FALSE, + AF_INET, NULL, 0, FALSE, NULL, new_client_id, "eth0", @@ -330,7 +332,7 @@ static void test_fqdn (void) { test_config (NULL, fqdn_expected, - AF_INET, "foo.bar.com", + AF_INET, "foo.bar.com", 0, TRUE, NULL, NULL, "eth0", @@ -370,7 +372,7 @@ test_fqdn_options_override (void) { test_config (fqdn_options_override_orig, fqdn_options_override_expected, - AF_INET, "example2.com", + AF_INET, "example2.com", 0, TRUE, NULL, NULL, "eth0", @@ -403,7 +405,7 @@ static void test_override_hostname (void) { test_config (override_hostname_orig, override_hostname_expected, - AF_INET, "blahblah", FALSE, + AF_INET, "blahblah", 0, FALSE, NULL, NULL, "eth0", @@ -431,7 +433,7 @@ static void test_override_hostname6 (void) { test_config (override_hostname6_orig, override_hostname6_expected, - AF_INET6, "blahblah.local", TRUE, + AF_INET6, "blahblah.local", 0, TRUE, NULL, NULL, "eth0", @@ -456,8 +458,8 @@ test_nonfqdn_hostname6 (void) { /* Non-FQDN hostname can now be used with dhclient */ test_config (NULL, nonfqdn_hostname6_expected, - AF_INET6, "blahblah", - TRUE, NULL, + AF_INET6, "blahblah", 0, TRUE, + NULL, NULL, "eth0", NULL); @@ -491,8 +493,7 @@ static void test_existing_alsoreq (void) { test_config (existing_alsoreq_orig, existing_alsoreq_expected, - AF_INET, NULL, - FALSE, + AF_INET, NULL, 0, FALSE, NULL, NULL, "eth0", @@ -530,8 +531,7 @@ static void test_existing_req (void) { test_config (existing_req_orig, existing_req_expected, - AF_INET, NULL, - FALSE, + AF_INET, NULL, 0, FALSE, NULL, NULL, "eth0", @@ -570,7 +570,7 @@ static void test_existing_multiline_alsoreq (void) { test_config (existing_multiline_alsoreq_orig, existing_multiline_alsoreq_expected, - AF_INET, NULL, FALSE, + AF_INET, NULL, 0, FALSE, NULL, NULL, "eth0", @@ -784,7 +784,7 @@ static void test_interface1 (void) { test_config (interface1_orig, interface1_expected, - AF_INET, NULL, FALSE, + AF_INET, NULL, 0, FALSE, NULL, NULL, "eth0", @@ -829,7 +829,7 @@ static void test_interface2 (void) { test_config (interface2_orig, interface2_expected, - AF_INET, NULL, FALSE, + AF_INET, NULL, 0, FALSE, NULL, NULL, "eth1", @@ -883,7 +883,7 @@ test_config_req_intf (void) "\n"; test_config (orig, expected, - AF_INET, NULL, FALSE, + AF_INET, NULL, 0, FALSE, NULL, NULL, "eth0", |