refactor: use separate LCs for BFD traffic test (#15787)

Description of PR
Use upstream and downstream LCs for BFD traffic test so it can cover the "port channel down but BFD not down" issue.

Summary:
Fixes # (issue) Microsoft ADO 30112186

Approach
What is the motivation for this PR?
In bfd/test_bfd_traffic.py, we want to pick 2 LCs, where one connected to T1 (downstream LC) and other one connected to T3 (upstream LC), because if we pick only 1 LC or pick 2 LCs but both are downstream LCs (or upstream LCs), we will not cover the issue of "port channel down but BFD not down".

How did you do it?
Randomly pick one upstream LC and one downstream LC.

How did you verify/test it?
I ran the updated code and can confirm that it works as expected.

co-authorized by: [email protected]

tests/bfd/bfd_helpers.py

@@ -574,7 +574,8 @@ def send_packets_batch_from_ptf(
 
 
 def get_backend_interface_in_use_by_counter(
-    dut,
+    src_dut,
+    dst_dut,
     packet_count,
     version,
     src_asic_router_mac,
@@ -584,10 +585,13 @@ def get_backend_interface_in_use_by_counter(
     src_asic_index,
     dst_asic_index,
 ):
-    clear_interface_counters(dut)
+    with SafeThreadPoolExecutor(max_workers=8) as executor:
+        for dut in [src_dut, dst_dut]:
+            executor.submit(clear_interface_counters, dut)
+
     send_packets_batch_from_ptf(packet_count, version, src_asic_router_mac, ptfadapter, ptf_src_port, dst_neighbor_ip)
-    src_output = dut.show_and_parse("show int counters -n asic{} -d all".format(src_asic_index))
-    dst_output = dut.show_and_parse("show int counters -n asic{} -d all".format(dst_asic_index))
+    src_output = src_dut.show_and_parse("show int counters -n asic{} -d all".format(src_asic_index))
+    dst_output = dst_dut.show_and_parse("show int counters -n asic{} -d all".format(dst_asic_index))
     src_bp_iface = None
     for item in src_output:
         if "BP" in item.get("iface", "") and int(item.get("tx_ok", "0").replace(',', '')) >= packet_count:
@@ -604,24 +608,25 @@ def get_backend_interface_in_use_by_counter(
     return src_bp_iface, dst_bp_iface
 
 
-def get_src_dst_asic_next_hops(version, dut, src_asic, dst_asic, request, backend_port_channels):
-    src_asic_next_hops = extract_ip_addresses_for_backend_portchannels(dut, dst_asic, version, backend_port_channels)
-    assert len(src_asic_next_hops) != 0, "Source next hops are empty"
-    dst_asic_next_hops = extract_ip_addresses_for_backend_portchannels(dut, src_asic, version, backend_port_channels)
-    assert len(dst_asic_next_hops) != 0, "Destination next hops are empty"
-
-    dut_asic_static_routes = get_dut_asic_static_routes(version, dut)
-
-    # Picking a static route to delete its BFD session
-    src_prefix = selecting_route_to_delete(dut_asic_static_routes, src_asic_next_hops.values())
-    request.config.src_prefix = src_prefix
-    assert src_prefix is not None and src_prefix != "", "Source prefix not found"
+def get_src_dst_asic_next_hops(version, src_dut, src_asic, src_backend_port_channels, dst_dut, dst_asic,
+                               dst_backend_port_channels):
+    src_asic_next_hops = extract_ip_addresses_for_backend_portchannels(
+        dst_dut,
+        dst_asic,
+        version,
+        backend_port_channels=dst_backend_port_channels,
+    )
 
-    dst_prefix = selecting_route_to_delete(dut_asic_static_routes, dst_asic_next_hops.values())
-    request.config.dst_prefix = dst_prefix
-    assert dst_prefix is not None and dst_prefix != "", "Destination prefix not found"
+    assert len(src_asic_next_hops) != 0, "Source next hops are empty"
+    dst_asic_next_hops = extract_ip_addresses_for_backend_portchannels(
+        src_dut,
+        src_asic,
+        version,
+        backend_port_channels=src_backend_port_channels,
+    )
 
-    return src_asic_next_hops, dst_asic_next_hops, src_prefix, dst_prefix
+    assert len(dst_asic_next_hops) != 0, "Destination next hops are empty"
+    return src_asic_next_hops, dst_asic_next_hops
 
 
 def get_port_channel_by_member(backend_port_channels, member):
@@ -641,6 +646,7 @@ def toggle_port_channel_or_member(
 ):
     request.config.portchannels_on_dut = "dut"
     request.config.selected_portchannels = [target_to_toggle]
+    request.config.dut = dut
     request.config.asic = asic
 
     batch_control_interface_state(dut, asic, [target_to_toggle], action)
@@ -655,21 +661,22 @@ def assert_bp_iface_after_shutdown(
     dst_bp_iface_after_shutdown,
     src_asic_index,
     dst_asic_index,
-    dut_hostname,
+    src_dut_hostname,
+    dst_dut_hostname,
 ):
     if src_bp_iface_before_shutdown == src_bp_iface_after_shutdown:
         pytest.fail(
             "Source backend interface in use on asic{} of dut {} does not change after shutdown".format(
                 src_asic_index,
-                dut_hostname,
+                src_dut_hostname,
             )
         )
 
     if dst_bp_iface_before_shutdown == dst_bp_iface_after_shutdown:
         pytest.fail(
             "Destination backend interface in use on asic{} of dut {} does not change after shutdown".format(
                 dst_asic_index,
-                dut_hostname,
+                dst_dut_hostname,
             )
         )
 
@@ -681,21 +688,22 @@ def assert_port_channel_after_shutdown(
     dst_port_channel_after_shutdown,
     src_asic_index,
     dst_asic_index,
-    dut_hostname,
+    src_dut_hostname,
+    dst_dut_hostname,
 ):
     if src_port_channel_before_shutdown == src_port_channel_after_shutdown:
         pytest.fail(
             "Source port channel in use on asic{} of dut {} does not change after shutdown".format(
                 src_asic_index,
-                dut_hostname,
+                src_dut_hostname,
             )
         )
 
     if dst_port_channel_before_shutdown == dst_port_channel_after_shutdown:
         pytest.fail(
             "Destination port channel in use on asic{} of dut {} does not change after shutdown".format(
                 dst_asic_index,
-                dut_hostname,
+                dst_dut_hostname,
             )
         )
 
@@ -715,8 +723,10 @@ def wait_until_given_bfd_down(next_hops, port_channel, asic_index, dut):
 
 
 def assert_traffic_switching(
-    dut,
-    backend_port_channels,
+    src_dut,
+    dst_dut,
+    src_backend_port_channels,
+    dst_backend_port_channels,
     src_asic_index,
     src_bp_iface_before_shutdown,
     src_bp_iface_after_shutdown,
@@ -733,16 +743,17 @@ def assert_traffic_switching(
         dst_bp_iface_after_shutdown,
         src_asic_index,
         dst_asic_index,
-        dut.hostname,
+        src_dut.hostname,
+        dst_dut.hostname,
     )
 
     src_port_channel_after_shutdown = get_port_channel_by_member(
-        backend_port_channels,
+        src_backend_port_channels,
         src_bp_iface_after_shutdown,
     )
 
     dst_port_channel_after_shutdown = get_port_channel_by_member(
-        backend_port_channels,
+        dst_backend_port_channels,
         dst_bp_iface_after_shutdown,
     )
 
@@ -753,5 +764,22 @@ def assert_traffic_switching(
         dst_port_channel_after_shutdown,
         src_asic_index,
         dst_asic_index,
-        dut.hostname,
+        src_dut.hostname,
+        dst_dut.hostname,
     )
+
+
+def get_upstream_and_downstream_dut_pool(frontend_nodes):
+    upstream_dut_pool = []
+    downstream_dut_pool = []
+    for node in frontend_nodes:
+        bgp_neighbors = node.get_bgp_neighbors()
+        for neighbor_info in bgp_neighbors.values():
+            if "t3" in neighbor_info["description"].lower():
+                upstream_dut_pool.append(node)
+                break
+            elif "t1" in neighbor_info["description"].lower():
+                downstream_dut_pool.append(node)
+                break
+
+    return upstream_dut_pool, downstream_dut_pool

tests/bfd/conftest.py

@@ -48,24 +48,25 @@ def bfd_cleanup_db(request, duthosts, enum_supervisor_dut_hostname):
     #         120, 4, 0, check_orch_cpu_utilization, dut, orch_cpu_threshold
     #     ), "Orch CPU utilization exceeds orch cpu threshold {} after finishing the test".format(orch_cpu_threshold)
 
-    logger.info("Verifying swss container status on RP")
     rp = duthosts[enum_supervisor_dut_hostname]
     container_status = True
     if hasattr(request.config, "rp_asic_ids"):
+        logger.info("Verifying swss container status on RP")
         for id in request.config.rp_asic_ids:
             docker_output = rp.shell(
                 "docker ps | grep swss{} | awk '{{print $NF}}'".format(id)
             )["stdout"]
             if len(docker_output) == 0:
                 container_status = False
+
     if not container_status:
-        config_reload(rp)
+        logger.error("swss container is not running on RP, so running config reload")
+        config_reload(rp, safe_reload=True)
 
     if hasattr(request.config, "src_dut") and hasattr(request.config, "dst_dut"):
         clear_bfd_configs(request.config.src_dut, request.config.src_asic.asic_index, request.config.src_prefix)
         clear_bfd_configs(request.config.dst_dut, request.config.dst_asic.asic_index, request.config.dst_prefix)
 
-    logger.info("Bringing up portchannels or respective members")
     portchannels_on_dut = None
     if hasattr(request.config, "portchannels_on_dut"):
         portchannels_on_dut = request.config.portchannels_on_dut
@@ -74,12 +75,10 @@ def bfd_cleanup_db(request, duthosts, enum_supervisor_dut_hostname):
         portchannels_on_dut = request.config.portchannels_on_dut
         selected_interfaces = request.config.selected_portchannel_members
     else:
-        logger.info(
-            "None of the portchannels are selected to flap. So skipping portchannel interface check"
-        )
         selected_interfaces = []
 
     if selected_interfaces:
+        logger.info("Bringing up portchannels or respective members")
         if portchannels_on_dut == "src":
             dut = request.config.src_dut
         elif portchannels_on_dut == "dst":
@@ -95,3 +94,7 @@ def bfd_cleanup_db(request, duthosts, enum_supervisor_dut_hostname):
             asic = request.config.asic
 
         ensure_interfaces_are_up(dut, asic, selected_interfaces)
+    else:
+        logger.info(
+            "None of the portchannels are selected to flap. So skipping portchannel interface check"
+        )