From 5092ead97ade9dc4157e0e6224238f2d879c03b5 Mon Sep 17 00:00:00 2001
From: divyagayathri-hcl <159437886+divyagayathri-hcl@users.noreply.github.com>
Date: Thu, 5 Dec 2024 03:20:38 +0000
Subject: [PATCH 01/10] [Thinkit] Add test case checking that no packets reach
 CPU in pristine state. (#788)

Co-authored-by: smolkaj <smolkaj@google.com>
---
 tests/qos/BUILD.bazel      |  15 +-
 tests/qos/cpu_qos_test.cc  | 311 +++++++++++++++++++++++++++++++++++--
 tests/qos/gnmi_parsers.cc  |   2 +-
 tests/qos/gnmi_parsers.h   |   2 +-
 tests/qos/openconfig.proto |  85 ----------
 5 files changed, 304 insertions(+), 111 deletions(-)
 delete mode 100644 tests/qos/openconfig.proto

diff --git a/tests/qos/BUILD.bazel b/tests/qos/BUILD.bazel
index 5b560947..8765bc91 100644
--- a/tests/qos/BUILD.bazel
+++ b/tests/qos/BUILD.bazel
@@ -38,6 +38,7 @@ cc_library(
         "//gutil:testing",
         "//lib:ixia_helper",
         "//lib/gnmi:gnmi_helper",
+        "//lib/gnmi:openconfig_cc_proto",
         "//lib/p4rt:packet_listener",
         "//lib/validator:validator_lib",
         "//p4_pdpi:ir",
@@ -59,9 +60,11 @@ cc_library(
         "//thinkit:mirror_testbed_fixture",
         "//thinkit:switch",
         "//thinkit/proto:generic_testbed_cc_proto",
+        "@com_github_gnmi//proto/gnmi:gnmi_cc_proto",
         "@com_github_google_glog//:glog",
         "@com_github_nlohmann_json//:nlohmann_json",
         "@com_github_p4lang_p4runtime//:p4info_cc_proto",
+        "@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
         "@com_google_absl//absl/cleanup",
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/container:flat_hash_set",
@@ -72,6 +75,7 @@ cc_library(
         "@com_google_absl//absl/time",
         "@com_google_absl//absl/types:optional",
         "@com_google_googletest//:gtest",
+        "@com_google_protobuf//:protobuf",
     ],
     alwayslink = True,
 )
@@ -99,8 +103,8 @@ cc_library(
     srcs = ["gnmi_parsers.cc"],
     hdrs = ["gnmi_parsers.h"],
     deps = [
-        ":openconfig_cc_proto",
         "//gutil:overload",
+        "//lib/gnmi:openconfig_cc_proto",
         "//p4_pdpi/netaddr:ipv4_address",
         "//p4_pdpi/netaddr:ipv6_address",
         "@com_google_absl//absl/status",
@@ -128,12 +132,3 @@ cmd_diff_test(
     tools = [":gnmi_parsers_test_runner"],
 )
 
-proto_library(
-    name = "openconfig_proto",
-    srcs = ["openconfig.proto"],
-)
-
-cc_proto_library(
-    name = "openconfig_cc_proto",
-    deps = [":openconfig_proto"],
-)
diff --git a/tests/qos/cpu_qos_test.cc b/tests/qos/cpu_qos_test.cc
index a2219b14..5daaf5ee 100644
--- a/tests/qos/cpu_qos_test.cc
+++ b/tests/qos/cpu_qos_test.cc
@@ -37,6 +37,7 @@
 #include "absl/types/optional.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/proto.h"
@@ -46,10 +47,12 @@
 #include "gutil/testing.h"
 #include "include/nlohmann/json.hpp"
 #include "lib/gnmi/gnmi_helper.h"
+#include "lib/gnmi/openconfig.pb.h"
 #include "lib/ixia_helper.h"
 #include "lib/p4rt/packet_listener.h"
 #include "lib/validator/validator_lib.h"
 #include "p4/config/v1/p4info.pb.h"
+#include "p4/v1/p4runtime.pb.h"
 #include "p4_pdpi/ir.h"
 #include "p4_pdpi/netaddr/ipv4_address.h"
 #include "p4_pdpi/netaddr/ipv6_address.h"
@@ -59,6 +62,7 @@
 #include "p4_pdpi/packetlib/packetlib.pb.h"
 #include "p4_pdpi/pd.h"
 #include "p4_pdpi/string_encodings/decimal_string.h"
+#include "proto/gnmi/gnmi.pb.h"
 #include "sai_p4/instantiations/google/sai_p4info.h"
 #include "sai_p4/instantiations/google/sai_pd.pb.h"
 #include "tests/forwarding/util.h"
@@ -217,6 +221,16 @@ absl::Status SetPortSpeed(const std::string &port_speed,
   return absl::OkStatus();
 }
 
+absl::Status SetPortMtu(int port_mtu, const std::string &interface_name,
+                        gnmi::gNMI::StubInterface &gnmi_stub) {
+  std::string config_path = absl::StrCat(
+      "interfaces/interface[name=", interface_name, "]/config/mtu");
+  std::string value = absl::StrCat("{\"config:mtu\":", port_mtu, "}");
+  RETURN_IF_ERROR(pins_test::SetGnmiConfigPath(&gnmi_stub, config_path,
+                                               GnmiSetType::kUpdate, value));
+  return absl::OkStatus();
+}
+
 absl::StatusOr<bool> CheckLinkUp(const std::string &iface,
                                  gnmi::gNMI::StubInterface &gnmi_stub) {
   std::string oper_status_state_path =
@@ -374,7 +388,6 @@ absl::StatusOr<p4::v1::TableEntry> MakeRouterInterface(
 // Purpose: Verify that P4Runtime per-entry ACL counters increment.
 TEST_P(CpuQosTestWithoutIxia, PerEntryAclCounterIncrementsWhenEntryIsHit) {
   LOG(INFO) << "-- START OF TEST ---------------------------------------------";
-  Testbed().Environment().SetTestCaseID("cfd7e8aa-6521-4683-9c07-038ea146934d");
 
   // Setup: the testbed consists of a SUT connected to a control device
   // that allows us to send and receive packets to/from the SUT.
@@ -505,10 +518,276 @@ TEST_P(CpuQosTestWithoutIxia, PerEntryAclCounterIncrementsWhenEntryIsHit) {
   LOG(INFO) << "-- END OF TEST -----------------------------------------------";
 }
 
+// Returns vector of packets for which we will test that the packet does not
+// reach the CPU (when we haven't explicitly configure the switch otherwise).
+absl::StatusOr<std::vector<packetlib::Packet>>
+TestPacketsThatShouldNotGetPunted() {
+  std::vector<packetlib::Packet> packets;
+
+  // IPv4/6 packets with low TTLs.
+  // TODO: TTL 0/1 packets currently *do* make it to the CPU by
+  // default on some of our targets, so we exclude them here for now.
+  for (int ttl : {/*0, 1,*/ 2, 3}) {
+    ASSIGN_OR_RETURN(packets.emplace_back(),
+                     gutil::ParseTextProto<packetlib::Packet>(absl::Substitute(
+                         R"pb(
+                           headers {
+                             ethernet_header {
+                               ethernet_destination: "00:01:02:02:02:02"
+                               ethernet_source: "00:01:02:03:04:05"
+                               ethertype: "0x0800"
+                             }
+                           }
+                           headers {
+                             ipv4_header {
+                               dscp: "0x00"
+                               ecn: "0x0"
+                               identification: "0xa3cd"
+                               flags: "0x0"
+                               fragment_offset: "0x0000"
+                               ttl: "$0"
+                               protocol: "0x05"
+                               ipv4_source: "10.0.0.2"
+                               ipv4_destination: "10.0.0.3"
+                             }
+                           }
+                           payload: "IPv4 packet with TTL $0."
+                         )pb",
+                         packetlib::IpTtl(ttl))));
+    ASSIGN_OR_RETURN(
+        packets.emplace_back(),
+        gutil::ParseTextProto<packetlib::Packet>(absl::Substitute(
+            R"pb(
+              headers {
+                ethernet_header {
+                  ethernet_destination: "00:01:02:02:02:02"
+                  ethernet_source: "00:01:02:03:04:05"
+                  ethertype: "0x86dd"
+                }
+              }
+              headers {
+                ipv6_header {
+                  dscp: "0x00"
+                  ecn: "0x0"
+                  flow_label: "0x00000"
+                  next_header: "0xfd"  # Reserved for experimentation.
+                  hop_limit: "$0"
+                  ipv6_source: "2001:db8:0:12::1"
+                  ipv6_destination: "2001:db8:0:12::2"
+                }
+              }
+              payload: "IPv6 packet with TTL $0."
+            )pb",
+            packetlib::IpTtl(ttl))));
+  }
+
+  // Ethernet broadcast packets (destination MAC ff:ff:ff:ff:ff:ff).
+  ASSIGN_OR_RETURN(
+      packets.emplace_back(),
+      gutil::ParseTextProto<packetlib::Packet>(
+          R"pb(
+            headers {
+              ethernet_header {
+                ethernet_destination: "ff:ff:ff:ff:ff:ff"
+                ethernet_source: "00:01:02:03:04:05"
+                # This means size(payload) = 0xff bytes = 255 bytes.
+                ethertype: "0x00ff"
+              }
+            }
+            payload: "Ethernet broadcast packet."
+          )pb"));
+  ASSIGN_OR_RETURN(packets.emplace_back(),
+                   gutil::ParseTextProto<packetlib::Packet>(
+                       R"pb(
+                         headers {
+                           ethernet_header {
+                             ethernet_destination: "ff:ff:ff:ff:ff:ff"
+                             ethernet_source: "00:11:22:33:44:55"
+                             ethertype: "0x0806"
+                           }
+                         }
+                         headers {
+                           arp_header {
+                             hardware_type: "0x0001"
+                             protocol_type: "0x0800"
+                             hardware_length: "0x06"
+                             protocol_length: "0x04"
+                             operation: "0x0001"
+                             sender_hardware_address: "00:11:22:33:44:55"
+                             sender_protocol_address: "10.0.0.1"
+                             target_hardware_address: "00:00:00:00:00:00"
+                             target_protocol_address: "10.0.0.2"
+                           }
+                         }
+                         payload: "ARP broadcast packet."
+                       )pb"));
+
+  // LLDP multicast packet.
+  // TODO: If packetlib starts supporting LLDP, we can replace this
+  // LLDP packet hex dump with a readable protobuf. For now, we can verify that
+  // this is indeed a valid LLDP packet using, e.g., https://hpd.gasmi.net/.
+  static constexpr absl::string_view kLldpPacketHexDump =
+      "0180c200000ef40304321f6688cc02070402320af046030404073235330602007808266a"
+      "753166326d3168342e6d747631352e6e65742e676f6f676c652e636f6d3a62702d342f36"
+      "31100c05010af0460302000000fd000a1e6a753166326d3168342e6d747631352e6e6574"
+      "2e676f6f676c652e636f6dfe0c001a11041666534220c811b3fe05001a1105920000";
+  packetlib::Packet packet =
+      packetlib::ParsePacket(absl::HexStringToBytes(kLldpPacketHexDump));
+  if (packet.headers_size() < 1 || !packet.headers(0).has_ethernet_header()) {
+    return gutil::InternalErrorBuilder();
+  }
+  packet.mutable_headers(0)
+      ->mutable_ethernet_header()
+      ->set_ethernet_destination("01:80:C2:00:00:0E");  // LLDP multicast.
+  packets.push_back(packet);
+
+  // Post-process packets to ensure they are valid.
+  for (auto &packet : packets) {
+    RETURN_IF_ERROR(packetlib::PadPacketToMinimumSize(packet).status());
+    RETURN_IF_ERROR(packetlib::UpdateAllComputedFields(packet).status());
+    LOG(INFO) << packet.DebugString();  // TODO: remove.
+  }
+  return packets;
+}
+
+// Queries via gNMI, parses, and returns as a proto the gNMI path
+// `qos/interfaces/interface[interface-id=CPU]/output/queues`, which contains
+// the state of all CPU queue counters.
+absl::StatusOr<openconfig::QueuesByName> GetCpuQueueStateViaGnmi(
+    gnmi::gNMI::StubInterface &gnmi_stub) {
+  ASSIGN_OR_RETURN(
+      std::string queues_json,
+      GetGnmiStatePathInfo(
+          &gnmi_stub,
+          "qos/interfaces/interface[interface-id=CPU]/output/queues",
+          "openconfig-qos:queues"));
+
+  google::protobuf::util::JsonParseOptions options;
+  options.ignore_unknown_fields = true;
+  openconfig::Queues queues_proto;
+  RETURN_IF_ERROR(
+      gutil::ToAbslStatus(google::protobuf::util::JsonStringToMessage(
+          queues_json, &queues_proto, options)));
+
+  // Convert `Queues` to `QueuesByName`, which is equivalent but more convenient
+  // for diffing.
+  openconfig::QueuesByName queues_by_name;
+  for (auto &queue : queues_proto.queues()) {
+    queues_by_name.mutable_queues()->insert({queue.name(), queue.state()});
+  }
+
+  return queues_by_name;
+}
+
+// Purpose: Verify that the CPU is protected from packets by default.
+TEST_P(CpuQosTestWithoutIxia,
+       NoUnexpectedPacketsReachCpuInPristineSwitchState) {
+  LOG(INFO) << "-- START OF TEST ---------------------------------------------";
+
+  // Setup: the testbed consists of a SUT connected to a control device
+  // that allows us to send and receive packets to/from the SUT.
+  thinkit::Switch &sut = Testbed().Sut();
+  thinkit::Switch &control_device = Testbed().ControlSwitch();
+  const P4Info &p4info = GetParam().p4info;
+  ASSERT_OK_AND_ASSIGN(const pdpi::IrP4Info ir_p4info,
+                       pdpi::CreateIrP4Info(p4info));
+
+  // Set up gNMI.
+  EXPECT_OK(Testbed().Environment().StoreTestArtifact("gnmi_config.json",
+                                                      GetParam().gnmi_config));
+  ASSERT_OK(pins_test::PushGnmiConfig(sut, GetParam().gnmi_config));
+  ASSERT_OK(pins_test::PushGnmiConfig(control_device, GetParam().gnmi_config));
+  ASSERT_OK_AND_ASSIGN(auto gnmi_stub, sut.CreateGnmiStub());
+
+  // TODO: Poll for config to be applied, links to come up instead.
+  LOG(INFO) << "Sleeping 10 seconds to wait for config to be applied/links to "
+               "come up.";
+  absl::SleepFor(absl::Seconds(10));
+
+  // Pick a link to be used for packet injection.
+  ASSERT_OK_AND_ASSIGN(SutToControlLink link_used_for_test_packets,
+                       PickSutToControlDeviceLinkThatsUp(Testbed()));
+  LOG(INFO) << "Link used to inject test packets: "
+            << link_used_for_test_packets;
+
+  // Set up P4Runtime.
+  EXPECT_OK(
+      Testbed().Environment().StoreTestArtifact("p4info.textproto", p4info));
+  ASSERT_OK_AND_ASSIGN(
+      std::unique_ptr<pdpi::P4RuntimeSession> sut_p4rt_session,
+      pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(sut, p4info));
+  ASSERT_OK_AND_ASSIGN(
+      std::unique_ptr<pdpi::P4RuntimeSession> control_p4rt_session,
+      pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(control_device,
+                                                             p4info));
+  // We install a RIF to make this test non-trivial, as all packets are dropped
+  // by default if no RIF exists (b/190736007).
+  ASSERT_OK_AND_ASSIGN(
+      p4::v1::TableEntry router_interface_entry,
+      MakeRouterInterface(
+          /*router_interface_id=*/"ingress-rif-to-workaround-b/190736007",
+          /*p4rt_port_name=*/link_used_for_test_packets.sut_port_p4rt_name,
+          // An arbitrary MAC address will do.
+          /*mac=*/netaddr::MacAddress(0x00, 0x07, 0xE9, 0x42, 0xAC, 0x28),
+          /*ir_p4info=*/ir_p4info));
+  ASSERT_OK(pdpi::InstallPiTableEntry(sut_p4rt_session.get(),
+                                      router_interface_entry));
+
+  // Extract loopback IPs from gNMI config, to avoid using them in test packets.
+  ASSERT_OK_AND_ASSIGN(std::optional<netaddr::Ipv4Address> loopback_ipv4,
+                       ParseLoopbackIpv4(GetParam().gnmi_config));
+  ASSERT_OK_AND_ASSIGN(std::optional<netaddr::Ipv6Address> loopback_ipv6,
+                       ParseLoopbackIpv6(GetParam().gnmi_config));
+
+  // Read CPU queue state prior to injecting test packets. The state should
+  // remain unchanged when we inject test packets.
+  ASSERT_OK_AND_ASSIGN(openconfig::QueuesByName initial_cpu_queue_state,
+                       GetCpuQueueStateViaGnmi(*gnmi_stub));
+
+  // Inject test packets and verify that the CPU queue state remains
+  // unchanged.
+  ASSERT_OK_AND_ASSIGN(std::vector<packetlib::Packet> test_packets,
+                       TestPacketsThatShouldNotGetPunted());
+  for (const packetlib::Packet &packet : test_packets) {
+    // Ensure we are not hitting the loopback IP, as this would be a case in
+    // which we *do* expect the packet to arrive at the CPU.
+    for (const packetlib::Header &header : packet.headers()) {
+      if (header.has_ipv4_header() && loopback_ipv4.has_value()) {
+        ASSERT_NE(header.ipv4_header().ipv4_destination(),
+                  loopback_ipv4->ToString())
+            << "TODO: Implement logic to pick non-loopback IP "
+               "address.";
+      }
+      if (header.has_ipv6_header() && loopback_ipv6.has_value()) {
+        ASSERT_NE(header.ipv6_header().ipv6_destination(),
+                  loopback_ipv6->ToString())
+            << "TODO: Implement logic to pick non-loopback IP "
+               "address.";
+      }
+    }
+
+    LOG(INFO) << "injecting test packet: " << packet.DebugString();
+    ASSERT_OK_AND_ASSIGN(std::string raw_packet,
+                         packetlib::SerializePacket(packet));
+    ASSERT_OK(pins::InjectEgressPacket(
+        /*port=*/link_used_for_test_packets.control_device_port_p4rt_name,
+        /*packet=*/raw_packet, ir_p4info, control_p4rt_session.get()));
+
+    LOG(INFO) << "Sleeping for " << kMaxQueueCounterUpdateTime
+              << " before checking for queue counter increment.";
+    absl::SleepFor(kMaxQueueCounterUpdateTime);
+    ASSERT_OK_AND_ASSIGN(openconfig::QueuesByName cpu_queue_state,
+                         GetCpuQueueStateViaGnmi(*gnmi_stub));
+    EXPECT_THAT(cpu_queue_state, EqualsProto(initial_cpu_queue_state))
+        << "for injected test packet: " << packet.DebugString();
+    initial_cpu_queue_state = cpu_queue_state;
+  }
+  LOG(INFO) << "-- END OF TEST -----------------------------------------------";
+}
+
 // Purpose: Verify DSCP-to-queue mapping for traffic to switch loopback IP.
 TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) {
   LOG(INFO) << "-- START OF TEST ---------------------------------------------";
-  Testbed().Environment().SetTestCaseID("61bb0173-0c49-4067-b15a-5c3dd7823126");
 
   // Setup: the testbed consists of a SUT connected to a control device
   // that allows us to send and receive packets to/from the SUT.
@@ -548,13 +827,15 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) {
       pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(control_device,
                                                              p4info));
   // TODO: Unless a RIF exists at the test packet ingress port,
-  // packets will be dropped. Remove this once these RIFs are set up via gNMI.
+  // packets will be dropped. Remove this once these RIFs are set up via
+  // gNMI.
   if (Testbed().Environment().MaskKnownFailures()) {
     ASSERT_OK_AND_ASSIGN(
         p4::v1::TableEntry router_interface_entry,
         MakeRouterInterface(
             /*router_interface_id=*/"ingress-rif-to-workaround-b/190736007",
-            /*p4rt_port_name=*/link_used_for_test_packets.sut_port_p4rt_name,
+            /*p4rt_port_name=*/
+            link_used_for_test_packets.sut_port_p4rt_name,
             /*mac=*/kSutMacAddress,
             /*ir_p4info=*/ir_p4info));
     ASSERT_OK(
@@ -575,10 +856,11 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) {
   ASSERT_OK_AND_ASSIGN(std::optional<netaddr::Ipv6Address> loopback_ipv6,
                        ParseLoopbackIpv6(GetParam().gnmi_config));
   ASSERT_TRUE(loopback_ipv4.has_value() || loopback_ipv6.has_value())
-      << "Expected a loopback IP to be configured via gNMI; nothing to test.";
+      << "Expected a loopback IP to be configured via gNMI; nothing to "
+         "test.";
 
-  // Verify DSCP-to-queue mapping for all DSCPs using IP test packets destined
-  // to the loopback IP(s).
+  // Verify DSCP-to-queue mapping for all DSCPs using IP test packets
+  // destined to the loopback IP(s).
   constexpr int kMaxDscp = 63;
   for (int dscp = 0; dscp <= kMaxDscp; ++dscp) {
     for (bool ipv4 : {true, false}) {
@@ -588,7 +870,8 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) {
                 << (ipv4 ? "IPv4" : "IPv6") << " packet with DSCP " << dscp;
 
       // Identify target queue for current DSCP.
-      // The algorithm for picking a queue is somewhat adhoc and PINS specific.
+      // The algorithm for picking a queue is somewhat adhoc and PINS
+      // specific.
       std::string target_queue = kDefaultQueueName;
       if (queue_name_by_ipv4_dscp.has_value()) {
         target_queue = gutil::FindOrDefault(*queue_name_by_ipv4_dscp, dscp,
@@ -608,12 +891,12 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) {
       // Inject test packet.
       ASSERT_OK_AND_ASSIGN(
           packetlib::Packet packet,
-          ipv4
-              ? MakeIpv4PacketWithDscp(/*dst_mac=*/kSutMacAddress,
-                                       /*dst_ip=*/*loopback_ipv4, /*dscp=*/dscp)
-              : MakeIpv6PacketWithDscp(/*dst_mac=*/kSutMacAddress,
-                                       /*dst_ip=*/*loopback_ipv6,
-                                       /*dscp=*/dscp));
+          ipv4 ? MakeIpv4PacketWithDscp(/*dst_mac=*/kSutMacAddress,
+                                        /*dst_ip=*/*loopback_ipv4,
+                                        /*dscp=*/dscp)
+               : MakeIpv6PacketWithDscp(/*dst_mac=*/kSutMacAddress,
+                                        /*dst_ip=*/*loopback_ipv6,
+                                        /*dscp=*/dscp));
       ASSERT_OK_AND_ASSIGN(std::string raw_packet,
                            packetlib::SerializePacket(packet));
       ASSERT_OK(pins::InjectEgressPacket(
diff --git a/tests/qos/gnmi_parsers.cc b/tests/qos/gnmi_parsers.cc
index 615e970c..0615ece6 100644
--- a/tests/qos/gnmi_parsers.cc
+++ b/tests/qos/gnmi_parsers.cc
@@ -7,9 +7,9 @@
 #include "google/protobuf/util/json_util.h"
 #include "gutil/overload.h"
 #include "gutil/status.h"
+#include "lib/gnmi/openconfig.pb.h"
 #include "p4_pdpi/netaddr/ipv4_address.h"
 #include "p4_pdpi/netaddr/ipv6_address.h"
-#include "tests/qos/openconfig.pb.h"
 
 namespace pins_test {
 
diff --git a/tests/qos/gnmi_parsers.h b/tests/qos/gnmi_parsers.h
index 217e763d..68761512 100644
--- a/tests/qos/gnmi_parsers.h
+++ b/tests/qos/gnmi_parsers.h
@@ -8,9 +8,9 @@
 
 #include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
+#include "lib/gnmi/openconfig.pb.h"
 #include "p4_pdpi/netaddr/ipv4_address.h"
 #include "p4_pdpi/netaddr/ipv6_address.h"
-#include "tests/qos/openconfig.pb.h"
 
 namespace pins_test {
 
diff --git a/tests/qos/openconfig.proto b/tests/qos/openconfig.proto
deleted file mode 100644
index 9c0473e7..00000000
--- a/tests/qos/openconfig.proto
+++ /dev/null
@@ -1,85 +0,0 @@
-// This file contains proto definitions mirroring OpenConfig YANG models,
-// allowing us to parse JSON values following these YANG models automatically
-// using proto2's json_util.
-//
-// We only model the fields we're interested in, since we can ignore other
-// fields during parsing using json_util's `ignore_unknown_fields` option.
-//
-// This file also contains proto messages that do not mirror YANG models
-// directly, but are more convenient representations, e.g. for diffing.
-
-syntax = "proto3";
-
-package pins_test.openconfig;
-
-// -- Proto messages mirroring YANG models -------------------------------------
-
-// Mirrors `container queues` in `openconfig-qos-elements.yang`.
-message Queues {
-  repeated Queue queues = 1 [json_name = "queue"];
-
-  // Mirrors `queues/queue` in `openconfig-qos-elements.yang`.
-  message Queue {
-    string name = 1;
-    State state = 2;
-
-    // Mirrors `queues/queue/state` in `openconfig-qos-elements.yang`.
-    message State {
-      string dropped_pkts = 1 [json_name = "dropped-pkts"];
-      string transmit_pkts = 2 [json_name = "transmit-pkts"];
-    }
-  }
-}
-
-// Mirrors `container interfaces` in `openconfig-interfaces.yang`.
-message Interfaces {
-  repeated Interface interfaces = 1 [json_name = "interface"];
-
-  // Mirrors `interfaces/interface` in `openconfig-interfaces.yang`.
-  message Interface {
-    string name = 1;
-    Config config = 2;
-    Subinterfaces subinterfaces = 3;
-
-    message Config {
-      string name = 1;
-      string type = 2;
-    }
-  }
-}
-
-// Mirrors `container subinterfaces` in `openconfig-interfaces.yang`.
-message Subinterfaces {
-  repeated Subinterface subinterfaces = 1 [json_name = "subinterface"];
-
-  // Mirrors `subinterfaces/subinterface` in `openconfig-interfaces.yang`.
-  message Subinterface {
-    Ip ipv4 = 1 [json_name = "openconfig-if-ip:ipv4"];
-    Ip ipv6 = 2 [json_name = "openconfig-if-ip:ipv6"];
-
-    message Ip {
-      Addresses addresses = 1;
-    }
-  }
-}
-
-// Mirror `container addreses` in `openconfig-if-ip.yang`
-message Addresses {
-  repeated Address addresses = 1 [json_name = "address"];
-
-  message Address {
-    string ip = 1;
-  }
-}
-
-// Mirrors the root of the config tree.
-message Config {
-  Interfaces interfaces = 1 [json_name = "openconfig-interfaces:interfaces"];
-}
-
-// -- Proto messages defined for convenience -----------------------------------
-
-// Equivalent to `Queues`, but results in more readable diffs.
-message QueuesByName {
-  map<string, Queues.Queue.State> queues = 1;
-}

From 21211d8ae815ecfc0f75d6b36cbb6ee52813ba5a Mon Sep 17 00:00:00 2001
From: divyagayathri-hcl <159437886+divyagayathri-hcl@users.noreply.github.com>
Date: Thu, 5 Dec 2024 03:22:25 +0000
Subject: [PATCH 02/10] [Thinkit] Add parsers to extract loopback IPs from gNMI
 config, Fix IPv4/v6 typo causing test failure, Tweak test output, Increase
 sleep time after applying gNMI config as a temporary workaround & Update port
 name for fixing test failure. (#791)

Co-authored-by: smolkaj <smolkaj@google.com>
Co-authored-by: kishanps <kishanps@google.com>
---
 tests/qos/BUILD.bazel     |   3 +
 tests/qos/cpu_qos_test.cc | 113 ++++++++++++++++++++------------------
 2 files changed, 63 insertions(+), 53 deletions(-)

diff --git a/tests/qos/BUILD.bazel b/tests/qos/BUILD.bazel
index 8765bc91..9a226fb9 100644
--- a/tests/qos/BUILD.bazel
+++ b/tests/qos/BUILD.bazel
@@ -30,7 +30,9 @@ cc_library(
         "cpu_qos_test.h",
     ],
     deps = [
+        ":gnmi_parsers",
         "//gutil:collections",
+        "//gutil:overload",
         "//gutil:proto",
         "//gutil:proto_matchers",
         "//gutil:status",
@@ -74,6 +76,7 @@ cc_library(
         "@com_google_absl//absl/strings:str_format",
         "@com_google_absl//absl/time",
         "@com_google_absl//absl/types:optional",
+        "@com_google_absl//absl/types:variant",
         "@com_google_googletest//:gtest",
         "@com_google_protobuf//:protobuf",
     ],
diff --git a/tests/qos/cpu_qos_test.cc b/tests/qos/cpu_qos_test.cc
index 5daaf5ee..89994c64 100644
--- a/tests/qos/cpu_qos_test.cc
+++ b/tests/qos/cpu_qos_test.cc
@@ -18,6 +18,7 @@
 #include <memory>
 #include <ostream>
 #include <string>
+#include <variant>
 #include <vector>
 
 #include "absl/cleanup/cleanup.h"
@@ -35,11 +36,13 @@
 #include "absl/time/clock.h"
 #include "absl/time/time.h"
 #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"
 #include "gutil/proto_matchers.h"
 #include "gutil/status.h"
@@ -66,6 +69,7 @@
 #include "sai_p4/instantiations/google/sai_p4info.h"
 #include "sai_p4/instantiations/google/sai_pd.pb.h"
 #include "tests/forwarding/util.h"
+#include "tests/qos/gnmi_parsers.h"
 #include "thinkit/control_device.h"
 #include "thinkit/generic_testbed.h"
 #include "thinkit/mirror_testbed.h"
@@ -78,6 +82,8 @@ namespace {
 using ::gutil::EqualsProto;
 using ::gutil::IsOkAndHolds;
 using ::p4::config::v1::P4Info;
+using ::testing::Contains;
+using ::testing::Not;
 
 // Size of the "frame check sequence" (FCS) that is part of Layer 2 Ethernet
 // frames.
@@ -89,6 +95,12 @@ constexpr int kFrameCheckSequenceSize = 4;
 // 10 seconds.
 constexpr absl::Duration kMaxQueueCounterUpdateTime = absl::Seconds(15);
 
+// After pushing gNMI config to a switch, the tests sleep for this duration
+// assuming that the gNMI config will have been fully applied afterwards.
+// TODO: Instead of hard-coding this time, tests should dynamically
+// poll the state of the switch to ensure config has been applied.
+constexpr absl::Duration kTimeToWaitForGnmiConfigToApply = absl::Seconds(15);
+
 struct QueueInfo {
   std::string gnmi_queue_name;      // Openconfig queue name.
   std::string p4_queue_name;        // P4 queue name.
@@ -133,7 +145,7 @@ absl::Status SetUpPuntToCPU(const netaddr::MacAddress &dmac,
             src_ip { value: "$1" mask: "255.255.255.255" }
             dst_ip { value: "$2" mask: "255.255.255.255" }
           }
-          action { trap { qos_queue: "$3" } }
+          action { acl_trap { qos_queue: "$3" } }
           priority: 1
         }
       )pb",
@@ -327,20 +339,6 @@ ParseIpv6DscpToQueueMapping(absl::string_view gnmi_config) {
   return ParseIpv4DscpToQueueMapping(gnmi_config);
 }
 
-absl::StatusOr<std::optional<netaddr::Ipv4Address>> ParseLoopbackIpv4(
-    absl::string_view gnmi_config) {
-  // TODO: Actually parse IP -- hard-coded for now.
-  return absl::nullopt;
-}
-
-absl::StatusOr<std::optional<netaddr::Ipv6Address>> ParseLoopbackIpv6(
-    absl::string_view gnmi_config) {
-  // TODO: Actually parse IP -- hard-coded for now.
-  ASSIGN_OR_RETURN(auto ip,
-                   netaddr::Ipv6Address::OfString("2607:f8b0:8096:3125::"));
-  return ip;
-}
-
 // Represents a link connecting the switch under test (SUT) to a control device.
 struct SutToControlLink {
   std::string sut_port_gnmi_name;
@@ -362,10 +360,10 @@ absl::StatusOr<SutToControlLink> PickSutToControlDeviceLinkThatsUp(
     thinkit::MirrorTestbed &testbed) {
   // TODO: Pick dynamically instead of hard-coding.
   return SutToControlLink{
-      .sut_port_gnmi_name = "Ethernet28",
-      .sut_port_p4rt_name = "516",
-      .control_device_port_gnmi_name = "Ethernet28",
-      .control_device_port_p4rt_name = "516",
+      .sut_port_gnmi_name = "Ethernet0",
+      .sut_port_p4rt_name = "1",
+      .control_device_port_gnmi_name = "Ethernet0",
+      .control_device_port_p4rt_name = "1",
   };
 }
 
@@ -645,7 +643,6 @@ TestPacketsThatShouldNotGetPunted() {
   for (auto &packet : packets) {
     RETURN_IF_ERROR(packetlib::PadPacketToMinimumSize(packet).status());
     RETURN_IF_ERROR(packetlib::UpdateAllComputedFields(packet).status());
-    LOG(INFO) << packet.DebugString();  // TODO: remove.
   }
   return packets;
 }
@@ -700,9 +697,9 @@ TEST_P(CpuQosTestWithoutIxia,
   ASSERT_OK_AND_ASSIGN(auto gnmi_stub, sut.CreateGnmiStub());
 
   // TODO: Poll for config to be applied, links to come up instead.
-  LOG(INFO) << "Sleeping 10 seconds to wait for config to be applied/links to "
-               "come up.";
-  absl::SleepFor(absl::Seconds(10));
+  LOG(INFO) << "Sleeping " << kTimeToWaitForGnmiConfigToApply
+            << " to wait for config to be applied/links to come up.";
+  absl::SleepFor(kTimeToWaitForGnmiConfigToApply);
 
   // Pick a link to be used for packet injection.
   ASSERT_OK_AND_ASSIGN(SutToControlLink link_used_for_test_packets,
@@ -734,10 +731,10 @@ TEST_P(CpuQosTestWithoutIxia,
                                       router_interface_entry));
 
   // Extract loopback IPs from gNMI config, to avoid using them in test packets.
-  ASSERT_OK_AND_ASSIGN(std::optional<netaddr::Ipv4Address> loopback_ipv4,
-                       ParseLoopbackIpv4(GetParam().gnmi_config));
-  ASSERT_OK_AND_ASSIGN(std::optional<netaddr::Ipv6Address> loopback_ipv6,
-                       ParseLoopbackIpv6(GetParam().gnmi_config));
+  using IpAddresses =
+      std::vector<std::variant<netaddr::Ipv4Address, netaddr::Ipv6Address>>;
+  ASSERT_OK_AND_ASSIGN(IpAddresses loopback_ips,
+                       ParseLoopbackIps(GetParam().gnmi_config));
 
   // Read CPU queue state prior to injecting test packets. The state should
   // remain unchanged when we inject test packets.
@@ -752,15 +749,19 @@ TEST_P(CpuQosTestWithoutIxia,
     // Ensure we are not hitting the loopback IP, as this would be a case in
     // which we *do* expect the packet to arrive at the CPU.
     for (const packetlib::Header &header : packet.headers()) {
-      if (header.has_ipv4_header() && loopback_ipv4.has_value()) {
-        ASSERT_NE(header.ipv4_header().ipv4_destination(),
-                  loopback_ipv4->ToString())
+      if (header.has_ipv4_header()) {
+        ASSERT_OK_AND_ASSIGN(auto ip_dst,
+                             netaddr::Ipv4Address::OfString(
+                                 header.ipv4_header().ipv4_destination()));
+        ASSERT_THAT(loopback_ips, Not(Contains(ip_dst)))
             << "TODO: Implement logic to pick non-loopback IP "
                "address.";
       }
-      if (header.has_ipv6_header() && loopback_ipv6.has_value()) {
-        ASSERT_NE(header.ipv6_header().ipv6_destination(),
-                  loopback_ipv6->ToString())
+      if (header.has_ipv6_header()) {
+        ASSERT_OK_AND_ASSIGN(auto ip_dst,
+                             netaddr::Ipv6Address::OfString(
+                                 header.ipv6_header().ipv6_destination()));
+        ASSERT_THAT(loopback_ips, Not(Contains(ip_dst)))
             << "TODO: Implement logic to pick non-loopback IP "
                "address.";
       }
@@ -806,9 +807,9 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) {
   ASSERT_OK(pins_test::PushGnmiConfig(control_device, GetParam().gnmi_config));
   ASSERT_OK_AND_ASSIGN(auto gnmi_stub, sut.CreateGnmiStub());
   // TODO: Poll for config to be applied, links to come up instead.
-  LOG(INFO) << "Sleeping 10 seconds to wait for config to be applied/links to "
-               "come up.";
-  absl::SleepFor(absl::Seconds(10));
+  LOG(INFO) << "Sleeping " << kTimeToWaitForGnmiConfigToApply
+            << " to wait for config to be applied/links to come up.";
+  absl::SleepFor(kTimeToWaitForGnmiConfigToApply);
 
   // Pick a link to be used for packet injection.
   ASSERT_OK_AND_ASSIGN(SutToControlLink link_used_for_test_packets,
@@ -851,11 +852,11 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) {
   const std::string kDefaultQueueName = "BE1";
 
   // Extract loopback IPs from gNMI config.
-  ASSERT_OK_AND_ASSIGN(std::optional<netaddr::Ipv4Address> loopback_ipv4,
-                       ParseLoopbackIpv4(GetParam().gnmi_config));
-  ASSERT_OK_AND_ASSIGN(std::optional<netaddr::Ipv6Address> loopback_ipv6,
-                       ParseLoopbackIpv6(GetParam().gnmi_config));
-  ASSERT_TRUE(loopback_ipv4.has_value() || loopback_ipv6.has_value())
+  using IpAddresses =
+      std::vector<std::variant<netaddr::Ipv4Address, netaddr::Ipv6Address>>;
+  ASSERT_OK_AND_ASSIGN(IpAddresses loopback_ips,
+                       ParseLoopbackIps(GetParam().gnmi_config));
+  ASSERT_TRUE(!loopback_ips.empty())
       << "Expected a loopback IP to be configured via gNMI; nothing to "
          "test.";
 
@@ -863,11 +864,15 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) {
   // destined to the loopback IP(s).
   constexpr int kMaxDscp = 63;
   for (int dscp = 0; dscp <= kMaxDscp; ++dscp) {
-    for (bool ipv4 : {true, false}) {
-      if (ipv4 && !loopback_ipv4.has_value()) continue;
-      if (!ipv4 && !loopback_ipv6.has_value()) continue;
+    for (auto &loopback_ip : loopback_ips) {
+      netaddr::Ipv4Address *loopback_ipv4 =
+          std::get_if<netaddr::Ipv4Address>(&loopback_ip);
+      netaddr::Ipv6Address *loopback_ipv6 =
+          std::get_if<netaddr::Ipv6Address>(&loopback_ip);
+      ASSERT_TRUE(loopback_ipv4 != nullptr || loopback_ipv6 != nullptr);
       LOG(INFO) << "Testing DSCP-to-queue mapping for "
-                << (ipv4 ? "IPv4" : "IPv6") << " packet with DSCP " << dscp;
+                << (loopback_ipv4 != nullptr ? "IPv4" : "IPv6")
+                << " packet with DSCP " << dscp;
 
       // Identify target queue for current DSCP.
       // The algorithm for picking a queue is somewhat adhoc and PINS
@@ -891,12 +896,13 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) {
       // Inject test packet.
       ASSERT_OK_AND_ASSIGN(
           packetlib::Packet packet,
-          ipv4 ? MakeIpv4PacketWithDscp(/*dst_mac=*/kSutMacAddress,
-                                        /*dst_ip=*/*loopback_ipv4,
-                                        /*dscp=*/dscp)
-               : MakeIpv6PacketWithDscp(/*dst_mac=*/kSutMacAddress,
-                                        /*dst_ip=*/*loopback_ipv6,
-                                        /*dscp=*/dscp));
+          loopback_ipv4 != nullptr
+              ? MakeIpv4PacketWithDscp(/*dst_mac=*/kSutMacAddress,
+                                       /*dst_ip=*/*loopback_ipv4,
+                                       /*dscp=*/dscp)
+              : MakeIpv6PacketWithDscp(/*dst_mac=*/kSutMacAddress,
+                                       /*dst_ip=*/*loopback_ipv6,
+                                       /*dscp=*/dscp));
       ASSERT_OK_AND_ASSIGN(std::string raw_packet,
                            packetlib::SerializePacket(packet));
       ASSERT_OK(pins::InjectEgressPacket(
@@ -920,8 +926,9 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) {
       EXPECT_EQ(
           CumulativeNumPacketsEnqueued(queue_counters_after_test_packet),
           CumulativeNumPacketsEnqueued(queue_counters_before_test_packet) + 1)
-          << "expected counter to increment for queue '" << target_queue
-          << "' targeted by the following test packet:\n"
+          << "Counters for queue " << target_queue
+          << " did not increment within " << kMaxQueueCounterUpdateTime
+          << " after injecting the following test packet:\n"
           << packet.DebugString()
           << "\nBefore: " << queue_counters_before_test_packet
           << "\nAfter : " << queue_counters_after_test_packet;

From c157aa6c692f00801e37ca2897fef7359162068a Mon Sep 17 00:00:00 2001
From: bibhuprasad-hcl <161687009+bibhuprasad-hcl@users.noreply.github.com>
Date: Thu, 5 Dec 2024 03:23:22 +0000
Subject: [PATCH 03/10] Enable FCS and IPV6 test. Fix test case id (#792)

Co-authored-by: Srikishen Pondicherry Shanmugam <kishanps@google.com>
---
 tests/gnmi/ethcounter_ixia_test.cc | 157 ++++++++++++++---------------
 1 file changed, 74 insertions(+), 83 deletions(-)

diff --git a/tests/gnmi/ethcounter_ixia_test.cc b/tests/gnmi/ethcounter_ixia_test.cc
index 08e279d5..eafb4654 100644
--- a/tests/gnmi/ethcounter_ixia_test.cc
+++ b/tests/gnmi/ethcounter_ixia_test.cc
@@ -564,7 +564,6 @@ absl::StatusOr<uint32_t> NameToP4Id(std::string iface,
   return std::stoul(id_str);
 }
 
-#if 0
 TEST_P(ExampleIxiaTestFixture, TestInFcsErrors) {
   LOG(INFO) << "\n\n\n\n\n\n\n\n\n\n---------- Starting TestInFcsErrors "
                "----------\n\n\n\n\n";
@@ -583,47 +582,46 @@ TEST_P(ExampleIxiaTestFixture, TestInFcsErrors) {
   absl::flat_hash_map<std::string, thinkit::InterfaceInfo> interface_info =
       generic_testbed->GetSutInterfaceInfo();
 
-  // Connect to TestTracker for test status
-  generic_testbed->Environment().SetTestCaseID(
-      "3da5c6f0-c85e-465f-9221-1e07523092d6");
-
   // Hook up to GNMI
-  ASSERT_OK_AND_ASSIGN(
-      std::unique_ptr<gnmi::gNMI::StubInterface> gnmi_stub,
-      generic_testbed->Sut().CreateGnmiStub());
+  ASSERT_OK_AND_ASSIGN(std::unique_ptr<gnmi::gNMI::StubInterface> gnmi_stub,
+                       generic_testbed->Sut().CreateGnmiStub());
 
   // go through all the ports that interface to the Ixia and set them
   // to 200GB since the Ixia ports are all 200GB.
   for (const auto &[interface, info] : interface_info) {
     if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) {
-      LOG(INFO) << "gwc: Host Interface " << interface;
+      LOG(INFO) << "Host Interface " << interface;
       EXPECT_OK(SetPortSpeed(kSpeed200GB, interface, gnmi_stub.get()));
     }
   }
 
   // Wait to let the links come up
-  absl::SleepFor(absl::Seconds(20));
+  absl::SleepFor(absl::Seconds(30));
 
   // Loop through the interface_info looking for Ixia/SUT interface pairs,
-  // checking if the link is up. we need one pair with link up for the
-  // ingress interface/IXIA bad fcs traffic generation
-  std::string ixia_interface = "";
-  std::string sut_interface = "";
+  // checking if the link is up. We need one pair with link up for the
+  // ingress interface/IXIA traffic generation.
+  ASSERT_OK_AND_ASSIGN(std::vector<IxiaLink> ready_links,
+                       GetReadyIxiaLinks(*generic_testbed, *gnmi_stub));
 
-  for (const auto &[interface, info] : interface_info) {
-    if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) {
-      auto sut_link_up = CheckLinkUp(interface, gnmi_stub.get());
-      EXPECT_TRUE(sut_link_up.ok());
-      if (sut_link_up.ok() && sut_link_up.value()) {
-        ixia_interface = info.peer_interface_name;
-        sut_interface = interface;
-        break;
+  // If links didn't come up, lets try 100GB as some testbeds have 100GB
+  // IXIA connections.
+  if (ready_links.empty()) {
+    for (const auto &[interface, info] : interface_info) {
+      if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) {
+        ASSERT_OK(SetPortSpeed(kSpeed100GB, interface, gnmi_stub.get()));
       }
     }
+    absl::SleepFor(absl::Seconds(30));
+    ASSERT_OK_AND_ASSIGN(ready_links,
+                         GetReadyIxiaLinks(*generic_testbed, *gnmi_stub));
   }
 
-  ASSERT_FALSE(ixia_interface.empty());
-  ASSERT_FALSE(sut_interface.empty());
+  ASSERT_GE(ready_links.size(), 1) << "Ixia link is not ready";
+
+  std::string ixia_interface = ready_links[0].ixia_interface;
+  std::string sut_interface = ready_links[0].sut_interface;
+  
   LOG(INFO) << "\n\nChose Ixia interface " << ixia_interface
             << " and SUT interface " << sut_interface << "\n\n";
 
@@ -694,16 +692,9 @@ TEST_P(ExampleIxiaTestFixture, TestInFcsErrors) {
 
   ASSERT_OK(ixia::StartTraffic(tref, ixref, *generic_testbed));
 
-  // Wait until 10 seconds after the traffic started
-  absl::Time t1;
-  t1 = absl::Now();
-
-  absl::Time t2;
-  while (1) {
-    t2 = absl::Now();
-    if (t2 >= t1 + absl::Seconds(10)) break;
-    absl::SleepFor(absl::Milliseconds(100));
-  }
+  // Wait until 10 (traffic) + 25 (stats update) seconds after
+  // the traffic started.
+  absl::SleepFor(absl::Seconds(35));
 
   ASSERT_OK(ixia::StopTraffic(tref, *generic_testbed));
 
@@ -750,9 +741,7 @@ TEST_P(ExampleIxiaTestFixture, TestInFcsErrors) {
   LOG(INFO) << "\n\n\n\n\n---------- Finished TestInFcsErrors "
                "----------\n\n\n\n\n\n\n\n\n\n";
 }
-#endif
 
-#if 1
 TEST_P(ExampleIxiaTestFixture, TestIPv4Pkts) {
   LOG(INFO) << "\n\n\n\n\n\n\n\n\n\n---------- Starting TestIPv4Pkts "
                "----------\n\n\n\n\n";
@@ -771,10 +760,6 @@ TEST_P(ExampleIxiaTestFixture, TestIPv4Pkts) {
   absl::flat_hash_map<std::string, thinkit::InterfaceInfo> interface_info =
       generic_testbed->GetSutInterfaceInfo();
 
-  // Connect to TestTracker for test status
-  generic_testbed->Environment().SetTestCaseID(
-      "2fac23ff-6794-4a31-8fce-a1aa76afe72e");
-
   // Hook up to GNMI
   ASSERT_OK_AND_ASSIGN(std::unique_ptr<gnmi::gNMI::StubInterface> gnmi_stub,
                        generic_testbed->Sut().CreateGnmiStub());
@@ -789,7 +774,7 @@ TEST_P(ExampleIxiaTestFixture, TestIPv4Pkts) {
   }
 
   // Wait to let the links come up
-  absl::SleepFor(absl::Seconds(20));
+  absl::SleepFor(absl::Seconds(30));
 
   // Loop through the interface_info looking for Ixia/SUT interface pairs,
   // checking if the link is up.  we need one pair with link up for the
@@ -797,7 +782,7 @@ TEST_P(ExampleIxiaTestFixture, TestIPv4Pkts) {
   ASSERT_OK_AND_ASSIGN(std::vector<IxiaLink> ready_links,
                        GetReadyIxiaLinks(*generic_testbed, *gnmi_stub));
 
-  // If links didnt come up, lets try 100GB as some testbeds have 100GB
+  // If links didn't come up, lets try 100GB as some testbeds have 100GB
   // IXIA connections.
   if (ready_links.empty()) {
     for (const auto &[interface, info] : interface_info) {
@@ -805,22 +790,32 @@ TEST_P(ExampleIxiaTestFixture, TestIPv4Pkts) {
         ASSERT_OK(SetPortSpeed(kSpeed100GB, interface, gnmi_stub.get()));
       }
     }
-    absl::SleepFor(absl::Seconds(20));
+    absl::SleepFor(absl::Seconds(30));
 
     ASSERT_OK_AND_ASSIGN(ready_links,
                          GetReadyIxiaLinks(*generic_testbed, *gnmi_stub));
   }
 
-  ASSERT_GE(ready_links.size(), 2) << "Ixia links are not ready";
+  ASSERT_GE(ready_links.size(), 1) << "Ixia link is not ready";
 
   std::string ixia_interface = ready_links[0].ixia_interface;
   std::string sut_in_interface = ready_links[0].sut_interface;
 
-  std::string sut_out_interface = ready_links[1].sut_interface;  
-
   ASSERT_FALSE(ixia_interface.empty());
   ASSERT_FALSE(sut_in_interface.empty());
-  ASSERT_FALSE(sut_out_interface.empty());
+
+  // Now loop through again and pick an egress interface. This one doesn't
+  // have to be up, just a different interface.
+  std::string sut_out_interface = "";
+
+  for (const auto &[interface, info] : interface_info) {
+    if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) {
+      if (interface != sut_in_interface) {
+        sut_out_interface = interface;
+        break;
+      }
+    }
+  }
 
   // Look up the port numbers for the ingress and egress interfaces
   ASSERT_OK_AND_ASSIGN(uint32_t in_id,
@@ -1028,7 +1023,6 @@ TEST_P(ExampleIxiaTestFixture, TestIPv4Pkts) {
   LOG(INFO) << "\n\n\n\n\n---------- Finished TestIPv4Pkts "
                "----------\n\n\n\n\n\n\n\n\n\n";
 }
-#endif
 
 #if 0
 TEST_P(ExampleIxiaTestFixture, TestOutDiscards) {
@@ -1049,10 +1043,6 @@ TEST_P(ExampleIxiaTestFixture, TestOutDiscards) {
   absl::flat_hash_map<std::string, thinkit::InterfaceInfo> interface_info =
       generic_testbed->GetSutInterfaceInfo();
 
-  // Connect to TestTracker for test status
-  generic_testbed->Environment().SetTestCaseID(
-      "0539b74c-656f-4a0c-aa8f-39d7721520ad");
-
   // Hook up to GNMI
   ASSERT_OK_AND_ASSIGN(auto gnmi_stub, generic_testbed->Sut().CreateGnmiStub());
 
@@ -1312,7 +1302,6 @@ TEST_P(ExampleIxiaTestFixture, TestOutDiscards) {
 }
 #endif
 
-#if 0
 TEST_P(ExampleIxiaTestFixture, TestIPv6Pkts) {
   LOG(INFO) << "\n\n\n\n\n\n\n\n\n\n---------- Starting TestIPv6Pkts "
                "----------\n\n\n\n\n";
@@ -1331,15 +1320,11 @@ TEST_P(ExampleIxiaTestFixture, TestIPv6Pkts) {
   absl::flat_hash_map<std::string, thinkit::InterfaceInfo> interface_info =
       generic_testbed->GetSutInterfaceInfo();
 
-  // Connect to TestTracker for test status
-  generic_testbed->Environment().SetTestCaseID(
-      "3f3859b8-7602-479c-a2ad-ab964ded694b");
-
   // Hook up to GNMI
   ASSERT_OK_AND_ASSIGN(auto gnmi_stub, generic_testbed->Sut().CreateGnmiStub());
 
   // go through all the ports that interface to the Ixia and set them
-  // to 100GB since the Ixia ports are all 100GB.
+  // to 200GB since the Ixia ports are all 200GB.
   for (const auto &[interface, info] : interface_info) {
     if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) {
       LOG(INFO) << "Host Interface " << interface;
@@ -1348,26 +1333,36 @@ TEST_P(ExampleIxiaTestFixture, TestIPv6Pkts) {
   }
 
   // Wait to let the links come up
-  absl::SleepFor(absl::Seconds(20));
+  absl::SleepFor(absl::Seconds(30));
 
   // Loop through the interface_info looking for Ixia/SUT interface pairs,
   // checking if the link is up.  we need one pair with link up for the
   // ingress interface/IXIA traffic generation
-  std::string ixia_interface = "";
-  std::string sut_in_interface = "";
+  ASSERT_OK_AND_ASSIGN(std::vector<IxiaLink> ready_links,
+                       GetReadyIxiaLinks(*generic_testbed, *gnmi_stub));
 
-  for (const auto &[interface, info] : interface_info) {
-    if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) {
-      auto sut_link_up = CheckLinkUp(interface, gnmi_stub.get());
-      EXPECT_TRUE(sut_link_up.ok());
-      if (sut_link_up.ok() && sut_link_up.value()) {
-        ixia_interface = info.peer_interface_name;
-        sut_in_interface = interface;
-        break;
+  // If links didn't come up, lets try 100GB as some testbeds have 100GB
+  // IXIA connections.
+  if (ready_links.empty()) {
+    for (const auto &[interface, info] : interface_info) {
+      if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) {
+        ASSERT_OK(SetPortSpeed(kSpeed100GB, interface, gnmi_stub.get()));
       }
     }
+    absl::SleepFor(absl::Seconds(30));
+
+    ASSERT_OK_AND_ASSIGN(ready_links,
+                         GetReadyIxiaLinks(*generic_testbed, *gnmi_stub));
   }
 
+  ASSERT_GE(ready_links.size(), 1) << "Ixia link is not ready";
+  
+  std::string ixia_interface = ready_links[0].ixia_interface;
+  std::string sut_in_interface = ready_links[0].sut_interface;
+  
+  ASSERT_FALSE(ixia_interface.empty());
+  ASSERT_FALSE(sut_in_interface.empty());
+  
   // Now loop through again and pick an egress interface.  This one doesn't
   // have to be up, just a different interface.
   std::string sut_out_interface = "";
@@ -1381,10 +1376,6 @@ TEST_P(ExampleIxiaTestFixture, TestIPv6Pkts) {
     }
   }
 
-  ASSERT_FALSE(ixia_interface.empty());
-  ASSERT_FALSE(sut_in_interface.empty());
-  ASSERT_FALSE(sut_out_interface.empty());
-
   // Look up the port number for the egress interface
   ASSERT_OK_AND_ASSIGN(uint32_t in_id,
                        NameToP4Id(sut_in_interface, gnmi_stub.get()));
@@ -1562,12 +1553,17 @@ TEST_P(ExampleIxiaTestFixture, TestIPv6Pkts) {
   EXPECT_EQ(delta_out.out_ipv4_pkts, 0);
   EXPECT_GE(delta_out.in_ipv6_pkts, delta_out.out_pkts - 10);
   EXPECT_LE(delta_out.in_ipv6_pkts, delta_out.out_pkts + 10);
-  EXPECT_LE(delta_out.out_ipv6_pkts, delta_out.out_pkts + 10);
-  EXPECT_GE(delta_out.out_ipv6_pkts, delta_out.out_pkts - 10);
-  EXPECT_EQ(delta_out.out_ipv6_discarded_pkts, 0);
-  EXPECT_LE(delta_in.in_ipv6_pkts, delta_out.out_pkts + 10);
-  EXPECT_GE(delta_in.in_ipv6_pkts, delta_out.out_pkts - 10);
 
+  // TODO: Remove mask after bug is addressed.
+  if (!generic_testbed->Environment().MaskKnownFailures()) {
+    EXPECT_LE(delta_out.out_ipv6_pkts, delta_out.out_pkts + 10);
+    EXPECT_GE(delta_out.out_ipv6_pkts, delta_out.out_pkts - 10);
+    EXPECT_LE(delta_in.in_ipv6_pkts, delta_out.out_pkts + 10);
+    EXPECT_GE(delta_in.in_ipv6_pkts, delta_out.out_pkts - 10);
+  }
+
+  EXPECT_EQ(delta_out.out_ipv6_discarded_pkts, 0);
+  
   LOG(INFO) << "\n\n\n\n\n---------- Restore ----------\n\n\n\n\n";
 
   // go through all the ports that interface to the Ixia and set them
@@ -1586,7 +1582,6 @@ TEST_P(ExampleIxiaTestFixture, TestIPv6Pkts) {
   LOG(INFO) << "\n\n\n\n\n---------- Finished TestIPv6Pkts "
                "----------\n\n\n\n\n\n\n\n\n\n";
 }
-#endif
 
 #if 0
 TEST_P(ExampleIxiaTestFixture, TestCPUOutDiscards) {
@@ -1607,10 +1602,6 @@ TEST_P(ExampleIxiaTestFixture, TestCPUOutDiscards) {
   absl::flat_hash_map<std::string, thinkit::InterfaceInfo> interface_info =
       generic_testbed->GetSutInterfaceInfo();
 
-  // Connect to TestTracker for test status
-  generic_testbed->Environment().SetTestCaseID(
-      "3c8823ab-1aba-48a5-8af6-90d2f268c699");
-
   // Hook up to GNMI
   ASSERT_OK_AND_ASSIGN(auto gnmi_stub, generic_testbed->Sut().CreateGnmiStub());
 

From 977c92bc41b06fb170d70f3d09a18f7001aedd2c Mon Sep 17 00:00:00 2001
From: bibhuprasad-hcl <161687009+bibhuprasad-hcl@users.noreply.github.com>
Date: Thu, 5 Dec 2024 03:24:31 +0000
Subject: [PATCH 04/10] [Thinkit] Ixia test cleanup. Increase timeout to wait
 for config push. (#793)

Co-authored-by: Srikishen Pondicherry Shanmugam <kishanps@google.com>
---
 tests/gnmi/ethcounter_ixia_test.cc | 69 ++++++++++--------------------
 1 file changed, 22 insertions(+), 47 deletions(-)

diff --git a/tests/gnmi/ethcounter_ixia_test.cc b/tests/gnmi/ethcounter_ixia_test.cc
index eafb4654..f019cdb9 100644
--- a/tests/gnmi/ethcounter_ixia_test.cc
+++ b/tests/gnmi/ethcounter_ixia_test.cc
@@ -137,10 +137,6 @@ absl::Status TrapToCPU(thinkit::Switch &sut) {
 // specified. Set is_ipv6 to true to get the IPv6 version. Otherwise it will
 // use IPv4.
 //
-// Note: after seeing occasional problems with forwarding not working
-// and following b/190736007 and chats with @kishanps I have added
-// a RIF to the ingress port as well as one for the egress port jic.
-//
 absl::Status ForwardToEgress(uint32_t in_port, uint32_t out_port, bool is_ipv6,
                              thinkit::Switch &sut) {
   constexpr absl::string_view kVrfId = "vrf-80";
@@ -696,8 +692,6 @@ TEST_P(ExampleIxiaTestFixture, TestInFcsErrors) {
   // the traffic started.
   absl::SleepFor(absl::Seconds(35));
 
-  ASSERT_OK(ixia::StopTraffic(tref, *generic_testbed));
-
   // Re-read the same counters via GNMI from the SUT
   ASSERT_OK_AND_ASSIGN(auto final_counters,
                        ReadCounters(sut_interface, gnmi_stub.get()));
@@ -774,7 +768,7 @@ TEST_P(ExampleIxiaTestFixture, TestIPv4Pkts) {
   }
 
   // Wait to let the links come up
-  absl::SleepFor(absl::Seconds(30));
+  absl::SleepFor(absl::Seconds(60));
 
   // Loop through the interface_info looking for Ixia/SUT interface pairs,
   // checking if the link is up.  we need one pair with link up for the
@@ -929,21 +923,12 @@ TEST_P(ExampleIxiaTestFixture, TestIPv4Pkts) {
   // absl::SleepFor(absl::Seconds(120));
   ASSERT_OK(ixia::StartTraffic(full_tref, ixref, *generic_testbed));
 
-  // Wait until 10 seconds after the traffic started
+  // Wait until 10 (traffic) + 25 (stats update) seconds after
+  // the traffic started.
   absl::Time t1;
   t1 = absl::Now();
 
-  absl::Time t2;
-  while (1) {
-    t2 = absl::Now();
-    if (t2 >= t1 + absl::Seconds(10)) break;
-    absl::SleepFor(absl::Milliseconds(100));
-  }
-
-  LOG(INFO) << "Time at stop is " << t2;
-  LOG(INFO) << "Delta is " << t2 - t1;
-
-  ASSERT_OK(ixia::StopTraffic(full_tref, *generic_testbed));
+  absl::SleepFor(absl::Seconds(35));
 
   // Re-read the same counters via GNMI from the SUT
   ASSERT_OK_AND_ASSIGN(auto final_in_counters,
@@ -952,11 +937,11 @@ TEST_P(ExampleIxiaTestFixture, TestIPv4Pkts) {
                        ReadCounters(sut_out_interface, gnmi_stub.get()));
 
   // Check the time again
-  absl::Time t3 = absl::Now();
-  LOG(INFO) << "Time after statistics read is " << t3;
-  LOG(INFO) << "Delta is " << t3 - t1;
-  uint64_t seconds = ((t3 - t1) / absl::Seconds(1)) + 1;
-
+  absl::Time t2 = absl::Now();
+  LOG(INFO) << "Time after statistics read is " << t2;
+  LOG(INFO) << "Delta is " << t2 - t1;
+  uint64_t seconds = absl::ToInt64Seconds(t2 - t1);
+  
   // Display the final counters
   LOG(INFO) << "\n\nTestIPv4Pkts:\n\n"
             << "\n\nFinal Ingress Counters (" << sut_in_interface << "):\n";
@@ -990,7 +975,7 @@ TEST_P(ExampleIxiaTestFixture, TestIPv4Pkts) {
   EXPECT_GE(delta_out.out_octets, delta_out.out_pkts * kMtu - 10000);
   EXPECT_LE(delta_out.out_octets, delta_out.out_pkts * kMtu + 10000);
   EXPECT_LE(delta_out.out_unicast_pkts, delta_out.out_pkts + 10);
-  EXPECT_LE(delta_out.out_multicast_pkts, 5);
+  EXPECT_LE(delta_out.out_multicast_pkts, 10);
   EXPECT_EQ(delta_out.out_broadcast_pkts, 0);
   EXPECT_EQ(delta_out.out_errors, 0);
   EXPECT_EQ(delta_out.out_discards, 0);
@@ -1235,7 +1220,7 @@ TEST_P(ExampleIxiaTestFixture, TestOutDiscards) {
   absl::Time t3 = absl::Now();
   LOG(INFO) << "Time after statistics read is " << t3;
   LOG(INFO) << "Delta is " << t3 - t1;
-  uint64_t seconds = ((t3 - t1) / absl::Seconds(1)) + 1;
+  uint64_t seconds = absl::ToInt64Seconds(t2 - t1);
 
   // Display the final counters
   LOG(INFO) << "\n\nTestOutDiscards:\n\n"
@@ -1333,7 +1318,7 @@ TEST_P(ExampleIxiaTestFixture, TestIPv6Pkts) {
   }
 
   // Wait to let the links come up
-  absl::SleepFor(absl::Seconds(30));
+  absl::SleepFor(absl::Seconds(60));
 
   // Loop through the interface_info looking for Ixia/SUT interface pairs,
   // checking if the link is up.  we need one pair with link up for the
@@ -1403,7 +1388,7 @@ TEST_P(ExampleIxiaTestFixture, TestIPv6Pkts) {
 
   // Fetch the initial conditions for the egress interface.
   // Note: Not currently fetching the FEC mode since the gNMI get on that
-  // fails even if I populate redis first.  See b/197778604.  As a result
+  // fails even if I populate redis first. As a result
   // the link will not come up at the end of the test after I switch it
   // back to 100GB.  TBD.
   ASSERT_OK_AND_ASSIGN(auto out_initial_loopback,
@@ -1484,21 +1469,11 @@ TEST_P(ExampleIxiaTestFixture, TestIPv6Pkts) {
   // absl::SleepFor(absl::Seconds(15));
   ASSERT_OK(ixia::StartTraffic(full_tref, ixref, *generic_testbed));
 
-  // Wait until 10 seconds after the traffic started
+  // Wait until 10 (traffic) + 25 (stats update) seconds after
+  // the traffic started.
   absl::Time t1;
   t1 = absl::Now();
-
-  absl::Time t2;
-  while (1) {
-    t2 = absl::Now();
-    if (t2 >= t1 + absl::Seconds(10)) break;
-    absl::SleepFor(absl::Milliseconds(100));
-  }
-
-  LOG(INFO) << "Time at stop is " << t2;
-  LOG(INFO) << "Delta is " << t2 - t1;
-
-  ASSERT_OK(ixia::StopTraffic(full_tref, *generic_testbed));
+  absl::SleepFor(absl::Seconds(35));
 
   // Re-read the same counters via GNMI from the SUT
   ASSERT_OK_AND_ASSIGN(auto final_in_counters,
@@ -1507,10 +1482,10 @@ TEST_P(ExampleIxiaTestFixture, TestIPv6Pkts) {
                        ReadCounters(sut_out_interface, gnmi_stub.get()));
 
   // Check the time again
-  absl::Time t3 = absl::Now();
-  LOG(INFO) << "Time after statistics read is " << t3;
-  LOG(INFO) << "Delta is " << t3 - t1;
-  uint64_t seconds = ((t3 - t1) / absl::Seconds(1)) + 1;
+  absl::Time t2 = absl::Now();
+  LOG(INFO) << "Time after statistics read is " << t2;
+  LOG(INFO) << "Delta is " << t2 - t1;
+  uint64_t seconds = absl::ToInt64Seconds(t2 - t1);
 
   // Display the final counters
   LOG(INFO) << "\n\nTestIPv6Pkts:\n\n"
@@ -1546,7 +1521,7 @@ TEST_P(ExampleIxiaTestFixture, TestIPv6Pkts) {
   EXPECT_GE(delta_out.out_octets, delta_out.out_pkts * kMtu - 10000);
   EXPECT_LE(delta_out.out_unicast_pkts, delta_out.out_pkts + 10);
   EXPECT_GE(delta_out.out_unicast_pkts, delta_out.out_pkts - 10);
-  EXPECT_LE(delta_out.out_multicast_pkts, 5);
+  EXPECT_LE(delta_out.out_multicast_pkts, 10);
   EXPECT_EQ(delta_out.out_broadcast_pkts, 0);
   EXPECT_EQ(delta_out.out_errors, 0);
   EXPECT_EQ(delta_out.out_discards, 0);
@@ -1759,7 +1734,7 @@ TEST_P(ExampleIxiaTestFixture, TestCPUOutDiscards) {
   absl::Time t3 = absl::Now();
   LOG(INFO) << "Time after statistics read is " << t3;
   LOG(INFO) << "Delta is " << t3 - t1;
-  uint64_t seconds = ((t3 - t1) / absl::Seconds(1)) + 1;
+  uint64_t seconds = absl::ToInt64Seconds(t2 - t1);
 
   // Display the final counters
   LOG(INFO) << "\n\nTestCPUOutDiscards:\n\n"

From 11d9fc527e0e985c9d127b8084a7799e9bf9c623 Mon Sep 17 00:00:00 2001
From: divyagayathri-hcl <159437886+divyagayathri-hcl@users.noreply.github.com>
Date: Thu, 5 Dec 2024 03:25:23 +0000
Subject: [PATCH 05/10] [Thinkit] Extract queue configuration from gNMI config
 and test all the queues. (#797)

Co-authored-by: smolkaj <smolkaj@google.com>
Co-authored-by: kishanps <kishanps@google.com>
---
 tests/qos/BUILD.bazel     |   1 +
 tests/qos/cpu_qos_test.cc | 111 ++++++++++++++++++++++----------------
 tests/qos/cpu_qos_test.h  |   4 ++
 3 files changed, 70 insertions(+), 46 deletions(-)

diff --git a/tests/qos/BUILD.bazel b/tests/qos/BUILD.bazel
index 9a226fb9..24ac5725 100644
--- a/tests/qos/BUILD.bazel
+++ b/tests/qos/BUILD.bazel
@@ -74,6 +74,7 @@ cc_library(
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
+        "@com_google_absl//absl/synchronization",
         "@com_google_absl//absl/time",
         "@com_google_absl//absl/types:optional",
         "@com_google_absl//absl/types:variant",
diff --git a/tests/qos/cpu_qos_test.cc b/tests/qos/cpu_qos_test.cc
index 89994c64..09071f12 100644
--- a/tests/qos/cpu_qos_test.cc
+++ b/tests/qos/cpu_qos_test.cc
@@ -33,6 +33,7 @@
 #include "absl/strings/str_format.h"
 #include "absl/strings/string_view.h"
 #include "absl/strings/substitute.h"
+#include "absl/synchronization/mutex.h"
 #include "absl/time/clock.h"
 #include "absl/time/time.h"
 #include "absl/types/optional.h"
@@ -107,19 +108,70 @@ struct QueueInfo {
   int rate_packets_per_second = 0;  // Rate of packets in packets per second.
 };
 
-// TODO: Parse QueueInfo from gNMI config.
+// Extract the queue configurations from the gNMI configuration.
+// The function returns a map keyed on queue name and value
+// holds queue configuration information.
+// TODO: Need to handle exceptions cleanly for failures
+// during json parsing which can crash the test run.
+// Currently we are assuming validity of config json parameter passed into
+// the test.
 absl::StatusOr<absl::flat_hash_map<std::string, QueueInfo>>
-GetDefaultQueueInfo() {
-  return absl::flat_hash_map<std::string, QueueInfo>{
-      {"BE1", QueueInfo{"BE1", "0x2", 120}},
-      {"AF1", QueueInfo{"AF1", "0x3", 120}},
-      {"AF2", QueueInfo{"AF2", "0x4", 800}},
-      {"AF3", QueueInfo{"AF3", "0x5", 120}},
-      {"AF4", QueueInfo{"AF4", "0x6", 4000}},
-      {"LLQ1", QueueInfo{"LLQ1", "0x0", 800}},
-      {"LLQ2", QueueInfo{"LLQ2", "0x1", 800}},
-      {"NC1", QueueInfo{"NC1", "0x7", 16000}},
-  };
+ExtractQueueInfoViaGnmiConfig(absl::string_view gnmi_config) {
+  nlohmann::json config = nlohmann::json::parse(gnmi_config);
+  if (!config.is_object()) {
+    return absl::InvalidArgumentError("Could not parse gnmi configuration.");
+  }
+
+  absl::flat_hash_map<std::string, QueueInfo> queue_info_by_queue_name;
+  auto &qos_interfaces =
+      config["openconfig-qos:qos"]["interfaces"]["interface"];
+
+  std::string cpu_scheduler_policy;
+  for (auto &interface : qos_interfaces) {
+    if (interface["interface-id"].get<std::string>() == "CPU") {
+      cpu_scheduler_policy =
+          interface["output"]["scheduler-policy"]["config"]["name"]
+              .get<std::string>();
+      break;
+    }
+  }
+
+  auto &scheduler_policies =
+      config["openconfig-qos:qos"]["scheduler-policies"]["scheduler-policy"];
+  for (auto &policy : scheduler_policies) {
+    if (policy["name"].get<std::string>() == cpu_scheduler_policy) {
+      for (auto &scheduler : policy["schedulers"]["scheduler"]) {
+        std::string queue_name =
+            scheduler["inputs"]["input"][0]["config"]["queue"]
+                .get<std::string>();
+        queue_info_by_queue_name[queue_name].gnmi_queue_name = queue_name;
+        std::string peak_rate = scheduler["two-rate-three-color"]["config"]
+                                         ["google-pins-qos:pir-pkts"]
+                                             .get<std::string>();
+        if (!absl::SimpleAtoi(peak_rate, &queue_info_by_queue_name[queue_name]
+                                              .rate_packets_per_second)) {
+          return absl::InternalError(
+              absl::StrCat("Unable to parse rate as int ", peak_rate,
+                           " for queue ", queue_name));
+        }
+        LOG(INFO) << "Queue: " << queue_name
+                  << ", configured rate:" << peak_rate;
+      }
+      break;
+    }
+  }
+
+  // TODO: Remove these once P4 uses gnmi queue names
+  queue_info_by_queue_name["BE1"].p4_queue_name = "0x2";
+  queue_info_by_queue_name["AF1"].p4_queue_name = "0x3";
+  queue_info_by_queue_name["AF2"].p4_queue_name = "0x4";
+  queue_info_by_queue_name["AF3"].p4_queue_name = "0x5";
+  queue_info_by_queue_name["AF4"].p4_queue_name = "0x6";
+  queue_info_by_queue_name["LLQ1"].p4_queue_name = "0x0";
+  queue_info_by_queue_name["LLQ2"].p4_queue_name = "0x1";
+  queue_info_by_queue_name["NC1"].p4_queue_name = "0x7";
+
+  return queue_info_by_queue_name;
 }
 
 // Set up the switch to punt packets to CPU.
@@ -937,7 +989,7 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) {
   LOG(INFO) << "-- END OF TEST -----------------------------------------------";
 }
 
-TEST_P(CpuQosTestWithIxia, TestCPUQueueRateLimit) {
+TEST_P(CpuQosTestWithIxia, TestCPUQueueAssignmentAndRateLimit) {
   // Pick a testbed with an Ixia Traffic Generator.
   auto requirements =
       gutil::ParseProtoOrDie<thinkit::TestRequirements>(
@@ -955,47 +1007,18 @@ TEST_P(CpuQosTestWithIxia, TestCPUQueueRateLimit) {
 
   thinkit::Switch& sut = generic_testbed->Sut();
 
-  // Connect to TestTracker for test status.
-  if (auto &id = GetParam().test_case_id; id.has_value()) {
-    generic_testbed->Environment().SetTestCaseID(*id);
-  }
-
   // Push GNMI config.
   ASSERT_OK(pins_test::PushGnmiConfig(sut, GetParam().gnmi_config));
 
   // Hook up to GNMI.
   ASSERT_OK_AND_ASSIGN(auto gnmi_stub, sut.CreateGnmiStub());
 
-  // Get Queues
-  // TODO: Extract Queue info from config instead of hardcoded
-  // default.
-  ASSERT_OK_AND_ASSIGN(auto queues, GetDefaultQueueInfo());
-
-  // Set up P4Runtime session.
-  // TODO: Use `CreateWithP4InfoAndClearTables` cl/397193959 when
-  // its available.
-  ASSERT_OK_AND_ASSIGN(std::unique_ptr<pdpi::P4RuntimeSession> sut_p4_session,
-                       pdpi::P4RuntimeSession::Create(generic_testbed->Sut()));
-  auto clear_table_entries = absl::Cleanup(
-      [&]() { ASSERT_OK(pdpi::ClearTableEntries(sut_p4_session.get())); });
-
   // 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);
   const auto source_ip = netaddr::Ipv4Address(192, 168, 10, 1);
   const auto dest_ip = netaddr::Ipv4Address(172, 0, 0, 1);
 
-  // BE1 is guaranteed to exist in the map which is currently hardocoded
-  // and we will test for BE1 queue.
-  // TODO: When we replace hardcoding with extraction of members
-  // from the config, we need to add iteration logic to go over the configured
-  // queues.
-  QueueInfo queue_under_test = queues["BE1"];
-
-  ASSERT_OK(SetUpPuntToCPU(dest_mac, source_ip, dest_ip,
-                           queue_under_test.p4_queue_name, GetParam().p4info,
-                           *sut_p4_session));
-
   static constexpr absl::Duration kPollInterval = absl::Seconds(5);
   static constexpr absl::Duration kTotalTime = absl::Seconds(30);
   static const int kIterations = kTotalTime / kPollInterval;
@@ -1007,10 +1030,6 @@ TEST_P(CpuQosTestWithIxia, TestCPUQueueRateLimit) {
        gnmi_counters_check++) {
     absl::SleepFor(kPollInterval);
 
-    ASSERT_OK_AND_ASSIGN(
-        final_counters,
-        GetGnmiQueueCounters("CPU", queue_under_test.gnmi_queue_name,
-                             *gnmi_stub));
   }
 }
 
diff --git a/tests/qos/cpu_qos_test.h b/tests/qos/cpu_qos_test.h
index 2b5e3fdb..9d58d266 100644
--- a/tests/qos/cpu_qos_test.h
+++ b/tests/qos/cpu_qos_test.h
@@ -80,6 +80,10 @@ struct ParamsForTestsWithIxia {
   std::string gnmi_config;
   p4::config::v1::P4Info p4info;
   absl::optional<std::string> test_case_id;
+  // This is be the minimum guaranteed bandwidth for control path to Tester in
+  // the testbed. This is required to ensure the per queue rate limits to be
+  // tested are within this guaranteed end to end bandwidth.
+  int control_plane_bandwidth_bps;
 };
 
 class CpuQosTestWithIxia

From 581bbfdf5dc6bde77a92d6f134f7c200d46a7066 Mon Sep 17 00:00:00 2001
From: divyagayathri-hcl <159437886+divyagayathri-hcl@users.noreply.github.com>
Date: Thu, 5 Dec 2024 03:26:33 +0000
Subject: [PATCH 06/10] [Comb] Add test for flow rate limit & Adjust rate and
 burst. (#798)

Co-authored-by: smolkaj <smolkaj@google.com>
Co-authored-by: kishanps <kishanps@google.com>
---
 tests/qos/cpu_qos_test.cc | 310 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 307 insertions(+), 3 deletions(-)

diff --git a/tests/qos/cpu_qos_test.cc b/tests/qos/cpu_qos_test.cc
index 09071f12..11badcc3 100644
--- a/tests/qos/cpu_qos_test.cc
+++ b/tests/qos/cpu_qos_test.cc
@@ -212,6 +212,46 @@ absl::Status SetUpPuntToCPU(const netaddr::MacAddress &dmac,
   return pdpi::InstallPiTableEntries(&p4_session, ir_p4info, pi_entries);
 }
 
+// Set up the switch to punt packets to CPU with meter.
+absl::StatusOr<p4::v1::TableEntry> SetUpPuntToCPUWithRateLimit(
+    const netaddr::MacAddress &dmac, const netaddr::Ipv4Address &src_ip,
+    const netaddr::Ipv4Address &dst_ip, absl::string_view p4_queue,
+    int rate_bytes_per_second, int burst_in_bytes,
+    const p4::config::v1::P4Info &p4info, pdpi::P4RuntimeSession &p4_session) {
+  ASSIGN_OR_RETURN(auto ir_p4info, pdpi::CreateIrP4Info(p4info));
+
+  RETURN_IF_ERROR(pdpi::SetMetadataAndSetForwardingPipelineConfig(
+      &p4_session,
+      p4::v1::SetForwardingPipelineConfigRequest::RECONCILE_AND_COMMIT, p4info))
+      << "SetForwardingPipelineConfig: Failed to push P4Info: ";
+
+  // TODO (b/204954722): Remove after bug is fixed.
+  RETURN_IF_ERROR(pdpi::ClearTableEntries(&p4_session));
+
+  auto acl_entry = gutil::ParseProtoOrDie<sai::TableEntry>(absl::Substitute(
+      R"pb(
+        acl_ingress_table_entry {
+          match {
+            dst_mac { value: "$0" mask: "ff:ff:ff:ff:ff:ff" }
+            is_ipv4 { value: "0x1" }
+            src_ip { value: "$1" mask: "255.255.255.255" }
+            dst_ip { value: "$2" mask: "255.255.255.255" }
+          }
+          action { acl_trap { qos_queue: "$3" } }
+          priority: 1
+          meter_config { bytes_per_second: $4 burst_bytes: $5 }
+        }
+      )pb",
+      dmac.ToString(), src_ip.ToString(), dst_ip.ToString(), p4_queue,
+      rate_bytes_per_second, burst_in_bytes));
+
+  LOG(INFO) << "InstallPiTableEntry";
+  ASSIGN_OR_RETURN(const p4::v1::TableEntry pi_acl_entry,
+                   pdpi::PartialPdTableEntryToPiTableEntry(ir_p4info, acl_entry));
+  RETURN_IF_ERROR(pdpi::InstallPiTableEntry(&p4_session, pi_acl_entry));
+  return pi_acl_entry;
+}
+
 // These are the counters we track in these tests.
 struct QueueCounters {
   int64_t num_packets_transmitted = 0;
@@ -989,13 +1029,41 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) {
   LOG(INFO) << "-- END OF TEST -----------------------------------------------";
 }
 
-TEST_P(CpuQosTestWithIxia, TestCPUQueueAssignmentAndRateLimit) {
+// Buffering and software bottlenecks can cause
+// some amount of variance in rate measured end to end.
+// Level of tolerance for packet rate verification.
+// This could be parameterized in future if this is platform
+// dependent.
+constexpr float kTolerancePercent = 3.0;
+
+// Ixia configurations:
+// 1. Frames sent per second by Ixia.
+// 2. Total frames sent by Ixia.
+// 3. Default framesize.
+// 4. Maximum framesize.
+// 5. Minimum framesize.
+constexpr int kFramesPerSecond = 1000000;
+constexpr int kTotalFrames = 10000000;
+constexpr absl::Duration kTrafficDuration =
+    absl::Seconds(kTotalFrames / kFramesPerSecond);
+constexpr int kDefaultFrameSize = 1514;
+constexpr int kMaxFrameSize = 9000;
+constexpr int kMinFrameSize = 64;
+
+struct PacketReceiveInfo {
+  absl::Mutex mutex;
+  int num_packets_punted ABSL_GUARDED_BY(mutex) = 0;
+  absl::Time time_first_packet_punted ABSL_GUARDED_BY(mutex);
+  absl::Time time_last_packet_punted ABSL_GUARDED_BY(mutex);
+};
+
+TEST_P(CpuQosTestWithIxia, TestCPUQueueAssignmentAndQueueRateLimit) {
   // Pick a testbed with an Ixia Traffic Generator.
   auto requirements =
       gutil::ParseProtoOrDie<thinkit::TestRequirements>(
           R"pb(interface_requirements {
                  count: 1
-                 interface_mode: TRAFFIC_GENERATOR
+                 interface_modes: TRAFFIC_GENERATOR
                })pb");
 
   ASSERT_OK_AND_ASSIGN(
@@ -1005,7 +1073,9 @@ TEST_P(CpuQosTestWithIxia, TestCPUQueueAssignmentAndRateLimit) {
   ASSERT_OK(generic_testbed->Environment().StoreTestArtifact(
       "gnmi_config.txt", GetParam().gnmi_config));
 
-  thinkit::Switch& sut = generic_testbed->Sut();
+  ASSERT_GT(GetParam().control_plane_bandwidth_bps, 0);
+
+  thinkit::Switch &sut = generic_testbed->Sut();
 
   // Push GNMI config.
   ASSERT_OK(pins_test::PushGnmiConfig(sut, GetParam().gnmi_config));
@@ -1033,5 +1103,239 @@ TEST_P(CpuQosTestWithIxia, TestCPUQueueAssignmentAndRateLimit) {
   }
 }
 
+TEST_P(CpuQosTestWithIxia, TestPuntFlowRateLimitAndCounters) {
+  // Pick a testbed with an Ixia Traffic Generator.
+  auto requirements =
+      gutil::ParseProtoOrDie<thinkit::TestRequirements>(
+          R"pb(interface_requirements {
+                 count: 1
+                 interface_modes: TRAFFIC_GENERATOR
+               })pb");
+
+  ASSERT_OK_AND_ASSIGN(
+      std::unique_ptr<thinkit::GenericTestbed> generic_testbed,
+      GetParam().testbed_interface->GetTestbedWithRequirements(requirements));
+
+  ASSERT_OK(generic_testbed->Environment().StoreTestArtifact(
+      "gnmi_config.txt", GetParam().gnmi_config));
+
+  ASSERT_GT(GetParam().control_plane_bandwidth_bps, 0);
+
+  thinkit::Switch &sut = generic_testbed->Sut();
+
+  // Push GNMI config.
+  ASSERT_OK(pins_test::PushGnmiConfig(sut, GetParam().gnmi_config));
+
+  // Hook up to GNMI.
+  ASSERT_OK_AND_ASSIGN(auto gnmi_stub, sut.CreateGnmiStub());
+
+  // 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);
+  const auto source_ip = netaddr::Ipv4Address(192, 168, 10, 1);
+  const auto dest_ip = netaddr::Ipv4Address(172, 0, 0, 1);
+
+  // Go through all the ports that interface to the Ixia and set them
+  // to 100GB since the Ixia ports are all 100GB.
+  const absl::flat_hash_map<std::string, thinkit::InterfaceInfo>
+      interface_info = generic_testbed->GetSutInterfaceInfo();
+  for (const auto &[interface, info] : interface_info) {
+    if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR))
+    { 
+      ASSERT_OK(SetPortSpeed("\"openconfig-if-ethernet:SPEED_100GB\"",
+                             interface, *gnmi_stub));
+    }
+  }
+
+  // Wait to let the links come up. Switch guarantees state paths to reflect
+  // in 10s. Lets wait for a bit more.
+  absl::SleepFor(kTimeToWaitForGnmiConfigToApply);
+
+  // TODO: Move this to helper function.
+  // Loop through the interface_info looking for Ixia/SUT interface pairs,
+  // checking if the link is up.  we need one pair with link up for the
+  // ingress interface/IXIA traffic generation.
+  std::string ixia_interface;
+  std::string sut_interface;
+  bool sut_link_up = false;
+  for (const auto &[interface, info] : interface_info) {
+    if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) {
+      ASSERT_OK_AND_ASSIGN(sut_link_up, CheckLinkUp(interface, *gnmi_stub));
+      if (sut_link_up) {
+        ixia_interface = info.peer_interface_name;
+        sut_interface = interface;
+        break;
+      }
+    }
+  }
+
+  ASSERT_TRUE(sut_link_up);
+
+  // We will perform the following steps with Ixia:
+  // Set up Ixia traffic.
+  // Send Ixia traffic.
+  // Stop Ixia traffic.
+
+  ASSERT_OK_AND_ASSIGN(ixia::IxiaPortInfo ixia_port,
+                       ixia::ExtractPortInfo(ixia_interface));
+
+  ASSERT_OK_AND_ASSIGN(
+      std::string topology_ref,
+      pins_test::ixia::IxiaConnect(ixia_port.hostname, *generic_testbed));
+
+  ASSERT_OK_AND_ASSIGN(
+      std::string vport_ref,
+      pins_test::ixia::IxiaVport(topology_ref, ixia_port.card, ixia_port.port,
+                                 *generic_testbed));
+
+  ASSERT_OK_AND_ASSIGN(
+      std::string traffic_ref,
+      pins_test::ixia::IxiaSession(vport_ref, *generic_testbed));
+
+  ASSERT_OK(pins_test::ixia::SetFrameRate(traffic_ref, kFramesPerSecond,
+                                          *generic_testbed));
+
+  ASSERT_OK(pins_test::ixia::SetFrameCount(traffic_ref, kTotalFrames,
+                                           *generic_testbed));
+
+  ASSERT_OK(pins_test::ixia::SetFrameSize(traffic_ref, kMaxFrameSize,
+                                          *generic_testbed));
+
+  ASSERT_OK(pins_test::ixia::SetSrcMac(traffic_ref, source_mac.ToString(),
+                                       *generic_testbed));
+
+  ASSERT_OK(pins_test::ixia::SetDestMac(traffic_ref, dest_mac.ToString(),
+                                        *generic_testbed));
+
+  ASSERT_OK(pins_test::ixia::AppendIPv4(traffic_ref, *generic_testbed));
+
+  ASSERT_OK(pins_test::ixia::SetSrcIPv4(traffic_ref, source_ip.ToString(),
+                                        *generic_testbed));
+
+  ASSERT_OK(pins_test::ixia::SetDestIPv4(traffic_ref, dest_ip.ToString(),
+                                         *generic_testbed));
+
+  // Set up P4Runtime session.
+  ASSERT_OK_AND_ASSIGN(std::unique_ptr<pdpi::P4RuntimeSession> sut_p4_session,
+                       pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(
+                           generic_testbed->Sut(), GetParam().p4info));
+
+  // Listen for punted packets from the SUT.
+  PacketReceiveInfo packet_receive_info;
+  {
+    absl::MutexLock lock(&packet_receive_info.mutex);
+    packet_receive_info.num_packets_punted = 0;
+  }
+
+  // Get Queues.
+  ASSERT_OK_AND_ASSIGN(auto queues,
+                       ExtractQueueInfoViaGnmiConfig(GetParam().gnmi_config));
+
+  for (auto &[queue_name, queue_info] : queues) {
+    // Lets set flow rate limit to be half of queue limit so that queue limit
+    // doesnt take effect.
+    int flow_rate_limit_in_bytes_per_second =
+        (kMaxFrameSize * queue_info.rate_packets_per_second) / 2;
+
+    if (flow_rate_limit_in_bytes_per_second >
+        GetParam().control_plane_bandwidth_bps) {
+      flow_rate_limit_in_bytes_per_second =
+          GetParam().control_plane_bandwidth_bps / 2;
+    }
+
+    // TODO : Need to fix supported CPU queues. Currently, punting
+    // to queue 0 is not supported by OA in SONiC.
+    if (generic_testbed->Environment().MaskKnownFailures() &&
+        queue_info.p4_queue_name == "0x0") {
+      continue;
+    }
+
+    LOG(INFO) << "\n\n\nTesting Queue : " << queue_info.gnmi_queue_name
+              << "\n===================\n\n\n";
+
+    ASSERT_OK_AND_ASSIGN(
+        p4::v1::TableEntry pi_acl_entry,
+        SetUpPuntToCPUWithRateLimit(dest_mac, source_ip, dest_ip,
+                                    queue_info.p4_queue_name,
+                                    flow_rate_limit_in_bytes_per_second,
+                                    /*burst_in_bytes=*/kMaxFrameSize,
+                                    GetParam().p4info, *sut_p4_session));
+
+    // Reset received packet count at tester for each iteration.
+    {
+      absl::MutexLock lock(&packet_receive_info.mutex);
+      packet_receive_info.num_packets_punted = 0;
+    }
+
+    // Check that the counters are initially zero.
+    ASSERT_THAT(
+        pdpi::ReadPiCounterData(sut_p4_session.get(), pi_acl_entry),
+        IsOkAndHolds(EqualsProto(R"pb(byte_count: 0 packet_count: 0)pb")));
+
+    ASSERT_OK(pins_test::ixia::StartTraffic(traffic_ref, topology_ref,
+                                            *generic_testbed));
+
+    // Wait for Traffic to be sent.
+    absl::SleepFor(kTrafficDuration);
+
+    ASSERT_OK(pins_test::ixia::StopTraffic(traffic_ref, *generic_testbed));
+
+    // Check for counters every 5 seconds upto 30 seconds till the fetched gNMI
+    // queue counter stats match packets and bytes sent by Ixia.
+    // Check that the counters increment within kMaxQueueCounterUpdateTime.
+    absl::Time time_packet_sent = absl::Now();
+    p4::v1::CounterData counter_data;
+    do {
+      ASSERT_OK_AND_ASSIGN(
+          counter_data,
+          pdpi::ReadPiCounterData(sut_p4_session.get(), pi_acl_entry));
+    } while (counter_data.packet_count() != kTotalFrames &&
+             absl::Now() - time_packet_sent < kMaxQueueCounterUpdateTime);
+    p4::v1::CounterData expected_counter_data;
+    expected_counter_data.set_packet_count(kTotalFrames);
+    expected_counter_data.set_byte_count(static_cast<int64_t>(kMaxFrameSize) *
+                                         static_cast<int64_t>(kTotalFrames));
+    EXPECT_THAT(counter_data, EqualsProto(expected_counter_data))
+        << "Counter for the table entry given below did not match expectation "
+           "within "
+        << kMaxQueueCounterUpdateTime
+        << " after injecting the Ixia test packets via CPU queue "
+        << queue_name;
+
+    // TODO: Remove masking this failure once bug is fixed.
+    if (!generic_testbed->Environment().MaskKnownFailures()) {
+      absl::MutexLock lock(&packet_receive_info.mutex);
+
+      LOG(INFO) << "Packets received at Controller: "
+                << packet_receive_info.num_packets_punted;
+      LOG(INFO) << "Timestamp of first received packet: "
+                << packet_receive_info.time_first_packet_punted;
+      LOG(INFO) << "Timestamp of last received packet: "
+                << packet_receive_info.time_last_packet_punted;
+
+      absl::Duration duration = packet_receive_info.time_last_packet_punted -
+                                packet_receive_info.time_first_packet_punted;
+      LOG(INFO) << "Duration of packets received: " << duration;
+      LOG(INFO) << "Frame size: " << kMaxFrameSize;
+      int64_t rate_received_in_bytes_per_second = 0;
+      int64_t useconds = absl::ToInt64Microseconds(duration);
+      ASSERT_NE(useconds, 0);
+      int64_t num_bytes =
+          packet_receive_info.num_packets_punted * kMaxFrameSize;
+      LOG(INFO) << "Num bytes received: " << num_bytes;
+      rate_received_in_bytes_per_second = num_bytes * 1000000 / useconds;
+      LOG(INFO) << "Rate of packets received (bps): "
+                << rate_received_in_bytes_per_second;
+      EXPECT_LE(
+          rate_received_in_bytes_per_second,
+          flow_rate_limit_in_bytes_per_second * (1 + kTolerancePercent / 100));
+      EXPECT_GE(
+          rate_received_in_bytes_per_second,
+          flow_rate_limit_in_bytes_per_second * (1 - kTolerancePercent / 100));
+    }
+  }  // for each queue.
+
+}
+
 }  // namespace
 }  // namespace pins_test

From d2b906d382da79d24d355f5c0d86eb5f44346a9f Mon Sep 17 00:00:00 2001
From: bibhuprasad-hcl <161687009+bibhuprasad-hcl@users.noreply.github.com>
Date: Thu, 5 Dec 2024 03:27:09 +0000
Subject: [PATCH 07/10] [Thinkit] Increase tolerance for packet bytes (#799)

Co-authored-by: Srikishen Pondicherry Shanmugam <kishanps@google.com>
---
 tests/gnmi/ethcounter_ixia_test.cc | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/gnmi/ethcounter_ixia_test.cc b/tests/gnmi/ethcounter_ixia_test.cc
index f019cdb9..788b048d 100644
--- a/tests/gnmi/ethcounter_ixia_test.cc
+++ b/tests/gnmi/ethcounter_ixia_test.cc
@@ -972,8 +972,8 @@ TEST_P(ExampleIxiaTestFixture, TestIPv4Pkts) {
   // during the test to account for this.
   EXPECT_GE(delta_out.out_pkts, 90000000);
   EXPECT_LE(delta_out.out_pkts, delta_out.out_pkts + 10);
-  EXPECT_GE(delta_out.out_octets, delta_out.out_pkts * kMtu - 10000);
-  EXPECT_LE(delta_out.out_octets, delta_out.out_pkts * kMtu + 10000);
+  EXPECT_GE(delta_out.out_octets, delta_out.out_pkts * kMtu - 15000);
+  EXPECT_LE(delta_out.out_octets, delta_out.out_pkts * kMtu + 15000);
   EXPECT_LE(delta_out.out_unicast_pkts, delta_out.out_pkts + 10);
   EXPECT_LE(delta_out.out_multicast_pkts, 10);
   EXPECT_EQ(delta_out.out_broadcast_pkts, 0);
@@ -982,13 +982,13 @@ TEST_P(ExampleIxiaTestFixture, TestIPv4Pkts) {
   
   // TODO: Remove mask after bug is addressed.
   if (!generic_testbed->Environment().MaskKnownFailures()) {
+    EXPECT_LE(delta_in.in_ipv4_pkts, delta_out.out_pkts + 10);
+    EXPECT_GE(delta_in.in_ipv4_pkts, delta_out.out_pkts - 10);
     EXPECT_GE(delta_out.out_ipv4_pkts, delta_out.out_pkts - 10);
     EXPECT_LE(delta_out.out_ipv4_pkts, delta_out.out_pkts + 10);
   }
   EXPECT_EQ(delta_out.out_ipv6_pkts, 0);
   EXPECT_EQ(delta_out.out_ipv6_discarded_pkts, 0);
-  EXPECT_LE(delta_in.in_ipv4_pkts, delta_out.out_pkts + 10);
-  EXPECT_LE(delta_in.in_ipv4_pkts, delta_out.out_pkts - 10);
 
   LOG(INFO) << "\n\n\n\n\n---------- Restore ----------\n\n\n\n\n";
 
@@ -1253,8 +1253,8 @@ TEST_P(ExampleIxiaTestFixture, TestOutDiscards) {
   uint64_t expect = 90000000;
   EXPECT_GT(delta_out.out_pkts, ((expect * 35) / 100));
   EXPECT_LT(delta_out.out_pkts, ((expect * 45) / 100));
-  EXPECT_LE(delta_out.out_octets, delta_out.out_pkts * kMtu + 10000);
-  EXPECT_GE(delta_out.out_octets, delta_out.out_pkts * kMtu - 10000);
+  EXPECT_LE(delta_out.out_octets, delta_out.out_pkts * kMtu + 15000);
+  EXPECT_GE(delta_out.out_octets, delta_out.out_pkts * kMtu - 15000);
   EXPECT_LE(delta_out.out_unicast_pkts, delta_out.out_pkts + 10);
   EXPECT_GE(delta_out.out_unicast_pkts, delta_out.out_pkts - 10);
   EXPECT_LE(delta_out.out_multicast_pkts, 5);
@@ -1517,8 +1517,8 @@ TEST_P(ExampleIxiaTestFixture, TestIPv6Pkts) {
   // during the test to account for this.
   EXPECT_GE(delta_out.out_pkts, 90000000);
   EXPECT_LE(delta_out.out_pkts, 90000000 + 10);
-  EXPECT_LE(delta_out.out_octets, delta_out.out_pkts * kMtu + 10000);
-  EXPECT_GE(delta_out.out_octets, delta_out.out_pkts * kMtu - 10000);
+  EXPECT_LE(delta_out.out_octets, delta_out.out_pkts * kMtu + 15000);
+  EXPECT_GE(delta_out.out_octets, delta_out.out_pkts * kMtu - 15000);
   EXPECT_LE(delta_out.out_unicast_pkts, delta_out.out_pkts + 10);
   EXPECT_GE(delta_out.out_unicast_pkts, delta_out.out_pkts - 10);
   EXPECT_LE(delta_out.out_multicast_pkts, 10);

From fd98b847be98aa427602e4445a25be14727b0853 Mon Sep 17 00:00:00 2001
From: VSuryaprasad-hcl <159443973+VSuryaprasad-HCL@users.noreply.github.com>
Date: Thu, 5 Dec 2024 03:27:52 +0000
Subject: [PATCH 08/10] [P4_Symbolic] Extend IR proto with error codes. (#800)

Co-authored-by: kishanps <kishanps@google.com>
---
 p4_symbolic/ir/BUILD.bazel                    |  1 +
 p4_symbolic/ir/expected/basic.txt             | 48 +++++++++++++++++++
 .../ir/expected/complex_conditional.txt       | 48 +++++++++++++++++++
 p4_symbolic/ir/expected/conditional.txt       | 48 +++++++++++++++++++
 .../ir/expected/conditional_sequence.txt      | 48 +++++++++++++++++++
 .../ir/expected/default_transition.txt        | 48 +++++++++++++++++++
 .../ir/expected/extract_parser_operation.txt  | 48 +++++++++++++++++++
 p4_symbolic/ir/expected/hardcoded.txt         | 48 +++++++++++++++++++
 .../ir/expected/hex_string_transition.txt     | 48 +++++++++++++++++++
 p4_symbolic/ir/expected/reflector.txt         | 48 +++++++++++++++++++
 p4_symbolic/ir/expected/sai_parser.txt        | 48 +++++++++++++++++++
 p4_symbolic/ir/expected/set_invalid.txt       | 48 +++++++++++++++++++
 .../ir/expected/set_parser_operation.txt      | 48 +++++++++++++++++++
 p4_symbolic/ir/expected/string_optional.txt   | 48 +++++++++++++++++++
 p4_symbolic/ir/expected/table.txt             | 48 +++++++++++++++++++
 p4_symbolic/ir/expected/table_hit_1.txt       | 48 +++++++++++++++++++
 p4_symbolic/ir/expected/table_hit_2.txt       | 48 +++++++++++++++++++
 p4_symbolic/ir/ir.cc                          | 30 ++++++++++++
 p4_symbolic/ir/ir.proto                       | 10 ++++
 19 files changed, 809 insertions(+)

diff --git a/p4_symbolic/ir/BUILD.bazel b/p4_symbolic/ir/BUILD.bazel
index 3d491988..75604cb2 100644
--- a/p4_symbolic/ir/BUILD.bazel
+++ b/p4_symbolic/ir/BUILD.bazel
@@ -89,6 +89,7 @@ cc_library(
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
+        "@com_google_protobuf//:protobuf",
     ],
 )
 
diff --git a/p4_symbolic/ir/expected/basic.txt b/p4_symbolic/ir/expected/basic.txt
index a817b615..08c4b038 100644
--- a/p4_symbolic/ir/expected/basic.txt
+++ b/p4_symbolic/ir/expected/basic.txt
@@ -709,6 +709,54 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
 =====MyIngress.ipv4_lpm Entries=====
 
diff --git a/p4_symbolic/ir/expected/complex_conditional.txt b/p4_symbolic/ir/expected/complex_conditional.txt
index 16b2da87..cd6ab5e6 100644
--- a/p4_symbolic/ir/expected/complex_conditional.txt
+++ b/p4_symbolic/ir/expected/complex_conditional.txt
@@ -2100,4 +2100,52 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
diff --git a/p4_symbolic/ir/expected/conditional.txt b/p4_symbolic/ir/expected/conditional.txt
index eb4e9766..135c206c 100644
--- a/p4_symbolic/ir/expected/conditional.txt
+++ b/p4_symbolic/ir/expected/conditional.txt
@@ -732,4 +732,52 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
diff --git a/p4_symbolic/ir/expected/conditional_sequence.txt b/p4_symbolic/ir/expected/conditional_sequence.txt
index d7c57de3..a123108f 100644
--- a/p4_symbolic/ir/expected/conditional_sequence.txt
+++ b/p4_symbolic/ir/expected/conditional_sequence.txt
@@ -2369,4 +2369,52 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
diff --git a/p4_symbolic/ir/expected/default_transition.txt b/p4_symbolic/ir/expected/default_transition.txt
index c5457a1b..72ff7d14 100644
--- a/p4_symbolic/ir/expected/default_transition.txt
+++ b/p4_symbolic/ir/expected/default_transition.txt
@@ -201,4 +201,52 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
diff --git a/p4_symbolic/ir/expected/extract_parser_operation.txt b/p4_symbolic/ir/expected/extract_parser_operation.txt
index c5457a1b..72ff7d14 100644
--- a/p4_symbolic/ir/expected/extract_parser_operation.txt
+++ b/p4_symbolic/ir/expected/extract_parser_operation.txt
@@ -201,4 +201,52 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
diff --git a/p4_symbolic/ir/expected/hardcoded.txt b/p4_symbolic/ir/expected/hardcoded.txt
index 7a4a4290..4b82aba5 100644
--- a/p4_symbolic/ir/expected/hardcoded.txt
+++ b/p4_symbolic/ir/expected/hardcoded.txt
@@ -301,4 +301,52 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
diff --git a/p4_symbolic/ir/expected/hex_string_transition.txt b/p4_symbolic/ir/expected/hex_string_transition.txt
index 90beac85..103f0dc0 100644
--- a/p4_symbolic/ir/expected/hex_string_transition.txt
+++ b/p4_symbolic/ir/expected/hex_string_transition.txt
@@ -361,4 +361,52 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
diff --git a/p4_symbolic/ir/expected/reflector.txt b/p4_symbolic/ir/expected/reflector.txt
index 9b3cccca..3e7fdf55 100644
--- a/p4_symbolic/ir/expected/reflector.txt
+++ b/p4_symbolic/ir/expected/reflector.txt
@@ -220,4 +220,52 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
diff --git a/p4_symbolic/ir/expected/sai_parser.txt b/p4_symbolic/ir/expected/sai_parser.txt
index 19e8a201..419203b3 100644
--- a/p4_symbolic/ir/expected/sai_parser.txt
+++ b/p4_symbolic/ir/expected/sai_parser.txt
@@ -1656,4 +1656,52 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
diff --git a/p4_symbolic/ir/expected/set_invalid.txt b/p4_symbolic/ir/expected/set_invalid.txt
index fdcd0217..6ac196d8 100644
--- a/p4_symbolic/ir/expected/set_invalid.txt
+++ b/p4_symbolic/ir/expected/set_invalid.txt
@@ -375,4 +375,52 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
diff --git a/p4_symbolic/ir/expected/set_parser_operation.txt b/p4_symbolic/ir/expected/set_parser_operation.txt
index b595647b..038d917b 100644
--- a/p4_symbolic/ir/expected/set_parser_operation.txt
+++ b/p4_symbolic/ir/expected/set_parser_operation.txt
@@ -457,4 +457,52 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
diff --git a/p4_symbolic/ir/expected/string_optional.txt b/p4_symbolic/ir/expected/string_optional.txt
index bbb3f6c7..f626b62d 100644
--- a/p4_symbolic/ir/expected/string_optional.txt
+++ b/p4_symbolic/ir/expected/string_optional.txt
@@ -575,4 +575,52 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
diff --git a/p4_symbolic/ir/expected/table.txt b/p4_symbolic/ir/expected/table.txt
index e19cbad8..d6807c96 100644
--- a/p4_symbolic/ir/expected/table.txt
+++ b/p4_symbolic/ir/expected/table.txt
@@ -345,6 +345,54 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
 =====MyIngress.ports_exact Entries=====
 
diff --git a/p4_symbolic/ir/expected/table_hit_1.txt b/p4_symbolic/ir/expected/table_hit_1.txt
index a21d9246..18c169e9 100644
--- a/p4_symbolic/ir/expected/table_hit_1.txt
+++ b/p4_symbolic/ir/expected/table_hit_1.txt
@@ -610,4 +610,52 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
diff --git a/p4_symbolic/ir/expected/table_hit_2.txt b/p4_symbolic/ir/expected/table_hit_2.txt
index b49b0915..d8e13181 100644
--- a/p4_symbolic/ir/expected/table_hit_2.txt
+++ b/p4_symbolic/ir/expected/table_hit_2.txt
@@ -752,4 +752,52 @@ parsers {
     }
   }
 }
+errors {
+  key: "HeaderTooShort"
+  value {
+    name: "HeaderTooShort"
+    value: 4
+  }
+}
+errors {
+  key: "NoError"
+  value {
+    name: "NoError"
+  }
+}
+errors {
+  key: "NoMatch"
+  value {
+    name: "NoMatch"
+    value: 2
+  }
+}
+errors {
+  key: "PacketTooShort"
+  value {
+    name: "PacketTooShort"
+    value: 1
+  }
+}
+errors {
+  key: "ParserInvalidArgument"
+  value {
+    name: "ParserInvalidArgument"
+    value: 6
+  }
+}
+errors {
+  key: "ParserTimeout"
+  value {
+    name: "ParserTimeout"
+    value: 5
+  }
+}
+errors {
+  key: "StackOutOfBounds"
+  value {
+    name: "StackOutOfBounds"
+    value: 3
+  }
+}
 
diff --git a/p4_symbolic/ir/ir.cc b/p4_symbolic/ir/ir.cc
index 3e52de06..0519900c 100644
--- a/p4_symbolic/ir/ir.cc
+++ b/p4_symbolic/ir/ir.cc
@@ -32,6 +32,7 @@
 #include "absl/strings/str_format.h"
 #include "absl/strings/string_view.h"
 #include "absl/strings/strip.h"
+#include "google/protobuf/struct.pb.h"
 #include "gutil/status.h"
 #include "p4/config/v1/p4info.pb.h"
 #include "p4_symbolic/bmv2/bmv2.pb.h"
@@ -1020,6 +1021,29 @@ absl::StatusOr<Parser> ExtractParser(const bmv2::Parser &bmv2_parser) {
   return parser;
 }
 
+// Translate an error code definition from the BMv2 protobuf message.
+absl::StatusOr<Error> ExtractError(
+    const google::protobuf::ListValue &bmv2_error) {
+  // A BMv2 error must have 2 elements.
+  if (bmv2_error.values_size() != 2) {
+    return gutil::InvalidArgumentErrorBuilder()
+           << "Error field must contain 2 elements. Found: "
+           << bmv2_error.DebugString();
+  }
+
+  if (!bmv2_error.values(0).has_string_value() ||
+      !bmv2_error.values(1).has_number_value()) {
+    return gutil::InvalidArgumentErrorBuilder()
+           << "Error field must be [string, int]. Found: "
+           << bmv2_error.DebugString();
+  }
+
+  Error output;
+  output.set_name(bmv2_error.values(0).string_value());
+  output.set_value(bmv2_error.values(1).number_value());
+  return output;
+}
+
 }  // namespace
 
 // Main Translation function.
@@ -1213,6 +1237,12 @@ absl::StatusOr<P4Program> Bmv2AndP4infoToIr(const bmv2::P4Program &bmv2,
     }
   }
 
+  // Translate errors from BMv2.
+  for (const google::protobuf::ListValue &bmv2_error : bmv2.errors()) {
+    ASSIGN_OR_RETURN(Error error, ExtractError(bmv2_error));
+    (*output.mutable_errors())[error.name()] = std::move(error);
+  }
+
   // Create the Control Flow Graph (CFG) of the program and perform analysis for
   // optimized symbolic execution.
   ASSIGN_OR_RETURN(std::unique_ptr<ControlFlowGraph> cfg,
diff --git a/p4_symbolic/ir/ir.proto b/p4_symbolic/ir/ir.proto
index 89f61a8f..40982e5b 100644
--- a/p4_symbolic/ir/ir.proto
+++ b/p4_symbolic/ir/ir.proto
@@ -50,6 +50,8 @@ message P4Program {
   map<string, Pipeline> pipeline = 5;
   // Parsers, keyed by parser names.
   map<string, Parser> parsers = 6;
+  // Errors, keyed by error names.
+  map<string, Error> errors = 7;
 
   // TODO: If needed in the future, action calls can be added here, for
   // action calls that are not wrapped in other control constructs.
@@ -272,6 +274,14 @@ message Pipeline {
   string initial_control = 2;
 }
 
+// Defines an error code that may be used in the program.
+message Error {
+  // The name of the error.
+  string name = 1;
+  // The value of the error code.
+  uint32 value = 2;
+}
+
 // An abstract p4 statement corresponding to a top level operation within
 // an action body.
 message Statement {

From a130752ca609d176f396bccb648234654591fd71 Mon Sep 17 00:00:00 2001
From: VSuryaprasad-hcl <159443973+VSuryaprasad-HCL@users.noreply.github.com>
Date: Thu, 5 Dec 2024 03:28:29 +0000
Subject: [PATCH 09/10] [Dvaas] Allow packet_ins for custom test vectors.
 (#801)

Co-authored-by: kishanps <kishanps@google.com>
---
 dvaas/BUILD.bazel                             |   4 +-
 dvaas/dataplane_validation.cc                 |   7 +-
 dvaas/user_provided_packet_test_vector.cc     |  64 ++++-
 dvaas/user_provided_packet_test_vector.h      |  13 +-
 .../user_provided_packet_test_vector_test.cc  | 154 ++++++++++-
 ..._provided_packet_test_vector_test.expected | 258 ++++++++++++++++++
 6 files changed, 479 insertions(+), 21 deletions(-)

diff --git a/dvaas/BUILD.bazel b/dvaas/BUILD.bazel
index 2ca01581..8159b06c 100644
--- a/dvaas/BUILD.bazel
+++ b/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",
diff --git a/dvaas/dataplane_validation.cc b/dvaas/dataplane_validation.cc
index 38b12642..a8a6b860 100644
--- a/dvaas/dataplane_validation.cc
+++ b/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)));
diff --git a/dvaas/user_provided_packet_test_vector.cc b/dvaas/user_provided_packet_test_vector.cc
index a2f11d7f..8bda97d2 100644
--- a/dvaas/user_provided_packet_test_vector.cc
+++ b/dvaas/user_provided_packet_test_vector.cc
@@ -11,6 +11,8 @@
 #include "dvaas/test_vector.pb.h"
 #include "gutil/proto.h"
 #include "gutil/status.h"
+#include "p4/v1/p4runtime.pb.h"
+#include "p4_pdpi/ir.h"
 #include "p4_pdpi/packetlib/packetlib.h"
 
 namespace dvaas {
@@ -30,10 +32,43 @@ absl::StatusOr<Packet> LegitimizePacket(Packet packet) {
   return packet;
 }
 
-// Checks that the given `vector` is well-formed and if so adds it to
-// `legitimized_test_vectors_by_id`, or returns error otherwise.
+// Checks that the given `packet_in`'s metadata is well-formed and if so
+// returns it, or returns error otherwise.
+absl::Status LegitimizePacketInMetadata(const PacketIn& packet_in,
+                                        const pdpi::IrP4Info& ir_info) {
+  // Check that PacketIn metadata is valid according to P4Runtime, in particular
+  // "Section 16.1: Packet I/O".
+  pdpi::IrPacketIn ir_packet_in;
+  for (const pdpi::IrPacketMetadata& metadata : packet_in.metadata()) {
+    *ir_packet_in.add_metadata() = metadata;
+  }
+
+  // Translate IR `packet_in` into PI and translate resulting `packet_in` back
+  // into IR to validate its metadata is well-formed. If not, return an error.
+  ASSIGN_OR_RETURN(p4::v1::PacketIn pi_packet_in,
+                   pdpi::IrPacketInToPi(ir_info, ir_packet_in));
+  return absl::OkStatus();
+}
+
+absl::StatusOr<PacketIn> LegitimizePacketIn(PacketIn packet_in,
+                                            const pdpi::IrP4Info& ir_info) {
+  RETURN_IF_ERROR(
+      packetlib::UpdateMissingComputedFields(*packet_in.mutable_parsed())
+          .status());
+  RETURN_IF_ERROR(packetlib::ValidatePacket(packet_in.parsed()));
+  ASSIGN_OR_RETURN(std::string raw_packet,
+                   packetlib::RawSerializePacket(packet_in.parsed()));
+  packet_in.set_hex(absl::BytesToHexString(raw_packet));
+
+  RETURN_IF_ERROR(LegitimizePacketInMetadata(packet_in, ir_info));
+  return packet_in;
+}
+
+// Checks that the given input `vector` is well-formed using
+// `ir_info` and if so adds it to `legitimized_test_vectors_by_id`, or returns
+// error otherwise.
 absl::Status LegitimizeTestVector(
-    PacketTestVector vector,
+    PacketTestVector vector, const pdpi::IrP4Info& ir_info,
     PacketTestVectorById& legitimized_test_vectors_by_id) {
   if (vector.input().type() != SwitchInput::DATAPLANE) {
     return gutil::UnimplementedErrorBuilder()
@@ -56,9 +91,21 @@ absl::Status LegitimizeTestVector(
   for (SwitchOutput& output : *vector.mutable_acceptable_outputs()) {
     // Punted output packets are not supported for now.
     if (!output.packet_ins().empty()) {
-      return gutil::UnimplementedErrorBuilder()
-             << "TODO: support vectors expecting `packet_ins` "
-                "(punting)";
+      for (int i = 0; i < output.packet_ins().size(); ++i) {
+        PacketIn& output_packet_ins = *output.mutable_packet_ins(i);
+        ASSIGN_OR_RETURN(
+            int output_tag, ExtractTestPacketTag(output_packet_ins.parsed()),
+            _.SetPrepend() << "output packet in #" << (i + 1) << " invalid: ");
+        ASSIGN_OR_RETURN(
+            output_packet_ins, LegitimizePacketIn(output_packet_ins, ir_info),
+            _.SetPrepend() << "output packet in #" << (i + 1) << " invalid: ");
+        if (output_tag != tag) {
+          return gutil::InvalidArgumentErrorBuilder()
+                 << "mismatch of input packet in tag vs output packet in tag "
+                    "for output packet in #"
+                 << (i + 1) << ": " << tag << " vs " << output_tag;
+        }
+      }
     }
     // Legitimize forwarded output packets.
     for (int i = 0; i < output.packets().size(); ++i) {
@@ -95,11 +142,12 @@ absl::Status LegitimizeTestVector(
 }  // namespace
 
 absl::StatusOr<PacketTestVectorById> LegitimizeUserProvidedTestVectors(
-    absl::Span<const PacketTestVector> user_provided_test_vectors) {
+    absl::Span<const PacketTestVector> user_provided_test_vectors,
+    const pdpi::IrP4Info& ir_info) {
   PacketTestVectorById legitimized_test_vectors_by_id;
   for (const PacketTestVector& vector : user_provided_test_vectors) {
     absl::Status status =
-        LegitimizeTestVector(vector, legitimized_test_vectors_by_id);
+        LegitimizeTestVector(vector, ir_info, legitimized_test_vectors_by_id);
     if (!status.ok()) {
       return gutil::StatusBuilder(status.code())
              << "problem in user-provided packet test vector: "
diff --git a/dvaas/user_provided_packet_test_vector.h b/dvaas/user_provided_packet_test_vector.h
index c368f0f8..778de862 100644
--- a/dvaas/user_provided_packet_test_vector.h
+++ b/dvaas/user_provided_packet_test_vector.h
@@ -15,10 +15,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifndef PINS_DVAAS_USER_PROVIDED_PACKET_TEST_VECTOR_H_
-#define PINS_DVAAS_USER_PROVIDED_PACKET_TEST_VECTOR_H_
-
-#include <vector>
+#ifndef PINS_USER_PROVIDED_PACKET_TEST_VECTOR_H_
+#define PINS_USER_PROVIDED_PACKET_TEST_VECTOR_H_
 
 #include "absl/status/statusor.h"
 #include "absl/types/span.h"
@@ -51,9 +49,12 @@ namespace dvaas {
 //     * Unique among all test vectors.
 // * The input must be of type `DATAPLANE` (other types may be supported in the
 //   future).
+// * Switch output packet ins must have valid metadata conforming to P4Runtime
+//   "Section 16.1: Packet I/O".
 absl::StatusOr<PacketTestVectorById> LegitimizeUserProvidedTestVectors(
-    absl::Span<const PacketTestVector> user_provided_test_vectors);
+    absl::Span<const PacketTestVector> user_provided_test_vectors,
+    const pdpi::IrP4Info& ir_info);
 
 }  // namespace dvaas
 
-#endif  // PINS_DVAAS_USER_PROVIDED_PACKET_TEST_VECTOR_H_
+#endif  // PINS_USER_PROVIDED_PACKET_TEST_VECTOR_H_
diff --git a/dvaas/user_provided_packet_test_vector_test.cc b/dvaas/user_provided_packet_test_vector_test.cc
index 4b45c797..a7f502d6 100644
--- a/dvaas/user_provided_packet_test_vector_test.cc
+++ b/dvaas/user_provided_packet_test_vector_test.cc
@@ -33,6 +33,7 @@
 #include "gutil/status_matchers.h"
 #include "gutil/testing.h"
 #include "p4_pdpi/packetlib/packetlib.pb.h"
+#include "p4_pdpi/testing/test_p4info.h"
 
 namespace dvaas {
 namespace {
@@ -63,8 +64,9 @@ void RunTestCase(const TestCase& test_case) {
 
   // Print output.
   std::cout << "-- Output --------------------------------------------------\n";
+  pdpi::IrP4Info ir_p4info = pdpi::GetTestIrP4Info();
   absl::StatusOr<PacketTestVectorById> output =
-      LegitimizeUserProvidedTestVectors(test_case.vectors);
+      LegitimizeUserProvidedTestVectors(test_case.vectors, ir_p4info);
   if (!output.ok()) {
     // Print error without stack trace, for golden testing.
     std::cout << "ERROR: "
@@ -236,6 +238,52 @@ std::vector<TestCase> GetPositiveTestCases() {
           },
   };
 
+  test_cases.emplace_back() = TestCase{
+      .description = "packet_in with matching tag",
+      .vectors =
+          {
+              gutil::ParseProtoOrDie<PacketTestVector>(R"pb(
+                input {
+                  type: DATAPLANE
+                  packet {
+                    port: "1"
+                    parsed {
+                      headers {
+                        ethernet_header {
+                          ethernet_destination: "ff:ee:dd:cc:bb:aa"
+                          ethernet_source: "55:44:33:22:11:00"
+                          ethertype: "0x000f"
+                        }
+                      }
+                      payload: "test packet #42"
+                    }
+                  }
+                }
+                acceptable_outputs {
+                  packet_ins {
+                    metadata {
+                      name: "ingress_port"
+                      value: { hex_str: "0x00f" }
+                    }
+                    metadata {
+                      name: "target_egress_port"
+                      value: { str: "1" }
+                    }
+                    parsed {
+                      headers {
+                        ethernet_header {
+                          ethernet_destination: "5:5:5:5:5:5"
+                          ethernet_source: "5:5:5:5:5:5"
+                          ethertype: "0x000f"
+                        }
+                      }
+                      payload: "test packet #42"
+                    }
+                  }
+                }
+              )pb"),
+          },
+  };
   return test_cases;
 }  // namespace
 
@@ -430,19 +478,119 @@ std::vector<TestCase> GetNegativeTestCases() {
           },
   };
 
+  test_cases.emplace_back() = TestCase{
+      .description = "packet_in with mismatching tag",
+      .vectors =
+          {
+              gutil::ParseProtoOrDie<PacketTestVector>(R"pb(
+                input {
+                  type: DATAPLANE
+                  packet {
+                    port: "1"
+                    parsed {
+                      headers {
+                        ethernet_header {
+                          ethernet_destination: "ff:ee:dd:cc:bb:aa"
+                          ethernet_source: "55:44:33:22:11:00"
+                          ethertype: "0x000f"
+                        }
+                      }
+                      payload: "test packet #42"
+                    }
+                  }
+                }
+                acceptable_outputs {
+                  packet_ins {
+                    metadata {
+                      name: "ingress_port"
+                      value: { hex_str: "0x00f" }
+                    }
+                    metadata {
+                      name: "target_egress_port"
+                      value: { str: "1" }
+                    }
+                    parsed {
+                      headers {
+                        ethernet_header {
+                          ethernet_destination: "5:5:5:5:5:5"
+                          ethernet_source: "5:5:5:5:5:5"
+                          ethertype: "0x000f"
+                        }
+                      }
+                      payload: "test packet #24"
+                    }
+                  }
+                }
+              )pb"),
+          },
+  };
+
+  test_cases.emplace_back() = TestCase{
+      .description = "packet_in with malformed metadata",
+      .vectors =
+          {
+              gutil::ParseProtoOrDie<PacketTestVector>(R"pb(
+                input {
+                  type: DATAPLANE
+                  packet {
+                    port: "1"
+                    parsed {
+                      headers {
+                        ethernet_header {
+                          ethernet_destination: "ff:ee:dd:cc:bb:aa"
+                          ethernet_source: "55:44:33:22:11:00"
+                          ethertype: "0x000f"
+                        }
+                      }
+                      payload: "test packet #42"
+                    }
+                  }
+                }
+                acceptable_outputs {
+                  packet_ins {
+                    metadata {
+                      name: "foobar"
+                      value: { str: "foobar" }
+                    }
+                    metadata {
+                      name: "ingress_port"
+                      value: { hex_str: "0x00f" }
+                    }
+                    metadata {
+                      name: "target_egress_port"
+                      value: { str: "1" }
+                    }
+                    parsed {
+                      headers {
+                        ethernet_header {
+                          ethernet_destination: "5:5:5:5:5:5"
+                          ethernet_source: "5:5:5:5:5:5"
+                          ethertype: "0x000f"
+                        }
+                      }
+                      payload: "test packet #24"
+                    }
+                  }
+                }
+              )pb"),
+          },
+  };
   return test_cases;
 }
 
 TEST(InternalizeUserProvidedTestVectorsTest, PositiveTestCases) {
+  pdpi::IrP4Info ir_p4info = pdpi::GetTestIrP4Info();
   for (TestCase test_case : GetPositiveTestCases()) {
-    EXPECT_THAT(LegitimizeUserProvidedTestVectors(test_case.vectors), IsOk());
+    EXPECT_THAT(LegitimizeUserProvidedTestVectors(test_case.vectors, ir_p4info),
+                IsOk());
     RunTestCase(test_case);
   }
 }
 
 TEST(InternalizeUserProvidedTestVectorsTest, NegativeTestCases) {
+  pdpi::IrP4Info ir_p4info = pdpi::GetTestIrP4Info();
   for (TestCase test_case : GetNegativeTestCases()) {
-    EXPECT_THAT(LegitimizeUserProvidedTestVectors(test_case.vectors),
+    EXPECT_THAT(LegitimizeUserProvidedTestVectors(test_case.vectors, ir_p4info),
                 Not(IsOk()));
     RunTestCase(test_case);
   }
diff --git a/dvaas/user_provided_packet_test_vector_test.expected b/dvaas/user_provided_packet_test_vector_test.expected
index 47929a7d..94c3d897 100644
--- a/dvaas/user_provided_packet_test_vector_test.expected
+++ b/dvaas/user_provided_packet_test_vector_test.expected
@@ -162,6 +162,62 @@ diff of internalized vector vs input vector:
 added: input.packet.hex: "050505050505050505050505000e74657374207061636b6574202335"
 
 
+================================================================================
+InternalizeUserProvidedTestVectors Test: packet_in with matching tag
+================================================================================
+-- Input ---------------------------------------------------
+-- Input Packet Test Vector #1 --
+input {
+  type: DATAPLANE
+  packet {
+    port: "1"
+    parsed {
+      headers {
+        ethernet_header {
+          ethernet_destination: "ff:ee:dd:cc:bb:aa"
+          ethernet_source: "55:44:33:22:11:00"
+          ethertype: "0x000f"
+        }
+      }
+      payload: "test packet #42"
+    }
+  }
+}
+acceptable_outputs {
+  packet_ins {
+    metadata {
+      name: "ingress_port"
+      value {
+        hex_str: "0x00f"
+      }
+    }
+    metadata {
+      name: "target_egress_port"
+      value {
+        str: "1"
+      }
+    }
+    parsed {
+      headers {
+        ethernet_header {
+          ethernet_destination: "5:5:5:5:5:5"
+          ethernet_source: "5:5:5:5:5:5"
+          ethertype: "0x000f"
+        }
+      }
+      payload: "test packet #42"
+    }
+  }
+}
+
+-- Output --------------------------------------------------
+-- Internalized Packet Test Vector #1 --
+test packet ID extracted from payload: 42
+diff of internalized vector vs input vector:
+added: input.packet.hex: "ffeeddccbbaa554433221100000f74657374207061636b657420233432"
+added: acceptable_outputs[0].packet_ins[0].hex: "050505050505050505050505000f74657374207061636b657420233432"
+
+
 ================================================================================
 InternalizeUserProvidedTestVectors Test: input type != DATAPLANE
 ================================================================================
@@ -531,3 +587,205 @@ input {
 }
 acceptable_outputs {
 }
+
+
+================================================================================
+InternalizeUserProvidedTestVectors Test: packet_in with mismatching tag
+================================================================================
+-- Input ---------------------------------------------------
+-- Input Packet Test Vector #1 --
+input {
+  type: DATAPLANE
+  packet {
+    port: "1"
+    parsed {
+      headers {
+        ethernet_header {
+          ethernet_destination: "ff:ee:dd:cc:bb:aa"
+          ethernet_source: "55:44:33:22:11:00"
+          ethertype: "0x000f"
+        }
+      }
+      payload: "test packet #42"
+    }
+  }
+}
+acceptable_outputs {
+  packet_ins {
+    metadata {
+      name: "ingress_port"
+      value {
+        hex_str: "0x00f"
+      }
+    }
+    metadata {
+      name: "target_egress_port"
+      value {
+        str: "1"
+      }
+    }
+    parsed {
+      headers {
+        ethernet_header {
+          ethernet_destination: "5:5:5:5:5:5"
+          ethernet_source: "5:5:5:5:5:5"
+          ethertype: "0x000f"
+        }
+      }
+      payload: "test packet #24"
+    }
+  }
+}
+
+-- Output --------------------------------------------------
+ERROR: INVALID_ARGUMENT: problem in user-provided packet test vector: mismatch of input packet in tag vs output packet in tag for output packet in #1: 42 vs 24
+Dumping offending test vector:
+input {
+  type: DATAPLANE
+  packet {
+    port: "1"
+    parsed {
+      headers {
+        ethernet_header {
+          ethernet_destination: "ff:ee:dd:cc:bb:aa"
+          ethernet_source: "55:44:33:22:11:00"
+          ethertype: "0x000f"
+        }
+      }
+      payload: "test packet #42"
+    }
+  }
+}
+acceptable_outputs {
+  packet_ins {
+    metadata {
+      name: "ingress_port"
+      value {
+        hex_str: "0x00f"
+      }
+    }
+    metadata {
+      name: "target_egress_port"
+      value {
+        str: "1"
+      }
+    }
+    parsed {
+      headers {
+        ethernet_header {
+          ethernet_destination: "5:5:5:5:5:5"
+          ethernet_source: "5:5:5:5:5:5"
+          ethertype: "0x000f"
+        }
+      }
+      payload: "test packet #24"
+    }
+  }
+}
+
+
+================================================================================
+InternalizeUserProvidedTestVectors Test: packet_in with malformed metadata
+================================================================================
+-- Input ---------------------------------------------------
+-- Input Packet Test Vector #1 --
+input {
+  type: DATAPLANE
+  packet {
+    port: "1"
+    parsed {
+      headers {
+        ethernet_header {
+          ethernet_destination: "ff:ee:dd:cc:bb:aa"
+          ethernet_source: "55:44:33:22:11:00"
+          ethertype: "0x000f"
+        }
+      }
+      payload: "test packet #42"
+    }
+  }
+}
+acceptable_outputs {
+  packet_ins {
+    metadata {
+      name: "foobar"
+      value {
+        str: "foobar"
+      }
+    }
+    metadata {
+      name: "ingress_port"
+      value {
+        hex_str: "0x00f"
+      }
+    }
+    metadata {
+      name: "target_egress_port"
+      value {
+        str: "1"
+      }
+    }
+    parsed {
+      headers {
+        ethernet_header {
+          ethernet_destination: "5:5:5:5:5:5"
+          ethernet_source: "5:5:5:5:5:5"
+          ethertype: "0x000f"
+        }
+      }
+      payload: "test packet #24"
+    }
+  }
+}
+
+-- Output --------------------------------------------------
+ERROR: INVALID_ARGUMENT: problem in user-provided packet test vector: output packet in #1 invalid: 'packet-in' message is invalid: Metadata with name foobar not defined.
+Dumping offending test vector:
+input {
+  type: DATAPLANE
+  packet {
+    port: "1"
+    parsed {
+      headers {
+        ethernet_header {
+          ethernet_destination: "ff:ee:dd:cc:bb:aa"
+          ethernet_source: "55:44:33:22:11:00"
+          ethertype: "0x000f"
+        }
+      }
+      payload: "test packet #42"
+    }
+  }
+}
+acceptable_outputs {
+  packet_ins {
+    metadata {
+      name: "foobar"
+      value {
+        str: "foobar"
+      }
+    }
+    metadata {
+      name: "ingress_port"
+      value {
+        hex_str: "0x00f"
+      }
+    }
+    metadata {
+      name: "target_egress_port"
+      value {
+        str: "1"
+      }
+    }
+    parsed {
+      headers {
+        ethernet_header {
+          ethernet_destination: "5:5:5:5:5:5"
+          ethernet_source: "5:5:5:5:5:5"
+          ethertype: "0x000f"
+        }
+      }
+      payload: "test packet #24"
+    }
+  }
+}

From 3bcc079300f867acb0133760459e6b15c6f53f47 Mon Sep 17 00:00:00 2001
From: VSuryaprasad-hcl <159443973+VSuryaprasad-HCL@users.noreply.github.com>
Date: Thu, 5 Dec 2024 03:31:47 +0000
Subject: [PATCH 10/10] [Dvaas] Follow-up CL to address comments after submit
 for cl/601288379: "Allow packet_ins for custom test vectors" (#802)

Co-authored-by: kishanps <kishanps@google.com>
---
 dvaas/user_provided_packet_test_vector.cc | 66 +++++++++++------------
 1 file changed, 30 insertions(+), 36 deletions(-)

diff --git a/dvaas/user_provided_packet_test_vector.cc b/dvaas/user_provided_packet_test_vector.cc
index 8bda97d2..944ec813 100644
--- a/dvaas/user_provided_packet_test_vector.cc
+++ b/dvaas/user_provided_packet_test_vector.cc
@@ -19,16 +19,20 @@ namespace dvaas {
 
 namespace {
 
+absl::StatusOr<std::string> LegitimizeParsedPacketAndReturnAsHex(
+    packetlib::Packet& packet) {
+  RETURN_IF_ERROR(packetlib::UpdateMissingComputedFields(packet).status());
+  RETURN_IF_ERROR(packetlib::ValidatePacket(packet));
+  ASSIGN_OR_RETURN(std::string raw_packet,
+                   packetlib::RawSerializePacket(packet));
+  return absl::BytesToHexString(raw_packet);
+}
+
 // Checks that the given `input_packet` is well-formed, returning it with
 // omittable fields filled in if that is the case, or an error otherwise.
 absl::StatusOr<Packet> LegitimizePacket(Packet packet) {
-  RETURN_IF_ERROR(
-      packetlib::UpdateMissingComputedFields(*packet.mutable_parsed())
-          .status());
-  RETURN_IF_ERROR(packetlib::ValidatePacket(packet.parsed()));
-  ASSIGN_OR_RETURN(std::string raw_packet,
-                   packetlib::RawSerializePacket(packet.parsed()));
-  packet.set_hex(absl::BytesToHexString(raw_packet));
+  ASSIGN_OR_RETURN(*packet.mutable_hex(), LegitimizeParsedPacketAndReturnAsHex(
+                                              *packet.mutable_parsed()));
   return packet;
 }
 
@@ -43,24 +47,17 @@ absl::Status LegitimizePacketInMetadata(const PacketIn& packet_in,
     *ir_packet_in.add_metadata() = metadata;
   }
 
-  // Translate IR `packet_in` into PI and translate resulting `packet_in` back
-  // into IR to validate its metadata is well-formed. If not, return an error.
-  ASSIGN_OR_RETURN(p4::v1::PacketIn pi_packet_in,
-                   pdpi::IrPacketInToPi(ir_info, ir_packet_in));
-  return absl::OkStatus();
+  // Translate IR `packet_in` into PI to validate its metadata is well-formed.
+  // If not, return an error.
+  return pdpi::IrPacketInToPi(ir_info, ir_packet_in).status();
 }
 
 absl::StatusOr<PacketIn> LegitimizePacketIn(PacketIn packet_in,
                                             const pdpi::IrP4Info& ir_info) {
-  RETURN_IF_ERROR(
-      packetlib::UpdateMissingComputedFields(*packet_in.mutable_parsed())
-          .status());
-  RETURN_IF_ERROR(packetlib::ValidatePacket(packet_in.parsed()));
-  ASSIGN_OR_RETURN(std::string raw_packet,
-                   packetlib::RawSerializePacket(packet_in.parsed()));
-  packet_in.set_hex(absl::BytesToHexString(raw_packet));
-
   RETURN_IF_ERROR(LegitimizePacketInMetadata(packet_in, ir_info));
+  ASSIGN_OR_RETURN(
+      *packet_in.mutable_hex(),
+      LegitimizeParsedPacketAndReturnAsHex(*packet_in.mutable_parsed()));
   return packet_in;
 }
 
@@ -89,22 +86,19 @@ absl::Status LegitimizeTestVector(
            << "must specify at least 1 acceptable output, but 0 were found";
   }
   for (SwitchOutput& output : *vector.mutable_acceptable_outputs()) {
-    // Punted output packets are not supported for now.
-    if (!output.packet_ins().empty()) {
-      for (int i = 0; i < output.packet_ins().size(); ++i) {
-        PacketIn& output_packet_ins = *output.mutable_packet_ins(i);
-        ASSIGN_OR_RETURN(
-            int output_tag, ExtractTestPacketTag(output_packet_ins.parsed()),
-            _.SetPrepend() << "output packet in #" << (i + 1) << " invalid: ");
-        ASSIGN_OR_RETURN(
-            output_packet_ins, LegitimizePacketIn(output_packet_ins, ir_info),
-            _.SetPrepend() << "output packet in #" << (i + 1) << " invalid: ");
-        if (output_tag != tag) {
-          return gutil::InvalidArgumentErrorBuilder()
-                 << "mismatch of input packet in tag vs output packet in tag "
-                    "for output packet in #"
-                 << (i + 1) << ": " << tag << " vs " << output_tag;
-        }
+    for (int i = 0; i < output.packet_ins().size(); ++i) {
+      PacketIn& output_packet_ins = *output.mutable_packet_ins(i);
+      ASSIGN_OR_RETURN(
+          int output_tag, ExtractTestPacketTag(output_packet_ins.parsed()),
+          _.SetPrepend() << "output packet in #" << (i + 1) << " invalid: ");
+      ASSIGN_OR_RETURN(
+          output_packet_ins, LegitimizePacketIn(output_packet_ins, ir_info),
+          _.SetPrepend() << "output packet in #" << (i + 1) << " invalid: ");
+      if (output_tag != tag) {
+        return gutil::InvalidArgumentErrorBuilder()
+               << "mismatch of input packet in tag vs output packet in tag "
+                  "for output packet in #"
+               << (i + 1) << ": " << tag << " vs " << output_tag;
       }
     }
     // Legitimize forwarded output packets.