Increase stability and fix test issues of loopback action test (#10424)

Increase stability and fix test issues of loopback action test:

1. Fix a typo in tests/iface_loopback_action/conftest.py
2. Check neighbor status after configuring it
3. Use teamd instead of bond in the server for the lag
4. Wait 2s for the counters polling before checking them
5. Validate the loopback action behavior before the reload/reboot test
6. Validate the interface status after the reload/reboot before checking the traffic

tests/common/devices/sonic.py

@@ -2346,6 +2346,17 @@ def get_sfp_type(self, portname):
         sfp_type = re.search(r'[QO]?SFP-?[\d\w]{0,3}', out["stdout_lines"][0]).group()
         return sfp_type
 
+    def get_counter_poll_status(self):
+        result_dict = {}
+        output = self.shell("counterpoll show")["stdout_lines"][2::]
+        for line in output:
+            counter_type, interval, status = re.split(r'\s\s+', line)
+            interval = int(re.search(r'\d+', interval).group(0))
+            result_dict[counter_type] = {}
+            result_dict[counter_type]['interval'] = interval
+            result_dict[counter_type]['status'] = status
+        return result_dict
+
 
 def assert_exit_non_zero(shell_output):
     if shell_output['rc'] != 0:

tests/iface_loopback_action/conftest.py

@@ -16,7 +16,7 @@ def pytest_addoption(parser):
     """
 
     parser.addoption(
-        "--rif_loppback_reboot_type",
+        "--rif_loopback_reboot_type",
         action="store",
         type=str,
         default="cold",

tests/iface_loopback_action/iface_loopback_action_helper.py

@@ -49,6 +49,8 @@ def generate_and_verify_traffic(duthost, ptfadapter, rif_interface, src_port_ind
     eth_dst = duthost.facts["router_mac"]
     eth_src = ptfadapter.dataplane.get_mac(0, src_port_index).decode('utf-8')
     duthost.shell("sudo ip neigh replace {} lladdr {} dev {}".format(ip_dst, eth_src, rif_interface))
+    pytest_assert(wait_until(60, 3, 0, check_neighbor, duthost, ip_dst, eth_src, rif_interface),
+                  "Failed to add neighbor for {}.".format(ip_dst))
     logger.info("Traffic info is: eth_dst- {}, eth_src- {}, ip_src- {}, ip_dst- {}, vlan_vid- {}".format(
         eth_dst, eth_src, ip_src, ip_dst, vlan_vid))
     pkt = testutils.simple_ip_packet(
@@ -77,6 +79,30 @@ def generate_and_verify_traffic(duthost, ptfadapter, rif_interface, src_port_ind
         testutils.verify_packet(ptfadapter, exp_pkt, src_port_index)
 
 
+def check_neighbor(duthost, ip_address, mac_address, interface):
+    """
+    Verify the static ip neighbor is configured successfully
+    :param duthost: DUT host object
+    :param ip_address: Neighbor ip address
+    :param mac_address: Neighbor mac address
+    :param interface: Neighbor interface
+    """
+    asic_db_output = duthost.shell("redis-cli -n 1 keys *NEIGHBOR_ENTRY* | grep {}".format(ip_address))['stdout_lines']
+    if len(asic_db_output) < 1:
+        logger.error('No neighbor entry or extra neighbor entries of {} in ASIC db.'.format(ip_address))
+        return False
+
+    asic_db_neighbor_entry = asic_db_output[0]
+    mac_in_asic_db = duthost.shell("redis-cli -n 1 hget '{}' '{}'".format(
+        asic_db_neighbor_entry, "SAI_NEIGHBOR_ENTRY_ATTR_DST_MAC_ADDRESS"))['stdout_lines']
+
+    if len(mac_in_asic_db) < 1 or mac_in_asic_db[0].upper() != mac_address.upper():
+        logger.error('The neighbor entry of {} in ASIC db is not correct.'.format(ip_address))
+        return False
+
+    return True
+
+
 def get_tested_up_ports(duthost, ptf_ifaces_map, count=10):
     """
     Get the specified number of up ports
@@ -435,8 +461,7 @@ def add_ptf_bond(ptfhost, port, bond_id, ip_addr):
     """
     try:
         bond_port = 'bond{}'.format(bond_id)
-        ptfhost.shell("ip link add {} type bond".format(bond_port))
-        ptfhost.shell("ip link set {} type bond miimon 100 mode 802.3ad".format(bond_port))
+        ptfhost.shell("teamd -t {} -d -c '{{\"runner\": {{\"name\": \"lacp\"}}}}'".format(bond_port))
         ptfhost.shell("ip link set {} down".format(port))
         ptfhost.shell("ip link set {} master {}".format(port, bond_port))
         ptfhost.shell("ip link set dev {} up".format(bond_port))
@@ -501,6 +526,19 @@ def clear_rif_counter(duthost):
     duthost.shell("sonic-clear rifcounters")
 
 
+def check_ip_interface_up(duthost, interfaces):
+    """
+    Check the ip interfaces are all up
+    :param duthost: DUT host object
+    :param interfaces: List of ip interfaces to check
+    """
+    output = duthost.shell("show ip interface")['stdout']
+    for interface in interfaces:
+        if not re.search("{}\\s.*up\\/up".format(interface), output):
+            return False
+    return True
+
+
 def show_loopback_action(duthost):
     """
     Get the loopback action for every rif interface
@@ -562,6 +600,9 @@ def verify_rif_tx_err_count(duthost, rif_interfaces, expect_counts):
     :param rif_interfaces: List of rif interface
     :param expect_counts: expected TX ERR for for every rif interface
     """
+    # Wait for the rif counters polling
+    counter_poll_rif_interval = duthost.get_counter_poll_status()['RIF_STAT']['interval']
+    time.sleep(counter_poll_rif_interval / 1000 + 1)
     rif_tx_err_map = get_rif_tx_err_count(duthost)
     for rif_interface, expected_count in zip(rif_interfaces, expect_counts):
         tx_err_count = int(rif_tx_err_map[rif_interface])

tests/iface_loopback_action/test_iface_loopback_action.py

@@ -11,7 +11,7 @@
 from .iface_loopback_action_helper import config_loopback_action
 from .iface_loopback_action_helper import clear_rif_counter
 from .iface_loopback_action_helper import verify_interface_loopback_action
-from .iface_loopback_action_helper import verify_rif_tx_err_count, is_rif_counters_ready
+from .iface_loopback_action_helper import verify_rif_tx_err_count, is_rif_counters_ready, check_ip_interface_up
 from .iface_loopback_action_helper import shutdown_rif_interfaces, startup_rif_interfaces
 from tests.common.platform.interface_utils import check_interface_status_of_up_ports
 
@@ -88,11 +88,21 @@ def test_loopback_action_reload(request, duthost, localhost, ptfadapter, ports_c
     count_list = [NUM_OF_TOTAL_PACKETS if action == ACTION_DROP else 0 for action in action_list]
     with allure.step("Configure the loopback action for {} to {}".format(rif_interfaces, action_list)):
         config_loopback_action(duthost, rif_interfaces, action_list)
+    with allure.step("Verify the loopback action is correct before config reload"):
+        with allure.step("Check the looback action is configured correctly with cli command"):
+            verify_interface_loopback_action(duthost, rif_interfaces, action_list)
+        with allure.step("Check the loopback traffic"):
+            with allure.step("Clear the rif counter"):
+                clear_rif_counter(duthost)
+            with allure.step("Check the traffic can be received or dropped as expected"):
+                verify_traffic(duthost, ptfadapter, rif_interfaces, ports_configuration, action_list)
+            with allure.step("Check the TX_ERR in rif counter statistic will increase or not as expected"):
+                verify_rif_tx_err_count(duthost, rif_interfaces, count_list)
     with allure.step("Save configuration"):
         duthost.shell("config save -y")
     with allure.step("System reload"):
 
-        reboot_type = request.config.getoption("--rif_loppback_reboot_type")
+        reboot_type = request.config.getoption("--rif_loopback_reboot_type")
         if reboot_type == "random":
             reload_types = ["reload", "cold", "fast", "warm"]
             reboot_type = random.choice(reload_types)
@@ -110,6 +120,9 @@ def test_loopback_action_reload(request, duthost, localhost, ptfadapter, ports_c
         with allure.step("Check the looback action is configured correctly with cli command"):
             verify_interface_loopback_action(duthost, rif_interfaces, action_list)
         with allure.step("Check the loopback traffic"):
+            with allure.step("Check all ip interfaces are up"):
+                pytest_assert(wait_until(20, 5, 0, check_ip_interface_up, duthost, rif_interfaces),
+                              "Not all ip interfaces are up.")
             with allure.step("Clear the rif counter"):
                 clear_rif_counter(duthost)
             with allure.step("Check the traffic can be received or dropped as expected"):