Fix warm_boot_state_adapter implementation (sonic-net#477)

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

p4rt_app/p4runtime/p4runtime_impl.cc

@@ -1440,9 +1440,9 @@ absl::Status P4RuntimeImpl::ConfigureAppDbTables(
   nlohmann::json ext_tables_json = {};
 
   // Setup definitions for each each P4 ACL table.
-  for (const auto& pair : ir_p4info.tables_by_name()) {
-    std::string table_name = pair.first;
-    pdpi::IrTableDefinition table = pair.second;
+  for (const pdpi::IrTableDefinition& table :
+       OrderTablesBySize(ir_p4info.tables_by_name())) {
+    std::string table_name = table.preamble().alias();
     ASSIGN_OR_RETURN(table::Type table_type, GetTableType(table),
                      _ << "Failed to configure table " << table_name << ".");
 

p4rt_app/sonic/app_db_acl_def_table_manager.cc

@@ -22,6 +22,7 @@
 #include "absl/status/status.h"
 #include "absl/strings/numbers.h"
 #include "absl/strings/str_cat.h"
+#include "absl/strings/str_join.h"
 #include "absl/strings/str_replace.h"
 #include "absl/strings/str_split.h"
 #include "absl/strings/string_view.h"
@@ -46,6 +47,8 @@ constexpr absl::string_view kCompositeMatchAnnotationLabel = "composite_field";
 constexpr absl::string_view kUdfMatchAnnotationLabel = "sai_udf";
 constexpr absl::string_view kActionAnnotationLabel = "sai_action";
 constexpr absl::string_view kActionParamAnnotationLabel = "sai_action_param";
+constexpr absl::string_view kActionParamObjectTypeAnnotationLabel =
+    "sai_action_param_object_type";
 
 using ::absl::Status;
 using ::absl::StatusOr;
@@ -70,6 +73,16 @@ bool IsValidColor(absl::string_view color) {
          color == "SAI_PACKET_COLOR_YELLOW" || color == "SAI_PACKET_COLOR_RED";
 }
 
+bool IsValidRedirectObjectType(absl::string_view type) {
+  return type == "SAI_OBJECT_TYPE_BRIDGE_PORT" ||
+         type == "SAI_OBJECT_TYPE_IPMC_GROUP" ||
+         type == "SAI_OBJECT_TYPE_L2MC_GROUP" ||
+         type == "SAI_OBJECT_TYPE_LAG" || type == "SAI_OBJECT_TYPE_NEXT_HOP" ||
+         type == "SAI_OBJECT_TYPE_NEXT_HOP_GROUP" ||
+         type == "SAI_OBJECT_TYPE_PORT" ||
+         type == "SAI_OBJECT_TYPE_SYSTEM_PORT";
+}
+
 // Generates the AppDB DEFINITION:ACL_* Entry key for an ACL table.
 StatusOr<std::string> GenerateSonicDbKeyFromIrTable(
     const IrTableDefinition& table) {
@@ -517,6 +530,7 @@ struct IrActionInfo {
     std::string action;     // SAI action name.
     std::string parameter;  // Parameter name (empty for unparameterized action)
     std::string color;      // SaiAction color (empty for unmetered actions)
+    std::string object_type;  // SAI object type (empty by default)
   };
 
   std::string name;  // Action name
@@ -554,6 +568,30 @@ StatusOr<IrActionInfo::SaiAction> ExtractActionAndColor(
   return sai_action;
 }
 
+// If the sai_action_param_object_type is included,
+//   Returns the sai_object_type.
+// Otherwise,
+//   Returns an empty string.
+StatusOr<std::string> ExtractActionParamSaiObjectType(
+    const std::vector<std::string>& arg_list) {
+  if (arg_list.empty()) {
+    return "";
+  }
+  if (arg_list.size() > 1) {
+    return InvalidArgumentErrorBuilder()
+           << "ACL action parameter object type annotation '@"
+           << kActionParamObjectTypeAnnotationLabel << "("
+           << absl::StrJoin(arg_list, ",")
+           << ")' has too many arguments (expected 1).";
+  }
+  if (!IsValidRedirectObjectType(arg_list[0])) {
+    return InvalidArgumentErrorBuilder()
+           << "Annotation argument '" << arg_list[0]
+           << "' is not a valid sai object type.";
+  }
+  return arg_list[0];
+}
+
 StatusOr<IrActionInfo::SaiAction> ParseActionParam(
     const IrActionDefinition::IrActionParamDefinition& param) {
   auto annotation_args_result = pdpi::GetAnnotationAsArgList(
@@ -574,6 +612,16 @@ StatusOr<IrActionInfo::SaiAction> ParseActionParam(
                    ExtractActionAndColor(annotation_args_result.value()),
                    _ << " Failed to process action parameter ["
                      << param.ShortDebugString() << "].");
+  // This is an optional annotation.
+  auto annotation_object_type_args_list = pdpi::GetAnnotationAsArgList(
+      kActionParamObjectTypeAnnotationLabel, param.param().annotations());
+  if (annotation_object_type_args_list.ok()) {
+    ASSIGN_OR_RETURN(sai_action.object_type,
+                     ExtractActionParamSaiObjectType(
+                         annotation_object_type_args_list.value()),
+                     _ << " Failed to process action parameter ["
+                       << param.ShortDebugString() << "].");
+  }
   if (!sai_action.color.empty()) {
     return InvalidArgumentErrorBuilder()
            << "Action parameter [" << param.ShortDebugString()
@@ -692,6 +740,8 @@ StatusOr<IrActionInfo> ParseAction(const IrActionDefinition& action) {
 //  {"action": "SAI_PACKET_ACTION_SET_TC", "param": "tc"}
 //  {"action": "SAI_PACKET_ACTION_DROP", "packet_color": "RED"}
 //  {"action": "SAI_PACKET_ACTION_COPY"}
+//  {"action": "SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT", "param": "0x0001",
+//   "object_type": "SAI_OBJECT_TYPE_IPMC_GROUP"}
 nlohmann::json CreateSaiActionJson(const IrActionInfo::SaiAction& parameter) {
   nlohmann::json json;
   json["action"] = parameter.action;
@@ -701,6 +751,9 @@ nlohmann::json CreateSaiActionJson(const IrActionInfo::SaiAction& parameter) {
   if (!parameter.color.empty()) {
     json["packet_color"] = parameter.color;
   }
+  if (!parameter.object_type.empty()) {
+    json["object_type"] = parameter.object_type;
+  }
 
   return json;
 }

p4rt_app/sonic/app_db_acl_def_table_manager_test.cc

@@ -64,6 +64,17 @@ TEST(InsertAclTableDefinition, InsertsAclTableDefinition) {
                    name: "in_port"
                    annotations: "@sai_field(SAI_ACL_TABLE_ATTR_FIELD_IN_PORT)")pb",
               pdpi::STRING)
+          .match_field(
+              R"pb(id: 125
+                   name: "vrf_id"
+                   annotations: "@sai_field(SAI_ACL_TABLE_ATTR_FIELD_VRF_ID)")pb",
+              pdpi::STRING)
+          .match_field(
+              R"pb(id: 126
+                   name: "ipmc_table_hit"
+                   bitwidth: 1
+                   annotations: "@sai_field(SAI_ACL_TABLE_ATTR_FIELD_IPMC_NPU_META_DST_HIT)")pb",
+              pdpi::HEX_STRING)
           .entry_action(
               IrActionDefinitionBuilder()
                   .preamble(
@@ -120,6 +131,62 @@ TEST(InsertAclTableDefinition, InsertsAclTableDefinition) {
                         annotations: "@sai_action_param(QOS_QUEUE)"
                       )pb",
                       pdpi::STRING))
+          .entry_action(
+              IrActionDefinitionBuilder()
+                  .preamble(
+                      R"pb(
+                        alias: "redirect_action_that_includes_ipmc_type"
+                      )pb")
+                  .param(
+                      R"pb(
+                        id: 1
+                        name: "multicast_group_id"
+                        annotations: "@sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)"
+                        annotations: "@sai_action_param_object_type(SAI_OBJECT_TYPE_IPMC_GROUP)"
+                      )pb",
+                      pdpi::STRING))
+          .entry_action(
+              IrActionDefinitionBuilder()
+                  .preamble(
+                      R"pb(
+                        alias: "redirect_action_that_includes_l2mc_type"
+                      )pb")
+                  .param(
+                      R"pb(
+                        id: 1
+                        name: "multicast_group_id"
+                        annotations: "@sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)"
+                        annotations: "@sai_action_param_object_type(SAI_OBJECT_TYPE_L2MC_GROUP)"
+                      )pb",
+                      pdpi::STRING))
+          .entry_action(
+              IrActionDefinitionBuilder()
+                  .preamble(
+                      R"pb(
+                        alias: "redirect_action_that_includes_next_hop_type"
+                      )pb")
+                  .param(
+                      R"pb(
+                        id: 1
+                        name: "multicast_group_id"
+                        annotations: "@sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)"
+                        annotations: "@sai_action_param_object_type(SAI_OBJECT_TYPE_NEXT_HOP)"
+                      )pb",
+                      pdpi::STRING))
+          .entry_action(
+              IrActionDefinitionBuilder()
+                  .preamble(
+                      R"pb(
+                        alias: "redirect_action_that_includes_port_type"
+                      )pb")
+                  .param(
+                      R"pb(
+                        id: 1
+                        name: "multicast_group_id"
+                        annotations: "@sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)"
+                        annotations: "@sai_action_param_object_type(SAI_OBJECT_TYPE_PORT)"
+                      )pb",
+                      pdpi::STRING))
           .size(512)
           .meter_unit(p4::config::v1::MeterSpec::BYTES)
           .counter_unit(p4::config::v1::CounterSpec::BOTH)();
@@ -143,6 +210,17 @@ TEST(InsertAclTableDefinition, InsertsAclTableDefinition) {
             "format": "STRING",
             "sai_field": "SAI_ACL_TABLE_ATTR_FIELD_IN_PORT"})JSON")
                             .dump()},
+      {"match/vrf_id", nlohmann::json::parse(R"JSON(
+           {"kind": "sai_field",
+            "format": "STRING",
+            "sai_field": "SAI_ACL_TABLE_ATTR_FIELD_VRF_ID"})JSON")
+                           .dump()},
+      {"match/ipmc_table_hit", nlohmann::json::parse(R"JSON(
+           {"kind": "sai_field",
+            "format": "HEX_STRING",
+            "bitwidth": 1,
+            "sai_field": "SAI_ACL_TABLE_ATTR_FIELD_IPMC_NPU_META_DST_HIT"})JSON")
+                                   .dump()},
       {"action/single_param_action", nlohmann::json::parse(R"JSON(
            [{"action": "SAI_PACKET_ACTION_FORWARD"},
             {"action": "QOS_QUEUE", "param": "qos"}])JSON")
@@ -167,6 +245,30 @@ TEST(InsertAclTableDefinition, InsertsAclTableDefinition) {
             {"action": "SAI_PACKET_ACTION_DROP", "packet_color": "SAI_PACKET_COLOR_RED"},
             {"action": "QOS_QUEUE", "param": "qos"}])JSON")
                                                        .dump()},
