[SAI_P4] Define l3 admit in-port match capability and remove legacy rif action from certain P4 instances.

Signed-off-by: Bibhuprasad Singh <[email protected]>

Author: kishanps

sai_p4/BUILD.bazel

@@ -24,8 +24,6 @@ cc_library(
 	"//gutil/gutil:status",
         "@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
         "@com_google_absl//absl/status",
-        "@com_google_absl//absl/status:statusor",
-        "@com_google_absl//absl/strings",
         "@com_google_protobuf//:protobuf",
     ],
 )
@@ -38,7 +36,6 @@ cc_test(
         ":capabilities_cc_proto",
 	"//gutil/gutil:proto_matchers",
 	"//gutil/gutil:status_matchers",
-        "@com_google_absl//absl/status",
         "@com_google_googletest//:gtest_main",
     ],
 )

sai_p4/capabilities.cc

@@ -3,10 +3,7 @@
 #include <utility>
 
 #include "absl/status/status.h"
-#include "absl/status/statusor.h"
-#include "absl/strings/str_cat.h"
 #include "google/protobuf/any.pb.h"
-#include "gutil/gutil/status.h"
 #include "p4/v1/p4runtime.pb.h"
 #include "sai_p4/capabilities.pb.h"
 
@@ -25,22 +22,17 @@ absl::Status AddExperimentalResourceCapabilities(
   return absl::OkStatus();
 }
 
