From 30798e0af42ebb3f13c06b2791ddba4bc62865e1 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 19 Dec 2019 10:13:38 +0100 Subject: [PATCH 1/7] n-dhcp4: fix logging broadcast messages Log the broadcast address instead of the server IP as destination when needed. --- shared/n-dhcp4/src/n-dhcp4-c-connection.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/shared/n-dhcp4/src/n-dhcp4-c-connection.c b/shared/n-dhcp4/src/n-dhcp4-c-connection.c index 8a2ddb3636..66c252b781 100644 --- a/shared/n-dhcp4/src/n-dhcp4-c-connection.c +++ b/shared/n-dhcp4/src/n-dhcp4-c-connection.c @@ -992,6 +992,7 @@ static int n_dhcp4_c_connection_send_request(NDhcp4CConnection *connection, char server_addr[INET_ADDRSTRLEN]; char client_addr[INET_ADDRSTRLEN]; int r; + bool broadcast = false; /* * Increment the base time and reset the xid field, @@ -1026,12 +1027,14 @@ static int n_dhcp4_c_connection_send_request(NDhcp4CConnection *connection, case N_DHCP4_C_MESSAGE_SELECT: case N_DHCP4_C_MESSAGE_REBOOT: case N_DHCP4_C_MESSAGE_DECLINE: + broadcast = true; r = n_dhcp4_c_connection_packet_broadcast(connection, request); if (r) return r; break; case N_DHCP4_C_MESSAGE_INFORM: case N_DHCP4_C_MESSAGE_REBIND: + broadcast = true; r = n_dhcp4_c_connection_udp_broadcast(connection, request); if (r) return r; @@ -1052,6 +1055,8 @@ static int n_dhcp4_c_connection_send_request(NDhcp4CConnection *connection, n_dhcp4_c_log(connection->client_config, LOG_INFO, "sent %s to %s", message_type_to_str(request->userdata.message_type), + broadcast ? + "255.255.255.255" : inet_ntop(AF_INET, &connection->server_ip, server_addr, sizeof(server_addr))); } else { @@ -1060,6 +1065,8 @@ static int n_dhcp4_c_connection_send_request(NDhcp4CConnection *connection, message_type_to_str(request->userdata.message_type), inet_ntop(AF_INET, &request->userdata.client_addr, client_addr, sizeof(client_addr)), + broadcast ? + "255.255.255.255" : inet_ntop(AF_INET, &connection->server_ip, server_addr, sizeof(server_addr))); } From af03b779804f8c0b7efff548691921ae8c29c77a Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 18 Dec 2019 14:51:14 +0100 Subject: [PATCH 2/7] n-dhcp4: support init-reboot state Currently the client always starts from the INIT state (i.e. sending a discover message). If a requested-ip was specified by the caller, it is added as an option in the discover. It was reported that some DHCP servers don't respond to discover messages with the requested-ip option set [1][2]. The RFC allows to skip the discover by entering the INIT-REBOOT state and starting directly with a broadcast request message containing the requested IP address. Implement that. [1] https://bugzilla.redhat.com/show_bug.cgi?id=1781856 [2] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/310 --- shared/n-dhcp4/src/n-dhcp4-c-connection.c | 2 +- shared/n-dhcp4/src/n-dhcp4-c-probe.c | 84 +++++++++++++++++++++-- shared/n-dhcp4/src/n-dhcp4-private.h | 1 + 3 files changed, 80 insertions(+), 7 deletions(-) diff --git a/shared/n-dhcp4/src/n-dhcp4-c-connection.c b/shared/n-dhcp4/src/n-dhcp4-c-connection.c index 66c252b781..29dbc27a42 100644 --- a/shared/n-dhcp4/src/n-dhcp4-c-connection.c +++ b/shared/n-dhcp4/src/n-dhcp4-c-connection.c @@ -319,7 +319,6 @@ void n_dhcp4_c_connection_get_timeout(NDhcp4CConnection *connection, switch (connection->request->userdata.type) { case N_DHCP4_C_MESSAGE_DISCOVER: case N_DHCP4_C_MESSAGE_SELECT: - case N_DHCP4_C_MESSAGE_REBOOT: case N_DHCP4_C_MESSAGE_INFORM: /* * Resend with an exponential backoff and a one second random @@ -338,6 +337,7 @@ void n_dhcp4_c_connection_get_timeout(NDhcp4CConnection *connection, break; case N_DHCP4_C_MESSAGE_REBIND: case N_DHCP4_C_MESSAGE_RENEW: + case N_DHCP4_C_MESSAGE_REBOOT: /* * Resend every sixty seconds with a one second random slack. * diff --git a/shared/n-dhcp4/src/n-dhcp4-c-probe.c b/shared/n-dhcp4/src/n-dhcp4-c-probe.c index 2bced4cf8a..1810df1e8d 100644 --- a/shared/n-dhcp4/src/n-dhcp4-c-probe.c +++ b/shared/n-dhcp4/src/n-dhcp4-c-probe.c @@ -170,8 +170,6 @@ _c_public_ void n_dhcp4_client_probe_config_set_inform_only(NDhcp4ClientProbeCon * INIT-REBOOT path, as described by the DHCP specification. In most cases, you * do not want this. * - * XXX: This is currently not implemented, and setting the property has no effect. - * * Background: The INIT-REBOOT path allows a DHCP client to skip * server-discovery when rebooting/resuming their machine. The DHCP * client simply re-requests the lease it had acquired before. This @@ -438,11 +436,17 @@ int n_dhcp4_client_probe_new(NDhcp4ClientProbe **probep, if (r) return r; + if (probe->config->init_reboot && probe->config->requested_ip.s_addr != INADDR_ANY) + probe->state = N_DHCP4_CLIENT_PROBE_STATE_INIT_REBOOT; + else + probe->state = N_DHCP4_CLIENT_PROBE_STATE_INIT; + if (active) { /* * Defer the sending of DISCOVER by a random amount (by default up to 9 seconds). */ - probe->ns_deferred = ns_now + (n_dhcp4_client_probe_config_get_random(probe->config) % (probe->config->ms_start_delay * 1000000ULL)); + if (probe->state == N_DHCP4_CLIENT_PROBE_STATE_INIT) + probe->ns_deferred = ns_now + (n_dhcp4_client_probe_config_get_random(probe->config) % (probe->config->ms_start_delay * 1000000ULL)); probe->client->current_probe = probe; } else { r = n_dhcp4_client_probe_raise(probe, @@ -575,6 +579,14 @@ void n_dhcp4_client_probe_get_timeout(NDhcp4ClientProbe *probe, uint64_t *timeou n_dhcp4_c_connection_get_timeout(&probe->connection, &timeout); switch (probe->state) { + case N_DHCP4_CLIENT_PROBE_STATE_INIT_REBOOT: + /* send DHCP request immediately */ + timeout = 1; + break; + case N_DHCP4_CLIENT_PROBE_STATE_REBOOTING: + if (probe->ns_reinit && (!timeout || probe->ns_reinit < timeout)) + timeout = probe->ns_reinit; + break; case N_DHCP4_CLIENT_PROBE_STATE_INIT: if (probe->ns_deferred && (!timeout || probe->ns_deferred < timeout)) timeout = probe->ns_deferred; @@ -626,6 +638,52 @@ static int n_dhcp4_client_probe_outgoing_append_options(NDhcp4ClientProbe *probe return 0; } +static int n_dhcp4_client_probe_transition_reboot(NDhcp4ClientProbe *probe, uint64_t ns_now) { + _c_cleanup_(n_dhcp4_outgoing_freep) NDhcp4Outgoing *request = NULL; + int r; + + switch (probe->state) { + case N_DHCP4_CLIENT_PROBE_STATE_INIT_REBOOT: + r = n_dhcp4_c_connection_listen(&probe->connection); + if (r) + return r; + + r = n_dhcp4_c_connection_reboot_new(&probe->connection, &request, &probe->config->requested_ip); + if (r) + return r; + + r = n_dhcp4_client_probe_outgoing_append_options(probe, request); + if (r) + return r; + + r = n_dhcp4_c_connection_start_request(&probe->connection, request, ns_now); + if (r) + return r; + else + request = NULL; /* consumed */ + + probe->state = N_DHCP4_CLIENT_PROBE_STATE_REBOOTING; + probe->ns_reinit = ns_now + 2000000000ULL; + + break; + + case N_DHCP4_CLIENT_PROBE_STATE_SELECTING: + case N_DHCP4_CLIENT_PROBE_STATE_INIT: + case N_DHCP4_CLIENT_PROBE_STATE_REBOOTING: + case N_DHCP4_CLIENT_PROBE_STATE_REQUESTING: + case N_DHCP4_CLIENT_PROBE_STATE_GRANTED: + case N_DHCP4_CLIENT_PROBE_STATE_BOUND: + case N_DHCP4_CLIENT_PROBE_STATE_RENEWING: + case N_DHCP4_CLIENT_PROBE_STATE_REBINDING: + case N_DHCP4_CLIENT_PROBE_STATE_EXPIRED: + default: + abort(); + break; + } + + return 0; +} + static int n_dhcp4_client_probe_transition_deferred(NDhcp4ClientProbe *probe, uint64_t ns_now) { _c_cleanup_(n_dhcp4_outgoing_freep) NDhcp4Outgoing *request = NULL; int r; @@ -635,12 +693,14 @@ static int n_dhcp4_client_probe_transition_deferred(NDhcp4ClientProbe *probe, ui r = n_dhcp4_c_connection_listen(&probe->connection); if (r) return r; + /* fall-through */ + case N_DHCP4_CLIENT_PROBE_STATE_REBOOTING: r = n_dhcp4_c_connection_discover_new(&probe->connection, &request); if (r) return r; - if (probe->config->requested_ip.s_addr != INADDR_ANY) { + if (!probe->config->init_reboot && probe->config->requested_ip.s_addr != INADDR_ANY) { r = n_dhcp4_outgoing_append_requested_ip(request, probe->config->requested_ip); if (r) return r; @@ -663,7 +723,6 @@ static int n_dhcp4_client_probe_transition_deferred(NDhcp4ClientProbe *probe, ui case N_DHCP4_CLIENT_PROBE_STATE_SELECTING: case N_DHCP4_CLIENT_PROBE_STATE_INIT_REBOOT: - case N_DHCP4_CLIENT_PROBE_STATE_REBOOTING: case N_DHCP4_CLIENT_PROBE_STATE_REQUESTING: case N_DHCP4_CLIENT_PROBE_STATE_GRANTED: case N_DHCP4_CLIENT_PROBE_STATE_BOUND: @@ -876,6 +935,7 @@ static int n_dhcp4_client_probe_transition_ack(NDhcp4ClientProbe *probe, NDhcp4I break; case N_DHCP4_CLIENT_PROBE_STATE_REQUESTING: + case N_DHCP4_CLIENT_PROBE_STATE_REBOOTING: r = n_dhcp4_client_probe_raise(probe, &node, @@ -900,7 +960,6 @@ static int n_dhcp4_client_probe_transition_ack(NDhcp4ClientProbe *probe, NDhcp4I case N_DHCP4_CLIENT_PROBE_STATE_INIT: case N_DHCP4_CLIENT_PROBE_STATE_SELECTING: case N_DHCP4_CLIENT_PROBE_STATE_INIT_REBOOT: - case N_DHCP4_CLIENT_PROBE_STATE_REBOOTING: case N_DHCP4_CLIENT_PROBE_STATE_BOUND: case N_DHCP4_CLIENT_PROBE_STATE_GRANTED: case N_DHCP4_CLIENT_PROBE_STATE_EXPIRED: @@ -1079,6 +1138,19 @@ int n_dhcp4_client_probe_dispatch_timer(NDhcp4ClientProbe *probe, uint64_t ns_no int r; switch (probe->state) { + case N_DHCP4_CLIENT_PROBE_STATE_INIT_REBOOT: + r = n_dhcp4_client_probe_transition_reboot(probe, ns_now); + if (r) + return r; + break; + case N_DHCP4_CLIENT_PROBE_STATE_REBOOTING: + if (ns_now >= probe->ns_reinit) { + r = n_dhcp4_client_probe_transition_deferred(probe, ns_now); + if (r) + return r; + } + + break; case N_DHCP4_CLIENT_PROBE_STATE_INIT: if (ns_now >= probe->ns_deferred) { r = n_dhcp4_client_probe_transition_deferred(probe, ns_now); diff --git a/shared/n-dhcp4/src/n-dhcp4-private.h b/shared/n-dhcp4/src/n-dhcp4-private.h index 1cf5f25e55..fcfb0f35b8 100644 --- a/shared/n-dhcp4/src/n-dhcp4-private.h +++ b/shared/n-dhcp4/src/n-dhcp4-private.h @@ -351,6 +351,7 @@ struct NDhcp4ClientProbe { unsigned int state; /* current probe state */ uint64_t ns_deferred; /* timeout for deferred action */ + uint64_t ns_reinit; NDhcp4ClientLease *current_lease; /* current lease */ NDhcp4CConnection connection; /* client connection wrapper */ From f860e929c04ef27ffb16eb7fb5456123a8595d2c Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 20 Dec 2019 09:48:45 +0100 Subject: [PATCH 3/7] n-dhcp4: use packet socket in rebinding state After t1, the client tries to renew the lease by contacting via the udp socket the server specified in the server-id option. If this fails, after t2 it tries to contact any server using broadcast. For this to work, the packet socket must be used. --- shared/n-dhcp4/src/n-dhcp4-c-connection.c | 16 ++++++++++++++-- shared/n-dhcp4/src/n-dhcp4-c-probe.c | 17 ++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/shared/n-dhcp4/src/n-dhcp4-c-connection.c b/shared/n-dhcp4/src/n-dhcp4-c-connection.c index 29dbc27a42..e51a3e3249 100644 --- a/shared/n-dhcp4/src/n-dhcp4-c-connection.c +++ b/shared/n-dhcp4/src/n-dhcp4-c-connection.c @@ -139,7 +139,19 @@ int n_dhcp4_c_connection_listen(NDhcp4CConnection *connection) { _c_cleanup_(c_closep) int fd_packet = -1; int r; - c_assert(connection->state == N_DHCP4_C_CONNECTION_STATE_INIT); + c_assert(connection->state == N_DHCP4_C_CONNECTION_STATE_INIT || + connection->state == N_DHCP4_C_CONNECTION_STATE_DRAINING || + connection->state == N_DHCP4_C_CONNECTION_STATE_UDP); + + if (connection->fd_packet >= 0) { + epoll_ctl(connection->fd_epoll, EPOLL_CTL_DEL, connection->fd_packet, NULL); + connection->fd_packet = c_close(connection->fd_packet); + } + + if (connection->fd_udp >= 0) { + epoll_ctl(connection->fd_epoll, EPOLL_CTL_DEL, connection->fd_udp, NULL); + connection->fd_udp = c_close(connection->fd_udp); + } r = n_dhcp4_c_socket_packet_new(&fd_packet, connection->client_config->ifindex); if (r) @@ -1027,13 +1039,13 @@ static int n_dhcp4_c_connection_send_request(NDhcp4CConnection *connection, case N_DHCP4_C_MESSAGE_SELECT: case N_DHCP4_C_MESSAGE_REBOOT: case N_DHCP4_C_MESSAGE_DECLINE: + case N_DHCP4_C_MESSAGE_REBIND: broadcast = true; r = n_dhcp4_c_connection_packet_broadcast(connection, request); if (r) return r; break; case N_DHCP4_C_MESSAGE_INFORM: - case N_DHCP4_C_MESSAGE_REBIND: broadcast = true; r = n_dhcp4_c_connection_udp_broadcast(connection, request); if (r) diff --git a/shared/n-dhcp4/src/n-dhcp4-c-probe.c b/shared/n-dhcp4/src/n-dhcp4-c-probe.c index 1810df1e8d..4fb7d3892a 100644 --- a/shared/n-dhcp4/src/n-dhcp4-c-probe.c +++ b/shared/n-dhcp4/src/n-dhcp4-c-probe.c @@ -785,6 +785,10 @@ static int n_dhcp4_client_probe_transition_t2(NDhcp4ClientProbe *probe, uint64_t switch (probe->state) { case N_DHCP4_CLIENT_PROBE_STATE_BOUND: case N_DHCP4_CLIENT_PROBE_STATE_RENEWING: + r = n_dhcp4_c_connection_listen(&probe->connection); + if (r) + return r; + r = n_dhcp4_c_connection_rebind_new(&probe->connection, &request); if (r) return r; @@ -907,11 +911,22 @@ static int n_dhcp4_client_probe_transition_ack(NDhcp4ClientProbe *probe, NDhcp4I _c_cleanup_(n_dhcp4_incoming_freep) NDhcp4Incoming *message = message_take; _c_cleanup_(n_dhcp4_client_lease_unrefp) NDhcp4ClientLease *lease = NULL; NDhcp4CEventNode *node; + struct in_addr client = {}; + struct in_addr server = {}; int r; switch (probe->state) { - case N_DHCP4_CLIENT_PROBE_STATE_RENEWING: case N_DHCP4_CLIENT_PROBE_STATE_REBINDING: + n_dhcp4_incoming_get_yiaddr(message, &client); + + r = n_dhcp4_incoming_query_server_identifier(message, &server); + if (r) + return r; + r = n_dhcp4_c_connection_connect(&probe->connection, &client, &server); + if (r) + return r; + /* fall-through */ + case N_DHCP4_CLIENT_PROBE_STATE_RENEWING: r = n_dhcp4_client_probe_raise(probe, &node, From 36f8822c9b06a23b804ce8dcffa5726c7d14fb2f Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 23 Dec 2019 15:53:30 +0100 Subject: [PATCH 4/7] n-dhcp4: handle invalid return codes gracefully Instead of terminating the program when the dispatch function returns an invalid return code, log an error message and convert the error code to a valid, generic one. https://bugs.archlinux.org/task/64880 --- shared/n-dhcp4/src/n-dhcp4-client.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/shared/n-dhcp4/src/n-dhcp4-client.c b/shared/n-dhcp4/src/n-dhcp4-client.c index 0bfe48ee95..4fa3d65daa 100644 --- a/shared/n-dhcp4/src/n-dhcp4-client.c +++ b/shared/n-dhcp4/src/n-dhcp4-client.c @@ -681,7 +681,12 @@ _c_public_ int n_dhcp4_client_dispatch(NDhcp4Client *client) { /* continue normally */ } else if (r) { - c_assert(r < _N_DHCP4_E_INTERNAL); + if (r >= _N_DHCP4_E_INTERNAL) { + n_dhcp4_c_log(client->config, LOG_ERR, + "invalid internal error code %d after dispatch", + r); + return N_DHCP4_E_INTERNAL; + } return r; } } From c9fbdf3cb0d31e6ca24baa5a43a56d878317a620 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 17 Dec 2019 18:18:48 +0100 Subject: [PATCH 5/7] dhcp: test parsing of domain-search option Add a test for the parsing of the the domain-search option. --- src/dhcp/nm-dhcp-nettools.c | 59 ++++++++++++++++++++++---------- src/dhcp/nm-dhcp-utils.h | 2 ++ src/dhcp/tests/test-dhcp-utils.c | 55 +++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 18 deletions(-) diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c index 2cb7c51b61..f59b27e356 100644 --- a/src/dhcp/nm-dhcp-nettools.c +++ b/src/dhcp/nm-dhcp-nettools.c @@ -775,23 +775,12 @@ lease_parse_domainname (NDhcp4ClientLease *lease, str->str); } -static void -lease_parse_search_domains (NDhcp4ClientLease *lease, - NMIP4Config *ip4_config, - GHashTable *options) +char ** +nm_dhcp_parse_search_list (guint8 *data, size_t n_data) { - nm_auto_free_gstring GString *str = NULL; - uint8_t *data, *cache; - size_t n_data, n_cache = 0; - int r; - - r = n_dhcp4_client_lease_query (lease, NM_DHCP_OPTION_DHCP4_DOMAIN_SEARCH_LIST, &data, &n_data); - if (r) - return; - - cache = data; - - nm_gstring_prepare (&str); + GPtrArray *array = NULL; + guint8 *cache = data; + size_t n_cache = 0; for (;;) { nm_auto_free_gstring GString *domain = NULL; @@ -801,8 +790,42 @@ lease_parse_search_domains (NDhcp4ClientLease *lease, if (!lease_option_print_domain_name (domain, cache, &n_cache, &data, &n_data)) break; - g_string_append (nm_gstring_add_space_delimiter (str), domain->str); - nm_ip4_config_add_search (ip4_config, domain->str); + if (!array) + array = g_ptr_array_new (); + + g_ptr_array_add (array, g_string_free (domain, FALSE)); + domain = NULL; + } + + if (array) { + g_ptr_array_add (array, NULL); + return (char **) g_ptr_array_free (array, FALSE); + } else + return NULL; +} + +static void +lease_parse_search_domains (NDhcp4ClientLease *lease, + NMIP4Config *ip4_config, + GHashTable *options) +{ + nm_auto_free_gstring GString *str = NULL; + uint8_t *data; + size_t n_data; + gs_strfreev char **domains = NULL; + guint i; + int r; + + r = n_dhcp4_client_lease_query (lease, NM_DHCP_OPTION_DHCP4_DOMAIN_SEARCH_LIST, &data, &n_data); + if (r) + return; + + domains = nm_dhcp_parse_search_list (data, n_data); + nm_gstring_prepare (&str); + + for (i = 0; domains && domains[i]; i++) { + g_string_append (nm_gstring_add_space_delimiter (str), domains[i]); + nm_ip4_config_add_search (ip4_config, domains[i]); } nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, diff --git a/src/dhcp/nm-dhcp-utils.h b/src/dhcp/nm-dhcp-utils.h index e4c3331478..ecb91809be 100644 --- a/src/dhcp/nm-dhcp-utils.h +++ b/src/dhcp/nm-dhcp-utils.h @@ -36,5 +36,7 @@ gboolean nm_dhcp_utils_get_leasefile_path (int addr_family, const char *uuid, char **out_leasefile_path); +char **nm_dhcp_parse_search_list (guint8 *data, size_t n_data); + #endif /* __NETWORKMANAGER_DHCP_UTILS_H__ */ diff --git a/src/dhcp/tests/test-dhcp-utils.c b/src/dhcp/tests/test-dhcp-utils.c index d038906964..e601534eb2 100644 --- a/src/dhcp/tests/test-dhcp-utils.c +++ b/src/dhcp/tests/test-dhcp-utils.c @@ -199,6 +199,60 @@ test_vendor_option_metered (void) g_hash_table_destroy (options); } +static void +test_parse_search_list (void) +{ + guint8 *data; + char **domains; + + data = (guint8 []) { + 0x05, 'l', 'o', 'c', 'a', 'l', + 0x00 + }; + domains = nm_dhcp_parse_search_list (data, 7); + g_assert (domains); + g_assert_cmpint (g_strv_length (domains), ==, 1); + g_assert_cmpstr (domains[0], ==, "local"); + g_strfreev (domains); + + data = (guint8 []) { + 0x04, 't', 'e', 's', 't', + 0x07, 'e', 'x', 'a', 'm', 'p', 'l', 'e', + 0x03, 'c', 'o', 'm', + 0x00, + 0xc0, 0x05, + 0x03, 'a', 'b', 'c', + 0xc0, 0x0d, + 0x06, 'f', 'o', 'o', 'b', 'a', 'r', + 0x00 + }; + domains = nm_dhcp_parse_search_list (data, 34); + g_assert (domains); + g_assert_cmpint (g_strv_length (domains), ==, 4); + g_assert_cmpstr (domains[0], ==, "test.example.com"); + g_assert_cmpstr (domains[1], ==, "example.com"); + g_assert_cmpstr (domains[2], ==, "abc.com"); + g_assert_cmpstr (domains[3], ==, "foobar"); + g_strfreev (domains); + + data = (guint8 []) { + 0x40, 'b', 'a', 'd', + }; + domains = nm_dhcp_parse_search_list (data, 4); + g_assert (!domains); + + data = (guint8 []) { + 0x04, 'o', 'k', 'a', 'y', + 0x00, + 0x40, 'b', 'a', 'd', + }; + domains = nm_dhcp_parse_search_list (data, 10); + g_assert (domains); + g_assert_cmpint (g_strv_length (domains), ==, 1); + g_assert_cmpstr (domains[0], ==, "okay"); + g_strfreev (domains); +} + static void ip4_test_route (NMIP4Config *ip4_config, guint route_num, @@ -732,6 +786,7 @@ int main (int argc, char **argv) g_test_add_func ("/dhcp/ip4-prefix-classless", test_ip4_prefix_classless); g_test_add_func ("/dhcp/client-id-from-string", test_client_id_from_string); g_test_add_func ("/dhcp/vendor-option-metered", test_vendor_option_metered); + g_test_add_func ("/dhcp/parse-search-list", test_parse_search_list); return g_test_run (); } From dd3114deb04184b38da6a9bd2d1e602985380c55 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 19 Dec 2019 11:27:10 +0100 Subject: [PATCH 6/7] dhcp: nettools: fix parsing of classless routes option Fixes: 6adade6f21d5 ('dhcp: add nettools dhcp4 client') --- src/dhcp/nm-dhcp-nettools.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c index f59b27e356..fa5abda539 100644 --- a/src/dhcp/nm-dhcp-nettools.c +++ b/src/dhcp/nm-dhcp-nettools.c @@ -128,6 +128,7 @@ lease_option_next_route (struct in_addr *destp, uint8_t *data = *datap; size_t n_data = *n_datap; uint8_t plen; + uint8_t bytes; if (classless) { if (!lease_option_consume (&plen, sizeof (plen), &data, &n_data)) @@ -136,7 +137,9 @@ lease_option_next_route (struct in_addr *destp, if (plen > 32) return FALSE; - if (!lease_option_consume (&dest, plen / 8, &data, &n_data)) + bytes = plen == 0 ? 0 : ((plen - 1) / 8) + 1; + + if (!lease_option_consume (&dest, bytes, &data, &n_data)) return FALSE; } else { if (!lease_option_next_in_addr (&dest, &data, &n_data)) From 6af6f70d812249bdbc27d63fbd9544c57c33d04d Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 23 Dec 2019 15:39:06 +0100 Subject: [PATCH 7/7] dhcp: nettools: start from init-reboot phase when reusing address If we know the address used previously, also tell the client to start from the init-reboot phase, so that it will start with a DHCP request instead of a discover. --- src/dhcp/nm-dhcp-nettools.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c index fa5abda539..30820c615a 100644 --- a/src/dhcp/nm-dhcp-nettools.c +++ b/src/dhcp/nm-dhcp-nettools.c @@ -1311,8 +1311,10 @@ ip4_start (NMDhcpClient *client, sd_dhcp_lease_get_address (lease, &last_addr); } - if (last_addr.s_addr) + if (last_addr.s_addr) { n_dhcp4_client_probe_config_set_requested_ip (config, last_addr); + n_dhcp4_client_probe_config_set_init_reboot (config, TRUE); + } /* Add requested options */ for (i = 0; _nm_dhcp_option_dhcp4_options[i].name; i++) {