+      {"action/redirect_action_that_includes_ipmc_type",
+       nlohmann::json::parse(R"JSON(
+           [{"action": "SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT",
+             "param": "multicast_group_id",
+             "object_type": "SAI_OBJECT_TYPE_IPMC_GROUP"}])JSON")
+           .dump()},
+      {"action/redirect_action_that_includes_l2mc_type",
+       nlohmann::json::parse(R"JSON(
+           [{"action": "SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT",
+             "param": "multicast_group_id",
+             "object_type": "SAI_OBJECT_TYPE_L2MC_GROUP"}])JSON")
+           .dump()},
+      {"action/redirect_action_that_includes_next_hop_type",
+       nlohmann::json::parse(R"JSON(
+           [{"action": "SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT",
+             "param": "multicast_group_id",
+             "object_type": "SAI_OBJECT_TYPE_NEXT_HOP"}])JSON")
+           .dump()},
+      {"action/redirect_action_that_includes_port_type",
+       nlohmann::json::parse(R"JSON(
+           [{"action": "SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT",
+             "param": "multicast_group_id",
+             "object_type": "SAI_OBJECT_TYPE_PORT"}])JSON")
+           .dump()},
       {"size", "512"},
       {"meter/unit", "BYTES"},
       {"counter/unit", "BOTH"},
