Merge branch 'main' of https://github.com/VSuryaprasad-HCL/sonic-pins …

…into symbolic_import_branch_128

dvaas/BUILD.bazel

@@ -252,7 +252,9 @@ cc_library(
         ":test_vector_cc_proto",
         "//gutil:proto",
         "//gutil:status",
+        "//p4_pdpi:ir",
         "//p4_pdpi/packetlib",
+        "@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
@@ -274,6 +276,7 @@ cc_test(
         "//gutil:status_matchers",
         "//gutil:testing",
         "//p4_pdpi/packetlib:packetlib_cc_proto",
+        "//p4_pdpi/testing:test_p4info_cc",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
@@ -376,7 +379,6 @@ cc_library(
         "//lib/gnmi:gnmi_helper",
         "//lib/gnmi:openconfig_cc_proto",
         "//lib/p4rt:p4rt_port",
-        "//p4_pdpi:ir",
         "//p4_pdpi:ir_cc_proto",
         "//p4_pdpi:p4_runtime_session",
         "//p4_pdpi:p4_runtime_session_extras",

dvaas/dataplane_validation.cc

@@ -41,7 +41,6 @@
 #include "lib/gnmi/openconfig.pb.h"
 #include "lib/p4rt/p4rt_port.h"
 #include "p4/v1/p4runtime.pb.h"
-#include "p4_pdpi/ir.h"
 #include "p4_pdpi/ir.pb.h"
 #include "p4_pdpi/p4_runtime_session.h"
 #include "p4_pdpi/p4_runtime_session_extras.h"
@@ -254,8 +253,10 @@ absl::StatusOr<ValidationResult> DataplaneValidator::ValidateDataplane(
                      GenerateTestVectors(params, sut, *backend_, *writer));
   } else {
     LOG(INFO) << "Checking user-provided test vectors for well-formedness";
-    ASSIGN_OR_RETURN(test_vectors, LegitimizeUserProvidedTestVectors(
-                                       params.packet_test_vector_override));
+    ASSIGN_OR_RETURN(pdpi::IrP4Info ir_info, pdpi::GetIrP4Info(*sut.p4rt));
+    ASSIGN_OR_RETURN(test_vectors,
+                     LegitimizeUserProvidedTestVectors(
+                         params.packet_test_vector_override, ir_info));
   }
   RETURN_IF_ERROR(
       writer->AppendToTestArtifact("test_vectors.txt", ToString(test_vectors)));

dvaas/packet_injection.cc

@@ -153,10 +153,19 @@ absl::StatusOr<PacketTestRuns> SendTestPacketsAndCollectOutputs(
     if (packet_test_vector.input().type() == SwitchInput::DATAPLANE) {
       const Packet& packet = packet_test_vector.input().packet();
 
+      // Get corresponding control switch port for the packet's ingress port.
+      ASSIGN_OR_RETURN(const P4rtPortId sut_ingress_port,
+                       P4rtPortId::MakeFromP4rtEncoding(packet.port()));
+      ASSIGN_OR_RETURN(
+          const P4rtPortId control_switch_port,
+          parameters.mirror_testbed_port_map
+              .GetControlSwitchPortConnectedToSutPort(sut_ingress_port));
+
       // Inject to egress of control switch.
       RETURN_IF_ERROR(pins::InjectEgressPacket(
-          packet.port(), absl::HexStringToBytes(packet.hex()),
-          control_ir_p4info, &control_switch, injection_delay));
+          control_switch_port.GetP4rtEncoding(),
+          absl::HexStringToBytes(packet.hex()), control_ir_p4info,
+          &control_switch, injection_delay));
     } else {
       return absl::UnimplementedError(
           absl::StrCat("Test vector input type not supported\n",

dvaas/port_id_map.cc

@@ -88,9 +88,28 @@ MirrorTestbedP4rtPortIdMap::GetSutPortConnectedToControlSwitchPort(
         absl::Substitute("Control port '$0' was not found in control switch "
                          "to SUT P4RT port ID map.",
                          control_port));
-  } else {
-    return it->second;
   }
+  return it->second;
+}
+
+absl::StatusOr<pins_test::P4rtPortId>
+MirrorTestbedP4rtPortIdMap::GetControlSwitchPortConnectedToSutPort(
+    const pins_test::P4rtPortId& sut_port) const {
+  // Handle implicit identity map.
+  if (!control_to_sut_port_map_.has_value()) return sut_port;
+
+  // Handle explicit map.
+  const auto it = absl::c_find_if(*control_to_sut_port_map_,
+                                  [&](const auto& control_sut_port) {
+                                    return control_sut_port.second == sut_port;
+                                  });
+  if (it == control_to_sut_port_map_->end()) {
+    return absl::NotFoundError(
+        absl::Substitute("SUT port '$0' was not found in control switch "
+                         "to control switch P4RT port ID map.",
+                         sut_port));
+  }
+  return it->first;
 }
 
 absl::Status CheckAndStoreMappedAndUnmappedPortIds(

dvaas/port_id_map.h

@@ -66,6 +66,12 @@ class MirrorTestbedP4rtPortIdMap {
   absl::StatusOr<pins_test::P4rtPortId> GetSutPortConnectedToControlSwitchPort(
       const pins_test::P4rtPortId& control_port) const;
 
+  // Returns the P4RT port ID of the control switch interface connected to the
+  // interface on the SUT with the given P4RT port ID according to the port
+  // mapping.
+  absl::StatusOr<pins_test::P4rtPortId> GetControlSwitchPortConnectedToSutPort(
+      const pins_test::P4rtPortId& sut_port) const;
+
  private:
   MirrorTestbedP4rtPortIdMap(
       absl::flat_hash_map<pins_test::P4rtPortId, pins_test::P4rtPortId>

dvaas/port_id_map_test.cc

@@ -43,10 +43,17 @@ TEST(MirrorTestbedP4rtPortIdMap,
       MirrorTestbedP4rtPortIdMap::CreateFromControlSwitchToSutPortMap(
           {{port_1, port_2}}));
 
+  // Control -> SUT.
   ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_2),
               StatusIs(absl::StatusCode::kNotFound));
   ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_3),
               StatusIs(absl::StatusCode::kNotFound));
+
+  // SUT -> Control.
+  ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_1),
+              StatusIs(absl::StatusCode::kNotFound));
+  ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_3),
+              StatusIs(absl::StatusCode::kNotFound));
 }
 
 TEST(MirrorTestbedP4rtPortIdMap,
@@ -79,10 +86,17 @@ TEST(MirrorTestbedP4rtPortIdMap,
       MirrorTestbedP4rtPortIdMap::CreateFromSutToControlSwitchPortMap(
           {{port_1, port_2}}));
 
+  // Control -> SUT.
   ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_1),
               StatusIs(absl::StatusCode::kNotFound));
   ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_3),
               StatusIs(absl::StatusCode::kNotFound));
