[Comb] Add parsers for @refers_to and @referenced_by Return empty lis…

…t instead of KNotFound when annotation does not exist.[pdpi] Refactor code base to use annotation library for refers_to annotation. (sonic-net#277) * cleanup some header files.Internal Code Change.Don't craft packets with UDP headers if UDP port is already reserved. * [SFE-P4][Garbage Collection] Support multicast entity as input to garbage collection.Improve error messages of InstallPdTableEntr{y, ies}.[pdpi] Convert referring optionals to exacts in test program.[pdpi] Add IrTableEntryReference to P4info.[pdpi] Add conversions from/to string for IrBuiltIns * [pdpi] Add parsers for @refers_to and @referenced_by Return empty list instead of KNotFound when annotation does not exist.[pdpi] Refactor code base to use annotation library for refers_to annotation. --------- Co-authored-by: rhalstea <[email protected]> Co-authored-by: Srikishen Pondicherry Shanmugam <[email protected]>
rkavitha-hcl · Jul 1, 2024 · c478b38 · c478b38
1 parent 5b99679
commit c478b38
Show file tree

Hide file tree

Showing 10 changed files with 243 additions and 89 deletions.
diff --git a/p4_pdpi/BUILD.bazel b/p4_pdpi/BUILD.bazel
@@ -47,6 +47,37 @@ cc_test(
     ],
 )
 
