From b2401c96a1268b95a15cdf1f25e9e295c4a53a68 Mon Sep 17 00:00:00 2001 From: smolkaj Date: Wed, 23 Oct 2024 03:31:37 -0700 Subject: [PATCH 1/2] Add the implementation for smoke test in test/forwarding. --- tests/forwarding/BUILD.bazel | 51 ++++++++++++ tests/forwarding/p4_blackbox_fixture.h | 79 ++++++++++++++++++ tests/forwarding/smoke_test.cc | 107 +++++++++++++++++++++++++ tests/forwarding/smoke_test.h | 27 +++++++ tests/forwarding/test_data.cc | 56 +++++++++++++ tests/forwarding/test_data.h | 32 ++++++++ 6 files changed, 352 insertions(+) create mode 100644 tests/forwarding/p4_blackbox_fixture.h create mode 100644 tests/forwarding/smoke_test.cc create mode 100644 tests/forwarding/smoke_test.h create mode 100644 tests/forwarding/test_data.cc create mode 100644 tests/forwarding/test_data.h diff --git a/tests/forwarding/BUILD.bazel b/tests/forwarding/BUILD.bazel index 95394268..2df932c7 100644 --- a/tests/forwarding/BUILD.bazel +++ b/tests/forwarding/BUILD.bazel @@ -47,3 +47,54 @@ cc_library( "@com_google_absl//absl/time", ], ) + +cc_library( + name = "test_data", + testonly = True, + srcs = ["test_data.cc"], + hdrs = ["test_data.h"], + deps = [ + "//gutil:testing", + "//sai_p4/instantiations/google:sai_pd_cc_proto", + "@com_google_absl//absl/strings:str_format", + ], +) + +cc_library( + name = "p4_blackbox_fixture", + testonly = True, + hdrs = ["p4_blackbox_fixture.h"], + deps = [ + "//gutil:status_matchers", + "//p4_pdpi:p4_runtime_session", + "//p4_pdpi:pd", + "//sai_p4/instantiations/google:sai_p4info_cc", + "//sai_p4/instantiations/google:sai_pd_cc_proto", + "//thinkit:mirror_testbed", + "//thinkit:mirror_testbed_fixture", + "@com_google_googletest//:gtest", + ], +) + +cc_library( + name = "smoke_test", + testonly = True, + srcs = ["smoke_test.cc"], + hdrs = ["smoke_test.h"], + visibility = ["//platforms/networking/gpins/testing/blackbox/p4:__pkg__"], + deps = [ + ":p4_blackbox_fixture", + ":test_data", + "//gutil:proto_matchers", + "//gutil:status_matchers", + "//gutil:testing", + "//p4_pdpi:p4_runtime_session", + "//p4_pdpi:pd", + "//sai_p4/instantiations/google:sai_p4info_cc", + "//sai_p4/instantiations/google:sai_pd_cc_proto", + "//thinkit:mirror_testbed_fixture", + "@com_google_googletest//:gtest_main", + ], + alwayslink = True, +) + diff --git a/tests/forwarding/p4_blackbox_fixture.h b/tests/forwarding/p4_blackbox_fixture.h new file mode 100644 index 00000000..fe3e6178 --- /dev/null +++ b/tests/forwarding/p4_blackbox_fixture.h @@ -0,0 +1,79 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef PINS_TESTS_FORWARDING_P4_BLACKBOX_FIXTURE_H_ +#define PINS_TESTS_FORWARDING_P4_BLACKBOX_FIXTURE_H_ + +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "gutil/status_matchers.h" +#include "p4_pdpi/p4_runtime_session.h" +#include "p4_pdpi/pd.h" +#include "sai_p4/instantiations/google/sai_p4info.h" +#include "sai_p4/instantiations/google/sai_pd.pb.h" +#include "thinkit/mirror_testbed.h" +#include "thinkit/mirror_testbed_fixture.h" + +namespace gpins { + +// Fixture for P4 blackbox testing. It performs test specific setup and +// teardown: Creates an initializes a P4RT channel, to get the switch ready to +// accept programming operations. Clears the switch of all table entries before +// every test. +class P4BlackboxFixture : public thinkit::MirrorTestbedFixture { + public: + void SetUp() override { + MirrorTestbedFixture::SetUp(); + // Initialize the connection. + ASSERT_OK_AND_ASSIGN(sut_p4rt_session_, pdpi::P4RuntimeSession::Create( + GetMirrorTestbed().Sut())); + + //ASSERT_OK(pdpi::SetForwardingPipelineConfig(sut_p4rt_session_.get(), + // sai::GetP4Info(sai::Instantiation::kMiddleblock))); + ASSERT_OK(pdpi::SetMetadataAndSetForwardingPipelineConfig(sut_p4rt_session_.get(), + p4::v1::SetForwardingPipelineConfigRequest::RECONCILE_AND_COMMIT, + sai::GetP4Info(sai::Instantiation::kMiddleblock))); + + // Clear entries here in case the previous test did not (e.g. because it + // crashed). + ASSERT_OK(pdpi::ClearTableEntries(sut_p4rt_session_.get())); + // Check that switch is in a clean state. + ASSERT_OK_AND_ASSIGN(auto read_back_entries, + pdpi::ReadPiTableEntries(sut_p4rt_session_.get())); + ASSERT_EQ(read_back_entries.size(), 0); + } + + void TearDown() override { + if (SutP4RuntimeSession() != nullptr) { + // Clear all table entries to leave the switch in a clean state. + EXPECT_OK(pdpi::ClearTableEntries(SutP4RuntimeSession())); + } + + MirrorTestbedFixture::TearDown(); + } + + pdpi::P4RuntimeSession* SutP4RuntimeSession() const { + return sut_p4rt_session_.get(); + } + + const pdpi::IrP4Info& IrP4Info() const { return ir_p4info_; } + + private: + std::unique_ptr sut_p4rt_session_; + pdpi::IrP4Info ir_p4info_ = sai::GetIrP4Info(sai::Instantiation::kMiddleblock); +}; + +} // namespace gpins + +#endif // PINS_TESTS_FORWARDING_P4_BLACKBOX_FIXTURE_H_ diff --git a/tests/forwarding/smoke_test.cc b/tests/forwarding/smoke_test.cc new file mode 100644 index 00000000..05ae2ebd --- /dev/null +++ b/tests/forwarding/smoke_test.cc @@ -0,0 +1,107 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "tests/forwarding/smoke_test.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "gutil/proto_matchers.h" +#include "gutil/status_matchers.h" +#include "gutil/testing.h" +#include "p4_pdpi/p4_runtime_session.h" +#include "p4_pdpi/pd.h" +#include "sai_p4/instantiations/google/sai_p4info.h" +#include "sai_p4/instantiations/google/sai_pd.pb.h" +#include "tests/forwarding/test_data.h" + +namespace gpins { +namespace { + +TEST_P(SmokeTestFixture, InsertTableEntry) { + const sai::TableEntry pd_entry = gutil::ParseProtoOrDie( + R"PB( + router_interface_table_entry { + match { router_interface_id: "router-interface-1" } + action { + set_port_and_src_mac { port: "0x000" src_mac: "02:2a:10:00:00:03" } + } + } + )PB"); + + ASSERT_OK_AND_ASSIGN(const p4::v1::TableEntry pi_entry, + pdpi::PartialPdTableEntryToPiTableEntry(IrP4Info(), pd_entry)); + ASSERT_OK(pdpi::InstallPiTableEntry(SutP4RuntimeSession(), pi_entry)); +} + +TEST_P(SmokeTestFixture, InsertTableEntryWithRandomCharacterId) { + sai::TableEntry pd_entry = gutil::ParseProtoOrDie( + R"PB( + router_interface_table_entry { + match { router_interface_id: "\x01\x33\x00\xff,\":'}(*{+-" } + action { + set_port_and_src_mac { port: "0x000" src_mac: "02:2a:10:00:00:03" } + } + } + )PB"); + + ASSERT_OK_AND_ASSIGN(const p4::v1::TableEntry pi_entry, + pdpi::PartialPdTableEntryToPiTableEntry(IrP4Info(), pd_entry)); + ASSERT_OK(pdpi::InstallPiTableEntry(SutP4RuntimeSession(), pi_entry)); + ASSERT_OK_AND_ASSIGN(auto entries, + pdpi::ReadPiTableEntries(SutP4RuntimeSession())); + ASSERT_EQ(entries.size(), 1); + ASSERT_THAT(entries[0], gutil::EqualsProto(pi_entry)); +} + +TEST_P(SmokeTestFixture, InsertAndReadTableEntries) { + pdpi::P4RuntimeSession* session = SutP4RuntimeSession(); + const pdpi::IrP4Info& ir_p4info = IrP4Info(); + std::vector write_pd_entries = + sai_pd::CreateUpTo255GenericTableEntries(3); + + thinkit::TestEnvironment& test_environment = GetMirrorTestbed().Environment(); + std::vector write_pi_entries; + p4::v1::ReadResponse expected_read_response; + write_pi_entries.reserve(write_pd_entries.size()); + for (const auto& pd_entry : write_pd_entries) { + ASSERT_OK_AND_ASSIGN(p4::v1::TableEntry pi_entry, + pdpi::PartialPdTableEntryToPiTableEntry(ir_p4info, pd_entry)); + + ASSERT_OK(test_environment.AppendToTestArtifact( + "pi_entries_written.pb.txt", + absl::StrCat(pi_entry.DebugString(), "\n"))); + *expected_read_response.add_entities()->mutable_table_entry() = pi_entry; + write_pi_entries.push_back(std::move(pi_entry)); + } + + ASSERT_OK(pdpi::InstallPiTableEntries(session, ir_p4info, write_pi_entries)); + + p4::v1::ReadRequest read_request; + read_request.add_entities()->mutable_table_entry(); + ASSERT_OK_AND_ASSIGN(p4::v1::ReadResponse read_response, + pdpi::SetMetadataAndSendPiReadRequest(session, read_request)); + + for (const auto& entity : read_response.entities()) { + ASSERT_OK(test_environment.AppendToTestArtifact( + "pi_entries_read_back.pb.txt", + absl::StrCat(entity.table_entry().DebugString(), "\n"))); + } + + // Compare the result in proto format since the fields being compared are + // nested and out of order. + EXPECT_THAT(read_response, gutil::EqualsProto(expected_read_response)); +} + +} // namespace +} // namespace gpins diff --git a/tests/forwarding/smoke_test.h b/tests/forwarding/smoke_test.h new file mode 100644 index 00000000..700b2171 --- /dev/null +++ b/tests/forwarding/smoke_test.h @@ -0,0 +1,27 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef PINS_TESTS_FORWARDING_SMOKE_TEST_H_ +#define PINS_TESTS_FORWARDING_SMOKE_TEST_H_ + +#include "tests/forwarding/p4_blackbox_fixture.h" +#include "thinkit/mirror_testbed_fixture.h" + +namespace gpins { + +class SmokeTestFixture : public P4BlackboxFixture {}; + +} // namespace gpins + +#endif // PINS_TESTS_FORWARDING_SMOKE_TEST_H_ diff --git a/tests/forwarding/test_data.cc b/tests/forwarding/test_data.cc new file mode 100644 index 00000000..be71dd2c --- /dev/null +++ b/tests/forwarding/test_data.cc @@ -0,0 +1,56 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "tests/forwarding/test_data.h" + +#include + +#include "absl/strings/str_format.h" +#include "gutil/testing.h" +#include "sai_p4/instantiations/google/sai_pd.pb.h" + +namespace sai_pd { + +std::vector CreateUpTo255GenericTableEntries( + int num_table_entries) { + // Valid IPv4 address is 4 octets of numbers (from 0-255 in decimal) separated + // by periods. + num_table_entries = std::min(num_table_entries, 255); + std::vector pd_table_entries; + pd_table_entries.reserve(num_table_entries); + for (int i = 0; i < num_table_entries; ++i) { + sai::TableEntry& pd_entry = pd_table_entries.emplace_back(); + pd_entry = gutil::ParseProtoOrDie( + R"PB( + acl_ingress_table_entry { + match { + is_ipv4 { value: "0x1" } + dst_ip { value: "10.206.196.0" mask: "255.255.255.255" } + ip_protocol { value: "0x11" mask: "0xff" } + l4_dst_port { value: "0x0043" mask: "0xffff" } + } + action { trap { qos_queue: "0x1" } } + priority: 2040 + } + )PB"); + pd_entry.mutable_acl_ingress_table_entry() + ->mutable_match() + ->mutable_dst_ip() + ->set_value(absl::StrFormat("10.206.196.%d", i)); + } + + return pd_table_entries; +} + +} // namespace sai_pd diff --git a/tests/forwarding/test_data.h b/tests/forwarding/test_data.h new file mode 100644 index 00000000..838cb578 --- /dev/null +++ b/tests/forwarding/test_data.h @@ -0,0 +1,32 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef PINS_TESTS_FORWARDING_TEST_DATA_H_ +#define PINS_TESTS_FORWARDING_TEST_DATA_H_ + +#include "sai_p4/instantiations/google/sai_pd.pb.h" + +// TODO Move it to the sai_pd library when it is available. +namespace sai_pd { + +// Returns a vector of N generic table entries. It is up to the implementation +// to pick the table, match fields, action, etc, but the function guarantees the +// entries are valid. Useful for testing when the exact table entry doesn't +// matter. +std::vector CreateUpTo255GenericTableEntries( + int num_table_entries); + +} // namespace sai_pd + +#endif // PINS_TESTS_FORWARDING_TEST_DATA_H_ From 0fc5d64d8549480f9a971563efa17c25b802aa95 Mon Sep 17 00:00:00 2001 From: smolkaj Date: Wed, 23 Oct 2024 05:34:56 -0700 Subject: [PATCH 2/2] Add test to validate StartBertRequest parameters.make smoke_test public for all platforms.adding the interfaces as part of interface name.Fix smoke test dependency. --- tests/forwarding/BUILD.bazel | 4 +- tests/gnoi/BUILD.bazel | 2 + tests/gnoi/bert_tests.cc | 128 ++++++++++++++++++++++++++--------- 3 files changed, 100 insertions(+), 34 deletions(-) diff --git a/tests/forwarding/BUILD.bazel b/tests/forwarding/BUILD.bazel index 2df932c7..6431e3be 100644 --- a/tests/forwarding/BUILD.bazel +++ b/tests/forwarding/BUILD.bazel @@ -81,7 +81,7 @@ cc_library( testonly = True, srcs = ["smoke_test.cc"], hdrs = ["smoke_test.h"], - visibility = ["//platforms/networking/gpins/testing/blackbox/p4:__pkg__"], + visibility = ["//visibility:public"], deps = [ ":p4_blackbox_fixture", ":test_data", @@ -93,7 +93,7 @@ cc_library( "//sai_p4/instantiations/google:sai_p4info_cc", "//sai_p4/instantiations/google:sai_pd_cc_proto", "//thinkit:mirror_testbed_fixture", - "@com_google_googletest//:gtest_main", + "@com_google_googletest//:gtest", ], alwayslink = True, ) diff --git a/tests/gnoi/BUILD.bazel b/tests/gnoi/BUILD.bazel index cba106dd..dbbdb955 100644 --- a/tests/gnoi/BUILD.bazel +++ b/tests/gnoi/BUILD.bazel @@ -28,11 +28,13 @@ cc_library( ], deps = [ "//gutil:status_matchers", + "//gutil:testing", "//thinkit:mirror_testbed_fixture", "@com_github_gnoi//diag:diag_cc_proto", "@com_github_google_glog//:glog", "@com_github_grpc_grpc//:grpc++", "@com_google_absl//absl/strings", + "@com_google_absl//absl/time", "@com_google_googletest//:gtest", "@com_google_protobuf//:protobuf", ], diff --git a/tests/gnoi/bert_tests.cc b/tests/gnoi/bert_tests.cc index 24450696..10411f67 100644 --- a/tests/gnoi/bert_tests.cc +++ b/tests/gnoi/bert_tests.cc @@ -14,54 +14,118 @@ #include "tests/gnoi/bert_tests.h" +#include "absl/strings/str_cat.h" #include "absl/strings/substitute.h" +#include "absl/time/clock.h" +#include "absl/time/time.h" #include "diag/diag.pb.h" #include "glog/logging.h" #include "google/protobuf/text_format.h" #include "gtest/gtest.h" #include "gutil/status_matchers.h" +#include "gutil/testing.h" namespace bert { -// Test StartBERT with test duration longer than maximum allowed duration. -TEST_P(BertTest, StartBertFailsIfTestDurationTooLong) { +using ::testing::HasSubstr; + +// Test StartBERT with invalid request parameters. +TEST_P(BertTest, StartBertFailsIfRequestParametersInvalid) { thinkit::Switch& sut = GetMirrorTestbed().Sut(); ASSERT_OK_AND_ASSIGN(auto sut_gnoi_diag_stub, sut.CreateGnoiDiagStub()); - // Set BERT test duration to more than 24 hours: (24 hours + 1 sec). - constexpr uint32_t kTesDurationInSecs = (60 * 60 * 24) + 1; + // TODO (b/182417612) : Select one admin state "up" port. + gnoi::diag::StartBERTRequest valid_request = + gutil::ParseProtoOrDie(R"PROTO( + bert_operation_id: "OpId-1" + per_port_requests { + interface { + origin: "openconfig" + elem { name: "interfaces" } + elem { + name: "interface" + key { key: "name" value: "Ethernet0" } + } + } + prbs_polynomial: PRBS_POLYNOMIAL_PRBS23 + test_duration_in_secs: 600 + } + )PROTO"); + // Set a unique BERT operation id using current time. + valid_request.set_bert_operation_id( + absl::StrCat("OpId-", absl::ToUnixMillis(absl::Now()))); + gnoi::diag::StartBERTResponse response; - // TODO (ashishksingh) : Before using the "Ethernet0", run sanity test to - // ensure the ports are oper-state "up" on the SUT switch. - const std::string kStartBertRequest = - absl::Substitute(R"PROTO( - bert_operation_id: "OpId-1" - per_port_requests { - interface { - origin: "openconfig" - elem { - name: "interface" - key { key: "name" value: "Ethernet0" } - } - } - prbs_polynomial: PRBS_POLYNOMIAL_PRBS23 - test_duration_in_secs: $0 - } - )PROTO", - kTesDurationInSecs); + // Case 1: BERT test duration is 0 secs. + { + gnoi::diag::StartBERTRequest too_short_time_request = valid_request; + too_short_time_request.mutable_per_port_requests(0) + ->set_test_duration_in_secs(0); + grpc::ClientContext context; + LOG(INFO) << "Sending StartBERT request: " + << too_short_time_request.ShortDebugString(); + EXPECT_THAT(gutil::GrpcStatusToAbslStatus(sut_gnoi_diag_stub->StartBERT( + &context, too_short_time_request, &response)), + gutil::StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("is too short"))); + } - gnoi::diag::StartBERTRequest request; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(kStartBertRequest, - &request)); + // Case 2: BERT test duration is more than 24 hours: (24 hours + 1 sec). + { + gnoi::diag::StartBERTRequest too_long_time_request = valid_request; + too_long_time_request.mutable_per_port_requests(0) + ->set_test_duration_in_secs( + ToInt64Seconds(absl::Hours(24) + absl::Seconds(1))); + response.Clear(); + grpc::ClientContext context; + LOG(INFO) << "Sending StartBERT request: " + << too_long_time_request.ShortDebugString(); + EXPECT_THAT(gutil::GrpcStatusToAbslStatus(sut_gnoi_diag_stub->StartBERT( + &context, too_long_time_request, &response)), + gutil::StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("is too long"))); + } - gnoi::diag::StartBERTResponse response; - grpc::ClientContext context; - LOG(INFO) << "Sending StartBERT request: " << request.ShortDebugString(); + // Case 3: Invalid PRBS polynomial order. + { + gnoi::diag::StartBERTRequest unset_prbs_polynomial_request = valid_request; + unset_prbs_polynomial_request.mutable_per_port_requests(0) + ->clear_prbs_polynomial(); + response.Clear(); + grpc::ClientContext context; + LOG(INFO) << "Sending StartBERT request: " + << unset_prbs_polynomial_request.ShortDebugString(); + EXPECT_THAT(gutil::GrpcStatusToAbslStatus(sut_gnoi_diag_stub->StartBERT( + &context, unset_prbs_polynomial_request, &response)), + gutil::StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("PRBS polynomial is not set"))); + } - EXPECT_THAT(gutil::GrpcStatusToAbslStatus( - sut_gnoi_diag_stub->StartBERT(&context, request, &response)), - gutil::StatusIs(absl::StatusCode::kInvalidArgument, - testing::HasSubstr("too long"))); + // Case 4: Invalid interface. + { + gnoi::diag::StartBERTRequest invalid_interface_request = valid_request; + gnoi::types::Path invalid_interface = + gutil::ParseProtoOrDie( + R"PROTO( + origin: "openconfig" + elem { name: "interfaces" } + elem { + name: "interface" + key { key: "name" value: "InvalidPort" } + } + )PROTO"); + invalid_interface_request.mutable_per_port_requests(0) + ->mutable_interface() + ->CopyFrom(invalid_interface); + response.Clear(); + grpc::ClientContext context; + LOG(INFO) << "Sending StartBERT request: " + << invalid_interface_request.ShortDebugString(); + EXPECT_THAT(gutil::GrpcStatusToAbslStatus(sut_gnoi_diag_stub->StartBERT( + &context, invalid_interface_request, &response)), + gutil::StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("Interface is invalid"))); + } } } // namespace bert