+
+  // SUT -> Control.
+  ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_2),
+              StatusIs(absl::StatusCode::kNotFound));
+  ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_3),
+              StatusIs(absl::StatusCode::kNotFound));
 }
 
 TEST(MirrorTestbedP4rtPortIdMap,
@@ -101,32 +115,7 @@ TEST(MirrorTestbedP4rtPortIdMap,
               StatusIs(absl::StatusCode::kInvalidArgument));
 }
 
-TEST(MirrorTestbedP4rtPortIdMap,
-     RetrunsCorrectSutPortGivenControlPortGivenControlToSutMapping) {
-  ASSERT_OK_AND_ASSIGN(const auto port_1,
-                       P4rtPortId::MakeFromP4rtEncoding("1"));
-  ASSERT_OK_AND_ASSIGN(const auto port_2,
-                       P4rtPortId::MakeFromP4rtEncoding("2"));
-  ASSERT_OK_AND_ASSIGN(const auto port_3,
-                       P4rtPortId::MakeFromP4rtEncoding("3"));
-  ASSERT_OK_AND_ASSIGN(const auto port_4,
-                       P4rtPortId::MakeFromP4rtEncoding("4"));
-
-  ASSERT_OK_AND_ASSIGN(
-      const auto port_id_map,
-      MirrorTestbedP4rtPortIdMap::CreateFromControlSwitchToSutPortMap({
-          {port_1, port_2},
-          {port_3, port_4},
-      }));
-
-  ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_1),
-              IsOkAndHolds(Eq(port_2)));
-  ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_3),
-              IsOkAndHolds(Eq(port_4)));
-}
-
-TEST(MirrorTestbedP4rtPortIdMap,
-     RetrunsCorrectSutPortGivenControlPortGivenSutToControlMapping) {
+TEST(MirrorTestbedP4rtPortIdMap, ReturnsCorrectPortGivenSutToControlMapping) {
   ASSERT_OK_AND_ASSIGN(const auto port_1,
                        P4rtPortId::MakeFromP4rtEncoding("1"));
   ASSERT_OK_AND_ASSIGN(const auto port_2,
@@ -143,23 +132,37 @@ TEST(MirrorTestbedP4rtPortIdMap,
           {port_3, port_4},
       }));
 
+  // Control -> SUT.
   ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_2),
               IsOkAndHolds(Eq(port_1)));
   ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_4),
               IsOkAndHolds(Eq(port_3)));
