diff --git a/WORKSPACE.bazel b/WORKSPACE.bazel index 91baf0325..431c63c53 100644 --- a/WORKSPACE.bazel +++ b/WORKSPACE.bazel @@ -119,6 +119,15 @@ load("@rules_pkg//:deps.bzl", "rules_pkg_dependencies") rules_pkg_dependencies() +# -- Load Yggdrasil Decision Forests ------------------------------------------- + +load("@com_google_ydf//yggdrasil_decision_forests:library.bzl", ydf_load_deps = "load_dependencies") + +ydf_load_deps( + exclude_repo = "tensorflow", + repo_name = "@com_google_ydf", +) + # == Dependencies needed for testing and formatting only ======================= # -- Load p4c ------------------------------------------------------------------ diff --git a/dvaas/BUILD.bazel b/dvaas/BUILD.bazel index 4d88c9295..5c50b4a34 100644 --- a/dvaas/BUILD.bazel +++ b/dvaas/BUILD.bazel @@ -51,6 +51,7 @@ cc_library( "//p4_pdpi:p4_runtime_session_extras", "//p4_pdpi:sequencing", "//p4_pdpi/packetlib:packetlib_cc_proto", + "//p4_pdpi/string_encodings:hex_string", "//p4_symbolic/packet_synthesizer:coverage_goal_cc_proto", "//p4_symbolic/packet_synthesizer:packet_synthesizer_cc_proto", "//sai_p4/instantiations/google:p4_versions", @@ -71,6 +72,86 @@ cc_library( ], ) +proto_library( + name = "dvaas_detective_proto", + srcs = ["dvaas_detective.proto"], +) + +cc_proto_library( + name = "dvaas_detective_cc_proto", + deps = [":dvaas_detective_proto"], +) + +cc_library( + name = "dvaas_detective", + srcs = ["dvaas_detective.cc"], + hdrs = ["dvaas_detective.h"], + deps = [ + ":dvaas_detective_cc_proto", + ":test_vector_cc_proto", + "//gutil:overload", + "//gutil:proto", + "//p4_pdpi/packetlib:packetlib_cc_proto", + "@com_google_absl//absl/container:flat_hash_map", + "@com_google_absl//absl/memory", + "@com_google_absl//absl/status", + "@com_google_absl//absl/status:statusor", + "@com_google_absl//absl/strings", + "@com_google_absl//absl/strings:str_format", + "@com_google_absl//absl/types:variant", + "@com_google_ydf//yggdrasil_decision_forests/dataset:csv_example_reader", + "@com_google_ydf//yggdrasil_decision_forests/dataset:data_spec_cc_proto", + "@com_google_ydf//yggdrasil_decision_forests/dataset:data_spec_inference", + "@com_google_ydf//yggdrasil_decision_forests/learner:abstract_learner", + "@com_google_ydf//yggdrasil_decision_forests/learner:learner_library", + "@com_google_ydf//yggdrasil_decision_forests/learner/cart", + "@com_google_ydf//yggdrasil_decision_forests/learner/cart:cart_cc_proto", + "@com_google_ydf//yggdrasil_decision_forests/model:abstract_model", + "@com_google_ydf//yggdrasil_decision_forests/model/decision_tree", + "@com_google_ydf//yggdrasil_decision_forests/model/decision_tree:decision_tree_cc_proto", + "@com_google_ydf//yggdrasil_decision_forests/model/random_forest", + "@com_google_ydf//yggdrasil_decision_forests/utils:cast", + "@com_google_ydf//yggdrasil_decision_forests/utils:distribution_cc_proto", + ], +) + +# go/golden-test-with-coverage +cc_test( + name = "dvaas_detective_test", + srcs = ["dvaas_detective_test.cc"], + linkstatic = True, + deps = [ + ":dvaas_detective", + ":dvaas_detective_cc_proto", + ":test_vector_cc_proto", + "//gutil:proto", + "//gutil:status", + "//gutil:status_matchers", + "//gutil:testing", + "//p4_pdpi/packetlib:packetlib_cc_proto", + "@com_google_absl//absl/container:flat_hash_map", + "@com_google_absl//absl/log:check", + "@com_google_absl//absl/status", + "@com_google_googletest//:gtest_main", + "@com_google_ydf//yggdrasil_decision_forests/dataset:data_spec_cc_proto", + "@com_google_ydf//yggdrasil_decision_forests/model/decision_tree", + "@com_google_ydf//yggdrasil_decision_forests/model/decision_tree:decision_tree_cc_proto", + "@com_google_ydf//yggdrasil_decision_forests/model/random_forest", + ], +) + +cmd_diff_test( + name = "dvaas_detective_diff_test", + actual_cmd = " | ".join([ + "$(execpath :dvaas_detective_test)", + # Strip unnecessary lines for golden testing. + "sed '1,/^\\[ RUN/d'", # Strip everything up to a line beginning with '[ RUN'. + "sed '/^\\[/d'", # Strip every line beginning with '['. + ]), + expected = "dvaas_detective_test.expected", + tools = [":dvaas_detective_test"], +) + cc_test( name = "dataplane_validation_test", srcs = ["dataplane_validation_test.cc"], @@ -83,6 +164,7 @@ cc_test( "//gutil:test_artifact_writer", "//gutil:testing", "@com_google_absl//absl/container:btree", + "@com_google_absl//absl/log", "@com_google_absl//absl/status", "@com_google_absl//absl/strings:string_view", "@com_google_googletest//:gtest_main", @@ -106,6 +188,7 @@ cc_library( srcs = ["validation_result.cc"], hdrs = ["validation_result.h"], deps = [ + ":dvaas_detective", ":test_run_validation", ":test_vector_cc_proto", ":test_vector_stats", @@ -115,6 +198,7 @@ cc_library( "@com_google_absl//absl/algorithm:container", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", + "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", ], ) @@ -143,6 +227,7 @@ cc_library( "//p4_pdpi:p4_runtime_session", "//p4_pdpi/packetlib", "//p4_pdpi/packetlib:packetlib_cc_proto", + "//p4_pdpi/utils:ir", "//tests/forwarding:util", "@com_github_google_glog//:glog", "@com_github_p4lang_p4runtime//:p4runtime_cc_proto", @@ -168,6 +253,8 @@ cc_library( "@com_github_google_glog//:glog", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/container:flat_hash_set", + "@com_google_absl//absl/log", + "@com_google_absl//absl/log:check", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", @@ -185,19 +272,23 @@ cc_library( deps = [ ":packet_injection", ":port_id_map", + ":test_insights", ":test_run_validation", ":test_vector", ":test_vector_cc_proto", ":validation_result", "//gutil:proto", "//gutil:test_artifact_writer", + "//lib/p4rt:p4rt_port", "//p4_pdpi:ir", "//p4_pdpi:ir_cc_proto", "//p4_pdpi:p4_runtime_session", "//p4_pdpi:p4_runtime_session_extras", "//sai_p4/instantiations/google/test_tools:test_entries", - "@com_github_google_glog//:glog", + "//tests/lib:switch_test_setup_helpers", "@com_github_p4lang_p4runtime//:p4runtime_cc_proto", + "@com_google_absl//absl/container:btree", + "@com_google_absl//absl/log", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", @@ -525,6 +616,7 @@ cc_test( "//gutil:testing", "//p4_pdpi/packetlib:packetlib_cc_proto", "@com_google_absl//absl/status", + "@com_google_absl//absl/strings:string_view", "@com_google_googletest//:gtest_main", "@com_google_protobuf//:protobuf", ], diff --git a/dvaas/arriba_test_vector_validation.cc b/dvaas/arriba_test_vector_validation.cc index ba5c5a4d9..ea395ed2b 100644 --- a/dvaas/arriba_test_vector_validation.cc +++ b/dvaas/arriba_test_vector_validation.cc @@ -14,13 +14,17 @@ #include "dvaas/arriba_test_vector_validation.h" +#include #include #include +#include "absl/container/btree_set.h" +#include "absl/log/log.h" #include "absl/strings/str_join.h" #include "absl/strings/string_view.h" #include "dvaas/packet_injection.h" #include "dvaas/port_id_map.h" +#include "dvaas/test_insights.h" #include "dvaas/test_run_validation.h" #include "dvaas/test_vector.h" #include "dvaas/test_vector.pb.h" @@ -29,15 +33,34 @@ #include "gutil/proto.h" #include "gutil/status.h" #include "gutil/test_artifact_writer.h" +#include "lib/p4rt/p4rt_port.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/p4_runtime_session_extras.h" #include "sai_p4/instantiations/google/test_tools/test_entries.h" +#include "tests/lib/switch_test_setup_helpers.h" namespace dvaas { +absl::StatusOr> GetUsedP4rtPortIds( + const ArribaTestVector& arriba_test_vector, + const std::vector& used_entries_list, + const pdpi::IrP4Info& ir_p4_info) { + ASSIGN_OR_RETURN(absl::btree_set used_p4rt_port_ids, + pins_test::GetPortsUsed(ir_p4_info, used_entries_list)); + + for (const auto& [_, packet_test_vector] : + arriba_test_vector.packet_test_vector_by_id()) { + ASSIGN_OR_RETURN(pins_test::P4rtPortId p4rt_port_id, + pins_test::P4rtPortId::MakeFromP4rtEncoding( + packet_test_vector.input().packet().port())); + used_p4rt_port_ids.insert(p4rt_port_id); + } + return used_p4rt_port_ids; +} + absl::StatusOr ValidateAgainstArribaTestVector( pdpi::P4RuntimeSession& sut, pdpi::P4RuntimeSession& control_switch, const ArribaTestVector& arriba_test_vector, @@ -64,9 +87,11 @@ absl::StatusOr ValidateAgainstArribaTestVector( // Prepare single packet test vectors. PacketTestVectorById test_vector_by_id; + std::vector packet_test_vectors; for (const auto& [id, packet_test_vector] : arriba_test_vector.packet_test_vector_by_id()) { test_vector_by_id[id] = packet_test_vector; + packet_test_vectors.push_back(packet_test_vector); } PacketStatistics packet_statistics; @@ -112,6 +137,12 @@ absl::StatusOr ValidateAgainstArribaTestVector( RETURN_IF_ERROR(artifact_writer.AppendToTestArtifact( "test_outcomes.txtpb", gutil::PrintTextProto(test_outcomes))); + // Store test insights. + ASSIGN_OR_RETURN(const std::string insights_csv, + GetTestInsightsTableAsCsv(test_outcomes, ir_p4info)); + RETURN_IF_ERROR( + artifact_writer.AppendToTestArtifact("test_insights.csv", insights_csv)); + ValidationResult validation_result(std::move(test_outcomes), /*packet_synthesis_result=*/{}); RETURN_IF_ERROR(artifact_writer.AppendToTestArtifact( diff --git a/dvaas/arriba_test_vector_validation.h b/dvaas/arriba_test_vector_validation.h index 008e7e783..c94d10155 100644 --- a/dvaas/arriba_test_vector_validation.h +++ b/dvaas/arriba_test_vector_validation.h @@ -16,12 +16,15 @@ #define PINS_DVAAS_ARRIBA_TEST_VECTOR_VALIDATION_H_ #include +#include +#include "absl/container/btree_set.h" #include "absl/status/statusor.h" #include "dvaas/packet_injection.h" #include "dvaas/test_run_validation.h" #include "dvaas/test_vector.pb.h" #include "dvaas/validation_result.h" +#include "lib/p4rt/p4rt_port.h" #include "p4_pdpi/p4_runtime_session.h" namespace dvaas { @@ -54,11 +57,18 @@ struct ArribaTestVectorValidationParams { DefaultIsExpectedUnsolicitedPacket; }; +// Retrieves the set of P4RT ports used in the given `arriba_test_vector` (table +// entries and test packet). +absl::StatusOr> GetUsedP4rtPortIds( + const ArribaTestVector& arriba_test_vector, + const std::vector& used_entries_list, + const pdpi::IrP4Info& ir_p4_info); + // Validates the `sut` in the provided mirror testbed (`sut` and // `control_switch`) against the given `arriba_test_vector`. It does so by // installing the entries in the test vector (on SUT), injecting the input -// packets, collecting the output packets, and comparing the results with the -// expected outputs for each input packet. +// packets, collecting the output packets, and comparing the results with +// the expected outputs for each input packet. // // Pre-condition: // - The same P4Info and gNMI configs used in the generation of the given diff --git a/dvaas/dataplane_validation.cc b/dvaas/dataplane_validation.cc index 7b87b5f31..4c2a836b7 100644 --- a/dvaas/dataplane_validation.cc +++ b/dvaas/dataplane_validation.cc @@ -14,6 +14,7 @@ #include "dvaas/dataplane_validation.h" +#include #include #include #include @@ -30,6 +31,7 @@ #include "absl/strings/str_join.h" #include "absl/strings/str_replace.h" #include "absl/strings/string_view.h" +#include "absl/strings/strip.h" #include "absl/time/clock.h" #include "absl/time/time.h" #include "dvaas/packet_injection.h" @@ -58,6 +60,7 @@ #include "p4_pdpi/p4_runtime_session_extras.h" #include "p4_pdpi/packetlib/packetlib.pb.h" #include "p4_pdpi/sequencing.h" +#include "p4_pdpi/string_encodings/hex_string.h" #include "p4_symbolic/packet_synthesizer/packet_synthesizer.pb.h" #include "proto/gnmi/gnmi.pb.h" #include "sai_p4/instantiations/google/versions.h" @@ -426,7 +429,8 @@ absl::StatusOr GenerateTestVectors( backend.SynthesizePackets( ir_p4info, entities, p4_spec.p4_symbolic_config, ports, [&](absl::string_view stats) { - return writer.AppendToTestArtifact("test_packet_stats.txt", stats); + return writer.AppendToTestArtifact( + "auto_generated_test_packet_stats.txt", stats); }, params.coverage_goals_override, params.packet_synthesis_time_limit)); @@ -452,26 +456,63 @@ absl::StatusOr GenerateTestVectors( return generate_test_vectors_result; } +// Returns the packet hex as it appears in the Bmv2 textual log for the given +// switch input. +absl::StatusOr GetBmv2PacketHex( + const dvaas::SwitchInput& switch_input) { + switch (switch_input.type()) { + case SwitchInput::SUBMIT_TO_INGRESS: { + // Test vector prediction (test_vector_prediction.cc) adds this header to + // submit to ingress packets, so the packet hex in the BMv2 log + // will include this header. + // TODO: Clean this up to use a single source of truth once + // we have a better way to handle submit-to-ingress test vectors. + constexpr auto kSubmitToIngressSaiP4PacketOutHeader = + std::bitset<16>(0b0000'0000'0100'0000); + return absl::StrCat( + absl::StripPrefix( + pdpi::BitsetToHexString(kSubmitToIngressSaiP4PacketOutHeader), + "0x"), + absl::StripPrefix(switch_input.packet().hex(), "0x")); + } + case SwitchInput::PACKET_OUT: { + // TODO: Support PACKET_OUT (submit to egress) test vectors. + return absl::UnimplementedError( + "PACKET_OUT (submit to egress) test vectors are not supported."); + } + default: + return switch_input.packet().hex(); + } +} + absl::Status AttachPacketTrace( dvaas::PacketTestOutcome& failed_packet_test, absl::btree_map>& packet_traces, gutil::TestArtifactWriter& dvaas_test_artifact_writer) { // Store the full BMv2 textual log as test artifact. - const std::string& packet_hex = + const absl::string_view packet_hex = failed_packet_test.test_run().test_vector().input().packet().hex(); ASSIGN_OR_RETURN(int test_id, dvaas::ExtractIdFromTaggedPacketInHex(packet_hex)); + ASSIGN_OR_RETURN( + std::string bmv2_packet_hex, + GetBmv2PacketHex(failed_packet_test.test_run().test_vector().input())); + + if (failed_packet_test.test_run().test_vector().input().type() == + SwitchInput::PACKET_OUT) { + return absl::OkStatus(); + } + const std::string filename = "packet_" + std::to_string(test_id) + ".trace.txt"; - RETURN_IF_ERROR(dvaas_test_artifact_writer.AppendToTestArtifact( - filename, packet_traces[packet_hex][0].bmv2_textual_log())); - - auto it = packet_traces.find(packet_hex); + auto it = packet_traces.find(bmv2_packet_hex); if (it == packet_traces.end() || it->second.empty()) { return absl::InternalError( - absl::StrCat("Packet trace not found for packet ", packet_hex)); + absl::StrCat("Packet trace not found for packet ", bmv2_packet_hex)); } + RETURN_IF_ERROR(dvaas_test_artifact_writer.AppendToTestArtifact( + filename, packet_traces[bmv2_packet_hex][0].bmv2_textual_log())); // Augment failure description with packet trace summary. ASSIGN_OR_RETURN(std::string summarized_packet_trace, @@ -508,7 +549,9 @@ absl::Status StorePacketTestVectorAsArribaTestVector( // Restrict the ArribaTestVector's table entries to the ones hit by the packet // test vector according to the P4 simulation. - for (const auto &packet_trace : packet_traces.at(packet_hex)) { + ASSIGN_OR_RETURN(std::string bmv2_packet_hex, + GetBmv2PacketHex(packet_test_vector.input())); + for (const auto& packet_trace : packet_traces.at(bmv2_packet_hex)) { // In case of non-deterministic output (e.g. WCMP), find the union of all // entries that could be hit. for (const dvaas::Event &event : packet_trace.events()) { @@ -911,6 +954,9 @@ DataplaneValidator::ValidateDataplaneUsingExistingSwitchApis( } } + RETURN_IF_ERROR(dvaas_test_artifact_writer.AppendToTestArtifact( + "packet_test_outcomes.txtpb", gutil::PrintTextProto(test_outcomes))); + ValidationResult validation_result( std::move(test_outcomes), generate_test_vectors_result.packet_synthesis_result); diff --git a/dvaas/dataplane_validation_test.cc b/dvaas/dataplane_validation_test.cc index 9d7881bb0..e08f52199 100644 --- a/dvaas/dataplane_validation_test.cc +++ b/dvaas/dataplane_validation_test.cc @@ -5,6 +5,7 @@ #include #include "absl/container/btree_map.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/strings/string_view.h" #include "dvaas/packet_trace.pb.h" @@ -93,6 +94,117 @@ TEST(AttachPacketTraceTest, IsOk) { IsOk()); } +TEST(AttachPacketTraceTest, SubmitToIngressTestVectorIsHandledProperly) { + PacketTestOutcome failed_packet_test; + *failed_packet_test.mutable_test_result()->mutable_failure() = + gutil::ParseProtoOrDie(R"pb( + description: "Test failed" + )pb"); + *failed_packet_test.mutable_test_run() + ->mutable_test_vector() = gutil::ParseProtoOrDie(R"pb( + input { + type: SUBMIT_TO_INGRESS + packet { + port: "1" + parsed { + headers { + ethernet_header { + ethernet_destination: "02:da:c4:a7:b1:35" + ethernet_source: "02:88:e4:58:4b:92" + ethertype: "0x86dd" + } + } + headers { + ipv6_header { + version: "0x6" + dscp: "0x1b" + ecn: "0x1" + flow_label: "0x12345" + payload_length: "0x004f" + next_header: "0xfd" + hop_limit: "0xf2" + ipv6_source: "2002:ad12:4100:3::" + ipv6_destination: "2002:ad12:4100:1::" + } + } + payload: "test packet #2: SUBMIT_TO_INGRESS IPv6 unicast packet expected to get forwarded" + } + hex: "02dac4a7b1350288e4584b9286dd66d12345004ffdf22002ad124100000300000000000000002002ad1241000001000000000000000074657374207061636b65742023323a205355424d49545f544f5f494e4752455353204950763620756e6963617374207061636b657420657870656374656420746f2067657420666f72776172646564" + } + } + )pb"); + std::string packet_hex = + "004002dac4a7b1350288e4584b9286dd66d12345004ffdf22002ad124100000300000000" + "000000002002ad1241000001000000000000000074657374207061636b65742023323a20" + "5355424d49545f544f5f494e4752455353204950763620756e6963617374207061636b65" + "7420657870656374656420746f2067657420666f72776172646564"; + dvaas::PacketTrace packet_trace = gutil::ParseProtoOrDie(R"pb( + bmv2_textual_log: "BMv2 textual log" + events { packet_replication { number_of_packets_replicated: 1 } } + )pb"); + absl::btree_map> packet_traces; + packet_traces[packet_hex] = {packet_trace}; + DummyArtifactWriter dvaas_test_artifact_writer; + + EXPECT_THAT(AttachPacketTrace(failed_packet_test, packet_traces, + dvaas_test_artifact_writer), + IsOk()); +} + +TEST(AttachPacketTraceTest, SubmitToEgressTestVectorIsHandledProperly) { + PacketTestOutcome failed_packet_test; + *failed_packet_test.mutable_test_result()->mutable_failure() = + gutil::ParseProtoOrDie(R"pb( + description: "Test failed" + )pb"); + *failed_packet_test.mutable_test_run() + ->mutable_test_vector() = gutil::ParseProtoOrDie(R"pb( + input { + type: PACKET_OUT + packet { + port: "1" + parsed { + headers { + ethernet_header { + ethernet_destination: "02:da:c4:a7:b1:35" + ethernet_source: "02:88:e4:58:4b:92" + ethertype: "0x86dd" + } + } + headers { + ipv6_header { + version: "0x6" + dscp: "0x1b" + ecn: "0x1" + flow_label: "0x12345" + payload_length: "0x004f" + next_header: "0xfd" + hop_limit: "0xf2" + ipv6_source: "2002:ad12:4100:3::" + ipv6_destination: "2002:ad12:4100:1::" + } + } + payload: "test packet #2: SUBMIT_TO_INGRESS IPv6 unicast packet expected to get forwarded" + } + hex: "02dac4a7b1350288e4584b9286dd66d12345004ffdf22002ad124100000300000000000000002002ad1241000001000000000000000074657374207061636b65742023323a205355424d49545f544f5f494e4752455353204950763620756e6963617374207061636b657420657870656374656420746f2067657420666f72776172646564" + } + } + )pb"); + dvaas::PacketTrace packet_trace = gutil::ParseProtoOrDie(R"pb( + bmv2_textual_log: "BMv2 textual log" + events { packet_replication { number_of_packets_replicated: 1 } } + )pb"); + absl::btree_map> packet_traces; + packet_traces + [failed_packet_test.test_run().test_vector().input().packet().hex()] = { + packet_trace}; + DummyArtifactWriter dvaas_test_artifact_writer; + + EXPECT_THAT(AttachPacketTrace(failed_packet_test, packet_traces, + dvaas_test_artifact_writer), + gutil::StatusIs(absl::StatusCode::kUnimplemented)); +} + TEST(GetPacketTraceSummaryTest, GetPacketTraceSummaryGoldenTest) { dvaas::PacketTrace packet_trace = gutil::ParseProtoOrDie(R"pb( bmv2_textual_log: "BMv2 textual log" diff --git a/dvaas/dvaas_detective.cc b/dvaas/dvaas_detective.cc new file mode 100644 index 000000000..462e981ab --- /dev/null +++ b/dvaas/dvaas_detective.cc @@ -0,0 +1,496 @@ +// Copyright 2025 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 +// +// http://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 "dvaas/dvaas_detective.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "absl/container/flat_hash_map.h" +#include "absl/memory/memory.h" +#include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" +#include "absl/strings/string_view.h" +#include "absl/strings/substitute.h" +#include "absl/types/variant.h" +#include "dvaas/dvaas_detective.pb.h" +#include "dvaas/test_vector.pb.h" +#include "gutil/overload.h" +#include "gutil/proto.h" +#include "gutil/status.h" +#include "p4_pdpi/packetlib/packetlib.pb.h" +#include "yggdrasil_decision_forests/dataset/data_spec.pb.h" +#include "yggdrasil_decision_forests/dataset/data_spec_inference.h" +#include "yggdrasil_decision_forests/learner/abstract_learner.h" +#include "yggdrasil_decision_forests/learner/cart/cart.h" // IWYU pragma: keep +#include "yggdrasil_decision_forests/learner/cart/cart.pb.h" +#include "yggdrasil_decision_forests/learner/learner_library.h" +#include "yggdrasil_decision_forests/model/abstract_model.h" +#include "yggdrasil_decision_forests/model/decision_tree/decision_tree.h" +#include "yggdrasil_decision_forests/model/decision_tree/decision_tree.pb.h" +#include "yggdrasil_decision_forests/model/random_forest/random_forest.h" +#include "yggdrasil_decision_forests/utils/cast.h" +#include "yggdrasil_decision_forests/utils/distribution.pb.h" + +namespace dvaas { +namespace dvaas_internal { + +namespace ydf = ::yggdrasil_decision_forests; +using ydf::model::random_forest::RandomForestModel; + +namespace { + +using ydf::dataset::proto::DataSpecification; + +// In order to train our model, we must specify a label. For the most part, a +// label is just another categorical feature, except the model classifies data +// based on that feature. For example: +// - Categorical feature F can have value X, Y, or Z +// - Feature F is designated as the label +// - Therefore, the model classifies data as either X, Y, or Z +// Our code mostly treats the label like any other feature, but with a fixed +// feature name and values defined by the constants below. +constexpr absl::string_view kTestResultFeatureName = "test result"; +constexpr absl::string_view kPassTestResultFeatureValue = "pass"; +constexpr absl::string_view kFailTestResultFeatureValue = "fail"; + +// Feature names used by DVaaS Detective. +constexpr absl::string_view kExpectedOutputPacketsFeatureName = + "# expected output packets"; +constexpr absl::string_view kExpectedPuntedPacketsFeatureName = + "# expected punted packets"; +constexpr absl::string_view kAcceptableBehaviorsFeatureName = + "# acceptable switch behaviors according to P4 simulation"; + +std::string DetectiveClusterToString(const DetectiveCluster& cluster, + float total_predicted_outcomes) { + bool passed = cluster.predicted_outcome_is_pass(); + return absl::StrFormat( + "- %s -> %s\n" + " * accuracy: %.0f%%, %.0f out of %.0f test vectors that match the " + "conditions %s (remaining %.0f %s instead)\n" + " * coverage: %.0f%%, accounting for %.0f out of %.0f %s test vectors\n", + cluster.defining_property().empty() ? "" + : cluster.defining_property(), + passed ? "pass" : "fail", cluster.accuracy_of_predicted_outcome() * 100, + passed ? cluster.passing_tests() : cluster.failing_tests(), + cluster.passing_tests() + cluster.failing_tests(), + passed ? "pass" : "fail", + passed ? cluster.failing_tests() : cluster.passing_tests(), + passed ? "fail" : "pass", cluster.coverage_for_predicted_outcome() * 100, + passed ? cluster.passing_tests() : cluster.failing_tests(), + total_predicted_outcomes, passed ? "passing" : "failing"); +} + +// Return a `DetectiveCluster` created from leaf `node`: +// - `criteria` is list of branching decisions made at internal nodes (e.g. +// {"var X >= 12", "var Y < 2"}) corresponding to the path from root to +// `node`. +// - `data_spec` contains information for interpreting `node`. +absl::StatusOr MakeCluster( + const std::vector& criteria, + const ydf::model::decision_tree::proto::Node& node, + const DataSpecification& data_spec) { + DetectiveCluster cluster; + cluster.set_defining_property(absl::StrJoin(criteria, " && ")); + + // `node` contains a distribution over the possible outcomes (i.e. pass or + // fail). We need to know the index of the pass and fail outcomes in the + // distribution in order to interpret the outcome of `node`. It's possible + // that the distribution does not contain a pass or fail outcome, in which + // case we set the corresponding index to nullopt. + std::optional pass_index; + std::optional fail_index; + for (const ydf::dataset::proto::Column& column : data_spec.columns()) { + if (column.name() != kTestResultFeatureName) continue; + + for (const auto& [name, info] : column.categorical().items()) { + if (name == kPassTestResultFeatureValue) { + pass_index = info.index(); + } + if (name == kFailTestResultFeatureValue) { + fail_index = info.index(); + } + } + } + if (!pass_index.has_value() && !fail_index.has_value()) { + return absl::InternalError( + absl::StrCat("Node has neither passing nor failing tests: ", + gutil::PrintTextProto(node))); + } + + // Outcome is the output of the model (i.e. for data X, we predict outcome Y). + // For a `node`, the outcome is the top value in the `node`'s distribution. + // We expect the outcome to be either pass or fail. + int outcome_value_index = node.classifier().top_value(); + if (pass_index.has_value() && outcome_value_index == *pass_index) { + cluster.set_predicted_outcome_is_pass(true); + } else if (fail_index.has_value() && outcome_value_index == *fail_index) { + cluster.set_predicted_outcome_is_pass(false); + } else { + return absl::InternalError(absl::StrCat("Node has unexpected outcome: ", + gutil::PrintTextProto(node))); + } + cluster.set_passing_tests( + pass_index.has_value() + ? node.classifier().distribution().counts(*pass_index) + : 0); + cluster.set_failing_tests( + fail_index.has_value() + ? node.classifier().distribution().counts(*fail_index) + : 0); + cluster.set_accuracy_of_predicted_outcome( + node.classifier().distribution().counts(outcome_value_index) / + node.classifier().distribution().sum()); + return cluster; +} + +// Recursively extract `DetectiveCluster`s from `node`. +// - `data_spec` contains information about the features used in the model. +// - `criteria` is used to keep track of the path from the root to `node`. +// - `clusters` are appended when leaf nodes are reached. +absl::Status ExtractClustersRecursivey( + const ydf::model::decision_tree::NodeWithChildren& node, + const DataSpecification& data_spec, std::vector criteria, + std::vector& clusters) { + using ydf::model::decision_tree::proto::Condition; + using ydf::model::decision_tree::proto::NodeCondition; + + if (node.IsLeaf()) { + ASSIGN_OR_RETURN(clusters.emplace_back(), + MakeCluster(criteria, node.node(), data_spec)); + return absl::OkStatus(); + } + + // Create criteria for branches of this node. + NodeCondition condition = node.node().condition(); + const absl::string_view attribute_name = + data_spec.columns(node.node().condition().attribute()).name(); + std::string pos_criteria; + std::string neg_criteria; + switch (condition.condition().type_case()) { + // Condition for numerical features. + case Condition::kHigherCondition: { + float threshold = condition.condition().higher_condition().threshold(); + pos_criteria = absl::StrCat(attribute_name, " >= ", threshold); + neg_criteria = absl::StrCat(attribute_name, " < ", threshold); + break; + } + // Condition for categorical features. + case Condition::kContainsCondition: + // Condition for boolean features. + case Condition::kTrueValueCondition: + // Other conditions we are not currently interested in. + case Condition::kNaCondition: + case Condition::kContainsBitmapCondition: + case Condition::kDiscretizedHigherCondition: + case Condition::kObliqueCondition: + default: + return absl::InternalError(absl::StrCat( + "Unsupported condition type: ", gutil::PrintTextProto(condition))); + }; + + // Positive branch. + criteria.push_back(pos_criteria); + RETURN_IF_ERROR(ExtractClustersRecursivey(*node.pos_child(), data_spec, + criteria, clusters)); + criteria.pop_back(); + + // Negative branch. + criteria.push_back(neg_criteria); + return ExtractClustersRecursivey(*node.neg_child(), data_spec, criteria, + clusters); +} + +// Returns a `RandomForestModel` trained on various features extracted +// from`test_outcomes` to predict the validation result of test outcome +// (pass/fail). +absl::StatusOr> BuildModel( + const PacketTestOutcomes& test_outcomes) { + using ydf::model::AbstractLearner; + using yggdrasil_decision_forests::utils::down_cast; + + // Create temporary CSV file from `test_outcomes`. + ASSIGN_OR_RETURN(std::string temp_csv_path, + WriteTempCsvFileFromPacketTestOutcomes(test_outcomes)); + // TODO: b/383131744 - Write the file to test artifacts, look at bmv2 wrapper + // for reference. + + // Create data specification from the CSV file. + ydf::dataset::proto::DataSpecificationGuide guide; + // Don't require categorical features to have a minimum frequency. A single + // error is still valuable information. + guide.mutable_default_column_guide() + ->mutable_categorial() + ->set_min_vocab_frequency(0); + // Currently, we have numerical features whose value set may be {0, 1}. By + // default, ydf treats features with this set as BOOLEAN, but since we only + // support NUMERICAL features, we override this behavior. + // TODO: Update once we support BOOLEAN features. + guide.set_detect_boolean_as_numerical(true); + std::string dataset_path = absl::StrCat("csv:", temp_csv_path); + ASSIGN_OR_RETURN(DataSpecification data_spec, + ydf::dataset::CreateDataSpec(dataset_path, guide)); + + // Configure and create ydf learner. + constexpr int kMaxTreeDepth = 4; + constexpr int kMinNumberOfDataExamplesPerNode = 1; + ydf::model::proto::TrainingConfig training_config; + // We use a CART learner because it produces a single tree and considers all + // features when splitting nodes. + // https://ydf.readthedocs.io/en/stable/py_api/CartLearner/ + training_config.set_learner("CART"); + training_config.set_task(ydf::model::proto::Task::CLASSIFICATION); + // We want to group similar test outcomes together as passing or failing. + training_config.set_label(kTestResultFeatureName); + ydf::model::cart::proto::CartTrainingConfig& cart_config = + *training_config.MutableExtension(ydf::model::cart::proto::cart_config); + cart_config.mutable_decision_tree()->set_max_depth(kMaxTreeDepth); + cart_config.mutable_decision_tree()->set_min_examples( + kMinNumberOfDataExamplesPerNode); + // Ensures no data is excluded from training. + cart_config.set_validation_ratio(0); + ASSIGN_OR_RETURN(std::unique_ptr learner, + ydf::model::GetLearner(training_config)); + + // Train model + // NOTE: All data is used for both training and validation/pruning. This is + // typically not recommended due to overfitting and loss of generality. + // However, in our case, overfitting is exactly what we want. + ASSIGN_OR_RETURN( + std::unique_ptr model, + learner->TrainWithStatus(/*train_dataset=*/dataset_path, data_spec, + /*validation_dataset=*/dataset_path)); + + // Downcast is safe because we use a CART learner which produces a + // `RandomForestModel`. + return absl::WrapUnique( + down_cast(model.release())); +}; + +} // namespace + +std::string FeatureValueToString(const FeatureValue& value) { + return absl::visit( + gutil::Overload{ + [](const NumericalValue& num) { return absl::StrCat(num); }, + [](const CategoricalValue& str) { return str; }}, + value); +} + +std::vector GetListOfFeatureNames() { + return { + kExpectedOutputPacketsFeatureName, + kExpectedPuntedPacketsFeatureName, + kAcceptableBehaviorsFeatureName, + kTestResultFeatureName, + }; +} + +absl::flat_hash_map TestOutcomeToFeatureMap( + const PacketTestOutcome& test_outcome) { + absl::flat_hash_map result; + + // Feature extraction is scoped to ensure the extraction of one set of + // features is independent of the extraction of another set. If this function + // becomes too large, scoped blocks should be refactored into functions. + { + int num_expected_output_packets = 0; + int num_expected_packet_ins = 0; + for (const auto& output : + test_outcome.test_run().test_vector().acceptable_outputs()) { + num_expected_output_packets = + std::max(num_expected_output_packets, output.packets_size()); + num_expected_packet_ins = + std::max(num_expected_packet_ins, output.packet_ins_size()); + } + result[kExpectedOutputPacketsFeatureName] = + static_cast(num_expected_output_packets); + result[kExpectedPuntedPacketsFeatureName] = + static_cast(num_expected_packet_ins); + } + + result[kAcceptableBehaviorsFeatureName] = static_cast( + test_outcome.test_run().test_vector().acceptable_outputs_size()); + result[kTestResultFeatureName] = + test_outcome.test_result().has_failure() + ? std::string(kFailTestResultFeatureValue) + : std::string(kPassTestResultFeatureValue); + + return result; +} + +absl::StatusOr WriteTempCsvFileFromPacketTestOutcomes( + const PacketTestOutcomes& test_outcomes) { + // Create CSV rows. + std::vector ordered_feature_names = + GetListOfFeatureNames(); + std::vector csv_rows; + csv_rows.reserve(1 + test_outcomes.outcomes_size()); + csv_rows.push_back(absl::StrJoin(ordered_feature_names, ",")); + for (const auto& test_outcome : test_outcomes.outcomes()) { + absl::flat_hash_map feature_map = + TestOutcomeToFeatureMap(test_outcome); + std::vector ordered_feature_values; + ordered_feature_values.reserve(ordered_feature_names.size()); + for (const auto& feature_name : ordered_feature_names) { + ordered_feature_values.push_back( + FeatureValueToString(feature_map[feature_name])); + } + csv_rows.push_back(absl::StrJoin(ordered_feature_values, ",")); + } + + // Write CSV file. + char* tmp_file = std::tmpnam(nullptr); + if (tmp_file == nullptr) { + return absl::InternalError("Failed to create temporary file."); + } + std::ofstream(tmp_file) << absl::StrJoin(csv_rows, "\n"); + return tmp_file; +} + +absl::StatusOr ExtractExplanationFromModel( + const RandomForestModel& rf_model) { + if (rf_model.NumTrees() != 1) { + return absl::UnimplementedError( + absl::StrCat("We only support explanation extraction for models with " + "exactly one tree. Model has ", + rf_model.NumTrees(), " trees.")); + } + + // Extract clusters from the decision tree. + std::vector clusters; + RETURN_IF_ERROR(ExtractClustersRecursivey( + rf_model.decision_trees().at(0)->root(), rf_model.data_spec(), + /*criteria=*/{}, clusters)); + + // Initialize explanation and add coverage information. + DetectiveExplanation explanation; + *explanation.mutable_clusters() = {clusters.begin(), clusters.end()}; + float total_passing = 0; + float total_failing = 0; + for (const auto& [name, info] : + rf_model.LabelColumnSpec().categorical().items()) { + if (name == kPassTestResultFeatureValue) { + total_passing = info.count(); + } + if (name == kFailTestResultFeatureValue) { + total_failing = info.count(); + } + } + for (DetectiveCluster& cluster : *explanation.mutable_clusters()) { + if (cluster.predicted_outcome_is_pass() && total_passing != 0) { + cluster.set_coverage_for_predicted_outcome(cluster.passing_tests() / + total_passing); + } + if (!cluster.predicted_outcome_is_pass() && total_failing != 0) { + cluster.set_coverage_for_predicted_outcome(cluster.failing_tests() / + total_failing); + } + } + return explanation; +} + +absl::StatusOr CreateDetectiveExplanation( + const PacketTestOutcomes& test_outcomes) { + if (test_outcomes.outcomes().empty()) { + return absl::InvalidArgumentError("No PacketTestOutcomes provided."); + } + ASSIGN_OR_RETURN(std::unique_ptr model, + dvaas_internal::BuildModel(test_outcomes)); + return dvaas_internal::ExtractExplanationFromModel(*model); +} + +std::string DetectiveExplanationToString( + const DetectiveExplanation& explanation) { + std::vector passing_indices; + std::vector failing_indices; + float total_passing_outcomes = 0; + float total_failing_outcomes = 0; + for (int i = 0; i < explanation.clusters_size(); ++i) { + const DetectiveCluster& cluster = explanation.clusters(i); + if (explanation.clusters(i).predicted_outcome_is_pass()) { + passing_indices.push_back(i); + } else { + failing_indices.push_back(i); + } + total_passing_outcomes += cluster.passing_tests(); + total_failing_outcomes += cluster.failing_tests(); + } + + std::string result; + // Reserve string size with a rough estimate to avoid unnecessary copying + // during append operations. + result.reserve(250 * explanation.clusters_size()); + + float unaccounted_passing_outcomes = total_passing_outcomes; + float unaccounted_passing_coverage = 1.0; + result.append( + "DVaaS Detective: Found the following pattern(s) among passing test " + "vectors:\n"); + for (auto passing_index : passing_indices) { + const DetectiveCluster& cluster = explanation.clusters(passing_index); + result.append(DetectiveClusterToString(explanation.clusters(passing_index), + total_passing_outcomes)); + unaccounted_passing_outcomes -= cluster.passing_tests(); + unaccounted_passing_coverage -= cluster.coverage_for_predicted_outcome(); + } + result.append(unaccounted_passing_coverage == 0 + ? "- All passing test vectors accounted for\n" + : absl::Substitute( + "- $0 passing test vectors unaccounted for ($1%)\n", + unaccounted_passing_outcomes, + unaccounted_passing_coverage * 100)); + + float unaccounted_failing_outcomes = total_failing_outcomes; + float unaccounted_failing_coverage = 1.0; + result.append( + "\nDVaaS Detective: Found the following pattern(s) among failing test " + "vectors:\n"); + for (auto failing_index : failing_indices) { + const DetectiveCluster& cluster = explanation.clusters(failing_index); + result.append(DetectiveClusterToString(explanation.clusters(failing_index), + total_failing_outcomes)); + unaccounted_failing_outcomes -= cluster.failing_tests(); + unaccounted_failing_coverage -= cluster.coverage_for_predicted_outcome(); + } + result.append(unaccounted_failing_coverage == 0 + ? "- All failing test vectors accounted for\n" + : absl::Substitute( + "- $0 failing test vectors unaccounted for ($1%)\n", + unaccounted_failing_outcomes, + unaccounted_failing_coverage * 100)); + + return result; +} + +} // namespace dvaas_internal + +absl::StatusOr ExplainTestOutcomes( + const PacketTestOutcomes& test_outcomes) { + ASSIGN_OR_RETURN(DetectiveExplanation explanation, + dvaas_internal::CreateDetectiveExplanation(test_outcomes)); + return dvaas_internal::DetectiveExplanationToString(explanation); +} + +} // namespace dvaas diff --git a/dvaas/dvaas_detective.h b/dvaas/dvaas_detective.h new file mode 100644 index 000000000..8b33ff393 --- /dev/null +++ b/dvaas/dvaas_detective.h @@ -0,0 +1,100 @@ +// A library for explaining patterns in `PacketTestOutcomes` in a human-friendly +// way (see go/dvaas-detective). + +// Copyright 2025 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 +// +// http://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_DVAAS_DVAAS_DETECTIVE_H_ +#define PINS_DVAAS_DVAAS_DETECTIVE_H_ + +#include +#include +#include + +#include "absl/container/flat_hash_map.h" +#include "absl/status/statusor.h" +#include "absl/strings/string_view.h" +#include "dvaas/dvaas_detective.pb.h" +#include "dvaas/test_vector.pb.h" +#include "yggdrasil_decision_forests/model/random_forest/random_forest.h" + +namespace dvaas { + +// Provides a human-readable explanation of the broken and working forwarding +// functionality based on the given 'test_outcomes.' This is achieved by +// identifying high-level patterns/clusters among failing and passing test +// vectors that explain the data. +// +// The explanation is tailored towards network engineers who may not be familiar +// with the specifics of DVaaS. +// +// See go/dvaas-detective for more details. +absl::StatusOr ExplainTestOutcomes( + const PacketTestOutcomes& test_outcomes); + +// == END OF PUBLIC API - implementation details below ========================= +// == EXPOSED FOR TESTING ONLY ================================================= + +namespace dvaas_internal { + +// Creates an `Explanation` for `test_outcomes`. +absl::StatusOr CreateDetectiveExplanation( + const PacketTestOutcomes& test_outcomes); + +// Pretty printer for `DetectiveExplanation`. +std::string DetectiveExplanationToString( + const DetectiveExplanation& explanation); + +// Creates an `Explanation` from `rf_model`. +// - `model` must have exactly one tree. +// - `model` must have been created using the ydf library. +absl::StatusOr ExtractExplanationFromModel( + const yggdrasil_decision_forests::model::random_forest::RandomForestModel& + model); + +// Converts `test_outcomes` to CSV data and writes it to a temporary file. +// Returns path of temporary file on success. Returns error if temporary file +// cannot be created. +// - CSV data is created using `TestOutcomeToFeatureMap`. +absl::StatusOr WriteTempCsvFileFromPacketTestOutcomes( + const PacketTestOutcomes& test_outcomes); + +// Semantics for numerical features: +// https://ydf.readthedocs.io/en/latest/guide_feature_semantics/#ydfsemanticnumerical +using NumericalValue = float; + +// Semantics for categorical features: +// https://ydf.readthedocs.io/en/latest/guide_feature_semantics/#ydfsemanticcategorical +using CategoricalValue = std::string; + +// Feature values can have different types, each having it's own semantics. +// https://ydf.readthedocs.io/en/latest/guide_feature_semantics +using FeatureValue = std::variant; + +// Returns `value` as a printable string. +std::string FeatureValueToString(const FeatureValue& value); + +// Returns list of feature names used by DVaaS Detective. +std::vector GetListOfFeatureNames(); + +// Returns map from feature names to feature values extracted from +// `test_outcome`. List of feature names can be obtained using +// `GetListOfFeatureNames`. +absl::flat_hash_map TestOutcomeToFeatureMap( + const PacketTestOutcome& test_outcome); + +} // namespace dvaas_internal +} // namespace dvaas + +#endif // PINS_DVAAS_DVAAS_DETECTIVE_H_ diff --git a/dvaas/dvaas_detective.proto b/dvaas/dvaas_detective.proto new file mode 100644 index 000000000..d98ff49af --- /dev/null +++ b/dvaas/dvaas_detective.proto @@ -0,0 +1,63 @@ +// Copyright 2025 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. + +// Defines the proto messages used by DVaaS Detective (go/dvaas-detective). + +syntax = "proto3"; + +package dvaas; + +// Dvaas Detective attempts to explain a set of `PacketTestOutcomes` in a +// human-friendly way by clustering them. +message DetectiveExplanation { + // The set of `DetectiveCluster`s that partition `PacketTestOutcomes`. + repeated DetectiveCluster clusters = 1; +} + +// A grouping of `PacketTestOutcomes` produced by DVaaS Detective. +message DetectiveCluster { + // A human-readable defining property that determines group membership. In + // other words, a `PacketTestOutcome` is in this cluster IF AND ONLY IF it has + // this property (e.g. "input_packet_is_ipv4 == true && input_ttl == 2"). + string defining_property = 1; + + // Whether the predicted test outcome for this group is pass or fail. + bool predicted_outcome_is_pass = 2; + + // TODO: Replace counts with `PacketTestOutcomes`. + // Number of passing tests in this group. + double passing_tests = 3; + // Number of failing tests in this group. + double failing_tests = 4; + + // Accuracy of the predicted outcome for this group (i.e. the fraction of + // test outcomes in this group that match the predicted outcome). + double accuracy_of_predicted_outcome = 5; + + // Coverage is calculated with respect to all `PacketTestOutcomes` in a + // `DetectiveExplanation`. + // The fraction of all `PacketTestOutcomes` with the predicted outcome that + // are in this `DetectiveCluster`. + // For a passing prediction: + // coverage = passing_tests / all passing tests + // For a failing prediction: + // coverage = failing_tests / all failing tests + double coverage_for_predicted_outcome = 6; + + // NOTE: Accuracy and coverage are both important, but high values does not + // necessarily mean the explanation is good. For inuition, it's possible to + // achieve 100% accuracy/coverage with a restrictive/permissive defining + // property that provides no insight into acutal features beings tested. The + // selection of a defining property is key to a good explanation. +} diff --git a/dvaas/dvaas_detective_test.cc b/dvaas/dvaas_detective_test.cc new file mode 100644 index 000000000..72427697e --- /dev/null +++ b/dvaas/dvaas_detective_test.cc @@ -0,0 +1,681 @@ +// NOTE: This test is a golden test (go/golden-test-with-coverage), NOT a +// typical unit test. Although this test may contain ASSERT and EXPECT +// statements to check certain properties, such statements are not required. The +// primary purpose of this test is to print golden output to a golden file. Any +// changes to the golden output will require the golden file to be updated. +// Golden file changes will be reviewed during CL review. + +// Copyright 2025 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 +// +// http://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 "dvaas/dvaas_detective.h" + +#include +#include +#include +#include +#include +#include +#include + +#include "absl/container/flat_hash_map.h" +#include "absl/log/check.h" +#include "absl/status/status.h" +#include "dvaas/dvaas_detective.pb.h" +#include "dvaas/test_vector.pb.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "gutil/proto.h" +#include "gutil/status.h" +#include "gutil/status_matchers.h" +#include "gutil/testing.h" +#include "p4_pdpi/packetlib/packetlib.pb.h" +#include "yggdrasil_decision_forests/dataset/data_spec.pb.h" +#include "yggdrasil_decision_forests/model/decision_tree/decision_tree.h" +#include "yggdrasil_decision_forests/model/decision_tree/decision_tree.pb.h" +#include "yggdrasil_decision_forests/model/random_forest/random_forest.h" + +namespace dvaas { +namespace dvaas_internal { +namespace { + +namespace ydf = ::yggdrasil_decision_forests; + +// Returns a random forest model with the following single decision tree: +// ? expected(output packets) >= 1.5 # root non-leaf +// | +// +- T -> [fail : 0 passes, 5 fails] # multicast leaf +// | +// +- F -> ? [expected(packet-ins) >= 0.5] # unicast non-leaf +// | +// +- T -> [fail : 0 passes, 5 fails] # unicast w/ punt leaf +// | +// +- F -> [pass : 5 passes, 0 fails] # unicast w/o punt leaf +std::unique_ptr +CreateSkeletonRandomForestModel() { + using ydf::model::decision_tree::NodeWithChildren; + + std::unique_ptr rf_model = + std::make_unique(); + rf_model->set_data_spec( + gutil::ParseProtoOrDie(R"pb( + columns { type: NUMERICAL name: "expected(output packets)" } + columns { type: NUMERICAL name: "expected(packet-ins)" } + columns { + type: CATEGORICAL + name: "test result" + categorical { + items { + key: "" + value { index: 0 } + } + items { + key: "pass" + value { index: 1 count: 5 } + } + items { + key: "fail" + value { index: 2 count: 10 } + } + } + } + )pb")); + rf_model->set_label_col_idx(2); + + std::unique_ptr tree = + std::make_unique(); + { + tree->CreateRoot(); + NodeWithChildren* root = tree->mutable_root(); + *root->mutable_node() = + gutil::ParseProtoOrDie(R"pb( + condition { + attribute: 0 # expected(output packets) + condition { higher_condition { threshold: 1.5 } } + } + )pb"); + root->CreateChildren(); + NodeWithChildren* multicast_leaf_node = root->mutable_pos_child(); + *multicast_leaf_node->mutable_node() = + gutil::ParseProtoOrDie(R"pb( + classifier { + top_value: 2 # fail + distribution { + counts: 0 # + counts: 0 # passes + counts: 5 # fails + sum: 5 + } + } + )pb"); + NodeWithChildren* unicast_internal_node = root->mutable_neg_child(); + *unicast_internal_node->mutable_node() = + gutil::ParseProtoOrDie(R"pb( + condition { + attribute: 1 # expected(packet-ins) + condition { higher_condition { threshold: 0.5 } } + } + )pb"); + unicast_internal_node->CreateChildren(); + NodeWithChildren* unicast_without_punt_leaf_node = + unicast_internal_node->mutable_neg_child(); + *unicast_without_punt_leaf_node->mutable_node() = + gutil::ParseProtoOrDie(R"pb( + classifier { + top_value: 1 # pass + distribution { + counts: 0 # + counts: 5 # passes + counts: 0 # fails + sum: 5 + } + } + )pb"); + NodeWithChildren* unicast_with_punt_leaf_node = + unicast_internal_node->mutable_pos_child(); + *unicast_with_punt_leaf_node->mutable_node() = + gutil::ParseProtoOrDie(R"pb( + classifier { + top_value: 2 # fail + distribution { + counts: 0 # + counts: 0 # passes + counts: 5 # fails + sum: 5 + } + } + )pb"); + } + rf_model->AddTree(std::move(tree)); + return rf_model; +} + +void ExtractExplanationFromModelTest() { + // Print header. + std::cout << std::string(80, '=') << "\n" + << "ExtractExplanationFromModel Test\n" + << std::string(80, '=') << "\n"; + // Print input. + std::cout + << "-- Input: RandomForestWithSingleTree ----------------------------\n" + "? [expected(output packets) >= 1.5] # root non-leaf\n" + "|\n" + "+- T -> [fail : 0 passes, 5 fails] # multicast leaf\n" + "|\n" + "+- F -> ? [expected(packet-ins) >= 0.5] # unicast non-leaf\n" + " |\n" + " +- T -> [fail : 0 passes, 5 fails] # unicast w/ punt leaf\n" + " |\n" + " +- F -> [pass : 5 passes, 0 fails] # unicast w/o punt leaf\n"; + + std::unique_ptr rf_model = + CreateSkeletonRandomForestModel(); + ASSERT_OK_AND_ASSIGN(DetectiveExplanation explanation, + ExtractExplanationFromModel(*rf_model)); + std::cout << "-- Output: DetectiveExplanation proto ----------------------\n"; + std::cout << gutil::PrintTextProto(explanation); +} + +void DetectiveExplanationToStringTest() { + std::vector explanations = { + gutil::ParseProtoOrDie( + R"pb( + clusters { + defining_property: "input ttl < 2" + predicted_outcome_is_pass: true + passing_tests: 50 + failing_tests: 50 + accuracy_of_predicted_outcome: .5 + coverage_for_predicted_outcome: 1 + } + )pb"), + gutil::ParseProtoOrDie( + R"pb( + clusters { + defining_property: "input ttl < 2" + predicted_outcome_is_pass: false + passing_tests: 50 + failing_tests: 50 + accuracy_of_predicted_outcome: .5 + coverage_for_predicted_outcome: 1 + } + )pb"), + gutil::ParseProtoOrDie( + R"pb( + clusters { + defining_property: "input ttl < 2" + predicted_outcome_is_pass: true + passing_tests: 25 + failing_tests: 25 + accuracy_of_predicted_outcome: .5 + coverage_for_predicted_outcome: .5 + } + clusters { + defining_property: "input ttl >= 2" + predicted_outcome_is_pass: false + passing_tests: 25 + failing_tests: 25 + accuracy_of_predicted_outcome: .5 + coverage_for_predicted_outcome: .5 + } + )pb"), + gutil::ParseProtoOrDie( + R"pb( + clusters { + defining_property: "expected(output packets) < 2" + predicted_outcome_is_pass: true + passing_tests: 1220 + failing_tests: 0 + accuracy_of_predicted_outcome: 1 + coverage_for_predicted_outcome: .82 + } + clusters { + defining_property: "expected(packet-ins) < 1" + predicted_outcome_is_pass: true + passing_tests: 256 + failing_tests: 0 + accuracy_of_predicted_outcome: 1 + coverage_for_predicted_outcome: .17 + } + clusters { + defining_property: "expected(output packets) >= 2 && expected(packet-ins) >= 1" + predicted_outcome_is_pass: false + passing_tests: 12 + failing_tests: 30 + accuracy_of_predicted_outcome: .6 + coverage_for_predicted_outcome: 1 + } + )pb")}; + + for (const auto& explanation : explanations) { + // Print header. + std::cout << std::string(80, '=') << "\n" + << "DVaaS DetectiveExplanationToString Test\n" + << std::string(80, '=') << "\n"; + + // Print input. + std::cout << "-- Input: DetectiveExplanation proto " + "-----------------------------\n"; + std::cout << gutil::PrintTextProto(explanation); + + // Print output. + std::cout << "-- Output: DVAAS Detective String " + "--------------------------------\n"; + std::cout << DetectiveExplanationToString(explanation) << "\n"; + } +} + +PacketTestOutcomes TestOutcomeToFeatureMapTestCases() { + return gutil::ParseProtoOrDie(R"pb( + # Passing with 1 punt and 1 output packet + outcomes { + test_run { + test_vector { + input {} + acceptable_outputs { + packets {} + packet_ins {} + } + } + actual_output {} + } + test_result {} + } + # Failing multicast + outcomes { + test_run { + test_vector { + input {} + acceptable_outputs { + packets {} + packets {} + packets {} + packets {} + } + } + actual_output {} + } + test_result { failure {} } + } + # Pasing WCMP + outcomes { + test_run { + test_vector { + input {} + acceptable_outputs {} + acceptable_outputs {} + acceptable_outputs {} + acceptable_outputs {} + acceptable_outputs {} + } + actual_output {} + } + test_result {} + } + # Pasing WCMP with varying number packets and packet-ins. + outcomes { + test_run { + test_vector { + input {} + acceptable_outputs { + packets {} + packet_ins {} + packet_ins {} + } + acceptable_outputs { + packets {} + packets {} + packets {} + packet_ins {} + } + } + actual_output {} + } + test_result {} + } + )pb"); +} + +// Parameters for creating a faux `PacketTestOutcome`. +// - Every field corresponds to a feature returned by `TestOutcomeToFeatureMap` +struct PacketTestOutcomeParams { + // Number of expected output packets (max across alternavtives if + // `acceptable_outputs` > 1). + int num_expected_output_packets = 0; + // Number of expected packet-ins (max across alternatives if + // `acceptable_outputs` > 1). + int num_expected_packet_ins = 0; + // Number of acceptable outputs (should always be at least 1) + int num_acceptable_outputs = 1; + // Whether the test result is a pass or fail. + bool is_pass = true; +}; + +// Creates a faux `PacketTestOutcome` with the given `params`. +PacketTestOutcome MakeDummyOutcome(const PacketTestOutcomeParams& params) { + PacketTestOutcome outcome; + // There should always be at least one output. + CHECK_GT(params.num_acceptable_outputs, 0); + for (int i = 0; i < params.num_acceptable_outputs; ++i) { + outcome.mutable_test_run()->mutable_test_vector()->add_acceptable_outputs(); + } + dvaas::SwitchOutput* first_output = outcome.mutable_test_run() + ->mutable_test_vector() + ->mutable_acceptable_outputs(0); + for (int i = 0; i < params.num_expected_output_packets; ++i) { + first_output->add_packets(); + } + for (int i = 0; i < params.num_expected_packet_ins; ++i) { + first_output->add_packet_ins(); + } + if (!params.is_pass) { + outcome.mutable_test_result()->mutable_failure(); + } + + return outcome; +} + +PacketTestOutcomes MakeDummyOutcomes( + std::initializer_list params) { + PacketTestOutcomes outcomes; + for (const auto& param : params) { + *outcomes.add_outcomes() = MakeDummyOutcome(param); + } + return outcomes; +} + +// A test case for `DvaasDetectiveTest`. +struct ExplainTestOutcomesTestCase { + // Description of `test_outcomes`. + std::string description; + // PacketTestOutcomes explained by Dvaas Detective. + PacketTestOutcomes test_outcomes; +}; + +std::vector ExplainTestOutcomesTestCases() { + return { + { + .description = "No PacketTestOutcomes", + }, + { + .description = "1 passing unicast (i.e. All passing tests)", + .test_outcomes = MakeDummyOutcomes({ + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = true, + }, + }), + }, + { + .description = "1 failing unicast (i.e. All failing tests)", + .test_outcomes = MakeDummyOutcomes({ + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = false, + }, + }), + }, + { + .description = "1 passing unicast, 1 failing multicast", + .test_outcomes = MakeDummyOutcomes({ + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = true, + }, + PacketTestOutcomeParams{ + .num_expected_output_packets = 2, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = false, + }, + }), + }, + { + .description = "3 passing unicast, 1 failing unicast (i.e. Identical " + "tests pass more often than they fail)", + .test_outcomes = MakeDummyOutcomes({ + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = true, + }, + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = true, + }, + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = true, + }, + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = false, + }, + }), + }, + { + .description = "1 passing unicast, 3 failing unicast (i.e. Identical " + "tests fail more often than they pass)", + .test_outcomes = MakeDummyOutcomes({ + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = true, + }, + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = false, + }, + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = false, + }, + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = false, + }, + }), + }, + { + .description = "1 passing unicast, 1 failing unicast (i.e. Identical " + "tests 50/50 pass and fail)", + .test_outcomes = MakeDummyOutcomes({ + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = true, + }, + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = false, + }, + }), + }, + { + .description = + "1 passing unicast, 1 failing unicast+punt, 3 failing " + "multicast (i.e. Conjunction of features determines outcome)", + .test_outcomes = MakeDummyOutcomes({ + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = true, + }, + PacketTestOutcomeParams{ + .num_expected_output_packets = 1, + .num_expected_packet_ins = 1, + .num_acceptable_outputs = 1, + .is_pass = false, + }, + PacketTestOutcomeParams{ + .num_expected_output_packets = 2, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = false, + }, + PacketTestOutcomeParams{ + .num_expected_output_packets = 2, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = false, + }, + PacketTestOutcomeParams{ + .num_expected_output_packets = 2, + .num_expected_packet_ins = 0, + .num_acceptable_outputs = 1, + .is_pass = false, + }, + }), + }, + }; +} + +void TestOutcomeToFeatureMapTest(const PacketTestOutcome& test_outcome) { + // Print header. + std::cout << std::string(80, '=') << "\n" + << "TestOutcomeToFeatureMap Test\n" + << std::string(80, '=') << "\n"; + + // Print input. + std::cout + << "-- Input: PacketTestOutcome proto --------------------------------\n"; + std::cout << gutil::PrintTextProto(test_outcome); + + // Print output. + std::cout + << "-- Output: Feature Map -------------------------------------------\n"; + // Sort map for determinism. + absl::flat_hash_map feature_map = + TestOutcomeToFeatureMap(test_outcome); + // Check that GetListOfFeatureNames() is consistent with + // TestOutcomeToFeatureMap() and use GetListOfFeatureNames() to print in a + // deterministic order. + ASSERT_EQ(feature_map.size(), GetListOfFeatureNames().size()); + for (const auto& feature_name : GetListOfFeatureNames()) { + ASSERT_TRUE(feature_map.contains(feature_name)); + std::cout << feature_name << ": " + << FeatureValueToString(feature_map.at(feature_name)) << "\n"; + } + std::cout << "\n"; +} + +void ExplainTestOutcomesTest(const ExplainTestOutcomesTestCase& test) { + // Print header. + std::cout << std::string(80, '=') << "\n" + << "DVaaS Detective Explanation Test\n" + << std::string(80, '=') << "\n"; + + // Print input. + std::cout << "-- Input: PacketTestOutcomes (concise description) ---------\n"; + std::cout << test.description << "\n"; + + std::cout << "-- Output: Explanation Proto -------------------------------\n"; + absl::StatusOr explanation = + CreateDetectiveExplanation(test.test_outcomes); + if (!explanation.ok()) { + std::cout << gutil::StableStatusToString(explanation.status()) << "\n"; + return; + } + std::cout << gutil::PrintTextProto(*explanation); + + std::cout << "-- Output: Explanation Pretty Print ------------------------\n"; + std::cout << DetectiveExplanationToString(*explanation) << "\n"; +} + +void WriteCsvFileFromPacketTestOutcomesTest() { + PacketTestOutcomes test_outcomes = TestOutcomeToFeatureMapTestCases(); + // Create and write to temporary file + ASSERT_OK_AND_ASSIGN(std::string csv_file_path, + WriteTempCsvFileFromPacketTestOutcomes(test_outcomes)); + + // Print header. + std::cout << std::string(80, '=') << "\n" + << "WriteCsvFileFromPacketTestOutcomes Test\n" + << std::string(80, '=') << "\n"; + + // Print input. + std::cout + << "-- Input: PacketTestOutcomes proto -------------------------------\n"; + std::cout << gutil::PrintTextProto(test_outcomes); + + // Print output. + std::cout + << "-- Output: Contents of CSV file ----------------------------------\n"; + std::ifstream csv_ifstream(csv_file_path); + ASSERT_TRUE(csv_ifstream.is_open()); + std::string csv_row; + while (std::getline(csv_ifstream, csv_row)) { + std::cout << csv_row << "\n"; + } + csv_ifstream.close(); +} + +// This test is a golden test and not a typical unit test. The TEST unit serves +// as an entry point for calling functions that produce golden output. Different +// functions produce different portions of the golden output. All golden +// functions are called from a single TEST unit to avoid non-determinism in the +// output. +TEST(DvaasDetectiveTest, GoldenTest) { + PacketTestOutcomes test_outcomes = TestOutcomeToFeatureMapTestCases(); + for (const auto& test_outcome : test_outcomes.outcomes()) { + TestOutcomeToFeatureMapTest(test_outcome); + } + + WriteCsvFileFromPacketTestOutcomesTest(); + + ExtractExplanationFromModelTest(); + + DetectiveExplanationToStringTest(); + for (const auto& test : ExplainTestOutcomesTestCases()) { + ExplainTestOutcomesTest(test); + } +} + +// Support for multiple trees is not implemented. +TEST(DvaasDetectiveTest, DoesNotSupportModelsWithMultipleTrees) { + std::unique_ptr + rf_model_with_multiple_trees = CreateSkeletonRandomForestModel(); + rf_model_with_multiple_trees->AddTree( + std::make_unique()); + EXPECT_THAT(ExtractExplanationFromModel(*rf_model_with_multiple_trees), + gutil::StatusIs(absl::StatusCode::kUnimplemented)); +} + +} // namespace +} // namespace dvaas_internal +} // namespace dvaas diff --git a/dvaas/dvaas_detective_test.expected b/dvaas/dvaas_detective_test.expected new file mode 100644 index 000000000..00196de8c --- /dev/null +++ b/dvaas/dvaas_detective_test.expected @@ -0,0 +1,570 @@ +================================================================================ +TestOutcomeToFeatureMap Test +================================================================================ +-- Input: PacketTestOutcome proto -------------------------------- +test_run { + test_vector { + input { + } + acceptable_outputs { + packets { + } + packet_ins { + } + } + } + actual_output { + } +} +test_result { +} +-- Output: Feature Map ------------------------------------------- +# expected output packets: 1 +# expected punted packets: 1 +# acceptable switch behaviors according to P4 simulation: 1 +test result: pass + +================================================================================ +TestOutcomeToFeatureMap Test +================================================================================ +-- Input: PacketTestOutcome proto -------------------------------- +test_run { + test_vector { + input { + } + acceptable_outputs { + packets { + } + packets { + } + packets { + } + packets { + } + } + } + actual_output { + } +} +test_result { + failure { + } +} +-- Output: Feature Map ------------------------------------------- +# expected output packets: 4 +# expected punted packets: 0 +# acceptable switch behaviors according to P4 simulation: 1 +test result: fail + +================================================================================ +TestOutcomeToFeatureMap Test +================================================================================ +-- Input: PacketTestOutcome proto -------------------------------- +test_run { + test_vector { + input { + } + acceptable_outputs { + } + acceptable_outputs { + } + acceptable_outputs { + } + acceptable_outputs { + } + acceptable_outputs { + } + } + actual_output { + } +} +test_result { +} +-- Output: Feature Map ------------------------------------------- +# expected output packets: 0 +# expected punted packets: 0 +# acceptable switch behaviors according to P4 simulation: 5 +test result: pass + +================================================================================ +TestOutcomeToFeatureMap Test +================================================================================ +-- Input: PacketTestOutcome proto -------------------------------- +test_run { + test_vector { + input { + } + acceptable_outputs { + packets { + } + packet_ins { + } + packet_ins { + } + } + acceptable_outputs { + packets { + } + packets { + } + packets { + } + packet_ins { + } + } + } + actual_output { + } +} +test_result { +} +-- Output: Feature Map ------------------------------------------- +# expected output packets: 3 +# expected punted packets: 2 +# acceptable switch behaviors according to P4 simulation: 2 +test result: pass + +================================================================================ +WriteCsvFileFromPacketTestOutcomes Test +================================================================================ +-- Input: PacketTestOutcomes proto ------------------------------- +outcomes { + test_run { + test_vector { + input { + } + acceptable_outputs { + packets { + } + packet_ins { + } + } + } + actual_output { + } + } + test_result { + } +} +outcomes { + test_run { + test_vector { + input { + } + acceptable_outputs { + packets { + } + packets { + } + packets { + } + packets { + } + } + } + actual_output { + } + } + test_result { + failure { + } + } +} +outcomes { + test_run { + test_vector { + input { + } + acceptable_outputs { + } + acceptable_outputs { + } + acceptable_outputs { + } + acceptable_outputs { + } + acceptable_outputs { + } + } + actual_output { + } + } + test_result { + } +} +outcomes { + test_run { + test_vector { + input { + } + acceptable_outputs { + packets { + } + packet_ins { + } + packet_ins { + } + } + acceptable_outputs { + packets { + } + packets { + } + packets { + } + packet_ins { + } + } + } + actual_output { + } + } + test_result { + } +} +-- Output: Contents of CSV file ---------------------------------- +# expected output packets,# expected punted packets,# acceptable switch behaviors according to P4 simulation,test result +1,1,1,pass +4,0,1,fail +0,0,5,pass +3,2,2,pass +================================================================================ +ExtractExplanationFromModel Test +================================================================================ +-- Input: RandomForestWithSingleTree ---------------------------- +? [expected(output packets) >= 1.5] # root non-leaf +| ++- T -> [fail : 0 passes, 5 fails] # multicast leaf +| ++- F -> ? [expected(packet-ins) >= 0.5] # unicast non-leaf + | + +- T -> [fail : 0 passes, 5 fails] # unicast w/ punt leaf + | + +- F -> [pass : 5 passes, 0 fails] # unicast w/o punt leaf +-- Output: DetectiveExplanation proto ---------------------- +clusters { + defining_property: "expected(output packets) >= 1.5" + failing_tests: 5 + accuracy_of_predicted_outcome: 1 + coverage_for_predicted_outcome: 0.5 +} +clusters { + defining_property: "expected(output packets) < 1.5 && expected(packet-ins) >= 0.5" + failing_tests: 5 + accuracy_of_predicted_outcome: 1 + coverage_for_predicted_outcome: 0.5 +} +clusters { + defining_property: "expected(output packets) < 1.5 && expected(packet-ins) < 0.5" + predicted_outcome_is_pass: true + passing_tests: 5 + accuracy_of_predicted_outcome: 1 + coverage_for_predicted_outcome: 1 +} +================================================================================ +DVaaS DetectiveExplanationToString Test +================================================================================ +-- Input: DetectiveExplanation proto ----------------------------- +clusters { + defining_property: "input ttl < 2" + predicted_outcome_is_pass: true + passing_tests: 50 + failing_tests: 50 + accuracy_of_predicted_outcome: 0.5 + coverage_for_predicted_outcome: 1 +} +-- Output: DVAAS Detective String -------------------------------- +DVaaS Detective: Found the following pattern(s) among passing test vectors: +- input ttl < 2 -> pass + * accuracy: 50%, 50 out of 100 test vectors that match the conditions pass (remaining 50 fail instead) + * coverage: 100%, accounting for 50 out of 50 passing test vectors +- All passing test vectors accounted for + +DVaaS Detective: Found the following pattern(s) among failing test vectors: +- 50 failing test vectors unaccounted for (100%) + +================================================================================ +DVaaS DetectiveExplanationToString Test +================================================================================ +-- Input: DetectiveExplanation proto ----------------------------- +clusters { + defining_property: "input ttl < 2" + passing_tests: 50 + failing_tests: 50 + accuracy_of_predicted_outcome: 0.5 + coverage_for_predicted_outcome: 1 +} +-- Output: DVAAS Detective String -------------------------------- +DVaaS Detective: Found the following pattern(s) among passing test vectors: +- 50 passing test vectors unaccounted for (100%) + +DVaaS Detective: Found the following pattern(s) among failing test vectors: +- input ttl < 2 -> fail + * accuracy: 50%, 50 out of 100 test vectors that match the conditions fail (remaining 50 pass instead) + * coverage: 100%, accounting for 50 out of 50 failing test vectors +- All failing test vectors accounted for + +================================================================================ +DVaaS DetectiveExplanationToString Test +================================================================================ +-- Input: DetectiveExplanation proto ----------------------------- +clusters { + defining_property: "input ttl < 2" + predicted_outcome_is_pass: true + passing_tests: 25 + failing_tests: 25 + accuracy_of_predicted_outcome: 0.5 + coverage_for_predicted_outcome: 0.5 +} +clusters { + defining_property: "input ttl >= 2" + passing_tests: 25 + failing_tests: 25 + accuracy_of_predicted_outcome: 0.5 + coverage_for_predicted_outcome: 0.5 +} +-- Output: DVAAS Detective String -------------------------------- +DVaaS Detective: Found the following pattern(s) among passing test vectors: +- input ttl < 2 -> pass + * accuracy: 50%, 25 out of 50 test vectors that match the conditions pass (remaining 25 fail instead) + * coverage: 50%, accounting for 25 out of 50 passing test vectors +- 25 passing test vectors unaccounted for (50%) + +DVaaS Detective: Found the following pattern(s) among failing test vectors: +- input ttl >= 2 -> fail + * accuracy: 50%, 25 out of 50 test vectors that match the conditions fail (remaining 25 pass instead) + * coverage: 50%, accounting for 25 out of 50 failing test vectors +- 25 failing test vectors unaccounted for (50%) + +================================================================================ +DVaaS DetectiveExplanationToString Test +================================================================================ +-- Input: DetectiveExplanation proto ----------------------------- +clusters { + defining_property: "expected(output packets) < 2" + predicted_outcome_is_pass: true + passing_tests: 1220 + accuracy_of_predicted_outcome: 1 + coverage_for_predicted_outcome: 0.82 +} +clusters { + defining_property: "expected(packet-ins) < 1" + predicted_outcome_is_pass: true + passing_tests: 256 + accuracy_of_predicted_outcome: 1 + coverage_for_predicted_outcome: 0.17 +} +clusters { + defining_property: "expected(output packets) >= 2 && expected(packet-ins) >= 1" + passing_tests: 12 + failing_tests: 30 + accuracy_of_predicted_outcome: 0.6 + coverage_for_predicted_outcome: 1 +} +-- Output: DVAAS Detective String -------------------------------- +DVaaS Detective: Found the following pattern(s) among passing test vectors: +- expected(output packets) < 2 -> pass + * accuracy: 100%, 1220 out of 1220 test vectors that match the conditions pass (remaining 0 fail instead) + * coverage: 82%, accounting for 1220 out of 1488 passing test vectors +- expected(packet-ins) < 1 -> pass + * accuracy: 100%, 256 out of 256 test vectors that match the conditions pass (remaining 0 fail instead) + * coverage: 17%, accounting for 256 out of 1488 passing test vectors +- 12 passing test vectors unaccounted for (1%) + +DVaaS Detective: Found the following pattern(s) among failing test vectors: +- expected(output packets) >= 2 && expected(packet-ins) >= 1 -> fail + * accuracy: 60%, 30 out of 42 test vectors that match the conditions fail (remaining 12 pass instead) + * coverage: 100%, accounting for 30 out of 30 failing test vectors +- All failing test vectors accounted for + +================================================================================ +DVaaS Detective Explanation Test +================================================================================ +-- Input: PacketTestOutcomes (concise description) --------- +No PacketTestOutcomes +-- Output: Explanation Proto ------------------------------- +INVALID_ARGUMENT: No PacketTestOutcomes provided. + +================================================================================ +DVaaS Detective Explanation Test +================================================================================ +-- Input: PacketTestOutcomes (concise description) --------- +1 passing unicast (i.e. All passing tests) +-- Output: Explanation Proto ------------------------------- +clusters { + predicted_outcome_is_pass: true + passing_tests: 1 + accuracy_of_predicted_outcome: 1 + coverage_for_predicted_outcome: 1 +} +-- Output: Explanation Pretty Print ------------------------ +DVaaS Detective: Found the following pattern(s) among passing test vectors: +- -> pass + * accuracy: 100%, 1 out of 1 test vectors that match the conditions pass (remaining 0 fail instead) + * coverage: 100%, accounting for 1 out of 1 passing test vectors +- All passing test vectors accounted for + +DVaaS Detective: Found the following pattern(s) among failing test vectors: +- 0 failing test vectors unaccounted for (100%) + +================================================================================ +DVaaS Detective Explanation Test +================================================================================ +-- Input: PacketTestOutcomes (concise description) --------- +1 failing unicast (i.e. All failing tests) +-- Output: Explanation Proto ------------------------------- +clusters { + failing_tests: 1 + accuracy_of_predicted_outcome: 1 + coverage_for_predicted_outcome: 1 +} +-- Output: Explanation Pretty Print ------------------------ +DVaaS Detective: Found the following pattern(s) among passing test vectors: +- 0 passing test vectors unaccounted for (100%) + +DVaaS Detective: Found the following pattern(s) among failing test vectors: +- -> fail + * accuracy: 100%, 1 out of 1 test vectors that match the conditions fail (remaining 0 pass instead) + * coverage: 100%, accounting for 1 out of 1 failing test vectors +- All failing test vectors accounted for + +================================================================================ +DVaaS Detective Explanation Test +================================================================================ +-- Input: PacketTestOutcomes (concise description) --------- +1 passing unicast, 1 failing multicast +-- Output: Explanation Proto ------------------------------- +clusters { + defining_property: "# expected output packets >= 1.5" + failing_tests: 1 + accuracy_of_predicted_outcome: 1 + coverage_for_predicted_outcome: 1 +} +clusters { + defining_property: "# expected output packets < 1.5" + predicted_outcome_is_pass: true + passing_tests: 1 + accuracy_of_predicted_outcome: 1 + coverage_for_predicted_outcome: 1 +} +-- Output: Explanation Pretty Print ------------------------ +DVaaS Detective: Found the following pattern(s) among passing test vectors: +- # expected output packets < 1.5 -> pass + * accuracy: 100%, 1 out of 1 test vectors that match the conditions pass (remaining 0 fail instead) + * coverage: 100%, accounting for 1 out of 1 passing test vectors +- All passing test vectors accounted for + +DVaaS Detective: Found the following pattern(s) among failing test vectors: +- # expected output packets >= 1.5 -> fail + * accuracy: 100%, 1 out of 1 test vectors that match the conditions fail (remaining 0 pass instead) + * coverage: 100%, accounting for 1 out of 1 failing test vectors +- All failing test vectors accounted for + +================================================================================ +DVaaS Detective Explanation Test +================================================================================ +-- Input: PacketTestOutcomes (concise description) --------- +3 passing unicast, 1 failing unicast (i.e. Identical tests pass more often than they fail) +-- Output: Explanation Proto ------------------------------- +clusters { + predicted_outcome_is_pass: true + passing_tests: 3 + failing_tests: 1 + accuracy_of_predicted_outcome: 0.75 + coverage_for_predicted_outcome: 1 +} +-- Output: Explanation Pretty Print ------------------------ +DVaaS Detective: Found the following pattern(s) among passing test vectors: +- -> pass + * accuracy: 75%, 3 out of 4 test vectors that match the conditions pass (remaining 1 fail instead) + * coverage: 100%, accounting for 3 out of 3 passing test vectors +- All passing test vectors accounted for + +DVaaS Detective: Found the following pattern(s) among failing test vectors: +- 1 failing test vectors unaccounted for (100%) + +================================================================================ +DVaaS Detective Explanation Test +================================================================================ +-- Input: PacketTestOutcomes (concise description) --------- +1 passing unicast, 3 failing unicast (i.e. Identical tests fail more often than they pass) +-- Output: Explanation Proto ------------------------------- +clusters { + passing_tests: 1 + failing_tests: 3 + accuracy_of_predicted_outcome: 0.75 + coverage_for_predicted_outcome: 1 +} +-- Output: Explanation Pretty Print ------------------------ +DVaaS Detective: Found the following pattern(s) among passing test vectors: +- 1 passing test vectors unaccounted for (100%) + +DVaaS Detective: Found the following pattern(s) among failing test vectors: +- -> fail + * accuracy: 75%, 3 out of 4 test vectors that match the conditions fail (remaining 1 pass instead) + * coverage: 100%, accounting for 3 out of 3 failing test vectors +- All failing test vectors accounted for + +================================================================================ +DVaaS Detective Explanation Test +================================================================================ +-- Input: PacketTestOutcomes (concise description) --------- +1 passing unicast, 1 failing unicast (i.e. Identical tests 50/50 pass and fail) +-- Output: Explanation Proto ------------------------------- +clusters { + predicted_outcome_is_pass: true + passing_tests: 1 + failing_tests: 1 + accuracy_of_predicted_outcome: 0.5 + coverage_for_predicted_outcome: 1 +} +-- Output: Explanation Pretty Print ------------------------ +DVaaS Detective: Found the following pattern(s) among passing test vectors: +- -> pass + * accuracy: 50%, 1 out of 2 test vectors that match the conditions pass (remaining 1 fail instead) + * coverage: 100%, accounting for 1 out of 1 passing test vectors +- All passing test vectors accounted for + +DVaaS Detective: Found the following pattern(s) among failing test vectors: +- 1 failing test vectors unaccounted for (100%) + +================================================================================ +DVaaS Detective Explanation Test +================================================================================ +-- Input: PacketTestOutcomes (concise description) --------- +1 passing unicast, 1 failing unicast+punt, 3 failing multicast (i.e. Conjunction of features determines outcome) +-- Output: Explanation Proto ------------------------------- +clusters { + defining_property: "# expected output packets >= 1.5" + failing_tests: 3 + accuracy_of_predicted_outcome: 1 + coverage_for_predicted_outcome: 0.75 +} +clusters { + defining_property: "# expected output packets < 1.5 && # expected punted packets >= 0.5" + failing_tests: 1 + accuracy_of_predicted_outcome: 1 + coverage_for_predicted_outcome: 0.25 +} +clusters { + defining_property: "# expected output packets < 1.5 && # expected punted packets < 0.5" + predicted_outcome_is_pass: true + passing_tests: 1 + accuracy_of_predicted_outcome: 1 + coverage_for_predicted_outcome: 1 +} +-- Output: Explanation Pretty Print ------------------------ +DVaaS Detective: Found the following pattern(s) among passing test vectors: +- # expected output packets < 1.5 && # expected punted packets < 0.5 -> pass + * accuracy: 100%, 1 out of 1 test vectors that match the conditions pass (remaining 0 fail instead) + * coverage: 100%, accounting for 1 out of 1 passing test vectors +- All passing test vectors accounted for + +DVaaS Detective: Found the following pattern(s) among failing test vectors: +- # expected output packets >= 1.5 -> fail + * accuracy: 100%, 3 out of 3 test vectors that match the conditions fail (remaining 0 pass instead) + * coverage: 75%, accounting for 3 out of 4 failing test vectors +- # expected output packets < 1.5 && # expected punted packets >= 0.5 -> fail + * accuracy: 100%, 1 out of 1 test vectors that match the conditions fail (remaining 0 pass instead) + * coverage: 25%, accounting for 1 out of 4 failing test vectors +- All failing test vectors accounted for diff --git a/dvaas/mirror_testbed_config.cc b/dvaas/mirror_testbed_config.cc index d6b5a40e2..e35a0e023 100644 --- a/dvaas/mirror_testbed_config.cc +++ b/dvaas/mirror_testbed_config.cc @@ -32,7 +32,7 @@ namespace { // ID in `p4rt_port_ids` to an enabled Ethernet interface. absl::Status ConfigureSutInterfacesWithGivenP4RtPortIds( gnmi::gNMI::StubInterface& sut_gnmi_stub, - absl::btree_set& p4rt_port_ids) { + const absl::btree_set& p4rt_port_ids) { // Only map to enabled Ethernet interfaces. auto is_enabled_ethernet_interface = [](const pins_test::openconfig::Interfaces::Interface& interface) { @@ -77,16 +77,11 @@ absl::Status MirrorTestbedConfigurator::ConfigureForForwardingTest( return absl::FailedPreconditionError( "Configure function called on an already configured testbed."); } - if (params.configure_sut_port_ids_for_expected_entries) { - if (!params.sut_entries_to_expect_after_configuration.has_value()) { - return absl::InvalidArgumentError( - "`expected_sut_entries` must have a value when " - "`configure_sut_ports_for_expected_entries` is true."); - } + if (params.p4rt_port_ids_to_configure.has_value()) { if (!params.mirror_sut_ports_ids_to_control_switch) { return absl::InvalidArgumentError( "`mirror_sut_ports_to_control_switch` must be true when " - "configure_sut_ports_for_expected_entries` is true."); + "`used_p4rt_port_ids` is non-nullopt."); } } @@ -96,29 +91,14 @@ absl::Status MirrorTestbedConfigurator::ConfigureForForwardingTest( pins_test::GetInterfacesAsProto(*control_switch_api_.gnmi, gnmi::GetRequest::CONFIG)); - if (params.configure_sut_port_ids_for_expected_entries) { - // Get P4RT port ids in `used_entries`. - ASSIGN_OR_RETURN(p4::v1::GetForwardingPipelineConfigResponse response, - GetForwardingPipelineConfig(sut_api_.p4rt.get())); - ASSIGN_OR_RETURN(pdpi::IrP4Info ir_info, - pdpi::CreateIrP4Info(response.config().p4info())); - std::vector used_entries_list( - params.sut_entries_to_expect_after_configuration.value() - .entries() - .begin(), - params.sut_entries_to_expect_after_configuration.value() - .entries() - .end()); - ASSIGN_OR_RETURN(absl::btree_set used_p4rt_port_ids, - pins_test::GetPortsUsed(ir_info, used_entries_list)); - + if (params.p4rt_port_ids_to_configure.has_value()) { // Clear entities on SUT. This is needed to ensure we can modify the // interface configurations. RETURN_IF_ERROR(pdpi::ClearEntities(*sut_api_.p4rt)); // Change interface configurations on SUT to match `used_p4rt_port_ids`. RETURN_IF_ERROR(ConfigureSutInterfacesWithGivenP4RtPortIds( - *sut_api_.gnmi, used_p4rt_port_ids)); + *sut_api_.gnmi, *params.p4rt_port_ids_to_configure)); } if (params.mirror_sut_ports_ids_to_control_switch) { diff --git a/dvaas/mirror_testbed_config.h b/dvaas/mirror_testbed_config.h index 338b95d5e..436510a96 100644 --- a/dvaas/mirror_testbed_config.h +++ b/dvaas/mirror_testbed_config.h @@ -16,11 +16,14 @@ #define PINS_DVAAS_MIRROR_TESTBED_CONFIG_H_ #include +#include +#include "absl/container/btree_set.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "dvaas/switch_api.h" #include "lib/gnmi/openconfig.pb.h" +#include "lib/p4rt/p4rt_port.h" #include "p4_pdpi/p4_runtime_session.h" #include "thinkit/mirror_testbed.h" #include "thinkit/mirror_testbed_fixture.h" @@ -33,24 +36,16 @@ namespace dvaas { class MirrorTestbedConfigurator { public: struct Params { - // If true, configures interfaces on SUT to make sure - // for any P4RT port id in `sut_entries_to_expect_after_configuration` there - // is exactly one enabled ethernet interface with that id. Existing SUT - // table entries get wiped out during the process. - bool configure_sut_port_ids_for_expected_entries = false; - // Entries that are expected to be installed on SUT after configuration by - // the client of the configurator. Only used for determining interface port - // ids during configuration. Must have a value if - // `configure_sut_port_ids_for_expected_entries` is true. - // NOTE: The configurator won't install these entries on SUT. It is expected - // that the client will install the entries after calling - // `ConfigureForForwardingTest`. - std::optional - sut_entries_to_expect_after_configuration; + // If not nullopt, configures the interfaces on SUT to make sure for any + // P4RT port id in `used_p4rt_port_ids`, there is exactly one enabled + // ethernet interface with that id. Existing SUT table entries get wiped out + // during the process. + std::optional> + p4rt_port_ids_to_configure = std::nullopt; // If true, sets up control switch ports to be a mirror of SUT - // Must be true if `configure_sut_port_ids_for_expected_entries` is true. - // Existing control switch table entries get wiped out during the process. + // Must be true if `used_p4rt_port_ids` is non-nullopt. Existing control + // switch table entries get wiped out during the process. bool mirror_sut_ports_ids_to_control_switch = false; }; @@ -108,10 +103,11 @@ class MirrorTestbedConfigurator { SwitchApi sut_api_; SwitchApi control_switch_api_; - // Keeps the original interfaces configured on the control switch before call - // to `ConfigureForForwardingTest`. Successful call to - // `ConfigureForForwardingTest` sets its value. Successful call to - // `RestoreToOriginalConfiguration` resets it to nullopt. + // Keeps the original interfaces configured on the SUT and control switch + // before call to `ConfigureForForwardingTest`. Successful call to + // `ConfigureForForwardingTest` sets their value. Successful call to + // `RestoreToOriginalConfiguration` resets them to nullopt. + std::optional original_sut_interfaces_; std::optional original_control_interfaces_; }; diff --git a/dvaas/packet_injection.cc b/dvaas/packet_injection.cc index e9e1aa248..9fb55b882 100644 --- a/dvaas/packet_injection.cc +++ b/dvaas/packet_injection.cc @@ -22,6 +22,7 @@ #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/escaping.h" +#include "absl/strings/match.h" #include "absl/strings/str_cat.h" #include "absl/time/time.h" #include "dvaas/port_id_map.h" @@ -35,6 +36,7 @@ #include "p4_pdpi/p4_runtime_session.h" #include "p4_pdpi/packetlib/packetlib.h" #include "p4_pdpi/packetlib/packetlib.pb.h" +#include "p4_pdpi/utils/ir.h" #include "tests/forwarding/util.h" namespace dvaas { @@ -106,9 +108,16 @@ absl::StatusOr GetSutEgressPortFromControlSwitchPacketIn( const MirrorTestbedP4rtPortIdMap& mirror_testbed_port_map) { ASSIGN_OR_RETURN(const std::string control_switch_ingress_port_p4rt_encoding, GetIngressPortFromIrPacketIn(packet_in)); - ASSIGN_OR_RETURN(const P4rtPortId control_switch_ingress_port, - P4rtPortId::MakeFromP4rtEncoding( - control_switch_ingress_port_p4rt_encoding)); + P4rtPortId control_switch_ingress_port; + if (absl::StartsWith(control_switch_ingress_port_p4rt_encoding, "0x")) { + ASSIGN_OR_RETURN(control_switch_ingress_port, + P4rtPortId::MakeFromHexstringEncoding( + control_switch_ingress_port_p4rt_encoding)); + } else { + ASSIGN_OR_RETURN(control_switch_ingress_port, + P4rtPortId::MakeFromP4rtEncoding( + control_switch_ingress_port_p4rt_encoding)); + } return mirror_testbed_port_map.GetSutPortConnectedToControlSwitchPort( control_switch_ingress_port); } @@ -116,7 +125,9 @@ absl::StatusOr GetSutEgressPortFromControlSwitchPacketIn( absl::StatusOr GetIngressPortFromIrPacketIn( const pdpi::IrPacketIn& packet_in) { for (const auto& metadata : packet_in.metadata()) { - if (metadata.name() == "ingress_port") return metadata.value().str(); + if (metadata.name() == "ingress_port") { + return pdpi::IrValueString(metadata.value()); + } } return absl::InvalidArgumentError( "IrPacketIn does not contain 'ingress_port' metadata."); @@ -141,9 +152,10 @@ absl::StatusOr SendTestPacketsAndCollectOutputs( // Send packets. ASSIGN_OR_RETURN(const pdpi::IrP4Info control_ir_p4info, GetIrP4Info(control_switch)); + absl::flat_hash_map packet_injection_time_by_id; for (const auto& [test_id, packet_test_vector] : packet_test_vector_by_id) { + const Packet& packet = packet_test_vector.input().packet(); if (packet_test_vector.input().type() == SwitchInput::DATAPLANE) { - const Packet& packet = packet_test_vector.input().packet(); // Get corresponding control switch port for the packet's ingress port. ASSIGN_OR_RETURN(const P4rtPortId sut_ingress_port, @@ -158,11 +170,29 @@ absl::StatusOr SendTestPacketsAndCollectOutputs( control_switch_port.GetP4rtEncoding(), absl::HexStringToBytes(packet.hex()), control_ir_p4info, &control_switch, injection_delay)); + } else if (packet_test_vector.input().type() == + SwitchInput::SUBMIT_TO_INGRESS) { + // Inject to SUT ingress port. + ASSIGN_OR_RETURN(const pdpi::IrP4Info sut_ir_p4info, GetIrP4Info(sut)); + std::string raw_packet = absl::HexStringToBytes(packet.hex()); + RETURN_IF_ERROR( + pins::InjectIngressPacket(raw_packet, sut_ir_p4info, &sut)); + } else if (packet_test_vector.input().type() == SwitchInput::PACKET_OUT) { + ASSIGN_OR_RETURN(const P4rtPortId sut_egress_port, + P4rtPortId::MakeFromP4rtEncoding(packet.port())); + // Inject to SUT egress port. + ASSIGN_OR_RETURN(const pdpi::IrP4Info sut_ir_p4info, GetIrP4Info(sut)); + RETURN_IF_ERROR( + pins::InjectEgressPacket(sut_egress_port.GetP4rtEncoding(), + absl::HexStringToBytes(packet.hex()), + sut_ir_p4info, &sut, injection_delay)); } else { + // TODO: Add support for other input types. return absl::UnimplementedError( absl::StrCat("Test vector input type not supported\n", packet_test_vector.input().DebugString())); } + packet_injection_time_by_id[test_id] = absl::Now(); } LOG(INFO) << "Finished injecting test packets"; diff --git a/dvaas/packet_injection.h b/dvaas/packet_injection.h index ad22103e4..8967ac77c 100644 --- a/dvaas/packet_injection.h +++ b/dvaas/packet_injection.h @@ -18,11 +18,14 @@ #include #include #include +#include #include "absl/status/statusor.h" +#include "absl/time/time.h" #include "dvaas/port_id_map.h" #include "dvaas/test_vector.h" #include "dvaas/test_vector.pb.h" +#include "lib/p4rt/p4rt_port.h" #include "p4_pdpi/ir.pb.h" #include "p4_pdpi/p4_runtime_session.h" #include "p4_pdpi/packetlib/packetlib.pb.h" diff --git a/dvaas/test_insights.cc b/dvaas/test_insights.cc index 9b2c2f4ce..2d026be7f 100644 --- a/dvaas/test_insights.cc +++ b/dvaas/test_insights.cc @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -56,11 +57,17 @@ constexpr absl::string_view kNotAppliedAction = "not applied"; namespace { -struct PacketTestVectorCsvRow { +// Values for various features of a single packet test. +// The fields in the struct correspond to columns in the test insights table and +// each instance corresponds to one row in the table. +struct TestInsightsRow { int id; + std::optional functional_cluster_id; std::vector actions; std::string acceptable_outcomes; - std::string packet; + std::optional actual_outcome; + std::optional test_pass; + std::string input_packet; }; // Topologically sorts the partial orders and returns a total order. @@ -112,13 +119,25 @@ absl::StatusOr> GetTotalOrderFromPartialOrders( return total_order; } -// Infers the order of table application of `target_tables` from the given -// packet traces. Tables in `target_tables` that are not found in the packet +// Infers the order of table application tables from the given +// packet traces. Tables in `ir_p4info` that are not found in the packet // traces are ordered lexicographically and appended to the end of the // returned order. absl::StatusOr> InferOrderOfTablesFromPacketTraces( absl::Span test_vectors, - const absl::flat_hash_set& target_tables) { + const pdpi::IrP4Info& ir_p4info) { + // Get SAI tables from P4Info. + absl::flat_hash_set target_tables; + for (const auto& [table_name, table] : ir_p4info.tables_by_name()) { + // Avoid BMv2's internal tables. + if (table.preamble().id() == 0) continue; + // Avoid v1model auxiliary tables. + if (table_name == "egress_port_loopback_table") continue; + if (table_name == "v1model_auxiliary_vlan_membership_table") continue; + if (table_name == "ingress_clone_table") continue; + target_tables.insert(table.preamble().name()); + } + // Infer partial orders from packet traces. std::vector> tables_partial_orders; for (const dvaas::PacketTestVector& test_vector : test_vectors) { @@ -159,6 +178,30 @@ absl::StatusOr> InferOrderOfTablesFromPacketTraces( return tables_total_order; } +// For the given list of fully qualified table names, returns the corresponding +// list of table aliases (as defined in `ir_p4info`). +absl::StatusOr> GetTableAliases( + const std::vector& fully_qualified_table_names, + const pdpi::IrP4Info& ir_p4info) { + // Build a map from fully qualified table name to table alias. + absl::flat_hash_map + fully_qualified_table_name_to_alias; + for (const auto& [table_name, table] : ir_p4info.tables_by_name()) { + fully_qualified_table_name_to_alias[table.preamble().name()] = table_name; + } + + std::vector table_aliases; + for (const auto& table : fully_qualified_table_names) { + const auto it = fully_qualified_table_name_to_alias.find(table); + if (it == fully_qualified_table_name_to_alias.end()) { + return absl::InternalError("Table alias not found for table: " + table); + } else { + table_aliases.push_back(it->second); + } + } + return table_aliases; +} + // For each table, returns the set of actions that were applied according to // the given packet traces. absl::btree_map> GetTableToActions( @@ -194,53 +237,38 @@ absl::btree_map> GetTableToActions( return table_to_actions; } +// Return a text description of the switch output. +std::string GetSwitchOutputDescription(const SwitchOutput& switch_output) { + std::string outcome = "drop"; + if (!switch_output.packets().empty() && !switch_output.packet_ins().empty()) { + outcome = "punt_and_forward"; + } else if (!switch_output.packets().empty()) { + outcome = "forward"; + } else if (!switch_output.packet_ins().empty()) { + outcome = "punt"; + } + return outcome; +} + +// Return a text description of the acceptable outcomes. std::string GetAcceptableOutcomesDescription( const dvaas::PacketTestVector& packet_test_vector) { absl::btree_set acceptable_outcomes; for (const auto& acceptable_output : packet_test_vector.acceptable_outputs()) { - std::string outcome = "drop"; - if (!acceptable_output.packets().empty() && - !acceptable_output.packet_ins().empty()) { - outcome = "punt_and_forward"; - } else if (!acceptable_output.packets().empty()) { - outcome = "forward"; - } else if (!acceptable_output.packet_ins().empty()) { - outcome = "punt"; - } - acceptable_outcomes.insert(outcome); + acceptable_outcomes.insert(GetSwitchOutputDescription(acceptable_output)); } return absl::StrJoin(acceptable_outcomes, "/"); } -} // namespace - -absl::StatusOr GetTestInsightsTableAsCsv( +// Returns a list of test insights rows corresponding to the given list of +// packet test vectors. +absl::StatusOr> GetTestInsightsRows( absl::Span packet_test_vectors, - const pdpi::IrP4Info& ir_p4info) { - std::string csv; - - // Get SAI tables from P4Info. - absl::flat_hash_set target_tables; - absl::flat_hash_map - fully_qualified_table_name_to_alias; - for (const auto& [table_name, table] : ir_p4info.tables_by_name()) { - fully_qualified_table_name_to_alias[table.preamble().name()] = table_name; - // Avoid BMv2's internal tables. - if (table.preamble().id() == 0) continue; - // Avoid v1model auxiliary tables. - if (table_name == "egress_port_loopback_table") continue; - if (table_name == "v1model_auxiliary_vlan_membership_table") continue; - if (table_name == "ingress_clone_table") continue; - target_tables.insert(table.preamble().name()); - } - - ASSIGN_OR_RETURN( - std::vector ordered_tables, - InferOrderOfTablesFromPacketTraces(packet_test_vectors, target_tables)); + const std::vector& ordered_tables) { + std::vector rows; // Create one row per packet test vector. - std::vector rows; for (const PacketTestVector& packet_test_vector : packet_test_vectors) { std::vector packet_traces; for (const auto& acceptable_output : @@ -274,58 +302,117 @@ absl::StatusOr GetTestInsightsTableAsCsv( .actions = ordered_actions, .acceptable_outcomes = GetAcceptableOutcomesDescription(packet_test_vector), - .packet = gutil::PrintShortTextProto(packet_test_vector.input()), + .input_packet = gutil::PrintShortTextProto(packet_test_vector.input()), }); } - // Sort rows, put the rows with same actions together. - std::sort( - rows.begin(), rows.end(), - [](const PacketTestVectorCsvRow& lhs, const PacketTestVectorCsvRow& rhs) { - if (lhs.actions != rhs.actions) return lhs.actions < rhs.actions; - if (lhs.acceptable_outcomes != rhs.acceptable_outcomes) - return lhs.acceptable_outcomes < rhs.acceptable_outcomes; - if (lhs.packet != rhs.packet) return lhs.packet < rhs.packet; - if (lhs.id != rhs.id) return lhs.id < rhs.id; - return false; - }); - // Cluster the rows. Put rows with same (action, outcomes) in the same // cluster. - struct PacketTestVectorCsvRowFunctionalClusterComparator { - bool operator()(const PacketTestVectorCsvRow& lhs, - const PacketTestVectorCsvRow& rhs) const { + struct FunctionalClusterComparator { + bool operator()(const TestInsightsRow& lhs, + const TestInsightsRow& rhs) const { if (lhs.actions != rhs.actions) return lhs.actions < rhs.actions; return lhs.acceptable_outcomes < rhs.acceptable_outcomes; } }; - absl::btree_map + absl::btree_map row_to_functional_cluster_id; - for (const auto& row : rows) { + for (auto& row : rows) { row_to_functional_cluster_id.insert( {row, row_to_functional_cluster_id.size()}); + row.functional_cluster_id = row_to_functional_cluster_id.at(row); } + return rows; +} + +} // namespace + +absl::StatusOr GetTestInsightsTableAsCsv( + absl::Span packet_test_vectors, + const pdpi::IrP4Info& ir_p4info) { + std::string csv; + + ASSIGN_OR_RETURN( + const std::vector ordered_tables, + InferOrderOfTablesFromPacketTraces(packet_test_vectors, ir_p4info)); + + ASSIGN_OR_RETURN(std::vector rows, + GetTestInsightsRows(packet_test_vectors, ordered_tables)); + + // Sort rows, placing rows with the same functional cluster near each other. + std::sort(rows.begin(), rows.end(), + [](const TestInsightsRow& lhs, const TestInsightsRow& rhs) { + if (lhs.functional_cluster_id != rhs.functional_cluster_id) + return lhs.functional_cluster_id < rhs.functional_cluster_id; + return lhs.id < rhs.id; + }); + // Create CSV header. - std::vector ordered_table_aliases; - for (const auto& table : ordered_tables) { - const auto it = fully_qualified_table_name_to_alias.find(table); - if (it == fully_qualified_table_name_to_alias.end()) { - return absl::InternalError("Table alias not found for table: " + table); - } else { - ordered_table_aliases.push_back(it->second); - } - } + ASSIGN_OR_RETURN(std::vector ordered_table_aliases, + GetTableAliases(ordered_tables, ir_p4info)); absl::SubstituteAndAppend(&csv, "test vector id,functional cluster,$0, " "accceptable outcomes,input packet\n", absl::StrJoin(ordered_table_aliases, ",")); // Add CSV rows. + for (const auto& row : rows) { + absl::SubstituteAndAppend(&csv, "$0,$1,$2,$3,$4\n", row.id, + *row.functional_cluster_id, + absl::StrJoin(row.actions, ","), + row.acceptable_outcomes, row.input_packet); + } + + return csv; +} + +absl::StatusOr GetTestInsightsTableAsCsv( + const PacketTestOutcomes& test_outcomes, const pdpi::IrP4Info& ir_p4info) { + std::string csv; + + std::vector packet_test_vectors; + packet_test_vectors.reserve(test_outcomes.outcomes_size()); + for (const auto& test_outcome : test_outcomes.outcomes()) { + packet_test_vectors.push_back(test_outcome.test_run().test_vector()); + } + + ASSIGN_OR_RETURN( + const std::vector ordered_tables, + InferOrderOfTablesFromPacketTraces(packet_test_vectors, ir_p4info)); + + ASSIGN_OR_RETURN(std::vector rows, + GetTestInsightsRows(packet_test_vectors, ordered_tables)); + + int index = 0; + for (const PacketTestOutcome& test_outcome : test_outcomes.outcomes()) { + rows[index].test_pass = !test_outcome.test_result().has_failure(); + rows[index].actual_outcome = + GetSwitchOutputDescription(test_outcome.test_run().actual_output()); + ++index; + } + + // Sort rows, placing rows with the same functional cluster near each other. + std::sort(rows.begin(), rows.end(), + [](const TestInsightsRow& lhs, const TestInsightsRow& rhs) { + if (lhs.functional_cluster_id != rhs.functional_cluster_id) + return lhs.functional_cluster_id < rhs.functional_cluster_id; + return lhs.id < rhs.id; + }); + + // Create CSV header. + ASSIGN_OR_RETURN(std::vector ordered_table_aliases, + GetTableAliases(ordered_tables, ir_p4info)); + absl::SubstituteAndAppend(&csv, + "test vector id,functional cluster,$0," + "accceptable outcomes,actual outcome,test " + "pass,input packet\n", + absl::StrJoin(ordered_table_aliases, ",")); + // Add CSV rows. for (const auto& row : rows) { absl::SubstituteAndAppend( - &csv, "$0,$1,$2,$3,$4\n", row.id, row_to_functional_cluster_id.at(row), - absl::StrJoin(row.actions, ","), row.acceptable_outcomes, row.packet); + &csv, "$0,$1,$2,$3,$4,$5,$6\n", row.id, *row.functional_cluster_id, + absl::StrJoin(row.actions, ","), row.acceptable_outcomes, + *row.actual_outcome, (*row.test_pass ? "yes" : "no"), row.input_packet); } return csv; diff --git a/dvaas/test_insights.h b/dvaas/test_insights.h index 6eac1d73b..0742f612d 100644 --- a/dvaas/test_insights.h +++ b/dvaas/test_insights.h @@ -34,6 +34,14 @@ absl::StatusOr GetTestInsightsTableAsCsv( absl::Span packet_test_vectors, const pdpi::IrP4Info& ir_p4info); +// Returns a CSV table of test outcome insights for the given packet test +// outcomes. Each row corresponds to a one packet test outcome. Columns include +// various features of the test packet, including actions applied per table, +// acceptable outcomes, actual outcome, validation result, etc. +absl::StatusOr GetTestInsightsTableAsCsv( + const PacketTestOutcomes& packet_test_outcomes, + const pdpi::IrP4Info& ir_p4info); + } // namespace dvaas #endif // PINS_DVAAS_TEST_INSIGHTS_H_ diff --git a/dvaas/test_run_validation.cc b/dvaas/test_run_validation.cc index aa2e746e7..c6ed97db4 100644 --- a/dvaas/test_run_validation.cc +++ b/dvaas/test_run_validation.cc @@ -21,6 +21,8 @@ #include "absl/base/nullability.h" #include "absl/container/flat_hash_set.h" +#include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" @@ -32,17 +34,17 @@ #include "dvaas/switch_api.h" #include "dvaas/test_vector.pb.h" #include "glog/logging.h" +#include "gmock/gmock.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/repeated_ptr_field.h" #include "google/protobuf/util/message_differencer.h" +#include "gtest/gtest.h" #include "gutil/proto.h" #include "gutil/proto_ordering.h" #include "gutil/status.h" #include "gutil/status_matchers.h" #include "p4_pdpi/ir.pb.h" #include "p4_pdpi/packetlib/packetlib.pb.h" -#include "gmock/gmock.h" -#include "gtest/gtest.h" namespace dvaas { @@ -78,6 +80,20 @@ std::optional GetPacketInMetadataByName( return std::nullopt; } +absl::Status IgnoreField(MessageDifferencer& differ, + const google::protobuf::Descriptor& descriptor, + absl::string_view field_name) { + const google::protobuf::FieldDescriptor* field_descriptor = + descriptor.FindFieldByName(field_name); + if (field_descriptor == nullptr) { + return absl::InvalidArgumentError( + absl::StrFormat("Could not find field '%s' in '%s'", field_name, + descriptor.full_name())); + } + differ.IgnoreField(field_descriptor); + return absl::OkStatus(); +} + bool CompareSwitchOutputs(SwitchOutput actual_output, SwitchOutput expected_output, const SwitchOutputDiffParams& params, @@ -109,16 +125,17 @@ bool CompareSwitchOutputs(SwitchOutput actual_output, MessageDifferencer differ; // Ignore logical field `reasons_invalid` since it is redundant (computed // from other fields) and misleading (not part of the actual packet). - const google::protobuf::FieldDescriptor* reasons_invalid_descriptor = - packetlib::Packet::descriptor()->FindFieldByName("reasons_invalid"); - if (reasons_invalid_descriptor == nullptr) { - LOG(FATAL) << "Could not find field " // Crash ok: testonly code - "'reasons_invalid' in packetlib::Packet"; + CHECK_OK( // Crash ok: testonly code + IgnoreField(differ, *packetlib::Packet::descriptor(), + "reasons_invalid")); + for (auto* field : params.ignored_packetlib_fields) { + differ.IgnoreField(field); + } + if (params.ManualPayloadCheck) { + CHECK_OK( // Crash ok: testonly code + IgnoreField(differ, *packetlib::Packet::descriptor(), "payload")); } - differ.IgnoreField(reasons_invalid_descriptor); - for (auto* field : params.ignored_packetlib_fields) - differ.IgnoreField(field); std::string diff; differ.ReportDifferencesToString(&diff); if (!differ.Compare(expected_packet.parsed(), actual_packet.parsed())) { @@ -133,6 +150,14 @@ bool CompareSwitchOutputs(SwitchOutput actual_output, actual_packet.port()); return false; } + if (params.ManualPayloadCheck) { + std::optional result = params.ManualPayloadCheck( + actual_packet.parsed().payload(), expected_packet.parsed().payload()); + if (result.has_value()) { + *listener << "has packet " << i << " with invalid payload: " << *result; + return false; + } + } } if (!params.treat_expected_and_actual_outputs_as_having_no_packet_ins) { @@ -141,8 +166,20 @@ bool CompareSwitchOutputs(SwitchOutput actual_output, const PacketIn& expected_packet_in = expected_output.packet_ins(i); MessageDifferencer differ; - for (auto* field : params.ignored_packetlib_fields) + // Ignore logical field `reasons_invalid` since it is redundant (computed + // from other fields) and misleading (not part of the actual packet in). + CHECK_OK( // Crash ok: testonly code + IgnoreField(differ, *packetlib::Packet::descriptor(), + "reasons_invalid")); + for (auto* field : params.ignored_packetlib_fields) { differ.IgnoreField(field); + } + + if (params.ManualPayloadCheck) { + CHECK_OK( // Crash ok: testonly code + IgnoreField(differ, *packetlib::Packet::descriptor(), "payload")); + } + std::string diff; differ.ReportDifferencesToString(&diff); if (!differ.Compare(expected_packet_in.parsed(), @@ -151,6 +188,16 @@ bool CompareSwitchOutputs(SwitchOutput actual_output, << " with mismatched header fields:\n " << Indent(2, diff); return false; } + if (params.ManualPayloadCheck) { + std::optional result = + params.ManualPayloadCheck(actual_packet_in.parsed().payload(), + expected_packet_in.parsed().payload()); + if (result.has_value()) { + *listener << "has packet in " << i + << " with invalid payload: " << *result; + return false; + } + } // Check that received packet in has desired metadata (except for ignored // metadata). @@ -392,8 +439,8 @@ absl::StatusOr ValidateTestRun( "ModifyTestRunPreDiffing is set."); } RETURN_IF_ERROR(diff_params.ModifyExpectedOutputPreDiffing( - test_run.test_vector().input(), test_run.actual_output(), - *test_run.mutable_test_vector()->mutable_acceptable_outputs(), *sut)); + test_run.test_vector().input(), test_run.actual_output(), *sut, + *test_run.mutable_test_vector()->mutable_acceptable_outputs())); } PacketTestValidationResult result; diff --git a/dvaas/test_run_validation.h b/dvaas/test_run_validation.h index abf84c78d..86a18395c 100644 --- a/dvaas/test_run_validation.h +++ b/dvaas/test_run_validation.h @@ -16,6 +16,7 @@ #define PINS_test_run_validation_H_ #include +#include #include #include @@ -23,6 +24,7 @@ #include "absl/container/flat_hash_set.h" #include "absl/status/status.h" #include "absl/status/statusor.h" +#include "absl/strings/string_view.h" #include "dvaas/switch_api.h" #include "dvaas/test_vector.pb.h" #include "google/protobuf/descriptor.h" @@ -63,9 +65,19 @@ struct SwitchOutputDiffParams { // recommended as a last resort, fixing the simulation should be preferred. std::function& acceptable_outputs, - SwitchApi& sut)> + const SwitchApi& sut, + google::protobuf::RepeatedPtrField& acceptable_outputs)> ModifyExpectedOutputPreDiffing; + + // Used to override the default MessageDifferencer handling for only the + // `payload` field. When enabled this override will be applied to both Packet + // and PacketIn checks. + // + // On success returns std::nullopt, and on failure will return an error + // message. + std::function(absl::string_view actual_payload, + absl::string_view expected_payload)> + ManualPayloadCheck; }; // Validates the given `test_run` using the parameters in `diff_params`. The diff --git a/dvaas/test_run_validation_test.cc b/dvaas/test_run_validation_test.cc index 0e917e246..0fd34c0ea 100644 --- a/dvaas/test_run_validation_test.cc +++ b/dvaas/test_run_validation_test.cc @@ -1,10 +1,13 @@ #include "dvaas/test_run_validation.h" +#include #include #include "absl/status/status.h" +#include "absl/strings/string_view.h" #include "dvaas/switch_api.h" #include "dvaas/test_vector.pb.h" +#include "gmock/gmock.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/repeated_ptr_field.h" #include "gtest/gtest.h" @@ -12,8 +15,6 @@ #include "gutil/status_matchers.h" #include "gutil/testing.h" #include "p4_pdpi/packetlib/packetlib.pb.h" -#include "gmock/gmock.h" -#include "gtest/gtest.h" namespace dvaas { namespace { @@ -427,6 +428,27 @@ TEST(TestRunValidationTest, PacketFieldReasonsInvalidIsIgnored) { ASSERT_THAT(result, EqualsProto(R"pb()pb")); } +TEST(TestRunValidationTest, PacketInFieldReasonsInvalidIsIgnored) { + PacketTestRun test_run = gutil::ParseProtoOrDie(R"pb( + test_vector { + acceptable_outputs { + packet_ins { + parsed { + reasons_invalid: "invalid reason 1" + reasons_invalid: "invalid reason 2" + } + } + } + } + actual_output { packet_ins { parsed {} } } + )pb"); + + // Validation must succeed. + ASSERT_OK_AND_ASSIGN(PacketTestValidationResult result, + ValidateTestRun(test_run)); + ASSERT_THAT(result, EqualsProto(R"pb()pb")); +} + TEST(TestRunValidationTest, ModifyExpectedOutputPreDiffingTest) { PacketTestRun test_run = gutil::ParseProtoOrDie(R"pb( test_vector { @@ -459,10 +481,9 @@ TEST(TestRunValidationTest, ModifyExpectedOutputPreDiffingTest) { SwitchOutputDiffParams{ .ModifyExpectedOutputPreDiffing = [](const SwitchInput& input, - const SwitchOutput& actual_output, + const SwitchOutput& actual_output, const SwitchApi& sut, google::protobuf::RepeatedPtrField& - acceptable_expected_outputs, - SwitchApi& sut) { + acceptable_expected_outputs) { *acceptable_expected_outputs.at(0) .mutable_packets(0) ->mutable_parsed() @@ -475,5 +496,115 @@ TEST(TestRunValidationTest, ModifyExpectedOutputPreDiffingTest) { ASSERT_FALSE(result_with_modify.has_failure()); } +std::optional FailCustomPayloadCheck( + absl::string_view /*actual_payload*/, + absl::string_view /*expected_payload*/) { + return "failed payload check"; +} + +std::optional PassCustomPayloadCheck( + absl::string_view /*actual_payload*/, + absl::string_view /*expected_payload*/) { + return std::nullopt; +} + +TEST(TestRunValidationTest, CustomPayloadCheckForPacket) { + PacketTestRun test_run = gutil::ParseProtoOrDie(R"pb( + test_vector { + acceptable_outputs { + packets { + port: "1" + parsed { payload: "original-payload" } + } + } + } + actual_output { + packets { + port: "1" + parsed { payload: "new-payload" } + } + } + )pb"); + + // Sanity check to confirm that we would fail without the ManualPayloadCheck. + ASSERT_OK_AND_ASSIGN(PacketTestValidationResult result_without_modify, + ValidateTestRun(test_run)); + EXPECT_TRUE(result_without_modify.has_failure()); + + ASSERT_OK_AND_ASSIGN(PacketTestValidationResult result_with_modify, + ValidateTestRun(test_run, SwitchOutputDiffParams{ + .ManualPayloadCheck = + PassCustomPayloadCheck, + })); + EXPECT_FALSE(result_with_modify.has_failure()); +} + +TEST(TestRunValidationTest, CustomPayloadCheckForPacketIn) { + PacketTestRun test_run = gutil::ParseProtoOrDie(R"pb( + test_vector { + acceptable_outputs { + packet_ins { parsed { payload: "original-payload" } } + } + } + actual_output { packet_ins { parsed { payload: "new-payload" } } } + )pb"); + + // Sanity check to confirm that we would fail without the ManualPayloadCheck. + ASSERT_OK_AND_ASSIGN(PacketTestValidationResult result_without_modify, + ValidateTestRun(test_run)); + EXPECT_TRUE(result_without_modify.has_failure()); + + ASSERT_OK_AND_ASSIGN(PacketTestValidationResult result_with_modify, + ValidateTestRun(test_run, SwitchOutputDiffParams{ + .ManualPayloadCheck = + PassCustomPayloadCheck, + })); + EXPECT_FALSE(result_with_modify.has_failure()); +} + +TEST(TestRunValidationTest, CustomPayloadCheckFailsForPacket) { + PacketTestRun test_run = gutil::ParseProtoOrDie(R"pb( + test_vector { + acceptable_outputs { + packets { + port: "1" + parsed { payload: "original-payload" } + } + } + } + actual_output { + packets { + port: "1" + parsed { payload: "new-payload" } + } + } + )pb"); + + ASSERT_OK_AND_ASSIGN(PacketTestValidationResult result_with_modify, + ValidateTestRun(test_run, SwitchOutputDiffParams{ + .ManualPayloadCheck = + FailCustomPayloadCheck, + })); + EXPECT_TRUE(result_with_modify.has_failure()); +} + +TEST(TestRunValidationTest, CustomPayloadCheckFailsForPacketIn) { + PacketTestRun test_run = gutil::ParseProtoOrDie(R"pb( + test_vector { + acceptable_outputs { + packet_ins { parsed { payload: "original-payload" } } + } + } + actual_output { packet_ins { parsed { payload: "new-payload" } } } + )pb"); + + ASSERT_OK_AND_ASSIGN(PacketTestValidationResult result_with_modify, + ValidateTestRun(test_run, SwitchOutputDiffParams{ + .ManualPayloadCheck = + FailCustomPayloadCheck, + })); + EXPECT_TRUE(result_with_modify.has_failure()); +} + } // namespace } // namespace dvaas diff --git a/dvaas/test_vector_stats.cc b/dvaas/test_vector_stats.cc index 4e37f6a41..ac83e4682 100644 --- a/dvaas/test_vector_stats.cc +++ b/dvaas/test_vector_stats.cc @@ -53,14 +53,57 @@ void AddTestVectorStats(const PacketTestOutcome& outcome, num_punted == acceptable_output.packet_ins_size(); }); if (has_correct_num_outputs) { - stats.num_vectors_with_correct_number_of_outputs += 1; + stats.num_vectors_where_sut_produced_correct_number_of_outputs += 1; } - stats.num_vectors_forwarding += num_forwarded > 0 ? 1 : 0; - stats.num_vectors_punting += num_punted > 0 ? 1 : 0; - stats.num_vectors_dropping += num_forwarded == 0 && num_punted == 0; + stats.num_vectors_where_sut_forwarded_at_least_one_packet += + num_forwarded > 0 ? 1 : 0; + stats.num_vectors_where_sut_punted_at_least_one_packet += + num_punted > 0 ? 1 : 0; + stats.num_vectors_where_sut_produced_no_output += + num_forwarded == 0 && num_punted == 0; stats.num_packets_forwarded += num_forwarded; stats.num_packets_punted += num_punted; + + // Compute expectation stats. + bool forwarding_allowed = + absl::c_any_of(outcome.test_run().test_vector().acceptable_outputs(), + [](const SwitchOutput& acceptable_output) { + return acceptable_output.packets_size() > 0; + }); + bool forwarding_required = + absl::c_all_of(outcome.test_run().test_vector().acceptable_outputs(), + [](const SwitchOutput& acceptable_output) { + return acceptable_output.packets_size() > 0; + }); + bool punting_allowed = + absl::c_any_of(outcome.test_run().test_vector().acceptable_outputs(), + [](const SwitchOutput& acceptable_output) { + return acceptable_output.packet_ins_size() > 0; + }); + bool punting_required = + absl::c_all_of(outcome.test_run().test_vector().acceptable_outputs(), + [](const SwitchOutput& acceptable_output) { + return acceptable_output.packet_ins_size() > 0; + }); + bool no_output_allowed = + absl::c_any_of(outcome.test_run().test_vector().acceptable_outputs(), + [](const SwitchOutput& acceptable_output) { + return acceptable_output.packets_size() == 0 && + acceptable_output.packet_ins_size() == 0; + }); + bool no_output_required = + absl::c_all_of(outcome.test_run().test_vector().acceptable_outputs(), + [](const SwitchOutput& acceptable_output) { + return acceptable_output.packets_size() == 0 && + acceptable_output.packet_ins_size() == 0; + }); + stats.max_vectors_with_forwarding_expected += forwarding_allowed ? 1 : 0; + stats.min_vectors_with_forwarding_expected += forwarding_required ? 1 : 0; + stats.max_vectors_with_punting_expected += punting_allowed ? 1 : 0; + stats.min_vectors_with_punting_expected += punting_required ? 1 : 0; + stats.max_vectors_with_no_output_expected += no_output_allowed ? 1 : 0; + stats.min_vectors_with_no_output_expected += no_output_required ? 1 : 0; } } // namespace @@ -85,57 +128,59 @@ std::string ExplainPercent(int numerator, int denominator) { } std::string ExplainFraction(int numerator, int denominator) { - return absl::StrFormat("%s of %d", ExplainPercent(numerator, denominator), - denominator); + return absl::StrFormat("%d (%s) of %d", numerator, + ExplainPercent(numerator, denominator), denominator); } -} // namespace - -// 1. Don't print reproducibility message if no packets were replicated. -// 2. Don't print reproducibility message unless all test vector -// failures were reproducible or all test vector failures were -// irreproducible. -void ExplainReproducibilityRate(const TestVectorStats& stats, - std::string& result) { - if (stats.num_vectors_with_reproducibility_rate <= 0) return; - if (stats.num_deterministic_failures == - stats.num_vectors_with_reproducibility_rate) { - absl::StrAppendFormat( - &result, - "All of %d test vectors attempted had deterministically " - "reproducible failures\n", - stats.num_vectors_with_reproducibility_rate); - } else if (stats.num_deterministic_failures == 0) { - absl::StrAppendFormat( - &result, - "None of %d test vectors attempted had deterministically " - "reproducible failures\n", - stats.num_vectors_with_reproducibility_rate); - } +std::string ExplainRange(int min, int max) { + if (min == max) return absl::StrFormat("%d", min); + return absl::StrFormat("%d - %d", min, max); } +} // namespace + std::string ExplainTestVectorStats(const TestVectorStats& stats) { std::string result; absl::StrAppendFormat( - &result, "%s test vectors passed\n", + &result, "%s test vectors passed\nThe System Under Test (SUT):\n", ExplainFraction(stats.num_vectors_passed, stats.num_vectors)); + absl::StrAppendFormat( &result, - "%s test vectors produced the correct number and type of output " - "packets\n", - ExplainFraction(stats.num_vectors_with_correct_number_of_outputs, - stats.num_vectors)); + "- forwarded: for %d test vectors " + "(expected: %s)\n", + stats.num_vectors_where_sut_forwarded_at_least_one_packet, + ExplainRange(stats.min_vectors_with_forwarding_expected, + stats.max_vectors_with_forwarding_expected)); absl::StrAppendFormat( &result, - "%d test vectors forwarded, producing %d forwarded output packets\n", - stats.num_vectors_forwarding, stats.num_packets_forwarded); + "- punted: for %d test vectors " + "(expected: %s)\n", + stats.num_vectors_where_sut_punted_at_least_one_packet, + ExplainRange(stats.min_vectors_with_punting_expected, + stats.max_vectors_with_punting_expected)); absl::StrAppendFormat( - &result, "%d test vectors punted, producing %d punted output packets\n", - stats.num_vectors_punting, stats.num_packets_punted); - absl::StrAppendFormat(&result, "%d test vectors produced no output packets\n", - stats.num_vectors_dropping); - ExplainReproducibilityRate(stats, result); + &result, + "- dropped: for %d test vectors " + "(expected: %s)\n", + stats.num_vectors_where_sut_produced_no_output, + ExplainRange(stats.min_vectors_with_no_output_expected, + stats.max_vectors_with_no_output_expected)); + + // If some test vectors didn't pass, but came close, explain how many. + const int num_almost_passed = + stats.num_vectors_where_sut_produced_correct_number_of_outputs - + stats.num_vectors_passed; + if (num_almost_passed > 0) { + absl::StrAppendFormat( + &result, + "- came close* to expected behavior: for %d failing test vectors\n" + " (*produced expected number of forwarded/punted packets, but with " + "unexpected header/payload values)", + num_almost_passed); + } + return result; } diff --git a/dvaas/test_vector_stats.h b/dvaas/test_vector_stats.h index c4da88a58..a01d80f1a 100644 --- a/dvaas/test_vector_stats.h +++ b/dvaas/test_vector_stats.h @@ -25,12 +25,23 @@ namespace dvaas { struct TestVectorStats { int num_vectors = 0; int num_vectors_passed = 0; + // Can be higher than `num_vectors_passed`, e.g. because the outputs // could have incorrect header field values. - int num_vectors_with_correct_number_of_outputs = 0; - int num_vectors_forwarding = 0; - int num_vectors_punting = 0; - int num_vectors_dropping = 0; + int num_vectors_where_sut_produced_correct_number_of_outputs = 0; + + int num_vectors_where_sut_forwarded_at_least_one_packet = 0; + int num_vectors_where_sut_punted_at_least_one_packet = 0; + int num_vectors_where_sut_produced_no_output = 0; + + // Due to non-determinism (e.g. WCMP), the number of vectors where we expect + // the SUT to forward/punt/drop can be a range. + int max_vectors_with_forwarding_expected = 0; + int min_vectors_with_forwarding_expected = 0; + int max_vectors_with_punting_expected = 0; + int min_vectors_with_punting_expected = 0; + int max_vectors_with_no_output_expected = 0; + int min_vectors_with_no_output_expected = 0; int num_packets_forwarded = 0; int num_packets_punted = 0; diff --git a/dvaas/test_vector_stats_test.cc b/dvaas/test_vector_stats_test.cc index 00899cd5e..51871fa15 100644 --- a/dvaas/test_vector_stats_test.cc +++ b/dvaas/test_vector_stats_test.cc @@ -174,6 +174,50 @@ PacketTestOutcomes GetPacketTestOutcomes() { # No reproducibility rate is given. } } + + # Unexpected drop instead of forward. + outcomes { + test_run { + test_vector { + input {} + # Deterministic forward. + acceptable_outputs { + packets {} + packet_ins: [] + } + } + actual_output { + packets: [] + packet_ins: [] + } + } + # Failed. + test_result { failure {} } + } + + # Non-deterministic number of expected punts. + outcomes { + test_run { + test_vector { + input {} + # Non-deterministic punt. + acceptable_outputs { + packets: [] + packet_ins: [] + } + acceptable_outputs { + packets: [] + packet_ins: {} + } + } + actual_output { + packets: [] + packet_ins: [] + } + } + # Passed. + test_result {} + } )pb"); } @@ -181,31 +225,8 @@ TEST(TestVectorStatsGoldenTest, ComputeTestVectorStatsAndExplainTestVectorStats) { PacketTestOutcomes outcomes = GetPacketTestOutcomes(); TestVectorStats stats = ComputeTestVectorStats(outcomes); - std::cout << ExplainTestVectorStats(stats); -} - -TEST(TestVectorStatsGoldenTest, ReproducibilityRateScenarios) { - PacketTestOutcomes outcomes = GetPacketTestOutcomes(); - outcomes.mutable_outcomes(1) - ->mutable_test_result() - ->mutable_failure() - ->mutable_minimization_analysis() - ->set_reproducibility_rate(1.0); - outcomes.mutable_outcomes(4) - ->mutable_test_result() - ->mutable_failure() - ->mutable_minimization_analysis() - ->set_reproducibility_rate(1.0); - TestVectorStats stats = ComputeTestVectorStats(outcomes); - std::cout << ExplainTestVectorStats(stats); - - outcomes.mutable_outcomes(4) - ->mutable_test_result() - ->mutable_failure() - ->mutable_minimization_analysis() - ->set_reproducibility_rate(0.0); - stats = ComputeTestVectorStats(outcomes); - std::cout << ExplainTestVectorStats(stats); + std::cout << "-- ExplainTestVectorStats test ------------------------------\n" + << ExplainTestVectorStats(stats) << "\n\n"; } } // namespace diff --git a/dvaas/test_vector_stats_test.expected b/dvaas/test_vector_stats_test.expected index cf29a8e5d..cf30b8f37 100644 --- a/dvaas/test_vector_stats_test.expected +++ b/dvaas/test_vector_stats_test.expected @@ -1,17 +1,8 @@ -57.14% of 7 test vectors passed -100.00% of 7 test vectors produced the correct number and type of output packets -4 test vectors forwarded, producing 6 forwarded output packets -3 test vectors punted, producing 3 punted output packets -1 test vectors produced no output packets -None of 2 test vectors attempted had deterministically reproducible failures -57.14% of 7 test vectors passed -100.00% of 7 test vectors produced the correct number and type of output packets -4 test vectors forwarded, producing 6 forwarded output packets -3 test vectors punted, producing 3 punted output packets -1 test vectors produced no output packets -All of 2 test vectors attempted had deterministically reproducible failures -57.14% of 7 test vectors passed -100.00% of 7 test vectors produced the correct number and type of output packets -4 test vectors forwarded, producing 6 forwarded output packets -3 test vectors punted, producing 3 punted output packets -1 test vectors produced no output packets +-- ExplainTestVectorStats test ------------------------------ +5 (55.56%) of 9 test vectors passed +The System Under Test (SUT): +- forwarded: for 4 test vectors (expected: 5) +- punted: for 3 test vectors (expected: 3 - 4) +- dropped: for 3 test vectors (expected: 1 - 2) +- came close* to expected behavior: for 3 failing test vectors + (*produced expected number of forwarded/punted packets, but with unexpected header/payload values) diff --git a/dvaas/thinkit_tests/BUILD.bazel b/dvaas/thinkit_tests/BUILD.bazel index 5a542ab7c..86d5f0ec3 100644 --- a/dvaas/thinkit_tests/BUILD.bazel +++ b/dvaas/thinkit_tests/BUILD.bazel @@ -30,11 +30,14 @@ cc_library( "//dvaas:test_vector_cc_proto", "//dvaas:validation_result", "//gutil:status_matchers", + "//lib/p4rt:p4rt_port", "//p4_pdpi:p4_runtime_session", "//p4_pdpi:p4_runtime_session_extras", "//tests/lib:switch_test_setup_helpers", "//thinkit:mirror_testbed", "//thinkit:mirror_testbed_fixture", + "@com_google_absl//absl/container:btree", + "@com_google_absl//absl/log", "@com_google_googletest//:gtest", ], alwayslink = True, @@ -48,6 +51,7 @@ proto_library( "//dvaas:test_vector_proto", "//p4_pdpi:ir_proto", "@com_github_p4lang_p4runtime//:p4info_proto", + "@com_google_protobuf//:any_proto", ], ) diff --git a/dvaas/thinkit_tests/dvaas_regression.proto b/dvaas/thinkit_tests/dvaas_regression.proto index 54e337c91..fb9048655 100644 --- a/dvaas/thinkit_tests/dvaas_regression.proto +++ b/dvaas/thinkit_tests/dvaas_regression.proto @@ -17,6 +17,7 @@ syntax = "proto3"; package dvaas; import "dvaas/test_vector.proto"; +import "google/protobuf/any.proto"; import "p4/config/v1/p4info.proto"; import "p4_pdpi/ir.proto"; @@ -25,4 +26,5 @@ message DvaasRegressionTestProto { pdpi.IrEntities minimal_set_of_entities = 2; p4.config.v1.P4Info p4info = 3; bool currently_failing = 4; + .google.protobuf.Any user_metadata = 5; } diff --git a/dvaas/thinkit_tests/dvaas_regression_test.cc b/dvaas/thinkit_tests/dvaas_regression_test.cc index 471f98648..f89851fd3 100644 --- a/dvaas/thinkit_tests/dvaas_regression_test.cc +++ b/dvaas/thinkit_tests/dvaas_regression_test.cc @@ -2,17 +2,19 @@ #include #include +#include +#include "absl/container/btree_set.h" +#include "absl/log/log.h" #include "dvaas/dataplane_validation.h" #include "dvaas/mirror_testbed_config.h" #include "dvaas/switch_api.h" #include "dvaas/test_vector.pb.h" #include "dvaas/validation_result.h" -#include "gutil/status_matchers.h" -#include "p4_pdpi/p4_runtime_session_extras.h" #include "gmock/gmock.h" #include "gtest/gtest.h" -#include "gutil/status_matchers.h" +#include "gutil/status_matchers.h" +#include "lib/p4rt/p4rt_port.h" #include "p4_pdpi/p4_runtime_session.h" #include "p4_pdpi/p4_runtime_session_extras.h" #include "tests/lib/switch_test_setup_helpers.h" @@ -48,18 +50,25 @@ TEST_P(DvaasRegressionTest, DvaasRegressionTest) { // TODO: Directly pass in IR entities instead of extracting IR // table entries once support for IR entities has been added to // `ConfigureForForwardingTest`. - pdpi::IrTableEntries ir_table_entries; + std::vector used_entries_list; for (const pdpi::IrEntity& ir_entity : GetParam() .dvaas_regression_test_proto.minimal_set_of_entities() .entities()) { if (ir_entity.has_table_entry()) { - *ir_table_entries.add_entries() = ir_entity.table_entry(); + used_entries_list.push_back(ir_entity.table_entry()); } } + + ASSERT_OK_AND_ASSIGN(pdpi::IrP4Info ir_p4_info, + pdpi::GetIrP4Info(*configured_testbed.SutApi().p4rt)); + ASSERT_OK_AND_ASSIGN( + absl::btree_set used_p4rt_port_ids, + pins_test::GetPortsUsed(ir_p4_info, used_entries_list)); + LOG(INFO) << "Number of used P4rt port ids: " << used_p4rt_port_ids.size(); + ASSERT_OK(configured_testbed.ConfigureForForwardingTest({ - .configure_sut_port_ids_for_expected_entries = true, - .sut_entries_to_expect_after_configuration = ir_table_entries, + .p4rt_port_ids_to_configure = used_p4rt_port_ids, .mirror_sut_ports_ids_to_control_switch = true, })); // Install the entities on the switch. diff --git a/dvaas/user_provided_packet_test_vector.cc b/dvaas/user_provided_packet_test_vector.cc index 91a9c9dd4..1eb8052c4 100644 --- a/dvaas/user_provided_packet_test_vector.cc +++ b/dvaas/user_provided_packet_test_vector.cc @@ -72,14 +72,19 @@ absl::StatusOr LegitimizePacketInAndReturnId( absl::Status LegitimizeTestVector( PacketTestVector vector, const pdpi::IrP4Info& ir_info, PacketTestVectorById& legitimized_test_vectors_by_id) { - if (vector.input().type() != SwitchInput::DATAPLANE) { - return gutil::UnimplementedErrorBuilder() - << "only supported input type is DATAPLANE; found " - << SwitchInput::Type_Name(vector.input().type()); - } // Legitimize input packet. Packet& input_packet = *vector.mutable_input()->mutable_packet(); + + // TODO: Revisit this when the decision about what to do with + // input ports for submit_to_ingress test vectors is finalized. + if (vector.input().type() == SwitchInput::SUBMIT_TO_INGRESS) { + // The port will be ignored for submit_to_ingress packets so using an + // arbitrary port is fine. + if (input_packet.port().empty()) { + input_packet.set_port("1"); + } + } ASSIGN_OR_RETURN(int id, LegitimizePacketAndReturnId(input_packet), _.SetPrepend() << "invalid input packet: "); diff --git a/dvaas/user_provided_packet_test_vector_test.cc b/dvaas/user_provided_packet_test_vector_test.cc index 01e774227..0c79cace6 100644 --- a/dvaas/user_provided_packet_test_vector_test.cc +++ b/dvaas/user_provided_packet_test_vector_test.cc @@ -295,19 +295,52 @@ std::vector GetPositiveTestCases() { )pb"), }, }; - return test_cases; -} // namespace - -std::vector GetNegativeTestCases() { - std::vector test_cases; test_cases.emplace_back() = TestCase{ - .description = "input type != DATAPLANE", + .description = "input type = SUBMIT_TO_INGRESS", .vectors = { gutil::ParseProtoOrDie(R"pb( input { type: SUBMIT_TO_INGRESS + packet { + parsed { + headers { + ethernet_header { + ethernet_destination: "ff:ee:dd:cc:bb:aa" + ethernet_source: "55:44:33:22:11:00" + ethertype: "0x0010" + } + } + payload: "test packet #42:" + } + } + } + acceptable_outputs { + packets { + parsed { + headers { + ethernet_header { + ethernet_destination: "ff:ee:dd:cc:bb:aa" + ethernet_source: "55:44:33:22:11:00" + ethertype: "0x0010" + } + } + payload: "test packet #42:" + } + } + } + )pb"), + }, + }; + + test_cases.emplace_back() = TestCase{ + .description = "input type = PACKET_OUT", + .vectors = + { + gutil::ParseProtoOrDie(R"pb( + input { + type: PACKET_OUT packet { port: "1" parsed { @@ -322,10 +355,29 @@ std::vector GetNegativeTestCases() { } } } - acceptable_outputs {} + acceptable_outputs { + packets { + port: "1" + parsed { + headers { + ethernet_header { + ethernet_destination: "ff:ee:dd:cc:bb:aa" + ethernet_source: "55:44:33:22:11:00" + ethertype: "0x0010" + } + } + payload: "test packet #42:" + } + } + } )pb"), }, }; + return test_cases; +} + +std::vector GetNegativeTestCases() { + std::vector test_cases; test_cases.emplace_back() = TestCase{ .description = "missing expectation", @@ -586,6 +638,7 @@ std::vector GetNegativeTestCases() { )pb"), }, }; + return test_cases; } diff --git a/dvaas/user_provided_packet_test_vector_test.expected b/dvaas/user_provided_packet_test_vector_test.expected index 815702593..97014c5e1 100644 --- a/dvaas/user_provided_packet_test_vector_test.expected +++ b/dvaas/user_provided_packet_test_vector_test.expected @@ -219,14 +219,13 @@ added: acceptable_outputs[0].packet_ins[0].hex: "0505050505050505050505050010746 ================================================================================ -InternalizeUserProvidedTestVectors Test: input type != DATAPLANE +InternalizeUserProvidedTestVectors Test: input type = SUBMIT_TO_INGRESS ================================================================================ -- Input --------------------------------------------------- -- Input Packet Test Vector #1 -- input { type: SUBMIT_TO_INGRESS packet { - port: "1" parsed { headers { ethernet_header { @@ -240,13 +239,36 @@ input { } } acceptable_outputs { + packets { + parsed { + headers { + ethernet_header { + ethernet_destination: "ff:ee:dd:cc:bb:aa" + ethernet_source: "55:44:33:22:11:00" + ethertype: "0x0010" + } + } + payload: "test packet #42:" + } + } } -- Output -------------------------------------------------- -ERROR: UNIMPLEMENTED: problem in user-provided packet test vector: only supported input type is DATAPLANE; found SUBMIT_TO_INGRESS -Dumping offending test vector: +-- Internalized Packet Test Vector #1 -- +test packet ID extracted from payload: 42 +diff of internalized vector vs input vector: +added: input.packet.port: "1" +added: input.packet.hex: "ffeeddccbbaa554433221100001074657374207061636b6574202334323a" +added: acceptable_outputs[0].packets[0].hex: "ffeeddccbbaa554433221100001074657374207061636b6574202334323a" + + +================================================================================ +InternalizeUserProvidedTestVectors Test: input type = PACKET_OUT +================================================================================ +-- Input --------------------------------------------------- +-- Input Packet Test Vector #1 -- input { - type: SUBMIT_TO_INGRESS + type: PACKET_OUT packet { port: "1" parsed { @@ -262,8 +284,28 @@ input { } } acceptable_outputs { + packets { + port: "1" + parsed { + headers { + ethernet_header { + ethernet_destination: "ff:ee:dd:cc:bb:aa" + ethernet_source: "55:44:33:22:11:00" + ethertype: "0x0010" + } + } + payload: "test packet #42:" + } + } } +-- Output -------------------------------------------------- +-- Internalized Packet Test Vector #1 -- +test packet ID extracted from payload: 42 +diff of internalized vector vs input vector: +added: input.packet.hex: "ffeeddccbbaa554433221100001074657374207061636b6574202334323a" +added: acceptable_outputs[0].packets[0].hex: "ffeeddccbbaa554433221100001074657374207061636b6574202334323a" + ================================================================================ InternalizeUserProvidedTestVectors Test: missing expectation diff --git a/dvaas/validation_result.cc b/dvaas/validation_result.cc index 68d74e8f7..877651d3e 100644 --- a/dvaas/validation_result.cc +++ b/dvaas/validation_result.cc @@ -19,7 +19,9 @@ #include "absl/algorithm/container.h" #include "absl/status/status.h" +#include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" +#include "dvaas/dvaas_detective.h" #include "dvaas/test_run_validation.h" #include "dvaas/test_vector.pb.h" #include "dvaas/test_vector_stats.h" @@ -64,8 +66,21 @@ absl::Status ValidationResult::HasSuccessRateOfAtLeast( return outcome.test_result().has_failure(); }); if (it == test_outcomes_.outcomes().end()) return absl::OkStatus(); + + absl::StatusOr explanation = + ExplainTestOutcomes(test_outcomes_); + std::string explanation_string; + if (explanation.ok()) { + explanation_string = *explanation; + } else { + explanation_string = absl::StrCat( + "NOTICE: Dvaas Detective failed to generate an explanation: ", + explanation.status().message()); + } + return gutil::OutOfRangeErrorBuilder() - << ExplainTestVectorStats(test_vector_stats_) + << ExplainTestVectorStats(test_vector_stats_) << "\n" + << explanation_string << "\nShowing the first failure only. Refer to the test artifacts " "for the full list of errors.\n" << ExplainFailure(it->test_result().failure()); diff --git a/lib/p4rt/BUILD.bazel b/lib/p4rt/BUILD.bazel index 9b2c42728..d9e7d816c 100644 --- a/lib/p4rt/BUILD.bazel +++ b/lib/p4rt/BUILD.bazel @@ -79,6 +79,7 @@ cc_library( "//gutil:status", "//p4_pdpi/string_encodings:byte_string", "//p4_pdpi/string_encodings:decimal_string", + "//p4_pdpi/string_encodings:hex_string", "@com_google_absl//absl/numeric:bits", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", diff --git a/lib/p4rt/p4rt_port.cc b/lib/p4rt/p4rt_port.cc index ed18e3965..0f5ae86e9 100644 --- a/lib/p4rt/p4rt_port.cc +++ b/lib/p4rt/p4rt_port.cc @@ -28,6 +28,7 @@ #include "gutil/status.h" #include "p4_pdpi/string_encodings/byte_string.h" #include "p4_pdpi/string_encodings/decimal_string.h" +#include "p4_pdpi/string_encodings/hex_string.h" namespace pins_test { @@ -61,6 +62,13 @@ std::vector P4rtPortId::MakeVectorFromOpenConfigEncodings( return result; } +absl::StatusOr P4rtPortId::MakeFromHexstringEncoding( + absl::string_view hexstring_encoding) { + ASSIGN_OR_RETURN(uint32_t p4rt_port_id_int, + pdpi::HexStringToUint32(hexstring_encoding)); + return P4rtPortId(p4rt_port_id_int); +} + uint32_t P4rtPortId::GetOpenConfigEncoding() const { return p4rt_port_id_; } std::string P4rtPortId::GetP4rtEncoding() const { diff --git a/lib/p4rt/p4rt_port.h b/lib/p4rt/p4rt_port.h index 953abb7c1..dcd2c76c2 100644 --- a/lib/p4rt/p4rt_port.h +++ b/lib/p4rt/p4rt_port.h @@ -56,6 +56,10 @@ class P4rtPortId { static std::vector MakeVectorFromOpenConfigEncodings(absl::Span p4rt_port_id); + // Constructs a P4rtPortId from a hexstring encoding. + static absl::StatusOr MakeFromHexstringEncoding( + absl::string_view hexstring_encoding); + // Getters. // Returns OpenConfig encoding of the port ID, e.g. the uint32 `42`. uint32_t GetOpenConfigEncoding() const; diff --git a/lib/p4rt/p4rt_port_test.cc b/lib/p4rt/p4rt_port_test.cc index 2764b91c7..f7a42f7f0 100644 --- a/lib/p4rt/p4rt_port_test.cc +++ b/lib/p4rt/p4rt_port_test.cc @@ -62,6 +62,17 @@ TEST(P4rtPortId, RoundtripsFromString) { port_id); } +TEST(P4rtPortId, MakeFromHexstringEncodingWorks) { + constexpr int kP4rtPortIdInt = 42; + const std::string kP4rtPortIdString = "42"; + const std::string kP4rtPortIdHexString = "0x2a"; + ASSERT_OK_AND_ASSIGN( + P4rtPortId port_id, + P4rtPortId::MakeFromHexstringEncoding(kP4rtPortIdHexString)); + EXPECT_EQ(port_id.GetOpenConfigEncoding(), kP4rtPortIdInt); + EXPECT_EQ(port_id.GetP4rtEncoding(), kP4rtPortIdString); +} + TEST(P4rtPortId, GetBmv2P4rtEncodingWorksForPortsFittingIn9Bits) { EXPECT_THAT(P4rtPortId::MakeFromOpenConfigEncoding(0).GetBmv2P4rtEncoding(), IsOkAndHolds(Eq(std::string(1, '\0')))); diff --git a/p4rt_app/p4runtime/BUILD.bazel b/p4rt_app/p4runtime/BUILD.bazel index d82a28409..239b5e400 100644 --- a/p4rt_app/p4runtime/BUILD.bazel +++ b/p4rt_app/p4runtime/BUILD.bazel @@ -1,3 +1,5 @@ +load("@com_google_googleapis_imports//:imports.bzl", "cc_proto_library") + # Copyright 2020 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/pins_infra_deps.bzl b/pins_infra_deps.bzl index 6973ffdca..291dad4d0 100644 --- a/pins_infra_deps.bzl +++ b/pins_infra_deps.bzl @@ -236,15 +236,12 @@ def pins_infra_deps(): ], sha256 = "a89e203d3cf264e564fcb96b6e06dd70bc0557356eb48400ce4b5d97c2c3720d", ) - if not native.existing_rule("com_github_nlohmann_json"): + if not native.existing_rule("com_google_ydf"): http_archive( - name = "com_github_nlohmann_json", - # JSON for Modern C++ - url = "https://github.com/nlohmann/json/archive/v3.7.3.zip", - strip_prefix = "json-3.7.3", - sha256 = "e109cd4a9d1d463a62f0a81d7c6719ecd780a52fb80a22b901ed5b6fe43fb45b", - build_file_content = """cc_library(name="json", - visibility=["//visibility:public"], - hdrs=["single_include/nlohmann/json.hpp"] - )""", + name = "com_google_ydf", + urls = [ + "https://github.com/google/yggdrasil-decision-forests/archive/09873a58cd4127dca1a0e8f0835800fad6ca8295.zip", + ], + strip_prefix = "yggdrasil-decision-forests-09873a58cd4127dca1a0e8f0835800fad6ca8295", + sha256 = "4e1fb83693a59d34094899c170718f79204fbcd0a33c317e9bc8629dd36f3918", ) diff --git a/tests/forwarding/BUILD.bazel b/tests/forwarding/BUILD.bazel index a8dd580d3..21227ba42 100644 --- a/tests/forwarding/BUILD.bazel +++ b/tests/forwarding/BUILD.bazel @@ -306,9 +306,13 @@ cc_library( "//dvaas:validation_result", "//gutil:status", "//gutil:status_matchers", + "//lib/p4rt:p4rt_port", "//p4_pdpi:ir_cc_proto", "//p4_pdpi:p4_runtime_session", + "//p4_pdpi:p4_runtime_session_extras", "//thinkit:mirror_testbed_fixture", + "@com_google_absl//absl/container:btree", + "@com_google_absl//absl/log", "@com_google_googletest//:gtest", ], alwayslink = True, diff --git a/tests/forwarding/arriba_test.cc b/tests/forwarding/arriba_test.cc index cc02c0a8f..ad8ac6a8b 100644 --- a/tests/forwarding/arriba_test.cc +++ b/tests/forwarding/arriba_test.cc @@ -14,6 +14,11 @@ #include "tests/forwarding/arriba_test.h" +#include +#include + +#include "absl/container/btree_set.h" +#include "absl/log/log.h" #include "dvaas/arriba_test_vector_validation.h" #include "dvaas/mirror_testbed_config.h" #include "dvaas/validation_result.h" @@ -21,7 +26,10 @@ #include "gtest/gtest.h" #include "gutil/status.h" #include "gutil/status_matchers.h" +#include "lib/p4rt/p4rt_port.h" +#include "p4_pdpi/ir.pb.h" #include "p4_pdpi/p4_runtime_session.h" +#include "p4_pdpi/p4_runtime_session_extras.h" namespace pins_test { namespace { @@ -31,10 +39,21 @@ TEST_P(ArribaTest, SwitchUnderTestPassesArribaTestVector) { dvaas::MirrorTestbedConfigurator::Create( &GetParam().mirror_testbed->GetMirrorTestbed())); + std::vector used_entries_list( + GetParam().arriba_test_vector.ir_table_entries().entries().begin(), + GetParam().arriba_test_vector.ir_table_entries().entries().end()); + + ASSERT_OK_AND_ASSIGN(pdpi::IrP4Info ir_p4_info, + pdpi::GetIrP4Info(*configured_testbed.SutApi().p4rt)); + + ASSERT_OK_AND_ASSIGN( + absl::btree_set used_p4rt_port_ids, + GetUsedP4rtPortIds(GetParam().arriba_test_vector, used_entries_list, + ir_p4_info)); + LOG(INFO) << "Number of used P4rt port ids: " << used_p4rt_port_ids.size(); + ASSERT_OK(configured_testbed.ConfigureForForwardingTest({ - .configure_sut_port_ids_for_expected_entries = true, - .sut_entries_to_expect_after_configuration = - GetParam().arriba_test_vector.ir_table_entries(), + .p4rt_port_ids_to_configure = used_p4rt_port_ids, .mirror_sut_ports_ids_to_control_switch = true, }));