@@ -1447,6 +1549,21 @@ class BadActionParamTest
          R"pb(id: 1
               name: "param"
               annotations: "@sai_action_param(SAI_PACKET_ACTION_DROP)")pb"},
+        {"InvalidRedirectObjectType",
+         R"pb(
+           annotations: "@sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)"
+           annotations: "@sai_action_param_object_type(SAI_OBJECT_TYPE_INVALID)"
+         )pb"},
+        {"TooManySaiObjectTypeArguments",
+         R"pb(
+           annotations: "@sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)"
+           annotations: "@sai_action_param_object_type(SAI_OBJECT_TYPE_PORT, SAI_EXTRA)"
+         )pb"},
+        {"MissingSaiObjectTypeArgument",
+         R"pb(
+           annotations: "@sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)"
+           annotations: "@sai_action_param_object_type()"
+         )pb"},
 #ifdef __EXCEPTIONS
         {"UnknownSaiAction",
          R"pb(id: 1

p4rt_app/tests/lib/p4runtime_grpc_service.cc

@@ -131,6 +131,11 @@ P4RuntimeGrpcService::P4RuntimeGrpcService(const P4RuntimeImplOptions& options)
           &fake_host_stats_table_, kHostStatsTableName),
   };
 
+  // Create FakeWarmBootStateAdapter and save the pointer.
+  auto fake_warm_boot_state_adapter =
+      std::make_unique<p4rt_app::sonic::FakeWarmBootStateAdapter>();
+  fake_warm_boot_state_adapter_ = fake_warm_boot_state_adapter.get();
+
   // Create FakePacketIoInterface and save the pointer.
   auto fake_packetio_interface =
       absl::make_unique<sonic::FakePacketIoInterface>();
