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)