From ef776e255dfd201bf27dea0d2ad0f3436cda778a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 5 Jan 2021 15:55:54 +0100 Subject: [PATCH 01/13] shared: add nm_g_bytes_new_from_str() helper --- shared/nm-glib-aux/nm-shared-utils.c | 15 +++++++++++++++ shared/nm-glib-aux/nm-shared-utils.h | 2 ++ 2 files changed, 17 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index ac922c0125..5bab4bd2f2 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -364,6 +364,21 @@ again: return b; } +GBytes * +nm_g_bytes_new_from_str(const char *str) +{ + gsize l; + + if (!str) + return NULL; + + /* the returned array is guaranteed to have a trailing '\0' + * character *after* the length. */ + + l = strlen(str); + return g_bytes_new_take(nm_memdup(str, l + 1u), l); +} + /** * nm_utils_gbytes_equals: * @bytes: (allow-none): a #GBytes array to compare. Note that diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index 5a9a4b0069..fe61459f5f 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -319,6 +319,8 @@ nm_utils_is_separator(const char c) GBytes *nm_gbytes_get_empty(void); +GBytes *nm_g_bytes_new_from_str(const char *str); + static inline gboolean nm_gbytes_equal0(GBytes *a, GBytes *b) { From 9afdbb97ea762831209163c924c3f4cd4435b17e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 4 Jan 2021 19:39:10 +0100 Subject: [PATCH 02/13] cloud-setup/trivial: rename variables "config_data" to "get_config_data" The code is not entirely straight forward. Consistent naming is hence important. In "nmcs-provider-ec2.c", variables of this kind are called "get_config_data". That also matches to the type of the data (NMCSProviderGetConfigTaskData). Rename the variables to make naming consistent. Also, I find the longer name to be clearer. --- clients/cloud-setup/nmcs-provider-azure.c | 30 +++++++++++------------ clients/cloud-setup/nmcs-provider-gcp.c | 25 ++++++++++--------- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/clients/cloud-setup/nmcs-provider-azure.c b/clients/cloud-setup/nmcs-provider-azure.c index a33fc3e0d1..42086b35e1 100644 --- a/clients/cloud-setup/nmcs-provider-azure.c +++ b/clients/cloud-setup/nmcs-provider-azure.c @@ -94,7 +94,7 @@ detect(NMCSProvider *provider, GTask *task) /*****************************************************************************/ typedef struct { - NMCSProviderGetConfigTaskData *config_data; + NMCSProviderGetConfigTaskData *get_config_data; guint n_ifaces_pending; GError * error; } AzureData; @@ -117,7 +117,7 @@ _azure_iface_data_free(AzureIfaceData *iface_data) static void _get_config_maybe_task_return(AzureData *azure_data, GError *error_take) { - NMCSProviderGetConfigTaskData *config_data = azure_data->config_data; + NMCSProviderGetConfigTaskData *get_config_data = azure_data->get_config_data; if (error_take) { if (!azure_data->error) @@ -138,16 +138,16 @@ _get_config_maybe_task_return(AzureData *azure_data, GError *error_take) _LOGD("get-config: cancelled"); else _LOGD("get-config: failed: %s", azure_data->error->message); - g_task_return_error(config_data->task, g_steal_pointer(&azure_data->error)); + g_task_return_error(get_config_data->task, g_steal_pointer(&azure_data->error)); } else { _LOGD("get-config: success"); - g_task_return_pointer(config_data->task, - g_hash_table_ref(config_data->result_dict), + g_task_return_pointer(get_config_data->task, + g_hash_table_ref(get_config_data->result_dict), (GDestroyNotify) g_hash_table_unref); } nm_g_slice_free(azure_data); - g_object_unref(config_data->task); + g_object_unref(get_config_data->task); } static void @@ -176,7 +176,7 @@ _get_config_fetch_done_cb(NMHttpClient *http_client, fip_str = g_bytes_get_data(response, NULL); iface_data->iface_get_config = - g_hash_table_lookup(azure_data->config_data->result_dict, iface_data->hwaddr); + g_hash_table_lookup(azure_data->get_config_data->result_dict, iface_data->hwaddr); iface_get_config = iface_data->iface_get_config; iface_get_config->iface_idx = iface_data->iface_idx; @@ -293,7 +293,7 @@ _get_config_ips_prefix_list_cb(GObject *source, GAsyncResult *result, gpointer u 10000, 1000, NM_MAKE_STRV(NM_AZURE_METADATA_HEADER), - g_task_get_cancellable(azure_data->config_data->task), + g_task_get_cancellable(azure_data->get_config_data->task), NULL, NULL, _get_config_fetch_done_cb_private_ipv4s, @@ -319,7 +319,7 @@ _get_config_ips_prefix_list_cb(GObject *source, GAsyncResult *result, gpointer u 10000, 1000, NM_MAKE_STRV(NM_AZURE_METADATA_HEADER), - g_task_get_cancellable(azure_data->config_data->task), + g_task_get_cancellable(azure_data->get_config_data->task), NULL, NULL, _get_config_fetch_done_cb_subnet_cidr_prefix, @@ -357,17 +357,17 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) } iface_data->iface_get_config = - g_hash_table_lookup(azure_data->config_data->result_dict, iface_data->hwaddr); + g_hash_table_lookup(azure_data->get_config_data->result_dict, iface_data->hwaddr); if (!iface_data->iface_get_config) { - if (!iface_data->azure_data->config_data->any) { + if (!iface_data->azure_data->get_config_data->any) { _LOGD("interface[%" G_GSSIZE_FORMAT "]: ignore hwaddr %s", iface_data->iface_idx, iface_data->hwaddr); goto done; } iface_data->iface_get_config = nmcs_provider_get_config_iface_data_new(FALSE); - g_hash_table_insert(azure_data->config_data->result_dict, + g_hash_table_insert(azure_data->get_config_data->result_dict, g_strdup(iface_data->hwaddr), iface_data->iface_get_config); } @@ -385,7 +385,7 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) 10000, 1000, NM_MAKE_STRV(NM_AZURE_METADATA_HEADER), - g_task_get_cancellable(azure_data->config_data->task), + g_task_get_cancellable(azure_data->get_config_data->task), NULL, NULL, _get_config_ips_prefix_list_cb, @@ -478,7 +478,7 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat 10000, 1000, NM_MAKE_STRV(NM_AZURE_METADATA_HEADER), - g_task_get_cancellable(azure_data->config_data->task), + g_task_get_cancellable(azure_data->get_config_data->task), NULL, NULL, _get_config_iface_cb, @@ -494,7 +494,7 @@ get_config(NMCSProvider *provider, NMCSProviderGetConfigTaskData *get_config_dat azure_data = g_slice_new(AzureData); *azure_data = (AzureData){ - .config_data = get_config_data, + .get_config_data = get_config_data, .n_ifaces_pending = 0, }; diff --git a/clients/cloud-setup/nmcs-provider-gcp.c b/clients/cloud-setup/nmcs-provider-gcp.c index 09b09d0f67..a0b137aa94 100644 --- a/clients/cloud-setup/nmcs-provider-gcp.c +++ b/clients/cloud-setup/nmcs-provider-gcp.c @@ -89,7 +89,7 @@ detect(NMCSProvider *provider, GTask *task) /*****************************************************************************/ typedef struct { - NMCSProviderGetConfigTaskData *config_data; + NMCSProviderGetConfigTaskData *get_config_data; guint n_ifaces_pending; GError * error; } GCPData; @@ -104,7 +104,7 @@ typedef struct { static void _get_config_maybe_task_return(GCPData *gcp_data, GError *error_take) { - NMCSProviderGetConfigTaskData *config_data = gcp_data->config_data; + NMCSProviderGetConfigTaskData *get_config_data = gcp_data->get_config_data; if (error_take) { if (!gcp_data->error) @@ -125,16 +125,16 @@ _get_config_maybe_task_return(GCPData *gcp_data, GError *error_take) _LOGD("get-config: cancelled"); else _LOGD("get-config: failed: %s", gcp_data->error->message); - g_task_return_error(config_data->task, g_steal_pointer(&gcp_data->error)); + g_task_return_error(get_config_data->task, g_steal_pointer(&gcp_data->error)); } else { _LOGD("get-config: success"); - g_task_return_pointer(config_data->task, - g_hash_table_ref(config_data->result_dict), + g_task_return_pointer(get_config_data->task, + g_hash_table_ref(get_config_data->result_dict), (GDestroyNotify) g_hash_table_unref); } nm_g_slice_free(gcp_data); - g_object_unref(config_data->task); + g_object_unref(get_config_data->task); } static void @@ -256,7 +256,7 @@ _get_config_ips_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat HTTP_POLL_TIMEOUT_MS, HTTP_RATE_LIMIT_MS, NM_MAKE_STRV(NM_GCP_METADATA_HEADER), - g_task_get_cancellable(gcp_data->config_data->task), + g_task_get_cancellable(gcp_data->get_config_data->task), NULL, NULL, _get_config_fip_cb, @@ -289,7 +289,8 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) goto iface_error; hwaddr = nmcs_utils_hwaddr_normalize(g_bytes_get_data(response, NULL), -1); - iface_data->iface_get_config = g_hash_table_lookup(gcp_data->config_data->result_dict, hwaddr); + iface_data->iface_get_config = + g_hash_table_lookup(gcp_data->get_config_data->result_dict, hwaddr); if (!iface_data->iface_get_config) { _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: did not find a matching device", iface_data->iface_idx); @@ -311,7 +312,7 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) HTTP_POLL_TIMEOUT_MS, HTTP_RATE_LIMIT_MS, NM_MAKE_STRV(NM_GCP_METADATA_HEADER), - g_task_get_cancellable(gcp_data->config_data->task), + g_task_get_cancellable(gcp_data->get_config_data->task), NULL, NULL, _get_config_ips_list_cb, @@ -396,7 +397,7 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat HTTP_POLL_TIMEOUT_MS, HTTP_RATE_LIMIT_MS, NM_MAKE_STRV(NM_GCP_METADATA_HEADER), - g_task_get_cancellable(gcp_data->config_data->task), + g_task_get_cancellable(gcp_data->get_config_data->task), NULL, NULL, _get_config_iface_cb, @@ -417,7 +418,7 @@ get_config(NMCSProvider *provider, NMCSProviderGetConfigTaskData *get_config_dat gcp_data = g_slice_new(GCPData); *gcp_data = (GCPData){ - .config_data = get_config_data, + .get_config_data = get_config_data, .n_ifaces_pending = 0, .error = NULL, }; @@ -429,7 +430,7 @@ get_config(NMCSProvider *provider, NMCSProviderGetConfigTaskData *get_config_dat HTTP_POLL_TIMEOUT_MS, HTTP_RATE_LIMIT_MS, NM_MAKE_STRV(NM_GCP_METADATA_HEADER), - g_task_get_cancellable(gcp_data->config_data->task), + g_task_get_cancellable(gcp_data->get_config_data->task), NULL, NULL, _get_net_ifaces_list_cb, From 5fb2f7e717be727c596651f6e0ebdad4c91ecc39 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 5 Jan 2021 10:20:41 +0100 Subject: [PATCH 03/13] cloud-setup/trivial: rename "response_data" variable We have three implementations of providers, that all do something similar. Name the variable with the HTTP response GBytes the same everywhere. --- clients/cloud-setup/nmcs-provider-azure.c | 4 ++-- clients/cloud-setup/nmcs-provider-ec2.c | 24 +++++++++++------------ clients/cloud-setup/nmcs-provider-gcp.c | 4 ++-- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/clients/cloud-setup/nmcs-provider-azure.c b/clients/cloud-setup/nmcs-provider-azure.c index 42086b35e1..561eb8175a 100644 --- a/clients/cloud-setup/nmcs-provider-azure.c +++ b/clients/cloud-setup/nmcs-provider-azure.c @@ -263,7 +263,7 @@ _get_config_ips_prefix_list_cb(GObject *source, GAsyncResult *result, gpointer u if (line_len == 0) continue; - /* Truncate the string. It's safe to do, because we own @response_data an it has an + /* Truncate the string. It's safe to do, because we own @response an it has an * extra NULL character after the buffer. */ ((char *) line)[line_len] = '\0'; @@ -431,7 +431,7 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat if (line_len == 0) continue; - /* Truncate the string. It's safe to do, because we own @response_data an it has an + /* Truncate the string. It's safe to do, because we own @response an it has an * extra NULL character after the buffer. */ ((char *) line)[line_len] = '\0'; diff --git a/clients/cloud-setup/nmcs-provider-ec2.c b/clients/cloud-setup/nmcs-provider-ec2.c index 40810757de..40bed1575c 100644 --- a/clients/cloud-setup/nmcs-provider-ec2.c +++ b/clients/cloud-setup/nmcs-provider-ec2.c @@ -72,11 +72,11 @@ G_DEFINE_TYPE(NMCSProviderEC2, nmcs_provider_ec2, NMCS_TYPE_PROVIDER); static gboolean _detect_get_meta_data_check_cb(long response_code, - GBytes * response_data, + GBytes * response, gpointer check_user_data, GError **error) { - return response_code == 200 && nmcs_utils_parse_get_full_line(response_data, "ami-id"); + return response_code == 200 && nmcs_utils_parse_get_full_line(response, "ami-id"); } static void @@ -187,13 +187,13 @@ _get_config_fetch_done_cb(NMHttpClient *http_client, gboolean is_local_ipv4) { GetConfigIfaceData *iface_data; - const char * hwaddr = NULL; - gs_unref_bytes GBytes *response_data = NULL; - gs_free_error GError *error = NULL; + const char * hwaddr = NULL; + gs_unref_bytes GBytes *response = NULL; + gs_free_error GError *error = NULL; nm_utils_user_data_unpack(user_data, &iface_data, &hwaddr); - nm_http_client_poll_get_finish(http_client, result, NULL, &response_data, &error); + nm_http_client_poll_get_finish(http_client, result, NULL, &response, &error); if (!error) { NMCSProviderGetConfigIfaceData *config_iface_data; @@ -206,7 +206,7 @@ _get_config_fetch_done_cb(NMHttpClient *http_client, gs_free const char **s_addrs = NULL; gsize i, len; - s_addrs = nm_utils_strsplit_set_full(g_bytes_get_data(response_data, NULL), + s_addrs = nm_utils_strsplit_set_full(g_bytes_get_data(response, NULL), "\n", NM_UTILS_STRSPLIT_SET_FLAGS_STRSTRIP); len = NM_PTRARRAY_LEN(s_addrs); @@ -225,7 +225,7 @@ _get_config_fetch_done_cb(NMHttpClient *http_client, } } else { if (nm_utils_parse_inaddr_prefix_bin(AF_INET, - g_bytes_get_data(response_data, NULL), + g_bytes_get_data(response, NULL), NULL, &tmp_addr, &tmp_prefix)) { @@ -413,7 +413,7 @@ _get_config_metadata_ready_cb(GObject *source, GAsyncResult *result, gpointer us static gboolean _get_config_metadata_ready_check(long response_code, - GBytes * response_data, + GBytes * response, gpointer check_user_data, GError **error) { @@ -428,12 +428,12 @@ _get_config_metadata_ready_check(long response_code, const char * c_hwaddr; gssize iface_idx_counter = 0; - if (response_code != 200 || !response_data) { + if (response_code != 200 || !response) { /* we wait longer. */ return FALSE; } - r_data = g_bytes_get_data(response_data, &r_len); + r_data = g_bytes_get_data(response, &r_len); /* NMHttpClient guarantees that there is a trailing NUL after the data. */ nm_assert(r_data[r_len] == 0); @@ -444,7 +444,7 @@ _get_config_metadata_ready_check(long response_code, if (cur_line_len == 0) continue; - /* Truncate the string. It's safe to do, because we own @response_data an it has an + /* Truncate the string. It's safe to do, because we own @response an it has an * extra NUL character after the buffer. */ ((char *) cur_line)[cur_line_len] = '\0'; diff --git a/clients/cloud-setup/nmcs-provider-gcp.c b/clients/cloud-setup/nmcs-provider-gcp.c index a0b137aa94..45e1c3622b 100644 --- a/clients/cloud-setup/nmcs-provider-gcp.c +++ b/clients/cloud-setup/nmcs-provider-gcp.c @@ -218,7 +218,7 @@ _get_config_ips_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat while (nm_utils_parse_next_line(&response_str, &response_len, &line, &line_len)) { gint64 fip_index; - /* Truncate the string. It's safe to do, because we own @response_data an it has an + /* Truncate the string. It's safe to do, because we own @response an it has an * extra NUL character after the buffer. */ ((char *) line)[line_len] = '\0'; @@ -358,7 +358,7 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat if (line_len == 0) continue; - /* Truncate the string. It's safe to do, because we own @response_data an it has an + /* Truncate the string. It's safe to do, because we own @response an it has an * extra NUL character after the buffer. */ ((char *) line)[line_len] = '\0'; if (line[line_len - 1] == '/') From 494819bbbf973bef683e17c0a4fe0c814b6ad838 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 4 Jan 2021 19:20:14 +0100 Subject: [PATCH 04/13] cloud-setup: move common code for get_config() to base class and improve cancellation First note that all three provider implementations are very similar. That is why NMCSProvider's implementation does already some work that is common to all implementations. For example, it provides the NMCSProviderGetConfigTaskData structure to help tracking the data of the request. Also note that the GCP/Azure implementations didn't handle the cancellation correctly. They always would pass g_task_get_cancellable(get_config_data->task) to the asynchronous requests. That is the GCancellable provider by the caller. That is fine when there is only one async operation ongoing. But that is not the case, we have parallel HTTP requests. Then, when an error happened, the overall get_config() operations fails and the still pending requests should all be aborted. However, we must not cancel the GCancellable of the user (because that is not owned by us). The correct solution is to use an internal cancellable in those cases. Anyway. Since all of this is similar, we can extend the base class to handle things for us. This also gets the cancellation right by having a "get_config_data->intern_cancellable". --- clients/cloud-setup/nmcs-provider-azure.c | 264 ++++++++++------------ clients/cloud-setup/nmcs-provider-ec2.c | 246 ++++++-------------- clients/cloud-setup/nmcs-provider-gcp.c | 183 +++++++-------- clients/cloud-setup/nmcs-provider.c | 70 +++++- clients/cloud-setup/nmcs-provider.h | 25 +- 5 files changed, 350 insertions(+), 438 deletions(-) diff --git a/clients/cloud-setup/nmcs-provider-azure.c b/clients/cloud-setup/nmcs-provider-azure.c index 561eb8175a..a18a0d64b4 100644 --- a/clients/cloud-setup/nmcs-provider-azure.c +++ b/clients/cloud-setup/nmcs-provider-azure.c @@ -94,128 +94,87 @@ detect(NMCSProvider *provider, GTask *task) /*****************************************************************************/ typedef struct { - NMCSProviderGetConfigTaskData *get_config_data; - guint n_ifaces_pending; - GError * error; -} AzureData; - -typedef struct { + NMCSProviderGetConfigTaskData * get_config_data; NMCSProviderGetConfigIfaceData *iface_get_config; - AzureData * azure_data; gssize iface_idx; guint n_ips_prefix_pending; char * hwaddr; } AzureIfaceData; static void -_azure_iface_data_free(AzureIfaceData *iface_data) +_azure_iface_data_destroy(AzureIfaceData *iface_data) { g_free(iface_data->hwaddr); nm_g_slice_free(iface_data); } static void -_get_config_maybe_task_return(AzureData *azure_data, GError *error_take) -{ - NMCSProviderGetConfigTaskData *get_config_data = azure_data->get_config_data; - - if (error_take) { - if (!azure_data->error) - azure_data->error = error_take; - else if (!nm_utils_error_is_cancelled(azure_data->error) - && nm_utils_error_is_cancelled(error_take)) { - nm_clear_error(&azure_data->error); - azure_data->error = error_take; - } else - g_error_free(error_take); - } - - if (azure_data->n_ifaces_pending > 0) - return; - - if (azure_data->error) { - if (nm_utils_error_is_cancelled(azure_data->error)) - _LOGD("get-config: cancelled"); - else - _LOGD("get-config: failed: %s", azure_data->error->message); - g_task_return_error(get_config_data->task, g_steal_pointer(&azure_data->error)); - } else { - _LOGD("get-config: success"); - g_task_return_pointer(get_config_data->task, - g_hash_table_ref(get_config_data->result_dict), - (GDestroyNotify) g_hash_table_unref); - } - - nm_g_slice_free(azure_data); - g_object_unref(get_config_data->task); -} - -static void -_get_config_fetch_done_cb(NMHttpClient *http_client, - GAsyncResult *result, - gpointer user_data, - gboolean is_ipv4) +_get_config_fetch_done_cb(NMHttpClient * http_client, + GAsyncResult * result, + AzureIfaceData *iface_data, + gboolean is_ipv4) { + NMCSProviderGetConfigTaskData * get_config_data; NMCSProviderGetConfigIfaceData *iface_get_config; - gs_unref_bytes GBytes *response = NULL; - AzureIfaceData * iface_data = user_data; - gs_free_error GError *error = NULL; - const char * fip_str = NULL; - AzureData * azure_data; - - azure_data = iface_data->azure_data; + gs_unref_bytes GBytes *response = NULL; + gs_free_error GError *error = NULL; + const char * fip_str = NULL; + in_addr_t tmp_addr; + int tmp_prefix; nm_http_client_poll_get_finish(http_client, result, NULL, &response, &error); + if (nm_utils_error_is_cancelled(error)) + return; + + get_config_data = iface_data->get_config_data; + if (error) - goto done; + goto out_done; - if (!error) { - in_addr_t tmp_addr; - int tmp_prefix; + fip_str = g_bytes_get_data(response, NULL); + iface_data->iface_get_config = + g_hash_table_lookup(get_config_data->result_dict, iface_data->hwaddr); + iface_get_config = iface_data->iface_get_config; + iface_get_config->iface_idx = iface_data->iface_idx; - fip_str = g_bytes_get_data(response, NULL); - iface_data->iface_get_config = - g_hash_table_lookup(azure_data->get_config_data->result_dict, iface_data->hwaddr); - iface_get_config = iface_data->iface_get_config; - iface_get_config->iface_idx = iface_data->iface_idx; + if (is_ipv4) { + if (!nm_utils_parse_inaddr_bin(AF_INET, fip_str, NULL, &tmp_addr)) { + error = + nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "ip is not a valid private ip address"); + goto out_done; + } + _LOGD("interface[%" G_GSSIZE_FORMAT "]: adding private ip %s", + iface_data->iface_idx, + fip_str); + iface_get_config->ipv4s_arr[iface_get_config->ipv4s_len] = tmp_addr; + iface_get_config->has_ipv4s = TRUE; + iface_get_config->ipv4s_len++; + } else { + tmp_prefix = (_nm_utils_ascii_str_to_int64(fip_str, 10, 0, 32, -1)); - if (is_ipv4) { - if (!nm_utils_parse_inaddr_bin(AF_INET, fip_str, NULL, &tmp_addr)) { - error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, - "ip is not a valid private ip address"); - goto done; - } - _LOGD("interface[%" G_GSSIZE_FORMAT "]: adding private ip %s", - iface_data->iface_idx, - fip_str); - iface_get_config->ipv4s_arr[iface_get_config->ipv4s_len] = tmp_addr; - iface_get_config->has_ipv4s = TRUE; - iface_get_config->ipv4s_len++; - } else { - tmp_prefix = (_nm_utils_ascii_str_to_int64(fip_str, 10, 0, 32, -1)); - - if (tmp_prefix == -1) { - _LOGD("interface[%" G_GSSIZE_FORMAT "]: invalid prefix %d", - iface_data->iface_idx, - tmp_prefix); - goto done; - } - _LOGD("interface[%" G_GSSIZE_FORMAT "]: adding prefix %d", + if (tmp_prefix == -1) { + _LOGD("interface[%" G_GSSIZE_FORMAT "]: invalid prefix %d", iface_data->iface_idx, tmp_prefix); - iface_get_config->cidr_prefix = tmp_prefix; - iface_get_config->has_cidr = TRUE; + goto out_done; } + _LOGD("interface[%" G_GSSIZE_FORMAT "]: adding prefix %d", + iface_data->iface_idx, + tmp_prefix); + iface_get_config->cidr_prefix = tmp_prefix; + iface_get_config->has_cidr = TRUE; } -done: - --iface_data->n_ips_prefix_pending; - if (iface_data->n_ips_prefix_pending == 0) { - _azure_iface_data_free(iface_data); - --azure_data->n_ifaces_pending; - _get_config_maybe_task_return(azure_data, g_steal_pointer(&error)); +out_done: + if (!error) { + --iface_data->n_ips_prefix_pending; + if (iface_data->n_ips_prefix_pending > 0) + return; } + + --get_config_data->n_pending; + _nmcs_provider_get_config_task_maybe_return(get_config_data, g_steal_pointer(&error)); } static void @@ -235,20 +194,24 @@ _get_config_fetch_done_cb_subnet_cidr_prefix(GObject * source, static void _get_config_ips_prefix_list_cb(GObject *source, GAsyncResult *result, gpointer user_data) { - gs_unref_bytes GBytes *response = NULL; - AzureIfaceData * iface_data = user_data; - gs_free_error GError *error = NULL; - const char * response_str = NULL; - gsize response_len; - AzureData * azure_data; - const char * line; - gsize line_len; - - azure_data = iface_data->azure_data; + gs_unref_bytes GBytes *response = NULL; + AzureIfaceData * iface_data = user_data; + gs_free_error GError * error = NULL; + const char * response_str = NULL; + gsize response_len; + NMCSProviderGetConfigTaskData *get_config_data; + const char * line; + gsize line_len; nm_http_client_poll_get_finish(NM_HTTP_CLIENT(source), result, NULL, &response, &error); + + if (nm_utils_error_is_cancelled(error)) + return; + + get_config_data = iface_data->get_config_data; + if (error) - goto done; + goto out_error; response_str = g_bytes_get_data(response, &response_len); /* NMHttpClient guarantees that there is a trailing NUL after the data. */ @@ -293,7 +256,7 @@ _get_config_ips_prefix_list_cb(GObject *source, GAsyncResult *result, gpointer u 10000, 1000, NM_MAKE_STRV(NM_AZURE_METADATA_HEADER), - g_task_get_cancellable(azure_data->get_config_data->task), + get_config_data->intern_cancellable, NULL, NULL, _get_config_fetch_done_cb_private_ipv4s, @@ -319,7 +282,7 @@ _get_config_ips_prefix_list_cb(GObject *source, GAsyncResult *result, gpointer u 10000, 1000, NM_MAKE_STRV(NM_AZURE_METADATA_HEADER), - g_task_get_cancellable(azure_data->get_config_data->task), + get_config_data->intern_cancellable, NULL, NULL, _get_config_fetch_done_cb_subnet_cidr_prefix, @@ -327,47 +290,48 @@ _get_config_ips_prefix_list_cb(GObject *source, GAsyncResult *result, gpointer u } return; -done: - _azure_iface_data_free(iface_data); - --azure_data->n_ifaces_pending; - _get_config_maybe_task_return(azure_data, g_steal_pointer(&error)); +out_error: + --get_config_data->n_pending; + _nmcs_provider_get_config_task_maybe_return(get_config_data, g_steal_pointer(&error)); } static void _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) { + NMCSProviderGetConfigTaskData *get_config_data; gs_unref_bytes GBytes *response = NULL; AzureIfaceData * iface_data = user_data; gs_free_error GError *error = NULL; gs_free const char * uri = NULL; char buf[100]; - AzureData * azure_data; - - azure_data = iface_data->azure_data; nm_http_client_poll_get_finish(NM_HTTP_CLIENT(source), result, NULL, &response, &error); + if (nm_utils_error_is_cancelled(error)) + return; + + get_config_data = iface_data->get_config_data; + if (error) - goto done; + goto out_done; iface_data->hwaddr = nmcs_utils_hwaddr_normalize(g_bytes_get_data(response, NULL), -1); - if (!iface_data->hwaddr) { - goto done; - } + if (!iface_data->hwaddr) + goto out_done; iface_data->iface_get_config = - g_hash_table_lookup(azure_data->get_config_data->result_dict, iface_data->hwaddr); + g_hash_table_lookup(get_config_data->result_dict, iface_data->hwaddr); if (!iface_data->iface_get_config) { - if (!iface_data->azure_data->get_config_data->any) { + if (!get_config_data->any) { _LOGD("interface[%" G_GSSIZE_FORMAT "]: ignore hwaddr %s", iface_data->iface_idx, iface_data->hwaddr); - goto done; + goto out_done; } iface_data->iface_get_config = nmcs_provider_get_config_iface_data_new(FALSE); - g_hash_table_insert(azure_data->get_config_data->result_dict, + g_hash_table_insert(get_config_data->result_dict, g_strdup(iface_data->hwaddr), iface_data->iface_get_config); } @@ -385,26 +349,25 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) 10000, 1000, NM_MAKE_STRV(NM_AZURE_METADATA_HEADER), - g_task_get_cancellable(azure_data->get_config_data->task), + get_config_data->intern_cancellable, NULL, NULL, _get_config_ips_prefix_list_cb, iface_data); return; -done: - nm_g_slice_free(iface_data); - --azure_data->n_ifaces_pending; - _get_config_maybe_task_return(azure_data, g_steal_pointer(&error)); +out_done: + --get_config_data->n_pending; + _nmcs_provider_get_config_task_maybe_return(get_config_data, g_steal_pointer(&error)); } static void _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_data) { + NMCSProviderGetConfigTaskData *get_config_data; gs_unref_ptrarray GPtrArray *ifaces_arr = NULL; gs_unref_bytes GBytes *response = NULL; gs_free_error GError *error = NULL; - AzureData * azure_data = user_data; const char * response_str; gsize response_len; const char * line; @@ -413,8 +376,13 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat nm_http_client_poll_get_finish(NM_HTTP_CLIENT(source), result, NULL, &response, &error); + if (nm_utils_error_is_cancelled(error)) + return; + + get_config_data = user_data; + if (error) { - _get_config_maybe_task_return(azure_data, g_steal_pointer(&error)); + _nmcs_provider_get_config_task_maybe_return(get_config_data, g_steal_pointer(&error)); return; } @@ -422,7 +390,7 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat /* NMHttpClient guarantees that there is a trailing NUL after the data. */ nm_assert(response_str[response_len] == 0); - ifaces_arr = g_ptr_array_new(); + ifaces_arr = g_ptr_array_new_with_free_func((GDestroyNotify) _azure_iface_data_destroy); while (nm_utils_parse_next_line(&response_str, &response_len, &line, &line_len)) { AzureIfaceData *iface_data; @@ -444,8 +412,8 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat iface_data = g_slice_new(AzureIfaceData); *iface_data = (AzureIfaceData){ + .get_config_data = get_config_data, .iface_get_config = NULL, - .azure_data = azure_data, .iface_idx = iface_idx, .n_ips_prefix_pending = 0, .hwaddr = NULL, @@ -456,21 +424,23 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat _LOGD("found azure interfaces: %u", ifaces_arr->len); if (ifaces_arr->len == 0) { - error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "no Azure interfaces found"); - _get_config_maybe_task_return(azure_data, g_steal_pointer(&error)); + _nmcs_provider_get_config_task_maybe_return( + get_config_data, + nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "no Azure interfaces found")); return; } for (i = 0; i < ifaces_arr->len; ++i) { - AzureIfaceData * data = ifaces_arr->pdata[i]; - gs_free const char *uri = NULL; + AzureIfaceData * iface_data = ifaces_arr->pdata[i]; + gs_free const char *uri = NULL; char buf[100]; - _LOGD("azure interface[%" G_GSSIZE_FORMAT "]: retrieving configuration", data->iface_idx); + _LOGD("azure interface[%" G_GSSIZE_FORMAT "]: retrieving configuration", + iface_data->iface_idx); - nm_sprintf_buf(buf, "%" G_GSSIZE_FORMAT "/macAddress", data->iface_idx); + nm_sprintf_buf(buf, "%" G_GSSIZE_FORMAT "/macAddress", iface_data->iface_idx); - azure_data->n_ifaces_pending++; + get_config_data->n_pending++; nm_http_client_poll_get(NM_HTTP_CLIENT(source), (uri = _azure_uri_interfaces(buf)), HTTP_TIMEOUT_MS, @@ -478,25 +448,21 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat 10000, 1000, NM_MAKE_STRV(NM_AZURE_METADATA_HEADER), - g_task_get_cancellable(azure_data->get_config_data->task), + get_config_data->intern_cancellable, NULL, NULL, _get_config_iface_cb, - data); + iface_data); } + + get_config_data->extra_data_destroy = (GDestroyNotify) g_ptr_array_unref; + get_config_data->extra_data = g_steal_pointer(&ifaces_arr); } static void get_config(NMCSProvider *provider, NMCSProviderGetConfigTaskData *get_config_data) { gs_free const char *uri = NULL; - AzureData * azure_data; - - azure_data = g_slice_new(AzureData); - *azure_data = (AzureData){ - .get_config_data = get_config_data, - .n_ifaces_pending = 0, - }; nm_http_client_poll_get(nmcs_provider_get_http_client(provider), (uri = _azure_uri_interfaces()), @@ -505,11 +471,11 @@ get_config(NMCSProvider *provider, NMCSProviderGetConfigTaskData *get_config_dat 15000, 1000, NM_MAKE_STRV(NM_AZURE_METADATA_HEADER), - g_task_get_cancellable(get_config_data->task), + get_config_data->intern_cancellable, NULL, NULL, _get_net_ifaces_list_cb, - azure_data); + get_config_data); } /*****************************************************************************/ diff --git a/clients/cloud-setup/nmcs-provider-ec2.c b/clients/cloud-setup/nmcs-provider-ec2.c index 40bed1575c..276479f254 100644 --- a/clients/cloud-setup/nmcs-provider-ec2.c +++ b/clients/cloud-setup/nmcs-provider-ec2.c @@ -129,124 +129,69 @@ detect(NMCSProvider *provider, GTask *task) /*****************************************************************************/ -typedef struct { - NMCSProviderGetConfigTaskData *get_config_data; - GError * error; - GCancellable * cancellable; - gulong cancelled_id; - guint n_pending; -} GetConfigIfaceData; - -static void -_get_config_task_maybe_return(GetConfigIfaceData *iface_data, GError *error_take) -{ - NMCSProviderGetConfigTaskData *get_config_data = iface_data->get_config_data; - - if (error_take) { - if (!iface_data->error) - iface_data->error = error_take; - else if (!nm_utils_error_is_cancelled(iface_data->error) - && nm_utils_error_is_cancelled(error_take)) { - nm_clear_error(&iface_data->error); - iface_data->error = error_take; - } else - g_error_free(error_take); - - nm_clear_g_cancellable(&iface_data->cancellable); - } - - if (iface_data->n_pending > 0) - return; - - nm_clear_g_cancellable_disconnect(g_task_get_cancellable(get_config_data->task), - &iface_data->cancelled_id); - - nm_clear_g_cancellable(&iface_data->cancellable); - - if (iface_data->error) { - if (nm_utils_error_is_cancelled(iface_data->error)) - _LOGD("get-config: cancelled"); - else - _LOGD("get-config: failed: %s", iface_data->error->message); - g_task_return_error(get_config_data->task, g_steal_pointer(&iface_data->error)); - } else { - _LOGD("get-config: success"); - g_task_return_pointer(get_config_data->task, - g_hash_table_ref(get_config_data->result_dict), - (GDestroyNotify) g_hash_table_unref); - } - - nm_g_slice_free(iface_data); - g_object_unref(get_config_data->task); -} - static void _get_config_fetch_done_cb(NMHttpClient *http_client, GAsyncResult *result, gpointer user_data, gboolean is_local_ipv4) { - GetConfigIfaceData *iface_data; - const char * hwaddr = NULL; - gs_unref_bytes GBytes *response = NULL; - gs_free_error GError *error = NULL; + NMCSProviderGetConfigTaskData *get_config_data; + const char * hwaddr = NULL; + gs_unref_bytes GBytes *response = NULL; + gs_free_error GError * error = NULL; + NMCSProviderGetConfigIfaceData *config_iface_data; + in_addr_t tmp_addr; + int tmp_prefix; - nm_utils_user_data_unpack(user_data, &iface_data, &hwaddr); + nm_utils_user_data_unpack(user_data, &get_config_data, &hwaddr); nm_http_client_poll_get_finish(http_client, result, NULL, &response, &error); - if (!error) { - NMCSProviderGetConfigIfaceData *config_iface_data; - in_addr_t tmp_addr; - int tmp_prefix; + if (nm_utils_error_is_cancelled(error)) + return; - config_iface_data = g_hash_table_lookup(iface_data->get_config_data->result_dict, hwaddr); + if (error) + goto out; - if (is_local_ipv4) { - gs_free const char **s_addrs = NULL; - gsize i, len; + config_iface_data = g_hash_table_lookup(get_config_data->result_dict, hwaddr); - s_addrs = nm_utils_strsplit_set_full(g_bytes_get_data(response, NULL), - "\n", - NM_UTILS_STRSPLIT_SET_FLAGS_STRSTRIP); - len = NM_PTRARRAY_LEN(s_addrs); + if (is_local_ipv4) { + gs_free const char **s_addrs = NULL; + gsize i, len; - nm_assert(!config_iface_data->has_ipv4s); - nm_assert(!config_iface_data->ipv4s_arr); - config_iface_data->has_ipv4s = TRUE; - config_iface_data->ipv4s_len = 0; - if (len > 0) { - config_iface_data->ipv4s_arr = g_new(in_addr_t, len); + s_addrs = nm_utils_strsplit_set_full(g_bytes_get_data(response, NULL), + "\n", + NM_UTILS_STRSPLIT_SET_FLAGS_STRSTRIP); + len = NM_PTRARRAY_LEN(s_addrs); - for (i = 0; i < len; i++) { - if (nm_utils_parse_inaddr_bin(AF_INET, s_addrs[i], NULL, &tmp_addr)) - config_iface_data->ipv4s_arr[config_iface_data->ipv4s_len++] = tmp_addr; - } - } - } else { - if (nm_utils_parse_inaddr_prefix_bin(AF_INET, - g_bytes_get_data(response, NULL), - NULL, - &tmp_addr, - &tmp_prefix)) { - nm_assert(!config_iface_data->has_cidr); - config_iface_data->has_cidr = TRUE; - config_iface_data->cidr_prefix = tmp_prefix; - config_iface_data->cidr_addr = tmp_addr; + nm_assert(!config_iface_data->has_ipv4s); + nm_assert(!config_iface_data->ipv4s_arr); + config_iface_data->has_ipv4s = TRUE; + config_iface_data->ipv4s_len = 0; + if (len > 0) { + config_iface_data->ipv4s_arr = g_new(in_addr_t, len); + + for (i = 0; i < len; i++) { + if (nm_utils_parse_inaddr_bin(AF_INET, s_addrs[i], NULL, &tmp_addr)) + config_iface_data->ipv4s_arr[config_iface_data->ipv4s_len++] = tmp_addr; } } + } else { + if (nm_utils_parse_inaddr_prefix_bin(AF_INET, + g_bytes_get_data(response, NULL), + NULL, + &tmp_addr, + &tmp_prefix)) { + nm_assert(!config_iface_data->has_cidr); + config_iface_data->has_cidr = TRUE; + config_iface_data->cidr_prefix = tmp_prefix; + config_iface_data->cidr_addr = tmp_addr; + } } - /* If nm_utils_error_is_cancelled(error), then our internal iface_data->cancellable - * was cancelled, because the overall request failed. From point of view of the - * caller, this does not mean that a cancellation happened. It also means, our - * request overall is already about to fail. */ - nm_assert(!nm_utils_error_is_cancelled(error) || iface_data->error); - - iface_data->n_pending--; - _get_config_task_maybe_return(iface_data, - nm_utils_error_is_cancelled(error) ? NULL - : g_steal_pointer(&error)); +out: + get_config_data->n_pending--; + _nmcs_provider_get_config_task_maybe_return(get_config_data, g_steal_pointer(&error)); } static void @@ -263,21 +208,6 @@ _get_config_fetch_done_cb_local_ipv4s(GObject *source, GAsyncResult *result, gpo _get_config_fetch_done_cb(NM_HTTP_CLIENT(source), result, user_data, TRUE); } -static void -_get_config_fetch_cancelled_cb(GObject *object, gpointer user_data) -{ - GetConfigIfaceData *iface_data = user_data; - - nm_clear_g_signal_handler(g_task_get_cancellable(iface_data->get_config_data->task), - &iface_data->cancelled_id); - _get_config_task_maybe_return(iface_data, nm_utils_error_new_cancelled(FALSE, NULL)); -} - -typedef struct { - NMCSProviderGetConfigTaskData *get_config_data; - GHashTable * response_parsed; -} GetConfigMetadataData; - typedef struct { gssize iface_idx; char path[0]; @@ -286,60 +216,33 @@ typedef struct { static void _get_config_metadata_ready_cb(GObject *source, GAsyncResult *result, gpointer user_data) { - GetConfigMetadataData * metadata_data = user_data; - GetConfigIfaceData * iface_data; - NMCSProviderGetConfigTaskData *get_config_data = metadata_data->get_config_data; - gs_unref_hashtable GHashTable *response_parsed = - g_steal_pointer(&metadata_data->response_parsed); - gs_free_error GError *error = NULL; - GCancellable * cancellable; + NMCSProviderGetConfigTaskData *get_config_data; + gs_unref_hashtable GHashTable *response_parsed = NULL; + gs_free_error GError *error = NULL; GetConfigMetadataMac *v_mac_data; const char * v_hwaddr; GHashTableIter h_iter; NMHttpClient * http_client; - nm_g_slice_free(metadata_data); - nm_http_client_poll_get_finish(NM_HTTP_CLIENT(source), result, NULL, NULL, &error); - iface_data = g_slice_new(GetConfigIfaceData); - *iface_data = (GetConfigIfaceData){ - .get_config_data = get_config_data, - .n_pending = 0, - }; - - if (nm_utils_error_is_cancelled(error)) { - _get_config_task_maybe_return(iface_data, g_steal_pointer(&error)); + if (nm_utils_error_is_cancelled(error)) return; - } + + get_config_data = user_data; + + response_parsed = g_steal_pointer(&get_config_data->extra_data); + get_config_data->extra_data_destroy = NULL; /* We ignore errors. Only if we got no response at all, it's a problem. * Otherwise, we proceed with whatever we could fetch. */ if (!response_parsed) { - _get_config_task_maybe_return( - iface_data, + _nmcs_provider_get_config_task_maybe_return( + get_config_data, nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "meta data for interfaces not found")); return; } - cancellable = g_task_get_cancellable(get_config_data->task); - if (cancellable) { - gulong cancelled_id; - - cancelled_id = g_cancellable_connect(cancellable, - G_CALLBACK(_get_config_fetch_cancelled_cb), - iface_data, - NULL); - if (cancelled_id == 0) { - /* the callback was already invoked synchronously and the task already returned. */ - return; - } - - iface_data->cancelled_id = cancelled_id; - } - - iface_data->cancellable = g_cancellable_new(); - http_client = nmcs_provider_get_http_client(g_task_get_source_object(get_config_data->task)); g_hash_table_iter_init(&h_iter, response_parsed); @@ -373,7 +276,7 @@ _get_config_metadata_ready_cb(GObject *source, GAsyncResult *result, gpointer us hwaddr, v_mac_data->path); - iface_data->n_pending++; + get_config_data->n_pending++; nm_http_client_poll_get( http_client, (uri1 = _ec2_uri_interfaces(v_mac_data->path, @@ -384,13 +287,13 @@ _get_config_metadata_ready_cb(GObject *source, GAsyncResult *result, gpointer us 10000, 1000, NULL, - iface_data->cancellable, + get_config_data->intern_cancellable, NULL, NULL, _get_config_fetch_done_cb_subnet_ipv4_cidr_block, - nm_utils_user_data_pack(iface_data, hwaddr)); + nm_utils_user_data_pack(get_config_data, hwaddr)); - iface_data->n_pending++; + get_config_data->n_pending++; nm_http_client_poll_get( http_client, (uri2 = _ec2_uri_interfaces(v_mac_data->path, @@ -401,14 +304,14 @@ _get_config_metadata_ready_cb(GObject *source, GAsyncResult *result, gpointer us 10000, 1000, NULL, - iface_data->cancellable, + get_config_data->intern_cancellable, NULL, NULL, _get_config_fetch_done_cb_local_ipv4s, - nm_utils_user_data_pack(iface_data, hwaddr)); + nm_utils_user_data_pack(get_config_data, hwaddr)); } - _get_config_task_maybe_return(iface_data, NULL); + _nmcs_provider_get_config_task_maybe_return(get_config_data, NULL); } static gboolean @@ -417,7 +320,7 @@ _get_config_metadata_ready_check(long response_code, gpointer check_user_data, GError **error) { - GetConfigMetadataData *metadata_data = check_user_data; + NMCSProviderGetConfigTaskData *get_config_data = check_user_data; gs_unref_hashtable GHashTable *response_parsed = NULL; const guint8 * r_data; const char * cur_line; @@ -465,7 +368,7 @@ _get_config_metadata_ready_check(long response_code, } has_all = TRUE; - g_hash_table_iter_init(&h_iter, metadata_data->get_config_data->result_dict); + g_hash_table_iter_init(&h_iter, get_config_data->result_dict); while (g_hash_table_iter_next(&h_iter, (gpointer *) &c_hwaddr, NULL)) { if (!response_parsed || !g_hash_table_contains(response_parsed, c_hwaddr)) { has_all = FALSE; @@ -473,21 +376,18 @@ _get_config_metadata_ready_check(long response_code, } } - nm_clear_pointer(&metadata_data->response_parsed, g_hash_table_unref); - metadata_data->response_parsed = g_steal_pointer(&response_parsed); + nm_clear_pointer(&get_config_data->extra_data, g_hash_table_unref); + if (response_parsed) { + get_config_data->extra_data = g_steal_pointer(&response_parsed); + get_config_data->extra_data_destroy = (GDestroyNotify) g_hash_table_unref; + } return has_all; } static void get_config(NMCSProvider *provider, NMCSProviderGetConfigTaskData *get_config_data) { - gs_free char * uri = NULL; - GetConfigMetadataData *metadata_data; - - metadata_data = g_slice_new(GetConfigMetadataData); - *metadata_data = (GetConfigMetadataData){ - .get_config_data = get_config_data, - }; + gs_free char *uri = NULL; /* First we fetch the "macs/". If the caller requested some particular * MAC addresses, then we poll until we see them. They might not yet be @@ -500,11 +400,11 @@ get_config(NMCSProvider *provider, NMCSProviderGetConfigTaskData *get_config_dat 15000, 1000, NULL, - g_task_get_cancellable(get_config_data->task), + get_config_data->intern_cancellable, _get_config_metadata_ready_check, - metadata_data, + get_config_data, _get_config_metadata_ready_cb, - metadata_data); + get_config_data); } /*****************************************************************************/ diff --git a/clients/cloud-setup/nmcs-provider-gcp.c b/clients/cloud-setup/nmcs-provider-gcp.c index 45e1c3622b..a8b50f98f7 100644 --- a/clients/cloud-setup/nmcs-provider-gcp.c +++ b/clients/cloud-setup/nmcs-provider-gcp.c @@ -89,57 +89,22 @@ detect(NMCSProvider *provider, GTask *task) /*****************************************************************************/ typedef struct { - NMCSProviderGetConfigTaskData *get_config_data; - guint n_ifaces_pending; - GError * error; -} GCPData; - -typedef struct { + NMCSProviderGetConfigTaskData * get_config_data; NMCSProviderGetConfigIfaceData *iface_get_config; - GCPData * gcp_data; gssize iface_idx; guint n_fips_pending; } GCPIfaceData; static void -_get_config_maybe_task_return(GCPData *gcp_data, GError *error_take) +_gcp_iface_data_destroy(GCPIfaceData *iface_data) { - NMCSProviderGetConfigTaskData *get_config_data = gcp_data->get_config_data; - - if (error_take) { - if (!gcp_data->error) - gcp_data->error = error_take; - else if (!nm_utils_error_is_cancelled(gcp_data->error) - && nm_utils_error_is_cancelled(error_take)) { - nm_clear_error(&gcp_data->error); - gcp_data->error = error_take; - } else - g_error_free(error_take); - } - - if (gcp_data->n_ifaces_pending > 0) - return; - - if (gcp_data->error) { - if (nm_utils_error_is_cancelled(gcp_data->error)) - _LOGD("get-config: cancelled"); - else - _LOGD("get-config: failed: %s", gcp_data->error->message); - g_task_return_error(get_config_data->task, g_steal_pointer(&gcp_data->error)); - } else { - _LOGD("get-config: success"); - g_task_return_pointer(get_config_data->task, - g_hash_table_ref(get_config_data->result_dict), - (GDestroyNotify) g_hash_table_unref); - } - - nm_g_slice_free(gcp_data); - g_object_unref(get_config_data->task); + nm_g_slice_free(iface_data); } static void _get_config_fip_cb(GObject *source, GAsyncResult *result, gpointer user_data) { + NMCSProviderGetConfigTaskData * get_config_data; NMCSProviderGetConfigIfaceData *iface_get_config; gs_unref_bytes GBytes *response = NULL; GCPIfaceData * iface_data = user_data; @@ -147,20 +112,22 @@ _get_config_fip_cb(GObject *source, GAsyncResult *result, gpointer user_data) const char * fip_str = NULL; NMIPRoute ** routes_arr; NMIPRoute * route_new; - GCPData * gcp_data; - - gcp_data = iface_data->gcp_data; nm_http_client_poll_get_finish(NM_HTTP_CLIENT(source), result, NULL, &response, &error); + if (nm_utils_error_is_cancelled(error)) + return; + + get_config_data = iface_data->get_config_data; + if (error) - goto iface_done; + goto out_done; fip_str = g_bytes_get_data(response, NULL); if (!nm_utils_ipaddr_valid(AF_INET, fip_str)) { error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "forwarded-ip is not a valid ip address"); - goto iface_done; + goto out_done; } _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: adding forwarded-ip %s", @@ -173,42 +140,46 @@ _get_config_fip_cb(GObject *source, GAsyncResult *result, gpointer user_data) route_new = nm_ip_route_new(AF_INET, fip_str, 32, NULL, 100, &error); if (error) - goto iface_done; + goto out_done; nm_ip_route_set_attribute(route_new, NM_IP_ROUTE_ATTRIBUTE_TYPE, g_variant_new_string("local")); routes_arr[iface_get_config->iproutes_len] = route_new; ++iface_get_config->iproutes_len; -iface_done: - --iface_data->n_fips_pending; - if (iface_data->n_fips_pending == 0) { - nm_g_slice_free(iface_data); - --gcp_data->n_ifaces_pending; +out_done: + if (!error) { + --iface_data->n_fips_pending; + if (iface_data->n_fips_pending > 0) + return; } - _get_config_maybe_task_return(gcp_data, g_steal_pointer(&error)); + --get_config_data->n_pending; + _nmcs_provider_get_config_task_maybe_return(get_config_data, g_steal_pointer(&error)); } static void _get_config_ips_list_cb(GObject *source, GAsyncResult *result, gpointer user_data) { + NMCSProviderGetConfigTaskData *get_config_data; gs_unref_ptrarray GPtrArray *uri_arr = NULL; gs_unref_bytes GBytes *response = NULL; GCPIfaceData * iface_data = user_data; gs_free_error GError *error = NULL; const char * response_str = NULL; gsize response_len; - GCPData * gcp_data; const char * line; gsize line_len; guint i; - gcp_data = iface_data->gcp_data; - nm_http_client_poll_get_finish(NM_HTTP_CLIENT(source), result, NULL, &response, &error); + if (nm_utils_error_is_cancelled(error)) + return; + + get_config_data = iface_data->get_config_data; + if (error) - goto fips_error; + goto out_error; response_str = g_bytes_get_data(response, &response_len); /* NMHttpClient guarantees that there is a trailing NUL after the data. */ @@ -240,7 +211,7 @@ _get_config_ips_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat if (iface_data->n_fips_pending == 0) { error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "found no forwarded ip"); - goto fips_error; + goto out_error; } iface_data->iface_get_config->iproutes_arr = g_new(NMIPRoute *, iface_data->n_fips_pending); @@ -256,7 +227,7 @@ _get_config_ips_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat HTTP_POLL_TIMEOUT_MS, HTTP_RATE_LIMIT_MS, NM_MAKE_STRV(NM_GCP_METADATA_HEADER), - g_task_get_cancellable(gcp_data->get_config_data->task), + get_config_data->intern_cancellable, NULL, NULL, _get_config_fip_cb, @@ -264,39 +235,40 @@ _get_config_ips_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat } return; -fips_error: - nm_g_slice_free(iface_data); - --gcp_data->n_ifaces_pending; - _get_config_maybe_task_return(gcp_data, g_steal_pointer(&error)); +out_error: + --get_config_data->n_pending; + _nmcs_provider_get_config_task_maybe_return(get_config_data, g_steal_pointer(&error)); } static void _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) { - gs_unref_bytes GBytes *response = NULL; - GCPIfaceData * iface_data = user_data; - gs_free_error GError *error = NULL; - gs_free const char * hwaddr = NULL; - gs_free const char * uri = NULL; - char sbuf[100]; - GCPData * gcp_data; - - gcp_data = iface_data->gcp_data; + gs_unref_bytes GBytes *response = NULL; + GCPIfaceData * iface_data = user_data; + gs_free_error GError * error = NULL; + gs_free const char * hwaddr = NULL; + gs_free const char * uri = NULL; + char sbuf[100]; + NMCSProviderGetConfigTaskData *get_config_data; nm_http_client_poll_get_finish(NM_HTTP_CLIENT(source), result, NULL, &response, &error); + if (nm_utils_error_is_cancelled(error)) + return; + + get_config_data = iface_data->get_config_data; + if (error) - goto iface_error; + goto out_error; hwaddr = nmcs_utils_hwaddr_normalize(g_bytes_get_data(response, NULL), -1); - iface_data->iface_get_config = - g_hash_table_lookup(gcp_data->get_config_data->result_dict, hwaddr); + iface_data->iface_get_config = g_hash_table_lookup(get_config_data->result_dict, hwaddr); if (!iface_data->iface_get_config) { _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: did not find a matching device", iface_data->iface_idx); error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "no matching hwaddr found for GCP interface"); - goto iface_error; + goto out_error; } _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: found a matching device with hwaddr %s", @@ -312,17 +284,16 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) HTTP_POLL_TIMEOUT_MS, HTTP_RATE_LIMIT_MS, NM_MAKE_STRV(NM_GCP_METADATA_HEADER), - g_task_get_cancellable(gcp_data->get_config_data->task), + get_config_data->intern_cancellable, NULL, NULL, _get_config_ips_list_cb, iface_data); return; -iface_error: - nm_g_slice_free(iface_data); - --gcp_data->n_ifaces_pending; - _get_config_maybe_task_return(gcp_data, g_steal_pointer(&error)); +out_error: + --get_config_data->n_pending; + _nmcs_provider_get_config_task_maybe_return(get_config_data, g_steal_pointer(&error)); } static void @@ -330,18 +301,23 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat { gs_unref_ptrarray GPtrArray *ifaces_arr = NULL; gs_unref_bytes GBytes *response = NULL; - gs_free_error GError *error = NULL; - GCPData * gcp_data = user_data; - const char * response_str; - gsize response_len; - const char * line; - gsize line_len; - guint i; + gs_free_error GError * error = NULL; + NMCSProviderGetConfigTaskData *get_config_data; + const char * response_str; + gsize response_len; + const char * line; + gsize line_len; + guint i; nm_http_client_poll_get_finish(NM_HTTP_CLIENT(source), result, NULL, &response, &error); + if (nm_utils_error_is_cancelled(error)) + return; + + get_config_data = user_data; + if (error) { - _get_config_maybe_task_return(gcp_data, g_steal_pointer(&error)); + _nmcs_provider_get_config_task_maybe_return(get_config_data, g_steal_pointer(&error)); return; } @@ -349,7 +325,7 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat /* NMHttpClient guarantees that there is a trailing NUL after the data. */ nm_assert(response_str[response_len] == 0); - ifaces_arr = g_ptr_array_new(); + ifaces_arr = g_ptr_array_new_with_free_func((GDestroyNotify) _gcp_iface_data_destroy); while (nm_utils_parse_next_line(&response_str, &response_len, &line, &line_len)) { GCPIfaceData *iface_data; @@ -370,17 +346,23 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat iface_data = g_slice_new(GCPIfaceData); *iface_data = (GCPIfaceData){ + .get_config_data = get_config_data, .iface_get_config = NULL, - .gcp_data = gcp_data, .iface_idx = iface_idx, .n_fips_pending = 0, }; g_ptr_array_add(ifaces_arr, iface_data); } - gcp_data->n_ifaces_pending = ifaces_arr->len; _LOGI("found GCP interfaces: %u", ifaces_arr->len); + if (ifaces_arr->len == 0) { + _nmcs_provider_get_config_task_maybe_return( + get_config_data, + nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "no GCP interfaces found")); + return; + } + for (i = 0; i < ifaces_arr->len; ++i) { GCPIfaceData * data = ifaces_arr->pdata[i]; gs_free const char *uri = NULL; @@ -390,6 +372,7 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat nm_sprintf_buf(sbuf, "%" G_GSSIZE_FORMAT "/mac", data->iface_idx); + get_config_data->n_pending++; nm_http_client_poll_get(NM_HTTP_CLIENT(source), (uri = _gcp_uri_interfaces(sbuf)), HTTP_TIMEOUT_MS, @@ -397,31 +380,21 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat HTTP_POLL_TIMEOUT_MS, HTTP_RATE_LIMIT_MS, NM_MAKE_STRV(NM_GCP_METADATA_HEADER), - g_task_get_cancellable(gcp_data->get_config_data->task), + get_config_data->intern_cancellable, NULL, NULL, _get_config_iface_cb, data); } - if (ifaces_arr->len == 0) { - error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "no GCP interfaces found"); - _get_config_maybe_task_return(gcp_data, g_steal_pointer(&error)); - } + get_config_data->extra_data = g_steal_pointer(&ifaces_arr); + get_config_data->extra_data_destroy = (GDestroyNotify) g_ptr_array_unref; } static void get_config(NMCSProvider *provider, NMCSProviderGetConfigTaskData *get_config_data) { gs_free const char *uri = NULL; - GCPData * gcp_data; - - gcp_data = g_slice_new(GCPData); - *gcp_data = (GCPData){ - .get_config_data = get_config_data, - .n_ifaces_pending = 0, - .error = NULL, - }; nm_http_client_poll_get(nmcs_provider_get_http_client(provider), (uri = _gcp_uri_interfaces()), @@ -430,11 +403,11 @@ get_config(NMCSProvider *provider, NMCSProviderGetConfigTaskData *get_config_dat HTTP_POLL_TIMEOUT_MS, HTTP_RATE_LIMIT_MS, NM_MAKE_STRV(NM_GCP_METADATA_HEADER), - g_task_get_cancellable(gcp_data->get_config_data->task), + get_config_data->intern_cancellable, NULL, NULL, _get_net_ifaces_list_cb, - gcp_data); + get_config_data); } /*****************************************************************************/ diff --git a/clients/cloud-setup/nmcs-provider.c b/clients/cloud-setup/nmcs-provider.c index 9fac4b6679..6512b76faf 100644 --- a/clients/cloud-setup/nmcs-provider.c +++ b/clients/cloud-setup/nmcs-provider.c @@ -114,18 +114,60 @@ _iface_data_free(gpointer data) } static void -_get_config_data_free(gpointer data) +_get_config_task_maybe_return(NMCSProviderGetConfigTaskData *get_config_data, GError *error_take) { - NMCSProviderGetConfigTaskData *get_config_data = data; + gs_free_error GError *error = error_take; - if (get_config_data->extra_destroy) - get_config_data->extra_destroy(get_config_data->extra_data); + nm_assert(get_config_data); + nm_assert(G_IS_TASK(get_config_data->task)); + + if (!error) { + if (get_config_data->n_pending > 0) + return; + } + + g_cancellable_cancel(get_config_data->intern_cancellable); + + if (error) { + if (nm_utils_error_is_cancelled(error)) + _LOGD("get-config: cancelled"); + else + _LOGD("get-config: failed: %s", error->message); + g_task_return_error(get_config_data->task, g_steal_pointer(&error)); + } else { + _LOGD("get-config: success"); + g_task_return_pointer(get_config_data->task, + g_hash_table_ref(get_config_data->result_dict), + (GDestroyNotify) g_hash_table_unref); + } + + nm_clear_g_signal_handler(g_task_get_cancellable(get_config_data->task), + &get_config_data->extern_cancelled_id); + + if (get_config_data->extra_data_destroy) + get_config_data->extra_data_destroy(get_config_data->extra_data); nm_clear_pointer(&get_config_data->result_dict, g_hash_table_unref); + nm_g_object_unref(get_config_data->intern_cancellable); + g_object_unref(get_config_data->task); nm_g_slice_free(get_config_data); } +void +_nmcs_provider_get_config_task_maybe_return(NMCSProviderGetConfigTaskData *get_config_data, + GError * error_take) +{ + nm_assert(!error_take || !nm_utils_error_is_cancelled(error_take)); + _get_config_task_maybe_return(get_config_data, error_take); +} + +static void +_get_config_cancelled_cb(GObject *object, gpointer user_data) +{ + _get_config_task_maybe_return(user_data, nm_utils_error_new_cancelled(FALSE, NULL)); +} + void nmcs_provider_get_config(NMCSProvider * self, gboolean any, @@ -139,6 +181,8 @@ nmcs_provider_get_config(NMCSProvider * self, g_return_if_fail(NMCS_IS_PROVIDER(self)); g_return_if_fail(!cancellable || G_IS_CANCELLABLE(cancellable)); + _LOGD("get-config: starting"); + get_config_data = g_slice_new(NMCSProviderGetConfigTaskData); *get_config_data = (NMCSProviderGetConfigTaskData){ .task = nm_g_task_new(self, cancellable, nmcs_provider_get_config, callback, user_data), @@ -146,8 +190,6 @@ nmcs_provider_get_config(NMCSProvider * self, .result_dict = g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, _iface_data_free), }; - g_task_set_task_data(get_config_data->task, get_config_data, _get_config_data_free); - nmcs_wait_for_objects_register(get_config_data->task); for (; hwaddrs && hwaddrs[0]; hwaddrs++) { @@ -156,7 +198,21 @@ nmcs_provider_get_config(NMCSProvider * self, nmcs_provider_get_config_iface_data_new(TRUE)); } - _LOGD("get-config: starting"); + if (cancellable) { + gulong cancelled_id; + + cancelled_id = g_cancellable_connect(cancellable, + G_CALLBACK(_get_config_cancelled_cb), + get_config_data, + NULL); + if (cancelled_id == 0) { + /* the callback was already invoked synchronously and the task already returned. */ + return; + } + + get_config_data->extern_cancelled_id = cancelled_id; + get_config_data->intern_cancellable = g_cancellable_new(); + } NMCS_PROVIDER_GET_CLASS(self)->get_config(self, get_config_data); } diff --git a/clients/cloud-setup/nmcs-provider.h b/clients/cloud-setup/nmcs-provider.h index a26c6a366b..3edd874337 100644 --- a/clients/cloud-setup/nmcs-provider.h +++ b/clients/cloud-setup/nmcs-provider.h @@ -37,11 +37,25 @@ nmcs_provider_get_config_iface_data_is_valid(const NMCSProviderGetConfigIfaceDat NMCSProviderGetConfigIfaceData *nmcs_provider_get_config_iface_data_new(gboolean was_requested); typedef struct { - GTask * task; - GHashTable * result_dict; + GTask *task; + + GHashTable *result_dict; + + /* this cancellable should be used for the provider implementation + * to listen for cancellation. */ + GCancellable *intern_cancellable; + + /* the provider implementation may attach extra data. */ gpointer extra_data; - GDestroyNotify extra_destroy; - bool any : 1; + GDestroyNotify extra_data_destroy; + + gulong extern_cancelled_id; + + /* the provider implementation may use this field to track the number of pending + * operations. */ + guint n_pending; + + bool any : 1; } NMCSProviderGetConfigTaskData; #define NMCS_TYPE_PROVIDER (nmcs_provider_get_type()) @@ -93,6 +107,9 @@ gboolean nmcs_provider_detect_finish(NMCSProvider *provider, GAsyncResult *resul /*****************************************************************************/ +void _nmcs_provider_get_config_task_maybe_return(NMCSProviderGetConfigTaskData *get_config_data, + GError * error_take); + void nmcs_provider_get_config(NMCSProvider * provider, gboolean any, const char *const * hwaddrs, From 2ba984a80a1a6b299692e6144c63f2068e3d21f4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 5 Jan 2021 13:58:09 +0100 Subject: [PATCH 05/13] cloud-setup: strip whitespace from nmcs_utils_hwaddr_normalize() This function should be accepting, and not reject leading/trailing white space. --- clients/cloud-setup/nm-cloud-setup-utils.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/clients/cloud-setup/nm-cloud-setup-utils.c b/clients/cloud-setup/nm-cloud-setup-utils.c index 38eb0bcf72..f09d1e5988 100644 --- a/clients/cloud-setup/nm-cloud-setup-utils.c +++ b/clients/cloud-setup/nm-cloud-setup-utils.c @@ -485,22 +485,30 @@ char * nmcs_utils_hwaddr_normalize(const char *hwaddr, gssize len) { gs_free char *hwaddr_clone = NULL; + char * hw; guint8 buf[ETH_ALEN]; + gsize l; nm_assert(len >= -1); if (len < 0) { if (!hwaddr) return NULL; - } else { - if (len == 0) - return NULL; - nm_assert(hwaddr); - hwaddr = nm_strndup_a(300, hwaddr, len, &hwaddr_clone); - } + l = strlen(hwaddr); + } else + l = len; + + if (l == 0) + return NULL; + + nm_assert(hwaddr); + hw = nm_strndup_a(300, hwaddr, l, &hwaddr_clone); + + g_strstrip(hw); + /* we cannot use _nm_utils_hwaddr_aton() because that requires a delimiter. * Azure exposes MAC addresses without delimiter, so accept that too. */ - if (!nm_utils_hexstr2bin_full(hwaddr, + if (!nm_utils_hexstr2bin_full(hw, FALSE, FALSE, FALSE, From 511b4ab4112bf21bf0b42ccea85db228ffe720b5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 5 Jan 2021 14:08:24 +0100 Subject: [PATCH 06/13] cloud-setup: add and use nmcs_utils_hwaddr_normalize_gbytes() Previously we would call nmcs_utils_hwaddr_normalize(g_bytes_get_data(response, NULL), -1); which treats the data in response as NUL terminated. That is not entirely wrong, because the HTTP request's response is guaranteed to have a NUL termination at the end. However, it doesn't seam to good either. For one, we already have the length. Use it. But also, if the response contains any NUL bytes in the middle, then this would wrongly only consider the first line. We should not accept "00:11:22:33:44:55\0bogus" as valid. While at it, reject NUL characters from nmcs_utils_hwaddr_normalize() -- except one NUL at the end. --- clients/cloud-setup/nm-cloud-setup-utils.c | 11 ++++++++++- clients/cloud-setup/nm-cloud-setup-utils.h | 10 ++++++++++ clients/cloud-setup/nmcs-provider-azure.c | 2 +- clients/cloud-setup/nmcs-provider-gcp.c | 2 +- 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/clients/cloud-setup/nm-cloud-setup-utils.c b/clients/cloud-setup/nm-cloud-setup-utils.c index f09d1e5988..24b3ee6a29 100644 --- a/clients/cloud-setup/nm-cloud-setup-utils.c +++ b/clients/cloud-setup/nm-cloud-setup-utils.c @@ -495,8 +495,17 @@ nmcs_utils_hwaddr_normalize(const char *hwaddr, gssize len) if (!hwaddr) return NULL; l = strlen(hwaddr); - } else + } else { l = len; + if (l > 0 && hwaddr[l - 1] == '\0') { + /* we accept one '\0' at the end of the string. */ + l--; + } + if (memchr(hwaddr, '\0', l)) { + /* but we don't accept other NUL characters in the middle. */ + return NULL; + } + } if (l == 0) return NULL; diff --git a/clients/cloud-setup/nm-cloud-setup-utils.h b/clients/cloud-setup/nm-cloud-setup-utils.h index c2a3d30949..217f07550e 100644 --- a/clients/cloud-setup/nm-cloud-setup-utils.h +++ b/clients/cloud-setup/nm-cloud-setup-utils.h @@ -78,6 +78,16 @@ gboolean nmcs_utils_poll_finish(GAsyncResult *result, gpointer *probe_user_data, char *nmcs_utils_hwaddr_normalize(const char *hwaddr, gssize len); +static inline char * +nmcs_utils_hwaddr_normalize_gbytes(GBytes *hwaddr) +{ + const char *str; + gsize len; + + str = g_bytes_get_data(hwaddr, &len); + return nmcs_utils_hwaddr_normalize(str, len); +} + /*****************************************************************************/ const char *nmcs_utils_parse_memmem(GBytes *mem, const char *needle); diff --git a/clients/cloud-setup/nmcs-provider-azure.c b/clients/cloud-setup/nmcs-provider-azure.c index a18a0d64b4..65ff058c77 100644 --- a/clients/cloud-setup/nmcs-provider-azure.c +++ b/clients/cloud-setup/nmcs-provider-azure.c @@ -315,7 +315,7 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) if (error) goto out_done; - iface_data->hwaddr = nmcs_utils_hwaddr_normalize(g_bytes_get_data(response, NULL), -1); + iface_data->hwaddr = nmcs_utils_hwaddr_normalize_gbytes(response); if (!iface_data->hwaddr) goto out_done; diff --git a/clients/cloud-setup/nmcs-provider-gcp.c b/clients/cloud-setup/nmcs-provider-gcp.c index a8b50f98f7..d30749484f 100644 --- a/clients/cloud-setup/nmcs-provider-gcp.c +++ b/clients/cloud-setup/nmcs-provider-gcp.c @@ -261,7 +261,7 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) if (error) goto out_error; - hwaddr = nmcs_utils_hwaddr_normalize(g_bytes_get_data(response, NULL), -1); + hwaddr = nmcs_utils_hwaddr_normalize_gbytes(response); iface_data->iface_get_config = g_hash_table_lookup(get_config_data->result_dict, hwaddr); if (!iface_data->iface_get_config) { _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: did not find a matching device", From f0faf2e1a1eeaa15f7e84c1ccc8d4d8b28d2fe9f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 5 Jan 2021 14:20:48 +0100 Subject: [PATCH 07/13] cloud-setup: handle unknown interaces in get_config() for GCP/Azure The API of mcs_provider_get_config() allows to explicitly request for certain interfaces (MAC addresses), but it also allows to fetch any. That means, the result dictionary will be pre-populated with the MAC addresses that were requested, but if we encounter an unknown interface, then that is not a reason to fail. --- clients/cloud-setup/nmcs-provider-azure.c | 46 ++++++++++------ clients/cloud-setup/nmcs-provider-ec2.c | 2 + clients/cloud-setup/nmcs-provider-gcp.c | 66 +++++++++++++++++------ clients/cloud-setup/nmcs-provider.h | 15 ++++-- 4 files changed, 92 insertions(+), 37 deletions(-) diff --git a/clients/cloud-setup/nmcs-provider-azure.c b/clients/cloud-setup/nmcs-provider-azure.c index 65ff058c77..b2490b87f9 100644 --- a/clients/cloud-setup/nmcs-provider-azure.c +++ b/clients/cloud-setup/nmcs-provider-azure.c @@ -98,13 +98,12 @@ typedef struct { NMCSProviderGetConfigIfaceData *iface_get_config; gssize iface_idx; guint n_ips_prefix_pending; - char * hwaddr; + const char * hwaddr; } AzureIfaceData; static void _azure_iface_data_destroy(AzureIfaceData *iface_data) { - g_free(iface_data->hwaddr); nm_g_slice_free(iface_data); } @@ -135,8 +134,7 @@ _get_config_fetch_done_cb(NMHttpClient * http_client, fip_str = g_bytes_get_data(response, NULL); iface_data->iface_get_config = g_hash_table_lookup(get_config_data->result_dict, iface_data->hwaddr); - iface_get_config = iface_data->iface_get_config; - iface_get_config->iface_idx = iface_data->iface_idx; + iface_get_config = iface_data->iface_get_config; if (is_ipv4) { if (!nm_utils_parse_inaddr_bin(AF_INET, fip_str, NULL, &tmp_addr)) { @@ -301,6 +299,7 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) NMCSProviderGetConfigTaskData *get_config_data; gs_unref_bytes GBytes *response = NULL; AzureIfaceData * iface_data = user_data; + gs_free char * v_hwaddr = NULL; gs_free_error GError *error = NULL; gs_free const char * uri = NULL; char buf[100]; @@ -315,27 +314,44 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) if (error) goto out_done; - iface_data->hwaddr = nmcs_utils_hwaddr_normalize_gbytes(response); - - if (!iface_data->hwaddr) + v_hwaddr = nmcs_utils_hwaddr_normalize_gbytes(response); + if (!v_hwaddr) { + _LOGI("interface[%" G_GSSIZE_FORMAT "]: invalid MAC address returned", + iface_data->iface_idx); + error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, + "invalid MAC address for index %" G_GSSIZE_FORMAT, + iface_data->iface_idx); goto out_done; + } - iface_data->iface_get_config = - g_hash_table_lookup(get_config_data->result_dict, iface_data->hwaddr); - - if (!iface_data->iface_get_config) { + if (!g_hash_table_lookup_extended(get_config_data->result_dict, + v_hwaddr, + (gpointer *) &iface_data->hwaddr, + (gpointer *) &iface_data->iface_get_config)) { if (!get_config_data->any) { - _LOGD("interface[%" G_GSSIZE_FORMAT "]: ignore hwaddr %s", - iface_data->iface_idx, - iface_data->hwaddr); + _LOGD("get-config: skip fetching meta data for %s (%" G_GSSIZE_FORMAT ")", + v_hwaddr, + iface_data->iface_idx); goto out_done; } iface_data->iface_get_config = nmcs_provider_get_config_iface_data_new(FALSE); g_hash_table_insert(get_config_data->result_dict, - g_strdup(iface_data->hwaddr), + (char *) (iface_data->hwaddr = g_steal_pointer(&v_hwaddr)), iface_data->iface_get_config); + } else { + if (iface_data->iface_get_config->iface_idx >= 0) { + _LOGI("interface[%" G_GSSIZE_FORMAT "]: duplicate MAC address %s returned", + iface_data->iface_idx, + iface_data->hwaddr); + error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, + "duplicate MAC address for index %" G_GSSIZE_FORMAT, + iface_data->iface_idx); + goto out_done; + } } + iface_data->iface_get_config->iface_idx = iface_data->iface_idx; + _LOGD("interface[%" G_GSSIZE_FORMAT "]: found a matching device with hwaddr %s", iface_data->iface_idx, iface_data->hwaddr); diff --git a/clients/cloud-setup/nmcs-provider-ec2.c b/clients/cloud-setup/nmcs-provider-ec2.c index 276479f254..49b909bf77 100644 --- a/clients/cloud-setup/nmcs-provider-ec2.c +++ b/clients/cloud-setup/nmcs-provider-ec2.c @@ -269,6 +269,7 @@ _get_config_metadata_ready_cb(GObject *source, GAsyncResult *result, gpointer us } nm_assert(config_iface_data->iface_idx == -1); + config_iface_data->iface_idx = v_mac_data->iface_idx; _LOGD("get-config: start fetching meta data for #%" G_GSSIZE_FORMAT ", %s (%s)", @@ -364,6 +365,7 @@ _get_config_metadata_ready_check(long response_code, mac_data->iface_idx = iface_idx_counter++; memcpy(mac_data->path, cur_line, cur_line_len + 1u); + /* here we will ignore duplicate responses. */ g_hash_table_insert(response_parsed, hwaddr, mac_data); } diff --git a/clients/cloud-setup/nmcs-provider-gcp.c b/clients/cloud-setup/nmcs-provider-gcp.c index d30749484f..aa01356113 100644 --- a/clients/cloud-setup/nmcs-provider-gcp.c +++ b/clients/cloud-setup/nmcs-provider-gcp.c @@ -134,9 +134,8 @@ _get_config_fip_cb(GObject *source, GAsyncResult *result, gpointer user_data) iface_data->iface_idx, fip_str); - iface_get_config = iface_data->iface_get_config; - iface_get_config->iface_idx = iface_data->iface_idx; - routes_arr = iface_get_config->iproutes_arr; + iface_get_config = iface_data->iface_get_config; + routes_arr = iface_get_config->iproutes_arr; route_new = nm_ip_route_new(AF_INET, fip_str, 32, NULL, 100, &error); if (error) @@ -243,13 +242,15 @@ out_error: static void _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) { - gs_unref_bytes GBytes *response = NULL; - GCPIfaceData * iface_data = user_data; - gs_free_error GError * error = NULL; - gs_free const char * hwaddr = NULL; - gs_free const char * uri = NULL; + gs_unref_bytes GBytes *response = NULL; + GCPIfaceData * iface_data = user_data; + gs_free_error GError * error = NULL; + gs_free char * v_hwaddr = NULL; + const char * hwaddr = NULL; + gs_free const char * uri = NULL; char sbuf[100]; NMCSProviderGetConfigTaskData *get_config_data; + gboolean is_requested; nm_http_client_poll_get_finish(NM_HTTP_CLIENT(source), result, NULL, &response, &error); @@ -259,20 +260,51 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) get_config_data = iface_data->get_config_data; if (error) - goto out_error; + goto out_done; - hwaddr = nmcs_utils_hwaddr_normalize_gbytes(response); - iface_data->iface_get_config = g_hash_table_lookup(get_config_data->result_dict, hwaddr); - if (!iface_data->iface_get_config) { - _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: did not find a matching device", + v_hwaddr = nmcs_utils_hwaddr_normalize_gbytes(response); + if (!v_hwaddr) { + _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: invalid MAC address returned", iface_data->iface_idx); error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, - "no matching hwaddr found for GCP interface"); - goto out_error; + "invalid MAC address for index %" G_GSSIZE_FORMAT, + iface_data->iface_idx); + goto out_done; } - _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: found a matching device with hwaddr %s", + if (!g_hash_table_lookup_extended(get_config_data->result_dict, + v_hwaddr, + (gpointer *) &hwaddr, + (gpointer *) &iface_data->iface_get_config)) { + if (!get_config_data->any) { + _LOGD("get-config: skip fetching meta data for %s (%" G_GSSIZE_FORMAT ")", + v_hwaddr, + iface_data->iface_idx); + goto out_done; + } + iface_data->iface_get_config = nmcs_provider_get_config_iface_data_new(FALSE); + g_hash_table_insert(get_config_data->result_dict, + (char *) (hwaddr = g_steal_pointer(&v_hwaddr)), + iface_data->iface_get_config); + is_requested = FALSE; + } else { + if (iface_data->iface_get_config->iface_idx >= 0) { + _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: duplicate MAC address %s returned", + iface_data->iface_idx, + hwaddr); + error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, + "duplicate MAC address for index %" G_GSSIZE_FORMAT, + iface_data->iface_idx); + goto out_done; + } + is_requested = TRUE; + } + + iface_data->iface_get_config->iface_idx = iface_data->iface_idx; + + _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: found a %sdevice with hwaddr %s", iface_data->iface_idx, + is_requested ? "requested " : "", hwaddr); nm_sprintf_buf(sbuf, "%" G_GSSIZE_FORMAT "/forwarded-ips/", iface_data->iface_idx); @@ -291,7 +323,7 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) iface_data); return; -out_error: +out_done: --get_config_data->n_pending; _nmcs_provider_get_config_task_maybe_return(get_config_data, g_steal_pointer(&error)); } diff --git a/clients/cloud-setup/nmcs-provider.h b/clients/cloud-setup/nmcs-provider.h index 3edd874337..3b0c2529ed 100644 --- a/clients/cloud-setup/nmcs-provider.h +++ b/clients/cloud-setup/nmcs-provider.h @@ -12,11 +12,16 @@ typedef struct { in_addr_t *ipv4s_arr; gsize ipv4s_len; - gssize iface_idx; - in_addr_t cidr_addr; - guint8 cidr_prefix; - bool has_ipv4s : 1; - bool has_cidr : 1; + + /* If the interface was seen, get_config() should set this to a + * unique, increasing, positive index. If the interface is requested, + * it is initialized to -1. */ + gssize iface_idx; + + in_addr_t cidr_addr; + guint8 cidr_prefix; + bool has_ipv4s : 1; + bool has_cidr : 1; NMIPRoute **iproutes_arr; gsize iproutes_len; From 30e1f733473f8990040cc8a4a7531c7d46b87aea Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 5 Jan 2021 16:21:56 +0100 Subject: [PATCH 08/13] cloud-setup: add nmcs_utils_ipaddr_normalize*() helper --- clients/cloud-setup/nm-cloud-setup-utils.c | 53 ++++++++++++++++++++++ clients/cloud-setup/nm-cloud-setup-utils.h | 20 ++++++++ 2 files changed, 73 insertions(+) diff --git a/clients/cloud-setup/nm-cloud-setup-utils.c b/clients/cloud-setup/nm-cloud-setup-utils.c index 24b3ee6a29..95a84d10f6 100644 --- a/clients/cloud-setup/nm-cloud-setup-utils.c +++ b/clients/cloud-setup/nm-cloud-setup-utils.c @@ -533,6 +533,59 @@ nmcs_utils_hwaddr_normalize(const char *hwaddr, gssize len) /*****************************************************************************/ +gboolean +nmcs_utils_ipaddr_normalize_bin(int addr_family, + const char *addr, + gssize len, + int * out_addr_family, + gpointer out_addr_bin) +{ + gs_free char *addr_clone = NULL; + char * ad; + gsize l; + + nm_assert(len >= -1); + + if (len < 0) { + if (!addr) + return FALSE; + l = strlen(addr); + } else { + l = len; + if (l > 0 && addr[l - 1] == '\0') { + /* we accept one '\0' at the end of the string. */ + l--; + } + if (memchr(addr, '\0', l)) { + /* but we don't accept other NUL characters in the middle. */ + return FALSE; + } + } + + if (l == 0) + return FALSE; + + nm_assert(addr); + ad = nm_strndup_a(300, addr, l, &addr_clone); + + g_strstrip(ad); + + return nm_utils_parse_inaddr_bin(addr_family, ad, out_addr_family, out_addr_bin); +} + +char * +nmcs_utils_ipaddr_normalize(int addr_family, const char *addr, gssize len) +{ + NMIPAddr ipaddr; + + if (!nmcs_utils_ipaddr_normalize_bin(addr_family, addr, len, &addr_family, &ipaddr)) + return NULL; + + return nm_utils_inet_ntop_dup(addr_family, &ipaddr); +} + +/*****************************************************************************/ + const char * nmcs_utils_parse_memmem(GBytes *mem, const char *needle) { diff --git a/clients/cloud-setup/nm-cloud-setup-utils.h b/clients/cloud-setup/nm-cloud-setup-utils.h index 217f07550e..52edd27994 100644 --- a/clients/cloud-setup/nm-cloud-setup-utils.h +++ b/clients/cloud-setup/nm-cloud-setup-utils.h @@ -90,6 +90,26 @@ nmcs_utils_hwaddr_normalize_gbytes(GBytes *hwaddr) /*****************************************************************************/ +gboolean nmcs_utils_ipaddr_normalize_bin(int addr_family, + const char *addr, + gssize len, + int * out_addr_family, + gpointer out_addr_bin); + +char *nmcs_utils_ipaddr_normalize(int addr_family, const char *addr, gssize len); + +static inline char * +nmcs_utils_ipaddr_normalize_gbytes(int addr_family, GBytes *addr) +{ + const char *str; + gsize len; + + str = g_bytes_get_data(addr, &len); + return nmcs_utils_ipaddr_normalize(addr_family, str, len); +} + +/*****************************************************************************/ + const char *nmcs_utils_parse_memmem(GBytes *mem, const char *needle); const char *nmcs_utils_parse_get_full_line(GBytes *mem, const char *needle); From 288b38e7190a03a52279429c9ba5331e1b70d2aa Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 5 Jan 2021 16:32:04 +0100 Subject: [PATCH 09/13] cloud-setup: use nmcs_utils_ipaddr_normalize_gbytes() in GCP provider --- clients/cloud-setup/nmcs-provider-gcp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clients/cloud-setup/nmcs-provider-gcp.c b/clients/cloud-setup/nmcs-provider-gcp.c index aa01356113..41b56ff4fd 100644 --- a/clients/cloud-setup/nmcs-provider-gcp.c +++ b/clients/cloud-setup/nmcs-provider-gcp.c @@ -109,7 +109,7 @@ _get_config_fip_cb(GObject *source, GAsyncResult *result, gpointer user_data) gs_unref_bytes GBytes *response = NULL; GCPIfaceData * iface_data = user_data; gs_free_error GError *error = NULL; - const char * fip_str = NULL; + gs_free char * ipaddr = NULL; NMIPRoute ** routes_arr; NMIPRoute * route_new; @@ -123,8 +123,8 @@ _get_config_fip_cb(GObject *source, GAsyncResult *result, gpointer user_data) if (error) goto out_done; - fip_str = g_bytes_get_data(response, NULL); - if (!nm_utils_ipaddr_valid(AF_INET, fip_str)) { + ipaddr = nmcs_utils_ipaddr_normalize_gbytes(AF_INET, response); + if (!ipaddr) { error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "forwarded-ip is not a valid ip address"); goto out_done; @@ -132,12 +132,12 @@ _get_config_fip_cb(GObject *source, GAsyncResult *result, gpointer user_data) _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: adding forwarded-ip %s", iface_data->iface_idx, - fip_str); + ipaddr); iface_get_config = iface_data->iface_get_config; routes_arr = iface_get_config->iproutes_arr; - route_new = nm_ip_route_new(AF_INET, fip_str, 32, NULL, 100, &error); + route_new = nm_ip_route_new(AF_INET, ipaddr, 32, NULL, 100, &error); if (error) goto out_done; From fc8315cd9416cab19577e81c0af2d999b0c9b227 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 5 Jan 2021 16:32:04 +0100 Subject: [PATCH 10/13] cloud-setup: use nmcs_utils_ipaddr_normalize_bin() in Azure provider --- clients/cloud-setup/nmcs-provider-azure.c | 27 +++++++++++++++-------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/clients/cloud-setup/nmcs-provider-azure.c b/clients/cloud-setup/nmcs-provider-azure.c index b2490b87f9..da7cf54084 100644 --- a/clients/cloud-setup/nmcs-provider-azure.c +++ b/clients/cloud-setup/nmcs-provider-azure.c @@ -118,8 +118,7 @@ _get_config_fetch_done_cb(NMHttpClient * http_client, gs_unref_bytes GBytes *response = NULL; gs_free_error GError *error = NULL; const char * fip_str = NULL; - in_addr_t tmp_addr; - int tmp_prefix; + gsize fip_len; nm_http_client_poll_get_finish(http_client, result, NULL, &response, &error); @@ -131,32 +130,42 @@ _get_config_fetch_done_cb(NMHttpClient * http_client, if (error) goto out_done; - fip_str = g_bytes_get_data(response, NULL); + fip_str = g_bytes_get_data(response, &fip_len); + nm_assert(fip_str[fip_len] == '\0'); + iface_data->iface_get_config = g_hash_table_lookup(get_config_data->result_dict, iface_data->hwaddr); iface_get_config = iface_data->iface_get_config; if (is_ipv4) { - if (!nm_utils_parse_inaddr_bin(AF_INET, fip_str, NULL, &tmp_addr)) { + char tmp_addr_str[NM_UTILS_INET_ADDRSTRLEN]; + in_addr_t tmp_addr; + + if (!nmcs_utils_ipaddr_normalize_bin(AF_INET, fip_str, fip_len, NULL, &tmp_addr)) { error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "ip is not a valid private ip address"); goto out_done; } _LOGD("interface[%" G_GSSIZE_FORMAT "]: adding private ip %s", iface_data->iface_idx, - fip_str); + _nm_utils_inet4_ntop(tmp_addr, tmp_addr_str)); iface_get_config->ipv4s_arr[iface_get_config->ipv4s_len] = tmp_addr; iface_get_config->has_ipv4s = TRUE; iface_get_config->ipv4s_len++; } else { - tmp_prefix = (_nm_utils_ascii_str_to_int64(fip_str, 10, 0, 32, -1)); + int tmp_prefix = -1; + + if (fip_len > 0 && memchr(fip_str, '\0', fip_len - 1)) { + /* we have an embedded "\0" inside the string (except trailing). That is not + * allowed*/ + } else + tmp_prefix = _nm_utils_ascii_str_to_int64(fip_str, 10, 0, 32, -1); if (tmp_prefix == -1) { - _LOGD("interface[%" G_GSSIZE_FORMAT "]: invalid prefix %d", - iface_data->iface_idx, - tmp_prefix); + _LOGD("interface[%" G_GSSIZE_FORMAT "]: invalid prefix", iface_data->iface_idx); goto out_done; } + _LOGD("interface[%" G_GSSIZE_FORMAT "]: adding prefix %d", iface_data->iface_idx, tmp_prefix); From e81b442d8b26a51865f5b8f4b7d3a8cf3eb37bbe Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 5 Jan 2021 17:59:32 +0100 Subject: [PATCH 11/13] cloud-setup: fail get_config() for Azure on invalid prefix While it's not clear whether we should be strict or forgiving when fetching the HTTP meta data, we should be consistent. On a parse error of the IP addresses we fail. Hence also fail on a parse error for the subnet. --- clients/cloud-setup/nmcs-provider-azure.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clients/cloud-setup/nmcs-provider-azure.c b/clients/cloud-setup/nmcs-provider-azure.c index da7cf54084..5d31fd03a1 100644 --- a/clients/cloud-setup/nmcs-provider-azure.c +++ b/clients/cloud-setup/nmcs-provider-azure.c @@ -163,6 +163,8 @@ _get_config_fetch_done_cb(NMHttpClient * http_client, if (tmp_prefix == -1) { _LOGD("interface[%" G_GSSIZE_FORMAT "]: invalid prefix", iface_data->iface_idx); + error = + nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "subnet does not give a valid prefix"); goto out_done; } From 8128f791c931d8a3d752115a050135ff8f99fda9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 5 Jan 2021 18:31:30 +0100 Subject: [PATCH 12/13] cloud-setup: assign incremental iface_idx for Azure,GCP get_config() result We use the iface_idx for example to determine the routing table, by using table 30400+iface_idx. While the HTTP API for Azure has a index, it does not mean that we should use that index as-is for our purpose. Instead, treat those indexes separately and ensure that the iface_idx that we return is numbering the interfaces starting from zero. --- clients/cloud-setup/nmcs-provider-azure.c | 43 ++++++++++++----------- clients/cloud-setup/nmcs-provider-gcp.c | 42 ++++++++++++---------- 2 files changed, 46 insertions(+), 39 deletions(-) diff --git a/clients/cloud-setup/nmcs-provider-azure.c b/clients/cloud-setup/nmcs-provider-azure.c index 5d31fd03a1..e55ecc9d28 100644 --- a/clients/cloud-setup/nmcs-provider-azure.c +++ b/clients/cloud-setup/nmcs-provider-azure.c @@ -96,7 +96,8 @@ detect(NMCSProvider *provider, GTask *task) typedef struct { NMCSProviderGetConfigTaskData * get_config_data; NMCSProviderGetConfigIfaceData *iface_get_config; - gssize iface_idx; + gssize intern_iface_idx; + gssize extern_iface_idx; guint n_ips_prefix_pending; const char * hwaddr; } AzureIfaceData; @@ -147,7 +148,7 @@ _get_config_fetch_done_cb(NMHttpClient * http_client, goto out_done; } _LOGD("interface[%" G_GSSIZE_FORMAT "]: adding private ip %s", - iface_data->iface_idx, + iface_data->intern_iface_idx, _nm_utils_inet4_ntop(tmp_addr, tmp_addr_str)); iface_get_config->ipv4s_arr[iface_get_config->ipv4s_len] = tmp_addr; iface_get_config->has_ipv4s = TRUE; @@ -162,14 +163,14 @@ _get_config_fetch_done_cb(NMHttpClient * http_client, tmp_prefix = _nm_utils_ascii_str_to_int64(fip_str, 10, 0, 32, -1); if (tmp_prefix == -1) { - _LOGD("interface[%" G_GSSIZE_FORMAT "]: invalid prefix", iface_data->iface_idx); + _LOGD("interface[%" G_GSSIZE_FORMAT "]: invalid prefix", iface_data->intern_iface_idx); error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "subnet does not give a valid prefix"); goto out_done; } _LOGD("interface[%" G_GSSIZE_FORMAT "]: adding prefix %d", - iface_data->iface_idx, + iface_data->intern_iface_idx, tmp_prefix); iface_get_config->cidr_prefix = tmp_prefix; iface_get_config->has_cidr = TRUE; @@ -258,7 +259,7 @@ _get_config_ips_prefix_list_cb(GObject *source, GAsyncResult *result, gpointer u (uri = _azure_uri_interfaces(nm_sprintf_buf( buf, "%" G_GSSIZE_FORMAT "/ipv4/ipAddress/%" G_GINT64_FORMAT "/privateIpAddress", - iface_data->iface_idx, + iface_data->intern_iface_idx, ips_prefix_idx))), HTTP_TIMEOUT_MS, 512 * 1024, @@ -284,7 +285,7 @@ _get_config_ips_prefix_list_cb(GObject *source, GAsyncResult *result, gpointer u nm_http_client_poll_get( NM_HTTP_CLIENT(source), (uri = _azure_uri_interfaces( - nm_sprintf_buf(buf, "%" G_GSSIZE_FORMAT, iface_data->iface_idx), + nm_sprintf_buf(buf, "%" G_GSSIZE_FORMAT, iface_data->intern_iface_idx), "/ipv4/subnet/0/prefix/")), HTTP_TIMEOUT_MS, 512 * 1024, @@ -328,10 +329,10 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) v_hwaddr = nmcs_utils_hwaddr_normalize_gbytes(response); if (!v_hwaddr) { _LOGI("interface[%" G_GSSIZE_FORMAT "]: invalid MAC address returned", - iface_data->iface_idx); + iface_data->intern_iface_idx); error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "invalid MAC address for index %" G_GSSIZE_FORMAT, - iface_data->iface_idx); + iface_data->intern_iface_idx); goto out_done; } @@ -342,7 +343,7 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) if (!get_config_data->any) { _LOGD("get-config: skip fetching meta data for %s (%" G_GSSIZE_FORMAT ")", v_hwaddr, - iface_data->iface_idx); + iface_data->intern_iface_idx); goto out_done; } iface_data->iface_get_config = nmcs_provider_get_config_iface_data_new(FALSE); @@ -352,22 +353,22 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) } else { if (iface_data->iface_get_config->iface_idx >= 0) { _LOGI("interface[%" G_GSSIZE_FORMAT "]: duplicate MAC address %s returned", - iface_data->iface_idx, + iface_data->intern_iface_idx, iface_data->hwaddr); error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "duplicate MAC address for index %" G_GSSIZE_FORMAT, - iface_data->iface_idx); + iface_data->intern_iface_idx); goto out_done; } } - iface_data->iface_get_config->iface_idx = iface_data->iface_idx; + iface_data->iface_get_config->iface_idx = iface_data->extern_iface_idx; _LOGD("interface[%" G_GSSIZE_FORMAT "]: found a matching device with hwaddr %s", - iface_data->iface_idx, + iface_data->intern_iface_idx, iface_data->hwaddr); - nm_sprintf_buf(buf, "%" G_GSSIZE_FORMAT "/ipv4/ipAddress/", iface_data->iface_idx); + nm_sprintf_buf(buf, "%" G_GSSIZE_FORMAT "/ipv4/ipAddress/", iface_data->intern_iface_idx); nm_http_client_poll_get(NM_HTTP_CLIENT(source), (uri = _azure_uri_interfaces(buf)), @@ -400,6 +401,7 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat const char * line; gsize line_len; guint i; + gssize extern_iface_idx_cnt = 0; nm_http_client_poll_get_finish(NM_HTTP_CLIENT(source), result, NULL, &response, &error); @@ -421,7 +423,7 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat while (nm_utils_parse_next_line(&response_str, &response_len, &line, &line_len)) { AzureIfaceData *iface_data; - gssize iface_idx; + gssize intern_iface_idx; if (line_len == 0) continue; @@ -433,15 +435,16 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat if (line[line_len - 1] == '/' && line_len != 0) ((char *) line)[--line_len] = '\0'; - iface_idx = _nm_utils_ascii_str_to_int64(line, 10, 0, G_MAXSSIZE, -1); - if (iface_idx < 0) + intern_iface_idx = _nm_utils_ascii_str_to_int64(line, 10, 0, G_MAXSSIZE, -1); + if (intern_iface_idx < 0) continue; iface_data = g_slice_new(AzureIfaceData); *iface_data = (AzureIfaceData){ .get_config_data = get_config_data, .iface_get_config = NULL, - .iface_idx = iface_idx, + .intern_iface_idx = intern_iface_idx, + .extern_iface_idx = extern_iface_idx_cnt++, .n_ips_prefix_pending = 0, .hwaddr = NULL, }; @@ -463,9 +466,9 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat char buf[100]; _LOGD("azure interface[%" G_GSSIZE_FORMAT "]: retrieving configuration", - iface_data->iface_idx); + iface_data->intern_iface_idx); - nm_sprintf_buf(buf, "%" G_GSSIZE_FORMAT "/macAddress", iface_data->iface_idx); + nm_sprintf_buf(buf, "%" G_GSSIZE_FORMAT "/macAddress", iface_data->intern_iface_idx); get_config_data->n_pending++; nm_http_client_poll_get(NM_HTTP_CLIENT(source), diff --git a/clients/cloud-setup/nmcs-provider-gcp.c b/clients/cloud-setup/nmcs-provider-gcp.c index 41b56ff4fd..425621b43f 100644 --- a/clients/cloud-setup/nmcs-provider-gcp.c +++ b/clients/cloud-setup/nmcs-provider-gcp.c @@ -91,7 +91,8 @@ detect(NMCSProvider *provider, GTask *task) typedef struct { NMCSProviderGetConfigTaskData * get_config_data; NMCSProviderGetConfigIfaceData *iface_get_config; - gssize iface_idx; + gssize intern_iface_idx; + gssize extern_iface_idx; guint n_fips_pending; } GCPIfaceData; @@ -109,7 +110,7 @@ _get_config_fip_cb(GObject *source, GAsyncResult *result, gpointer user_data) gs_unref_bytes GBytes *response = NULL; GCPIfaceData * iface_data = user_data; gs_free_error GError *error = NULL; - gs_free char * ipaddr = NULL; + gs_free char * ipaddr = NULL; NMIPRoute ** routes_arr; NMIPRoute * route_new; @@ -131,7 +132,7 @@ _get_config_fip_cb(GObject *source, GAsyncResult *result, gpointer user_data) } _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: adding forwarded-ip %s", - iface_data->iface_idx, + iface_data->intern_iface_idx, ipaddr); iface_get_config = iface_data->iface_get_config; @@ -198,14 +199,14 @@ _get_config_ips_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat g_ptr_array_add(uri_arr, g_strdup_printf("%" G_GSSIZE_FORMAT "/forwarded-ips/%" G_GINT64_FORMAT, - iface_data->iface_idx, + iface_data->intern_iface_idx, fip_index)); } iface_data->n_fips_pending = uri_arr->len; _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: found %u forwarded ips", - iface_data->iface_idx, + iface_data->intern_iface_idx, iface_data->n_fips_pending); if (iface_data->n_fips_pending == 0) { @@ -265,10 +266,10 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) v_hwaddr = nmcs_utils_hwaddr_normalize_gbytes(response); if (!v_hwaddr) { _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: invalid MAC address returned", - iface_data->iface_idx); + iface_data->intern_iface_idx); error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "invalid MAC address for index %" G_GSSIZE_FORMAT, - iface_data->iface_idx); + iface_data->intern_iface_idx); goto out_done; } @@ -279,7 +280,7 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) if (!get_config_data->any) { _LOGD("get-config: skip fetching meta data for %s (%" G_GSSIZE_FORMAT ")", v_hwaddr, - iface_data->iface_idx); + iface_data->intern_iface_idx); goto out_done; } iface_data->iface_get_config = nmcs_provider_get_config_iface_data_new(FALSE); @@ -290,24 +291,24 @@ _get_config_iface_cb(GObject *source, GAsyncResult *result, gpointer user_data) } else { if (iface_data->iface_get_config->iface_idx >= 0) { _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: duplicate MAC address %s returned", - iface_data->iface_idx, + iface_data->intern_iface_idx, hwaddr); error = nm_utils_error_new(NM_UTILS_ERROR_UNKNOWN, "duplicate MAC address for index %" G_GSSIZE_FORMAT, - iface_data->iface_idx); + iface_data->intern_iface_idx); goto out_done; } is_requested = TRUE; } - iface_data->iface_get_config->iface_idx = iface_data->iface_idx; + iface_data->iface_get_config->iface_idx = iface_data->extern_iface_idx; _LOGI("GCP interface[%" G_GSSIZE_FORMAT "]: found a %sdevice with hwaddr %s", - iface_data->iface_idx, + iface_data->intern_iface_idx, is_requested ? "requested " : "", hwaddr); - nm_sprintf_buf(sbuf, "%" G_GSSIZE_FORMAT "/forwarded-ips/", iface_data->iface_idx); + nm_sprintf_buf(sbuf, "%" G_GSSIZE_FORMAT "/forwarded-ips/", iface_data->intern_iface_idx); nm_http_client_poll_get(NM_HTTP_CLIENT(source), (uri = _gcp_uri_interfaces(sbuf)), @@ -340,6 +341,7 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat const char * line; gsize line_len; guint i; + gssize extern_iface_idx_cnt = 0; nm_http_client_poll_get_finish(NM_HTTP_CLIENT(source), result, NULL, &response, &error); @@ -361,7 +363,7 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat while (nm_utils_parse_next_line(&response_str, &response_len, &line, &line_len)) { GCPIfaceData *iface_data; - gssize iface_idx; + gssize intern_iface_idx; if (line_len == 0) continue; @@ -372,15 +374,16 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat if (line[line_len - 1] == '/') ((char *) line)[--line_len] = '\0'; - iface_idx = _nm_utils_ascii_str_to_int64(line, 10, 0, G_MAXSSIZE, -1); - if (iface_idx < 0) + intern_iface_idx = _nm_utils_ascii_str_to_int64(line, 10, 0, G_MAXSSIZE, -1); + if (intern_iface_idx < 0) continue; iface_data = g_slice_new(GCPIfaceData); *iface_data = (GCPIfaceData){ .get_config_data = get_config_data, .iface_get_config = NULL, - .iface_idx = iface_idx, + .intern_iface_idx = intern_iface_idx, + .extern_iface_idx = extern_iface_idx_cnt++, .n_fips_pending = 0, }; g_ptr_array_add(ifaces_arr, iface_data); @@ -400,9 +403,10 @@ _get_net_ifaces_list_cb(GObject *source, GAsyncResult *result, gpointer user_dat gs_free const char *uri = NULL; char sbuf[100]; - _LOGD("GCP interface[%" G_GSSIZE_FORMAT "]: retrieving configuration", data->iface_idx); + _LOGD("GCP interface[%" G_GSSIZE_FORMAT "]: retrieving configuration", + data->intern_iface_idx); - nm_sprintf_buf(sbuf, "%" G_GSSIZE_FORMAT "/mac", data->iface_idx); + nm_sprintf_buf(sbuf, "%" G_GSSIZE_FORMAT "/mac", data->intern_iface_idx); get_config_data->n_pending++; nm_http_client_poll_get(NM_HTTP_CLIENT(source), From f0c4b3b287ff08af4aa0a34cee2735836d6bdd70 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 5 Jan 2021 14:21:05 +0100 Subject: [PATCH 13/13] man: improve "nm-cloud-setup" manual page It's questionable whether the manual page should explain exactly what it does. However, it's a good exercise writing this up (to review what happens). Also, a manual page that simply says "it configures the network automatically" without going into how exactly, is not very useful either. --- man/nm-cloud-setup.xml | 100 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 90 insertions(+), 10 deletions(-) diff --git a/man/nm-cloud-setup.xml b/man/nm-cloud-setup.xml index 16c695a97f..2ffee5b055 100644 --- a/man/nm-cloud-setup.xml +++ b/man/nm-cloud-setup.xml @@ -49,11 +49,12 @@ desirable to automatically configure the network of that VM. In simple setups, the VM only has one network interface and the public cloud supports automatic configuration via DHCP, DHCP6 or IPv6 autoconf. - However, on the virtual machine might have multiple network + However, the virtual machine might have multiple network interfaces, or multiple IP addresses and IP subnets - on one interface. Also, the administrator can reconfigure those settings - while the machine is running. NetworkManager's nm-cloud-setup is a tool - that automatically picks up such configuration and updates the network + on one interface which cannot be configured via DHCP. Also, the administrator + may reconfigure the network while the machine is running. NetworkManager's + nm-cloud-setup is a tool + that automatically picks up such configuration in cloud environments and updates the network configuration of the host. Multiple cloud providers are supported. See . @@ -87,6 +88,11 @@ nmcli connection add type ethernet .... + + By setting the user-data org.freedesktop.nm-cloud-setup.skip=yes + on the profile, nm-cloud-setup will skip the device. + + nm-cloud-setup modifies the run time configuration akin to nmcli device modify. With this approach, the configuration is not persisted and only preserved until the device disconnects. @@ -119,7 +125,7 @@ Usually /usr/libexec/nm-cloud-setup is not run directly, but only by systemctl restart nm-cloud-setup.service. This ensures that the tool only runs once at any time. It also allows to integrate - use the nm-cloud-setup systemd timer, + with the nm-cloud-setup systemd timer, and to enable/disable the service via systemd. As you need to set environment variable to configure nm-cloud-setup binary, @@ -161,7 +167,7 @@ NM_CLOUD_SETUP_LOG: control the logging verbosity. Set it - one of TRACE, DEBUG, INFO, + to one of TRACE, DEBUG, INFO, WARN, ERR or OFF. The program will print message on stdout and the default level is WARN. @@ -187,7 +193,7 @@ Amazon EC2 (AWS) - The tools tries to fetch configuration from http://169.254.169.254/. Currently, it only + For AWS, the tools tries to fetch configuration from http://169.254.169.254/. Currently, it only configures IPv4 and does nothing about IPv6. It will do the following. @@ -233,7 +239,7 @@ nmcli device modify "eth0" ipv4.addresses "172.16.5.3/24,172.16.5.4/24" ipv4.routes "0.0.0.0/0 172.16.5.1 10 table=30401" ipv4.routing-rules "priority 30401 from 172.16.5.3/32 table 30401, priority 30401 from 172.16.5.4/32 table 30401". Note that this replaces the previous addresses, routes and rules with the new information. But also note that this only changes the run time configuration of the device. The - connection profile is not affected by that. + connection profile on disk is not affected. @@ -242,13 +248,87 @@ Google Cloud Platform (GCP) - The tools tries to fetch configuration from http://metadata.google.internal/. + + For GCP, the meta data is fetched from URIs starting with http://metadata.google.internal/computeMetadata/v1/ with a + HTTP header "Metadata-Flavor: Google". + Currently, the tool only configures IPv4 and does nothing about IPv6. It will do the following. + + + + + First fetch http://metadata.google.internal/computeMetadata/v1/instance/id to detect whether the tool + runs on Google Cloud Platform. Only if the platform is detected, it will continue fetching the configuration. + + + Fetch http://metadata.google.internal/computeMetadata/v1/instance/network-interfaces/ to get the list + of available interface indexes. These indexes can be used for further lookups. + + + Then, for each interface fetch http://metadata.google.internal/computeMetadata/v1/instance/network-interfaces/$IFACE_INDEX/mac to get + the corresponding MAC address of the found interfaces. The MAC address is used to identify the device later on. + + + Then, for each interface with a MAC address fetch http://metadata.google.internal/computeMetadata/v1/instance/network-interfaces/$IFACE_INDEX/forwarded-ips/ + and then all the found IP addresses at http://metadata.google.internal/computeMetadata/v1/instance/network-interfaces/$IFACE_INDEX/forwarded-ips/$FIPS_INDEX. + + + + At this point, we have a list of all interfaces (by MAC address) and their configured IPv4 addresses. + For each device, we lookup the currently applied connection in NetworkManager. That implies, that the device is currently activated + in NetworkManager. If no such device was in NetworkManager, or if the profile has user-data org.freedesktop.nm-cloud-setup.skip=yes, + we skip the device. Now for each found IP address we add a static route "$FIPS_ADDR/32 0.0.0.0 100 type=local" and reapply the change. + The effect is not unlike calling nmcli device modify "$DEVICE" ipv4.routes "$FIPS_ADDR/32 0.0.0.0 100 type=local [,...]" for all relevant + devices and all found addresses. + + Microsoft Azure - The tools tries to fetch configuration from http://169.254.169.254/. + + For Azure, the meta data is fetched from URIs starting with http://169.254.169.254/metadata/instance with a + URL parameter "?format=text&api-version=2017-04-02" and a HTTP header "Metadata:true". + Currently, the tool only configures IPv4 and does nothing about IPv6. It will do the following. + + + + + First fetch http://169.254.169.254/metadata/instance?format=text&api-version=2017-04-02 to detect whether the tool + runs on Azure Cloud. Only if the platform is detected, it will continue fetching the configuration. + + + Fetch http://169.254.169.254/metadata/instance/network/interface/?format=text&api-version=2017-04-02 to get the list + of available interface indexes. These indexes can be used for further lookups. + + + Then, for each interface fetch http://169.254.169.254/metadata/instance/network/interface/$IFACE_INDEX/macAddress?format=text&api-version=2017-04-02 + to get the corresponding MAC address of the found interfaces. The MAC address is used to identify the device later on. + + + Then, for each interface with a MAC address fetch http://169.254.169.254/metadata/instance/network/interface/$IFACE_INDEX/ipv4/ipAddress/?format=text&api-version=2017-04-02 + to get the list of (indexes of) IP addresses on that interface. + + + + Then, for each IP address index fetch the address at + http://169.254.169.254/metadata/instance/network/interface/$IFACE_INDEX/ipv4/ipAddress/$ADDR_INDEX/privateIpAddress?format=text&api-version=2017-04-02. + Also fetch the size of the subnet (the netmask) for the interface from + http://169.254.169.254/metadata/instance/network/interface/$IFACE_INDEX/ipv4/subnet/0/prefix/?format=text&api-version=2017-04-02. + + + + At this point, we have a list of all interfaces (by MAC address) and their configured IPv4 addresses. + For each device, we lookup the currently applied connection in NetworkManager. That implies, that the device is currently activated + in NetworkManager. If no such device was in NetworkManager, or if the profile has user-data org.freedesktop.nm-cloud-setup.skip=yes, + we skip the device. Now for each found IP address we add a static address "$ADDR/$SUBNET_PREFIX". Also we configure policy routing + by adding a static route "$ADDR/$SUBNET_PREFIX $GATEWAY 10, table=$TABLE" where $GATEWAY is the first IP address in the subnet and table + is 30400 plus the interface index. Also we add a policy routing rule "priority $TABLE from $ADDR/32 table $TABLE". + The effect is not unlike calling + nmcli device modify "$DEVICE" ipv4.addresses "$ADDR/$SUBNET [,...]" ipv4.routes "$ADDR/32 $GATEWAY 10 table=$TABLE" ipv4.routing-rules "priority $TABLE from $ADDR/32 table $TABLE" + for all relevant devices and all found addresses. + +