[Thinkit] Remove ability to set P4Info.Fix issue with punt expectations. Add AclDeny to ingress security table and check if the packets get dropped. Update to push a SUT P4Info if given one.Remove if (!params.punt_action.has_value()) in acl_feature_test. Make rewrites explicit in ACL feature test.Add a test to match on src_mac in AclEgressTable

tests/forwarding/acl_feature_test.cc

@@ -29,6 +29,7 @@
 #include "glog/logging.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include "gutil/proto.h"
 #include "gutil/status.h"  // IWYU pragma: keep
 #include "gutil/status.h"
 #include "gutil/status_matchers.h"  // IWYU pragma: keep
@@ -38,6 +39,7 @@
 #include "p4/v1/p4runtime.pb.h"
 #include "p4_pdpi/ir.h"
 #include "p4_pdpi/ir.pb.h"
+#include "p4_pdpi/netaddr/mac_address.h"
 #include "p4_pdpi/p4_runtime_session.h"
 #include "p4_pdpi/p4_runtime_session_extras.h"
 #include "p4_pdpi/packetlib/packetlib.h"
@@ -80,25 +82,33 @@ absl::Status SetUpIngressAclForwardingAllPackets(
 }
 
 // Helper function to build a UDP packet
-dvaas::PacketTestVector UdpPacket(std::string control_port,
-                                  absl::string_view dst_mac,
-                                  absl::string_view dst_ip,
-                                  std::optional<sai::PuntAction> punt_action) {
+dvaas::PacketTestVector UdpPacket(
+    absl::string_view egress_port,
+    const sai::NexthopRewriteOptions& rewrite_options,
+    std::optional<sai::PuntAction> punt_action) {
   ProtoFixtureRepository repo;
 
   repo.RegisterValue("@payload", dvaas::MakeTestPacketTagFromUniqueId(1))
-      .RegisterValue("@ingress_port", control_port)
-      .RegisterValue("@egress_port", control_port)
-      .RegisterValue("@dst_ip", dst_ip)
-      .RegisterValue("@dst_mac", dst_mac)
+      .RegisterValue("@ingress_port", egress_port)
+      .RegisterValue("@egress_port", egress_port)
+      .RegisterValue("@ingress_dst_mac", "00:aa:bb:cc:cc:dd")
+      .RegisterValue("@ingress_src_mac", "00:00:22:22:00:00")
+      .RegisterValue("@egress_dst_mac",
+                     (rewrite_options.dst_mac_rewrite.has_value()
+                          ? rewrite_options.dst_mac_rewrite->ToString()
+                          : "@ingress_dst_mac"))
+      .RegisterValue("@egress_src_mac",
+                     (rewrite_options.src_mac_rewrite.has_value()
+                          ? rewrite_options.src_mac_rewrite->ToString()
+                          : "@ingress_src_mac"))
       .RegisterValue("@ttl", "0x10")
       .RegisterValue("@decremented_ttl", "0x0f");
 
   dvaas::PacketTestVector test_vector =
       repo.RegisterSnippetOrDie<packetlib::Header>("@ethernet", R"pb(
             ethernet_header {
-              ethernet_destination: @dst_mac,
-              ethernet_source: "00:00:22:22:00:00"
+              ethernet_destination: @ingress_dst_mac,
+              ethernet_source: @ingress_src_mac,
               ethertype: "0x0800"  # Udp
             }
           )pb")
@@ -115,7 +125,7 @@ dvaas::PacketTestVector UdpPacket(std::string control_port,
               # payload_length: filled in automatically.
               protocol: "0x11"
               ipv4_source: "10.0.0.8"
-              ipv4_destination: @dst_ip
+              ipv4_destination: "10.0.0.1"
             }
           )pb")
           .RegisterSnippetOrDie<packetlib::Header>("@udp", R"pb(
@@ -133,8 +143,8 @@ dvaas::PacketTestVector UdpPacket(std::string control_port,
               "@output_packet", ParsePacketAndFillInComputedFields(repo, R"pb(
                 headers: @ethernet {
                   ethernet_header {
-                    ethernet_destination: "02:03:04:05:06:07"
-                    ethernet_source: "00:01:02:03:04:05"
+                    ethernet_destination: @egress_dst_mac
+                    ethernet_source: @egress_src_mac
                   }
                 }
                 headers: @ipv4 { ipv4_header { ttl: @decremented_ttl } }
@@ -148,7 +158,17 @@ dvaas::PacketTestVector UdpPacket(std::string control_port,
             }
             acceptable_outputs {
               packets { port: @egress_port parsed: @output_packet }
-              packet_ins { parsed: @output_packet }
+              packet_ins {
+                metadata {
+                  name: "ingress_port"
+                  value: { str: @ingress_port }
+                }
+                metadata {
+                  name: "target_egress_port"
+                  value: { str: @egress_port }
+                }
+                parsed: @input_packet
+              }
             }
           )pb");
 
@@ -165,19 +185,16 @@ dvaas::PacketTestVector UdpPacket(std::string control_port,
 
 // Helper routine to install L3 route
 absl::Status InstallL3Route(pdpi::P4RuntimeSession* switch_session,
-                            pdpi::IrP4Info ir_p4info, std::string given_port,
+                            const pdpi::IrP4Info& ir_p4info,
+                            absl::string_view egress_port,
+                            const sai::NexthopRewriteOptions& rewrite_options,
                             std::optional<sai::PuntAction> punt_action) {
   std::vector<p4::v1::Entity> pi_entities;
   LOG(INFO) << "Installing L3 route";
 
   sai::EntryBuilder entry_builder =
-      sai::EntryBuilder()
-          .AddVrfEntry("vrf-1")
-          .AddPreIngressAclEntryAssigningVrfForGivenIpType(
-              "vrf-1", sai::IpVersion::kIpv4)
-          .AddDefaultRouteForwardingAllPacketsToGivenPort(
-              given_port, sai::IpVersion::kIpv4, "vrf-1")
-          .AddEntryAdmittingAllPacketsToL3();
+      sai::EntryBuilder().AddEntriesForwardingIpPacketsToGivenPort(
+          egress_port, sai::IpVersion::kIpv4And6, rewrite_options);
 
   if (punt_action.has_value()) {
     entry_builder.AddEntryPuntingAllPackets(punt_action.value());
@@ -193,22 +210,27 @@ absl::Status InstallL3Route(pdpi::P4RuntimeSession* switch_session,
 }
 
 TEST_P(AclFeatureTestFixture, AclDenyAction) {
+  dvaas::PacketTestVector test_vector;
   const AclFeatureTestParams& params = GetParam();
+  dvaas::DataplaneValidationParams dvaas_params = params.dvaas_params;
 
   thinkit::MirrorTestbed& testbed =
       GetParam().mirror_testbed->GetMirrorTestbed();
-  std::unique_ptr<pdpi::P4RuntimeSession> sut_p4rt_session,
-      control_switch_p4rt_session;
 
+  // Initialize the connection, clear all entities, and (for the SUT) push
+  // P4Info.
   ASSERT_OK_AND_ASSIGN(
-      std::tie(sut_p4rt_session, control_switch_p4rt_session),
-      pins_test::ConfigureSwitchPairAndReturnP4RuntimeSessionPair(
-          testbed.Sut(), testbed.ControlSwitch(), std::nullopt,
-          GetParam().p4info));
-
-  // Initialize the connection, clear table entries, and push GNMI
-  // configuration (if given) for the SUT and Control switch.
+      std::unique_ptr<pdpi::P4RuntimeSession> sut_p4rt_session,
+      pins_test::ConfigureSwitchAndReturnP4RuntimeSession(
+          testbed.Sut(), /*gnmi_config=*/std::nullopt, GetParam().sut_p4info));
+  ASSERT_OK_AND_ASSIGN(
+      std::unique_ptr<pdpi::P4RuntimeSession> control_switch_p4rt_session,
+      pins_test::ConfigureSwitchAndReturnP4RuntimeSession(
+          testbed.ControlSwitch(), /*gnmi_config=*/std::nullopt,
+          /*p4info=*/std::nullopt));
   ASSERT_NE(sut_p4rt_session, nullptr);
+  ASSERT_NE(control_switch_p4rt_session, nullptr);
+
   ASSERT_OK_AND_ASSIGN(
       p4::v1::GetForwardingPipelineConfigResponse sut_config,
       pdpi::GetForwardingPipelineConfig(sut_p4rt_session.get()));
@@ -217,33 +239,158 @@ TEST_P(AclFeatureTestFixture, AclDenyAction) {
   ASSERT_OK_AND_ASSIGN(pdpi::IrP4Info sut_ir_p4info,
                        pdpi::CreateIrP4Info(sut_config.config().p4info()));
 
-  ASSERT_OK(pdpi::ClearTableEntries(sut_p4rt_session.get()));
-
   // Get control ports to test on.
   ASSERT_OK_AND_ASSIGN(
       auto gnmi_stub_control,
       GetParam().mirror_testbed->GetMirrorTestbed().Sut().CreateGnmiStub());
   ASSERT_OK_AND_ASSIGN(std::string control_port,
                        pins_test::GetAnyUpInterfacePortId(*gnmi_stub_control));
 
+  // Since we don't care about the egress packet's source and destination mac,
+  // we use the default rewrite options.
+  const sai::NexthopRewriteOptions rewrite_options;
+
   ASSERT_OK(InstallL3Route(sut_p4rt_session.get(), sut_ir_p4info, control_port,
-                           params.punt_action));
+                           rewrite_options, params.punt_action));
+
+  test_vector = UdpPacket(control_port, rewrite_options, params.punt_action);
 
-  // remove the skip
+  // Run test with custom packet test vector.
+  dvaas_params.packet_test_vector_override = {test_vector};
+  ASSERT_OK_AND_ASSIGN(
+      dvaas::ValidationResult validation_result,
+      GetParam().dvaas->ValidateDataplane(testbed, dvaas_params));
+
+  // Log statistics and check that things succeeded.
+  validation_result.LogStatistics();
+  EXPECT_OK(validation_result.HasSuccessRateOfAtLeast(1.0));
+
+  ASSERT_OK_AND_ASSIGN(sut_p4rt_session,
+                       pdpi::P4RuntimeSession::Create(testbed.Sut()));
+
+  // Install AclDeny
+  ASSERT_OK_AND_ASSIGN(auto proto_entry,
+                       gutil::ParseTextProto<pdpi::IrTableEntry>(
+                           R"pb(table_name: "acl_ingress_security_table"
+                                priority: 1
+                                action { name: "acl_deny" }
+                           )pb"));
+
+  EXPECT_OK(pdpi::InstallIrTableEntry(*sut_p4rt_session.get(), proto_entry));
+  for (dvaas::SwitchOutput& output :
+       *test_vector.mutable_acceptable_outputs()) {
+    output.clear_packet_ins();
+    output.clear_packets();
+  }
+
+  dvaas_params.packet_test_vector_override = {test_vector};
+  ASSERT_OK_AND_ASSIGN(
+      dvaas::ValidationResult validation_result2,
+      GetParam().dvaas->ValidateDataplane(testbed, dvaas_params));
+
+  // Log statistics and check that things succeeded.
+  validation_result2.LogStatistics();
+  EXPECT_OK(validation_result2.HasSuccessRateOfAtLeast(1.0));
+}
+
+TEST_P(AclFeatureTestFixture, AclEgressTable) {
+  dvaas::PacketTestVector test_vector;
+  const AclFeatureTestParams& params = GetParam();
+  dvaas::DataplaneValidationParams dvaas_params = params.dvaas_params;
+  dvaas_params.artifact_prefix = "sanity_dvaas";
+  const netaddr::MacAddress output_src_mac(0x1, 0x2, 0x3, 0x1, 0x2, 0x3);
+
+  // we are not testing punt action in this test
+  // so skip for those variants
   if (params.punt_action.has_value()) {
-    // Run test with custom packet test vector.
-    dvaas::DataplaneValidationParams dvaas_params = params.dvaas_params;
-    dvaas_params.packet_test_vector_override = {
-        UdpPacket(control_port, /*dst_mac=*/"00:aa:bb:cc:cc:dd",
-                  /*dst_ip=*/"10.0.0.1", params.punt_action)};
-    ASSERT_OK_AND_ASSIGN(
-        dvaas::ValidationResult validation_result,
-        GetParam().dvaas->ValidateDataplane(testbed, dvaas_params));
-
-    // Log statistics and check that things succeeded.
-    validation_result.LogStatistics();
-    EXPECT_OK(validation_result.HasSuccessRateOfAtLeast(1.0));
+    GTEST_SKIP();
+  }
+
+  thinkit::MirrorTestbed& testbed =
+      GetParam().mirror_testbed->GetMirrorTestbed();
+
+
+  // Initialize the connection, clear all entities, and (for the SUT) push
+  // P4Info.
+  ASSERT_OK_AND_ASSIGN(
+      std::unique_ptr<pdpi::P4RuntimeSession> sut_p4rt_session,
+      pins_test::ConfigureSwitchAndReturnP4RuntimeSession(
+          testbed.Sut(), /*gnmi_config=*/std::nullopt, GetParam().sut_p4info));
+  ASSERT_OK_AND_ASSIGN(
+      std::unique_ptr<pdpi::P4RuntimeSession> control_switch_p4rt_session,
+      pins_test::ConfigureSwitchAndReturnP4RuntimeSession(
+          testbed.ControlSwitch(), /*gnmi_config=*/std::nullopt,
+          /*p4info=*/std::nullopt));
+  ASSERT_NE(sut_p4rt_session, nullptr);
+  ASSERT_NE(control_switch_p4rt_session, nullptr);
+
+  ASSERT_OK_AND_ASSIGN(
+      p4::v1::GetForwardingPipelineConfigResponse sut_config,
+      pdpi::GetForwardingPipelineConfig(sut_p4rt_session.get()));
+  ASSERT_OK(testbed.Environment().StoreTestArtifact(
+      "sut_p4Info.textproto", sut_config.config().p4info().DebugString()));
+  ASSERT_OK_AND_ASSIGN(pdpi::IrP4Info sut_ir_p4info,
+                       pdpi::CreateIrP4Info(sut_config.config().p4info()));
+
+  // Get control ports to test on.
+  ASSERT_OK_AND_ASSIGN(
+      auto gnmi_stub_control,
+      GetParam().mirror_testbed->GetMirrorTestbed().Sut().CreateGnmiStub());
+  ASSERT_OK_AND_ASSIGN(std::string control_port,
+                       pins_test::GetAnyUpInterfacePortId(*gnmi_stub_control));
+
+  const sai::NexthopRewriteOptions rewrite_options = {.src_mac_rewrite =
+                                                          output_src_mac};
+
+  ASSERT_OK(InstallL3Route(sut_p4rt_session.get(), sut_ir_p4info, control_port,
+                           rewrite_options, /*punt_action=*/std::nullopt));
+
+  test_vector =
+      UdpPacket(control_port, rewrite_options, /*punt_action=*/std::nullopt);
+
+  // Run test with custom packet test vector.
+  dvaas_params.packet_test_vector_override = {test_vector};
+  ASSERT_OK_AND_ASSIGN(
+      dvaas::ValidationResult validation_result,
+      GetParam().dvaas->ValidateDataplane(testbed, dvaas_params));
+
+  // Log statistics and check that things succeeded.
+  validation_result.LogStatistics();
+  EXPECT_OK(validation_result.HasSuccessRateOfAtLeast(1.0));
+
+  ASSERT_OK_AND_ASSIGN(sut_p4rt_session,
+                       pdpi::P4RuntimeSession::Create(testbed.Sut()));
+
+  // Install AclEgress Drop
+  ASSERT_OK_AND_ASSIGN(auto proto_entry,
+                       gutil::ParseTextProto<pdpi::IrTableEntry>(
+                           R"pb(table_name: "acl_egress_table"
+                                priority: 1
+                                matches {
+                                  name: "src_mac"
+                                  ternary {
+                                    value { mac: "01:02:03:01:02:03" }
+                                    mask { mac: "ff:ff:ff:ff:ff:ff" }
+                                  }
+                                }
+                                action { name: "acl_drop" }
+                           )pb"));
+
+  EXPECT_OK(pdpi::InstallIrTableEntry(*sut_p4rt_session.get(), proto_entry));
+
+  for (dvaas::SwitchOutput& output :
+       *test_vector.mutable_acceptable_outputs()) {
+    output.clear_packets();
   }
+  dvaas_params.packet_test_vector_override = {test_vector};
+  dvaas_params.artifact_prefix = "real_test_dvaas";
+  ASSERT_OK_AND_ASSIGN(
+      dvaas::ValidationResult validation_result2,
+      GetParam().dvaas->ValidateDataplane(testbed, dvaas_params));
+
+  // Log statistics and check that things succeeded.
+  validation_result2.LogStatistics();
+  EXPECT_OK(validation_result2.HasSuccessRateOfAtLeast(1.0));
 }
 
 }  // namespace

tests/forwarding/acl_feature_test.h

@@ -15,13 +15,14 @@
 #ifndef PINS_TESTS_FORWARDING_ACL_FEATURE_TEST_H_
 #define PINS_TESTS_FORWARDING_ACL_FEATURE_TEST_H_
 
+#include <memory>
 #include <optional>
-#include <tuple>
+#include <string>
 
 #include "dvaas/dataplane_validation.h"
-#include "dvaas/test_vector.h"
 #include "dvaas/test_vector.pb.h"
-#include "dvaas/validation_result.h"
+#include "gtest/gtest.h"
+#include "p4/config/v1/p4info.pb.h"
 #include "sai_p4/instantiations/google/test_tools/test_entries.h"
 #include "thinkit/mirror_testbed_fixture.h"
 
@@ -31,7 +32,9 @@ struct AclFeatureTestParams {
   // Using a shared_ptr because parameterized tests require objects to be
   // copyable.
   std::shared_ptr<thinkit::MirrorTestbedInterface> mirror_testbed;
-  std::optional<p4::config::v1::P4Info> p4info;
+  // Pushed to the SUT if given, otherwise assumes the correct one is already
+  // configured.
+  std::optional<p4::config::v1::P4Info> sut_p4info;
   std::string test_name;
   // ACL action variant to test out different behavior
   std::optional<sai::PuntAction> punt_action;