+cc_library(
+    name = "reference_annotations",
+    srcs = [
+        "reference_annotations.cc",
+    ],
+    hdrs = [
+        "reference_annotations.h",
+    ],
+    deps = [
+        "//gutil:status",
+        "//p4_pdpi/utils:annotation_parser",
+        "@com_google_absl//absl/status:statusor",
+        "@com_google_protobuf//:protobuf",
+    ],
+)
+
+cc_test(
+    name = "reference_annotations_test",
+    srcs = [
+        "reference_annotations_test.cc",
+    ],
+    deps = [
+        ":reference_annotations",
+        "//gutil:status_matchers",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/strings",
+        "@com_google_googletest//:gtest_main",
+        "@com_google_protobuf//:protobuf",
+    ],
+)
+
 cc_library(
     name = "sequencing_util",
     srcs = ["sequencing_util.cc"],
@@ -221,6 +252,7 @@ cc_library(
     local_defines = ["PDPI_DISABLE_TRANSLATION_OPTIONS_DEFAULT"],
     deps = [
         ":ir_cc_proto",
+        ":reference_annotations",
         ":translation_options",
         "//gutil:collections",
         "//gutil:proto",

diff --git a/p4_pdpi/ir.cc b/p4_pdpi/ir.cc
@@ -44,6 +44,7 @@
 #include "p4/config/v1/p4types.pb.h"
 #include "p4/v1/p4runtime.pb.h"
 #include "p4_pdpi/ir.pb.h"
+#include "p4_pdpi/reference_annotations.h"
 #include "p4_pdpi/translation_options.h"
 #include "p4_pdpi/utils/ir.h"
 
@@ -62,6 +63,7 @@ using ::pdpi::IrActionInvocation;
 using ::pdpi::IrMatchFieldDefinition;
 using ::pdpi::IrP4Info;
 using ::pdpi::IrTableDefinition;
+using ::pdpi::ParsedRefersToAnnotation;
 
 namespace {
 
@@ -228,35 +230,27 @@ absl::StatusOr<uint32_t> MatchFieldNameToId(
 absl::StatusOr<std::vector<IrMatchFieldReference>> GetRefersToAnnotations(
     const p4::config::v1::P4Info &p4info,
     const ::google::protobuf::RepeatedPtrField<std::string> &annotations) {
-  constexpr absl::string_view kError = "Found invalid @refers_to annotation: ";
   std::vector<IrMatchFieldReference> result;
-  for (absl::string_view annotation_contents : annotations) {
-    if (absl::ConsumePrefix(&annotation_contents, "@refers_to(")) {
-      if (!absl::ConsumeSuffix(&annotation_contents, ")")) {
-        return gutil::InvalidArgumentErrorBuilder() << kError << "Missing )";
-      }
-      std::vector<std::string> parts = absl::StrSplit(annotation_contents, ',');
-      if (parts.size() != 2) {
-        return gutil::InvalidArgumentErrorBuilder()
-               << kError << "Incorrect number of arguments, required 2 but got "
-               << parts.size() << " instead.";
-      }
+  ASSIGN_OR_RETURN(
+      const std::vector<ParsedRefersToAnnotation> refers_to_annotations,
+      ParseAllRefersToAnnotations(annotations));
 
-      absl::string_view table = absl::StripAsciiWhitespace(parts[0]);
-      absl::string_view match_field = absl::StripAsciiWhitespace(parts[1]);
+  for (const auto &refers_to_annotation : refers_to_annotations) {
+    absl::string_view table = refers_to_annotation.table;
+    absl::string_view match_field = refers_to_annotation.field;
 
-      ASSIGN_OR_RETURN(uint32_t table_id, TableAliasToId(p4info, table));
-      ASSIGN_OR_RETURN(uint32_t match_field_id,
-                       MatchFieldNameToId(p4info, table_id, match_field));
+    ASSIGN_OR_RETURN(uint32_t table_id, TableAliasToId(p4info, table));
+    ASSIGN_OR_RETURN(uint32_t match_field_id,
+                     MatchFieldNameToId(p4info, table_id, match_field));
 
-      IrMatchFieldReference reference;
-      reference.set_table(std::string(table));
-      reference.set_match_field(std::string(match_field));
-      reference.set_table_id(table_id);
-      reference.set_match_field_id(match_field_id);
-      result.push_back(reference);
-    }
+    IrMatchFieldReference reference;
+    reference.set_table(std::string(table));
+    reference.set_match_field(std::string(match_field));
+    reference.set_table_id(table_id);
+    reference.set_match_field_id(match_field_id);
+    result.push_back(reference);
   }
+
   return result;
 }
 

diff --git a/p4_pdpi/reference_annotations.cc b/p4_pdpi/reference_annotations.cc
@@ -0,0 +1,52 @@
+#include "p4_pdpi/reference_annotations.h"
+
+#include <string>
+#include <vector>
+
+#include "absl/status/statusor.h"
+#include "google/protobuf/repeated_ptr_field.h"
+#include "gutil/status.h"
+#include "p4_pdpi/utils/annotation_parser.h"
+
+namespace pdpi {
+
+using google::protobuf::RepeatedPtrField;
+
+namespace {
+
+absl::StatusOr<ParsedRefersToAnnotation> ParseAsRefersToAnnotation(
+    std::string body) {
+  ASSIGN_OR_RETURN(auto arg_list, annotation::ParseAsArgList(body));
+  if (arg_list.size() != 2) {
+    return gutil::InvalidArgumentErrorBuilder()
+           << "Invalid argument. Expected 2 args, but got " << arg_list.size();
+  }
+  return ParsedRefersToAnnotation{.table = arg_list[0], .field = arg_list[1]};
+}
+
+absl::StatusOr<ParsedReferencedByAnnotation> ParseAsReferencedByAnnotation(
+    std::string body) {
+  ASSIGN_OR_RETURN(auto arg_list, annotation::ParseAsArgList(body));
+  if (arg_list.size() != 2) {
+    return gutil::InvalidArgumentErrorBuilder()
+           << "Invalid argument. Expected 2 args, but got " << arg_list.size();
+  }
+  return ParsedReferencedByAnnotation{.table = arg_list[0],
+                                      .field = arg_list[1]};
+}
+
+}  // namespace
+
+absl::StatusOr<std::vector<ParsedRefersToAnnotation>>
+ParseAllRefersToAnnotations(const RepeatedPtrField<std::string>& annotations) {
+  return GetAllParsedAnnotations<ParsedRefersToAnnotation>(
+      "refers_to", annotations, ParseAsRefersToAnnotation);
+}
+
+absl::StatusOr<std::vector<ParsedReferencedByAnnotation>>
+ParseAllReferencedByAnnotations(
+    const RepeatedPtrField<std::string>& annotations) {
+  return GetAllParsedAnnotations<ParsedReferencedByAnnotation>(
+      "referenced_by", annotations, ParseAsReferencedByAnnotation);
+}
+}  // namespace pdpi
diff --git a/p4_pdpi/reference_annotations.h b/p4_pdpi/reference_annotations.h
@@ -0,0 +1,42 @@
+#ifndef PINS_P4_PDPI_REFERENCE_ANNOTATIONS_H_
+#define PINS_P4_PDPI_REFERENCE_ANNOTATIONS_H_
+
+#include <string>
+#include <vector>
+
+#include "absl/status/statusor.h"
+#include "google/protobuf/repeated_ptr_field.h"
+
+namespace pdpi {
+
+// Parsed contents of an `@refers_to(table, field)` annotation.
+struct ParsedRefersToAnnotation {
+  std::string table;
+  std::string field;
+};
+
+// Contents of an `@reference_by(table, field)` annotation.
+struct ParsedReferencedByAnnotation {
+  std::string table;
+  std::string field;
+};
+
+// Returns a list of `RefersToAnnotation` parsed from the `annotations`.
+// Returns empty list if no annotation contained the label `@refers_to`.
+// Return InvalidArgument if any annotation with the label `@refers_to` did not
+// contain exactly 2 arguments.
+absl::StatusOr<std::vector<ParsedRefersToAnnotation>>
+ParseAllRefersToAnnotations(
+    const google::protobuf::RepeatedPtrField<std::string>& annotations);
+
+// Returns a list of `ReferencedByAnnotation`s parsed from the `annotations`.
+// Returns empty list if no annotation contained the label `@referenced_by`.
+// Return InvalidArgument if any annotation with the label `@referenced_by` did
+// not contain exactly 2 arguments.
+absl::StatusOr<std::vector<ParsedReferencedByAnnotation>>
+ParseAllReferencedByAnnotations(
+    const google::protobuf::RepeatedPtrField<std::string>& annotations);
+
+}  // namespace pdpi
+
+#endif  // PINS_P4_PDPI_REFERENCE_ANNOTATIONS_H_
diff --git a/p4_pdpi/reference_annotations_test.cc b/p4_pdpi/reference_annotations_test.cc
@@ -0,0 +1,65 @@
+#include "p4_pdpi/reference_annotations.h"
+
+#include <string>
+
+#include "absl/status/status.h"
+#include "absl/strings/substitute.h"
+#include "gmock/gmock.h"
+#include "google/protobuf/repeated_ptr_field.h"
+#include "gtest/gtest.h"
+#include "gutil/status_matchers.h"
+
+namespace pdpi {
+namespace {
+
+using ::gutil::IsOkAndHolds;
+using ::gutil::StatusIs;
+using ::testing::ElementsAre;
+
+// -- Matchers -----------------------------------------------------------------
+
+MATCHER_P2(HasTableAndField, table, field,
+           absl::Substitute("Matches: [table: $0, field: $1]", table, field)) {
+  return arg.table == table && arg.field == field;
+}
+
+// -- Tests --------------------------------------------------------------------
+
+TEST(ParseAllRefersToAnnotationArgs, ReturnsAllRefersToAnnotations) {
+  google::protobuf::RepeatedPtrField<std::string> annotations;
+  annotations.Add("@refers_to(a,b)");
+  annotations.Add("@referenced_by(c,d)");
+  annotations.Add("@refers_to(\n        e,f  \t)");
+  EXPECT_THAT(ParseAllRefersToAnnotations(annotations),
+              IsOkAndHolds(ElementsAre(HasTableAndField("a", "b"),
+                                       HasTableAndField("e", "f"))));
+}
+
+TEST(ParseAllRefersToAnnotationArgs,
+     FailsWithRefersToWithMoreThanTwoArguments) {
+  google::protobuf::RepeatedPtrField<std::string> annotations;
+  annotations.Add("@refers_to(a,b,c)");
+  EXPECT_THAT(ParseAllRefersToAnnotations(annotations),
+              StatusIs(absl::StatusCode::kInvalidArgument));
+}
+
+TEST(ParseAllReferencedByAnnotationArgs, ReturnsAllRefersToAnnotations) {
+  google::protobuf::RepeatedPtrField<std::string> annotations;
+  annotations.Add("@referenced_by(a,b)");
+  annotations.Add("@refers_to(c,d)");
+  annotations.Add("@referenced_by(\n        e,f  \t)");
+  EXPECT_THAT(ParseAllReferencedByAnnotations(annotations),
+              IsOkAndHolds(ElementsAre(HasTableAndField("a", "b"),
+                                       HasTableAndField("e", "f"))));
+}
+
+TEST(ParseAllReferencedByAnnotationArgs,
+     FailsWithReferencedByWithMoreThanTwoArguments) {
+  google::protobuf::RepeatedPtrField<std::string> annotations;
+  annotations.Add("@referenced_by(a,b,c)");
+  EXPECT_THAT(ParseAllReferencedByAnnotations(annotations),
+              StatusIs(absl::StatusCode::kInvalidArgument));
+}
+
+}  // namespace
+}  // namespace pdpi
diff --git a/p4_pdpi/utils/BUILD.bazel b/p4_pdpi/utils/BUILD.bazel
@@ -37,7 +37,7 @@ cc_test(
     srcs = ["annotation_parser_test.cc"],
     deps = [
         ":annotation_parser",
-        "//gutil:status",
+        "//gutil:status_matchers",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",

diff --git a/p4_pdpi/utils/annotation_parser.cc b/p4_pdpi/utils/annotation_parser.cc
@@ -26,7 +26,9 @@
 
 namespace pdpi {
 namespace {
-inline bool IsWhitespace(char c) { return c == ' ' || c == '\t'; }
+inline bool IsWhitespace(char c) {
+  return c == ' ' || c == '\t' || c == '\n' || c == '\r';
+}
 }  // namespace
 
 // Use a custom parser for improved speed and error reporting compared to a
@@ -147,6 +149,8 @@ absl::StatusOr<std::vector<std::string>> ParseAsArgList(std::string value) {
           break;
         case '\t':
         case ' ':
+        case '\n':
+        case '\r':
           continue;
         case ')':
         case ']':

diff --git a/p4_pdpi/utils/annotation_parser.h b/p4_pdpi/utils/annotation_parser.h
@@ -84,7 +84,6 @@ std::vector<annotation::AnnotationComponents> GetAllAnnotations(
 }
 
 // Returns a list of the parsed body of all annotations with the given label.
-// Returns a Status with code kNotFound if there is no matching annotation.
 //
 // Note: Currently, template type deduction does not work for this function. To
 // use GetAllParsedAnnotations, declare it with the parser's StatusOr<T> T type.
@@ -110,10 +109,6 @@ absl::StatusOr<std::vector<T>> GetAllParsedAnnotations(
       values.push_back(value);
     }
   }
-  if (values.empty()) {
-    return gutil::NotFoundErrorBuilder()
-           << "No annotation contained label \"" << label << "\"";
-  }
   return values;
 }
 
@@ -136,6 +131,10 @@ absl::StatusOr<T> GetParsedAnnotation(absl::string_view label,
   // precompiler from splitting GetAllParsedAnnotations<T into its own argument.
   ASSIGN_OR_RETURN(auto values, (GetAllParsedAnnotations<T, Container>(
                                     label, annotations, parser)));
+  if (values.empty()) {
+    return gutil::NotFoundErrorBuilder()
+           << "No annotations contained label \"" << label << "\"";
+  }
   if (values.size() > 1) {
     return gutil::InvalidArgumentErrorBuilder()
            << "Multiple annotations contained label \"" << label << "\"";
@@ -155,7 +154,6 @@ absl::StatusOr<std::vector<std::string>> GetAnnotationAsArgList(
 
 // Returns the string list form of the body of all annotations with the given
 // label.
-// Returns a Status with code kNotFound if there is no matching annotation.
 template <typename Container>
 absl::StatusOr<std::vector<std::vector<std::string>>>
 GetAllAnnotationsAsArgList(absl::string_view label,
@@ -173,7 +171,6 @@ absl::StatusOr<std::string> GetAnnotationBody(absl::string_view label,
 }
 
 // Returns all annotation bodies from all annotations with the given label.
-// Returns a Status with code kNotFound if there is no matching annotation.
 template <typename Container>
 absl::StatusOr<std::vector<std::string>> GetAllAnnotationBodies(
     absl::string_view label, const Container& annotations) {