From afb9bb0dac24fdc12271b9632cb721045e9f92a0 Mon Sep 17 00:00:00 2001 From: Francesco Giudici Date: Fri, 23 Aug 2019 12:23:21 +0200 Subject: [PATCH 1/5] dhcp: add a shared function to retrieve the dhcp lease file For each plugin we try to come up with a lease file constructed in the same way, i.e., plugin name + iface + connection duid. If the file isn't already there, for some plugins (dhclient) we do extra checks in order to allow to use lease files generated outside of NetworkManager. Let's allow to generate the common NetworkManager dhcp lease file name in a shared function, reporting to the caller if the file isn't already there, so that further plugin specific checks can be performed if needed. --- src/dhcp/nm-dhcp-utils.c | 53 ++++++++++++++++++++++++++++++++++++++++ src/dhcp/nm-dhcp-utils.h | 8 +++++- 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/dhcp/nm-dhcp-utils.c b/src/dhcp/nm-dhcp-utils.c index 3f9368f2dd..4723b129d5 100644 --- a/src/dhcp/nm-dhcp-utils.c +++ b/src/dhcp/nm-dhcp-utils.c @@ -25,6 +25,7 @@ #include "nm-dhcp-utils.h" #include "nm-utils.h" +#include "nm-config.h" #include "NetworkManagerUtils.h" #include "platform/nm-platform.h" #include "nm-dhcp-client-logging.h" @@ -762,3 +763,55 @@ nm_dhcp_utils_client_id_string_to_bytes (const char *client_id) return bytes; } +/** + * nm_dhcp_utils_get_leasefile_path: + * @addr_family: the IP address family + * @plugin_name: the name of the plugin part of the lease file name + * @iface: the interface name to which the lease relates to + * @uuid: uuid of the connection to which the lease relates to + * @out_leasefile_path: will store the computed lease file path + * + * Constructs the lease file name on the basis of the calling plugin, + * interface name and connection uuid. Then returns in @out_leasefile_path + * the full path of the lease filename. + * + * Returns: TRUE if the lease file already exists, FALSE otherwise. + */ +gboolean +nm_dhcp_utils_get_leasefile_path (int addr_family, + const char *plugin_name, + const char *iface, + const char *uuid, + char **out_leasefile_path) +{ + gs_free char *rundir_path = NULL; + gs_free char *statedir_path = NULL; + + rundir_path = g_strdup_printf (NMRUNDIR "/%s%s-%s-%s.lease", + plugin_name, + addr_family == AF_INET6 ? "6" : "", + uuid, + iface); + + if (g_file_test (rundir_path, G_FILE_TEST_EXISTS)) { + *out_leasefile_path = g_steal_pointer (&rundir_path); + return TRUE; + } + + statedir_path = g_strdup_printf (NMSTATEDIR "/%s%s-%s-%s.lease", + plugin_name, + addr_family == AF_INET6 ? "6" : "", + uuid, + iface); + + if (g_file_test (statedir_path, G_FILE_TEST_EXISTS)) { + *out_leasefile_path = g_steal_pointer (&statedir_path); + return TRUE; + } + + if (nm_config_get_configure_and_quit (nm_config_get ()) == NM_CONFIG_CONFIGURE_AND_QUIT_INITRD) + *out_leasefile_path = g_steal_pointer (&rundir_path); + else + *out_leasefile_path = g_steal_pointer (&statedir_path); + return FALSE; +} diff --git a/src/dhcp/nm-dhcp-utils.h b/src/dhcp/nm-dhcp-utils.h index 39ae76932b..36099b5f8f 100644 --- a/src/dhcp/nm-dhcp-utils.h +++ b/src/dhcp/nm-dhcp-utils.h @@ -40,7 +40,13 @@ NMPlatformIP6Address nm_dhcp_utils_ip6_prefix_from_options (GHashTable *options) char *nm_dhcp_utils_duid_to_string (GBytes *duid); -GBytes * nm_dhcp_utils_client_id_string_to_bytes (const char *client_id); +GBytes *nm_dhcp_utils_client_id_string_to_bytes (const char *client_id); + +gboolean nm_dhcp_utils_get_leasefile_path (int addr_family, + const char *plugin_name, + const char *iface, + const char *uuid, + char **out_leasefile_path); #endif /* __NETWORKMANAGER_DHCP_UTILS_H__ */ From 89814d90aa33ccf214163faae7a7b15cc39f65bf Mon Sep 17 00:00:00 2001 From: Francesco Giudici Date: Fri, 23 Aug 2019 17:37:37 +0200 Subject: [PATCH 2/5] dhcp: internal: use the shared function to retrieve the lease file path --- src/dhcp/nm-dhcp-systemd.c | 40 +++++--------------------------------- 1 file changed, 5 insertions(+), 35 deletions(-) diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index f0084f1b86..d9b6077440 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -30,7 +30,6 @@ #include "nm-std-aux/unaligned.h" #include "nm-utils.h" -#include "nm-config.h" #include "nm-dhcp-utils.h" #include "nm-dhcp-options.h" #include "nm-core-utils.h" @@ -535,37 +534,6 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, /*****************************************************************************/ -static char * -get_leasefile_path (int addr_family, const char *iface, const char *uuid) -{ - char *rundir_path; - char *statedir_path; - - rundir_path = g_strdup_printf (NMRUNDIR "/internal%s-%s-%s.lease", - addr_family == AF_INET6 ? "6" : "", - uuid, - iface); - - if (g_file_test (rundir_path, G_FILE_TEST_EXISTS)) - return rundir_path; - - statedir_path = g_strdup_printf (NMSTATEDIR "/internal%s-%s-%s.lease", - addr_family == AF_INET6 ? "6" : "", - uuid, - iface); - - if ( g_file_test (statedir_path, G_FILE_TEST_EXISTS) - || nm_config_get_configure_and_quit (nm_config_get ()) != NM_CONFIG_CONFIGURE_AND_QUIT_INITRD) { - g_free (rundir_path); - return statedir_path; - } else { - g_free (statedir_path); - return rundir_path; - } -} - -/*****************************************************************************/ - static void bound4_handle (NMDhcpSystemd *self) { @@ -704,9 +672,11 @@ ip4_start (NMDhcpClient *client, return FALSE; } - lease_file = get_leasefile_path (AF_INET, - nm_dhcp_client_get_iface (client), - nm_dhcp_client_get_uuid (client)); + nm_dhcp_utils_get_leasefile_path (AF_INET, + "internal", + nm_dhcp_client_get_iface (client), + nm_dhcp_client_get_uuid (client), + &lease_file); if (last_ip4_address) inet_pton (AF_INET, last_ip4_address, &last_addr); From ee20761ea897a097cf46fdb223c0e0f40b1fba91 Mon Sep 17 00:00:00 2001 From: Francesco Giudici Date: Fri, 23 Aug 2019 18:25:23 +0200 Subject: [PATCH 3/5] dhcp: prefer nm_assert() to g_assert*() --- src/dhcp/nm-dhcp-client.c | 2 +- src/dhcp/nm-dhcp-dhclient-utils.c | 2 +- src/dhcp/nm-dhcp-dhclient.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c index 9f585c5dc9..a134555b87 100644 --- a/src/dhcp/nm-dhcp-client.c +++ b/src/dhcp/nm-dhcp-client.c @@ -686,7 +686,7 @@ nm_dhcp_client_stop (NMDhcpClient *self, gboolean release) _LOGI ("canceled DHCP transaction, DHCP client pid %d", old_pid); else _LOGI ("canceled DHCP transaction"); - g_assert (priv->pid == -1); + nm_assert (priv->pid == -1); nm_dhcp_client_set_state (self, NM_DHCP_STATE_DONE, NULL, NULL); } diff --git a/src/dhcp/nm-dhcp-dhclient-utils.c b/src/dhcp/nm-dhcp-dhclient-utils.c index bf3df399b6..359779f19c 100644 --- a/src/dhcp/nm-dhcp-dhclient-utils.c +++ b/src/dhcp/nm-dhcp-dhclient-utils.c @@ -128,7 +128,7 @@ add_ip4_config (GString *str, GBytes *client_id, const char *hostname, gboolean guint i; p = g_bytes_get_data (client_id, &l); - g_assert (p); + nm_assert (p); /* Allow type 0 (non-hardware address) to be represented as a string * as long as all the characters are printable. diff --git a/src/dhcp/nm-dhcp-dhclient.c b/src/dhcp/nm-dhcp-dhclient.c index 72ea3d48f8..fa142b4b5d 100644 --- a/src/dhcp/nm-dhcp-dhclient.c +++ b/src/dhcp/nm-dhcp-dhclient.c @@ -213,7 +213,7 @@ merge_dhclient_config (NMDhcpDhclient *self, orig_path, orig, out_new_client_id); - g_assert (new); + nm_assert (new); return g_file_set_contents (conf_file, new, From f60a60a0d056b8bb16f6bcea406ee53935e799b5 Mon Sep 17 00:00:00 2001 From: Francesco Giudici Date: Fri, 23 Aug 2019 18:27:28 +0200 Subject: [PATCH 4/5] dhcp: dhclient: use the shared function to retrieve the lease file path ... but leave in place the custom checks dependant on the dhclient plugin --- src/dhcp/nm-dhcp-dhclient.c | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/src/dhcp/nm-dhcp-dhclient.c b/src/dhcp/nm-dhcp-dhclient.c index fa142b4b5d..ec9676e5e4 100644 --- a/src/dhcp/nm-dhcp-dhclient.c +++ b/src/dhcp/nm-dhcp-dhclient.c @@ -38,7 +38,6 @@ #include "nm-glib-aux/nm-dedup-multi.h" #include "nm-utils.h" -#include "nm-config.h" #include "nm-dhcp-dhclient-utils.h" #include "nm-dhcp-manager.h" #include "NetworkManagerUtils.h" @@ -118,35 +117,14 @@ get_dhclient_leasefile (int addr_family, const char *uuid, char **out_preferred_path) { - gs_free char *rundir_path = NULL; gs_free char *path = NULL; - /* First, see if the lease file is in /run */ - rundir_path = g_strdup_printf (NMRUNDIR "/dhclient%s-%s-%s.lease", - _addr_family_to_path_part (addr_family), - uuid, - iface); - - if (g_file_test (rundir_path, G_FILE_TEST_EXISTS)) { - NM_SET_OUT (out_preferred_path, g_strdup (rundir_path)); - return g_steal_pointer (&rundir_path); - } - - /* /var/lib/NetworkManager is the preferred leasefile path */ - path = g_strdup_printf (NMSTATEDIR "/dhclient%s-%s-%s.lease", - _addr_family_to_path_part (addr_family), - uuid, - iface); - - if (g_file_test (path, G_FILE_TEST_EXISTS)) { + if (nm_dhcp_utils_get_leasefile_path (addr_family, "dhclient", iface, uuid, &path)) { NM_SET_OUT (out_preferred_path, g_strdup (path)); return g_steal_pointer (&path); } - if (nm_config_get_configure_and_quit (nm_config_get ()) == NM_CONFIG_CONFIGURE_AND_QUIT_INITRD) - NM_SET_OUT (out_preferred_path, g_steal_pointer (&rundir_path)); - else - NM_SET_OUT (out_preferred_path, g_steal_pointer (&path)); + NM_SET_OUT (out_preferred_path, g_steal_pointer (&path)); /* If the leasefile we're looking for doesn't exist yet in the new location * (eg, /var/lib/NetworkManager) then look in old locations to maintain From 9f8951692822497ee559e20bce00ea2e6f50209b Mon Sep 17 00:00:00 2001 From: Francesco Giudici Date: Mon, 26 Aug 2019 18:38:02 +0200 Subject: [PATCH 5/5] dhcp: nettools: read/write lease files Use the same format of systemd-netword, so that we will be compatible with the leases created/read by the current "internal" plugin. Note that actually only the leased address is processed when reading a lease file, so no need to save more than the ip address when saving the lease. --- src/dhcp/nm-dhcp-nettools.c | 59 +++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c index a43a047e13..d0f426f4cd 100644 --- a/src/dhcp/nm-dhcp-nettools.c +++ b/src/dhcp/nm-dhcp-nettools.c @@ -42,6 +42,7 @@ #include "nm-dhcp-client-logging.h" #include "n-dhcp4/src/n-dhcp4.h" #include "systemd/nm-sd-utils-shared.h" +#include "systemd/nm-sd-utils-dhcp.h" /*****************************************************************************/ @@ -63,6 +64,7 @@ typedef struct { NDhcp4ClientLease *lease; GIOChannel *channel; guint event_id; + char *lease_file; } NMDhcpNettoolsPrivate; struct _NMDhcpNettools { @@ -873,9 +875,35 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, /*****************************************************************************/ +static void +lease_save (NDhcp4ClientLease *lease, const char *lease_file) +{ + struct in_addr a_address; + nm_auto_free_gstring GString *new_contents = NULL; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; + + nm_assert (lease); + nm_assert (lease_file); + + new_contents = g_string_new ("# This is private data. Do not parse.\n"); + + n_dhcp4_client_lease_get_yiaddr (lease, &a_address); + if (a_address.s_addr == INADDR_ANY) + return; + + g_string_append_printf (new_contents, + "ADDRESS=%s\n", nm_utils_inet4_ntop (a_address.s_addr, sbuf)); + + g_file_set_contents (lease_file, + new_contents->str, + -1, + NULL); +} + static void bound4_handle (NMDhcpNettools *self, NDhcp4ClientLease *lease) { + NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE (self); const char *iface = nm_dhcp_client_get_iface (NM_DHCP_CLIENT (self)); gs_unref_object NMIP4Config *ip4_config = NULL; gs_unref_hashtable GHashTable *options = NULL; @@ -899,6 +927,7 @@ bound4_handle (NMDhcpNettools *self, NDhcp4ClientLease *lease) } nm_dhcp_option_add_requests_to_options (options, _nm_dhcp_option_dhcp4_options); + lease_save (lease, priv->lease_file); nm_dhcp_client_set_state (NM_DHCP_CLIENT (self), NM_DHCP_STATE_BOUND, @@ -1120,6 +1149,7 @@ ip4_start (NMDhcpClient *client, nm_auto (n_dhcp4_client_probe_config_freep) NDhcp4ClientProbeConfig *config = NULL; NMDhcpNettools *self = NM_DHCP_NETTOOLS (client); NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE (self); + gs_free char *lease_file = NULL; struct in_addr last_addr = { 0 }; const char *hostname; int r, i; @@ -1141,11 +1171,32 @@ ip4_start (NMDhcpClient *client, */ n_dhcp4_client_probe_config_set_start_delay (config, 1); - if (last_ip4_address) { + nm_dhcp_utils_get_leasefile_path (AF_INET, + "internal", + nm_dhcp_client_get_iface (client), + nm_dhcp_client_get_uuid (client), + &lease_file); + + if (last_ip4_address) inet_pton (AF_INET, last_ip4_address, &last_addr); - n_dhcp4_client_probe_config_set_requested_ip (config, last_addr); + else { + /* + * TODO: we stick to the systemd-networkd lease file format. Quite easy for now to + * just use the functions in systemd code. Anyway, as in the end we just use the + * ip address from all the options found in the lease, write a function that parses + * the lease file just for the assigned address and returns it in &last_address. + * Then drop reference to systemd-networkd structures and functions. + */ + nm_auto (sd_dhcp_lease_unrefp) sd_dhcp_lease *lease = NULL; + + dhcp_lease_load (&lease, lease_file); + if (lease) + sd_dhcp_lease_get_address (lease, &last_addr); } + if (last_addr.s_addr) + n_dhcp4_client_probe_config_set_requested_ip (config, last_addr); + /* Add requested options */ for (i = 0; _nm_dhcp_option_dhcp4_options[i].name; i++) { if (_nm_dhcp_option_dhcp4_options[i].include) { @@ -1194,6 +1245,9 @@ ip4_start (NMDhcpClient *client, } } + g_free (priv->lease_file); + priv->lease_file = g_steal_pointer (&lease_file); + r = n_dhcp4_client_probe (priv->client, &priv->probe, config); if (r) { nm_utils_error_set_errno (error, r, "failed to start DHCP client: %s"); @@ -1233,6 +1287,7 @@ dispose (GObject *object) { NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE ((NMDhcpNettools *) object); + nm_clear_pointer (&priv->lease_file, g_free); nm_clear_pointer (&priv->channel, g_io_channel_unref); nm_clear_g_source (&priv->event_id); nm_clear_pointer (&priv->lease, n_dhcp4_client_lease_unref);