From 071abdc2220a64fe46c8fec7d8b4ca58a06e51f8 Mon Sep 17 00:00:00 2001 From: Angela Zhang Date: Tue, 21 Nov 2023 11:35:22 -0800 Subject: [PATCH] PUBLIC: [p4-constraints] Add support for action restrictions in the interpreter. (#115) In this CL, ReasonEntryViolatesConstraint was extended to check action constraints for a given table entry for actions and action profile action sets. EvaluationContext has been refactored to include both TableEntry and ActionInvocation information for evaluation. Please see go/supporting-actions-in-p4-constraints for more details. PiperOrigin-RevId: 579986405 Co-authored-by: PINS Team --- p4_constraints/BUILD.bazel | 1 + p4_constraints/backend/BUILD.bazel | 1 + p4_constraints/backend/interpreter.cc | 306 +++++++++++---- p4_constraints/backend/interpreter.h | 61 ++- .../backend/interpreter_golden_test_runner.cc | 32 +- p4_constraints/backend/interpreter_test.cc | 360 +++++++++++++++++- p4_constraints/quote.cc | 47 ++- 7 files changed, 689 insertions(+), 119 deletions(-) diff --git a/p4_constraints/BUILD.bazel b/p4_constraints/BUILD.bazel index 6aebb04..7e3a20e 100644 --- a/p4_constraints/BUILD.bazel +++ b/p4_constraints/BUILD.bazel @@ -64,6 +64,7 @@ cc_library( ":ast_cc_proto", ":constraint_source", "//gutils:proto", + "//gutils:source_location", "//gutils:status", "@com_google_absl//absl/log", "@com_google_absl//absl/status:statusor", diff --git a/p4_constraints/backend/BUILD.bazel b/p4_constraints/backend/BUILD.bazel index 75fcec5..6e73fb8 100644 --- a/p4_constraints/backend/BUILD.bazel +++ b/p4_constraints/backend/BUILD.bazel @@ -103,6 +103,7 @@ cc_test( linkstatic = True, deps = [ ":constraint_info", + ":errors", ":interpreter", "//gutils:ordered_map", "//gutils:parse_text_proto", diff --git a/p4_constraints/backend/interpreter.cc b/p4_constraints/backend/interpreter.cc index 51e177a..a603d33 100644 --- a/p4_constraints/backend/interpreter.cc +++ b/p4_constraints/backend/interpreter.cc @@ -184,8 +184,8 @@ absl::StatusOr> ParseKey( } } -absl::StatusOr ParseEntry(const p4::v1::TableEntry& entry, - const TableInfo& table_info) { +absl::StatusOr ParseTableEntry( + const p4::v1::TableEntry& entry, const TableInfo& table_info) { absl::flat_hash_map keys; // Parse all keys that are explicitly present. @@ -234,10 +234,45 @@ absl::StatusOr ParseEntry(const p4::v1::TableEntry& entry, << key_info.type.DebugString(); } - return TableEntry{ + TableEntry table_entry{ .table_name = table_info.name, .priority = entry.priority(), - .keys = keys, + .keys = std::move(keys), + }; + + return EvaluationContext{ + .constraint_context = std::move(table_entry), + .constraint_source = table_info.constraint_source, + }; +} + +absl::StatusOr ParseAction(const p4::v1::Action& action, + const ActionInfo& action_info) { + absl::flat_hash_map action_parameters; + + // Parse action parameters. + for (const p4::v1::Action_Param& param : action.params()) { + int32_t param_id = param.param_id(); + Integer param_value = ParseP4RTInteger(param.value()); + auto it = action_info.params_by_id.find(param_id); + if (it == action_info.params_by_id.end()) { + return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC) + << "unknown action param with ID " << P4IDToString(param_id); + } + if (action_parameters.contains(it->second.name)) { + return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC) + << "duplicate action param with ID " << P4IDToString(param_id); + } + action_parameters[it->second.name] = param_value; + } + ActionInvocation action_invocation{ + .action_id = action.action_id(), + .action_name = action_info.name, + .action_parameters = std::move(action_parameters), + }; + return EvaluationContext{ + .constraint_context = std::move(action_invocation), + .constraint_source = action_info.constraint_source, }; } @@ -258,7 +293,7 @@ absl::StatusOr EvalToBool(const Expression& expr, eval_cache->insert({&expr, absl::get(result)}); return absl::get(result); } else { - return RuntimeTypeError(context.source, expr.start_location(), + return RuntimeTypeError(context.constraint_source, expr.start_location(), expr.end_location()) << "expected expression of type bool"; } @@ -271,7 +306,7 @@ absl::StatusOr EvalToInt(const Expression& expr, ASSIGN_OR_RETURN(EvalResult result, Eval(expr, context, eval_cache)); if (absl::holds_alternative(result)) return absl::get(result); - return RuntimeTypeError(context.source, expr.start_location(), + return RuntimeTypeError(context.constraint_source, expr.start_location(), expr.end_location()) << "expected expression of integral type"; } @@ -325,7 +360,7 @@ absl::StatusOr EvalAndCastTo(const Type& type, break; } } - return RuntimeTypeError(context.source, expr.start_location(), + return RuntimeTypeError(context.constraint_source, expr.start_location(), expr.end_location()) << "cannot cast expression of type " << expr.type() << " to type " << type; @@ -563,33 +598,63 @@ absl::StatusOr ExplainConstraintViolation( expr, context, eval_cache, size_cache)); ASSIGN_OR_RETURN(bool truth_value, EvalToBool(*explanation, context, &eval_cache)); - ASSIGN_OR_RETURN( - std::string reason, - QuoteSubConstraint(context.source, explanation->start_location(), - explanation->end_location())); - absl::flat_hash_set relevant_fields = + ASSIGN_OR_RETURN(std::string reason, + QuoteSubConstraint(context.constraint_source, + explanation->start_location(), + explanation->end_location())); + + const absl::flat_hash_set relevant_fields = ast::GetMatchFields(*explanation); - // Ordered for determinism when golden testing. - std::string key_info = absl::StrJoin( - gutils::Ordered(context.entry.keys), "", - [&](std::string* out, const std::pair& pair) { - if (relevant_fields.contains(pair.first)) { - absl::StrAppend(out, "Field: \"", pair.first, - "\" -> Value: ", EvalResultToString(pair.second), - "\n"); - } - }); - - return absl::StrFormat( - "All entries must %ssatisfy:" - "\n\n%s\n" - "But your entry does%s.\n" - ">>>Relevant Entry Info<<<\n" - "Table Name: \"%s\"\n" - "Priority:%d\n" - "%s", - (truth_value ? "not " : ""), reason, (truth_value ? "" : " not"), - context.entry.table_name, context.entry.priority, key_info); + return std::visit( + gutils::Overload{ + [&](const TableEntry& table_entry) -> std::string { + // Ordered for determinism when golden testing. + std::string key_info = absl::StrJoin( + gutils::Ordered(table_entry.keys), "", + [&](std::string* out, + const std::pair& pair) { + if (relevant_fields.contains(pair.first)) { + absl::StrAppend( + out, "Field: \"", pair.first, + "\" -> Value: ", EvalResultToString(pair.second), "\n"); + } + }); + return absl::StrFormat( + "All entries must %ssatisfy:" + "\n\n%s\n" + "But your entry does%s.\n" + ">>>Relevant Entry Info<<<\n" + "Table Name: \"%s\"\n" + "Priority:%d\n" + "%s", + (truth_value ? "not " : ""), reason, + (truth_value ? "" : " not"), table_entry.table_name, + table_entry.priority, key_info); + }, + [&](const ActionInvocation& action_invocation) -> std::string { + std::string param_info = absl::StrJoin( + gutils::Ordered(action_invocation.action_parameters), "", + [&](std::string* out, + const std::pair& pair) { + if (relevant_fields.contains(pair.first)) { + absl::StrAppend(out, "Param name: \"", pair.first, + "\" -> Value: ", pair.second.get_str(), + "\n"); + } + }); + return absl::StrFormat( + "All actions must %ssatisfy:" + "\n\n%s\n" + "But your entry does%s.\n" + ">>>Relevant Action Info<<<\n" + "Action Name: \"%s\"\n" + "%s", + (truth_value ? "not " : ""), reason, + (truth_value ? "" : " not"), action_invocation.action_name, + param_info); + }, + }, + context.constraint_context); } // -- Main evaluator ----------------------------------------------------------- @@ -614,29 +679,59 @@ absl::StatusOr Eval_(const Expression& expr, } case Expression::kKey: { - auto it = context.entry.keys.find(expr.key()); - if (it == context.entry.keys.end()) { - return RuntimeTypeError(context.source, expr.start_location(), - expr.end_location()) + const TableEntry* table_entry = + std::get_if(&context.constraint_context); + if (table_entry == nullptr) { + return RuntimeTypeError(context.constraint_source, + expr.start_location(), expr.end_location()) + << "Found a reference to a key in an action constraint."; + } + + auto it = table_entry->keys.find(expr.key()); + if (it == table_entry->keys.end()) { + return RuntimeTypeError(context.constraint_source, + expr.start_location(), expr.end_location()) << "unknown key " << expr.key() << " in table " - << context.entry.table_name; + << table_entry->table_name; } return it->second; } case Expression::kActionParameter: { - return absl::UnimplementedError( - "TODO: b/293656077 - Support action constraints"); + const ActionInvocation* action_invocation = + std::get_if(&context.constraint_context); + if (action_invocation == nullptr) { + return RuntimeTypeError(context.constraint_source, + expr.start_location(), expr.end_location()) + << "Found a reference to an action parameter in a table " + "constraint."; + } + auto it = + action_invocation->action_parameters.find(expr.action_parameter()); + if (it == action_invocation->action_parameters.end()) { + return RuntimeTypeError(context.constraint_source, + expr.start_location(), expr.end_location()) + << "unknown action parameter " << expr.action_parameter() + << " in action " << action_invocation->action_name; + } + return it->second; } case Expression::kAttributeAccess: { + const TableEntry* table_entry = + std::get_if(&context.constraint_context); + if (table_entry == nullptr) { + return RuntimeTypeError(context.constraint_source, + expr.start_location(), expr.end_location()) + << "The constraint context does not contain a TableEntry."; + } const std::string attribute_name = expr.attribute_access().attribute_name(); if (attribute_name == "priority") { - return Integer(context.entry.priority); + return Integer(table_entry->priority); } else { - return RuntimeTypeError(context.source, expr.start_location(), - expr.end_location()) + return RuntimeTypeError(context.constraint_source, + expr.start_location(), expr.end_location()) << "unknown attribute '" << attribute_name << "'"; } } @@ -672,8 +767,8 @@ absl::StatusOr Eval_(const Expression& expr, absl::StatusOr result = absl::visit(EvalFieldAccess{.field = field}, composite_value); if (!result.ok()) { - return RuntimeTypeError(context.source, expr.start_location(), - expr.end_location()) + return RuntimeTypeError(context.constraint_source, + expr.start_location(), expr.end_location()) << result.status().message(); } return result; @@ -732,57 +827,124 @@ absl::StatusOr Eval(const Expression& expr, const EvaluationContext& context, EvaluationCache* eval_cache) { ASSIGN_OR_RETURN(EvalResult result, Eval_(expr, context, eval_cache)); - RETURN_IF_ERROR(DynamicTypeCheck(context.source, expr, result)); + RETURN_IF_ERROR(DynamicTypeCheck(context.constraint_source, expr, result)); return result; } +absl::StatusOr ReasonEntryViolatesConstraint( + const p4::v1::Action& action, const ConstraintInfo& constraint_info) { + const uint32_t action_id = action.action_id(); + auto* action_info = GetActionInfoOrNull(constraint_info, action_id); + if (action_info == nullptr) { + return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC) + << "action entry with unknown action ID " << P4IDToString(action_id); + } + // Check if action has an action restriction. + if (!action_info->constraint.has_value()) return ""; + + const Expression& constraint = *action_info->constraint; + if (constraint.type().type_case() != Type::kBoolean) { + return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC) + << "action " << action_info->name + << " has non-boolean constraint: " << constraint.DebugString(); + } + + EvaluationCache eval_cache; + ast::SizeCache size_cache; + ASSIGN_OR_RETURN(const EvaluationContext eval_context, + ParseAction(action, *action_info), + _ << " while parsing P4RT table entry for action '" + << action_info->name << "':"); + + ASSIGN_OR_RETURN(bool entry_meets_constraint, + EvalToBool(constraint, eval_context, &eval_cache)); + if (!entry_meets_constraint) { + return ExplainConstraintViolation(constraint, eval_context, eval_cache, + size_cache); + } + return ""; +} + +absl::StatusOr ReasonEntryViolatesConstraint( + const p4::v1::ActionProfileActionSet& action_set, + const ConstraintInfo& constraint_info) { + for (const p4::v1::ActionProfileAction& action_profile_action : + action_set.action_profile_actions()) { + ASSIGN_OR_RETURN(std::string reason, + ReasonEntryViolatesConstraint( + action_profile_action.action(), constraint_info)); + if (!reason.empty()) return reason; + } + return ""; +} + } // namespace internal_interpreter // -- Public interface --------------------------------------------------------- absl::StatusOr ReasonEntryViolatesConstraint( - const p4::v1::TableEntry& entry, const ConstraintInfo& context) { - using ::p4_constraints::ast::SizeCache; + const p4::v1::TableEntry& entry, const ConstraintInfo& constraint_info) { using ::p4_constraints::internal_interpreter::EvalToBool; using ::p4_constraints::internal_interpreter::EvaluationCache; using ::p4_constraints::internal_interpreter::EvaluationContext; using ::p4_constraints::internal_interpreter::ExplainConstraintViolation; using ::p4_constraints::internal_interpreter::P4IDToString; - using ::p4_constraints::internal_interpreter::ParseEntry; + using ::p4_constraints::internal_interpreter::ParseAction; + using ::p4_constraints::internal_interpreter::ParseTableEntry; using ::p4_constraints::internal_interpreter::TableEntry; // Find table associated with entry. - auto* table_info = GetTableInfoOrNull(context, entry.table_id()); + auto* table_info = GetTableInfoOrNull(constraint_info, entry.table_id()); if (table_info == nullptr) { return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC) << "table entry with unknown table ID " << P4IDToString(entry.table_id()); } // Check if entry satisfies table constraint (if present). - if (!table_info->constraint.has_value()) return ""; - const Expression& constraint = table_info->constraint.value(); - if (constraint.type().type_case() != Type::kBoolean) { - return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC) - << "table " << table_info->name - << " has non-boolean constraint: " << constraint.DebugString(); + if (table_info->constraint.has_value()) { + const Expression& constraint = table_info->constraint.value(); + if (constraint.type().type_case() != Type::kBoolean) { + return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC) + << "table " << table_info->name + << " has non-boolean constraint: " << constraint.DebugString(); + } + // Parse entry and check constraint. + ASSIGN_OR_RETURN(const EvaluationContext eval_context, + ParseTableEntry(entry, *table_info), + _ << " while parsing P4RT table entry for table '" + << table_info->name << "':"); + EvaluationCache eval_cache; + ast::SizeCache size_cache; + ASSIGN_OR_RETURN(bool entry_satisfies_constraint, + EvalToBool(constraint, eval_context, &eval_cache)); + + if (!entry_satisfies_constraint) { + return ExplainConstraintViolation(constraint, eval_context, eval_cache, + size_cache); + } } - // Parse entry and check constraint. - ASSIGN_OR_RETURN(const TableEntry parsed_entry, - ParseEntry(entry, *table_info), - _ << " while parsing P4RT table entry for table '" - << table_info->name << "':"); - EvaluationContext eval_context{ - .entry = parsed_entry, - .source = table_info->constraint_source, - }; - EvaluationCache eval_cache; - ASSIGN_OR_RETURN(bool entry_satisfies_constraint, - EvalToBool(constraint, eval_context, &eval_cache)); - if (entry_satisfies_constraint) return ""; - SizeCache size_cache; - return ExplainConstraintViolation(constraint, eval_context, eval_cache, - size_cache); + if (!entry.has_action()) return ""; + + switch (entry.action().type_case()) { + case p4::v1::TableAction::kAction: + return internal_interpreter::ReasonEntryViolatesConstraint( + entry.action().action(), constraint_info); + case p4::v1::TableAction::kActionProfileMemberId: + case p4::v1::TableAction::kActionProfileGroupId: + return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC) + << "action restrictions not supported for entries with the given " + "kind of action: " + << entry.DebugString(); + case p4::v1::TableAction::kActionProfileActionSet: { + return internal_interpreter::ReasonEntryViolatesConstraint( + entry.action().action_profile_action_set(), constraint_info); + } + case p4::v1::TableAction::TYPE_NOT_SET: + break; + } + return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC) + << "unknown action type " << entry.action().type_case(); } } // namespace p4_constraints diff --git a/p4_constraints/backend/interpreter.h b/p4_constraints/backend/interpreter.h index 178f5da..7fcf9ad 100644 --- a/p4_constraints/backend/interpreter.h +++ b/p4_constraints/backend/interpreter.h @@ -36,16 +36,14 @@ namespace p4_constraints { -// Checks if a given table entry satisfies the entry constraint attached to its -// associated table. Returns the empty string if this is the case, or a -// human-readable nonempty string explaining why it is not the case otherwise. -// This function has the same runtime on successes as `EntryMeetsConstraint` -// and longer runtimes on failure (by a constant factor) and should generally be -// preferred. Returns an InvalidArgument Status if the entry belongs to a table -// not present in ConstraintInfo, or if it is inconsistent with the table +// Checks if a given table entry satisfies the entry/action(s) constraint(s) +// attached to its associated table/action(s). Returns the empty string if this +// is the case or a human-readable nonempty string explaining why it is not the +// case otherwise. Returns an InvalidArgument Status if the entry/action(s) +// don't belong in ConstraintInfo or is inconsistent with the table/action(s) // definition in ConstraintInfo. absl::StatusOr ReasonEntryViolatesConstraint( - const p4::v1::TableEntry& entry, const ConstraintInfo& context); + const p4::v1::TableEntry& entry, const ConstraintInfo& constraint_info); // -- END OF PUBLIC INTERFACE -------------------------------------------------- @@ -141,17 +139,23 @@ struct TableEntry { // In contrast to p4::v1::TableEntry, all keys must be present, i.e. this must // be a total map from key names to values. absl::flat_hash_map keys; - // TODO(smolkaj): once we support actions, they will be added here. }; -// Parses p4::v1::TableEntry -absl::StatusOr ParseEntry(const p4::v1::TableEntry& entry, - const TableInfo& table_info); +// Parsed representation of p4::v1::Action. +struct ActionInvocation { + uint32_t action_id; + std::string action_name; + // Map of param names to param values. + absl::flat_hash_map action_parameters; +}; // Context under which an `Expression` is evaluated. An `EvaluationContext` is // "valid" for a given `Expression` iff the following holds: -// -`entry` must contain all fields in the expression, with correct type. If -// not, an Error Status is returned +// - If the `Expression` being evaluated is a Table constraint, then +// `constraint_context` is a TableEntry type. If the `Expression` being +// evaluated is an Action constraint, then `constraint_context` is an +// ActionInvocation type. `constraint_context` must contain all fields in the +// expression, with correct type. If not, an Error Status is returned. // -`source` must be the source from which the expression was parsed. If not, // behaviour is undefined (depending on the source, either an InternalError // will be given or a non-sense quote will be returned) @@ -160,17 +164,32 @@ absl::StatusOr ParseEntry(const p4::v1::TableEntry& entry, // expensive copies. This leads to the possibility of dangling references, use // with caution. struct EvaluationContext { - const TableEntry& entry; - const ConstraintSource& source; + std::variant constraint_context; + const ConstraintSource& constraint_source; }; +// Parses p4::v1::TableEntry into an EvaluationContext using keys, table name, +// and constraint source from table_info. Returns InvalidArgument if it is not +// possible to parse entry fields into keys or if an exact match key is not +// present in the entry. +absl::StatusOr ParseTableEntry( + const p4::v1::TableEntry& entry, const TableInfo& table_info); + +// Parses p4::v1::Action into an EvaluationContext using action parameters, +// action name, and constraint source from action_info. Returns InvalidArgument +// if an Action parameter cannot be found in action_info or if there are +// duplicate action parameters. +absl::StatusOr ParseAction(const p4::v1::Action& action, + const ActionInfo& action_info); + // Used to memoize evaluation results to avoid re-computation. using EvaluationCache = absl::flat_hash_map; // Evaluates `expr` over `context.entry` to an `EvalResult`. Returns error -// status if AST is malformed and uses `context.source` to quote constraint. -// `eval_cache` holds boolean results, useful for avoiding recomputation when an -// explanation is desired. Passing a nullptr for `eval-cache` disables caching. +// status if AST is malformed and uses `context.constraint_source` to quote +// constraint. `eval_cache` holds boolean results, useful for avoiding +// recomputation when an explanation is desired. Passing a nullptr for +// `eval-cache` disables caching. absl::StatusOr Eval(const ast::Expression& expr, const EvaluationContext& context, EvaluationCache* eval_cache); @@ -188,8 +207,8 @@ absl::StatusOr Eval(const ast::Expression& expr, // Uses `eval_cache` and `size_cache` to avoid recomputation, allowing it to run // in linear time. Given current language specification, search only requires // traversal of nodes with type boolean. Traversal of non-boolean nodes or an -// invalid AST will return InternalError Status. Uses `context.source` to quote -// constraint on error. +// invalid AST will return InternalError Status. Uses +// `context.constraint_source` to quote constraint on error. absl::StatusOr MinimalSubexpressionLeadingToEvalResult( const ast::Expression& expression, const EvaluationContext& context, EvaluationCache& eval_cache, ast::SizeCache& size_cache); diff --git a/p4_constraints/backend/interpreter_golden_test_runner.cc b/p4_constraints/backend/interpreter_golden_test_runner.cc index be49952..0d09d7a 100644 --- a/p4_constraints/backend/interpreter_golden_test_runner.cc +++ b/p4_constraints/backend/interpreter_golden_test_runner.cc @@ -17,6 +17,7 @@ #include #include +#include #include #include "absl/log/check.h" @@ -29,6 +30,7 @@ #include "gutils/parse_text_proto.h" #include "gutils/status_macros.h" #include "p4_constraints/backend/constraint_info.h" +#include "p4_constraints/backend/errors.h" #include "p4_constraints/backend/interpreter.h" #include "p4_constraints/backend/type_checker.h" #include "p4_constraints/constraint_source.h" @@ -228,9 +230,20 @@ std::vector TestCases() { return test_cases; } -std::string EntryToString(const TableEntry& entry) { +absl::StatusOr EntryToString(const EvaluationContext& context, + const TableInfo& table_info) { + const TableEntry* table_entry = + std::get_if(&context.constraint_context); + if (table_entry == nullptr) { + return RuntimeTypeError(context.constraint_source, + table_info.constraint->start_location(), + table_info.constraint->end_location()) + << "The constraint context does not contain a TableEntry."; + } + std::string key_info = absl::StrJoin( - gutils::Ordered(entry.keys), "\n", [](std::string* out, auto pair) { + gutils::Ordered(table_entry->keys), "\n", + [](std::string* out, auto pair) { absl::StrAppend(out, "Key:\"", pair.first, "\" -> Value: ", EvalResultToString(pair.second)); }); @@ -238,7 +251,7 @@ std::string EntryToString(const TableEntry& entry) { "Table Name: \"%s\"\n" "Priority:%d\n" "%s\n", - entry.table_name, entry.priority, key_info); + table_entry->table_name, table_entry->priority, key_info); } absl::Status main() { @@ -249,15 +262,20 @@ absl::Status main() { if (table_info == nullptr) { return absl::InvalidArgumentError("No table info"); } - ASSIGN_OR_RETURN(TableEntry input, - ParseEntry(test_case.table_entry, *table_info)); + ASSIGN_OR_RETURN(const EvaluationContext context, + ParseTableEntry(test_case.table_entry, *table_info)); + + absl::StatusOr entry = EntryToString(context, *table_info); + if (!entry.ok()) { + std::cout << "=== ERROR ===\n" << entry.status(); + return absl::InvalidArgumentError(entry.status().ToString()); + } std::cout << "### ReasonEntryViolatestConstraint Test ###################\n" << "=== INPUT ===\n" << "--- Constraint ---\n" << test_case.constraint << "\n" << "--- Table Entry ---\n" - << EntryToString(input) << "\n"; - + << *entry << "\n"; absl::StatusOr result = ReasonEntryViolatesConstraint(test_case.table_entry, constraint_info); if (result.ok()) { diff --git a/p4_constraints/backend/interpreter_test.cc b/p4_constraints/backend/interpreter_test.cc index a0ae28c..f3eef9c 100644 --- a/p4_constraints/backend/interpreter_test.cc +++ b/p4_constraints/backend/interpreter_test.cc @@ -110,8 +110,8 @@ class ReasonEntryViolatesConstraintTest : public ::testing::Test { }}; const EvaluationContext kEvaluationContext{ - .entry = kParsedEntry, - .source = kDummySource, + .constraint_context = kParsedEntry, + .constraint_source = kDummySource, }; const p4::v1::TableEntry kTableEntry = @@ -178,15 +178,366 @@ class ReasonEntryViolatesConstraintTest : public ::testing::Test { EvaluationContext MakeEvaluationContext(const TableEntry& entry) { return EvaluationContext{ - .entry = entry, - .source = kDummySource, + .constraint_context = entry, + .constraint_source = kDummySource, }; } + + // Constraint to check that multicast_group_id != 0. + const Expression kMulticastGroupIdConstraint = ExpressionWithType(kBool, R"pb( + binary_expression { + binop: NE + left { + type { fixed_unsigned { bitwidth: 32 } } + action_parameter: "multicast_group_id" + } + right { + type { fixed_unsigned { bitwidth: 32 } } + type_cast { + type { arbitrary_int {} } + integer_constant: "0" + } + } + } + )pb"); + + // Constraint to check that vlan_id != 0. + const Expression kVlanIdConstraint = ExpressionWithType(kBool, R"pb( + binary_expression { + binop: NE + left { + type { fixed_unsigned { bitwidth: 32 } } + action_parameter: "vlan_id" + } + right { + type { fixed_unsigned { bitwidth: 32 } } + type_cast { + type { arbitrary_int {} } + integer_constant: "0" + } + } + } + )pb"); + + const ActionInfo kMulticastGroupIdActionInfo{ + .id = 123, + .name = "multicast_group_id", + .constraint = kMulticastGroupIdConstraint, + .params_by_id = + { + {1, {1, "multicast_group_id", kFixedUnsigned32}}, + }, + .params_by_name = { + {"multicast_group_id", {1, "multicast_group_id", kFixedUnsigned32}}, + }}; + + const ActionInfo kActionInfoVlanId{ + .id = 124, + .name = "vlan_id", + .constraint = kVlanIdConstraint, + .params_by_id = + { + {1, {1, "vlan_id", kFixedUnsigned32}}, + }, + .params_by_name = { + {"vlan_id", {1, "vlan_id", kFixedUnsigned32}}, + }}; }; class EvalTest : public ReasonEntryViolatesConstraintTest {}; class EvalToBoolCacheTest : public ReasonEntryViolatesConstraintTest {}; +TEST_F(ReasonEntryViolatesConstraintTest, EntryShouldMeetActionConstraint) { + p4::v1::TableEntry table_entry = ParseTextProtoOrDie(R"pb( + table_id: 1 + action { + action { + action_id: 123 + params { param_id: 1 value: "\x6" } + } + } + )pb"); + + const ConstraintInfo constraint_info = { + .action_info_by_id = {{kMulticastGroupIdActionInfo.id, + kMulticastGroupIdActionInfo}}, + .table_info_by_id = {{kTableInfo.id, kTableInfo}}, + }; + + ASSERT_THAT(ReasonEntryViolatesConstraint(table_entry, constraint_info), + IsOkAndHolds(Eq(""))); +} + +TEST_F(ReasonEntryViolatesConstraintTest, MissingActionIdShouldFailForAction) { + p4::v1::TableEntry table_entry = ParseTextProtoOrDie(R"pb( + table_id: 1 + action { + action { + action_id: 123 + params { param_id: 1 value: "\x0" } + } + } + )pb"); + + const ConstraintInfo constraint_info = { + .action_info_by_id = {{1, kMulticastGroupIdActionInfo}}, + .table_info_by_id = {{kTableInfo.id, kTableInfo}}, + }; + ASSERT_THAT(ReasonEntryViolatesConstraint(table_entry, constraint_info), + StatusIs(StatusCode::kInvalidArgument)); +} + +TEST_F(ReasonEntryViolatesConstraintTest, + MissingActionIdShouldFailForActionProfileActionSet) { + p4::v1::TableEntry table_entry = ParseTextProtoOrDie(R"pb( + table_id: 1 + action { + action_profile_action_set { + action_profile_actions { + action { + action_id: 123 + params { param_id: 1 value: "\x6" } + } + weight: 1 + } + action_profile_actions { + action { + action_id: 124 + params { param_id: 1 value: "\x6" } + } + weight: 2 + } + } + } + )pb"); + + const ConstraintInfo constraint_info = { + .action_info_by_id = {{ + 1, // No action with action_id of 1. + kMulticastGroupIdActionInfo, + }, + { + 2, // No action with action_id of 2. + kActionInfoVlanId, + }}, + .table_info_by_id = {{kTableInfo.id, kTableInfo}}, + }; + ASSERT_THAT(ReasonEntryViolatesConstraint(table_entry, constraint_info), + StatusIs(StatusCode::kInvalidArgument)); +} + +TEST_F(ReasonEntryViolatesConstraintTest, + ActionProfileMemberIdReturnsUnimplementedError) { + p4::v1::TableEntry table_entry = ParseTextProtoOrDie(R"pb( + table_id: 1 + action { action_profile_member_id: 1 } + )pb"); + + const ConstraintInfo constraint_info = { + .action_info_by_id = {{kMulticastGroupIdActionInfo.id, + kMulticastGroupIdActionInfo}}, + .table_info_by_id = {{kTableInfo.id, kTableInfo}}, + }; + ASSERT_THAT(ReasonEntryViolatesConstraint(table_entry, constraint_info), + StatusIs(StatusCode::kInvalidArgument)); +} + +TEST_F(ReasonEntryViolatesConstraintTest, + ActionProfileGroupIdReturnsUnimplementedError) { + p4::v1::TableEntry table_entry = ParseTextProtoOrDie(R"pb( + table_id: 1 + action { action_profile_group_id: 1 } + )pb"); + + const ConstraintInfo constraint_info = { + .action_info_by_id = {{kMulticastGroupIdActionInfo.id, + kMulticastGroupIdActionInfo}}, + .table_info_by_id = {{kTableInfo.id, kTableInfo}}, + }; + ASSERT_THAT(ReasonEntryViolatesConstraint(table_entry, constraint_info), + StatusIs(StatusCode::kInvalidArgument)); +} + +TEST_F(ReasonEntryViolatesConstraintTest, EntryShouldViolateActionConstraint) { + p4::v1::TableEntry table_entry = ParseTextProtoOrDie(R"pb( + table_id: 1 + action { + action { + action_id: 123 + params { param_id: 1 value: "\x0" } + } + } + )pb"); + + // Constraint to check that multicast_group_id != 0. + Expression multicast_group_id_constraint = + ParseTextProtoOrDie(R"pb( + start_location { action_name: "multicast_group_id" } + end_location { action_name: "multicast_group_id" } + type { boolean {} } + binary_expression { + binop: NE + left { + type { fixed_unsigned { bitwidth: 32 } } + action_parameter: "multicast_group_id" + } + right { + type { fixed_unsigned { bitwidth: 32 } } + type_cast { + type { arbitrary_int {} } + integer_constant: "0" + } + } + } + )pb"); + + ActionInfo action_info = kMulticastGroupIdActionInfo; + action_info.constraint = multicast_group_id_constraint; + action_info.constraint_source.constraint_location.set_action_name( + "multicast_group_id"); + + const ConstraintInfo constraint_info = { + .action_info_by_id = {{action_info.id, action_info}}, + .table_info_by_id = {{kTableInfo.id, kTableInfo}}, + }; + ASSERT_THAT(ReasonEntryViolatesConstraint(table_entry, constraint_info), + IsOkAndHolds(Not(Eq("")))); +} + +TEST_F(ReasonEntryViolatesConstraintTest, + EntryShouldMeetActionProfileActionSetConstraint) { + p4::v1::TableEntry table_entry = ParseTextProtoOrDie(R"pb( + table_id: 1 + action { + action_profile_action_set { + action_profile_actions { + action { + action_id: 123 + params { param_id: 1 value: "\x6" } + } + weight: 1 + } + action_profile_actions { + action { + action_id: 124 + params { param_id: 1 value: "\x6" } + } + weight: 2 + } + } + } + )pb"); + + const ConstraintInfo constraint_info = { + .action_info_by_id = {{ + kMulticastGroupIdActionInfo.id, + kMulticastGroupIdActionInfo, + }, + { + kActionInfoVlanId.id, + kActionInfoVlanId, + }}, + .table_info_by_id = {{kTableInfo.id, kTableInfo}}, + }; + + ASSERT_THAT(ReasonEntryViolatesConstraint(table_entry, constraint_info), + IsOkAndHolds(Eq(""))); +} + +TEST_F(ReasonEntryViolatesConstraintTest, + EntryShouldViolateActionProfileActionSetConstraint) { + p4::v1::TableEntry table_entry = ParseTextProtoOrDie(R"pb( + table_id: 1 + action { + action_profile_action_set { + action_profile_actions { + action { + action_id: 123 + params { param_id: 1 value: "\x6" } + } + weight: 1 + } + action_profile_actions { + action { + action_id: 124 + params { param_id: 1 value: "\x0" } + } + weight: 2 + } + } + } + )pb"); + + // Constraint to check that multicast_group_id != 0. + Expression multicast_group_id_constraint = + ParseTextProtoOrDie(R"pb( + start_location { action_name: "multicast_group_id" } + end_location { action_name: "multicast_group_id" } + type { boolean {} } + binary_expression { + binop: NE + left { + type { fixed_unsigned { bitwidth: 32 } } + action_parameter: "multicast_group_id" + } + right { + type { fixed_unsigned { bitwidth: 32 } } + type_cast { + type { arbitrary_int {} } + integer_constant: "0" + } + } + } + )pb"); + + // Constraint to check that vlan_id != 0. + Expression vlan_id_constraint = ParseTextProtoOrDie(R"pb( + start_location { action_name: "vlan_id" } + end_location { action_name: "vlan_id" } + type { boolean {} } + binary_expression { + binop: NE + left { + type { fixed_unsigned { bitwidth: 32 } } + action_parameter: "vlan_id" + } + right { + type { fixed_unsigned { bitwidth: 32 } } + type_cast { + type { arbitrary_int {} } + integer_constant: "0" + } + } + } + )pb"); + + ActionInfo multicast_group_id_action_info = kMulticastGroupIdActionInfo; + ActionInfo vlan_id_action_info = kActionInfoVlanId; + + multicast_group_id_action_info.constraint = multicast_group_id_constraint; + multicast_group_id_action_info.constraint_source.constraint_location + .set_action_name("multicast_group_id"); + + vlan_id_action_info.constraint = vlan_id_constraint; + vlan_id_action_info.constraint_source.constraint_location.set_action_name( + "vlan_id"); + + const ConstraintInfo constraint_info = { + .action_info_by_id = {{ + multicast_group_id_action_info.id, + multicast_group_id_action_info, + }, + { + vlan_id_action_info.id, + vlan_id_action_info, + }}, + .table_info_by_id = {{kTableInfo.id, kTableInfo}}, + }; + + ASSERT_THAT(ReasonEntryViolatesConstraint(table_entry, constraint_info), + IsOkAndHolds(Not(Eq("")))); +} + TEST_F(ReasonEntryViolatesConstraintTest, EmptyExpressionErrors) { const Expression kExpr; EXPECT_THAT( @@ -251,6 +602,7 @@ TEST_F(ReasonEntryViolatesConstraintTest, EntriesWithLeadingZeroesWork) { } } )pb"); + // Sanity check that it holds with original entry. ASSERT_THAT(ReasonEntryViolatesConstraint( kTableEntry, MakeConstraintInfo(exact_equals_num)), diff --git a/p4_constraints/quote.cc b/p4_constraints/quote.cc index 61e0707..09bc006 100644 --- a/p4_constraints/quote.cc +++ b/p4_constraints/quote.cc @@ -22,6 +22,8 @@ #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "gutils/proto.h" +#include "gutils/source_location.h" +#include "gutils/status_builder.h" #include "gutils/status_macros.h" #include "p4_constraints/ast.h" #include "p4_constraints/ast.pb.h" @@ -39,6 +41,31 @@ absl::StatusOr Explain(const ast::SourceLocation& start, const ast::SourceLocation& end) { std::stringstream output; switch (start.source_case()) { + case ast::SourceLocation::kFilePath: + // Succinct mode: path/to/file:line:column. + output << start.file_path() << ":" << (start.line() + 1) << ":" + << (start.column() + 1); + if (end.line() > start.line()) + output << "-" << (end.line() + 1) << ":" << end.column(); + else if (end.column() > start.column() + 1) + output << "-" << end.column(); + output << ":\n"; + return output.str(); + + case ast::SourceLocation::kActionName: + output << "In @action_restriction of action '" << start.action_name() + << "'; at offset line " << (start.line() + 1); + if (end.line() > start.line()) + output << ", column " << (start.column() + 1) << " to line " + << (end.line() + 1) << ", column" << end.column(); + else if (end.column() > start.column() + 1) + output << ", columns " << (start.column() + 1) << " to " + << end.column(); + else + output << ", column " << (start.column() + 1); + output << ":\n"; + return output.str(); + case ast::SourceLocation::kTableName: // Verbose mode: At offset line 12, columns 17 to 20. output << "In @entry_restriction of table '" << start.table_name() @@ -51,24 +78,14 @@ absl::StatusOr Explain(const ast::SourceLocation& start, << end.column(); else output << ", column " << (start.column() + 1); - break; + output << ":\n"; + return output.str(); - case ast::SourceLocation::kFilePath: - // Succinct mode: path/to/file:line:column. - output << start.file_path() << ":" << (start.line() + 1) << ":" - << (start.column() + 1); - if (end.line() > start.line()) - output << "-" << (end.line() + 1) << ":" << end.column(); - else if (end.column() > start.column() + 1) - output << "-" << end.column(); + case ast::SourceLocation::SOURCE_NOT_SET: break; - - default: - return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC) - << "unknown source case: " << start.source_case(); } - output << ":\n"; - return output.str(); + return gutils::InvalidArgumentErrorBuilder(GUTILS_LOC) + << "Invalid source case: " << start.DebugString(); } // Returns a string that quotes and marks the given source location interval,