Skip to content

Commit

Permalink
PUBLIC: [p4-constraints] Cleanup existing code in p4-constraints cons…
Browse files Browse the repository at this point in the history
…traint_info and interpreter.

Small fixes to existing code that include formatting changes, adding tests for existing methods, adding a method definition with an existing declaration, returning an error when previously no error was returned, and fixing spelling mistakes in comments.

PiperOrigin-RevId: 581364378
  • Loading branch information
PINS Team authored and Bara Kopi committed Nov 13, 2023
1 parent d38d215 commit 7965f52
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 7 deletions.
1 change: 1 addition & 0 deletions p4_constraints/backend/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ cc_test(
":constraint_info",
"//gutils:proto",
"//gutils:status_matchers",
"//third_party/pins_infra/gutil:testing",
"@com_github_p4lang_p4runtime//:p4info_cc_proto",
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/log:check",
Expand Down
7 changes: 7 additions & 0 deletions p4_constraints/backend/constraint_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,13 @@ const TableInfo* GetTableInfoOrNull(const ConstraintInfo& constraint_info,
return &it->second;
}

const ActionInfo* GetActionInfoOrNull(const ConstraintInfo& constraint_info,
uint32_t action_id) {
auto it = constraint_info.action_info_by_id.find(action_id);
if (it == constraint_info.action_info_by_id.end()) return nullptr;
return &it->second;
}

absl::StatusOr<ConstraintInfo> P4ToConstraintInfo(
const p4::config::v1::P4Info& p4info) {
// Allocate output.
Expand Down
50 changes: 49 additions & 1 deletion p4_constraints/backend/constraint_info_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "gutils/proto.h"
#include "gutils/status_matchers.h"
#include "p4/config/v1/p4info.pb.h"
#include "third_party/pins_infra/gutil/testing.h"

using p4::config::v1::P4Info;

Expand All @@ -30,7 +31,11 @@ TEST(P4ToConstraintInfoTest, ValidActionRestrictionSucceeds) {
alias: "act_1"
annotations: "@action_restriction(\"multicast_group_id != 0\")"
}
params { id: 1 name: "multicast_group_id" bitwidth: 16 }
params {
id: 1
name: "multicast_group_id"
bitwidth: 16,
}
}
)pb";

Expand Down Expand Up @@ -106,4 +111,47 @@ Syntax error: @action_restriction must be enclosed in '("' and '")'
ASSERT_TRUE(!constraints.status().ok());
}

TEST(GetTableInfoOrNullTest, ShouldGetNonNullptrToTableInfo) {
// P4Info p4_info;

// ASSERT_OK(gutils::ReadProtoFromString(R"pb(
// tables {
// preamble {
// id: 1
// name: "table",
// }
// }
// )pb",
// &p4_info));

// ASSERT_OK_AND_ASSIGN(ConstraintInfo constraints,
// p4_constraints::P4ToConstraintInfo(p4_info));

// EXPECT_THAT(
// GetTableInfoOrNull(P4ToConstraintInfo(gutil::ParseProtoOrDie<P4Info>(R"pb(
// tables {
// preamble {
// id: 1
// name: "table",
// }
// }
// )pb")),
// 1),
// IsOkAndHolds(Not(Eq(nullptr))));

ASSERT_NE(GetTableInfoOrNull(
*P4ToConstraintInfo(gutil::ParseProtoOrDie<P4Info>(R"pb(
tables {
preamble {
id: 1
name: "table",
}
}
)pb")),
1),
nullptr);

// ASSERT_NE(GetTableInfoOrNull(constraints, 1), nullptr);
}

} // namespace p4_constraints
12 changes: 6 additions & 6 deletions p4_constraints/backend/interpreter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ absl::StatusOr<TableEntry> ParseEntry(const p4::v1::TableEntry& entry,
}

// Use default value for omitted keys.
// See Section 9.1.1. of the P4runtime specification.
// See Section 9.1.1. of the P4Runtime specification.
for (const auto& [name, key_info] : table_info.keys_by_name) {
if (keys.contains(name)) continue;
switch (key_info.type.type_case()) {
Expand Down Expand Up @@ -535,7 +535,7 @@ absl::StatusOr<const Expression*> MinimalSubexpressionLeadingToEvalResult(
*candidate_subexpressions[0], context, eval_cache, size_cache);
}
// Returns the `MinimalSubexpressionLeadingToEvalResult` from the
// candidate who has the smallest such subexpresson.
// candidate who has the smallest such subexpression.
ASSIGN_OR_RETURN(auto* subexpression_0,
MinimalSubexpressionLeadingToEvalResult(
*candidate_subexpressions[0], context,
Expand Down Expand Up @@ -628,10 +628,10 @@ absl::StatusOr<EvalResult> Eval_(const Expression& expr,
case Expression::kKey: {
auto it = context.entry.keys.find(expr.key());
if (it == context.entry.keys.end()) {
RuntimeTypeError(context.source, expr.start_location(),
expr.end_location())
<< "unknown key " << expr.key() << " in table "
<< context.entry.table_name;
return RuntimeTypeError(context.source, expr.start_location(),
expr.end_location())
<< "unknown key " << expr.key() << " in table "
<< context.entry.table_name;
}
return it->second;
}
Expand Down

0 comments on commit 7965f52

Please sign in to comment.