[vlanmgrd]: Fixing an issue causing mismatch between MAC and link-local IPv6 addresses of VLAN and Bridge interfaces

cfgmgr/vlanmgr.cpp

@@ -83,7 +83,11 @@ VlanMgr::VlanMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c
     //               /sbin/ip link del dummy 2>/dev/null;
     //               /sbin/ip link add dummy type dummy &&
     //               /sbin/ip link set dummy master Bridge &&
-    //               /sbin/ip link set dummy up"
+    //               /sbin/ip link set dummy up;
+    //               /sbin/ip link set Bridge down &&
+    //               /sbin/ip link set Bridge up"
+    // Note: We shutdown and start-up the Bridge at the end to ensure that its
+    //       link-local IPv6 address matches its MAC address.
 
     const std::string cmds = std::string("")
       + BASH_CMD + " -c \""
@@ -95,7 +99,9 @@ VlanMgr::VlanMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c
       + IP_CMD + " link del dev dummy 2>/dev/null; "
       + IP_CMD + " link add dummy type dummy && "
       + IP_CMD + " link set dummy master " + DOT1Q_BRIDGE_NAME + " && "
-      + IP_CMD + " link set dummy up" + "\"";
+      + IP_CMD + " link set dummy up; "
+      + IP_CMD + " link set " + DOT1Q_BRIDGE_NAME + " down && "
+      + IP_CMD + " link set " + DOT1Q_BRIDGE_NAME + " up\"";
 
     std::string res;
     EXEC_WITH_ERROR_THROW(cmds, res);
@@ -193,15 +199,34 @@ bool VlanMgr::setHostVlanMac(int vlan_id, const string &mac)
 {
     SWSS_LOG_ENTER();
 
+    std::string res;
+
+    /*
+     * Bring down the bridge before changing MAC addresses of the bridge and the VLAN interface.
+     * This is done so that the IPv6 link-local addresses of the bridge and the VLAN interface
+     * are updated after MAC change.
+     * /sbin/ip link set Bridge down
+     */
+    string bridge_down(IP_CMD " link set " DOT1Q_BRIDGE_NAME " down");
+    EXEC_WITH_ERROR_THROW(bridge_down, res);
+
     // The command should be generated as:
-    // /sbin/ip link set Vlan{{vlan_id}} address {{mac}}
+    // /sbin/ip link set Vlan{{vlan_id}} address {{mac}} &&
+    // /sbin/ip link set Bridge address {{mac}}
     ostringstream cmds;
     cmds << IP_CMD " link set " VLAN_PREFIX + std::to_string(vlan_id) + " address " << shellquote(mac) << " && "
             IP_CMD " link set " DOT1Q_BRIDGE_NAME " address " << shellquote(mac);
-
-    std::string res;
+    res.clear();
     EXEC_WITH_ERROR_THROW(cmds.str(), res);
 
+    /*
+     * Start up the bridge again.
+     * /sbin/ip link set Bridge up
+     */
+    string bridge_up(IP_CMD " link set " DOT1Q_BRIDGE_NAME " up");
+    res.clear();
+    EXEC_WITH_ERROR_THROW(bridge_up, res);
+
     return true;
 }
 

tests/conftest.py

@@ -1109,6 +1109,28 @@ def set_interface_status(self, interface, admin_status):
         tbl.set(interface, fvs)
         time.sleep(1)
 
+    def get_interface_oper_status(self, interface):
+        _, output = self.runcmd(f"ip --brief address show {interface}")
+        state = output.split()[1]
+        return state
+
+    def get_interface_link_local_ipv6(self, interface, subnet=False):
+        """
+        If subnet is True, the returned address will include the subnet length (e.g., fe80::aa:bbff:fecc:ddee/64)
+        """
+        _, output = self.runcmd(f"ip --brief address show {interface}")
+        ipv6 = output.split()[2]
+        if not subnet:
+            slash = ipv6.find('/')
+            if slash > 0:
+                ipv6 = ipv6[0:slash]
+        return ipv6
+
+    def get_interface_mac(self, interface):
+        _, output = self.runcmd(f"ip --brief link show {interface}")
+        mac = output.split()[2]
+        return mac
+
     # deps: acl, fdb_update, fdb, mirror_port_erspan, vlan, sub port intf
     def add_ip_address(self, interface, ip, vrf_name=None):
         if interface.startswith("PortChannel"):

