Merge pull request #20375 from yuwata/network-bridge-vlan-issue-20373

network: fix bridge and openvswitch issues

src/network/networkd-setlink.c

@@ -95,9 +95,16 @@ static int set_link_handler_internal(
         return 1;
 
 on_error:
-        if (op == SET_LINK_FLAGS) {
+        switch (op) {
+        case SET_LINK_FLAGS:
                 assert(link->set_flags_messages > 0);
                 link->set_flags_messages--;
+                break;
+        case SET_LINK_MASTER:
+                link->master_set = true;
+                break;
+        default:
+                break;
         }
 
         return 0;
@@ -106,7 +113,7 @@ on_error:
 static int link_set_addrgen_mode_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
         int r;
 
-        r = set_link_handler_internal(rtnl, m, link, SET_LINK_ADDRESS_GENERATION_MODE, true, NULL);
+        r = set_link_handler_internal(rtnl, m, link, SET_LINK_ADDRESS_GENERATION_MODE, /* ignore = */ true, NULL);
         if (r <= 0)
                 return r;
 
@@ -120,31 +127,31 @@ static int link_set_addrgen_mode_handler(sd_netlink *rtnl, sd_netlink_message *m
 }
 
 static int link_set_bond_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
-        return set_link_handler_internal(rtnl, m, link, SET_LINK_BOND, false, NULL);
+        return set_link_handler_internal(rtnl, m, link, SET_LINK_BOND, /* ignore = */ false, NULL);
 }
 
 static int link_set_bridge_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
-        return set_link_handler_internal(rtnl, m, link, SET_LINK_BRIDGE, false, NULL);
+        return set_link_handler_internal(rtnl, m, link, SET_LINK_BRIDGE, /* ignore = */ true, NULL);
 }
 
 static int link_set_bridge_vlan_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
-        return set_link_handler_internal(rtnl, m, link, SET_LINK_BRIDGE_VLAN, false, NULL);
+        return set_link_handler_internal(rtnl, m, link, SET_LINK_BRIDGE_VLAN, /* ignore = */ false, NULL);
 }
 
 static int link_set_can_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
-        return set_link_handler_internal(rtnl, m, link, SET_LINK_CAN, false, NULL);
+        return set_link_handler_internal(rtnl, m, link, SET_LINK_CAN, /* ignore = */ false, NULL);
 }
 
 static int link_set_flags_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
-        return set_link_handler_internal(rtnl, m, link, SET_LINK_FLAGS, false, get_link_update_flag_handler);
+        return set_link_handler_internal(rtnl, m, link, SET_LINK_FLAGS, /* ignore = */ false, get_link_update_flag_handler);
 }
 
 static int link_set_group_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
-        return set_link_handler_internal(rtnl, m, link, SET_LINK_GROUP, false, NULL);
+        return set_link_handler_internal(rtnl, m, link, SET_LINK_GROUP, /* ignore = */ false, NULL);
 }
 
 static int link_set_mac_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
-        return set_link_handler_internal(rtnl, m, link, SET_LINK_MAC, true, get_link_default_handler);
+        return set_link_handler_internal(rtnl, m, link, SET_LINK_MAC, /* ignore = */ true, get_link_default_handler);
 }
 
 static int link_set_mac_allow_retry_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
@@ -180,13 +187,18 @@ static int link_set_mac_allow_retry_handler(sd_netlink *rtnl, sd_netlink_message
 }
 
 static int link_set_master_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
-        return set_link_handler_internal(rtnl, m, link, SET_LINK_MASTER, false, get_link_master_handler);
+        return set_link_handler_internal(rtnl, m, link, SET_LINK_MASTER, /* ignore = */ false, get_link_master_handler);
+}
+
+static int link_unset_master_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
+        /* Some devices do not support setting master ifindex. Let's ignore error on unsetting master ifindex. */
+        return set_link_handler_internal(rtnl, m, link, SET_LINK_MASTER, /* ignore = */ true, get_link_master_handler);
 }
 
 static int link_set_mtu_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
         int r;
 
-        r = set_link_handler_internal(rtnl, m, link, SET_LINK_MTU, true, get_link_default_handler);
+        r = set_link_handler_internal(rtnl, m, link, SET_LINK_MTU, /* ignore = */ true, get_link_default_handler);
         if (r <= 0)
                 return r;
 
