From 2a657c2b5208d6720ce728a9d5868ae1f066247f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20M=C3=A4rdian?= Date: Fri, 3 Apr 2020 16:28:38 +0200 Subject: [PATCH 1/3] bridges: implement vlans & port-vlans options for NM backend NM:bridge:vlan: enable vlan-filtering on bridge, if vlans are set --- src/networkd.c | 10 ++ src/nm.c | 42 ++++++++ src/parse.c | 104 ++++++++++++++++++++ tests/generator/test_bridges.py | 163 ++++++++++++++++++++++++++++++++ 4 files changed, 319 insertions(+) diff --git a/src/networkd.c b/src/networkd.c index 982d18cb9..3e473c17c 100644 --- a/src/networkd.c +++ b/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) { diff --git a/src/nm.c b/src/nm.c index 583fd2d62..48e906391 100644 --- a/src/nm.c +++ b/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 */ diff --git a/src/parse.c b/src/parse.c index 64a9a809f..fd4c04a96 100644 --- a/src/parse.c +++ b/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} }; diff --git a/tests/generator/test_bridges.py b/tests/generator/test_bridges.py index 26b2961d5..ed4e3eb67 100644 --- a/tests/generator/test_bridges.py +++ b/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) From 0e96bd3e891edb31d6a7c15c238fcd066731c065 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Thu, 23 Jan 2025 16:56:37 +0100 Subject: [PATCH 2/3] Moved changes to current codebase Signed-off-by: Patryk Strusiewicz-Surmacki --- doc/.custom_wordlist.txt | 3 ++ doc/netplan-yaml.md | 17 ++++++++++ src/abi.h | 10 ++++++ src/generate.c | 1 + src/netplan.c | 48 ++++++++++++++++++++++++++++ src/networkd.c | 3 +- src/nm.c | 37 ++++++++++++---------- src/nm.h | 3 ++ src/parse.c | 56 +++++++++++++++++---------------- src/types.c | 3 ++ tests/generator/base.py | 3 +- tests/generator/test_bridges.py | 9 +++--- 12 files changed, 142 insertions(+), 51 deletions(-) diff --git a/doc/.custom_wordlist.txt b/doc/.custom_wordlist.txt index 1ec8f3fdc..0d333411c 100644 --- a/doc/.custom_wordlist.txt +++ b/doc/.custom_wordlist.txt @@ -133,6 +133,7 @@ networkd nm passthrough programmatically +pvid renderer reselection runtime @@ -145,6 +146,8 @@ udev unconfigured unencrypted untagged +vid +vlans vSwitch wpa wpasupplicant diff --git a/doc/netplan-yaml.md b/doc/netplan-yaml.md index b22755755..3758a232f 100644 --- a/doc/netplan-yaml.md +++ b/doc/netplan-yaml.md @@ -1337,6 +1337,13 @@ The specific settings for bridges are defined below. eth1: 20 ``` + - **`port-vlans`** (sequence of scalars) + + > Array of bridge VLAN objects. The VLAN list can be specified with the + > following syntax: $vid [pvid] [untagged] [, $vid [pvid] [untagged]].. + > where $vid is either a single id between 1 and 4094 or a range, + > represented as a couple of ids separated by a dash. + - **`forward-delay`** (scalar) > Specify the period of time the bridge will remain in Listening and @@ -1345,6 +1352,16 @@ The specific settings for bridges are defined below. > If no time suffix is specified, the value will be interpreted as > seconds. + - **`vlans`** (sequence of scalars) + > Array of bridge VLAN objects. The VLAN list can be specified with the + > following syntax: $vid [pvid] [untagged] [, $vid [pvid] [untagged]].. + > where $vid is either a single id between 1 and 4094 or a range, + > represented as a couple of ids separated by a dash. + + - **`vlan-filtering`** (boolean) + + > Enables VLAN filtering. Will be enabled by default if *vlans* are defined. + - **`hello-time`** (scalar) > Specify the interval between two hello packets being sent out from diff --git a/src/abi.h b/src/abi.h index 3bb25d31d..a370b4264 100644 --- a/src/abi.h +++ b/src/abi.h @@ -342,6 +342,9 @@ struct netplan_net_definition { char* max_age; guint path_cost; gboolean stp; + GArray* vlans; + GArray* port_vlans; + gboolean vlan_filtering; } bridge_params; gboolean custom_bridging; @@ -432,3 +435,10 @@ struct netplan_net_definition { NetplanRAOverrides ra_overrides; }; + +typedef struct { + guint vid; //[1..4094] + guint vid_to; //set iff vid range defined + gboolean pvid; + gboolean untagged; +} NetplanBridgeVlan; diff --git a/src/generate.c b/src/generate.c index aa5232437..17c8562b2 100644 --- a/src/generate.c +++ b/src/generate.c @@ -203,6 +203,7 @@ find_interface(gchar* interface, GHashTable* netdefs) int main(int argc, char** argv) { + g_log_set_handler(G_LOG_DOMAIN, G_LOG_LEVEL_DEBUG, g_log_default_handler, NULL); NetplanError* error = NULL; GOptionContext* opt_context; /* are we being called as systemd generator? */ diff --git a/src/netplan.c b/src/netplan.c index d6ab84902..2de50fbaf 100644 --- a/src/netplan.c +++ b/src/netplan.c @@ -28,6 +28,7 @@ #include "yaml-helpers.h" #include "util-internal.h" #include "names.h" +#include "nm.h" gchar *tmp = NULL; @@ -310,6 +311,53 @@ write_bridge_params(yaml_event_t* event, yaml_emitter_t* emitter, const NetplanN YAML_MAPPING_CLOSE(event, emitter); } + gboolean has_port_vlans = FALSE; + for (unsigned i = 0; i < interfaces->len; ++i) { + NetplanNetDefinition *nd = g_array_index(interfaces, NetplanNetDefinition*, i); + if (nd->bridge_params.port_vlans || DIRTY(nd, nd->bridge_params.port_vlans)) { + has_port_vlans = TRUE; + break; + } + } + + if (has_port_vlans) { + YAML_SCALAR_PLAIN(event, emitter, "port-vlans"); + YAML_MAPPING_OPEN(event, emitter); + for (unsigned i = 0; i < interfaces->len; ++i) { + NetplanNetDefinition *nd = g_array_index(interfaces, NetplanNetDefinition*, i); + if (nd->bridge_params.port_vlans || DIRTY(nd, nd->bridge_params.port_vlans)) { + YAML_SCALAR_PLAIN(event, emitter, nd->id); + YAML_SEQUENCE_OPEN(event, emitter); + for (unsigned i = 0; i < nd->bridge_params.port_vlans->len; ++i) { + NetplanBridgeVlan* vlan = g_array_index(nd->bridge_params.port_vlans, NetplanBridgeVlan*, i); + GString* v = bridge_vlan_str(vlan); + YAML_SCALAR_PLAIN(event, emitter, v->str); + g_string_free(v, TRUE); + } + YAML_SEQUENCE_CLOSE(event, emitter); + } + + } + YAML_MAPPING_CLOSE(event, emitter); + } + + if (def->bridge_params.vlan_filtering || def->bridge_params.vlans || DIRTY(def, def->bridge_params.vlans)) { + YAML_STRING(def, event, emitter, "vlan-filtering", "true"); + if (def->bridge_params.vlans || DIRTY(def, def->bridge_params.vlans)) { + YAML_SCALAR_PLAIN(event, emitter, "vlans"); + YAML_SEQUENCE_OPEN(event, emitter); + for (unsigned i = 0; i < def->bridge_params.vlans->len; ++i) { + NetplanBridgeVlan* vlan = g_array_index(def->bridge_params.vlans, NetplanBridgeVlan*, i); + GString* v = bridge_vlan_str(vlan); + YAML_SCALAR_PLAIN(event, emitter, v->str); + g_string_free(v, TRUE); + } + YAML_SEQUENCE_CLOSE(event, emitter); + } + } + + + YAML_MAPPING_CLOSE(event, emitter); } return TRUE; diff --git a/src/networkd.c b/src/networkd.c index 3e473c17c..fe44f3a2a 100644 --- a/src/networkd.c +++ b/src/networkd.c @@ -987,12 +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) { + 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) { g_string_append_printf(network, "Bond=%s\n", def->bond); diff --git a/src/nm.c b/src/nm.c index 48e906391..2e9178f24 100644 --- a/src/nm.c +++ b/src/nm.c @@ -148,11 +148,11 @@ wifi_band_str(const NetplanWifiBand band) /** * Return NM bridge vlan string. */ -static const char* +GString* bridge_vlan_str(const NetplanBridgeVlan* vlan) { GString* s = NULL; - s = g_string_sized_new(20); + s = g_string_sized_new(24); g_string_append_printf(s, "%u", vlan->vid); if (vlan->vid_to) @@ -161,7 +161,7 @@ bridge_vlan_str(const NetplanBridgeVlan* vlan) g_string_append(s, " pvid"); if (vlan->untagged) g_string_append(s, " untagged"); - return s->str; + return s; } /** @@ -395,7 +395,10 @@ write_nm_bond_parameters(const NetplanNetDefinition* def, GKeyFile *kf) STATIC void write_bridge_params_nm(const NetplanNetDefinition* def, GKeyFile *kf) { + g_autoptr(GString) vlans = NULL; + if (def->custom_bridging) { + vlans = g_string_sized_new(200); if (def->bridge_params.ageing_time) g_key_file_set_string(kf, "bridge", "ageing-time", def->bridge_params.ageing_time); if (def->bridge_params.priority) @@ -407,17 +410,17 @@ 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.vlan_filtering || def->bridge_params.vlans) + g_key_file_set_string(kf, "bridge", "vlan-filtering", "true"); 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(vlans, ", "); + GString* v = bridge_vlan_str(g_array_index(def->bridge_params.vlans,NetplanBridgeVlan*, i)); + g_string_append_printf(vlans, "%s", v->str); + g_string_free(v, TRUE); } - g_string_append(params, "\n"); + g_key_file_set_string(kf, "bridge", "vlans", vlans->str); } } } @@ -855,16 +858,16 @@ 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="); + if (def->bridge_params.port_vlans) { + g_autoptr(GString) vlans = g_string_sized_new(200); 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(vlans, ", "); + GString* v = bridge_vlan_str(g_array_index(def->bridge_params.port_vlans,NetplanBridgeVlan*, i)); + g_string_append_printf(vlans, "%s", v->str); + g_string_free(v, TRUE); } - g_string_append(s, "\n"); + g_key_file_set_string(kf, "bridge-port", "vlans", vlans->str); } } if (def->bond) { diff --git a/src/nm.h b/src/nm.h index 0a9ca6e15..a63932e0f 100644 --- a/src/nm.h +++ b/src/nm.h @@ -29,3 +29,6 @@ _netplan_netdef_write_nm( NETPLAN_INTERNAL gboolean _netplan_nm_cleanup(const char* rootdir); + +NETPLAN_INTERNAL GString* +bridge_vlan_str(const NetplanBridgeVlan* vlan); diff --git a/src/parse.c b/src/parse.c index fd4c04a96..c66ca1847 100644 --- a/src/parse.c +++ b/src/parse.c @@ -2167,8 +2167,8 @@ 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 gboolean +handle_generic_vlans(NetplanParser* npp, yaml_node_t* node, GArray** entryptr, GError** error) { static regex_t re; static gboolean re_inited = FALSE; @@ -2180,8 +2180,8 @@ handle_generic_vlans(yaml_document_t* doc, yaml_node_t* node, GArray** entryptr, 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); + yaml_node_t *entry = yaml_document_get_node(&npp->doc, *i); + assert_type(npp, entry, YAML_SCALAR_NODE); vlan = g_strdup(scalar(entry)); @@ -2191,7 +2191,7 @@ handle_generic_vlans(yaml_document_t* doc, yaml_node_t* node, GArray** entryptr, 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) + if (groups[g].rm_so == (int)(size_t)-1) continue; // Invalid group char cursorCopy[strlen(vlan) + 1]; @@ -2200,17 +2200,17 @@ handle_generic_vlans(yaml_document_t* doc, yaml_node_t* node, GArray** entryptr, guint v = 0; switch (g) { case 1: - v = g_ascii_strtoull(cursorCopy + groups[g].rm_so, NULL, 10); + v = (guint) 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); + return yaml_error(npp, 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); + v = (guint) 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); + return yaml_error(npp, 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); + return yaml_error(npp, node, error, "malformed vlan vid range '%s': %u > %u!", scalar(entry), data->vid, v); data->vid_to = v; break; case 5: @@ -2228,41 +2228,41 @@ handle_generic_vlans(yaml_document_t* doc, yaml_node_t* node, GArray** entryptr, continue; } - return yaml_error(node, error, "malformed vlan '%s', must be: $vid [pvid] [untagged] [, $vid [pvid] [untagged]]", scalar(entry)); + return yaml_error(npp, 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) +STATIC gboolean +handle_bridge_vlans(NetplanParser* npp, yaml_node_t* node, const void *, GError** error) { - return handle_generic_vlans(doc, node, &(cur_netdef->bridge_params.vlans), data, error); + return handle_generic_vlans(npp, node, &(npp->current.netdef->bridge_params.vlans), error); } -static gboolean -handle_bridge_port_vlans(yaml_document_t* doc, yaml_node_t* node, const void* data, GError** error) +STATIC gboolean +handle_bridge_port_vlans(NetplanParser* npp, yaml_node_t* node, const char*, const void*, 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); + key = yaml_document_get_node(&npp->doc, entry->key); + assert_type(npp, key, YAML_SCALAR_NODE); + value = yaml_document_get_node(&npp->doc, entry->value); + assert_type(npp, value, YAML_SEQUENCE_NODE); - component = g_hash_table_lookup(netdefs, scalar(key)); + component = g_hash_table_lookup(npp->parsed_defs, scalar(key)); if (!component) { - add_missing_node(key); + add_missing_node(npp, 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)); + return yaml_error(npp, node, error, "%s: interface '%s' already has port vlans", + npp->current.netdef->id, scalar(key)); - if (!handle_generic_vlans(doc, value, ref_ptr, data, error)) + if (!handle_generic_vlans(npp, value, ref_ptr, error)) return FALSE; } } @@ -2277,10 +2277,11 @@ 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}, + {"port-vlans", YAML_MAPPING_NODE, {.map={.custom=handle_bridge_port_vlans}}, netdef_offset(bridge_params.port_vlans)}, + {"vlans", YAML_SEQUENCE_NODE, {.generic=handle_bridge_vlans}, netdef_offset(bridge_params.vlans)}, + {"vlan-filtering", YAML_SCALAR_NODE, {.generic=handle_netdef_bool}, netdef_offset(bridge_params.vlan_filtering)}, {NULL} }; @@ -3772,6 +3773,7 @@ netplan_parser_load_yaml(NetplanParser* npp, const char* filename, GError** erro if (!load_yaml(filename, doc, error)) return FALSE; + return _netplan_parser_load_single_file(npp, filename, doc, error); } diff --git a/src/types.c b/src/types.c index 7a1c20ed2..1adb6c08a 100644 --- a/src/types.c +++ b/src/types.c @@ -339,6 +339,9 @@ reset_netdef(NetplanNetDefinition* netdef, NetplanDefType new_type, NetplanBacke FREE_AND_NULLIFY(netdef->bridge_params.forward_delay); FREE_AND_NULLIFY(netdef->bridge_params.hello_time); FREE_AND_NULLIFY(netdef->bridge_params.max_age); + free_garray_with_destructor(&netdef->bridge_params.vlans, g_free); + free_garray_with_destructor(&netdef->bridge_params.port_vlans, g_free); + netdef->bridge_params.vlan_filtering = FALSE; memset(&netdef->bridge_params, 0, sizeof(netdef->bridge_params)); netdef->custom_bridging = FALSE; diff --git a/tests/generator/base.py b/tests/generator/base.py index 30f42adde..fd21e06cc 100644 --- a/tests/generator/base.py +++ b/tests/generator/base.py @@ -183,8 +183,9 @@ def normalize_yaml_tree(self, data, full_key=''): ''' if isinstance(data, list): scalars_only = not any(list(map(lambda elem: (isinstance(elem, dict) or isinstance(elem, list)), data))) + # sort sequence alphabetically - if scalars_only: + if scalars_only and all(isinstance(x, type(data[0])) for x in data): data.sort() # remove duplicates (if needed) unique = set(data) diff --git a/tests/generator/test_bridges.py b/tests/generator/test_bridges.py index ed4e3eb67..33669d5f8 100644 --- a/tests/generator/test_bridges.py +++ b/tests/generator/test_bridges.py @@ -621,6 +621,7 @@ def test_bridge_vlans(self): br0: interfaces: [eno1, switchport] parameters: + vlan-filtering: true 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] @@ -646,8 +647,8 @@ def test_bridge_vlans(self): id=netplan-eno1 type=ethernet interface-name=eno1 -slave-type=bridge -master=br0 +slave-type=bridge # wokeignore:rule=slave +master=br0 # wokeignore:rule=master [bridge-port] vlans=99-999 pvid untagged, 1 untagged, 42 pvid @@ -665,8 +666,8 @@ def test_bridge_vlans(self): id=netplan-switchport type=ethernet interface-name=switchport -slave-type=bridge -master=br0 +slave-type=bridge # wokeignore:rule=slave +master=br0 # wokeignore:rule=master [bridge-port] vlans=4000-4094, 1 pvid, 13 untagged From 9c79dc84dacba734db2121e356d65c207e7a6cb9 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Tue, 4 Feb 2025 16:46:48 +0100 Subject: [PATCH 3/3] Added networkd backend Signed-off-by: Patryk Strusiewicz-Surmacki --- doc/netplan-yaml.md | 5 ++ examples/bridge_port_vlans.yaml | 16 ++++ src/abi.h | 3 +- src/netplan.c | 4 +- src/networkd.c | 61 +++++++++++-- src/nm.c | 7 ++ src/parse.c | 56 ++++++++++-- src/types.c | 1 + tests/generator/test_bridges.py | 152 ++++++++++++++++++++++++++------ 9 files changed, 263 insertions(+), 42 deletions(-) create mode 100644 examples/bridge_port_vlans.yaml diff --git a/doc/netplan-yaml.md b/doc/netplan-yaml.md index 3758a232f..b9017738b 100644 --- a/doc/netplan-yaml.md +++ b/doc/netplan-yaml.md @@ -1361,6 +1361,11 @@ The specific settings for bridges are defined below. - **`vlan-filtering`** (boolean) > Enables VLAN filtering. Will be enabled by default if *vlans* are defined. + + - **`vlan-default-pvid`** (scalar) + + > Specifies the default port VLAN ID. Can be set to values between 1 and 4094, + > or to value `none` if `networkd` is used as a renderer. Defaults to `1`. - **`hello-time`** (scalar) diff --git a/examples/bridge_port_vlans.yaml b/examples/bridge_port_vlans.yaml new file mode 100644 index 000000000..8ea9eb91e --- /dev/null +++ b/examples/bridge_port_vlans.yaml @@ -0,0 +1,16 @@ +network: + version: 2 + renderer: networkd + ethernets: + eno1: {} + switchport: {} + bridges: + br0: + interfaces: [eno1, eno2] + parameters: + vlan-filtering: true + vlan-default-pvid: 42 + vlans: [10 pvid untagged, 20 untagged, 50] + port-vlans: + eno1: [2 pvid untagged, 4 untagged] + eno2: [4000-4094, 0 pvid, 30 untagged] # 0 pvid can only be used with networkd backend diff --git a/src/abi.h b/src/abi.h index a370b4264..f2e3b4aca 100644 --- a/src/abi.h +++ b/src/abi.h @@ -345,6 +345,7 @@ struct netplan_net_definition { GArray* vlans; GArray* port_vlans; gboolean vlan_filtering; + char* vlan_default_pvid; } bridge_params; gboolean custom_bridging; @@ -438,7 +439,7 @@ struct netplan_net_definition { typedef struct { guint vid; //[1..4094] - guint vid_to; //set iff vid range defined + guint vid_to; //set if vid range is defined gboolean pvid; gboolean untagged; } NetplanBridgeVlan; diff --git a/src/netplan.c b/src/netplan.c index 2de50fbaf..8a63f5743 100644 --- a/src/netplan.c +++ b/src/netplan.c @@ -356,7 +356,9 @@ write_bridge_params(yaml_event_t* event, yaml_emitter_t* emitter, const NetplanN } } - + if (def->bridge_params.vlan_default_pvid) { + YAML_STRING(def, event, emitter, "vlan-default-pvid", def->bridge_params.vlan_default_pvid); + } YAML_MAPPING_CLOSE(event, emitter); } diff --git a/src/networkd.c b/src/networkd.c index fe44f3a2a..c52f93b07 100644 --- a/src/networkd.c +++ b/src/networkd.c @@ -215,11 +215,10 @@ 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); - } + if (def->bridge_params.vlan_default_pvid) + g_string_append_printf(params, "DefaultPVID=%s\n", def->bridge_params.vlan_default_pvid); + if(def->bridge_params.vlan_filtering || def->bridge_params.vlans) + g_string_append_printf(params, "VLANFiltering=true\n"); g_string_append_printf(s, "\n[Bridge]\n%s", params->str); @@ -836,6 +835,47 @@ combine_dhcp_overrides(const NetplanNetDefinition* def, NetplanDHCPOverrides* co return TRUE; } +/** + * Return networkd vlan string. + */ +GString* +bridge_vlan_networkd_str(const NetplanBridgeVlan* vlan) +{ + GString *id = g_string_sized_new(9); + GString *def = g_string_sized_new(200); + + g_string_append_printf(id, "%u", vlan->vid); + if (vlan->vid_to) + g_string_append_printf(id, "-%u", vlan->vid_to); + + + if (vlan->pvid) + g_string_append_printf(def, "PVID=%s\n", id->str); + else { + g_string_append_printf(def, "VLAN=%s\n", id->str); + } + + if (vlan->untagged) + g_string_append_printf(def, "EgressUntagged=%s\n", id->str); + + g_string_free(id, TRUE); + + return def; +} + +/** + * Write the needed networkd .network BridgeVLAN configuration for the selected vlan definition. + */ +STATIC void +write_vlans(GString_autoptr network, GArray* data) { + g_string_append(network, "\n[BridgeVLAN]\n"); + for (unsigned i = 0; i < data->len; ++i) { + GString* v = bridge_vlan_networkd_str(g_array_index(data,NetplanBridgeVlan*, i)); + g_string_append_printf(network, "%s", v->str); + g_string_free(v, TRUE); + } +} + /** * Write the needed networkd .network configuration for the selected netplan definition. */ @@ -988,11 +1028,16 @@ _netplan_netdef_write_network_file( 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); + // port's .network file + write_vlans(network, def->bridge_params.port_vlans); } } + + if (!def->bridge && !def->bond && def->backend != NETPLAN_BACKEND_OVS && def->bridge_params.vlans) { + // bridge's .network file + write_vlans(network, def->bridge_params.vlans); + } + if (def->bond && def->backend != NETPLAN_BACKEND_OVS) { g_string_append_printf(network, "Bond=%s\n", def->bond); diff --git a/src/nm.c b/src/nm.c index 2e9178f24..00aed877b 100644 --- a/src/nm.c +++ b/src/nm.c @@ -412,6 +412,13 @@ write_bridge_params_nm(const NetplanNetDefinition* def, GKeyFile *kf) g_key_file_set_boolean(kf, "bridge", "stp", def->bridge_params.stp); if(def->bridge_params.vlan_filtering || def->bridge_params.vlans) g_key_file_set_string(kf, "bridge", "vlan-filtering", "true"); + if (def->bridge_params.vlan_default_pvid) { + if (g_str_equal(def->bridge_params.vlan_default_pvid, "none")) { + g_fprintf(stderr, "ERROR: vlan-default-pvid cannot be set to 'none' if NetworkManager is used\n"); + exit(1); + } + g_key_file_set_string(kf, "bridge", "vlan-default-pvid", def->bridge_params.vlan_default_pvid); + } if (def->bridge_params.vlans) { for (unsigned i = 0; i < def->bridge_params.vlans->len; ++i) { if (i > 0) diff --git a/src/parse.c b/src/parse.c index c66ca1847..7b17cb140 100644 --- a/src/parse.c +++ b/src/parse.c @@ -2178,6 +2178,7 @@ handle_generic_vlans(NetplanParser* npp, yaml_node_t* node, GArray** entryptr, G re_inited = TRUE; } + unsigned pvids = 0; 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(&npp->doc, *i); @@ -2187,6 +2188,9 @@ handle_generic_vlans(NetplanParser* npp, yaml_node_t* node, GArray** entryptr, G size_t maxGroups = 7+1; regmatch_t groups[maxGroups]; + + guint minVid = npp->global_backend == NETPLAN_BACKEND_NETWORKD ? 0:1; + /* does it match the vlans= definition? */ if (regexec(&re, vlan, maxGroups, groups, 0) == 0) { NetplanBridgeVlan* data = g_new0(NetplanBridgeVlan, 1); @@ -2201,20 +2205,33 @@ handle_generic_vlans(NetplanParser* npp, yaml_node_t* node, GArray** entryptr, G switch (g) { case 1: v = (guint) g_ascii_strtoull(cursorCopy + groups[g].rm_so, NULL, 10); - if (v < 1 || v > 4094) - return yaml_error(npp, node, error, "malformed vlan vid '%u', must be in range [1..4094]", v); + if (v < minVid || v > 4094) { + g_free(data); + return yaml_error(npp, node, error, "malformed vlan vid '%u', must be in range [%d..4094]", v, minVid); + } data->vid = v; break; case 3: v = (guint) g_ascii_strtoull(cursorCopy + groups[g].rm_so, NULL, 10); - if (v < 1 || v > 4094) + if (v < 1 || v > 4094) { + g_free(data); return yaml_error(npp, node, error, "malformed vlan vid '%u', must be in range [1..4094]", v); - else if (v <= data->vid) - return yaml_error(npp, node, error, "malformed vlan vid range '%s': %u > %u!", scalar(entry), data->vid, v); + } + + else if (v <= data->vid) { + guint vid = data->vid; + g_free(data); + return yaml_error(npp, node, error, "malformed vlan vid range '%s': %u > %u!", scalar(entry), vid, v); + } + data->vid_to = v; break; case 5: data->pvid = TRUE; + if (++pvids > 1) { + g_free(data); + return yaml_error(npp, node, error, "malformed vlan pvid '%s': only single pvid can be defined", scalar(entry)); + } break; case 7: data->untagged = TRUE; @@ -2222,6 +2239,16 @@ handle_generic_vlans(NetplanParser* npp, yaml_node_t* node, GArray** entryptr, G default: g_assert_not_reached(); // LCOV_EXCL_LINE } } + + if (npp->global_backend == NETPLAN_BACKEND_NETWORKD && !data->pvid && data->vid == 0) { + g_free(data); + return yaml_error(npp, node, error, "malformed vlan '%s': value cannot be defined as 0 for non-pvid", scalar(entry)); + } + + if (data->vid_to > 0 && data->pvid) { + g_free(data); + return yaml_error(npp, node, error, "malformed vlan '%s': pvid cannot be defined as a range", scalar(entry)); + } if (!*entryptr) *entryptr = g_array_new(FALSE, FALSE, sizeof(NetplanBridgeVlan*)); g_array_append_val(*entryptr, data); @@ -2269,6 +2296,24 @@ handle_bridge_port_vlans(NetplanParser* npp, yaml_node_t* node, const char*, con return TRUE; } +/** + * Handler for vlan-default-pvid. + * @data: offset into NetplanNetDefinition where the const char* field to write is + * located + */ +STATIC gboolean +handle_vlan_default_pvid(NetplanParser* npp, yaml_node_t* node, const void* data, GError** error) +{ + const char* pvid = scalar(node); + GError** err = NULL; + guint64 val = 0; + if (strcmp(pvid, "none") != 0 && !g_ascii_string_to_unsigned(pvid, 10, 1, 4094 , &val, err)) { + return yaml_error(npp, node, error, "malformed value of vlan-default-pvid '%s': vlan-default-pvid can only be defined as a single port ID", pvid); + } + + return handle_netdef_str(npp, node, data, error); +} + 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)}, @@ -2282,6 +2327,7 @@ static const mapping_entry_handler bridge_params_handlers[] = { {"port-vlans", YAML_MAPPING_NODE, {.map={.custom=handle_bridge_port_vlans}}, netdef_offset(bridge_params.port_vlans)}, {"vlans", YAML_SEQUENCE_NODE, {.generic=handle_bridge_vlans}, netdef_offset(bridge_params.vlans)}, {"vlan-filtering", YAML_SCALAR_NODE, {.generic=handle_netdef_bool}, netdef_offset(bridge_params.vlan_filtering)}, + {"vlan-default-pvid", YAML_SCALAR_NODE, {.generic=handle_vlan_default_pvid}, netdef_offset(bridge_params.vlan_default_pvid)}, {NULL} }; diff --git a/src/types.c b/src/types.c index 1adb6c08a..e1eb77779 100644 --- a/src/types.c +++ b/src/types.c @@ -342,6 +342,7 @@ reset_netdef(NetplanNetDefinition* netdef, NetplanDefType new_type, NetplanBacke free_garray_with_destructor(&netdef->bridge_params.vlans, g_free); free_garray_with_destructor(&netdef->bridge_params.port_vlans, g_free); netdef->bridge_params.vlan_filtering = FALSE; + FREE_AND_NULLIFY(netdef->bridge_params.vlan_default_pvid); memset(&netdef->bridge_params, 0, sizeof(netdef->bridge_params)); netdef->custom_bridging = FALSE; diff --git a/tests/generator/test_bridges.py b/tests/generator/test_bridges.py index 33669d5f8..7f2e9f124 100644 --- a/tests/generator/test_bridges.py +++ b/tests/generator/test_bridges.py @@ -610,7 +610,7 @@ def test_bridge_stp(self): stp: no dhcp4: true''') - def test_bridge_vlans(self): + def test_bridge_vlans_nm(self): self.generate('''network: version: 2 renderer: NetworkManager @@ -622,9 +622,10 @@ def test_bridge_vlans(self): interfaces: [eno1, switchport] parameters: vlan-filtering: true - vlans: [1-100 pvid untagged, 42 untagged, 13, 1 pvid, 2-100 pvid untagged] + vlan-default-pvid: 123 + vlans: [100 pvid untagged, 42 untagged, 13] port-vlans: - eno1: [99-999 pvid untagged, 1 untagged, 42 pvid] + eno1: [99 pvid untagged, 1 untagged] switchport: [4000-4094, 1 pvid, 13 untagged]''') self.assert_nm({'br0': '''[connection] @@ -635,7 +636,8 @@ def test_bridge_vlans(self): [bridge] stp=true vlan-filtering=true -vlans=1-100 pvid untagged, 42 untagged, 13, 1 pvid, 2-100 pvid untagged +vlan-default-pvid=123 +vlans=100 pvid untagged, 42 untagged, 13 [ipv4] method=link-local @@ -651,7 +653,7 @@ def test_bridge_vlans(self): master=br0 # wokeignore:rule=master [bridge-port] -vlans=99-999 pvid untagged, 1 untagged, 42 pvid +vlans=99 pvid untagged, 1 untagged [ethernet] wake-on-lan=0 @@ -682,6 +684,70 @@ def test_bridge_vlans(self): method=ignore '''}) + def test_bridge_vlans_networkd(self): + self.generate('''network: + version: 2 + renderer: networkd + ethernets: + eno1: {} + switchport: {} + bridges: + br0: + interfaces: [eno1, switchport] + parameters: + vlan-filtering: true + vlan-default-pvid: 123 + vlans: [100 pvid untagged, 42 untagged, 13] + port-vlans: + eno1: [99 pvid untagged, 1 untagged] + switchport: [4000-4094, 0 pvid, 13 untagged]''') + + self.assert_networkd({'br0.netdev': '[NetDev]\nName=br0\nKind=bridge\n\n' + '[Bridge]\n' + 'STP=true\n' + 'DefaultPVID=123\n' + 'VLANFiltering=true\n', + 'br0.network': '''[Match] +Name=br0 + +[Network] +LinkLocalAddressing=ipv6 +ConfigureWithoutCarrier=yes + +[BridgeVLAN] +PVID=100 +EgressUntagged=100 +VLAN=42 +EgressUntagged=42 +VLAN=13 +''', + 'eno1.network': '''[Match] +Name=eno1 + +[Network] +LinkLocalAddressing=no +Bridge=br0 + +[BridgeVLAN] +PVID=99 +EgressUntagged=99 +VLAN=1 +EgressUntagged=1 +''', + 'switchport.network': '''[Match] +Name=switchport + +[Network] +LinkLocalAddressing=no +Bridge=br0 + +[BridgeVLAN] +VLAN=4000-4094 +PVID=0 +VLAN=13 +EgressUntagged=13 +'''}) + class TestConfigErrors(TestBase): @@ -797,28 +863,6 @@ def test_bridge_invalid_port_prio(self): 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 @@ -832,12 +876,23 @@ def test_bridge_invalid_vlan(self): def test_bridge_invalid_vlan_vid(self): err = self.generate('''network: version: 2 + renderer: NetworkManager 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_vlan_vid_networkd(self): + err = self.generate('''network: + version: 2 + renderer: networkd + bridges: + br0: + parameters: + vlans: [0]''', expect_fail=True) + self.assertIn("Error in network definition: malformed vlan '0': value cannot be defined as 0 for non-pvid", err) + def test_bridge_invalid_port_vlan_vid_to(self): err = self.generate('''network: version: 2 @@ -874,6 +929,34 @@ def test_bridge_invalid_vlan_vid_range(self): vlans: [100-1]''', expect_fail=True) self.assertIn("Error in network definition: malformed vlan vid range '100-1': 100 > 1!", err) + def test_bridge_invalid_vlan_default_pvid(self): + err = self.generate('''network: + version: 2 + bridges: + br0: + parameters: + vlan-default-pvid: 200-300''', expect_fail=True) + self.assertIn("Error in network definition: malformed value of vlan-default-pvid '200-300': \ +vlan-default-pvid can only be defined as a single port ID", err) + + def test_bridge_invalid_pvid_multiple(self): + err = self.generate('''network: + version: 2 + bridges: + br0: + parameters: + vlans: [99 pvid untagged, 1 untagged, 100 pvid]''', expect_fail=True) + self.assertIn("Error in network definition: malformed vlan pvid '100 pvid': only single pvid can be defined", err) + + def test_bridge_invalid_pvid_range(self): + err = self.generate('''network: + version: 2 + bridges: + br0: + parameters: + vlans: [10-42 pvid untagged, 1 untagged]''', expect_fail=True) + self.assertIn("Error in network definition: malformed vlan '10-42 pvid untagged': pvid cannot be defined as a range", err) + def test_bridge_port_vlan_add_missing_node(self): err = self.generate('''network: version: 2 @@ -888,3 +971,18 @@ def test_bridge_port_vlan_add_missing_node(self): port-vlans: eth0: [1]''', expect_fail=True) self.assertIn("Error in network definition: br0: interface 'eth0' is not defined", err) + + def test_bridge_port_invalid_vlan_default_pvid_none(self): + err = self.generate('''network: + version: 2 + renderer: NetworkManager + ethernets: + eno1: {} + switchport: {} + bridges: + br0: + interfaces: [eno1, switchport] + parameters: + vlan-filtering: true + vlan-default-pvid: none''', expect_fail=True) + self.assertIn("ERROR: vlan-default-pvid cannot be set to 'none' if NetworkManager is used", err)