bridges: implement vlans & port-vlans options for NM backend

NM:bridge:vlan: enable vlan-filtering on bridge, if vlans are set

Author: slyon

src/networkd.c

@@ -215,6 +215,11 @@ write_bridge_params_networkd(GString* s, const NetplanNetDefinition* def)
         if (def->bridge_params.max_age)
             g_string_append_printf(params, "MaxAgeSec=%s\n", def->bridge_params.max_age);
         g_string_append_printf(params, "STP=%s\n", def->bridge_params.stp ? "true" : "false");
+        if (def->bridge_params.vlans) {
+            // TODO: research and implement bridge vlans for networkd
+            g_fprintf(stderr, "ERROR: %s: networkd does not support bridge vlans\n", def->id);
+            exit(1);
+        }
 
         g_string_append_printf(s, "\n[Bridge]\n%s", params->str);
 
@@ -982,6 +987,11 @@ _netplan_netdef_write_network_file(
             g_string_append_printf(network, "Learning=%s\n", def->bridge_learning ? "true" : "false");
         if (def->bridge_neigh_suppress != NETPLAN_TRISTATE_UNSET)
             g_string_append_printf(network, "NeighborSuppression=%s\n", def->bridge_neigh_suppress ? "true" : "false");
+	if (def->bridge_params.port_vlans) {
+            // TODO: research and implement bridge port-vlans for networkd
+            g_fprintf(stderr, "ERROR: %s: networkd does not support bridge port-vlans\n", def->id);
+            exit(1);
+        }
 
     }
     if (def->bond && def->backend != NETPLAN_BACKEND_OVS) {

src/nm.c

@@ -145,6 +145,25 @@ wifi_band_str(const NetplanWifiBand band)
     }
 }
 
+/**
+ * Return NM bridge vlan string.
+ */
+static const char*
+bridge_vlan_str(const NetplanBridgeVlan* vlan)
+{
+    GString* s = NULL;
+    s = g_string_sized_new(20);
+
+    g_string_append_printf(s, "%u", vlan->vid);
+    if (vlan->vid_to)
+        g_string_append_printf(s, "-%u", vlan->vid_to);
+    if (vlan->pvid)
+        g_string_append(s, " pvid");
+    if (vlan->untagged)
+        g_string_append(s, " untagged");
+    return s->str;
+}
+
 /**
  * Return NM addr-gen-mode string.
  */
@@ -388,6 +407,18 @@ write_bridge_params_nm(const NetplanNetDefinition* def, GKeyFile *kf)
         if (def->bridge_params.max_age)
             g_key_file_set_string(kf, "bridge", "max-age", def->bridge_params.max_age);
         g_key_file_set_boolean(kf, "bridge", "stp", def->bridge_params.stp);
+        if (def->bridge_params.vlans) {
+            g_string_append(params, "vlan-filtering=true\n");
+            g_string_append(params, "vlans=");
+            for (unsigned i = 0; i < def->bridge_params.vlans->len; ++i) {
+                if (i > 0)
+                    g_string_append(params, ", ");
+                g_string_append_printf(params, "%s", bridge_vlan_str(
+                                       g_array_index(def->bridge_params.vlans,
+                                       NetplanBridgeVlan*, i)));
+            }
+            g_string_append(params, "\n");
+        }
     }
 }
 
