Change the log messages in addKernelNeigh/Route from ERROR to INFO (sonic-net#2437)

* Change the log messages from ERROR to INFO.
* Update the test_chassis_system_neigh test to check the mac address change of a neighbor.

Author: judyjoseph

cfgmgr/nbrmgr.cpp

@@ -397,7 +397,7 @@ void NbrMgr::doStateSystemNeighTask(Consumer &consumer)
 
             if (!addKernelNeigh(nbr_odev, ip_address, mac_address))
             {
-                SWSS_LOG_ERROR("Neigh entry add on dev %s failed for '%s'", nbr_odev.c_str(), kfvKey(t).c_str());
+                SWSS_LOG_INFO("Neigh entry add on dev %s failed for '%s'", nbr_odev.c_str(), kfvKey(t).c_str());
                 // Delete neigh to take care of deletion of exiting nbr for mac change. This makes sure that
                 // re-try will be successful and route addtion (below) will be attempted and be successful
                 delKernelNeigh(nbr_odev, ip_address);
@@ -411,7 +411,7 @@ void NbrMgr::doStateSystemNeighTask(Consumer &consumer)
 
             if (!addKernelRoute(nbr_odev, ip_address))
             {
-                SWSS_LOG_ERROR("Route entry add on dev %s failed for '%s'", nbr_odev.c_str(), kfvKey(t).c_str());
+                SWSS_LOG_INFO("Route entry add on dev %s failed for '%s'", nbr_odev.c_str(), kfvKey(t).c_str());
                 delKernelNeigh(nbr_odev, ip_address);
                 // Delete route to take care of deletion of exiting route of nbr for mac change.
                 delKernelRoute(ip_address);
@@ -522,8 +522,8 @@ bool NbrMgr::addKernelRoute(string odev, IpAddress ip_addr)
 
     if(ret)
     {
-        /* Just log error and return */
-        SWSS_LOG_ERROR("Failed to add route for %s, error: %d", ip_str.c_str(), ret);
+        /* This failure the caller expects is due to mac move */
+        SWSS_LOG_INFO("Failed to add route for %s, error: %d", ip_str.c_str(), ret);
         return false;
     }
 
@@ -586,8 +586,8 @@ bool NbrMgr::addKernelNeigh(string odev, IpAddress ip_addr, MacAddress mac_addr)
 
     if(ret)
     {
-        /* Just log error and return */
-        SWSS_LOG_ERROR("Failed to add Nbr for %s, error: %d", ip_str.c_str(), ret);
+        /* This failure the caller expects is due to mac move */
+        SWSS_LOG_INFO("Failed to add Nbr for %s, error: %d", ip_str.c_str(), ret);
         return false;
     }
 

tests/test_virtual_chassis.py

@@ -1,6 +1,7 @@
 from swsscommon import swsscommon
 from dvslib.dvs_database import DVSDatabase
 import ast
+import time
 
 class TestVirtualChassis(object):
 
@@ -239,156 +240,175 @@ def test_chassis_system_neigh(self, vct):
         # Test neighbor on Ethernet4 since Ethernet0 is used as Inband port
         test_neigh_dev = "Ethernet4"
         test_neigh_ip = "10.8.104.3"
-        test_neigh_mac = "00:01:02:03:04:05"
 
-        dvss = vct.dvss
-        print("name {}".format(dvss.keys()))
-        for name in dvss.keys():
-            dvs = dvss[name]
-
-            config_db = dvs.get_config_db()
-            metatbl = config_db.get_entry("DEVICE_METADATA", "localhost")
-
-            cfg_switch_type = metatbl.get("switch_type")
-
-            # Neighbor record verifiation done in line card
-            if cfg_switch_type == "voq":
-                lc_switch_id = metatbl.get("switch_id")
-                assert lc_switch_id != "", "Got error in getting switch_id from CONFIG_DB DEVICE_METADATA"
-                if lc_switch_id == "0":
-
-                    # Add a static neighbor
-                    _, res = dvs.runcmd(['sh', "-c", "ip neigh show"])
-                    _, res = dvs.runcmd(['sh', "-c", f"ip neigh add {test_neigh_ip} lladdr {test_neigh_mac} dev {test_neigh_dev}"])
-                    assert res == "", "Error configuring static neigh"
+        # Grouping together the checks done during neighbor entry create into a function chassis_system_neigh_create()
+        # if action is "add" it creates a new neighbor entry in local asic
+        # if action is "change" it updates an existing neighbor entry with the mac_address
+        def chassis_system_neigh_create():
+            dvss = vct.dvss
+            print("name {}".format(dvss.keys()))
+            for name in dvss.keys():
+                dvs = dvss[name]
 
-                    asic_db = dvs.get_asic_db()
-                    asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEIGHBOR_ENTRY", 1)
-                    neighkeys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEIGHBOR_ENTRY")
-                    assert len(neighkeys), "No neigh entries in ASIC_DB"
-
-                    # Check for presence of the neighbor in ASIC_DB
-                    test_neigh = ""
-                    for nkey in neighkeys:
-                        ne = ast.literal_eval(nkey)
-                        if ne['ip'] == test_neigh_ip:
-                            test_neigh = nkey
-                            break
+                config_db = dvs.get_config_db()
+                metatbl = config_db.get_entry("DEVICE_METADATA", "localhost")
+
+                cfg_switch_type = metatbl.get("switch_type")
+
+                # Neighbor record verifiation done in line card
+                if cfg_switch_type == "voq":
+                    lc_switch_id = metatbl.get("switch_id")
+                    assert lc_switch_id != "", "Got error in getting switch_id from CONFIG_DB DEVICE_METADATA"
+                    if lc_switch_id == "0":
+
+                        # Add a static neighbor
+                        _, res = dvs.runcmd(['sh', "-c", "ip neigh show"])
+                        _, res = dvs.runcmd(['sh', "-c", f"ip neigh {action} {test_neigh_ip} lladdr {mac_address} dev {test_neigh_dev}"])
+                        assert res == "", "Error configuring static neigh"
+
+                        asic_db = dvs.get_asic_db()
+                        asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEIGHBOR_ENTRY", 1)
+                        neighkeys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEIGHBOR_ENTRY")
+                        assert len(neighkeys), "No neigh entries in ASIC_DB"
+
+                        # Check for presence of the neighbor in ASIC_DB
+                        test_neigh = ""
+                        for nkey in neighkeys:
+                            ne = ast.literal_eval(nkey)
+                            if ne['ip'] == test_neigh_ip:
+                                test_neigh = nkey
+                                break
 
-                    assert test_neigh != "", "Neigh not found in ASIC_DB"
+                        assert test_neigh != "", "Neigh not found in ASIC_DB"
 
-                    # Preserve test neigh asic db key for delete verification later
-                    test_neigh_asic_db_key = test_neigh
+                        # Preserve test neigh asic db key for delete verification later
+                        test_neigh_asic_db_key = test_neigh
 
-                    # Check for presence of encap index, retrieve and store it for sync verification
-                    test_neigh_entry = asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEIGHBOR_ENTRY", test_neigh)
-                    test_neigh_entry_attrs = asic_db.wait_for_fields("ASIC_STATE:SAI_OBJECT_TYPE_NEIGHBOR_ENTRY", test_neigh, ["SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX"])
-                    print(test_neigh)
-                    print(test_neigh_entry)
-                    print(test_neigh_entry_attrs)
-                    encap_index = test_neigh_entry_attrs["SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX"]
-                    assert encap_index != "" and encap_index != None, "VOQ encap index is not programmed in ASIC_DB"
+                        # Check for presence of encap index, retrieve and store it for sync verification
+                        test_neigh_entry = asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEIGHBOR_ENTRY", test_neigh)
+                        test_neigh_entry_attrs = asic_db.wait_for_fields("ASIC_STATE:SAI_OBJECT_TYPE_NEIGHBOR_ENTRY", test_neigh, ["SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX"])
+                        print(test_neigh)
+                        print(test_neigh_entry)
+                        print(test_neigh_entry_attrs)
+                        encap_index = test_neigh_entry_attrs["SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX"]
+                        assert encap_index != "" and encap_index != None, "VOQ encap index is not programmed in ASIC_DB"
 
-                    break
-
-        # Verify neighbor record syncing with encap index
-        for name in dvss.keys():
-            if name.startswith("supervisor"):
-                dvs = dvss[name]
-                chassis_app_db = DVSDatabase(swsscommon.CHASSIS_APP_DB, dvs.redis_chassis_sock)
-                chassis_app_db.wait_for_n_keys("SYSTEM_NEIGH", 1)
-                sysneighkeys = chassis_app_db.get_keys("SYSTEM_NEIGH")
-
-                print(sysneighkeys)
-                test_sysneigh = ""
-                for sysnk in sysneighkeys:
-                    sysnk_tok = sysnk.split("|")
-                    assert len(sysnk_tok) == 3, "Invalid system neigh key in chassis app db"
-                    if sysnk_tok[2] == test_neigh_ip:
-                        test_sysneigh = sysnk
                         break
 
-                assert test_sysneigh != "", "Neigh is not sync-ed to chassis app db"
+            # Verify neighbor record syncing with encap index
+            for name in dvss.keys():
+                if name.startswith("supervisor"):
+                    dvs = dvss[name]
+                    chassis_app_db = DVSDatabase(swsscommon.CHASSIS_APP_DB, dvs.redis_chassis_sock)
+                    chassis_app_db.wait_for_n_keys("SYSTEM_NEIGH", 1)
+                    sysneighkeys = chassis_app_db.get_keys("SYSTEM_NEIGH")
+
+                    print(sysneighkeys)
+                    test_sysneigh = ""
+                    for sysnk in sysneighkeys:
+                        sysnk_tok = sysnk.split("|")
+                        assert len(sysnk_tok) == 3, "Invalid system neigh key in chassis app db"
+                        if sysnk_tok[2] == test_neigh_ip:
+                            test_sysneigh = sysnk
+                            break
 
-                # Preserve test sys neigh chassis app db key for delete verification later
-                test_sysneigh_chassis_app_db_key = test_sysneigh
+                    assert test_sysneigh != "", "Neigh is not sync-ed to chassis app db"
 
-                test_sysneigh_entry = chassis_app_db.get_entry("SYSTEM_NEIGH", test_sysneigh)
-                sys_neigh_encap_index = test_sysneigh_entry.get("encap_index")
-                assert sys_neigh_encap_index != "", "System neigh in chassis app db does not have encap index"
+                    # Preserve test sys neigh chassis app db key for delete verification later
+                    test_sysneigh_chassis_app_db_key = test_sysneigh
 
-                assert encap_index == sys_neigh_encap_index, "Encap index not sync-ed correctly"
+                    test_sysneigh_entry = chassis_app_db.get_entry("SYSTEM_NEIGH", test_sysneigh)
+                    sys_neigh_encap_index = test_sysneigh_entry.get("encap_index")
+                    assert sys_neigh_encap_index != "", "System neigh in chassis app db does not have encap index"
 
-                break
+                    assert encap_index == sys_neigh_encap_index, "Encap index not sync-ed correctly"
 
-        # Verify programming of remote neighbor in asic db and programming of static route and static
-        # neigh in the kernel for the remote neighbor. The neighbor created in linecard 1  will be a 
-        # remote neighbor in other linecards. Verity existence of the test neighbor in  linecards other 
-        # than linecard 1
-        for name in dvss.keys():
-            dvs = dvss[name]
+                    break
 
-            config_db = dvs.get_config_db()
-            metatbl = config_db.get_entry("DEVICE_METADATA", "localhost")
+            # Add a delay for the programming of neighbor in remote LC
+            time.sleep(10)
 
-            cfg_switch_type = metatbl.get("switch_type")
+            # Verify programming of remote neighbor in asic db and programming of static route and static
+            # neigh in the kernel for the remote neighbor. The neighbor created in linecard 1  will be a
+            # remote neighbor in other linecards. Verity existence of the test neighbor in  linecards other
+            # than linecard 1
+            for name in dvss.keys():
+                dvs = dvss[name]
 
-            # Neighbor record verifiation done in line card
-            if cfg_switch_type == "voq":    
-                lc_switch_id = metatbl.get("switch_id")
-                assert lc_switch_id != "", "Got error in getting switch_id from CONFIG_DB DEVICE_METADATA"
-                if lc_switch_id != "0":
-                    # Linecard other than linecard 1
-                    asic_db = dvs.get_asic_db()
-                    asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEIGHBOR_ENTRY", 1)
-                    neighkeys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEIGHBOR_ENTRY")
-                    assert len(neighkeys), "No neigh entries in ASIC_DB"
-                    
-                    # Check for presence of the remote neighbor in ASIC_DB
-                    remote_neigh = ""
-                    for nkey in neighkeys:
-                        ne = ast.literal_eval(nkey)
-                        if ne['ip'] == test_neigh_ip:
-                            remote_neigh = nkey
-                            break
+                config_db = dvs.get_config_db()
+                metatbl = config_db.get_entry("DEVICE_METADATA", "localhost")
+
+                cfg_switch_type = metatbl.get("switch_type")
+
+                # Neighbor record verifiation done in line card
+                if cfg_switch_type == "voq":
+                    lc_switch_id = metatbl.get("switch_id")
+                    assert lc_switch_id != "", "Got error in getting switch_id from CONFIG_DB DEVICE_METADATA"
+                    if lc_switch_id != "0":
+                        # Linecard other than linecard 1
+                        asic_db = dvs.get_asic_db()
+                        asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEIGHBOR_ENTRY", 1)
+                        neighkeys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEIGHBOR_ENTRY")
+                        assert len(neighkeys), "No neigh entries in ASIC_DB"
+
+                        # Check for presence of the remote neighbor in ASIC_DB
+                        remote_neigh = ""
+                        for nkey in neighkeys:
+                            ne = ast.literal_eval(nkey)
+                            if ne['ip'] == test_neigh_ip:
+                                remote_neigh = nkey
+                                break
 
-                    assert remote_neigh != "", "Remote neigh not found in ASIC_DB"
+                        assert remote_neigh != "", "Remote neigh not found in ASIC_DB"
 
-                    # Preserve remote neigh asic db neigh key for delete verification later
-                    test_remote_neigh_asic_db_key = remote_neigh
+                        # Preserve remote neigh asic db neigh key for delete verification later
+                        test_remote_neigh_asic_db_key = remote_neigh
 
-                    # Check for kernel entries
+                        # Check for kernel entries
 
-                    _, output = dvs.runcmd("ip neigh show")
-                    assert f"{test_neigh_ip} dev {inband_port}" in output, "Kernel neigh not found for remote neighbor"
+                        _, output = dvs.runcmd("ip neigh show")
+                        assert f"{test_neigh_ip} dev {inband_port}" in output, "Kernel neigh not found for remote neighbor"
 
-                    _, output = dvs.runcmd("ip route show")
-                    assert f"{test_neigh_ip} dev {inband_port} scope link" in output, "Kernel route not found for remote neighbor"
+                        _, output = dvs.runcmd("ip route show")
+                        assert f"{test_neigh_ip} dev {inband_port} scope link" in output, "Kernel route not found for remote neighbor"
 
-                    # Check for ASIC_DB entries. 
+                        # Check for ASIC_DB entries.
 
-                    # Check for presence of encap index, retrieve and store it for sync verification
-                    remote_neigh_entry = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEIGHBOR_ENTRY", remote_neigh)
+                        # Check for presence of encap index, retrieve and store it for sync verification
+                        remote_neigh_entry = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEIGHBOR_ENTRY", remote_neigh)
 
-                    # Validate encap index
-                    remote_encap_index = remote_neigh_entry.get("SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX")
-                    assert remote_encap_index != "", "VOQ encap index is not programmed for remote neigh in ASIC_DB"
-                    assert remote_encap_index == encap_index, "Encap index of remote neigh mismatch with allocated encap index"
+                        # Validate encap index
+                        remote_encap_index = remote_neigh_entry.get("SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX")
+                        assert remote_encap_index != "", "VOQ encap index is not programmed for remote neigh in ASIC_DB"
+                        assert remote_encap_index == encap_index, "Encap index of remote neigh mismatch with allocated encap index"
 
-                    # Validate MAC
-                    mac = remote_neigh_entry.get("SAI_NEIGHBOR_ENTRY_ATTR_DST_MAC_ADDRESS")
-                    assert mac != "", "MAC address is not programmed for remote neigh in ASIC_DB"
-                    assert mac == test_neigh_mac, "Encap index of remote neigh mismatch with allocated encap index"
+                        # Validate MAC
+                        mac = remote_neigh_entry.get("SAI_NEIGHBOR_ENTRY_ATTR_DST_MAC_ADDRESS")
+                        assert mac != "", "MAC address is not programmed for remote neigh in ASIC_DB"
+                        assert mac == mac_address, "Encap index of remote neigh mismatch with allocated encap index"
 
-                    # Check for other mandatory attributes
-                    # For remote neighbors, is_local must be "false" 
-                    is_local = remote_neigh_entry.get("SAI_NEIGHBOR_ENTRY_ATTR_IS_LOCAL")
-                    assert is_local != "", "is_local attribute is not programmed for remote neigh in ASIC_DB"
-                    assert is_local == "false", "is_local attribute is true for remote neigh"
+                        # Check for other mandatory attributes
+                        # For remote neighbors, is_local must be "false"
+                        is_local = remote_neigh_entry.get("SAI_NEIGHBOR_ENTRY_ATTR_IS_LOCAL")
+                        assert is_local != "", "is_local attribute is not programmed for remote neigh in ASIC_DB"
+                        assert is_local == "false", "is_local attribute is true for remote neigh"
 
-                    break
+                        break
+
+            return test_neigh_asic_db_key, test_sysneigh_chassis_app_db_key, test_remote_neigh_asic_db_key
+
+        # First step is to add a new neighbor and check local/chassis_db/remote entry creation
+        mac_address = "00:01:02:03:04:77"
+        action = "add"
+        chassis_system_neigh_create()
+
+        # Second step to update the mac address and check local/chassis_db/remote entry creation
+        mac_address = "00:01:02:03:04:05"
+        action = "change"
+        test_neigh_asic_db_key, test_sysneigh_chassis_app_db_key, test_remote_neigh_asic_db_key = chassis_system_neigh_create()
 
         # Verify system neighbor delete and clearing
+        dvss = vct.dvss
         for name in dvss.keys():
             dvs = dvss[name]