[Thinkit] Implement watch port test, Add member down test case, Add s…

…ingle member and watchport modify test cases, Add down ports to the group and verify watch port action & Use control switch for admin up/down operations in watch port tests. (#696)

Co-authored-by: kishanps <[email protected]>

dvaas/packet_injection.cc

@@ -134,7 +134,7 @@ absl::StatusOr<PacketTestRuns> SendTestPacketsAndCollectOutputs(
       const Packet& packet = packet_test_vector.input().packet();
 
       // Inject to egress of control switch.
-      RETURN_IF_ERROR(gpins::InjectEgressPacket(
+      RETURN_IF_ERROR(pins::InjectEgressPacket(
           packet.port(), absl::HexStringToBytes(packet.hex()),
           control_ir_p4info, &control_switch, injection_delay));
     } else {

lib/pins_control_device.cc

@@ -162,7 +162,7 @@ absl::Status PinsControlDevice::SendPacket(
         "No P4RuntimeSession exists; Likely failed to establish another "
         "P4RuntimeSession.");
   }
-  return gpins::InjectEgressPacket(interface_name_to_port_id_[interface],
+  return pins::InjectEgressPacket(interface_name_to_port_id_[interface],
                                    std::string(packet), ir_p4_info_,
                                    control_session_.get(), packet_delay);
 }

tests/forwarding/BUILD.bazel

@@ -190,15 +190,50 @@ cc_library(
 cc_library(
     name = "watch_port_test",
     testonly = True,
+    srcs = ["watch_port_test.cc"],
     hdrs = ["watch_port_test.h"],
     linkstatic = 1,
     deps = [
         ":group_programming_util",
         ":packet_test_util",
+        ":util",
+        "//dvaas:test_vector_cc_proto",
+        "//gutil:collections",
+        "//gutil:proto_matchers",
+        "//gutil:status_matchers",
+        "//gutil:testing",
+        "//lib/gnmi:gnmi_helper",
+        "//p4_pdpi:ir_cc_proto",
         "//p4_pdpi:p4_runtime_session",
+        "//p4_pdpi:pd",
+        "//p4_pdpi/packetlib",
+        "//p4_pdpi/packetlib:packetlib_cc_proto",
+        "//p4_pdpi/string_encodings:decimal_string",
+        "//p4rt_app/tests/lib:p4runtime_grpc_service",
+        "//sai_p4/instantiations/google:instantiations",
         "//sai_p4/instantiations/google:sai_p4info_cc",
+        "//sai_p4/instantiations/google:sai_pd_cc_proto",
+        "//tests:thinkit_sanity_tests",
+        "//thinkit:mirror_testbed",
         "//thinkit:mirror_testbed_fixture",
+        "//thinkit:switch",
+        "//thinkit:test_environment",
+        "@com_github_gnmi//proto/gnmi:gnmi_cc_grpc_proto",
+        "@com_github_google_glog//:glog",
         "@com_github_grpc_grpc//:grpc++",
+        "@com_github_p4lang_p4runtime//:p4info_cc_proto",
+        "@com_github_p4lang_p4runtime//:p4runtime_cc_grpc",
+        "@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
+        "@com_google_absl//absl/container:flat_hash_map",
+        "@com_google_absl//absl/container:flat_hash_set",
+        "@com_google_absl//absl/random",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
+        "@com_google_absl//absl/strings",
+        "@com_google_absl//absl/synchronization",
+        "@com_google_absl//absl/time",
+        "@com_google_absl//absl/types:span",
+        "@com_google_googletest//:gtest",
     ],
     alwayslink = True,
 )

tests/forwarding/group_programming_util.cc

@@ -33,9 +33,9 @@
 namespace pins {
 
 absl::Status ProgramNextHops(thinkit::TestEnvironment& test_environment,
-                             pdpi::P4RuntimeSession* const p4_session,
+                             pdpi::P4RuntimeSession& p4_session,
                              const pdpi::IrP4Info& ir_p4info,
-                             std::vector<pins::Member>& members) {
+                             std::vector<pins::GroupMember>& members) {
   int index = 0;
   std::vector<std::string> nexthops;
   std::vector<p4::v1::TableEntry> pi_entries;
@@ -103,7 +103,7 @@ absl::Status ProgramNextHops(thinkit::TestEnvironment& test_environment,
 
   // Program the switch.
   RETURN_IF_ERROR(
-      (pdpi::InstallPiTableEntries(p4_session, ir_p4info, pi_entries)));
+      (pdpi::InstallPiTableEntries(&p4_session, ir_p4info, pi_entries)));
 
   // Write the PI & PD entries to artifacts.
   for (const auto& pi : pi_entries) {
@@ -124,10 +124,10 @@ absl::Status ProgramNextHops(thinkit::TestEnvironment& test_environment,
 }
 
 absl::Status ProgramGroupWithMembers(thinkit::TestEnvironment& test_environment,
-                                     pdpi::P4RuntimeSession* const p4_session,
+                                     pdpi::P4RuntimeSession& p4_session,
                                      const pdpi::IrP4Info& ir_p4info,
                                      absl::string_view group_id,
-                                     absl::Span<const Member> members,
+                                     absl::Span<const GroupMember> members,
                                      const p4::v1::Update_Type& type) {
   auto group_update = gutil::ParseProtoOrDie<sai::TableEntry>(absl::Substitute(
       R"pb(
@@ -168,7 +168,7 @@ absl::Status ProgramGroupWithMembers(thinkit::TestEnvironment& test_environment,
   update->set_type(type);
   *update->mutable_entity()->mutable_table_entry() = pi_entry;
   RETURN_IF_ERROR(
-      pdpi::SetMetadataAndSendPiWriteRequest(p4_session, write_request));
+      pdpi::SetMetadataAndSendPiWriteRequest(&p4_session, write_request));
 
   // Append the PI & PD entries.
   RETURN_IF_ERROR(test_environment.AppendToTestArtifact(
@@ -178,7 +178,7 @@ absl::Status ProgramGroupWithMembers(thinkit::TestEnvironment& test_environment,
   return absl::OkStatus();
 }
 
-absl::Status DeleteGroup(pdpi::P4RuntimeSession* const p4_session,
+absl::Status DeleteGroup(pdpi::P4RuntimeSession& p4_session,
                          const pdpi::IrP4Info& ir_p4info,
                          absl::string_view group_id) {
   ASSIGN_OR_RETURN(
@@ -196,19 +196,20 @@ absl::Status DeleteGroup(pdpi::P4RuntimeSession* const p4_session,
               )pb",
               group_id))));
 
-  return pdpi::SetMetadataAndSendPiWriteRequest(p4_session, write_request);
+  return pdpi::SetMetadataAndSendPiWriteRequest(&p4_session, write_request);
 }
 
 // Verifies  the actual members received from P4 read response matches the
 // expected members.
 absl::Status VerifyGroupMembersFromP4Read(
-    pdpi::P4RuntimeSession* const p4_session, const pdpi::IrP4Info& ir_p4info,
-    absl::string_view group_id, absl::Span<const Member> expected_members) {
+    pdpi::P4RuntimeSession& p4_session, const pdpi::IrP4Info& ir_p4info,
+    absl::string_view group_id,
+    absl::Span<const GroupMember> expected_members) {
   p4::v1::ReadRequest read_request;
   read_request.add_entities()->mutable_table_entry();
   ASSIGN_OR_RETURN(
       p4::v1::ReadResponse read_response,
-      pdpi::SetMetadataAndSendPiReadRequest(p4_session, read_request));
+      pdpi::SetMetadataAndSendPiReadRequest(&p4_session, read_request));
 
   // Filter out WCMP group entries separately from the whole read response.
   absl::flat_hash_map<std::string, p4::v1::TableEntry>
@@ -238,7 +239,7 @@ absl::Status VerifyGroupMembersFromP4Read(
         gutil::ParseProtoOrDie<sai::WcmpGroupTableEntry::WcmpAction>(
             absl::Substitute(R"pb(
                                action { set_nexthop_id { nexthop_id: "$0" } }
-                               weight: 0
+                               weight: 1
                                watch_port: ""
                              )pb",
                              member.nexthop));
@@ -306,8 +307,8 @@ int RescaleWeightForTomahawk3(int weight) {
   return (weight - 1) / 2;
 }
 
-void RescaleMemberWeights(std::vector<Member>& members) {
-  for (Member& member : members) {
+void RescaleMemberWeights(std::vector<GroupMember>& members) {
+  for (GroupMember& member : members) {
     int old_weight = member.weight;
     member.weight = RescaleWeightForTomahawk3(old_weight);
     LOG(INFO) << "Rescaling member id: " << member.port
@@ -316,4 +317,32 @@ void RescaleMemberWeights(std::vector<Member>& members) {
   }
 }
 
+std::string DescribeDistribution(
+    int expected_total_test_packets,
+    absl::Span<const pins::GroupMember> members,
+    const absl::flat_hash_map<int, int>& num_packets_per_port,
+    bool expect_single_port) {
+  double total_weight = 0;
+  for (const auto& member : members) {
+    total_weight += member.weight;
+  }
+  std::string explanation = "";
+  for (const auto& member : members) {
+    double actual_packets = num_packets_per_port.contains(member.port)
+                                ? num_packets_per_port.at(member.port)
+                                : 0;
+    if (expect_single_port) {
+      absl::StrAppend(&explanation, "\nport ", member.port,
+                      ": actual_count = ", actual_packets);
+    } else {
+      double expected_packets =
+          expected_total_test_packets * member.weight / total_weight;
+      absl::StrAppend(&explanation, "\nport ", member.port, " with weight ",
+                      member.weight, ": expected_count = ", expected_packets,
+                      ", actual_count = ", actual_packets);
+    }
+  }
+  return explanation;
+}
+
 }  // namespace pins

tests/forwarding/group_programming_util.h

@@ -28,7 +28,7 @@ namespace pins {
 
 // Structure that holds the member details like port, weight and the
 // nexthop object key (output) that was created.
-struct Member {
+struct GroupMember {
   int weight = 1;
   int port = 0;
   std::string nexthop;
@@ -39,31 +39,31 @@ struct Member {
 // members.nexthop is an output here with the updated nexthop object that was
 // created.
 absl::Status ProgramNextHops(thinkit::TestEnvironment& test_environment,
-                             pdpi::P4RuntimeSession* const p4_session,
+                             pdpi::P4RuntimeSession& p4_session,
                              const pdpi::IrP4Info& ir_p4info,
-                             std::vector<pins::Member>& members);
+                             std::vector<pins::GroupMember>& members);
 
 // Programs (insert/modify) a nexthop group on the switch with the given
 // set of nexthops and weights. It is expected that the dependant nexthops are
 // already created for an insert/modify operation.
 absl::Status ProgramGroupWithMembers(thinkit::TestEnvironment& test_environment,
-                                     pdpi::P4RuntimeSession* const p4_session,
+                                     pdpi::P4RuntimeSession& p4_session,
                                      const pdpi::IrP4Info& ir_p4info,
                                      absl::string_view group_id,
-                                     absl::Span<const Member> members,
+                                     absl::Span<const GroupMember> members,
                                      const p4::v1::Update_Type& type);
 
 // Deletes the group with the given group_id. It is expected that the caller
 // takes care of cleaning up the dependant nexthops.
-absl::Status DeleteGroup(pdpi::P4RuntimeSession* const p4_session,
+absl::Status DeleteGroup(pdpi::P4RuntimeSession& p4_session,
                          const pdpi::IrP4Info& ir_p4info,
                          absl::string_view group_id);
 
 // Verifies the actual members received from P4 read response matches the
 // expected members.
 absl::Status VerifyGroupMembersFromP4Read(
-    pdpi::P4RuntimeSession* const p4_session, const pdpi::IrP4Info& ir_p4info,
-    absl::string_view group_id, absl::Span<const Member> expected_members);
+    pdpi::P4RuntimeSession& p4_session, const pdpi::IrP4Info& ir_p4info,
+    absl::string_view group_id, absl::Span<const GroupMember> expected_members);
 
 // Verifies the actual members inferred from receive traffic matches the
 // expected members.
@@ -85,7 +85,17 @@ int RescaleWeightForTomahawk3(int weight);
 // hardware behaviour, remove when hardware supports > 128 weights.
 // Halves member weights >= 2 and works only for sum of initial member weights
 // <= 256.
-void RescaleMemberWeights(std::vector<Member>& members);
+void RescaleMemberWeights(std::vector<GroupMember>& members);
+
+// Returns a human-readable description of the actual vs expected
+// distribution of packets on the group member ports.
+// expect_single_port specifies whether all packets are expected on a single
+// output port(since no hashing applies) or multiple ports(with hashing).
+std::string DescribeDistribution(
+    int expected_total_test_packets,
+    absl::Span<const pins::GroupMember> members,
+    const absl::flat_hash_map<int, int>& num_packets_per_port,
+    bool expect_single_port);
 
 }  // namespace pins
 

tests/forwarding/packet_test_util.cc

@@ -110,6 +110,17 @@ uint32_t GetIthL4Port(int i, uint32_t base) {
 
 }  // namespace
 
+// Clears the received packet output vector and the packet statistics counters.
+void TestData::ClearReceivedPackets() {
+  absl::MutexLock lock(&mutex);
+  for (auto& [packet, input_output] : input_output_per_packet) {
+    input_output.output.clear();
+  }
+  total_packets_sent = 0;
+  total_packets_received = 0;
+  total_invalid_packets_received = 0;
+}
+
 // Is this a valid test configuration?  Not all configurations are valid, e.g.
 // you can't modify the flow label in an IPv4 packet (because there is no flow
 // label there).

tests/forwarding/packet_test_util.h

@@ -79,6 +79,9 @@ struct TestData {
   int total_invalid_packets_received = 0;
   absl::flat_hash_map<std::string, TestInputOutput> input_output_per_packet
       ABSL_GUARDED_BY(mutex);
+  // Clears the received packets in the output vector and the send/receive
+  // counters.
+  void ClearReceivedPackets() ABSL_LOCKS_EXCLUDED(mutex);
 };
 
 // Checks wehether this a valid test configuration. Not all configurations are

tests/forwarding/util.cc

@@ -23,7 +23,7 @@
 #include "p4_pdpi/ir.pb.h"
 #include "sai_p4/tools/packetio_tools.h"
 
-namespace gpins {
+namespace pins {
 
 absl::Status TryUpToNTimes(int n, absl::Duration delay,
                            std::function<absl::Status(int)> callback) {
@@ -91,4 +91,4 @@ absl::Status InjectIngressPacket(const std::string& packet,
                    << request.ShortDebugString();
 }
 
-}  // namespace gpins
+}  // namespace pins

tests/forwarding/util.h

@@ -24,7 +24,7 @@
 #include "p4_pdpi/ir.pb.h"
 #include "p4_pdpi/p4_runtime_session.h"
 
-namespace gpins {
+namespace pins {
 
 // Calls given callback up to the given number of times with the given delay in
 // between successive attempts, returning ok status as soon as the callback
@@ -68,6 +68,6 @@ absl::StatusOr<T> TryStatusOrUpToNTimes(
   return result;
 }
 
-}  // namespace gpins
+}  // namespace pins
 
 #endif  // PINS_TESTS_FORWARDING_UTIL_H_