fix cgroup netdev renames (netdata#20050)

* fix cgroup netdev renames when multiple renames exist for the same device

* remove obsolete members and functions

Author: ktsaou

src/collectors/proc.plugin/proc_net_dev.c

@@ -383,9 +383,7 @@ static netdata_mutex_t netdev_mutex = NETDATA_MUTEX_INITIALIZER;
 
 // ----------------------------------------------------------------------------
 
-static inline void netdev_rename(struct netdev *d, struct rename_task *r) {
-    rename_task_verify_checksum(r);
-
+static inline void netdev_rename_unsafe(struct netdev *d, struct rename_task *r) {
     collector_info("CGROUP: renaming network interface '%s' as '%s' under '%s'", d->name, r->container_device, r->container_name);
 
     netdev_charts_release(d);
@@ -474,7 +472,9 @@ static void netdev_rename_this_device(struct netdev *d) {
     const DICTIONARY_ITEM *item = dictionary_get_and_acquire_item(netdev_renames, d->name);
     if(item) {
         struct rename_task *r = dictionary_acquired_item_value(item);
-        netdev_rename(d, r);
+        spinlock_lock(&r->spinlock);
+        netdev_rename_unsafe(d, r);
+        spinlock_unlock(&r->spinlock);
         dictionary_acquired_item_release(netdev_renames, item);
     }
 }

src/collectors/proc.plugin/proc_net_dev_renames.c

@@ -4,48 +4,50 @@
 
 DICTIONARY *netdev_renames = NULL;
 
-static XXH64_hash_t rename_task_hash(struct rename_task *r) {
-    struct rename_task local_copy = *r;
-    local_copy.checksum = 0;
-    return XXH3_64bits(&local_copy, sizeof(local_copy));
-}
+static void netdev_rename_task_cleanup_unsafe(struct rename_task *r) {
+    cgroup_netdev_release(r->cgroup_netdev_link);
+    r->cgroup_netdev_link = NULL;
 
-// Set the checksum for a rename_task
-static void rename_task_set_checksum(struct rename_task *r) {
-    r->checksum = rename_task_hash(r);
-}
+    rrdlabels_destroy(r->chart_labels);
+    r->chart_labels = NULL;
 
-// Verify the checksum for a rename_task
-void rename_task_verify_checksum(struct rename_task *r) {
-    if(r->checksum != rename_task_hash(r))
-        fatal("MEMORY CORRUPTION DETECTED in rename_task structure. "
-              "Expected checksum: 0x%016" PRIx64 ", "
-              "Calculated checksum: 0x%016" PRIx64,
-              r->checksum, rename_task_hash(r));
+    freez_and_set_to_null(r->container_name);
+    freez_and_set_to_null(r->container_device);
+    freez_and_set_to_null(r->ctx_prefix);
 }
 
 static void dictionary_netdev_rename_delete_cb(const DICTIONARY_ITEM *item __maybe_unused, void *value, void *data __maybe_unused) {
     struct rename_task *r = value;
 
-    rename_task_verify_checksum(r);
+    spinlock_lock(&r->spinlock);
+    netdev_rename_task_cleanup_unsafe(r);
+    spinlock_unlock(&r->spinlock);
+}
 
-    cgroup_netdev_release(r->cgroup_netdev_link);
-    r->cgroup_netdev_link = NULL;
+static bool dictionary_netdev_rename_conflict_cb(const DICTIONARY_ITEM *item __maybe_unused, void *old_value, void *new_value, void *data __maybe_unused) {
+    struct rename_task *old_r = old_value;
+    struct rename_task *new_r = new_value;
 
-    rrdlabels_destroy(r->chart_labels);
-    r->chart_labels = NULL;
+    spinlock_lock(&old_r->spinlock);
+    SWAP(old_r->cgroup_netdev_link, new_r->cgroup_netdev_link);
+    SWAP(old_r->chart_labels, new_r->chart_labels);
+    SWAP(old_r->container_device, new_r->container_device);
+    SWAP(old_r->container_name, new_r->container_name);
+    SWAP(old_r->ctx_prefix, new_r->ctx_prefix);
+    spinlock_unlock(&old_r->spinlock);
 
-    freez_and_set_to_null(r->container_name);
-    freez_and_set_to_null(r->container_device);
-    freez_and_set_to_null(r->ctx_prefix);
+    netdev_rename_task_cleanup_unsafe(new_r);
+
+    return true;
 }
 
 void netdev_renames_init(void) {
     static SPINLOCK spinlock = SPINLOCK_INITIALIZER;
 
     spinlock_lock(&spinlock);
     if(!netdev_renames) {
-        netdev_renames = dictionary_create_advanced(DICT_OPTION_FIXED_SIZE, NULL, sizeof(struct rename_task));
+        netdev_renames = dictionary_create_advanced(DICT_OPTION_FIXED_SIZE | DICT_OPTION_DONT_OVERWRITE_VALUE, NULL, sizeof(struct rename_task));
+        dictionary_register_conflict_callback(netdev_renames, dictionary_netdev_rename_conflict_cb, NULL);
         dictionary_register_delete_callback(netdev_renames, dictionary_netdev_rename_delete_cb, NULL);
     }
     spinlock_unlock(&spinlock);
@@ -67,12 +69,8 @@ void cgroup_rename_task_add(
         .ctx_prefix       = strdupz(ctx_prefix),
         .chart_labels     = rrdlabels_create(),
         .cgroup_netdev_link = cgroup_netdev_link,
-        .checksum = 0, // Will be set below
     };
     rrdlabels_migrate_to_these(tmp.chart_labels, labels);
-    
-    // Set the checksum after all fields are initialized
-    rename_task_set_checksum(&tmp);
 
     dictionary_set(netdev_renames, host_device, &tmp, sizeof(tmp));
 }

src/collectors/proc.plugin/proc_net_dev_renames.h

@@ -13,17 +13,20 @@
 extern DICTIONARY *netdev_renames;
 
 struct rename_task {
+    SPINLOCK spinlock;
     const char *container_device;
     const char *container_name;
     const char *ctx_prefix;
     RRDLABELS *chart_labels;
     const DICTIONARY_ITEM *cgroup_netdev_link;
-    XXH64_hash_t checksum;
 };
 
-void netdev_renames_init(void);
+#define freez_and_set_to_null(p) do { \
+    freez((void *)p); \
+    p = NULL; \
+} while(0)
 
-void rename_task_verify_checksum(struct rename_task *r);
+void netdev_renames_init(void);
 
 void cgroup_netdev_reset_all(void);
 void cgroup_netdev_release(const DICTIONARY_ITEM *link);