Merge pull request #17085 from yuwata/network-configure-mdb-entries-on-bridge-master

network: allow to configure mdb entries on bridge master

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);

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;
@@ -117,11 +123,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 +154,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,30 +202,39 @@ 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);
+        assert(link->manager);
 
         link->bridge_mdb_configured = false;
 
         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;
+        } 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) {
                 r = mdb_entry_configure(link, mdb_entry);
@@ -211,6 +244,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);

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);

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

test/test-network/systemd-networkd-tests.py

@@ -2969,6 +2969,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',