From a55100e66c80a8aa9df996f67e8673439013a595 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 17 Sep 2020 14:09:17 +0900 Subject: [PATCH 1/3] network: allow to configure bridge MDB entries on bridge master --- src/network/networkd-mdb.c | 50 +++++++++++++++++++++++++--------- src/network/networkd-network.c | 8 ------ 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/network/networkd-mdb.c b/src/network/networkd-mdb.c index f27d4da254..7b0c10a4af 100644 --- a/src/network/networkd-mdb.c +++ b/src/network/networkd-mdb.c @@ -117,11 +117,23 @@ static int set_mdb_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) return 1; } +static int link_get_bridge_master_ifindex(Link *link) { + assert(link); + + if (link->network && link->network->bridge) + return link->network->bridge->ifindex; + + if (streq_ptr(link->kind, "bridge")) + return link->ifindex; + + return 0; +} + /* send a request to the kernel to add an MDB entry */ static int mdb_entry_configure(Link *link, MdbEntry *mdb_entry) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL; struct br_mdb_entry entry; - int r; + int master, r; assert(link); assert(link->network); @@ -136,14 +148,20 @@ static int mdb_entry_configure(Link *link, MdbEntry *mdb_entry) { strna(a), mdb_entry->vlan_id); } + master = link_get_bridge_master_ifindex(link); + if (master <= 0) + return log_link_error_errno(link, SYNTHETIC_ERRNO(EINVAL), "Invalid bridge master ifindex %i", master); + entry = (struct br_mdb_entry) { - .state = MDB_PERMANENT, + /* If MDB entry is added on bridge master, then the state must be MDB_TEMPORARY. + * See br_mdb_add_group() in net/bridge/br_mdb.c of kernel. */ + .state = master == link->ifindex ? MDB_TEMPORARY : MDB_PERMANENT, .ifindex = link->ifindex, .vid = mdb_entry->vlan_id, }; /* create new RTM message */ - r = sd_rtnl_message_new_mdb(link->manager->rtnl, &req, RTM_NEWMDB, link->network->bridge->ifindex); + r = sd_rtnl_message_new_mdb(link->manager->rtnl, &req, RTM_NEWMDB, master); if (r < 0) return log_link_error_errno(link, r, "Could not create RTM_NEWMDB message: %m"); @@ -178,7 +196,6 @@ static int mdb_entry_configure(Link *link, MdbEntry *mdb_entry) { int link_set_bridge_mdb(Link *link) { MdbEntry *mdb_entry; - Link *master; int r; assert(link); @@ -188,20 +205,26 @@ int link_set_bridge_mdb(Link *link) { if (!link->network) return 0; - if (!link->network->bridge) { - link->bridge_mdb_configured = true; - return 0; - } + if (LIST_IS_EMPTY(link->network->static_mdb_entries)) + goto finish; if (!link_has_carrier(link)) return log_link_debug(link, "Link does not have carrier yet, setting MDB entries later."); - r = link_get(link->manager, link->network->bridge->ifindex, &master); - if (r < 0) - return log_link_error_errno(link, r, "Failed to get Link object for Bridge=%s", link->network->bridge->ifname); + if (link->network->bridge) { + Link *master; - if (!link_has_carrier(master)) - return log_link_debug(link, "Bridge interface %s does not have carrier yet, setting MDB entries later.", link->network->bridge->ifname); + r = link_get(link->manager, link->network->bridge->ifindex, &master); + if (r < 0) + return log_link_error_errno(link, r, "Failed to get Link object for Bridge=%s", link->network->bridge->ifname); + + if (!link_has_carrier(master)) + return log_link_debug(link, "Bridge interface %s does not have carrier yet, setting MDB entries later.", link->network->bridge->ifname); + + } else if (!streq_ptr(link->kind, "bridge")) { + log_link_warning(link, "Link is neither a bridge master nor a bridge port, ignoring [BridgeMDB] sections."); + goto finish; + } LIST_FOREACH(static_mdb_entries, mdb_entry, link->network->static_mdb_entries) { r = mdb_entry_configure(link, mdb_entry); @@ -211,6 +234,7 @@ int link_set_bridge_mdb(Link *link) { link->bridge_mdb_messages++; } +finish: if (link->bridge_mdb_messages == 0) { link->bridge_mdb_configured = true; link_check_ready(link); diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 36d01283c0..81876902f5 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -306,14 +306,6 @@ int network_verify(Network *network) { if (section_is_invalid(fdb->section)) fdb_entry_free(fdb); - if (!LIST_IS_EMPTY(network->static_mdb_entries) && !network->bridge) { - log_warning("%s: Cannot configure MDB entries on non-bridge port, ignoring [BridgeMDB] sections.", - network->filename); - - while ((mdb = network->static_mdb_entries)) - mdb_entry_free(mdb); - } - LIST_FOREACH_SAFE(static_mdb_entries, mdb, mdb_next, network->static_mdb_entries) if (mdb_entry_verify(mdb) < 0) mdb_entry_free(mdb); From 1797240104ba3472728ce7f52311a5db8969e0a4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 17 Sep 2020 18:02:09 +0900 Subject: [PATCH 2/3] network: old kernel may not support to configure bridge MDB entries on bridge master --- src/network/networkd-manager.h | 3 ++- src/network/networkd-mdb.c | 12 +++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-manager.h b/src/network/networkd-manager.h index f4b37bd6a9..255a0d965f 100644 --- a/src/network/networkd-manager.h +++ b/src/network/networkd-manager.h @@ -69,7 +69,8 @@ struct Manager { usec_t speed_meter_usec_new; usec_t speed_meter_usec_old; - bool dhcp4_prefix_root_cannot_set_table; + bool dhcp4_prefix_root_cannot_set_table:1; + bool bridge_mdb_on_master_not_supported:1; }; int manager_new(Manager **ret); diff --git a/src/network/networkd-mdb.c b/src/network/networkd-mdb.c index 7b0c10a4af..542ba75ad0 100644 --- a/src/network/networkd-mdb.c +++ b/src/network/networkd-mdb.c @@ -103,7 +103,13 @@ static int set_mdb_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) return 1; r = sd_netlink_message_get_errno(m); - if (r < 0 && r != -EEXIST) { + if (r == -EINVAL && streq_ptr(link->kind, "bridge") && (!link->network || !link->network->bridge)) { + /* To configure bridge MDB entries on bridge master, 1bc844ee0faa1b92e3ede00bdd948021c78d7088 (v5.4) is required. */ + if (!link->manager->bridge_mdb_on_master_not_supported) { + log_link_warning_errno(link, r, "Kernel seems not to support configuring bridge MDB entries on bridge master, ignoring: %m"); + link->manager->bridge_mdb_on_master_not_supported = true; + } + } else if (r < 0 && r != -EEXIST) { log_link_message_warning_errno(link, m, r, "Could not add MDB entry"); link_enter_failed(link); return 1; @@ -199,6 +205,7 @@ int link_set_bridge_mdb(Link *link) { int r; assert(link); + assert(link->manager); link->bridge_mdb_configured = false; @@ -224,6 +231,9 @@ int link_set_bridge_mdb(Link *link) { } else if (!streq_ptr(link->kind, "bridge")) { log_link_warning(link, "Link is neither a bridge master nor a bridge port, ignoring [BridgeMDB] sections."); goto finish; + } else if (link->manager->bridge_mdb_on_master_not_supported) { + log_link_debug(link, "Kernel seems not to support configuring bridge MDB entries on bridge master, ignoring [BridgeMDB] sections."); + goto finish; } LIST_FOREACH(static_mdb_entries, mdb_entry, link->network->static_mdb_entries) { From 9f773037a05a086db00350c52808dae35bf7a97f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 17 Sep 2020 14:32:03 +0900 Subject: [PATCH 3/3] test-network: add test for bridge MDB entries on bridge master --- test/test-network/conf/26-bridge-mdb-master.network | 8 ++++++++ test/test-network/systemd-networkd-tests.py | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/test/test-network/conf/26-bridge-mdb-master.network b/test/test-network/conf/26-bridge-mdb-master.network index b88ea397c4..3fa1737f81 100644 --- a/test/test-network/conf/26-bridge-mdb-master.network +++ b/test/test-network/conf/26-bridge-mdb-master.network @@ -3,3 +3,11 @@ Name=bridge99 [Network] IPv6AcceptRA=false + +[BridgeMDB] +VLANId=4066 +MulticastGroupAddress=ff02:aaaa:fee5:0000:0000:0000:0001:0004 + +[BridgeMDB] +VLANId=4067 +MulticastGroupAddress=224.0.1.2 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 12f91bf3c9..f2fd86aa4a 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -2923,6 +2923,11 @@ class NetworkdBridgeTests(unittest.TestCase, Utilities): self.assertRegex(output, 'dev bridge99 port test1 grp ff02:aaaa:fee5::1:3 permanent *vid 4064') self.assertRegex(output, 'dev bridge99 port test1 grp 224.0.1.1 permanent *vid 4065') + # Old kernel may not support bridge MDB entries on bridge master + if call('bridge mdb add dev bridge99 port bridge99 grp 224.0.1.3 temp vid 4068', stderr=subprocess.DEVNULL) == 0: + self.assertRegex(output, 'dev bridge99 port bridge99 grp ff02:aaaa:fee5::1:4 temp *vid 4066') + self.assertRegex(output, 'dev bridge99 port bridge99 grp 224.0.1.2 temp *vid 4067') + def test_bridge_property(self): copy_unit_to_networkd_unit_path('11-dummy.netdev', '12-dummy.netdev', '26-bridge.netdev', '26-bridge-slave-interface-1.network', '26-bridge-slave-interface-2.network',