+
+  // SUT -> Control.
+  ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_1),
+              IsOkAndHolds(Eq(port_2)));
+  ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_3),
+              IsOkAndHolds(Eq(port_4)));
 }
 
-TEST(MirrorTestbedP4rtPortIdMap, RetrunsCorrectSutPortWithImplicitIdentityMap) {
+TEST(MirrorTestbedP4rtPortIdMap, ReturnsCorrectPortWithImplicitIdentityMap) {
   const auto port_id_map = MirrorTestbedP4rtPortIdMap::CreateIdentityMap();
   ASSERT_OK_AND_ASSIGN(const auto port_1,
                        P4rtPortId::MakeFromP4rtEncoding("1"));
   ASSERT_OK_AND_ASSIGN(const auto port_2,
                        P4rtPortId::MakeFromP4rtEncoding("2"));
 
+  // Control -> SUT.
   ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_1),
               IsOkAndHolds(Eq(port_1)));
   ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_2),
               IsOkAndHolds(Eq(port_2)));
+
+  // SUT -> Control.
+  ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_1),
+              IsOkAndHolds(Eq(port_1)));
+  ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_2),
+              IsOkAndHolds(Eq(port_2)));
 }
 
 }  // namespace

dvaas/test_run_validation.cc

@@ -99,11 +99,13 @@ bool CompareSwitchOutputs(SwitchOutput actual_output,
     return false;
   }
 
