[dualtor][active-active] Fix test_hash (sonic-net#8580)

Approach
What is the motivation for this PR?
Fix test_hash

How did you do it?
Fix three things:

1. On dualtor-aa testbed, test_hash could not distribute the traffic evenly across all the uplinks of both ToRs as the traffic is ECMPed twice(once by NiC, once by the ToR), let's run test_hash in active-standby mode on active-active dualtor testbeds as NiC ECMP is not within the test scope.
2. for hash factor ip proto, skip use ICMP proto type for ipv4 and ipv6 traffic as NiC always duplicates ICMP packets to both ToRs even in active-standby mode.
3. Fix the port_id to active dut index mapping.

How did you verify/test it?
Run over dualtor-aa testbed.

ansible/roles/test/files/ptftests/py3/hash_test.py

@@ -103,6 +103,7 @@ def setUp(self):
             'single_fib_for_duts', 'multiple-fib')
 
         self.ipver = self.test_params.get('ipver', 'ipv4')
+        self.is_active_active_dualtor = self.test_params.get("is_active_active_dualtor", False)
 
         # set the base mac here to make it persistent across calls of check_ip_route
         self.base_mac = self.dataplane.get_mac(
@@ -224,9 +225,16 @@ def _get_ip_proto(self, ipv6=False):
         # ip_proto 254 is experimental
         # MLNX ASIC can't forward ip_proto 254, BRCM is OK, skip for all for simplicity
         skip_protos = [2, 253, 4, 41, 60, 254]
+
+        if self.is_active_active_dualtor:
+            # Skip ICMP for active-active dualtor as it is duplicated to both ToRs
+            skip_protos.append(1)
+
         if ipv6:
             # Skip ip_proto 0 for IPv6
             skip_protos.append(0)
+            # Skip IPv6-ICMP for active-active dualtor as it is duplicated to both ToRs
+            skip_protos.append(58)
 
         while True:
             ip_proto = random.randint(0, 255)

tests/common/fixtures/ptfhost_utils.py

@@ -12,6 +12,7 @@
 from tests.common import constants
 from tests.common.helpers.assertions import pytest_assert as pt_assert
 from tests.common.helpers.dut_utils import check_link_status
+from tests.common.dualtor.dual_tor_common import ActiveActivePortID
 from tests.common.dualtor.dual_tor_utils import update_linkmgrd_probe_interval, recover_linkmgrd_probe_interval
 from tests.common.utilities import wait_until
 
@@ -510,9 +511,11 @@ def ptf_test_port_map_active_active(ptfhost, tbinfo, duthosts, mux_server_url, d
             active_dut_index = 0 if mux_status['active_side'] == 'upper_tor' else 1
             active_dut_map[str(mux_status['port_index'])] = [active_dut_index]
         if active_active_ports_mux_status:
+            port_id_to_dut_index = {ActiveActivePortID.UPPER_TOR: 0, ActiveActivePortID.LOWER_TOR: 1}
             for port_index, port_status in list(active_active_ports_mux_status.items()):
-                active_dut_map[str(port_index)] = [active_dut_index for active_dut_index in (0, 1)
-                                                   if port_status[active_dut_index]]
+                active_dut_map[str(port_index)] = [
+                    dut_index for port_id, dut_index in port_id_to_dut_index.items() if port_status[port_id]
+                ]
 
     disabled_ptf_ports = set()
     for ptf_map in list(tbinfo['topo']['ptf_map_disabled'].values()):

tests/fib/test_fib.py

@@ -15,6 +15,9 @@
 from tests.common.dualtor.mux_simulator_control import mux_server_url       # noqa F401
 from tests.common.dualtor.mux_simulator_control import toggle_all_simulator_ports_to_rand_selected_tor_m    # noqa F401
 from tests.common.dualtor.mux_simulator_control import toggle_all_simulator_ports_to_random_side            # noqa F401
+from tests.common.dualtor.dual_tor_utils import config_active_active_dualtor_active_standby                 # noqa F401
+from tests.common.dualtor.dual_tor_utils import validate_active_active_dualtor_setup                        # noqa F401
+from tests.common.dualtor.dual_tor_common import active_active_ports                                        # noqa F401
 from tests.common.utilities import is_ipv4_address
 
 from tests.common.fixtures.fib_utils import fib_info_files_per_function     # noqa F401
@@ -79,7 +82,8 @@ def test_basic_fib(duthosts, ptfhost, ipv4, ipv6, mtu,
                    updated_tbinfo, mux_server_url,                      # noqa F401
                    mux_status_from_nic_simulator,
                    ignore_ttl, single_fib_for_duts,                     # noqa F401
-                   duts_running_config_facts, duts_minigraph_facts):
+                   duts_running_config_facts, duts_minigraph_facts,
+                   validate_active_active_dualtor_setup):               # noqa F401
 
     if 'dualtor' in updated_tbinfo['topo']['name']:
         wait(30, 'Wait some time for mux active/standby state to be stable after toggled mux state')
@@ -287,14 +291,34 @@ def add_default_route_to_dut(duts_running_config_facts, duthosts, tbinfo):
         yield
 
 
+@pytest.fixture
+def setup_active_active_ports(
+    active_active_ports, rand_selected_dut, rand_unselected_dut,                        # noqa F811
+    config_active_active_dualtor_active_standby, validate_active_active_dualtor_setup   # noqa F811
+):
+    if active_active_ports:
+        # The traffic from active-active mux ports are ECMPed twice:first time on the NiC to
+        # choose the ToR, second time on the ToR to choose the uplinks. The NiC ECMP is not
+        # within the test scope, and we also cannot guarantee that the traffic is evenly
+        # distributed among all the uplinks. So let's configure the active-active mux ports
+        # to let them work in active-standby mode.
+        logger.info("Configuring {} as active".format(rand_selected_dut.hostname))
+        logger.info("Configuring {} as standby".format(rand_unselected_dut.hostname))
+        config_active_active_dualtor_active_standby(rand_selected_dut, rand_unselected_dut, active_active_ports)
+
+    return
+
+
 def test_hash(add_default_route_to_dut, duthosts, fib_info_files_per_function, setup_vlan,      # noqa F811
               hash_keys, ptfhost, ipver, toggle_all_simulator_ports_to_rand_selected_tor_m,     # noqa F811
               updated_tbinfo, mux_server_url, mux_status_from_nic_simulator, ignore_ttl,        # noqa F811
-              single_fib_for_duts, duts_running_config_facts, duts_minigraph_facts):            # noqa F811
+              single_fib_for_duts, duts_running_config_facts, duts_minigraph_facts,             # noqa F811
+              setup_active_active_ports, active_active_ports):                                  # noqa F811
 
     if 'dualtor' in updated_tbinfo['topo']['name']:
         wait(30, 'Wait some time for mux active/standby state to be stable after toggled mux state')
 
