From 83a186a9ea55dc175e8c27c1f31836d98157e5af Mon Sep 17 00:00:00 2001 From: judyjoseph <53951155+judyjoseph@users.noreply.github.com> Date: Thu, 1 Sep 2022 09:54:10 -0700 Subject: [PATCH] Change the log messages in addKernelNeigh/Route from ERROR to INFO (#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. --- cfgmgr/nbrmgr.cpp | 12 +- tests/test_virtual_chassis.py | 262 ++++++++++++++++++---------------- 2 files changed, 147 insertions(+), 127 deletions(-) diff --git a/cfgmgr/nbrmgr.cpp b/cfgmgr/nbrmgr.cpp index d6d5f410e107..b2cc3df5c610 100644 --- a/cfgmgr/nbrmgr.cpp +++ b/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; } diff --git a/tests/test_virtual_chassis.py b/tests/test_virtual_chassis.py index 9f4d6ddedb92..c2fa85248303 100644 --- a/tests/test_virtual_chassis.py +++ b/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]