RFC: New "networkmanager.passthrough" structure (LP: #2080301)

.github/workflows/rpmbuild.yml

@@ -31,6 +31,7 @@ jobs:
           dnf -y install epel-release || true
           dnf config-manager --set-enabled crb || true  # Meson/CMocka on EL9
           dnf -y install centos-release-nfv-openvswitch || true  # OVS on EL9
+          dnf -y install gdb
           dnf -y builddep rpm/netplan.spec
           adduser test
           chown -R test:test .

doc/netplan-everywhere.md

@@ -101,12 +101,14 @@ network:
         uuid: "0f7a33ac-512e-4c03-b088-4db00fe3292e"
         name: "Ethernet connection 1"
         passthrough:
-          ethernet._: ""
-          ipv4.ignore-auto-dns: "true"
-          ipv6.addr-gen-mode: "default"
-          ipv6.method: "disabled"
-          ipv6.ip6-privacy: "-1"
-          proxy._: ""
+          ethernet: {}
+          ipv4:
+            ignore-auto-dns: "true"
+          ipv6:
+            addr-gen-mode: "default"
+            method: "disabled"
+            ip6-privacy: "-1"
+          proxy: {}
 ```
 
 All the configuration under the `passthrough` mapping is added to
@@ -126,17 +128,21 @@ network:
         uuid: "db5f0f67-1f4c-4d59-8ab8-3d278389cf87"
         name: "myvpnconnection"
         passthrough:
-          connection.type: "vpn"
-          vpn.ca: "path to ca.crt"
-          vpn.cert: "path to client.crt"
-          vpn.cipher: "AES-256-GCM"
-          vpn.connection-type: "tls"
-          vpn.dev: "tun"
-          vpn.key: "path to client.key"
-          vpn.remote: "1.2.3.4:1194"
-          vpn.service-type: "org.freedesktop.NetworkManager.openvpn"
-          ipv4.method: "auto"
-          ipv6.addr-gen-mode: "default"
-          ipv6.method: "auto"
-          proxy._: ""
+          connection:
+            type: "vpn"
+          vpn:
+            ca: "path to ca.crt"
+            cert: "path to client.crt"
+            cipher: "AES-256-GCM"
+            connection-type: "tls"
+            dev: "tun"
+            key: "path to client.key"
+            remote: "1.2.3.4:1194"
+            service-type: "org.freedesktop.NetworkManager.openvpn"
+          ipv4:
+            method: "auto"
+          ipv6:
+            addr-gen-mode: "default"
+            method: "auto"
+          proxy: {}
 ```

doc/netplan-yaml.md

@@ -2146,15 +2146,17 @@ network:
         uuid: "db5f0f67-1f4c-4d59-8ab8-3d278389cf87"
         name: "myvpnconnection"
         passthrough:
-          connection.type: "vpn"
-          vpn.ca: "path to ca.crt"
-          vpn.cert: "path to client.crt"
-          vpn.cipher: "AES-256-GCM"
-          vpn.connection-type: "tls"
-          vpn.dev: "tun"
-          vpn.key: "path to client.key"
-          vpn.remote: "1.2.3.4:1194"
-          vpn.service-type: "org.freedesktop.NetworkManager.openvpn"
+          connection:
+            type: "vpn"
+          vpn:
+            ca: "path to ca.crt"
+            cert: "path to client.crt"
+            cipher: "AES-256-GCM"
+            connection-type: "tls"
+            dev: "tun"
+            key: "path to client.key"
+            remote: "1.2.3.4:1194"
+            service-type: "org.freedesktop.NetworkManager.openvpn"
 ```
 
 ## Back end-specific configuration parameters

src/abi.h

@@ -211,7 +211,7 @@ typedef struct netplan_backend_settings {
     char *uuid;
     char *stable_id;
     char *device;
-    GData* passthrough; /* See g_datalist* functions */
+    GHashTable* passthrough;
 } NetplanBackendSettings;
 
 typedef struct netplan_vxlan NetplanVxlan;

src/netplan.c

@@ -340,14 +340,24 @@ typedef struct {
 } _passthrough_handler_data;
 
 STATIC void
-_passthrough_handler(GQuark key_id, gpointer value, gpointer user_data)
+_passthrough_handler_key_value(gpointer key, gpointer value, gpointer user_data)
 {
     _passthrough_handler_data *d = user_data;
-    const gchar* key = g_quark_to_string(key_id);
     YAML_NONNULL_STRING(d->event, d->emitter, key, value);
 err_path: return; // LCOV_EXCL_LINE
 }
 
+STATIC void
+_passthrough_handler(gpointer key, gpointer value, gpointer user_data)
+{
+    _passthrough_handler_data *d = user_data;
+    YAML_SCALAR_PLAIN(d->event, d->emitter, key);
+    YAML_MAPPING_OPEN(d->event, d->emitter);
+    g_hash_table_foreach(value, _passthrough_handler_key_value, user_data);
+    YAML_MAPPING_CLOSE(d->event, d->emitter);
+err_path: return; // LCOV_EXCL_LINE
+}
+
 STATIC gboolean
 write_backend_settings(yaml_event_t* event, yaml_emitter_t* emitter, NetplanBackendSettings s) {
     if (s.uuid || s.name || s.passthrough) {
@@ -356,13 +366,13 @@ write_backend_settings(yaml_event_t* event, yaml_emitter_t* emitter, NetplanBack
         YAML_NONNULL_STRING(event, emitter, "uuid", s.uuid);
         YAML_NONNULL_STRING(event, emitter, "name", s.name);
 
-        if (s.passthrough) {
+        if (s.passthrough != NULL && g_hash_table_size(s.passthrough) > 0) {
             YAML_SCALAR_PLAIN(event, emitter, "passthrough");
             YAML_MAPPING_OPEN(event, emitter);
             _passthrough_handler_data d;
             d.event = event;
             d.emitter = emitter;
-            g_datalist_foreach(&s.passthrough, _passthrough_handler, &d);
+            g_hash_table_foreach(s.passthrough, _passthrough_handler, &d);
             YAML_MAPPING_CLOSE(event, emitter);
         }
         YAML_MAPPING_CLOSE(event, emitter);

src/nm.c

@@ -98,8 +98,12 @@ type_str(const NetplanNetDefinition* def)
         case NETPLAN_DEF_TYPE_NM:
             /* needs to be overriden by passthrough "connection.type" setting */
             g_assert(def->backend_settings.passthrough != NULL);
-            GData *passthrough = def->backend_settings.passthrough;
-            return g_datalist_get_data(&passthrough, "connection.type");
+            GHashTable *passthrough = def->backend_settings.passthrough;
+            GHashTable* connection = g_hash_table_lookup(passthrough, "connection");
+            if (connection) {
+                return g_hash_table_lookup(connection, "type");
+            }
+            return NULL;
         // LCOV_EXCL_START
         default:
             g_assert_not_reached();
@@ -566,50 +570,42 @@ write_nm_vxlan_parameters(const NetplanNetDefinition* def, GKeyFile* kf)
  * "backend_settings.passthrough" and inject them into the keyfile as-is.
  */
 STATIC void
-write_fallback_key_value(GQuark key_id, gpointer value, gpointer user_data)
+write_fallback_key_value(gpointer group, gpointer value, gpointer user_data)
 {
     GKeyFile *kf = user_data;
-    gchar* val = value;
-    /* Group name may contain dots, but key name may not.
-     * The "tc" group is a special case, where it is the other way around, e.g.:
-     *   tc->qdisc.root
-     *   tc->tfilter.ffff: */
-    const gchar* key = g_quark_to_string(key_id);
-    gchar **group_key = g_strsplit(key, ".", -1);
-    guint len = g_strv_length(group_key);
-    g_autofree gchar* old_key = NULL;
-    gboolean has_key = FALSE;
-    g_autofree gchar* k = NULL;
-    g_autofree gchar* group = NULL;
-    if (!g_strcmp0(group_key[0], "tc") && len > 2) {
-        k = g_strconcat(group_key[1], ".", group_key[2], NULL);
-        group = g_strdup(group_key[0]);
-    } else {
-        k = group_key[len-1];
-        group_key[len-1] = NULL; //remove key from array
-        group = g_strjoinv(".", group_key); //re-combine group parts
-    }
+    GHashTableIter iter;
+    GHashTable* group_settings = value;
+    gpointer k, v;
 
-    has_key = g_key_file_has_key(kf, group, k, NULL);
-    old_key = g_key_file_get_string(kf, group, k, NULL);
-    g_key_file_set_string(kf, group, k, val);
-    /* delete the placeholder key, if this was just an empty group */
-    if (!g_strcmp0(k, NETPLAN_NM_EMPTY_GROUP))
-        g_key_file_remove_key(kf, group, k, NULL);
-    /* handle differing defaults:
-     * ipv6.ip6-privacy is "-1 (unknown)" by default in NM, it is "0 (off)" in netplan */
-    else if (g_strcmp0(key, "ipv6.ip6-privacy") == 0 && g_strcmp0(val, "-1") == 0) {
-        g_debug("NetworkManager: default override: clearing %s.%s", group, k);
-        g_key_file_remove_key(kf, group, k, NULL);
-    } else if (!has_key) {
-        g_debug("NetworkManager: passing through fallback key: %s.%s=%s", group, k, val);
-        g_key_file_set_comment(kf, group, k, "Netplan: passthrough setting", NULL);
-    } else if (g_strcmp0(val, old_key) != 0) {
-        g_debug("NetworkManager: fallback override: %s.%s=%s", group, k, val);
-        g_key_file_set_comment(kf, group, k, "Netplan: passthrough override", NULL);
+    /*
+     * An empty hash table means it's an empty group.
+     * Here we add and remove a bogus key so the group is created in the kf
+     */
+    if (g_hash_table_size(group_settings) == 0) {
+        g_key_file_set_string(kf, group, NETPLAN_NM_EMPTY_GROUP, "");
+        g_key_file_remove_key(kf, group, NETPLAN_NM_EMPTY_GROUP, NULL);
+        return;
     }
 
-    g_strfreev(group_key);
+    g_hash_table_iter_init(&iter, group_settings);
+
+    while (g_hash_table_iter_next(&iter, &k, &v)) {
+        gboolean has_key = g_key_file_has_key(kf, group, k, NULL);
+        g_autofree gchar* old_key = g_key_file_get_string(kf, group, k, NULL);
+        g_key_file_set_string(kf, group, k, v);
+        /* handle differing defaults:
+         * ipv6.ip6-privacy is "-1 (unknown)" by default in NM, it is "0 (off)" in netplan */
+        if (g_strcmp0(k, "ip6-privacy") == 0 && g_strcmp0(v, "-1") == 0) {
+            g_debug("NetworkManager: default override: clearing %s.%s", (gchar*)group, (gchar*)k);
+            g_key_file_remove_key(kf, group, k, NULL);
+        } else if (!has_key) {
+            g_debug("NetworkManager: passing through fallback key: %s.%s=%s", (gchar*)group, (gchar*)k, (gchar*)v);
+            g_key_file_set_comment(kf, group, k, "Netplan: passthrough setting", NULL);
+        } else if (g_strcmp0(v, old_key) != 0) {
+            g_debug("NetworkManager: fallback override: %s.%s=%s", (gchar*)group, (gchar*)k, (gchar*)v);
+            g_key_file_set_comment(kf, group, k, "Netplan: passthrough override", NULL);
+        }
+    }
 }
 
 /**
@@ -641,6 +637,12 @@ write_nm_conf_access_point(const NetplanNetDefinition* def, const char* rootdir,
     else
         g_assert(ap == NULL);
 
+    nm_type = type_str(def);
+    if (def->type == NETPLAN_DEF_TYPE_NM && nm_type == NULL) {
+        g_set_error(error, NETPLAN_BACKEND_ERROR, NETPLAN_ERROR_UNSUPPORTED, "ERROR: %s: NetworkManager connection type undefined\n", def->id);
+        return FALSE;
+    }
+
     if (def->type == NETPLAN_DEF_TYPE_VLAN && def->sriov_vlan_filter) {
         g_debug("%s is defined as a hardware SR-IOV filtered VLAN, postponing creation", def->id);
         return TRUE;
@@ -660,7 +662,6 @@ write_nm_conf_access_point(const NetplanNetDefinition* def, const char* rootdir,
         g_key_file_set_string(kf, "connection", "id", nd_nm_id);
     }
 
-    nm_type = type_str(def);
     if (nm_type && def->type != NETPLAN_DEF_TYPE_NM)
         g_key_file_set_string(kf, "connection", "type", nm_type);
 
@@ -932,7 +933,7 @@ write_nm_conf_access_point(const NetplanNetDefinition* def, const char* rootdir,
         g_debug("NetworkManager: using keyfile passthrough mode");
         /* Write all key-value pairs from the hashtable into the keyfile,
          * potentially overriding existing values, if not fully supported. */
-        g_datalist_foreach((GData**)&def->backend_settings.passthrough, write_fallback_key_value, kf);
+        g_hash_table_foreach(def->backend_settings.passthrough, write_fallback_key_value, kf);
     }
 
     g_autofree char* escaped_netdef_id = g_uri_escape_string(def->id, NULL, TRUE);
@@ -970,7 +971,7 @@ write_nm_conf_access_point(const NetplanNetDefinition* def, const char* rootdir,
              * AP passthrough values have higher priority than ND passthrough,
              * because they are more specific and bound to the current SSID's
              * NM connection profile. */
-            g_datalist_foreach((GData**)&ap->backend_settings.passthrough, write_fallback_key_value, kf);
+            g_hash_table_foreach(ap->backend_settings.passthrough, write_fallback_key_value, kf);
         }
     } else {
         /* TODO: make use of netplan_netdef_get_output_filename() */
@@ -1089,11 +1090,21 @@ netplan_state_finish_nm_write(
         GString *tmp = NULL;
         guint unmanaged = nd->backend == NETPLAN_BACKEND_NM ? 0 : 1;
 
+        if (nd->type == NETPLAN_DEF_TYPE_NM_PLACEHOLDER_ || nd->backend == NETPLAN_BACKEND_OVS) {
+            iter = iter->next;
+            continue;
+        }
+
+        nm_type = type_str(nd);
+        if (nd->type == NETPLAN_DEF_TYPE_NM && nm_type == NULL) {
+            /* Will happen when errors are ignored */
+            iter = iter->next;
+            continue;
+        }
+
         g_autofree char* netdef_id = _netplan_scrub_string(nd->id);
         /* Special case: manage or ignore any device of given type on empty "match: {}" stanza */
         if (nd->has_match && !nd->match.driver && !nd->match.mac && !nd->match.original_name) {
-            nm_type = type_str(nd);
-            g_assert(nm_type != NULL);
             g_string_append_printf(nm_conf, "[device-netplan.%s.%s]\nmatch-device=type:%s\n"
                                             "managed=%d\n\n", netplan_def_type_name(nd->type),
                                             netdef_id, nm_type, !unmanaged);

src/parse-nm.c

@@ -448,39 +448,44 @@ parse_bond_arp_ip_targets(GKeyFile* kf, GArray **targets_arr)
 
 /* Read the key-value pairs from the keyfile and pass them through to a map */
 STATIC void
-read_passthrough(GKeyFile* kf, GData** list)
+read_passthrough(GKeyFile* kf, GHashTable** list)
 {
     gchar **groups = NULL;
     gchar **keys = NULL;
-    gchar *group_key = NULL;
     gchar *value = NULL;
     gsize klen = 0;
     gsize glen = 0;
+    GHashTable* group;
 
-    if (!*list)
-        g_datalist_init(list);
+    if (*list == NULL) {
+        *list = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL);
+    }
     groups = g_key_file_get_groups(kf, &glen);
-    if (groups) {
+    if (groups != NULL) {
         for (unsigned i = 0; i < glen; ++i) {
             klen = 0;
+            group = g_hash_table_lookup(*list, groups[i]);
+            if (group == NULL) {
+                group = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, g_free);
+            }
             keys = g_key_file_get_keys(kf, groups[i], &klen, NULL);
             if (klen == 0) {
                 /* empty group */
-                g_datalist_set_data_full(list, g_strconcat(groups[i], ".", NETPLAN_NM_EMPTY_GROUP, NULL), g_strdup(""), g_free);
+                g_hash_table_insert(*list, g_strdup(groups[i]), group);
+                g_strfreev(keys);
                 continue;
             }
             for (unsigned j = 0; j < klen; ++j) {
                 value = g_key_file_get_string(kf, groups[i], keys[j], NULL);
-                if (!value) {
+                if (value == NULL) {
                     // LCOV_EXCL_START
                     g_warning("netplan: Keyfile: cannot read value of %s.%s", groups[i], keys[j]);
                     continue;
                     // LCOV_EXCL_STOP
                 }
-                group_key = g_strconcat(groups[i], ".", keys[j], NULL);
-                g_datalist_set_data_full(list, group_key, value, g_free);
-                g_free(group_key);
+                g_hash_table_insert(group, g_strdup(keys[j]), value);
             }
+            g_hash_table_insert(*list, g_strdup(groups[i]), group);
             g_strfreev(keys);
         }
         g_strfreev(groups);