[Thinkit] Update speed, Move tests to use mirroring function, gNMI po…

…… …rt breakout tests, Disable sFlow by default in QoS punt test case, Do not restore entire static config passed in & Make it clear when L3AdmitTest is restricting packets based on port IDs. (sonic-net#961)

* [Thinkit] Add provision to pass in ingress and egress ports to tests.

* [Comb] Move utilities to common libraries, Pull logic for buffer configuration into helper function, Migrate some users to ReadStreamChannelResponsesAndFinish, Toggle port speed before test, Adding packet, Add tolerance for Queue stats check, Renamed Packet to PacketAtPort, L3 admit tests should choose only ports that are UP & Update L3 Admit Test to not rely on a custom GNMI Config.

* [Thinkit] Re-mask packet-dropping bug, Added helper function which returns an `EK_PHYSICAL_PORT` name given an `EK_PORT` name, Add ingress and egress port parameters to QoS test, Enable modify in smoke test, gNMI port breakout tests, BERT test, Ensure that P4RT port IDs are properly reflected in the State path before considering switch configured, Ensure that P4RT port IDs in testbed are mirrored & Remove duplicate code from switch_test_setup_helpers test.

* [Thinkit] Update speed, Move tests to use mirroring function, gNMI port breakout tests, Disable sFlow by default in QoS punt test case, Do not restore entire static config passed in & Make it clear when L3AdmitTest is restricting packets based on port IDs.

---------

Co-authored-by: kishanps <kishanps@google.com>

tests/forwarding/l3_admit_test.cc

@@ -200,8 +200,14 @@ absl::Status AddL3Route(pdpi::P4RuntimeSession& session,
 absl::Status AdmitL3Route(pdpi::P4RuntimeSession& session,
                           const pdpi::IrP4Info& ir_p4info,
                           const L3AdmitOptions& options) {
-  LOG(INFO) << "Admiting L3 packets with DST MAC: " << options.dst_mac.first
-            << " & " << options.dst_mac.second;
+  if (options.in_port.has_value()) {
+    LOG(INFO) << "Admiting only L3 packets on port " << *options.in_port
+              << " with DST MAC: " << options.dst_mac.first << " & "
+              << options.dst_mac.second;
+  } else {
+    LOG(INFO) << "Admiting all L3 packets with DST MAC: "
+              << options.dst_mac.first << " & " << options.dst_mac.second;
+  }
   p4::v1::WriteRequest write_request;
   ASSIGN_OR_RETURN(
       *write_request.add_updates(),

tests/forwarding/l3_admit_test.h

@@ -58,17 +58,7 @@ class L3AdmitTestFixture : public testing::TestWithParam<L3AdmitTestParams> {
     // control switch means it will arrive on the same port ID on the SUT. To
     // achieve this, we mirror the SUTs OpenConfig interfaces <-> P4RT Port ID
     // mapping to the control switch.
-    ASSERT_OK_AND_ASSIGN(
-        std::unique_ptr<gnmi::gNMI::StubInterface> sut_gnmi_stub,
-        testbed.Sut().CreateGnmiStub());
-    ASSERT_OK_AND_ASSIGN(const pins_test::openconfig::Interfaces sut_interfaces,
-                         pins_test::GetInterfacesAsProto(
-                             *sut_gnmi_stub, gnmi::GetRequest::CONFIG));
-    ASSERT_OK_AND_ASSIGN(
-        std::unique_ptr<gnmi::gNMI::StubInterface> control_gnmi_stub,
-        testbed.ControlSwitch().CreateGnmiStub());
-    ASSERT_OK(
-        pins_test::SetInterfaceP4rtIds(*control_gnmi_stub, sut_interfaces));
+    ASSERT_OK(pins_test::MirrorSutP4rtPortIdConfigToControlSwitch(testbed));
 
     ASSERT_OK_AND_ASSIGN(ir_p4info_, pdpi::CreateIrP4Info(GetParam().p4info));
   }

tests/qos/BUILD.bazel

@@ -57,6 +57,7 @@ cc_library(
         "//sai_p4/instantiations/google:sai_pd_cc_proto",
         "//tests/forwarding:util",
         "//tests/lib:switch_test_setup_helpers",
+        "//tests/sflow:sflow_util",
         "//thinkit:control_device",
         "//thinkit:generic_testbed",
         "//thinkit:generic_testbed_fixture",

tests/qos/cpu_qos_test.cc

@@ -39,9 +39,7 @@
 #include "absl/types/optional.h"
 #include "absl/types/variant.h"
 #include "glog/logging.h"
-#include "gmock/gmock.h"
 #include "google/protobuf/util/json_util.h"
-#include "gtest/gtest.h"
 #include "gutil/collections.h"
 #include "gutil/overload.h"
 #include "gutil/proto.h"
@@ -73,11 +71,14 @@
 #include "tests/qos/gnmi_parsers.h"
 #include "tests/qos/packet_in_receiver.h"
 #include "tests/qos/qos_test_util.h"
+#include "tests/sflow/sflow_util.h"
 #include "thinkit/control_device.h"
 #include "thinkit/generic_testbed.h"
 #include "thinkit/mirror_testbed.h"
 #include "thinkit/proto/generic_testbed.pb.h"
 #include "thinkit/switch.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
 
 namespace pins_test {
 namespace {
@@ -1369,6 +1370,18 @@ TEST_P(CpuQosTestWithIxia, TestPuntFlowRateLimitAndCounters) {
 
   ASSERT_OK_AND_ASSIGN(auto gnmi_stub, sut.CreateGnmiStub());
 
+  // Disable sFlow since it would interfere with the test results.
+  ASSERT_OK(pins::SetSflowConfigEnabled(gnmi_stub.get(), /*enabled=*/false));
+
+  absl::Cleanup cleanup([&] {
+    // Restore sflow enable config.
+    ASSERT_OK_AND_ASSIGN(bool sflow_enabled,
+                         pins::IsSflowConfigEnabled(GetParam().gnmi_config));
+    EXPECT_OK(pins::SetSflowConfigEnabled(gnmi_stub.get(), sflow_enabled))
+        << "failed to restore sflow configuration -- switch config may be "
+           "corrupted, causing subsequent test to fail";
+  });
+
   // Flow details.
   const auto dest_mac = netaddr::MacAddress(02, 02, 02, 02, 02, 02);
   const auto source_mac = netaddr::MacAddress(00, 01, 02, 03, 04, 05);
@@ -1609,8 +1622,7 @@ TEST_P(CpuQosTestWithIxia, TestPuntFlowRateLimitAndCounters) {
           rate_received_in_bytes_per_second,
           flow_rate_limit_in_bytes_per_second * (1 - kTolerancePercent / 100));
     }
-  }  // for each queue.
-
+  } // for each queue.
 }
 
 }  // namespace

tests/qos/frontpanel_qos_test.cc

@@ -595,7 +595,7 @@ TEST_P(FrontpanelQosTest,
   ASSERT_OK_AND_ASSIGN(const std::string kIxiaDstPortHandle,
                        ixia::IxiaVport(kIxiaHandle, kIxiaDstPort, *testbed));
 
-  constexpr int64_t kSpeed400g = 400000000000;
+  constexpr int64_t kSpeed100g = 100000000000;
   constexpr int64_t kSpeed200g = 200000000000;
   if (kSutEgressPortSpeedInBitsPerSecond != kSpeed200g) {
     ASSERT_OK(SetPortSpeedInBitsPerSecond(PortSpeed(kSpeed200g), kSutEgressPort,
@@ -604,7 +604,7 @@ TEST_P(FrontpanelQosTest,
         PortSpeed(kSutEgressPortSpeedInBitsPerSecond), kSutEgressPort,
         *gnmi_stub));
   } else {
-    ASSERT_OK(SetPortSpeedInBitsPerSecond(PortSpeed(kSpeed400g), kSutEgressPort,
+    ASSERT_OK(SetPortSpeedInBitsPerSecond(PortSpeed(kSpeed100g), kSutEgressPort,
                                           *gnmi_stub));
     ASSERT_OK(SetPortSpeedInBitsPerSecond(
         PortSpeed(kSutEgressPortSpeedInBitsPerSecond), kSutEgressPort,

tests/thinkit_gnmi_interface_tests.cc

@@ -55,15 +55,17 @@ namespace {
 
 using ::nlohmann::json;
 using ::testing::HasSubstr;
+using ::testing::Not;
 
 }  // namespace
 
-void BreakoutDuringPortInUse(thinkit::Switch& sut,
-                             gnmi::gNMI::StubInterface* sut_gnmi_stub,
+void BreakoutDuringPortInUse(thinkit::Switch &sut,
+                             gnmi::gNMI::StubInterface *sut_gnmi_stub,
                              RandomPortBreakoutInfo port_info,
-                             const std::string& platform_json_contents,
-                             bool test_child_port_in_use,
-                             const p4::config::v1::P4Info& p4_info) {
+                             absl::string_view platform_json_contents,
+                             absl::string_view port_in_use,
+                             const p4::config::v1::P4Info &p4_info,
+                             const bool expect_breakout_failure) {
   // Get the original breakout info on the port.
   // This contains the state values of physical channels and
   // operational status information for ports in original breakout mode.
@@ -80,23 +82,11 @@ void BreakoutDuringPortInUse(thinkit::Switch& sut,
                                                          kStateUp, {p.first}));
   }
 
-  // Determine port to install router interface on.
-  auto in_use_port = port_info.port_name;
-  if (test_child_port_in_use) {
-    for (const auto& p : orig_breakout_info) {
-      if (p.first != port_info.port_name) {
-        in_use_port = p.first;
-        break;
-      }
-    }
-    ASSERT_NE(in_use_port, port_info.port_name);
-  }
-
   // Get the p4rt port index of the port on which to install router interface.
   ASSERT_OK_AND_ASSIGN(auto port_id_by_interface,
                        GetAllInterfaceNameToPortId(*sut_gnmi_stub));
   ASSERT_OK_AND_ASSIGN(std::string in_use_port_index_value,
-                       gutil::FindOrStatus(port_id_by_interface, in_use_port));
+                       gutil::FindOrStatus(port_id_by_interface, port_in_use));
   int in_use_port_index;
   ASSERT_TRUE(absl::SimpleAtoi(in_use_port_index_value, &in_use_port_index));
 
@@ -125,7 +115,7 @@ void BreakoutDuringPortInUse(thinkit::Switch& sut,
   ASSERT_OK_AND_ASSIGN(const p4::v1::TableEntry pi_entry,
                        pdpi::PartialPdTableEntryToPiTableEntry(pdpi::IrP4Info(), pd_entry));
 
-  LOG(INFO) << "Installing router interface on port " << in_use_port
+  LOG(INFO) << "Installing router interface on port " << port_in_use
             << " on SUT";
   ASSERT_OK(pdpi::InstallPiTableEntry(sut_p4_session.get(), pi_entry));
 
@@ -139,45 +129,48 @@ void BreakoutDuringPortInUse(thinkit::Switch& sut,
                                             port_info.port_name,
                                             port_info.supported_breakout_mode));
 
-  // Apply breakout config on port. Expect the set operation to fail
-  // since the port/its child is in use.
-  LOG(INFO) << "Configuring breakout mode " << port_info.supported_breakout_mode
-            << " on port " << port_info.port_name << " on DUT";
-  auto status = sut_gnmi_stub->Set(&context, req, &resp);
-  ASSERT_NE(status.ok(), true);
-  EXPECT_THAT(status.error_message(),
-              HasSubstr(absl::StrCat(
-                  "SET failed: YangToDb_port_breakout_subtree_xfmr: port ",
-                  in_use_port, " is in use")));
-
   // Get expected port information for new breakout mode.
   ASSERT_OK_AND_ASSIGN(
       auto new_breakout_info,
       GetExpectedPortInfoForBreakoutMode(port_info.port_name,
                                          port_info.supported_breakout_mode));
-  auto non_existing_port_list = GetNonExistingPortsAfterBreakout(
-      orig_breakout_info, new_breakout_info, false);
-  // Verify breakout related state paths.
-  ASSERT_OK(ValidateBreakoutState(sut_gnmi_stub, orig_breakout_info,
-                                  non_existing_port_list));
 
-  // Delete the created router interface on the port using P4 interface.
-  LOG(INFO) << "Deleting router interface on SUT";
-  ASSERT_OK(pdpi::ClearTableEntries(sut_p4_session.get()));
+  // Apply breakout config on port.
+  LOG(INFO) << "Configuring breakout mode " << port_info.supported_breakout_mode
+            << " on port " << port_info.port_name << " on DUT";
+  auto status = sut_gnmi_stub->Set(&context, req, &resp);
+  if (expect_breakout_failure) {
+    // Expect the set operation to fail since the port/its child is in use.
+    ASSERT_THAT(status, Not(gutil::IsOk()));
+    EXPECT_THAT(status.error_message(),
+                HasSubstr(absl::StrCat(
+                    "SET failed: YangToDb_port_breakout_subtree_xfmr: port ",
+                    port_in_use, " is in use")));
+    auto non_existing_port_list = GetNonExistingPortsAfterBreakout(
+        orig_breakout_info, new_breakout_info, false);
+    // Verify breakout related state paths.
+    ASSERT_OK(ValidateBreakoutState(sut_gnmi_stub, orig_breakout_info,
+                                    non_existing_port_list));
 
-  // Retry port breakout.
-  grpc::ClientContext context2;
-  status = sut_gnmi_stub->Set(&context2, req, &resp);
-  ASSERT_EQ(status.ok(), true);
+    // Delete the created router interface on the port using P4 interface.
+    LOG(INFO) << "Deleting router interface on SUT";
+    ASSERT_OK(pdpi::ClearTableEntries(sut_p4_session.get()));
+
+    // Retry port breakout.
+    grpc::ClientContext context2;
+    status = sut_gnmi_stub->Set(&context2, req, &resp);
+  }
+  ASSERT_OK(status);
 
   // Wait for breakout config to go through.
   // TODO: Investigate changing to polling loop.
   absl::SleepFor(absl::Seconds(60));
 
   // Verify that the config is successfully applied now.
-  non_existing_port_list = GetNonExistingPortsAfterBreakout(
+  auto non_existing_port_list = GetNonExistingPortsAfterBreakout(
       orig_breakout_info, new_breakout_info, true);
-  // Oper-status is expected to be down as breakout is applied on one side only.
+  // Oper-status is expected to be down as breakout is applied on one side
+  // only.
   for (auto& port : new_breakout_info) {
     port.second.oper_status = kStateDown;
     // For the logical ports that do not change on breakout, expect them to be
@@ -223,7 +216,9 @@ void TestGNMIParentPortInUseDuringBreakout(
                                       *sut_gnmi_stub, platform_json_contents,
                                       BreakoutType::kAny));
   BreakoutDuringPortInUse(sut, sut_gnmi_stub.get(), port,
-                          platform_json_contents, false, p4_info);
+                          platform_json_contents,
+                          /*port_in_use=*/port.port_name, p4_info,
+                          /*expect_breakout_failure=*/true);
 }
 
 void TestGNMIChildPortInUseDuringBreakout(
@@ -232,17 +227,60 @@ void TestGNMIChildPortInUseDuringBreakout(
   ASSERT_OK_AND_ASSIGN(auto sut_gnmi_stub, sut.CreateGnmiStub());
   // Get a random port from list of front panel ports that supports at least
   // one breakout mode of required type other than its current mode.
-  auto status_or = GetRandomPortWithSupportedBreakoutModes(
+  auto port = GetRandomPortWithSupportedBreakoutModes(
       *sut_gnmi_stub, platform_json_contents,
       /*new_breakout_type=*/BreakoutType::kAny,
       /*current_breakout_type=*/BreakoutType::kChannelized);
-  if (status_or.status().ok()) {
-    BreakoutDuringPortInUse(sut, sut_gnmi_stub.get(), status_or.value(),
-                            platform_json_contents, true, p4_info);
+  // Get a child port to install the router interface on.
+  // Get the original breakout info on the port.
+  // This contains the state values of physical channels and
+  // operational status information for ports in original breakout mode.
+  if (port.ok()) {
+    ASSERT_OK_AND_ASSIGN(auto orig_breakout_info,
+                         GetBreakoutStateInfoForPort(sut_gnmi_stub.get(),
+                                                     port->port_name,
+                                                     port->curr_breakout_mode));
+    std::string port_in_use;
+    for (const auto &p : orig_breakout_info) {
+      if (p.first != port->port_name) {
+        port_in_use = p.first;
+        break;
+      }
+    }
+    ASSERT_FALSE(port_in_use.empty());
+    BreakoutDuringPortInUse(sut, sut_gnmi_stub.get(), *port,
+                            platform_json_contents, port_in_use, p4_info,
+                            /*expect_breakout_failure=*/true);
   } else {
     LOG(ERROR) << "Failed to get random port with new breakout type: "
                << BreakoutType::kAny
                << " and current breakout type: " << BreakoutType::kChannelized;
   }
 }
+
+void TestGNMIOtherMasterPortInUseDuringBreakout(
+    thinkit::Switch &sut, std::string &platform_json_contents,
+    const p4::config::v1::P4Info &p4_info) {
+  ASSERT_OK_AND_ASSIGN(auto sut_gnmi_stub, sut.CreateGnmiStub());
+  // Get a random port from list of front panel ports that supports at least
+  // one breakout mode of required type other than its current mode.
+  ASSERT_OK_AND_ASSIGN(auto port, GetRandomPortWithSupportedBreakoutModes(
+                                      *sut_gnmi_stub, platform_json_contents,
+                                      BreakoutType::kAny));
+  // Get another random port from list of front panel ports to install router
+  // interface on.
+  auto platform_json = json::parse(platform_json_contents);
+  auto interfaces_json = platform_json.find(kInterfaces);
+  interfaces_json->erase(port.port_name);
+  auto updated_platform_json_contents = platform_json.dump();
+
+  ASSERT_OK_AND_ASSIGN(
+      auto port_in_use,
+      GetRandomPortWithSupportedBreakoutModes(
+          *sut_gnmi_stub, updated_platform_json_contents, BreakoutType::kAny));
+  BreakoutDuringPortInUse(sut, sut_gnmi_stub.get(), port,
+                          platform_json_contents, port_in_use.port_name,
+                          p4_info,
+                          /*expect_breakout_failure=*/false);
+}
 }  // namespace pins_test

tests/thinkit_gnmi_interface_tests.h

@@ -39,13 +39,19 @@ void TestGNMIChildPortInUseDuringBreakout(
     const p4::config::v1::P4Info& p4_info =
         sai::GetP4Info(sai::Instantiation::kMiddleblock));
 
+// Test port breakout on a port while other master port is in use.
+void TestGNMIOtherMasterPortInUseDuringBreakout(
+    thinkit::Switch &sut, std::string &platform_json_contents,
+    const p4::config::v1::P4Info &p4_info);
+
 // Helper function to test port in use.
 void BreakoutDuringPortInUse(thinkit::Switch &sut,
                              gnmi::gNMI::StubInterface *sut_gnmi_stub,
                              RandomPortBreakoutInfo port_info,
-                             const std::string& platform_json_contents,
-                             bool test_child_port_in_use,
-                             const p4::config::v1::P4Info& p4_info);
+                             absl::string_view platform_json_contents,
+                             absl::string_view port_in_use,
+                             const p4::config::v1::P4Info &p4_info,
+                             const bool expect_breakout_failure);
 
 // Test port breakout during parent port in use.
 void TestGNMIParentPortInUseDuringBreakout(thinkit::Switch &sut,

tests/thinkit_gnmi_interface_util.cc

@@ -803,8 +803,8 @@ absl::Status ValidateBreakoutState(
   return absl::OkStatus();
 }
 
-absl::StatusOr<std::string> GetPortIndex(
-    const std::string& platform_json_contents, absl::string_view port) {
+absl::StatusOr<std::string>
+GetPortIndex(absl::string_view platform_json_contents, absl::string_view port) {
   // Get interfaces from platform.json.
   const auto platform_json = json::parse(platform_json_contents);
   const auto interfaces_json = platform_json.find(kInterfaces);

tests/thinkit_gnmi_interface_util.h

@@ -151,7 +151,7 @@ ValidateBreakoutState(gnmi::gNMI::StubInterface *sut_gnmi_stub,
                       const std::vector<std::string> &non_existing_ports_list);
 
 absl::StatusOr<std::string>
-GetPortIndex(const std::string &platform_json_contents, absl::string_view port);
+GetPortIndex(absl::string_view platform_json_contents, absl::string_view port);
 
 std::string ConstructSupportedBreakoutMode(absl::string_view num_breakouts,
                                            absl::string_view breakout_speed);