+    is_active_active_dualtor = bool(active_active_ports)
     switch_type = duthosts[0].facts.get('switch_type')
     timestamp = datetime.now().strftime('%Y-%m-%d-%H:%M:%S')
     log_file = "/tmp/hash_test.HashTest.{}.{}.log".format(ipver, timestamp)
@@ -310,19 +334,21 @@ def test_hash(add_default_route_to_dut, duthosts, fib_info_files_per_function, s
         "ptftests",
         "hash_test.HashTest",
         platform_dir="ptftests",
-        params={"fib_info_files": fib_info_files_per_function[:3],   # Test at most 3 DUTs
-                "ptf_test_port_map": ptf_test_port_map_active_active(
-                    ptfhost, updated_tbinfo, duthosts, mux_server_url,
-                    duts_running_config_facts, duts_minigraph_facts,
-                    mux_status_from_nic_simulator()
-        ),
+        params={
+            "fib_info_files": fib_info_files_per_function[:3],   # Test at most 3 DUTs
+            "ptf_test_port_map": ptf_test_port_map_active_active(
+                ptfhost, updated_tbinfo, duthosts, mux_server_url,
+                duts_running_config_facts, duts_minigraph_facts,
+                mux_status_from_nic_simulator()
+            ),
             "hash_keys": hash_keys,
             "src_ip_range": ",".join(src_ip_range),
             "dst_ip_range": ",".join(dst_ip_range),
             "vlan_ids": VLANIDS,
             "ignore_ttl": ignore_ttl,
             "single_fib_for_duts": single_fib_for_duts,
-            "switch_type": switch_type
+            "switch_type": switch_type,
+            "is_active_active_dualtor": is_active_active_dualtor
         },
         log_file=log_file,
         qlen=PTF_QLEN,
@@ -397,15 +423,15 @@ def test_ipinip_hash_negative(add_default_route_to_dut, duthosts, fib_info_files
                            ptfhost, tbinfo, duthosts, mux_server_url,
                            duts_running_config_facts, duts_minigraph_facts,
                            mux_status_from_nic_simulator()
-                        ),
-                       "hash_keys": hash_keys,
-                       "src_ip_range": ",".join(src_ip_range),
-                       "dst_ip_range": ",".join(dst_ip_range),
-                       "vlan_ids": VLANIDS,
-                       "ignore_ttl": ignore_ttl,
-                       "single_fib_for_duts": single_fib_for_duts,
-                       "ipver": ipver
-                       },
+               ),
+                   "hash_keys": hash_keys,
+                   "src_ip_range": ",".join(src_ip_range),
+                   "dst_ip_range": ",".join(dst_ip_range),
+                   "vlan_ids": VLANIDS,
+                   "ignore_ttl": ignore_ttl,
+                   "single_fib_for_duts": single_fib_for_duts,
+                   "ipver": ipver
+               },
                log_file=log_file,
                qlen=PTF_QLEN,
                socket_recv_size=16384)