tests/test_vlan.py

@@ -1,9 +1,25 @@
 import distro
 import pytest
+import ipaddress
+import time
 
 from distutils.version import StrictVersion
 from dvslib.dvs_common import PollingConfig, wait_for_result
 
+
+def mac_to_link_local_ipv6(mac):
+    mac_bytes = mac.split(':')
+    mac_bytes = mac_bytes[:3] + ["ff", "fe"] + mac_bytes[3:]
+    second_digit = int(mac_bytes[0][1], 16)
+    second_digit ^= 0x2  # Reverse the second bit from right
+    mac_bytes[0] = mac_bytes[0][0] + format(second_digit, "x")
+    ipv6 = ["fe80:"]
+    for i in range(0, 7, 2):
+        ipv6 += [":", mac_bytes[i], mac_bytes[i + 1]]
+    ipv6 = "".join(ipv6)
+    return str(ipaddress.IPv6Address(ipv6))  # Conversion to IPv6Address is done to compress ipv6.
+
+
 @pytest.mark.usefixtures("testlog")
 @pytest.mark.usefixtures('dvs_vlan_manager')
 @pytest.mark.usefixtures('dvs_lag_manager')
@@ -586,6 +602,49 @@ def test_VlanMemberLinkDown(self, dvs):
         self.dvs_vlan.remove_vlan(vlan)
         self.dvs_vlan.get_and_verify_vlan_ids(0)
 
+    def test_MacMatchesLinkLocalIPv6(self, dvs):
+        """
+        Checks whether the MAC addresses assigned to the Bridge, dummy, Vlan1000 and Ethernet4
+        interfaces match their corresponding interface's link-local IPv6 address.
+        """
+        dvs.setup_db()
+
+        vlan_id = "1000"
+        vlan_member = "Ethernet4"
+        vlan_interface = f"Vlan{vlan_id}"
+
+        assert dvs.get_interface_oper_status("Bridge") == "UP"
+        assert dvs.get_interface_oper_status("dummy") != "DOWN"
+
+        bridge_mac = dvs.get_interface_mac("Bridge")
+        assert mac_to_link_local_ipv6(bridge_mac) == dvs.get_interface_link_local_ipv6("Bridge")
+
+        dummy_mac = dvs.get_interface_mac("dummy")
+        assert mac_to_link_local_ipv6(dummy_mac) == dvs.get_interface_link_local_ipv6("dummy")
+
+        self.dvs_vlan.create_vlan(vlan_id)
+        time.sleep(1)
+        assert dvs.get_interface_oper_status(vlan_interface) == "UP"
+        # The MAC address of the Bridge is expected to have changed, so we need to check again.
+        bridge_mac = dvs.get_interface_mac("Bridge")
+        assert mac_to_link_local_ipv6(bridge_mac) == dvs.get_interface_link_local_ipv6("Bridge")
+        vlan_mac = dvs.get_interface_mac(vlan_interface)
+        assert mac_to_link_local_ipv6(vlan_mac) == dvs.get_interface_link_local_ipv6(vlan_interface)
+
+        self.dvs_vlan.create_vlan_member(vlan_interface, vlan_member)
+        time.sleep(1)
+        dvs.set_interface_status(vlan_member, "up")
+        assert dvs.get_interface_oper_status(vlan_member) != "DOWN"
+        member_mac = dvs.get_interface_mac(vlan_member)
+        assert mac_to_link_local_ipv6(member_mac) == dvs.get_interface_link_local_ipv6(vlan_member)
+
+        # Tear down
+        dvs.set_interface_status(vlan_member, "down")
+        self.dvs_vlan.remove_vlan_member(vlan_interface, vlan_member)
+        self.dvs_vlan.remove_vlan(vlan_interface)
+        time.sleep(1)
+
+
 # Add Dummy always-pass test at end as workaroud
 # for issue when Flaky fail on final test it invokes module tear-down before retrying
 def test_nonflaky_dummy():