-absl::StatusOr<ExperimentalResourceCapabilities>
-GetExperimentalResourceCapabilities(const CapabilitiesResponse& response) {
+ExperimentalResourceCapabilities GetExperimentalResourceCapabilities(
+    const CapabilitiesResponse& response) {
   ExperimentalResourceCapabilities capabilities;
-  if (!response.has_experimental() ||
-      !response.experimental().UnpackTo(&capabilities)) {
-    return absl::NotFoundError(absl::StrCat(
-        "ExperimentalResourceCapabilities not found in CapabilitiesResponse: ",
-        response.DebugString()));
-  }
+  response.experimental().UnpackTo(&capabilities);
   return capabilities;
 };
 
-absl::StatusOr<WcmpGroupLimitations> GetWcmpGroupLimitations(
+WcmpGroupLimitations GetWcmpGroupLimitations(
     const CapabilitiesResponse& response) {
-  ASSIGN_OR_RETURN(ExperimentalResourceCapabilities capabilities,
-                   GetExperimentalResourceCapabilities(response));
+  ExperimentalResourceCapabilities capabilities =
+      GetExperimentalResourceCapabilities(response);
   return capabilities.wcmp_group_limitations();
 }
 

sai_p4/capabilities.h

@@ -2,7 +2,6 @@
 #define PINS_SAI_P4_CAPABILITIES_H_
 
 #include "absl/status/status.h"
-#include "absl/status/statusor.h"
 #include "p4/v1/p4runtime.pb.h"
 #include "sai_p4/capabilities.pb.h"
 
@@ -14,15 +13,14 @@ absl::Status AddExperimentalResourceCapabilities(
     ExperimentalResourceCapabilities capabilities,
     p4::v1::CapabilitiesResponse& response);
 
-// Returns the `ExperimentalResourceCapabilities` inside of `response`. Returns
-// an error if `response` does not contain `ExperimentalResourceCapabilities`.
-absl::StatusOr<ExperimentalResourceCapabilities>
-GetExperimentalResourceCapabilities(
+// If `response` contains a `ExperimentalResourceCapabilities`, returns it.
+// Otherwise, returns an empty `ExperimentalResourceCapabilities`.
+ExperimentalResourceCapabilities GetExperimentalResourceCapabilities(
     const p4::v1::CapabilitiesResponse& response);
 
-// Returns the `WcmpGroupLimitations` inside of `response`. Returns an error if
-// `response` does not contain a `ExperimentalResourceCapabilities`.
-absl::StatusOr<WcmpGroupLimitations> GetWcmpGroupLimitations(
+// If `response` contains a `WcmpGroupLimitations`, returns it.
+// Otherwise, returns an empty `WcmpGroupLimitations`.
+WcmpGroupLimitations GetWcmpGroupLimitations(
     const p4::v1::CapabilitiesResponse& response);
 
 }  // namespace sai

sai_p4/capabilities_test.cc

@@ -1,17 +1,15 @@
 #include "sai_p4/capabilities.h"
 
-#include "absl/status/status.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include "gutil/gutil/proto_matchers.h"
-#include "gutil/gutil/status_matchers.h"
+#include "gutil/gutil/status_matchers.h"  // IWYU pragma: keep
 #include "sai_p4/capabilities.pb.h"
 
 namespace sai {
 namespace {
 
 using ::gutil::EqualsProto;
-using ::gutil::IsOkAndHolds;
 using ::p4::v1::CapabilitiesResponse;
 
 TEST(CapabilitiesTest, AddExperimentalResourceCapabilitiesWorks) {
@@ -43,12 +41,22 @@ TEST(CapabilitiesTest, GetExperimentalResourceCapabilitiesWorks) {
   ASSERT_OK(AddExperimentalResourceCapabilities(
       experimental_resource_capabilities, response));
   EXPECT_THAT(GetExperimentalResourceCapabilities(response),
-              IsOkAndHolds(EqualsProto(experimental_resource_capabilities)));
+              EqualsProto(experimental_resource_capabilities));
 }
 
-TEST(CapabilitiesTest, GetExperimentalResourceCapabilitiesFailsWhenMissing) {
+TEST(CapabilitiesTest,
+     GetExperimentalResourceCapabilitiesReturnsEmptyWhenMissing) {
   EXPECT_THAT(GetExperimentalResourceCapabilities(CapabilitiesResponse()),
-              gutil::StatusIs(absl::StatusCode::kNotFound));
+              EqualsProto(ExperimentalResourceCapabilities()));
+}
+
+TEST(CapabilitiesTest,
+     GetExperimentalResourceCapabilitiesReturnsEmptyWhenWrongType) {
+  CapabilitiesResponse response;
+  p4::v1::TableEntry entry;
+  ASSERT_TRUE(response.mutable_experimental()->PackFrom(entry));
+  EXPECT_THAT(GetExperimentalResourceCapabilities(response),
+              EqualsProto(ExperimentalResourceCapabilities()));
 }
 
 TEST(CapabilitiesTest, GetWcmpGroupLimitationsWorks) {
@@ -63,21 +71,29 @@ TEST(CapabilitiesTest, GetWcmpGroupLimitationsWorks) {
       experimental_resource_capabilities, response));
   EXPECT_THAT(
       GetWcmpGroupLimitations(response),
-      IsOkAndHolds(EqualsProto(
-          experimental_resource_capabilities.wcmp_group_limitations())));
+      EqualsProto(experimental_resource_capabilities.wcmp_group_limitations()));
 }
 
-TEST(CapabilitiesTest, GetWcmpGroupLimitationsWorksWithDefaultValues) {
+TEST(CapabilitiesTest, GetWcmpGroupLimitationsWorksWhenMissing) {
   CapabilitiesResponse response;
   ASSERT_OK(AddExperimentalResourceCapabilities(
       ExperimentalResourceCapabilities(), response));
   EXPECT_THAT(GetWcmpGroupLimitations(response),
-              IsOkAndHolds(EqualsProto(WcmpGroupLimitations())));
+              EqualsProto(WcmpGroupLimitations()));
 }
 
-TEST(CapabilitiesTest, GetWcmpGroupLimitationsFailsWhenParentMessageMissing) {
+TEST(CapabilitiesTest,
+     GetWcmpGroupLimitationsReturnsEmptyWhenParentWhenMissing) {
   EXPECT_THAT(GetWcmpGroupLimitations(CapabilitiesResponse()),
-              gutil::StatusIs(absl::StatusCode::kNotFound));
+              EqualsProto(WcmpGroupLimitations()));
+}
+
+TEST(CapabilitiesTest, GetWcmpGroupLimitationsReturnsEmptyWhenWrongType) {
+  CapabilitiesResponse response;
+  p4::v1::TableEntry entry;
+  ASSERT_TRUE(response.mutable_experimental()->PackFrom(entry));
+  EXPECT_THAT(GetWcmpGroupLimitations(response),
+              EqualsProto(WcmpGroupLimitations()));
 }
 
 }  // namespace

sai_p4/fixed/l3_admit.p4

@@ -22,7 +22,9 @@ control l3_admit(in headers_t headers,
     key = {
       headers.ethernet.dst_addr : ternary @name("dst_mac") @id(1)
                                           @format(MAC_ADDRESS);
+#if defined(L3_ADMIT_SUPPORTS_IN_PORT)
       local_metadata.ingress_port : optional @name("in_port") @id(2);
+#endif
     }
     actions = {
       @proto_id(1) admit_to_l3;

sai_p4/fixed/routing.p4

@@ -414,9 +414,11 @@ control routing_resolution(in headers_t headers,
       router_interface_id_value : exact @id(1)
                                         @name("router_interface_id");
     }
-    // TODO: Remove once no longer in use on our switches.
     actions = {
+#if defined(RIF_PROGRAMMING_MY_MAC_SUPPORTED)
+      // TODO: Remove once no longer in use on our switches.
       @proto_id(1) set_port_and_src_mac;
+#endif
       @proto_id(2) unicast_set_port_and_src_mac_and_vlan_id;
       @proto_id(3) unicast_set_port_and_src_mac;
       @defaultonly NoAction;

sai_p4/instantiations/google/fabric_border_router.p4

@@ -3,9 +3,11 @@
 #define ACL_REDIRECT_TO_NEXTHOP_CAPABLE
 #define ACL_REDIRECT_TO_PORT_CAPABLE
 #define DSCP_REWRITE_CAPABLE
-#define NEXTHOP_DISABLE_REWRITES_CAPABLE
 #define IP_MULTICAST_CAPABLE
+#define L3_ADMIT_SUPPORTS_IN_PORT
+#define NEXTHOP_DISABLE_REWRITES_CAPABLE
 #define MIRROR_CAPABLE
+#define RIF_PROGRAMMING_MY_MAC_SUPPORTED
 #define TIMESTAMP_CAPABLE
 #define TUNNEL_ENCAP_CAPABLE
 

sai_p4/instantiations/google/middleblock.p4

@@ -3,9 +3,11 @@
 #define ACL_REDIRECT_TO_NEXTHOP_CAPABLE
 #define ACL_REDIRECT_TO_PORT_CAPABLE
 #define DSCP_REWRITE_CAPABLE
-#define NEXTHOP_DISABLE_REWRITES_CAPABLE
 #define IP_MULTICAST_CAPABLE
+#define L3_ADMIT_SUPPORTS_IN_PORT
+#define NEXTHOP_DISABLE_REWRITES_CAPABLE
 #define MIRROR_CAPABLE
+#define RIF_PROGRAMMING_MY_MAC_SUPPORTED
 #define TIMESTAMP_CAPABLE
 #define TUNNEL_ENCAP_CAPABLE
 

sai_p4/instantiations/google/tor.p4

@@ -16,9 +16,11 @@
 #define ACL_REDIRECT_TO_NEXTHOP_CAPABLE
 #define ACL_REDIRECT_TO_PORT_CAPABLE
 #define DSCP_REWRITE_CAPABLE
-#define NEXTHOP_DISABLE_REWRITES_CAPABLE
 #define IP_MULTICAST_CAPABLE
+#define L3_ADMIT_SUPPORTS_IN_PORT
+#define NEXTHOP_DISABLE_REWRITES_CAPABLE
 #define MIRROR_CAPABLE
+#define RIF_PROGRAMMING_MY_MAC_SUPPORTED
 #define TIMESTAMP_CAPABLE
 #define TUNNEL_ENCAP_CAPABLE