fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-03-05 04:10:36 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

dhcp: dhclient: remove the --timeout argument from the command line

Browse source 
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: fa46736013

https://bugzilla.redhat.com/show_bug.cgi?id=1491243
(cherry picked from commit 1cb4832f09)
This commit is contained in:
Francesco Giudici 2017-09-14 15:06:03 +02:00
parent b946aefe24
commit 5499dda250
4 changed files with 57 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
src/dhcp/nm-dhcp-dhclient-utils.c
View file

@ -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");

1
src/dhcp/nm-dhcp-dhclient-utils.h
View file

@ -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,

29
src/dhcp/nm-dhcp-dhclient.c
View file

@ -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;

42
src/dhcp/tests/test-dhcp-dhclient.c
View file

@ -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",