From 3338ea0530bc05b403049df6df8f246cb455b597 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 14 Sep 2020 15:40:08 +0200 Subject: [PATCH] device: track used shared-ips via NMNetns Note that when NetworkManager tries to allocate more than 256 networks, then previously the allocation would fail. We no longer fail, but log an error and reuse the last address (10.42.255.1/24). It's simpler to have code that cannot fail, because it's often hard to handle failure properly. Also, if the user would configure two shared profiles that explicitly use the same subnet, we also wouldn't fail. Why not? Is that not a problem as well? If it is not, there is no need to fail in this case. If it is a problem, then it would be much more important to handle this case otherwise -- since it's more likely to activate two profiles that accidentally use the same subnet than activating 257+ shared profiles. --- src/devices/nm-device.c | 50 +++++++++++------------------------------ 1 file changed, 13 insertions(+), 37 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index c1a2bab12b..f0fe5a0385 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -327,6 +327,8 @@ typedef struct _NMDevicePrivate { const NML3ConfigData *l3cds[_L3_CONFIG_DATA_TYPE_NUM]; + NMNetnsSharedIPHandle *shared_ip_handle; + int parent_ifindex; int auth_retries; @@ -9649,21 +9651,11 @@ nm_device_dhcp4_renew (NMDevice *self, gboolean release) /*****************************************************************************/ -static GHashTable *shared_ips = NULL; - -static void -shared_ip_release (gpointer data) -{ - g_hash_table_remove (shared_ips, data); - if (!g_hash_table_size (shared_ips)) - nm_clear_pointer (&shared_ips, g_hash_table_unref); -} - static NMIP4Config * shared4_new_config (NMDevice *self, NMConnection *connection) { - NMIP4Config *config = NULL; - gboolean is_generated = FALSE; + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + NMIP4Config *config; NMSettingIPConfig *s_ip4; NMPlatformIP4Address address = { .addr_source = NM_IP_CONFIG_SOURCE_SHARED, @@ -9673,42 +9665,24 @@ shared4_new_config (NMDevice *self, NMConnection *connection) g_return_val_if_fail (connection, NULL); s_ip4 = nm_connection_get_setting_ip4_config (connection); - if (s_ip4 && nm_setting_ip_config_get_num_addresses (s_ip4)) { + if ( s_ip4 + && nm_setting_ip_config_get_num_addresses (s_ip4) > 0) { /* Use the first user-supplied address */ NMIPAddress *user = nm_setting_ip_config_get_address (s_ip4, 0); in_addr_t a; nm_ip_address_get_address_binary (user, &a); nm_platform_ip4_address_set_addr (&address, a, nm_ip_address_get_prefix (user)); + nm_clear_pointer (&priv->shared_ip_handle, nm_netns_shared_ip_release); } else { - /* Find an unused address in the 10.42.x.x range */ - guint32 start = (guint32) ntohl (0x0a2a0001); /* 10.42.0.1 */ - guint32 count = 0; - - if (G_UNLIKELY (!shared_ips)) - shared_ips = g_hash_table_new (nm_direct_hash, NULL); - else { - while (g_hash_table_lookup (shared_ips, GUINT_TO_POINTER (start + count))) { - count += ntohl (0x100); - if (count > ntohl (0xFE00)) { - _LOGE (LOGD_SHARING, "ran out of shared IP addresses!"); - return FALSE; - } - } - } - nm_platform_ip4_address_set_addr (&address, start + count, 24); - g_hash_table_add (shared_ips, GUINT_TO_POINTER (address.address)); - is_generated = TRUE; + if (!priv->shared_ip_handle) + priv->shared_ip_handle = nm_netns_shared_ip_reserve (nm_device_get_netns (self)); + nm_platform_ip4_address_set_addr (&address, priv->shared_ip_handle->addr, 24); } config = nm_device_ip4_config_new (self); nm_ip4_config_add_address (config, &address); - if (is_generated) { - /* Remove the address lock when the object gets disposed */ - g_object_set_qdata_full (G_OBJECT (config), NM_CACHED_QUARK ("shared-ip"), - GUINT_TO_POINTER (address.address), - shared_ip_release); - } + return config; } @@ -15936,6 +15910,8 @@ _cleanup_generic_pre (NMDevice *self, CleanupType cleanup_type) queued_state_clear (self); + nm_clear_pointer (&priv->shared_ip_handle, nm_netns_shared_ip_release); + _cleanup_ip_pre (self, AF_INET, cleanup_type); _cleanup_ip_pre (self, AF_INET6, cleanup_type); }