@@ -745,10 +757,14 @@ int link_request_to_set_mac(Link *link, bool allow_retry) {
 
 int link_request_to_set_master(Link *link) {
         assert(link);
+        assert(link->network);
 
         link->master_set = false;
 
+        if (link->network->batadv || link->network->bond || link->network->bridge || link->network->vrf)
                 return link_request_set_link(link, SET_LINK_MASTER, link_set_master_handler, NULL);
+        else
+                return link_request_set_link(link, SET_LINK_MASTER, link_unset_master_handler, NULL);
 }
 
 int link_request_to_set_mtu(Link *link, uint32_t mtu) {

test/test-network/conf/26-bridge-issue-20373.netdev

@@ -0,0 +1,12 @@
+[NetDev]
+Name=bridge99
+Kind=bridge
+
+[Bridge]
+MulticastQuerier=yes
+MulticastSnooping=yes
+Priority=10
+STP=yes
+ForwardDelaySec=5
+MulticastIGMPVersion=2
+VLANFiltering=yes

test/test-network/conf/26-bridge-vlan-master-issue-20373.network

@@ -0,0 +1,20 @@
+[Match]
+Name=bridge99
+
+[Network]
+VLAN=vlan99
+IPForward=yes
+ConfigureWithoutCarrier=yes
+LLDP=yes
+IPv6AcceptRA=false
+
+[Bridge]
+Learning=yes
+MulticastRouter=no
+UseBPDU=yes
+
+[BridgeVLAN]
+VLAN=100
+
+[BridgeVLAN]
+VLAN=600

test/test-network/conf/26-bridge-vlan-slave-issue-20373.network

@@ -0,0 +1,29 @@
+[Match]
+Name=test1
+
+[Network]
+IPv6AcceptRA=no
+IPForward=yes
+Bridge=bridge99
+LinkLocalAddressing=no
+EmitLLDP=nearest-bridge
+LLDP=yes
+
+[Link]
+RequiredForOnline=no
+
+[Bridge]
+Learning=yes
+MulticastRouter=query
+UseBPDU=yes
+
+[BridgeVLAN]
+VLAN=100
+EgressUntagged=100
+PVID=100
+
+[BridgeVLAN]
+VLAN=560
+
+[BridgeVLAN]
+VLAN=600

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

@@ -3425,21 +3425,29 @@ class NetworkdBridgeTests(unittest.TestCase, Utilities):
     links = [
         'bridge99',
         'dummy98',
-        'test1']
+        'test1',
+        'vlan99',
+    ]
 
     units = [
         '11-dummy.netdev',
         '12-dummy.netdev',
+        '21-vlan.netdev',
+        '21-vlan.network',
         '26-bridge.netdev',
         '26-bridge-configure-without-carrier.network',
+        '26-bridge-issue-20373.netdev',
         '26-bridge-mdb-master.network',
         '26-bridge-mdb-slave.network',
         '26-bridge-slave-interface-1.network',
         '26-bridge-slave-interface-2.network',
+        '26-bridge-vlan-master-issue-20373.network',
         '26-bridge-vlan-master.network',
+        '26-bridge-vlan-slave-issue-20373.network',
         '26-bridge-vlan-slave.network',
         'bridge99-ignore-carrier-loss.network',
-        'bridge99.network']
+        'bridge99.network'
+    ]
 
     routing_policy_rule_tables = ['100']
 
@@ -3474,6 +3482,25 @@ class NetworkdBridgeTests(unittest.TestCase, Utilities):
             self.assertRegex(output, f'{i}')
         self.assertNotRegex(output, '4095')
 
+    def test_bridge_vlan_issue_20373(self):
+        copy_unit_to_networkd_unit_path('11-dummy.netdev', '26-bridge-vlan-slave-issue-20373.network',
+                                        '26-bridge-issue-20373.netdev', '26-bridge-vlan-master-issue-20373.network',
+                                        '21-vlan.netdev', '21-vlan.network')
+        start_networkd()
+        self.wait_online(['test1:enslaved', 'bridge99:degraded', 'vlan99:routable'])
+
+        output = check_output('bridge vlan show dev test1')
+        print(output)
+        self.assertIn('100 PVID Egress Untagged', output)
+        self.assertIn('560', output)
+        self.assertIn('600', output)
+
+        output = check_output('bridge vlan show dev bridge99')
+        print(output)
+        self.assertIn('1 PVID Egress Untagged', output)
+        self.assertIn('100', output)
+        self.assertIn('600', output)
+
     def test_bridge_mdb(self):
         copy_unit_to_networkd_unit_path('11-dummy.netdev', '26-bridge-mdb-slave.network',
                                         '26-bridge.netdev', '26-bridge-mdb-master.network')