-  if (actual_output.packet_ins_size() != expected_output.packet_ins_size()) {
-    *listener << "has mismatched number of packet ins (actual: "
-              << actual_output.packet_ins_size()
-              << " expected: " << expected_output.packet_ins_size() << ")\n";
-    return false;
+  if (!params.treat_expected_and_actual_outputs_as_having_no_packet_ins) {
+    if (actual_output.packet_ins_size() != expected_output.packet_ins_size()) {
+      *listener << "has mismatched number of packet ins (actual: "
+                << actual_output.packet_ins_size()
+                << " expected: " << expected_output.packet_ins_size() << ")\n";
+      return false;
+    }
   }
 
   std::sort(actual_output.mutable_packets()->pointer_begin(),
@@ -139,58 +141,63 @@ bool CompareSwitchOutputs(SwitchOutput actual_output,
     }
   }
 
-  for (int i = 0; i < expected_output.packet_ins_size(); ++i) {
-    const PacketIn& actual_packet_in = actual_output.packet_ins(i);
-    const PacketIn& expected_packet_in = expected_output.packet_ins(i);
-
-    MessageDifferencer differ;
-    for (auto* field : params.ignored_packetlib_fields)
-      differ.IgnoreField(field);
-    std::string diff;
-    differ.ReportDifferencesToString(&diff);
-    if (!differ.Compare(expected_packet_in.parsed(),
-                        actual_packet_in.parsed())) {
-      *listener << "has packet in " << i
-                << " with mismatched header fields:\n  " << Indent(2, diff);
-      return false;
-    }
-
-    // Check that received packet in has desired metadata (except for ignored
-    // metadata).
-    for (const auto& expected_metadata : expected_packet_in.metadata()) {
-      if (params.ignored_packet_in_metadata.contains(expected_metadata.name()))
-        continue;
-
-      std::optional<pdpi::IrPacketMetadata> actual_metadata =
-          GetPacketInMetadataByName(actual_packet_in, expected_metadata.name());
-      if (!actual_metadata.has_value()) {
-        *listener << "has packet in " << i << " with missing metadata field '"
-                  << expected_metadata.name() << "':\n  " << Indent(2, diff);
-        return false;
-      }
-
-      if (!differ.Compare(expected_metadata.value(),
-                          actual_metadata->value())) {
+  if (!params.treat_expected_and_actual_outputs_as_having_no_packet_ins) {
+    for (int i = 0; i < expected_output.packet_ins_size(); ++i) {
+      const PacketIn& actual_packet_in = actual_output.packet_ins(i);
+      const PacketIn& expected_packet_in = expected_output.packet_ins(i);
+
+      MessageDifferencer differ;
+      for (auto* field : params.ignored_packetlib_fields)
+        differ.IgnoreField(field);
+      std::string diff;
+      differ.ReportDifferencesToString(&diff);
+      if (!differ.Compare(expected_packet_in.parsed(),
+                          actual_packet_in.parsed())) {
         *listener << "has packet in " << i
-                  << " with mismatched value for metadata field '"
-                  << expected_metadata.name() << "':\n  " << Indent(2, diff);
+                  << " with mismatched header fields:\n  " << Indent(2, diff);
         return false;
       }
-    }
 
-    // Check that received packet in does not have additional metadata (except
-    // for ignored metadata).
-    for (const auto& actual_metadata : actual_packet_in.metadata()) {
-      if (params.ignored_packet_in_metadata.contains(actual_metadata.name()))
-        continue;
+      // Check that received packet in has desired metadata (except for ignored
+      // metadata).
+      for (const auto& expected_metadata : expected_packet_in.metadata()) {
+        if (params.ignored_packet_in_metadata.contains(
+                expected_metadata.name()))
+          continue;
+
+        std::optional<pdpi::IrPacketMetadata> actual_metadata =
+            GetPacketInMetadataByName(actual_packet_in,
+                                      expected_metadata.name());
+        if (!actual_metadata.has_value()) {
+          *listener << "has packet in " << i << " with missing metadata field '"
+                    << expected_metadata.name() << "':\n  " << Indent(2, diff);
+          return false;
+        }
+
+        if (!differ.Compare(expected_metadata.value(),
+                            actual_metadata->value())) {
+          *listener << "has packet in " << i
+                    << " with mismatched value for metadata field '"
+                    << expected_metadata.name() << "':\n  " << Indent(2, diff);
+          return false;
+        }
+      }
 
-      std::optional<pdpi::IrPacketMetadata> expected_metadata =
-          GetPacketInMetadataByName(expected_packet_in, actual_metadata.name());
-      if (!expected_metadata.has_value()) {
-        *listener << "has packet in " << i
-                  << " with additional metadata field '"
-                  << actual_metadata.name() << "':\n  " << Indent(2, diff);
-        return false;
+      // Check that received packet in does not have additional metadata (except
+      // for ignored metadata).
+      for (const auto& actual_metadata : actual_packet_in.metadata()) {
+        if (params.ignored_packet_in_metadata.contains(actual_metadata.name()))
+          continue;
+
+        std::optional<pdpi::IrPacketMetadata> expected_metadata =
+            GetPacketInMetadataByName(expected_packet_in,
+                                      actual_metadata.name());
+        if (!expected_metadata.has_value()) {
+          *listener << "has packet in " << i
+                    << " with additional metadata field '"
+                    << actual_metadata.name() << "':\n  " << Indent(2, diff);
+          return false;
+        }
       }
     }
   }
@@ -370,6 +377,9 @@ PacketTestValidationResult ValidateTestRun(
                       }),
         ")");
   }
+  if (diff_params.treat_expected_and_actual_outputs_as_having_no_packet_ins) {
+    absl::StrAppend(&expectation, "\n          (ignoring packet-ins)");
+  }
   std::string actual = DescribeActual(test_run.test_vector().input(),
                                       actual_output_characterization);
   if (actual_matches_expected) {

dvaas/test_run_validation.h

@@ -43,6 +43,14 @@ struct SwitchOutputDiffParams {
   // in `ignored_metadata_for_validation`, the field is ignored during
   // comparison.
   absl::flat_hash_set<std::string> ignored_packet_in_metadata;
+
+  // If true, packet-ins (punted packets) are ignored while comparing expected
+  // and actual output. Note that packet-ins are completely ignored on both
+  // sides. Effectively the expected and actual outputs are compared assuming
+  // neither has any packet-ins.
+  // TODO: Remove and replace with
+  // `ignore_missing_packet_ins_in_actual_output`.
+  bool treat_expected_and_actual_outputs_as_having_no_packet_ins = false;
 };
 
 // Validates the given `test_vector` using the parameters in `params` and