diff --git a/p4_symbolic/ir/BUILD.bazel b/p4_symbolic/ir/BUILD.bazel index 75604cb2..8ae8aa81 100644 --- a/p4_symbolic/ir/BUILD.bazel +++ b/p4_symbolic/ir/BUILD.bazel @@ -223,6 +223,13 @@ ir_parsing_test( p4_program = "//p4_symbolic/testdata:parser/set_parser_operation.p4", ) +ir_parsing_test( + name = "primitive_parser_operation_test", + golden_file = "expected/primitive_parser_operation.txt", + p4_deps = ["//p4_symbolic/testdata:common/headers.p4"], + p4_program = "//p4_symbolic/testdata:parser/primitive_parser_operation.p4", +) + ir_parsing_test( name = "default_transition_test", golden_file = "expected/default_transition.txt", diff --git a/p4_symbolic/ir/expected/primitive_parser_operation.txt b/p4_symbolic/ir/expected/primitive_parser_operation.txt new file mode 100644 index 00000000..2e3498c9 --- /dev/null +++ b/p4_symbolic/ir/expected/primitive_parser_operation.txt @@ -0,0 +1,551 @@ +headers { + key: "ethernet" + value { + name: "ethernet_t" + id: 2 + fields { + key: "dst_addr" + value { + name: "dst_addr" + bitwidth: 48 + } + } + fields { + key: "ether_type" + value { + name: "ether_type" + bitwidth: 16 + } + } + fields { + key: "src_addr" + value { + name: "src_addr" + bitwidth: 48 + } + } + } +} +headers { + key: "ipv4" + value { + name: "ipv4_t" + id: 3 + fields { + key: "do_not_fragment" + value { + name: "do_not_fragment" + bitwidth: 1 + } + } + fields { + key: "dscp" + value { + name: "dscp" + bitwidth: 6 + } + } + fields { + key: "dst_addr" + value { + name: "dst_addr" + bitwidth: 32 + } + } + fields { + key: "ecn" + value { + name: "ecn" + bitwidth: 2 + } + } + fields { + key: "frag_offset" + value { + name: "frag_offset" + bitwidth: 13 + } + } + fields { + key: "header_checksum" + value { + name: "header_checksum" + bitwidth: 16 + } + } + fields { + key: "identification" + value { + name: "identification" + bitwidth: 16 + } + } + fields { + key: "ihl" + value { + name: "ihl" + bitwidth: 4 + } + } + fields { + key: "more_fragments" + value { + name: "more_fragments" + bitwidth: 1 + } + } + fields { + key: "protocol" + value { + name: "protocol" + bitwidth: 8 + } + } + fields { + key: "reserved" + value { + name: "reserved" + bitwidth: 1 + } + } + fields { + key: "src_addr" + value { + name: "src_addr" + bitwidth: 32 + } + } + fields { + key: "total_len" + value { + name: "total_len" + bitwidth: 16 + } + } + fields { + key: "ttl" + value { + name: "ttl" + bitwidth: 8 + } + } + fields { + key: "version" + value { + name: "version" + bitwidth: 4 + } + } + } +} +headers { + key: "ipv6" + value { + name: "ipv6_t" + id: 4 + fields { + key: "dscp" + value { + name: "dscp" + bitwidth: 6 + } + } + fields { + key: "dst_addr" + value { + name: "dst_addr" + bitwidth: 128 + } + } + fields { + key: "ecn" + value { + name: "ecn" + bitwidth: 2 + } + } + fields { + key: "flow_label" + value { + name: "flow_label" + bitwidth: 20 + } + } + fields { + key: "hop_limit" + value { + name: "hop_limit" + bitwidth: 8 + } + } + fields { + key: "next_header" + value { + name: "next_header" + bitwidth: 8 + } + } + fields { + key: "payload_length" + value { + name: "payload_length" + bitwidth: 16 + } + } + fields { + key: "src_addr" + value { + name: "src_addr" + bitwidth: 128 + } + } + fields { + key: "version" + value { + name: "version" + bitwidth: 4 + } + } + } +} +headers { + key: "scalars" + value { + name: "scalars_0" + } +} +headers { + key: "standard_metadata" + value { + name: "standard_metadata" + id: 1 + fields { + key: "_padding" + value { + name: "_padding" + bitwidth: 3 + } + } + fields { + key: "checksum_error" + value { + name: "checksum_error" + bitwidth: 1 + } + } + fields { + key: "deq_qdepth" + value { + name: "deq_qdepth" + bitwidth: 19 + } + } + fields { + key: "deq_timedelta" + value { + name: "deq_timedelta" + bitwidth: 32 + } + } + fields { + key: "egress_global_timestamp" + value { + name: "egress_global_timestamp" + bitwidth: 48 + } + } + fields { + key: "egress_port" + value { + name: "egress_port" + bitwidth: 9 + } + } + fields { + key: "egress_rid" + value { + name: "egress_rid" + bitwidth: 16 + } + } + fields { + key: "egress_spec" + value { + name: "egress_spec" + bitwidth: 9 + } + } + fields { + key: "enq_qdepth" + value { + name: "enq_qdepth" + bitwidth: 19 + } + } + fields { + key: "enq_timestamp" + value { + name: "enq_timestamp" + bitwidth: 32 + } + } + fields { + key: "ingress_global_timestamp" + value { + name: "ingress_global_timestamp" + bitwidth: 48 + } + } + fields { + key: "ingress_port" + value { + name: "ingress_port" + bitwidth: 9 + } + } + fields { + key: "instance_type" + value { + name: "instance_type" + bitwidth: 32 + } + } + fields { + key: "mcast_grp" + value { + name: "mcast_grp" + bitwidth: 16 + } + } + fields { + key: "packet_length" + value { + name: "packet_length" + bitwidth: 32 + } + } + fields { + key: "parser_error" + value { + name: "parser_error" + bitwidth: 32 + } + } + fields { + key: "priority" + value { + name: "priority" + bitwidth: 3 + } + } + } +} +pipeline { + key: "egress" + value { + name: "egress" + initial_control: "__END_OF_PIPELINE__" + } +} +pipeline { + key: "ingress" + value { + name: "ingress" + initial_control: "__END_OF_PIPELINE__" + } +} +parsers { + key: "parser" + value { + name: "parser" + initial_state: "start" + parse_states { + key: "parse_ipv4" + value { + name: "parse_ipv4" + parser_ops { + extract { + header { + header_name: "ipv4" + } + } + } + parser_ops { + primitive { + assignment { + left { + field_value { + header_name: "ipv6" + field_name: "$valid$" + } + } + right { + bool_value { + } + } + } + } + } + transitions { + default_transition { + next_state: "__END_OF_PARSER__" + } + } + optimized_symbolic_execution_info { + merge_point: "__END_OF_PARSER__" + } + } + } + parse_states { + key: "parse_ipv6" + value { + name: "parse_ipv6" + parser_ops { + extract { + header { + header_name: "ipv6" + } + } + } + parser_ops { + primitive { + assignment { + left { + field_value { + header_name: "ipv4" + field_name: "$valid$" + } + } + right { + bool_value { + } + } + } + } + } + transitions { + default_transition { + next_state: "__END_OF_PARSER__" + } + } + optimized_symbolic_execution_info { + merge_point: "__END_OF_PARSER__" + } + } + } + parse_states { + key: "start" + value { + name: "start" + parser_ops { + extract { + header { + header_name: "ethernet" + } + } + } + parser_ops { + primitive { + assignment { + left { + field_value { + header_name: "ethernet" + field_name: "$valid$" + } + } + right { + bool_value { + value: true + } + } + } + } + } + transition_key_fields { + field { + header_name: "ethernet" + field_name: "ether_type" + } + } + transitions { + hex_string_transition { + value { + value: "0x0800" + } + mask { + } + next_state: "parse_ipv4" + } + } + transitions { + hex_string_transition { + value { + value: "0x86dd" + } + mask { + } + next_state: "parse_ipv6" + } + } + transitions { + default_transition { + next_state: "__END_OF_PARSER__" + } + } + optimized_symbolic_execution_info { + merge_point: "__END_OF_PARSER__" + continue_to_merge_point: true + } + } + } + } +} +errors { + key: "HeaderTooShort" + value { + name: "HeaderTooShort" + value: 4 + } +} +errors { + key: "NoError" + value { + name: "NoError" + } +} +errors { + key: "NoMatch" + value { + name: "NoMatch" + value: 2 + } +} +errors { + key: "PacketTooShort" + value { + name: "PacketTooShort" + value: 1 + } +} +errors { + key: "ParserInvalidArgument" + value { + name: "ParserInvalidArgument" + value: 6 + } +} +errors { + key: "ParserTimeout" + value { + name: "ParserTimeout" + value: 5 + } +} +errors { + key: "StackOutOfBounds" + value { + name: "StackOutOfBounds" + value: 3 + } +} + diff --git a/p4_symbolic/ir/ir.cc b/p4_symbolic/ir/ir.cc index 0519900c..9c0c3ee1 100644 --- a/p4_symbolic/ir/ir.cc +++ b/p4_symbolic/ir/ir.cc @@ -881,9 +881,88 @@ absl::StatusOr ExtractSetParserOp( return result; } +// Translates the "primitive" parser operation from the BMv2 protobuf message. +// Currently only "add_header" and "remove_header" primitives are supported, +// which correspond to "setValid" and "setInvalid" methods respectively. +absl::StatusOr ExtractPrimitiveParserOp( + const bmv2::ParserOperation &bmv2_op) { + ParserOperation::Primitive result; + + // The "primitive" parser operation must have exactly 1 parameter. + if (bmv2_op.parameters_size() != 1) { + return gutil::InvalidArgumentErrorBuilder() + << "Parser primitive op must have 1 parameter, found " + << bmv2_op.DebugString(); + } + + const ::google::protobuf::Struct &bmv2_param = bmv2_op.parameters(0); + + // Make sure the parameter struct contains the correct fields. + if (!bmv2_param.fields().contains("op") || + !bmv2_param.fields().at("op").has_string_value() || + !bmv2_param.fields().contains("parameters") || + !bmv2_param.fields().at("parameters").has_list_value()) { + return gutil::InvalidArgumentErrorBuilder() + << "Primitive operation has an invalid parameter: " + << bmv2_param.DebugString(); + } + + const std::string &bmv2_primitive_op = + bmv2_param.fields().at("op").string_value(); + const ::google::protobuf::ListValue &bmv2_primitive_parameters = + bmv2_param.fields().at("parameters").list_value(); + ASSIGN_OR_RETURN(bmv2::StatementOp op_case, + StatementOpToEnum(bmv2_primitive_op)); + + switch (op_case) { + case bmv2::StatementOp::add_header: + case bmv2::StatementOp::remove_header: { + // "add_header" or "remove_header" primitives must have exactly 1 + // parameter. + if (bmv2_primitive_parameters.values_size() != 1) { + return gutil::InvalidArgumentErrorBuilder() + << "setValid/setInvalid statements must contain 1 parameter, " + "found: " + << bmv2_primitive_parameters.DebugString(); + } + + ASSIGN_OR_RETURN(RValue header, + ExtractRValue(bmv2_primitive_parameters.values(0), {})); + + // "add_header" or "remove_header" primitives must have a header name as + // the parameter. + if (header.rvalue_case() != RValue::kHeaderValue) { + return gutil::InvalidArgumentErrorBuilder() + << "setValid/setInvalid statements must have header as the " + "parameter, found: " + << bmv2_primitive_parameters.DebugString(); + } + + const std::string &header_name = header.header_value().header_name(); + + // Set the field `
.$valid$` to true or false based on whether the + // primitive is `add_header` or `remove_header` respectively. + AssignmentStatement &assignment = *result.mutable_assignment(); + FieldValue &valid_field = + *assignment.mutable_left()->mutable_field_value(); + valid_field.set_header_name(header_name); + valid_field.set_field_name("$valid$"); + assignment.mutable_right()->mutable_bool_value()->set_value( + op_case == bmv2::StatementOp::add_header); + break; + } + default: { + return gutil::UnimplementedErrorBuilder() + << "Unsupported primitive op: " << bmv2_param.DebugString(); + } + } + + return result; +} + // Translates parser operations. -// Currently only "extract" and "set" parser operations are supported since -// others are not required at the moment. +// Currently only "extract", "set", and "primitive" parser operations are +// supported since others are not required at the moment. absl::StatusOr ExtractParserOperation( const bmv2::ParserOperation &bmv2_op) { ParserOperation result; @@ -898,6 +977,11 @@ absl::StatusOr ExtractParserOperation( ASSIGN_OR_RETURN(*result.mutable_set(), ExtractSetParserOp(bmv2_op)); break; } + case bmv2::ParserOperation::primitive: { + ASSIGN_OR_RETURN(*result.mutable_primitive(), + ExtractPrimitiveParserOp(bmv2_op)); + break; + } default: { return absl::UnimplementedError( absl::StrCat("Unsupported parser op: ", bmv2_op.DebugString())); diff --git a/p4_symbolic/ir/ir.proto b/p4_symbolic/ir/ir.proto index 40982e5b..215d3c0b 100644 --- a/p4_symbolic/ir/ir.proto +++ b/p4_symbolic/ir/ir.proto @@ -108,9 +108,9 @@ message ParseState { // Reference: // https://github.com/p4lang/behavioral-model/blob/main/docs/JSON_format.md#parser-operations. // -// Note that only "extract" and "set" operations are supported for now since -// others are not required at the moment. To support more complicated parsers, -// other operations may be supported in the future. +// Note that only "extract", "set", and "primitive" operations are supported for +// now since others are not required at the moment. To support more complicated +// parsers, other operations may be supported in the future. message ParserOperation { // Defines an "extract" parser operation, which performs extraction for a // fixed-width header. @@ -133,9 +133,23 @@ message ParserOperation { } } + // Defines a "primitive" parser operation. The exact list of primitives + // supported depends on the individual targets. The format is similar to + // actions. Reference: + // https://github.com/p4lang/behavioral-model/blob/main/docs/JSON_format.md#parser-operations + // https://github.com/p4lang/behavioral-model/blob/main/src/bm_sim/core/primitives.cpp + // https://github.com/p4lang/behavioral-model/blob/main/targets/simple_switch/primitives.cpp + message Primitive { + // The statement interpreted from the primitive. + oneof statement { + AssignmentStatement assignment = 1; + } + } + oneof operation { Extract extract = 1; Set set = 3; + Primitive primitive = 7; } } diff --git a/p4_symbolic/sai/deparser_test.cc b/p4_symbolic/sai/deparser_test.cc index 5d09dc76..3f2f617a 100644 --- a/p4_symbolic/sai/deparser_test.cc +++ b/p4_symbolic/sai/deparser_test.cc @@ -19,19 +19,24 @@ #include #include "absl/status/statusor.h" +#include "absl/strings/str_format.h" #include "glog/logging.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "gutil/proto_matchers.h" #include "gutil/status.h" #include "gutil/status_matchers.h" #include "p4/v1/p4runtime.pb.h" #include "p4_pdpi/packetlib/packetlib.h" #include "p4_pdpi/packetlib/packetlib.pb.h" #include "p4_pdpi/string_encodings/bit_string.h" +#include "p4_pdpi/string_encodings/hex_string.h" +#include "p4_symbolic/parser.h" #include "p4_symbolic/sai/fields.h" #include "p4_symbolic/sai/parser.h" #include "p4_symbolic/sai/sai.h" #include "p4_symbolic/symbolic/symbolic.h" +#include "p4_symbolic/z3_util.h" #include "sai_p4/instantiations/google/instantiations.h" #include "sai_p4/instantiations/google/sai_nonstandard_platforms.h" #include "z3++.h" @@ -204,45 +209,64 @@ TEST_P(SaiDeparserTest, PacketInHeaderIsNeverParsedIntegrationTest) { { ASSERT_OK_AND_ASSIGN(SaiFields fields, GetSaiFields(state_->context.ingress_headers)); - // TODO: Execute the test unconditionally one we add the - // packet-in header to SAI P4. - if (!fields.headers.packet_in.has_value()) { - GTEST_SKIP() << "test does not apply, as program has no packet-in header"; - } - state_->solver->add(fields.headers.packet_in.value().valid); + EXPECT_TRUE(fields.headers.packet_in.has_value()); + state_->solver->add(fields.headers.packet_in->valid); } // Should be unsatisifiable, because we never parse a packet-in header. ASSERT_EQ(state_->solver->check(), z3::check_result::unsat); } -TEST_P(SaiDeparserTest, DISABLED_PacketInPacketParserIntegrationTest) { - // Add packet_in constraint. - { - ASSERT_OK_AND_ASSIGN(SaiFields fields, - GetSaiFields(state_->context.ingress_headers)); - // TODO: Execute the test unconditionally one we add the - // packet-in header to SAI P4. - if (!fields.headers.packet_in.has_value()) { - GTEST_SKIP() << "test does not apply, as program has no packet-in header"; - } - state_->solver->add(fields.headers.packet_in.value().valid); - } +using SimpleSaiDeparserTest = testing::TestWithParam; + +TEST_P(SimpleSaiDeparserTest, PacketInHeaderDeparsingIsCorrect) { + // Set up. + z3::solver solver = z3::solver(Z3Context()); + const auto config = sai::GetNonstandardForwardingPipelineConfig( + /*instantiation=*/GetParam(), sai::NonstandardPlatform::kP4Symbolic); + ASSERT_OK_AND_ASSIGN(symbolic::Dataplane dataplane, + ParseToIr(config, /*entries=*/{})); + ASSERT_OK_AND_ASSIGN(auto headers, + symbolic::SymbolicGuardedMap::CreateSymbolicGuardedMap( + dataplane.program.headers())); + ASSERT_OK_AND_ASSIGN(SaiFields fields, GetSaiFields(headers)); + + // Add packet_in constraints. + EXPECT_TRUE(fields.headers.packet_in.has_value()); + solver.add(fields.headers.packet_in->valid); + constexpr int kIngressPort = 42; + constexpr int kTargetEgpressPort = 17; + constexpr int kUnusedPad = 0; + solver.add(fields.headers.packet_in->ingress_port == kIngressPort); + solver.add(fields.headers.packet_in->target_egress_port == + kTargetEgpressPort); + solver.add(fields.headers.packet_in->unused_pad == kUnusedPad); + packetlib::Header expected_header; + packetlib::SaiP4BMv2PacketInHeader& packet_in_header = + *expected_header.mutable_sai_p4_bmv2_packet_in_header(); + packet_in_header.set_ingress_port(pdpi::BitsetToHexString<9>(kIngressPort)); + packet_in_header.set_target_egress_port( + pdpi::BitsetToHexString<9>(kTargetEgpressPort)); + packet_in_header.set_unused_pad(pdpi::BitsetToHexString<6>(kUnusedPad)); // Solve and deparse. - ASSERT_EQ(state_->solver->check(), z3::check_result::sat); - auto model = state_->solver->get_model(); - ASSERT_OK_AND_ASSIGN(std::string raw_packet, - SaiDeparser(state_->context.ingress_headers, model)); + ASSERT_EQ(solver.check(), z3::check_result::sat); + auto model = solver.get_model(); + ASSERT_OK_AND_ASSIGN(std::string raw_packet, SaiDeparser(headers, model)); - // Check we indeed got a packet_in packet. - packetlib::Packet packet = packetlib::ParsePacket(raw_packet); + // Check we indeed got a packet_in packet with the correct fields. + packetlib::Packet packet = packetlib::ParsePacket( + raw_packet, + /*first_header=*/packetlib::Header::kSaiP4Bmv2PacketInHeader); LOG(INFO) << "Z3-generated packet = " << packet.DebugString(); - ASSERT_GE(packet.headers_size(), 0); - ASSERT_TRUE(packet.headers(0).has_sai_p4_bmv2_packet_in_header()); + ASSERT_GT(packet.headers_size(), 0); + ASSERT_THAT(packet.headers(0), gutil::EqualsProto(expected_header)); } -INSTANTIATE_TEST_SUITE_P(Instantiation, SaiDeparserTest, +INSTANTIATE_TEST_SUITE_P(SaiDeparserTest, SaiDeparserTest, + testing::ValuesIn(sai::AllSaiInstantiations())); + +INSTANTIATE_TEST_SUITE_P(SimpleSaiDeparserTest, SimpleSaiDeparserTest, testing::ValuesIn(sai::AllSaiInstantiations())); } // namespace diff --git a/p4_symbolic/sai/sai_test.cc b/p4_symbolic/sai/sai_test.cc index a37d37a5..cdbe283c 100644 --- a/p4_symbolic/sai/sai_test.cc +++ b/p4_symbolic/sai/sai_test.cc @@ -111,7 +111,7 @@ TEST(EvaluateSaiPipeline, IngressPortIsAmongPassedValues) { std::vector pi_entries; for (auto& pd_entry : pd_entries.entries()) { ASSERT_OK_AND_ASSIGN(pi_entries.emplace_back(), - pdpi::PdTableEntryToPi(ir_p4info, pd_entry)); + pdpi::PartialPdTableEntryToPiTableEntry(ir_p4info, pd_entry)); } // Evaluate the SAI pipeline. diff --git a/p4_symbolic/testdata/parser/primitive_parser_operation.p4 b/p4_symbolic/testdata/parser/primitive_parser_operation.p4 new file mode 100644 index 00000000..dfccd3f1 --- /dev/null +++ b/p4_symbolic/testdata/parser/primitive_parser_operation.p4 @@ -0,0 +1,79 @@ +#include +#include "../common/headers.p4" + +struct local_metadata_t { + /* empty */ +} + +struct headers_t { + ethernet_t ethernet; + ipv4_t ipv4; + ipv6_t ipv6; +} + +parser packet_parser(packet_in packet, out headers_t headers, + inout local_metadata_t local_metadata, + inout standard_metadata_t standard_metadata) { + state start { + transition parse_ethernet; + } + + state parse_ethernet { + packet.extract(headers.ethernet); + headers.ethernet.setValid(); + transition select(headers.ethernet.ether_type) { + ETHERTYPE_IPV4: parse_ipv4; + ETHERTYPE_IPV6: parse_ipv6; + default: accept; + } + } + + state parse_ipv4 { + packet.extract(headers.ipv4); + headers.ipv6.setInvalid(); + transition accept; + } + + state parse_ipv6 { + packet.extract(headers.ipv6); + headers.ipv4.setInvalid(); + transition accept; + } +} + +control empty_verify_checksum(inout headers_t headers, + inout local_metadata_t local_metadata) { + apply {} +} // control empty_verify_checksum + +control ingress(inout headers_t headers, + inout local_metadata_t local_metadata, + inout standard_metadata_t standard_metadata) { + apply {} +} // control ingress + +control egress(inout headers_t headers, + inout local_metadata_t local_metadata, + inout standard_metadata_t standard_metadata) { + apply {} +} // control egress + +control empty_compute_checksum(inout headers_t headers, + inout local_metadata_t local_metadata) { + apply {} +} // control empty_compute_checksum + +control packet_deparser(packet_out packet, in headers_t headers) { + apply { + packet.emit(headers.ethernet); + } +} // control packet_deparser + +V1Switch( + packet_parser(), + empty_verify_checksum(), + ingress(), + egress(), + empty_compute_checksum(), + packet_deparser() +) main; diff --git a/p4rt_app/sonic/app_db_acl_def_table_manager.cc b/p4rt_app/sonic/app_db_acl_def_table_manager.cc index 96c3cac5..7509edd2 100644 --- a/p4rt_app/sonic/app_db_acl_def_table_manager.cc +++ b/p4rt_app/sonic/app_db_acl_def_table_manager.cc @@ -662,7 +662,12 @@ absl::Status VerifyActionParamAgainstSchema( swss::acl::ActionSchema schema; const auto& param_name = ir_param.param().name(); try { - schema = swss::acl::ActionSchemaByName(sai_param.action); + if (sai_param.object_type.empty()) { + schema = swss::acl::ActionSchemaByName(sai_param.action); + } else { + schema = swss::acl::ActionSchemaByNameAndObjectType( + sai_param.action, sai_param.object_type); + } } catch (...) { return InvalidArgumentErrorBuilder() << absl::Substitute( "Action '$0' Param '$1' references unknown SAI field '$2'.", diff --git a/p4rt_app/sonic/app_db_acl_def_table_manager_test.cc b/p4rt_app/sonic/app_db_acl_def_table_manager_test.cc index 61525280..77ea86ea 100644 --- a/p4rt_app/sonic/app_db_acl_def_table_manager_test.cc +++ b/p4rt_app/sonic/app_db_acl_def_table_manager_test.cc @@ -141,10 +141,11 @@ TEST(InsertAclTableDefinition, InsertsAclTableDefinition) { R"pb( id: 1 name: "multicast_group_id" + bitwidth: 16 annotations: "@sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)" annotations: "@sai_action_param_object_type(SAI_OBJECT_TYPE_IPMC_GROUP)" )pb", - pdpi::STRING)) + pdpi::HEX_STRING)) .entry_action( IrActionDefinitionBuilder() .preamble( @@ -155,10 +156,11 @@ TEST(InsertAclTableDefinition, InsertsAclTableDefinition) { R"pb( id: 1 name: "multicast_group_id" + bitwidth: 16 annotations: "@sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)" annotations: "@sai_action_param_object_type(SAI_OBJECT_TYPE_L2MC_GROUP)" )pb", - pdpi::STRING)) + pdpi::HEX_STRING)) .entry_action( IrActionDefinitionBuilder() .preamble( @@ -168,7 +170,7 @@ TEST(InsertAclTableDefinition, InsertsAclTableDefinition) { .param( R"pb( id: 1 - name: "multicast_group_id" + name: "nexthop_id" annotations: "@sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)" annotations: "@sai_action_param_object_type(SAI_OBJECT_TYPE_NEXT_HOP)" )pb", @@ -182,7 +184,7 @@ TEST(InsertAclTableDefinition, InsertsAclTableDefinition) { .param( R"pb( id: 1 - name: "multicast_group_id" + name: "port_id" annotations: "@sai_action_param(SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT)" annotations: "@sai_action_param_object_type(SAI_OBJECT_TYPE_PORT)" )pb", @@ -260,13 +262,13 @@ TEST(InsertAclTableDefinition, InsertsAclTableDefinition) { {"action/redirect_action_that_includes_next_hop_type", nlohmann::json::parse(R"JSON( [{"action": "SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT", - "param": "multicast_group_id", + "param": "nexthop_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", + "param": "port_id", "object_type": "SAI_OBJECT_TYPE_PORT"}])JSON") .dump()}, {"size", "512"}, diff --git a/pins_infra_deps.bzl b/pins_infra_deps.bzl index a8c500c6..fd05c783 100644 --- a/pins_infra_deps.bzl +++ b/pins_infra_deps.bzl @@ -16,9 +16,9 @@ def pins_infra_deps(): if not native.existing_rule("com_github_bazelbuild_buildtools"): http_archive( name = "com_github_bazelbuild_buildtools", - sha256 = "44a6e5acc007e197d45ac3326e7f993f0160af9a58e8777ca7701e00501c0857", - strip_prefix = "buildtools-4.2.4", - url = "https://github.com/bazelbuild/buildtools/archive/4.2.4.tar.gz", + # sha256 = "44a6e5acc007e197d45ac3326e7f993f0160af9a58e8777ca7701e00501c0857", + strip_prefix = "buildtools-5.1.0", + url = "https://github.com/bazelbuild/buildtools/archive/refs/tags/5.1.0.tar.gz", ) if "boringssl" not in native.existing_rules(): http_archive( @@ -33,9 +33,6 @@ def pins_infra_deps(): if not native.existing_rule("com_github_grpc_grpc"): http_archive( name = "com_github_grpc_grpc", - # url = "https://github.com/grpc/grpc/archive/v1.46.0.zip", - # strip_prefix = "grpc-1.46.0", - # sha256 = "1cbd6d6dfc9b1235766fc6b1d66d4f1dbb87f877a44c2a799bc8ee6b383af0fa", url = "https://github.com/grpc/grpc/archive/v1.58.0.zip", strip_prefix = "grpc-1.58.0", sha256 = "aa329c7de707a03511c88206ef4483e9346ab6336b6be4378d294060aa7400b3", @@ -230,8 +227,8 @@ def pins_infra_deps(): if not native.existing_rule("sonic_swss_common"): http_archive( name = "sonic_swss_common", - url = "https://github.com/azure/sonic-swss-common/archive/e7917acd2d4a9c0121802437e3c899bd513ac888.zip", - strip_prefix = "sonic-swss-common-e7917acd2d4a9c0121802437e3c899bd513ac888", + url = "https://github.com/azure/sonic-swss-common/archive/f6c1614227f25dfa81ab5ccd0cb8cca265aaad7d.zip", + strip_prefix = "sonic-swss-common-f6c1614227f25dfa81ab5ccd0cb8cca265aaad7d", ) if not native.existing_rule("rules_pkg"): http_archive( diff --git a/tests/forwarding/BUILD.bazel b/tests/forwarding/BUILD.bazel index eded229e..281e623c 100644 --- a/tests/forwarding/BUILD.bazel +++ b/tests/forwarding/BUILD.bazel @@ -168,6 +168,38 @@ cc_library( ], ) +cc_library( + name = "l3_admit_test", + testonly = True, + srcs = ["l3_admit_test.cc"], + hdrs = ["l3_admit_test.h"], + deps = [ + ":util", + "//gutil:proto", + "//gutil:status_matchers", + "//lib/gnmi:gnmi_helper", + "//p4_pdpi:ir", + "//p4_pdpi:ir_cc_proto", + "//p4_pdpi:p4_runtime_session", + "//p4_pdpi/packetlib", + "//p4_pdpi/packetlib:packetlib_cc_proto", + "//sai_p4/instantiations/google:instantiations", + "//sai_p4/instantiations/google:sai_p4info_cc", + "//tests/lib:p4rt_fixed_table_programming_helper", + "//tests/lib:packet_in_helper", + "//thinkit:mirror_testbed_fixture", + "@com_github_google_glog//:glog", + "@com_github_p4lang_p4runtime//:p4info_cc_proto", + "@com_github_p4lang_p4runtime//:p4runtime_cc_proto", + "@com_google_absl//absl/status", + "@com_google_absl//absl/strings", + "@com_google_absl//absl/strings:str_format", + "@com_google_absl//absl/time", + "@com_google_googletest//:gtest", + ], + alwayslink = True, +) + cc_library( name = "fuzzer_tests", testonly = True, diff --git a/tests/forwarding/arbitration_test.cc b/tests/forwarding/arbitration_test.cc index 8ad11c7d..424927a3 100644 --- a/tests/forwarding/arbitration_test.cc +++ b/tests/forwarding/arbitration_test.cc @@ -105,31 +105,26 @@ WriteRequest GetWriteRequest(int num, absl::uint128 election_id, testing::Matcher NotPrimary() { return Not(gutil::IsOk()); } TEST_P(ArbitrationTestFixture, BecomePrimary) { - TestEnvironment().SetTestCaseID("c6506d76-5041-4f69-b398-a808ab473186"); ASSERT_OK_AND_ASSIGN(auto connection, BecomePrimary(0)); } TEST_P(ArbitrationTestFixture, FailToBecomePrimary) { - TestEnvironment().SetTestCaseID("60c56f72-96ca-4aea-8cdc-16e1b928d53a"); ASSERT_OK_AND_ASSIGN(auto connection, BecomePrimary(1)); ASSERT_THAT(BecomePrimary(0).status(), NotPrimary()); } TEST_P(ArbitrationTestFixture, ReplacePrimary) { - TestEnvironment().SetTestCaseID("03da98ad-c4c7-443f-bcc0-53f97103d0c3"); ASSERT_OK_AND_ASSIGN(auto connection1, BecomePrimary(1)); ASSERT_OK_AND_ASSIGN(auto connection2, BecomePrimary(2)); } TEST_P(ArbitrationTestFixture, ReplacePrimaryAfterFailure) { - TestEnvironment().SetTestCaseID("d5ffe4cc-ff0e-4d93-8334-a23f06c6232a"); ASSERT_OK_AND_ASSIGN(auto connection1, BecomePrimary(1)); ASSERT_THAT(BecomePrimary(0).status(), NotPrimary()); ASSERT_OK_AND_ASSIGN(auto connection2, BecomePrimary(2)); } TEST_P(ArbitrationTestFixture, FailToBecomePrimaryAfterPrimaryDisconnect) { - TestEnvironment().SetTestCaseID("53b4b886-c218-4c85-b212-13d32105c795"); { ASSERT_OK_AND_ASSIGN(auto connection, BecomePrimary(1)); ASSERT_OK(connection->Finish()); @@ -138,7 +133,6 @@ TEST_P(ArbitrationTestFixture, FailToBecomePrimaryAfterPrimaryDisconnect) { } TEST_P(ArbitrationTestFixture, ReconnectPrimary) { - TestEnvironment().SetTestCaseID("d95a4da4-139d-4bd6-a43c-dbdefb123fcf"); { ASSERT_OK_AND_ASSIGN(auto connection, BecomePrimary(0)); ASSERT_OK(connection->Finish()); @@ -147,13 +141,11 @@ TEST_P(ArbitrationTestFixture, ReconnectPrimary) { } TEST_P(ArbitrationTestFixture, DoublePrimary) { - TestEnvironment().SetTestCaseID("19614b15-ce8f-4832-9164-342c5585283a"); ASSERT_OK_AND_ASSIGN(auto connection, BecomePrimary(0)); ASSERT_THAT(BecomePrimary(0).status(), NotPrimary()); } TEST_P(ArbitrationTestFixture, LongEvolution) { - TestEnvironment().SetTestCaseID("a65deb93-e350-4322-a932-af699c4b583c"); { ASSERT_OK_AND_ASSIGN(auto connection1, BecomePrimary(1)); ASSERT_THAT(BecomePrimary(0).status(), NotPrimary()); @@ -182,7 +174,6 @@ TEST_P(ArbitrationTestFixture, LongEvolution) { } TEST_P(ArbitrationTestFixture, BackupCannotWrite) { - TestEnvironment().SetTestCaseID("64c714d8-73c6-48b1-ada6-8ac2e5267714"); ASSERT_OK_AND_ASSIGN(auto connection, BecomePrimary(2)); ASSERT_OK_AND_ASSIGN(auto stub, Stub()); @@ -200,7 +191,6 @@ TEST_P(ArbitrationTestFixture, BackupCannotWrite) { } TEST_P(ArbitrationTestFixture, BackupCanRead) { - TestEnvironment().SetTestCaseID("fb678921-d150-4535-b7b8-fc8cecb79a78"); ASSERT_OK_AND_ASSIGN(auto connection, BecomePrimary(1)); @@ -231,7 +221,6 @@ TEST_P(ArbitrationTestFixture, BackupCanRead) { } TEST_P(ArbitrationTestFixture, GetNotifiedOfActualPrimary) { - TestEnvironment().SetTestCaseID("46b83014-759b-4393-bb58-220c0ca38711"); ASSERT_OK_AND_ASSIGN(auto connection, BecomePrimary(1)); // Assemble arbitration request. @@ -265,7 +254,6 @@ TEST_P(ArbitrationTestFixture, GetNotifiedOfActualPrimary) { } TEST_P(ArbitrationTestFixture, NoIdControllerCannotBecomePrimary) { - TestEnvironment().SetTestCaseID("3699fc43-5ff8-44ee-8965-68f42c71c1ed"); // Assemble arbitration request. p4::v1::StreamMessageRequest request; @@ -295,7 +283,6 @@ TEST_P(ArbitrationTestFixture, NoIdControllerCannotBecomePrimary) { } TEST_P(ArbitrationTestFixture, OldPrimaryCannotWriteAfterNewPrimaryCameUp) { - TestEnvironment().SetTestCaseID("e4bc86a2-84f0-450a-888a-8a6f5f26fa8c"); int id1 = 1, id2 = 2; // Connects controller C1 with id=1 to become primary. @@ -321,7 +308,6 @@ TEST_P(ArbitrationTestFixture, OldPrimaryCannotWriteAfterNewPrimaryCameUp) { } TEST_P(ArbitrationTestFixture, PrimaryDowngradesItself) { - TestEnvironment().SetTestCaseID("3cb62c0f-4a1a-430c-978c-a3a2a11078cd"); int id1 = 1, id2 = 2; // Connects controller with id=2 to become primary. diff --git a/tests/forwarding/l3_admit_test.cc b/tests/forwarding/l3_admit_test.cc new file mode 100644 index 00000000..97d6c8a1 --- /dev/null +++ b/tests/forwarding/l3_admit_test.cc @@ -0,0 +1,98 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#include "tests/forwarding/l3_admit_test.h" + +#include +#include +#include + +#include "absl/status/status.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/str_format.h" +#include "absl/strings/string_view.h" +#include "absl/strings/substitute.h" +#include "absl/time/clock.h" +#include "absl/time/time.h" +#include "glog/logging.h" +#include "gmock/gmock.h" +#include "gutil/proto.h" +#include "gutil/status_matchers.h" +#include "lib/gnmi/gnmi_helper.h" +#include "p4/v1/p4runtime.pb.h" +#include "p4_pdpi/ir.h" +#include "p4_pdpi/ir.pb.h" +#include "p4_pdpi/p4_runtime_session.h" +#include "p4_pdpi/packetlib/packetlib.h" +#include "p4_pdpi/packetlib/packetlib.pb.h" +#include "tests/forwarding/util.h" +#include "tests/lib/p4rt_fixed_table_programming_helper.h" +#include "tests/lib/packet_in_helper.h" +#include "thinkit/mirror_testbed_fixture.h" + +namespace pins { +namespace { + +absl::Status AddAndSetDefaultVrf(pdpi::P4RuntimeSession& session, + const pdpi::IrP4Info& ir_p4info, + const std::string& vrf_id) { + pdpi::IrWriteRequest ir_write_request; + RETURN_IF_ERROR(gutil::ReadProtoFromString( + absl::Substitute(R"pb( + updates { + type: INSERT + table_entry { + table_name: "vrf_table" + matches { + name: "vrf_id" + exact { str: "$0" } + } + action { name: "no_action" } + } + } + updates { + type: INSERT + table_entry { + table_name: "acl_pre_ingress_table" + priority: 2000 + action { + name: "set_vrf" + params { + name: "vrf_id" + value { str: "$0" } + } + } + } + } + )pb", + vrf_id), + &ir_write_request)); + ASSIGN_OR_RETURN(p4::v1::WriteRequest pi_write_request, + pdpi::IrWriteRequestToPi(ir_p4info, ir_write_request)); + return pdpi::SetMetadataAndSendPiWriteRequest(&session, pi_write_request); +} +} // namespace + +TEST_P(L3AdmitTestFixture, L3PacketsAreRoutedWhenMacAddressIsInMyStation) { + LOG(INFO) << "Starting test."; + + // PacketIO handlers for both the SUT and control switch. + std::unique_ptr packetio_sut = + std::make_unique(p4rt_sut_switch_session_.get(), + PacketInHelper::NoFilter); + std::unique_ptr packetio_control = + std::make_unique(p4rt_control_switch_session_.get(), + PacketInHelper::NoFilter); +} + +} // namespace pins diff --git a/tests/forwarding/l3_admit_test.h b/tests/forwarding/l3_admit_test.h new file mode 100644 index 00000000..b64889ff --- /dev/null +++ b/tests/forwarding/l3_admit_test.h @@ -0,0 +1,40 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#ifndef PINS_TESTS_FORWARDING_L3_ADMIT_TEST_H_ +#define PINS_TESTS_FORWARDING_L3_ADMIT_TEST_H_ + +#include + +#include "p4/config/v1/p4info.pb.h" +#include "p4_pdpi/ir.pb.h" +#include "p4_pdpi/p4_runtime_session.h" +#include "sai_p4/instantiations/google/instantiations.h" +#include "sai_p4/instantiations/google/sai_p4info.h" +#include "tests/lib/packet_in_helper.h" +#include "thinkit/mirror_testbed_fixture.h" + +namespace pins { + +class L3AdmitTestFixture : public thinkit::MirrorTestbedFixture { + protected: + + // This test runs on a mirror testbed setup so we open a P4RT connection to + // both switches. + std::unique_ptr p4rt_sut_switch_session_; + std::unique_ptr p4rt_control_switch_session_; +}; + +} // namespace pins + +#endif // PINS_TESTS_FORWARDING_L3_ADMIT_TEST_H_ diff --git a/tests/forwarding/smoke_test.cc b/tests/forwarding/smoke_test.cc index e45cfbd9..53576bfb 100644 --- a/tests/forwarding/smoke_test.cc +++ b/tests/forwarding/smoke_test.cc @@ -75,8 +75,6 @@ TEST_P(SmokeTestFixture, DISABLED_ModifyWorks) { // TODO: Enable once the bug is fixed. TEST_P(SmokeTestFixture, DISABLED_Bug181149419) { - GetMirrorTestbed().Environment().SetTestCaseID( - "e6ba12b7-18e0-4681-9562-87e2fc01d429"); // Adding 8 mirror sessions should succeed. for (int i = 0; i < 8; i++) { sai::TableEntry pd_entry = gutil::ParseProtoOrDie( @@ -159,8 +157,6 @@ TEST_P(SmokeTestFixture, DISABLED_Bug181149419) { } TEST_P(SmokeTestFixture, InsertTableEntry) { - GetMirrorTestbed().Environment().SetTestCaseID( - "da103fbb-8fd4-4385-b997-34e12a41004b"); const sai::TableEntry pd_entry = gutil::ParseProtoOrDie( R"pb( router_interface_table_entry { @@ -177,8 +173,6 @@ TEST_P(SmokeTestFixture, InsertTableEntry) { } TEST_P(SmokeTestFixture, InsertTableEntryWithRandomCharacterId) { - GetMirrorTestbed().Environment().SetTestCaseID( - "bd22f5fe-4103-4729-91d0-cb2bc8258940"); sai::TableEntry pd_entry = gutil::ParseProtoOrDie( R"pb( router_interface_table_entry { @@ -199,8 +193,6 @@ TEST_P(SmokeTestFixture, InsertTableEntryWithRandomCharacterId) { } TEST_P(SmokeTestFixture, InsertAndReadTableEntries) { - GetMirrorTestbed().Environment().SetTestCaseID( - "8bdacde4-b261-4242-b65d-462c828a427d"); pdpi::P4RuntimeSession* session = SutP4RuntimeSession(); const pdpi::IrP4Info& ir_p4info = IrP4Info(); std::vector write_pd_entries = diff --git a/tests/forwarding/watch_port_test.cc b/tests/forwarding/watch_port_test.cc index ff970274..caae646b 100644 --- a/tests/forwarding/watch_port_test.cc +++ b/tests/forwarding/watch_port_test.cc @@ -515,7 +515,6 @@ namespace { TEST_P(WatchPortTestFixture, VerifyBasicWcmpPacketDistribution) { thinkit::TestEnvironment& environment = GetParam().testbed->GetMirrorTestbed().Environment(); - environment.SetTestCaseID("9a4c3dac-44bd-489e-9237-d396b66c85f5"); absl::Span controller_port_ids = GetParam().port_ids; const int group_size = kNumWcmpMembersForTest; @@ -597,7 +596,6 @@ TEST_P(WatchPortTestFixture, VerifyBasicWcmpPacketDistribution) { TEST_P(WatchPortTestFixture, VerifyBasicWatchPortAction) { thinkit::TestEnvironment& environment = GetParam().testbed->GetMirrorTestbed().Environment(); - environment.SetTestCaseID("992725de-2051-49bb-928f-7b089643a9bd"); absl::Span controller_port_ids = GetParam().port_ids; const int group_size = kNumWcmpMembersForTest; @@ -712,7 +710,6 @@ TEST_P(WatchPortTestFixture, VerifyWatchPortActionInCriticalState) { } thinkit::MirrorTestbed& testbed = GetParam().testbed->GetMirrorTestbed(); thinkit::TestEnvironment& environment = testbed.Environment(); - environment.SetTestCaseID("964c7a38-b073-4296-85be-2bba1e33c6f9"); absl::Span controller_port_ids = GetParam().port_ids; const int group_size = kNumWcmpMembersForTest; @@ -821,7 +818,6 @@ TEST_P(WatchPortTestFixture, VerifyWatchPortActionInCriticalState) { TEST_P(WatchPortTestFixture, VerifyWatchPortActionForSingleMember) { thinkit::TestEnvironment& environment = GetParam().testbed->GetMirrorTestbed().Environment(); - environment.SetTestCaseID("60da7a07-1217-4d63-9716-1219d62065ff"); absl::Span controller_port_ids = GetParam().port_ids; const int group_size = 1; @@ -935,7 +931,6 @@ TEST_P(WatchPortTestFixture, VerifyWatchPortActionForSingleMember) { TEST_P(WatchPortTestFixture, VerifyWatchPortActionForMemberModify) { thinkit::TestEnvironment& environment = GetParam().testbed->GetMirrorTestbed().Environment(); - environment.SetTestCaseID("e93160fb-be64-495b-bb4d-f06a92c51e76"); absl::Span controller_port_ids = GetParam().port_ids; const int group_size = kNumWcmpMembersForTest; @@ -1048,7 +1043,6 @@ TEST_P(WatchPortTestFixture, VerifyWatchPortActionForMemberModify) { TEST_P(WatchPortTestFixture, VerifyWatchPortActionForDownPortMemberInsert) { thinkit::TestEnvironment& environment = GetParam().testbed->GetMirrorTestbed().Environment(); - environment.SetTestCaseID("e54da480-d2cc-42c6-bced-0354b5ab3329"); absl::Span controller_port_ids = GetParam().port_ids; const int group_size = kNumWcmpMembersForTest; ASSERT_OK_AND_ASSIGN(std::vector members, diff --git a/tests/integration/system/random_blackbox_events_tests.cc b/tests/integration/system/random_blackbox_events_tests.cc index bb797963..2fbc5bab 100644 --- a/tests/integration/system/random_blackbox_events_tests.cc +++ b/tests/integration/system/random_blackbox_events_tests.cc @@ -97,7 +97,6 @@ TEST_P(RandomBlackboxEventsTest, ControlPlaneWithTrafficWithoutValidation) { count: 2 interface_mode: CONTROL_INTERFACE })pb"))); - testbed->Environment().SetTestCaseID("491b3f60-1369-4099-9385-da5dd44a087d"); // Initial sanity check. ASSERT_OK(SwitchReady(testbed->Sut())); diff --git a/tests/lib/BUILD.bazel b/tests/lib/BUILD.bazel index 9d930731..d6e7f410 100644 --- a/tests/lib/BUILD.bazel +++ b/tests/lib/BUILD.bazel @@ -112,3 +112,17 @@ cmd_diff_test( ":switch_test_setup_helpers_golden_test_runner", ], ) + +cc_library( + name = "packet_in_helper", + srcs = ["packet_in_helper.cc"], + hdrs = ["packet_in_helper.h"], + deps = [ + "//p4_pdpi:p4_runtime_session", + "@com_github_google_glog//:glog", + "@com_github_p4lang_p4runtime//:p4runtime_cc_proto", + "@com_google_absl//absl/status", + "@com_google_absl//absl/status:statusor", + "@com_google_absl//absl/synchronization", + ], +) diff --git a/tests/lib/packet_in_helper.cc b/tests/lib/packet_in_helper.cc new file mode 100644 index 00000000..9a5623d5 --- /dev/null +++ b/tests/lib/packet_in_helper.cc @@ -0,0 +1,89 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#include "tests/lib/packet_in_helper.h" + +#include "absl/status/status.h" +#include "absl/synchronization/mutex.h" +#include "glog/logging.h" +#include "p4/v1/p4runtime.pb.h" +#include "p4_pdpi/p4_runtime_session.h" + +namespace pins { + +PacketInHelper::PacketInHelper( + pdpi::P4RuntimeSession* p4rt_session, + std::function + packet_in_message_filter) + : p4rt_session_(*p4rt_session), + packet_in_message_filter_(std::move(packet_in_message_filter)) { + CHECK(p4rt_session != nullptr); // Crash OK in ctor. + + packet_in_thread_ = std::thread([&]() { + LOG(INFO) << "Start monitoring PacketIO events."; + p4::v1::StreamMessageResponse response; + while (p4rt_session_.StreamChannelRead(response)) { + if (packet_in_message_filter_(response)) { + PushBackPacketInMessage(response); + } + } + }); +} + +PacketInHelper::~PacketInHelper() { + // If the thread was never started then there isn't any other cleanup needed. + if (!packet_in_thread_.joinable()) return; + + // Otherwise we try to stop the P4RT session, and join back the thread. + absl::Status stop_session = p4rt_session_.Finish(); + if (!stop_session.ok()) { + LOG(ERROR) << "Problem stopping the P4RT session: " << stop_session; + return; + } + + packet_in_thread_.join(); + LOG(INFO) << "Stopped monitoring PacketIO events."; +} + +bool PacketInHelper::NoFilter(const p4::v1::StreamMessageResponse& response) { + return true; +} + +bool PacketInHelper::HasPacketInMessage() const { + absl::MutexLock l(&packet_in_lock_); + return !packet_in_messages_.empty(); +} + +absl::StatusOr +PacketInHelper::GetNextPacketInMessage() { + absl::MutexLock l(&packet_in_lock_); + + // If the queue is empty then we return an error. + if (packet_in_messages_.empty()) { + return absl::Status(absl::StatusCode::kOutOfRange, + "The PacketIn queue is empty."); + } + + // Otherwise, we return the next packet in the queue. + p4::v1::StreamMessageResponse message = packet_in_messages_.front(); + packet_in_messages_.pop(); + return message; +} + +void PacketInHelper::PushBackPacketInMessage( + const p4::v1::StreamMessageResponse& response) { + absl::MutexLock l(&packet_in_lock_); + packet_in_messages_.push(response); +} + +} // namespace pins diff --git a/tests/lib/packet_in_helper.h b/tests/lib/packet_in_helper.h new file mode 100644 index 00000000..89838d82 --- /dev/null +++ b/tests/lib/packet_in_helper.h @@ -0,0 +1,89 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#ifndef PINS_TESTS_LIB_PACKET_IO_HELPER_H_ +#define PINS_TESTS_LIB_PACKET_IO_HELPER_H_ + +#include +#include +#include // NOLINT: third_party code. + +#include "absl/status/statusor.h" +#include "absl/synchronization/mutex.h" +#include "p4/v1/p4runtime.pb.h" +#include "p4_pdpi/p4_runtime_session.h" + +namespace pins { + +// A helper class for managing P4RT PacketIO requests. On construction it will +// spawn a thread to asynchronously collect PacketIn messages from the switch. +// +// NOTE: This class will not take ownership of a P4RT session, but on +// destruction it will close the gRPC stream channel to the switch. +class PacketInHelper final { + public: + // Spawns a thread to collect PacketIn messages sent from the switch. + // + // A filter can be used to limit the type of messages collected. The filter + // should return true for any packets the test wants to collect, and false for + // any packets the test wants to ignore. + explicit PacketInHelper( + pdpi::P4RuntimeSession* p4rt_session, + std::function + packet_in_message_filter); + + // Closes the P4RuntimeSession's stream, and joins the PacketIn thread. + ~PacketInHelper(); + + // Always returns true so no packet gets filtered out. + static bool NoFilter(const p4::v1::StreamMessageResponse& response); + + // Returns true if the PacketIn queue has packets. Otherwise it returns false. + bool HasPacketInMessage() const ABSL_LOCKS_EXCLUDED(packet_in_lock_); + + // Returns the next packet in the queue. If no packet exists in the queue it + // will return an OUT_OF_BOUNDS error. + absl::StatusOr GetNextPacketInMessage() + ABSL_LOCKS_EXCLUDED(packet_in_lock_); + + private: + // Helper method used by the PacketIn thread to update the PacketIn messages. + void PushBackPacketInMessage(const p4::v1::StreamMessageResponse& response) + ABSL_LOCKS_EXCLUDED(packet_in_lock_); + + pdpi::P4RuntimeSession& p4rt_session_; + + // Thread is spawned in ctor and joined in dtor. It will wait for a PacketIn + // message then update the PacketIn message queue. + std::thread packet_in_thread_; + + // Accessing the packet-in queue is lock protected because the P4RT server is + // sending new packets while the tests can be reading them back. + mutable absl::Mutex packet_in_lock_; + + // Hold all PacketIn messages until the test reads them out. + std::queue packet_in_messages_ + ABSL_GUARDED_BY(packet_in_lock_); + + // A filter to restrict which packets are actually collected by the + // PacketInHelper class. + // + // return true to collect the packet. + // return false to ignore the packet. + std::function + packet_in_message_filter_; +}; + +} // namespace pins + +#endif // PINS_TESTS_LIB_PACKET_IO_HELPER_H_ diff --git a/tests/mtu_tests/mtu_tests.cc b/tests/mtu_tests/mtu_tests.cc index 937eeabc..1e4af7e3 100644 --- a/tests/mtu_tests/mtu_tests.cc +++ b/tests/mtu_tests/mtu_tests.cc @@ -208,7 +208,6 @@ void SetPortMtu(gnmi::gNMI::StubInterface* stub, absl::string_view intf, } TEST_P(MtuRoutingTestFixture, MtuTest) { - testbed_->Environment().SetTestCaseID("cfb97855-fe79-494a-aef2-92821aef3b1f"); // Get original mtu on port under test on SUT. std::string if_state_path = absl::StrCat( "interfaces/interface[name=", destination_link_.sut_interface, diff --git a/tests/qos/cpu_qos_test.cc b/tests/qos/cpu_qos_test.cc index 11badcc3..2011e722 100644 --- a/tests/qos/cpu_qos_test.cc +++ b/tests/qos/cpu_qos_test.cc @@ -94,7 +94,7 @@ constexpr int kFrameCheckSequenceSize = 4; // gNMI have to be incremented after a packet hits a queue. // Empirically, for PINS, queue counters currently seem to get updated every // 10 seconds. -constexpr absl::Duration kMaxQueueCounterUpdateTime = absl::Seconds(15); +constexpr absl::Duration kMaxQueueCounterUpdateTime = absl::Seconds(20); // After pushing gNMI config to a switch, the tests sleep for this duration // assuming that the gNMI config will have been fully applied afterwards. @@ -487,6 +487,17 @@ TEST_P(CpuQosTestWithoutIxia, PerEntryAclCounterIncrementsWhenEntryIsHit) { ASSERT_OK_AND_ASSIGN(const pdpi::IrP4Info ir_p4info, pdpi::CreateIrP4Info(p4info)); + // Set up P4Runtime. + EXPECT_OK( + Testbed().Environment().StoreTestArtifact("p4info.textproto", p4info)); + ASSERT_OK_AND_ASSIGN( + std::unique_ptr sut_p4rt_session, + pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(sut, p4info)); + ASSERT_OK_AND_ASSIGN( + std::unique_ptr control_p4rt_session, + pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(control_device, + p4info)); + // Set up gNMI. EXPECT_OK(Testbed().Environment().StoreTestArtifact("gnmi_config.json", GetParam().gnmi_config)); @@ -505,16 +516,6 @@ TEST_P(CpuQosTestWithoutIxia, PerEntryAclCounterIncrementsWhenEntryIsHit) { LOG(INFO) << "Link used to inject test packets: " << link_used_for_test_packets; - // Set up P4Runtime. - EXPECT_OK( - Testbed().Environment().StoreTestArtifact("p4info.textproto", p4info)); - ASSERT_OK_AND_ASSIGN( - std::unique_ptr sut_p4rt_session, - pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(sut, p4info)); - ASSERT_OK_AND_ASSIGN( - std::unique_ptr control_p4rt_session, - pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(control_device, - p4info)); // We install a RIF to make this test non-trivial, as all packets are dropped // by default if no RIF exists (b/190736007). ASSERT_OK_AND_ASSIGN( @@ -781,6 +782,17 @@ TEST_P(CpuQosTestWithoutIxia, ASSERT_OK_AND_ASSIGN(const pdpi::IrP4Info ir_p4info, pdpi::CreateIrP4Info(p4info)); + // Set up P4Runtime. + EXPECT_OK( + Testbed().Environment().StoreTestArtifact("p4info.textproto", p4info)); + ASSERT_OK_AND_ASSIGN( + std::unique_ptr sut_p4rt_session, + pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(sut, p4info)); + ASSERT_OK_AND_ASSIGN( + std::unique_ptr control_p4rt_session, + pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(control_device, + p4info)); + // Set up gNMI. EXPECT_OK(Testbed().Environment().StoreTestArtifact("gnmi_config.json", GetParam().gnmi_config)); @@ -799,16 +811,6 @@ TEST_P(CpuQosTestWithoutIxia, LOG(INFO) << "Link used to inject test packets: " << link_used_for_test_packets; - // Set up P4Runtime. - EXPECT_OK( - Testbed().Environment().StoreTestArtifact("p4info.textproto", p4info)); - ASSERT_OK_AND_ASSIGN( - std::unique_ptr sut_p4rt_session, - pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(sut, p4info)); - ASSERT_OK_AND_ASSIGN( - std::unique_ptr control_p4rt_session, - pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(control_device, - p4info)); // We install a RIF to make this test non-trivial, as all packets are dropped // by default if no RIF exists (b/190736007). ASSERT_OK_AND_ASSIGN( @@ -892,6 +894,17 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) { ASSERT_OK_AND_ASSIGN(const pdpi::IrP4Info ir_p4info, pdpi::CreateIrP4Info(p4info)); + // Set up P4Runtime. + EXPECT_OK( + Testbed().Environment().StoreTestArtifact("p4info.textproto", p4info)); + ASSERT_OK_AND_ASSIGN( + std::unique_ptr p4rt_session, + pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(sut, p4info)); + ASSERT_OK_AND_ASSIGN( + std::unique_ptr control_p4rt_session, + pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(control_device, + p4info)); + // Set up gNMI. EXPECT_OK(Testbed().Environment().StoreTestArtifact("gnmi_config.json", GetParam().gnmi_config)); @@ -909,16 +922,6 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) { LOG(INFO) << "Link used to inject test packets: " << link_used_for_test_packets; - // Set up P4Runtime. - EXPECT_OK( - Testbed().Environment().StoreTestArtifact("p4info.textproto", p4info)); - ASSERT_OK_AND_ASSIGN( - std::unique_ptr p4rt_session, - pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(sut, p4info)); - ASSERT_OK_AND_ASSIGN( - std::unique_ptr control_p4rt_session, - pdpi::P4RuntimeSession::CreateWithP4InfoAndClearTables(control_device, - p4info)); // TODO: Unless a RIF exists at the test packet ingress port, // packets will be dropped. Remove this once these RIFs are set up via // gNMI. @@ -1015,7 +1018,10 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) { CumulativeNumPacketsEnqueued(queue_counters_before_test_packet) && absl::Now() - time_packet_sent < kMaxQueueCounterUpdateTime); - EXPECT_EQ( + // We terminate early if this fails, as that can cause this loop to get + // out of sync when counters increment after a long delay, resulting in + // confusing error messages where counters increment by 2. + ASSERT_EQ( CumulativeNumPacketsEnqueued(queue_counters_after_test_packet), CumulativeNumPacketsEnqueued(queue_counters_before_test_packet) + 1) << "Counters for queue " << target_queue @@ -1034,7 +1040,7 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) { // Level of tolerance for packet rate verification. // This could be parameterized in future if this is platform // dependent. -constexpr float kTolerancePercent = 3.0; +constexpr float kTolerancePercent = 4.0; // Ixia configurations: // 1. Frames sent per second by Ixia. @@ -1057,6 +1063,39 @@ struct PacketReceiveInfo { absl::Time time_last_packet_punted ABSL_GUARDED_BY(mutex); }; +// Structure represents a link between SUT and Ixia. +// This is represented by Ixia interface name and the SUT's gNMI interface +// name. +struct IxiaLink { + std::string ixia_interface; + std::string sut_interface; +}; +// Go over the connections and return vector of connections +// whose links are up. +absl::StatusOr> GetReadyIxiaLinks( + thinkit::GenericTestbed &generic_testbed, + gnmi::gNMI::StubInterface &gnmi_stub) { + std::vector links; + absl::flat_hash_map interface_info = + generic_testbed.GetSutInterfaceInfo(); + // Loop through the interface_info looking for Ixia/SUT interface pairs, + // checking if the link is up. Add the pair to connections. + for (const auto &[interface, info] : interface_info) { + bool sut_link_up = false; + if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) + { + ASSIGN_OR_RETURN(sut_link_up, CheckLinkUp(interface, gnmi_stub)); + if (sut_link_up) { + links.push_back({ + .ixia_interface = info.peer_interface_name, + .sut_interface = interface, + }); + } + } + } + return links; +} + TEST_P(CpuQosTestWithIxia, TestCPUQueueAssignmentAndQueueRateLimit) { // Pick a testbed with an Ixia Traffic Generator. auto requirements = @@ -1116,6 +1155,9 @@ TEST_P(CpuQosTestWithIxia, TestPuntFlowRateLimitAndCounters) { std::unique_ptr generic_testbed, GetParam().testbed_interface->GetTestbedWithRequirements(requirements)); + // TODO: Skip test till known failure is fixed. + GTEST_SKIP() << "Skipping till b/203545459 is fixed"; + ASSERT_OK(generic_testbed->Environment().StoreTestArtifact( "gnmi_config.txt", GetParam().gnmi_config)); @@ -1136,40 +1178,46 @@ TEST_P(CpuQosTestWithIxia, TestPuntFlowRateLimitAndCounters) { const auto dest_ip = netaddr::Ipv4Address(172, 0, 0, 1); // Go through all the ports that interface to the Ixia and set them - // to 100GB since the Ixia ports are all 100GB. + // first to 200GB. const absl::flat_hash_map interface_info = generic_testbed->GetSutInterfaceInfo(); for (const auto &[interface, info] : interface_info) { if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) { - ASSERT_OK(SetPortSpeed("\"openconfig-if-ethernet:SPEED_100GB\"", + ASSERT_OK(SetPortSpeed("\"openconfig-if-ethernet:SPEED_200GB\"", interface, *gnmi_stub)); + ASSERT_OK(SetPortMtu(kMaxFrameSize, interface, *gnmi_stub)); } } // Wait to let the links come up. Switch guarantees state paths to reflect // in 10s. Lets wait for a bit more. + LOG(INFO) << "Sleeping " << kTimeToWaitForGnmiConfigToApply + << " to wait for config to be applied/links to come up."; absl::SleepFor(kTimeToWaitForGnmiConfigToApply); - // TODO: Move this to helper function. - // Loop through the interface_info looking for Ixia/SUT interface pairs, - // checking if the link is up. we need one pair with link up for the - // ingress interface/IXIA traffic generation. - std::string ixia_interface; - std::string sut_interface; - bool sut_link_up = false; - for (const auto &[interface, info] : interface_info) { - if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) { - ASSERT_OK_AND_ASSIGN(sut_link_up, CheckLinkUp(interface, *gnmi_stub)); - if (sut_link_up) { - ixia_interface = info.peer_interface_name; - sut_interface = interface; - break; + ASSERT_OK_AND_ASSIGN(std::vector ready_links, + GetReadyIxiaLinks(*generic_testbed, *gnmi_stub)); + // If links didnt come, lets try 100GB as some testbeds have 100GB + // IXIA connections. + if (ready_links.empty()) { + for (const auto &[interface, info] : interface_info) { + if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) + ASSERT_OK(SetPortSpeed("\"openconfig-if-ethernet:SPEED_100GB\"", + interface, *gnmi_stub)); } } - } - - ASSERT_TRUE(sut_link_up); + // Wait to let the links come up. Switch guarantees state paths to reflect + // in 10s. Lets wait for a bit more. + LOG(INFO) << "Sleeping " << kTimeToWaitForGnmiConfigToApply + << " to wait for config to be applied/links to come up."; + absl::SleepFor(kTimeToWaitForGnmiConfigToApply); + ASSERT_OK_AND_ASSIGN(ready_links, + GetReadyIxiaLinks(*generic_testbed, *gnmi_stub)); + + ASSERT_FALSE(ready_links.empty()) << "Ixia links are not ready"; + std::string ixia_interface = ready_links[0].ixia_interface; + std::string sut_interface = ready_links[0].sut_interface; // We will perform the following steps with Ixia: // Set up Ixia traffic. @@ -1260,6 +1308,9 @@ TEST_P(CpuQosTestWithIxia, TestPuntFlowRateLimitAndCounters) { flow_rate_limit_in_bytes_per_second, /*burst_in_bytes=*/kMaxFrameSize, GetParam().p4info, *sut_p4_session)); + ASSERT_OK_AND_ASSIGN( + QueueCounters initial_counters, + GetGnmiQueueCounters("CPU", queue_info.gnmi_queue_name, *gnmi_stub)); // Reset received packet count at tester for each iteration. { @@ -1302,8 +1353,40 @@ TEST_P(CpuQosTestWithIxia, TestPuntFlowRateLimitAndCounters) { << " after injecting the Ixia test packets via CPU queue " << queue_name; - // TODO: Remove masking this failure once bug is fixed. - if (!generic_testbed->Environment().MaskKnownFailures()) { + // Verify GNMI queue stats match packets received. + static constexpr absl::Duration kPollInterval = absl::Seconds(5); + static constexpr absl::Duration kTotalTime = absl::Seconds(20); + static const int kIterations = kTotalTime / kPollInterval; + // Check for counters every 5 seconds upto 20 seconds till they match. + for (int gnmi_counters_check = 0; gnmi_counters_check < kIterations; + gnmi_counters_check++) { + absl::SleepFor(kPollInterval); + QueueCounters final_counters; + QueueCounters delta_counters; + ASSERT_OK_AND_ASSIGN( + final_counters, + GetGnmiQueueCounters("CPU", queue_info.gnmi_queue_name, *gnmi_stub)); + delta_counters = { + .num_packets_transmitted = final_counters.num_packets_transmitted - + initial_counters.num_packets_transmitted, + .num_packet_dropped = final_counters.num_packet_dropped - + initial_counters.num_packet_dropped, + }; + LOG(INFO) << delta_counters; + absl::MutexLock lock(&packet_receive_info.mutex); + if (delta_counters.num_packets_transmitted == + packet_receive_info.num_packets_punted) { + break; + } + ASSERT_NE(gnmi_counters_check, kIterations - 1) + << "GNMI packet count " + << delta_counters.num_packets_transmitted + + delta_counters.num_packet_dropped + << " != Packets received at controller " + << packet_receive_info.num_packets_punted; + } + + { absl::MutexLock lock(&packet_receive_info.mutex); LOG(INFO) << "Packets received at Controller: " diff --git a/tests/qos/frontpanel_qos_test.cc b/tests/qos/frontpanel_qos_test.cc index e5137f6d..ba4a8934 100644 --- a/tests/qos/frontpanel_qos_test.cc +++ b/tests/qos/frontpanel_qos_test.cc @@ -22,9 +22,6 @@ TEST_P(FrontpanelQosTest, PacketsGetMappedToCorrectQueuesBasedOnDscp) { std::unique_ptr testbed, GetParam().testbed_interface->GetTestbedWithRequirements(requirements)); - // Set test case ID. - testbed->Environment().SetTestCaseID("TODO: insert proper ID"); - // Switch set up. ASSERT_OK_AND_ASSIGN( std::unique_ptr sut_p4rt,