@@ -824,6 +855,17 @@ write_nm_conf_access_point(const NetplanNetDefinition* def, const char* rootdir,
             g_key_file_set_uint64(kf, "bridge-port", "priority", def->bridge_params.port_priority);
         if (def->bridge_hairpin != NETPLAN_TRISTATE_UNSET)
             g_key_file_set_boolean(kf, "bridge-port", "hairpin-mode", def->bridge_hairpin);
+	if (def->bridge_params.port_vlans) {
+            g_string_append(s, "vlans=");
+            for (unsigned i = 0; i < def->bridge_params.port_vlans->len; ++i) {
+                if (i > 0)
+                    g_string_append(s, ", ");
+                g_string_append_printf(s, "%s", bridge_vlan_str(
+                                       g_array_index(def->bridge_params.port_vlans,
+                                       NetplanBridgeVlan*, i)));
+            }
+            g_string_append(s, "\n");
+        }
     }
     if (def->bond) {
         g_key_file_set_string(kf, "connection", "slave-type", "bond"); /* wokeignore:rule=slave */

src/parse.c

@@ -2167,6 +2167,108 @@ handle_bridge_port_priority(NetplanParser* npp, yaml_node_t* node, const char* k
     return TRUE;
 }
 
+static gboolean
+handle_generic_vlans(yaml_document_t* doc, yaml_node_t* node, GArray** entryptr, const void* data, GError** error)
+{
+    static regex_t re;
+    static gboolean re_inited = FALSE;
+
+    if (!re_inited) {
+        g_assert(regcomp(&re, "^([0-9]+)(-([0-9]+))?( (pvid))?( (untagged))?$", REG_EXTENDED) == 0);
+        re_inited = TRUE;
+    }
+
+    for (yaml_node_item_t *i = node->data.sequence.items.start; i < node->data.sequence.items.top; i++) {
+        g_autofree char* vlan = NULL;
+        yaml_node_t *entry = yaml_document_get_node(doc, *i);
+        assert_type(entry, YAML_SCALAR_NODE);
+
+        vlan = g_strdup(scalar(entry));
+
+        size_t maxGroups = 7+1;
+        regmatch_t groups[maxGroups];
+        /* does it match the vlans= definition? */
+        if (regexec(&re, vlan, maxGroups, groups, 0) == 0) {
+            NetplanBridgeVlan* data = g_new0(NetplanBridgeVlan, 1);
+            for (unsigned g = 1; g < maxGroups; g = g+2) {
+                if (groups[g].rm_so == (size_t)-1)
+                    continue; // Invalid group
+
+                char cursorCopy[strlen(vlan) + 1];
+                strcpy(cursorCopy, vlan);
+                cursorCopy[groups[g].rm_eo] = 0;
+                guint v = 0;
+                switch (g) {
+                    case 1:
+                        v = g_ascii_strtoull(cursorCopy + groups[g].rm_so, NULL, 10);
+                        if (v < 1 || v > 4094)
+                            return yaml_error(node, error, "malformed vlan vid '%u', must be in range [1..4094]", v);
+                        data->vid = v;
+                        break;
+                    case 3:
+                        v = g_ascii_strtoull(cursorCopy + groups[g].rm_so, NULL, 10);
+                        if (v < 1 || v > 4094)
+                            return yaml_error(node, error, "malformed vlan vid '%u', must be in range [1..4094]", v);
+                        else if (v <= data->vid)
+                            return yaml_error(node, error, "malformed vlan vid range '%s': %u > %u!", scalar(entry), data->vid, v);
+                        data->vid_to = v;
+                        break;
+                    case 5:
+                        data->pvid = TRUE;
+                        break;
+                    case 7:
+                        data->untagged = TRUE;
+                        break;
+                    default: g_assert_not_reached(); // LCOV_EXCL_LINE
+                }
+            }
+            if (!*entryptr)
+                *entryptr = g_array_new(FALSE, FALSE, sizeof(NetplanBridgeVlan*));
+            g_array_append_val(*entryptr, data);
+            continue;
+        }
+
+        return yaml_error(node, error, "malformed vlan '%s', must be: $vid [pvid] [untagged] [, $vid [pvid] [untagged]]", scalar(entry));
+    }
+
+    return TRUE;
+}
+
+static gboolean
+handle_bridge_vlans(yaml_document_t* doc, yaml_node_t* node, const void* data, GError** error)
+{
+    return handle_generic_vlans(doc, node, &(cur_netdef->bridge_params.vlans), data, error);
+}
+
+static gboolean
+handle_bridge_port_vlans(yaml_document_t* doc, yaml_node_t* node, const void* data, GError** error)
+{
+    for (yaml_node_pair_t* entry = node->data.mapping.pairs.start; entry < node->data.mapping.pairs.top; entry++) {
+        yaml_node_t* key, *value;
+        NetplanNetDefinition *component;
+        GArray** ref_ptr;
+
+        key = yaml_document_get_node(doc, entry->key);
+        assert_type(key, YAML_SCALAR_NODE);
+        value = yaml_document_get_node(doc, entry->value);
+        assert_type(value, YAML_SEQUENCE_NODE);
+
+        component = g_hash_table_lookup(netdefs, scalar(key));
+        if (!component) {
+            add_missing_node(key);
+        } else {
+            ref_ptr = &(component->bridge_params.port_vlans);
+            if (*ref_ptr)
+                return yaml_error(node, error, "%s: interface '%s' already has port vlans",
+                                  cur_netdef->id, scalar(key));
+
+            if (!handle_generic_vlans(doc, value, ref_ptr, data, error))
+                return FALSE;
+        }
+    }
+    return TRUE;
+}
+
 static const mapping_entry_handler bridge_params_handlers[] = {
     {"ageing-time", YAML_SCALAR_NODE, {.generic=handle_netdef_str}, netdef_offset(bridge_params.ageing_time)},
     {"aging-time", YAML_SCALAR_NODE, {.generic=handle_netdef_str}, netdef_offset(bridge_params.ageing_time)},
@@ -2175,8 +2277,10 @@ static const mapping_entry_handler bridge_params_handlers[] = {
     {"max-age", YAML_SCALAR_NODE, {.generic=handle_netdef_str}, netdef_offset(bridge_params.max_age)},
     {"path-cost", YAML_MAPPING_NODE, {.map={.custom=handle_bridge_path_cost}}, netdef_offset(bridge_params.path_cost)},
     {"port-priority", YAML_MAPPING_NODE, {.map={.custom=handle_bridge_port_priority}}, netdef_offset(bridge_params.port_priority)},
+    {"port-vlans", YAML_MAPPING_NODE, handle_bridge_port_vlans},
     {"priority", YAML_SCALAR_NODE, {.generic=handle_netdef_guint}, netdef_offset(bridge_params.priority)},
     {"stp", YAML_SCALAR_NODE, {.generic=handle_netdef_bool}, netdef_offset(bridge_params.stp)},
+    {"vlans", YAML_SEQUENCE_NODE, handle_bridge_vlans},
     {NULL}
 };
 

tests/generator/test_bridges.py

@@ -610,6 +610,77 @@ def test_bridge_stp(self):
         stp: no
       dhcp4: true''')
 
+    def test_bridge_vlans(self):
+        self.generate('''network:
+  version: 2
+  renderer: NetworkManager
+  ethernets:
+    eno1: {}
+    switchport: {}
+  bridges:
+    br0:
+      interfaces: [eno1, switchport]
+      parameters:
+        vlans: [1-100 pvid untagged, 42 untagged, 13, 1 pvid, 2-100 pvid untagged]
+        port-vlans:
+          eno1: [99-999 pvid untagged, 1 untagged, 42 pvid]
+          switchport: [4000-4094, 1 pvid, 13 untagged]''')
+
+        self.assert_nm({'br0': '''[connection]
+id=netplan-br0
+type=bridge
+interface-name=br0
+
+[bridge]
+stp=true
+vlan-filtering=true
+vlans=1-100 pvid untagged, 42 untagged, 13, 1 pvid, 2-100 pvid untagged
+
+[ipv4]
+method=link-local
+
+[ipv6]
+method=ignore
+''',
+                        'eno1': '''[connection]
+id=netplan-eno1
+type=ethernet
+interface-name=eno1
+slave-type=bridge
+master=br0
+
+[bridge-port]
+vlans=99-999 pvid untagged, 1 untagged, 42 pvid
+
+[ethernet]
+wake-on-lan=0
+
+[ipv4]
+method=link-local
+
+[ipv6]
+method=ignore
+''',
+                        'switchport': '''[connection]
+id=netplan-switchport
+type=ethernet
+interface-name=switchport
+slave-type=bridge
+master=br0
+
+[bridge-port]
+vlans=4000-4094, 1 pvid, 13 untagged
+
+[ethernet]
+wake-on-lan=0
+
+[ipv4]
+method=link-local
+
+[ipv6]
+method=ignore
+'''})
+
 
 class TestConfigErrors(TestBase):
 
@@ -724,3 +795,95 @@ def test_bridge_invalid_port_prio(self):
         port-priority:
           eno1: 257
       dhcp4: true''', expect_fail=True)
+
+    def test_bridge_no_vlan(self):
+        err = self.generate('''network:
+  version: 2
+  bridges:
+    br0:
+      parameters:
+        vlans: [99-999 pvid untagged, 1 untagged, 42 pvid]''', expect_fail=True)
+        self.assertIn("ERROR: br0: networkd does not support bridge vlans", err)
+
+    def test_bridge_no_port_vlan(self):
+        err = self.generate('''network:
+  version: 2
+  ethernets:
+    eno1: {}
+  bridges:
+    br0:
+      interfaces: [eno1]
+      parameters:
+        port-vlans:
+          eno1: [99-999 pvid untagged, 1 untagged, 42 pvid]''', expect_fail=True)
+        self.assertIn("ERROR: eno1: networkd does not support bridge port-vlans", err)
+
+    def test_bridge_invalid_vlan(self):
+        err = self.generate('''network:
+  version: 2
+  bridges:
+    br0:
+      parameters:
+        vlans: [1 unmapped INVALID]''', expect_fail=True)
+        self.assertIn("Error in network definition: malformed vlan '1 unmapped INVALID', must be: $vid [pvid] [untagged] \
+[, $vid [pvid] [untagged]]", err)
+
+    def test_bridge_invalid_vlan_vid(self):
+        err = self.generate('''network:
+  version: 2
+  bridges:
+    br0:
+      parameters:
+        vlans: [0]''', expect_fail=True)
+        self.assertIn("Error in network definition: malformed vlan vid '0', must be in range [1..4094]", err)
+
+    def test_bridge_invalid_port_vlan_vid_to(self):
+        err = self.generate('''network:
+  version: 2
+  ethernets:
+    eno1: {}
+  bridges:
+    br0:
+      interfaces: [eno1]
+      parameters:
+        port-vlans:
+          eno1: [1-4095]''', expect_fail=True)
+        self.assertIn("Error in network definition: malformed vlan vid '4095', must be in range [1..4094]", err)
+
+    def test_bridge_port_vlan_already_defined(self):
+        err = self.generate('''network:
+  version: 2
+  ethernets:
+    eno1: {}
+  bridges:
+    br0:
+      interfaces: [eno1]
+      parameters:
+        port-vlans:
+          eno1: [1]
+          eno1: [1]''', expect_fail=True)
+        self.assertIn("Error in network definition: br0: interface 'eno1' already has port vlans", err)
+
+    def test_bridge_invalid_vlan_vid_range(self):
+        err = self.generate('''network:
+  version: 2
+  bridges:
+    br0:
+      parameters:
+        vlans: [100-1]''', expect_fail=True)
+        self.assertIn("Error in network definition: malformed vlan vid range '100-1': 100 > 1!", err)
+
+    def test_bridge_port_vlan_add_missing_node(self):
+        err = self.generate('''network:
+  version: 2
+  ethernets:
+    eno1:
+      match:
+        name: eth0
+  bridges:
+    br0:
+      interfaces: [eno1]
+      parameters:
+        port-vlans:
+          eth0: [1]''', expect_fail=True)
+        self.assertIn("Error in network definition: br0: interface 'eth0' is not defined", err)