@@ -145,8 +150,7 @@ P4RuntimeGrpcService::P4RuntimeGrpcService(const P4RuntimeImplOptions& options)
   p4runtime_server_ = absl::make_unique<P4RuntimeImpl>(
       std::move(p4rt_table), std::move(vrf_table), std::move(hash_table),
       std::move(switch_table), std::move(port_table),
-      std::move(host_stats_table),
-      std::make_unique<p4rt_app::sonic::FakeWarmBootStateAdapter>(),
+      std::move(host_stats_table), std::move(fake_warm_boot_state_adapter),
       std::move(fake_packetio_interface),
      // TODO(PINS): To add fake component_state_helper, system_state_helper and netdev_translator.
      // fake_component_state_helper_, fake_system_state_helper_, fake_netdev_translator_, 
@@ -226,7 +230,7 @@ sonic::FakeSonicDbTable& P4RuntimeGrpcService::GetHostStatsStateDbTable() {
   return fake_host_stats_table_;
 }
 
-sonic::FakeWarmBootStateAdapter&
+sonic::FakeWarmBootStateAdapter*
 P4RuntimeGrpcService::GetWarmBootStateAdapter() {
   return fake_warm_boot_state_adapter_;
 }

p4rt_app/tests/lib/p4runtime_grpc_service.h

@@ -58,7 +58,7 @@ class P4RuntimeGrpcService {
   sonic::FakeSonicDbTable& GetHostStatsStateDbTable();
 
   // Accessor for WarmBootStateAdapter.
-  sonic::FakeWarmBootStateAdapter& GetWarmBootStateAdapter();
+  sonic::FakeWarmBootStateAdapter* GetWarmBootStateAdapter();
 
   // Accessor for PacketIO interface.
   sonic::FakePacketIoInterface& GetFakePacketIoInterface();
@@ -92,7 +92,7 @@ class P4RuntimeGrpcService {
   sonic::FakeSonicDbTable fake_host_stats_table_;
 
   // Faked warm-boot state.
-  sonic::FakeWarmBootStateAdapter fake_warm_boot_state_adapter_;
+  sonic::FakeWarmBootStateAdapter* fake_warm_boot_state_adapter_;
 
   // Faked PacketIO interface.
   sonic::FakePacketIoInterface* fake_packetio_interface_;  // No ownership.

p4rt_app/utils/BUILD.bazel

@@ -24,8 +24,11 @@ cc_library(
         "//gutil:status",
         "//p4_pdpi:ir_cc_proto",
         "//p4_pdpi/utils:annotation_parser",
+        "@com_github_p4lang_p4runtime//:p4info_cc_proto",
         "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
+        "@com_google_protobuf//:protobuf",
         "@sonic_swss_common//:libswsscommon",
     ],
 )
@@ -35,8 +38,11 @@ cc_test(
     srcs = ["table_utility_test.cc"],
     deps = [
         ":table_utility",
+        "//gutil:proto",
+        "//gutil:proto_matchers",
+        "//gutil:status_matchers",
         "//p4_pdpi:ir_cc_proto",
-        "@com_github_google_glog//:glog",
+        "@com_google_absl//absl/status",
         "@com_google_googletest//:gtest_main",
         "@com_google_protobuf//:protobuf",
     ],