From eaf131bd8f8c40ef5b4b63f2b61ef12fad37bb07 Mon Sep 17 00:00:00 2001 From: divyagayathri-hcl <159437886+divyagayathri-hcl@users.noreply.github.com> Date: Fri, 10 Jan 2025 21:17:08 +0000 Subject: [PATCH 1/2] [Thinkit] Update hard-coded dscp-to-queue mappings to match our configs, Configure switches sequentially with ConfigureSwitchPairAndReturnP4RuntimeSessionPair, Stop masking fixed bug, add helper methods for generating IPv6 and WCMP flows & Vlan tag test, gNMI port breakout tests. (#948) Co-authored-by: smolkaj --- tests/forwarding/l3_admit_test.cc | 10 +- .../lib/p4rt_fixed_table_programming_helper.h | 6 +- ...4rt_fixed_table_programming_helper_test.cc | 87 ++++++++- tests/lib/switch_test_setup_helpers.cc | 36 ++-- tests/qos/cpu_qos_test.cc | 179 ++++++++++++++++- tests/qos/qos_test_util.cc | 13 +- tests/thinkit_gnmi_interface_tests.cc | 17 +- tests/thinkit_gnmi_interface_util.cc | 27 +-- tests/thinkit_gnmi_interface_util_tests.cc | 181 ++++++------------ 9 files changed, 367 insertions(+), 189 deletions(-) diff --git a/tests/forwarding/l3_admit_test.cc b/tests/forwarding/l3_admit_test.cc index c3f65470c..9b50e4cd2 100644 --- a/tests/forwarding/l3_admit_test.cc +++ b/tests/forwarding/l3_admit_test.cc @@ -526,15 +526,7 @@ TEST_P(L3AdmitTestFixture, DISABLED_L3AdmitCanUseInPortToRestrictMacAddresses) { } LOG(INFO) << "Done collecting packets."; - if (GetMirrorTestbed().Environment().MaskKnownFailures()) { - // TODO: Reduce expected count by tolerance level. - const int kDropTolerance = 1; - int adjusted_good_packets = kNumberOfTestPacket - kDropTolerance; - EXPECT_GE(good_packet_count, adjusted_good_packets); - EXPECT_LE(good_packet_count, kNumberOfTestPacket); - } else { - EXPECT_EQ(good_packet_count, kNumberOfTestPacket); - } + EXPECT_EQ(good_packet_count, kNumberOfTestPacket); EXPECT_EQ(bad_packet_count, 0); } diff --git a/tests/lib/p4rt_fixed_table_programming_helper.h b/tests/lib/p4rt_fixed_table_programming_helper.h index 144dccebe..fd718ec0e 100644 --- a/tests/lib/p4rt_fixed_table_programming_helper.h +++ b/tests/lib/p4rt_fixed_table_programming_helper.h @@ -11,8 +11,8 @@ // 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 GOOGLE_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_ -#define GOOGLE_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_ +#ifndef PINS_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_ +#define PINS_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_ #include #include #include @@ -110,4 +110,4 @@ WcmpGroupTableUpdate(const pdpi::IrP4Info &ir_p4_info, } // namespace pins -#endif // GOOGLE_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_ +#endif // PINS_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_ diff --git a/tests/lib/p4rt_fixed_table_programming_helper_test.cc b/tests/lib/p4rt_fixed_table_programming_helper_test.cc index cfd40b63e..3685cbae5 100644 --- a/tests/lib/p4rt_fixed_table_programming_helper_test.cc +++ b/tests/lib/p4rt_fixed_table_programming_helper_test.cc @@ -13,6 +13,8 @@ // limitations under the License. #include "tests/lib/p4rt_fixed_table_programming_helper.h" +#include + #include "absl/status/status.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -27,6 +29,7 @@ namespace { using ::gutil::StatusIs; using ::testing::HasSubstr; +using ::testing::StrEq; MATCHER_P(HasExactMatch, value, "") { for (const auto& match_field : arg.entity().table_entry().match()) { @@ -145,7 +148,7 @@ TEST_P(L3RouteProgrammingTest, VrfTableAddFailsWithEmptyId) { StatusIs(absl::StatusCode::kInvalidArgument)); } -TEST_P(L3RouteProgrammingTest, Ipv4TableDoesNotRequireAnAction) { +TEST_P(L3RouteProgrammingTest, IpTableDoesNotRequireAnAction) { // The helper class will assume a default (e.g. drop). ASSERT_OK_AND_ASSIGN( p4::v1::Update pi_update, @@ -155,7 +158,7 @@ TEST_P(L3RouteProgrammingTest, Ipv4TableDoesNotRequireAnAction) { EXPECT_THAT(pi_update, HasExactMatch("vrf-0")); } -TEST_P(L3RouteProgrammingTest, Ipv4TableWithSetNexthopAction) { +TEST_P(L3RouteProgrammingTest, IpTableWithSetNexthopAction) { ASSERT_OK_AND_ASSIGN( p4::v1::Update pi_update, pins::Ipv4TableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT, @@ -171,7 +174,7 @@ TEST_P(L3RouteProgrammingTest, Ipv4TableWithSetNexthopAction) { EXPECT_THAT(pi_update, HasActionParam("nexthop-0")); } -TEST_P(L3RouteProgrammingTest, Ipv4TableEntryFailsWihInvalidParameters) { +TEST_P(L3RouteProgrammingTest, IpTableEntryFailsWihInvalidParameters) { EXPECT_THAT( pins::Ipv4TableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT, pins::IpTableOptions{ @@ -183,6 +186,26 @@ TEST_P(L3RouteProgrammingTest, Ipv4TableEntryFailsWihInvalidParameters) { HasSubstr("Expected 0 parameters"))); } +TEST_P(L3RouteProgrammingTest, Ipv4TableEntryCannotHaveIPv6Address) { + EXPECT_THAT(Ipv4TableUpdate(sai::GetIrP4Info(GetParam()), + p4::v1::Update::INSERT, + IpTableOptions{ + .dst_addr_lpm = std::make_pair("FE80::1", 32), + }), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("Invalid IPv4 address"))); +} + +TEST_P(L3RouteProgrammingTest, Ipv6TableEntryCannotHaveIPv4Address) { + EXPECT_THAT( + Ipv6TableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT, + IpTableOptions{ + .dst_addr_lpm = std::make_pair("10.1.1.1", 32), + }), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("invalid IPv6 address"))); +} + TEST_P(L3RouteProgrammingTest, L3AdmitTableWithoutInPort) { ASSERT_OK_AND_ASSIGN( p4::v1::Update pi_update, @@ -197,6 +220,13 @@ TEST_P(L3RouteProgrammingTest, L3AdmitTableWithoutInPort) { "\377\377\377\377\377\377")); } +TEST_P(L3RouteProgrammingTest, L3AdmitTableAllPacketsDoesNotSetAMatchKey) { + ASSERT_OK_AND_ASSIGN(p4::v1::Update pi_update, + L3AdmitAllTableUpdate(sai::GetIrP4Info(GetParam()), + p4::v1::Update::INSERT)); + EXPECT_TRUE(pi_update.entity().table_entry().match().empty()); +} + TEST_P(L3RouteProgrammingTest, L3AdmitTableWithInPort) { ASSERT_OK_AND_ASSIGN( p4::v1::Update pi_update, @@ -224,6 +254,57 @@ TEST_P(L3RouteProgrammingTest, L3AdmitTableMustSetPriority) { HasSubstr("require a positive non-zero priority"))); } +TEST_P(L3RouteProgrammingTest, WcmpGroupCanHaveMultipleNextHops) { + ASSERT_OK_AND_ASSIGN( + p4::v1::Update pi_update, + WcmpGroupTableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT, + /*wcmp_group_id=*/"group-1", + {WcmpAction{.nexthop_id = "nh-2", .weight = 1}, + WcmpAction{.nexthop_id = "nh-3", .weight = 2}})); + + EXPECT_THAT(pi_update, HasExactMatch("group-1")); + EXPECT_EQ(pi_update.entity() + .table_entry() + .action() + .action_profile_action_set() + .action_profile_actions_size(), + 2); +} + +TEST_P(L3RouteProgrammingTest, WcmpGroupActionCannotHaveWeightZero) { + EXPECT_THAT( + WcmpGroupTableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT, + /*wcmp_group_id=*/"group-1", + {WcmpAction{.nexthop_id = "nh-3", .weight = 0}}), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("Expected positive action set weight"))); +} + +TEST_P(L3RouteProgrammingTest, WcmpGroupActionCanSetWatchPort) { + ASSERT_OK_AND_ASSIGN(p4::v1::Update pi_update, + WcmpGroupTableUpdate(sai::GetIrP4Info(GetParam()), + p4::v1::Update::INSERT, + /*wcmp_group_id=*/"group-1", + {WcmpAction{ + .nexthop_id = "nh-3", + .weight = 2, + .watch_port = "1", + }})); + ASSERT_EQ(pi_update.entity() + .table_entry() + .action() + .action_profile_action_set() + .action_profile_actions_size(), + 1); + EXPECT_THAT(pi_update.entity() + .table_entry() + .action() + .action_profile_action_set() + .action_profile_actions(0) + .watch_port(), + StrEq("1")); +} + INSTANTIATE_TEST_SUITE_P( L3RouteProgrammingTestInstance, L3RouteProgrammingTest, testing::Values(sai::Instantiation::kMiddleblock, diff --git a/tests/lib/switch_test_setup_helpers.cc b/tests/lib/switch_test_setup_helpers.cc index fe5bb02b6..d8382fee4 100644 --- a/tests/lib/switch_test_setup_helpers.cc +++ b/tests/lib/switch_test_setup_helpers.cc @@ -2,6 +2,7 @@ #include #include +#include // NOLINT: third_party library. #include #include #include @@ -177,18 +178,29 @@ ConfigureSwitchPairAndReturnP4RuntimeSessionPair( std::optional gnmi_config, std::optional p4info, const pdpi::P4RuntimeSessionOptionalArgs& metadata) { - // TODO: Ideally, we would configure these in parallel instead. - ASSIGN_OR_RETURN(std::unique_ptr session1, - ConfigureSwitchAndReturnP4RuntimeSession( - thinkit_switch1, gnmi_config, p4info, metadata), - _.SetPrepend() << "failed to configure switch '" - << thinkit_switch1.ChassisName() << "': "); - ASSIGN_OR_RETURN(std::unique_ptr session2, - ConfigureSwitchAndReturnP4RuntimeSession( - thinkit_switch2, gnmi_config, p4info, metadata), - _.SetPrepend() << "failed to configure switch '" - << thinkit_switch2.ChassisName() << "': "); - return std::make_pair(std::move(session1), std::move(session2)); + // We configure both switches in parallel, since it may require rebooting the + // switch which is costly. + using T = absl::StatusOr>; + T session1, session2; + { + std::future future1 = std::async(std::launch::async, [&] { + return ConfigureSwitchAndReturnP4RuntimeSession( + thinkit_switch1, gnmi_config, p4info, metadata); + }); + std::future future2 = std::async(std::launch::async, [&] { + return ConfigureSwitchAndReturnP4RuntimeSession( + thinkit_switch2, gnmi_config, p4info, metadata); + }); + session1 = future1.get(); + session2 = future2.get(); + } + RETURN_IF_ERROR(session1.status()).SetPrepend() + << "failed to configure switch '" << thinkit_switch1.ChassisName() + << "': "; + RETURN_IF_ERROR(session2.status()).SetPrepend() + << "failed to configure switch '" << thinkit_switch2.ChassisName() + << "': "; + return std::make_pair(std::move(*session1), std::move(*session2)); } absl::Status MirrorSutP4rtPortIdConfigToControlSwitch( diff --git a/tests/qos/cpu_qos_test.cc b/tests/qos/cpu_qos_test.cc index ac2027820..f9f8256d0 100644 --- a/tests/qos/cpu_qos_test.cc +++ b/tests/qos/cpu_qos_test.cc @@ -333,9 +333,9 @@ absl::StatusOr PickSutToControlDeviceLinkThatsUp( thinkit::MirrorTestbed &testbed) { // TODO: Pick dynamically instead of hard-coding. return SutToControlLink{ - .sut_port_gnmi_name = "Ethernet0", + .sut_port_gnmi_name = "Ethernet1/1/1", .sut_port_p4rt_name = "1", - .control_device_port_gnmi_name = "Ethernet0", + .control_device_port_gnmi_name = "Ethernet1/1/1", .control_device_port_p4rt_name = "1", }; } @@ -718,6 +718,181 @@ TEST_P(CpuQosTestWithoutIxia, LOG(INFO) << "-- END OF TEST -----------------------------------------------"; } +// Purpose: Verify that VLAN tagged packets are received as expected. +TEST_P(CpuQosTestWithoutIxia, PuntToCpuWithVlanTag) { + LOG(INFO) << "-- START OF TEST ---------------------------------------------"; + + // Setup: the testbed consists of a SUT connected to a control device + // that allows us to send and receive packets to/from the SUT. + thinkit::Switch &sut = Testbed().Sut(); + thinkit::Switch &control_device = Testbed().ControlSwitch(); + const P4Info &p4info = GetParam().p4info; + ASSERT_OK_AND_ASSIGN(const pdpi::IrP4Info ir_p4info, + pdpi::CreateIrP4Info(p4info)); + + EXPECT_OK( + Testbed().Environment().StoreTestArtifact("p4info.textproto", p4info)); + std::unique_ptr sut_p4rt_session, + control_p4rt_session; + + ASSERT_OK_AND_ASSIGN( + std::tie(sut_p4rt_session, control_p4rt_session), + pins_test::ConfigureSwitchPairAndReturnP4RuntimeSessionPair( + sut, control_device, absl::nullopt, p4info)); + + // Pick a link to be used for packet injection. + ASSERT_OK_AND_ASSIGN(SutToControlLink link_used_for_test_packets, + PickSutToControlDeviceLinkThatsUp(Testbed())); + LOG(INFO) << "Link used to inject test packets: " + << link_used_for_test_packets; + std::vector test_packets; + // Test packet. + ASSERT_OK_AND_ASSIGN(packetlib::Packet ipv4_packet, + gutil::ParseTextProto(R"pb( + headers { + ethernet_header { + ethernet_destination: "00:01:02:02:02:02" + ethernet_source: "00:01:02:03:04:05" + ethertype: "0x8100" + } + } + headers { + vlan_header { + priority_code_point: "0x0" + drop_eligible_indicator: "0x1" + vlan_identifier: "0x123" + ethertype: "0x0800" + } + } + headers { + ipv4_header { + version: "0x4" + ihl: "0x5" + dscp: "0x00" + ecn: "0x0" + identification: "0xa3cd" + flags: "0x0" + fragment_offset: "0x0000" + ttl: "0x10" + protocol: "0x11" + ipv4_source: "10.0.0.2" + ipv4_destination: "10.0.0.3" + } + } + headers { + udp_header { source_port: "0x0000" destination_port: "0x0000" } + } + payload: "IPv4 test packet with VLAN tag" + )pb")); + + test_packets.push_back(ipv4_packet); + + ASSERT_OK_AND_ASSIGN(packetlib::Packet ipv6_packet, + gutil::ParseTextProto(R"pb( + headers { + ethernet_header { + ethernet_destination: "00:01:02:02:02:02" + ethernet_source: "00:01:02:03:04:05" + ethertype: "0x8100" + } + } + headers { + vlan_header { + priority_code_point: "0x0" + drop_eligible_indicator: "0x1" + vlan_identifier: "0x123" + ethertype: "0x86dd" + } + } + headers { + ipv6_header { + dscp: "0x00" + ecn: "0x0" + flow_label: "0x00000" + next_header: "0x11" + hop_limit: "0x40" + ipv6_source: "2001:db8:0:12::1" + ipv6_destination: "2001:db8:0:12::2" + } + } + headers { + udp_header { source_port: "0x0000" destination_port: "0x0000" } + } + payload: "IPv6 test packet with VLAN tag" + )pb")); + + test_packets.push_back(ipv6_packet); + + // Install ACL table entry to be hit with a test packet. + ASSERT_OK_AND_ASSIGN(const sai::TableEntry pd_acl_entry, + gutil::ParseTextProto(R"pb( + acl_ingress_table_entry { + priority: 1 + match { + dst_mac { value: "00:01:02:02:02:02" mask: "ff:ff:ff:ff:ff:ff" } + } + action { acl_trap { qos_queue: "0x3" } } + } + )pb")); + + ASSERT_OK_AND_ASSIGN( + const p4::v1::TableEntry pi_acl_entry, + pdpi::PartialPdTableEntryToPiTableEntry(ir_p4info, pd_acl_entry)); + ASSERT_OK(pdpi::InstallPiTableEntry(sut_p4rt_session.get(), pi_acl_entry)); + + struct VlanTestPacketCounters { + absl::Mutex mutex; + int num_vlan_packets_received ABSL_GUARDED_BY(mutex) = 0; + }; + + VlanTestPacketCounters packet_receive_info; + PacketInReceiver sut_packet_receiver( + *sut_p4rt_session, [&](const p4::v1::StreamMessageResponse pi_response) { + sai::StreamMessageResponse pd_response; + ASSERT_OK(pdpi::PiStreamMessageResponseToPd(ir_p4info, pi_response, + &pd_response)) + << " packet in PI to PD failed: " << pi_response.DebugString(); + ASSERT_TRUE(pd_response.has_packet()) + << " received unexpected stream message for packet in: " + << pd_response.DebugString(); + packetlib::Packet packet = + packetlib::ParsePacket(pd_response.packet().payload()); + EXPECT_EQ(packet.headers(1).vlan_header().vlan_identifier(), + ipv4_packet.headers(1).vlan_header().vlan_identifier()); + absl::MutexLock lock(&packet_receive_info.mutex); + packet_receive_info.num_vlan_packets_received++; + }); + + for (auto test_packet : test_packets) { + { + absl::MutexLock lock(&packet_receive_info.mutex); + packet_receive_info.num_vlan_packets_received = 0; + } + + ASSERT_OK_AND_ASSIGN(const std::string raw_packet, + packetlib::SerializePacket(test_packet)); + + const int kPacketCount = 10; + for (int iter = 0; iter < kPacketCount; iter++) { + ASSERT_OK(pins::InjectEgressPacket( + /*port=*/link_used_for_test_packets.control_device_port_p4rt_name, + /*packet=*/raw_packet, ir_p4info, control_p4rt_session.get(), + /*packet_delay=*/std::nullopt)); + } + + ASSERT_OK(pins::TryUpToNTimes(10, /*delay=*/absl::Seconds(1), [&] { + absl::MutexLock lock(&packet_receive_info.mutex); + if (packet_receive_info.num_vlan_packets_received == kPacketCount) { + return absl::OkStatus(); + } + return absl::InternalError(absl::StrCat( + "Received packets: ", packet_receive_info.num_vlan_packets_received, + "Expected packets", kPacketCount)); + })); + } + LOG(INFO) << "-- END OF TEST -----------------------------------------------"; +} + // Purpose: Verify DSCP-to-queue mapping for traffic to switch loopback IP. TEST_P(CpuQosTestWithoutIxia, TrafficToLoopbackIpGetsMappedToCorrectQueues) { LOG(INFO) << "-- START OF TEST ---------------------------------------------"; diff --git a/tests/qos/qos_test_util.cc b/tests/qos/qos_test_util.cc index 4e7fbc332..81728cea7 100644 --- a/tests/qos/qos_test_util.cc +++ b/tests/qos/qos_test_util.cc @@ -198,7 +198,7 @@ ParseIpv4DscpToQueueMapping(absl::string_view gnmi_config) { for (int dscp = 16; dscp <= 19; ++dscp) queue_by_dscp[dscp] = "AF2"; queue_by_dscp[21] = "LLQ2"; for (int dscp = 24; dscp <= 27; ++dscp) queue_by_dscp[dscp] = "AF3"; - for (int dscp = 32; dscp <= 35; ++dscp) queue_by_dscp[dscp] = "AF4"; + for (int dscp = 32; dscp <= 39; ++dscp) queue_by_dscp[dscp] = "AF4"; for (int dscp = 48; dscp <= 59; ++dscp) queue_by_dscp[dscp] = "NC1"; return queue_by_dscp; } @@ -212,16 +212,7 @@ ParseIpv6DscpToQueueMapping(absl::string_view gnmi_config) { absl::StatusOr> GetIpv4DscpToQueueMapping( absl::string_view port, gnmi::gNMI::StubInterface &gnmi_stub) { // TODO: Actually parse config -- hard-coded for now. - absl::flat_hash_map queue_by_dscp; - for (int dscp = 0; dscp < 64; ++dscp) queue_by_dscp[dscp] = "BE1"; - for (int dscp = 8; dscp <= 11; ++dscp) queue_by_dscp[dscp] = "AF1"; - queue_by_dscp[13] = "LLQ1"; - for (int dscp = 16; dscp <= 19; ++dscp) queue_by_dscp[dscp] = "AF2"; - queue_by_dscp[21] = "LLQ2"; - for (int dscp = 24; dscp <= 27; ++dscp) queue_by_dscp[dscp] = "AF3"; - for (int dscp = 32; dscp <= 35; ++dscp) queue_by_dscp[dscp] = "AF4"; - for (int dscp = 48; dscp <= 59; ++dscp) queue_by_dscp[dscp] = "NC1"; - return queue_by_dscp; + return ParseIpv4DscpToQueueMapping(/*gnmi_config=*/""); } absl::StatusOr> GetIpv6DscpToQueueMapping( diff --git a/tests/thinkit_gnmi_interface_tests.cc b/tests/thinkit_gnmi_interface_tests.cc index c430a1a61..7cfd587cb 100644 --- a/tests/thinkit_gnmi_interface_tests.cc +++ b/tests/thinkit_gnmi_interface_tests.cc @@ -75,14 +75,9 @@ void BreakoutDuringPortInUse(thinkit::Switch& sut, LOG(INFO) << "Using port " << port_info.port_name << " with current breakout mode " << port_info.curr_breakout_mode; // Verify that all ports for the selected port are operationally up. - auto resp_parse_str = "openconfig-interfaces:oper-status"; for (const auto& p : orig_breakout_info) { - auto if_state_path = absl::StrCat("interfaces/interface[name=", p.first, - "]/state/oper-status"); - ASSERT_OK_AND_ASSIGN( - auto state_path_response, - GetGnmiStatePathInfo(sut_gnmi_stub, if_state_path, resp_parse_str)); - EXPECT_THAT(state_path_response, HasSubstr(kStateUp)); + EXPECT_OK(pins_test::CheckInterfaceOperStateOverGnmi(*sut_gnmi_stub, + kStateUp, {p.first})); } // Determine port to install router interface on. @@ -176,11 +171,16 @@ void BreakoutDuringPortInUse(thinkit::Switch& sut, ASSERT_EQ(status.ok(), true); // Wait for breakout config to go through. - absl::SleepFor(absl::Seconds(30)); + // TODO: Investigate changing to polling loop. + absl::SleepFor(absl::Seconds(60)); // Verify that the config is successfully applied now. non_existing_port_list = GetNonExistingPortsAfterBreakout( orig_breakout_info, new_breakout_info, true); + // Oper-status is expected to be down as breakout is applied on one side only. + for (auto& port : new_breakout_info) { + port.second.oper_status = kStateDown; + } ASSERT_OK(ValidateBreakoutState(sut_gnmi_stub, new_breakout_info, non_existing_port_list)); @@ -194,6 +194,7 @@ void BreakoutDuringPortInUse(thinkit::Switch& sut, << port_info.port_name << " on DUT"; grpc::ClientContext context3; ASSERT_OK(sut_gnmi_stub->Set(&context3, req, &resp)); + // TODO: Investigate changing to polling loop. absl::SleepFor(absl::Seconds(60)); // Verify that the config is successfully applied. diff --git a/tests/thinkit_gnmi_interface_util.cc b/tests/thinkit_gnmi_interface_util.cc index bd1df62c5..98f84de0c 100644 --- a/tests/thinkit_gnmi_interface_util.cc +++ b/tests/thinkit_gnmi_interface_util.cc @@ -725,28 +725,17 @@ absl::Status ValidateBreakoutState( for (const auto& [port_name, breakout_info] : expected_port_info) { // Verify that the oper-status state path value is as expected. + auto expected_oper_status = breakout_info.oper_status; + StripSymbolFromString(expected_oper_status, '\"'); + RETURN_IF_ERROR(pins_test::CheckInterfaceOperStateOverGnmi( + *sut_gnmi_stub, expected_oper_status, {port_name})); + // Verify that the physical-channels state path value is as expected. auto interface_state_path = absl::StrCat( - "interfaces/interface[name=", port_name, "]/state/oper-status"); - auto response_parse_str = "openconfig-interfaces:oper-status"; + "interfaces/interface[name=", port_name, "]/state/physical-channel"); + auto response_parse_str = + "openconfig-platform-transceiver:physical-channel"; ASSIGN_OR_RETURN( auto state_path_response, - GetGnmiStatePathInfo(sut_gnmi_stub, interface_state_path, - response_parse_str), - _ << "Failed to get GNMI state path value for oper-status for " - "port " - << port_name); - if (!absl::StrContains(state_path_response, breakout_info.oper_status)) { - return gutil::InternalErrorBuilder().LogError() - << absl::StrCat("Port oper-status match failed for port ", - port_name, ". got: ", state_path_response, - ", want:", breakout_info.oper_status); - } - // Verify that the physical-channels state path value is as expected. - interface_state_path = absl::StrCat("interfaces/interface[name=", port_name, - "]/state/physical-channel"); - response_parse_str = "openconfig-platform-transceiver:physical-channel"; - ASSIGN_OR_RETURN( - state_path_response, GetGnmiStatePathInfo(sut_gnmi_stub, interface_state_path, response_parse_str), _ << "Failed to get GNMI state path value for physical-channels for " diff --git a/tests/thinkit_gnmi_interface_util_tests.cc b/tests/thinkit_gnmi_interface_util_tests.cc index c666cb8a7..514757e33 100644 --- a/tests/thinkit_gnmi_interface_util_tests.cc +++ b/tests/thinkit_gnmi_interface_util_tests.cc @@ -1790,53 +1790,25 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, expected_port_info["Ethernet1/1/1"] = pins_test::PortBreakoutInfo{"[0,1,2,3,4,5,6,7]", pins_test::kStateUp}; std::vector non_existing_port_list; - gnmi::GetRequest oper_status_req; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - R"pb(prefix { origin: "openconfig" } - path { - elem { name: "interfaces" } - elem { - name: "interface" - key { key: "name" value: "Ethernet1/1/1" } - } - elem { name: "state" } - elem { name: "oper-status" } - } - type: STATE)pb", - &oper_status_req)); - gnmi::GetResponse oper_status_resp; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - R"pb(notification { - timestamp: 1632102697699213032 - prefix { origin: "openconfig" } - update { - path { - elem { name: "interfaces" } - elem { - name: "interface" - key { key: "name" value: "Ethernet1/1/1" } - } - elem { name: "state" } - elem { name: "oper-status" } - } - val { - json_ietf_val: "{\"openconfig-interfaces:oper-status\":\"DOWN\"}" - } - } - })pb", - &oper_status_resp)); - EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(oper_status_req), _)) - .WillOnce( - DoAll(SetArgPointee<2>(oper_status_resp), Return(grpc::Status::OK))); + EXPECT_CALL(*mock_gnmi_stub_ptr, Get) + .WillOnce(DoAll( + SetArgPointee<2>(gutil::ParseProtoOrDie( + R"pb(notification { + timestamp: 1620348032128305716 + prefix { origin: "openconfig" } + update { + path { elem { name: "interfaces" } } + val { + json_ietf_val: "{\"openconfig-interfaces:interfaces\":{\"interface\":[{\"name\":\"Ethernet1/1/1\",\"state\":{\"oper-status\":\"DOWN\"}}]}}" + } + } + })pb")), + Return(grpc::Status::OK))); EXPECT_THAT( pins_test::ValidateBreakoutState( mock_gnmi_stub_ptr.get(), expected_port_info, non_existing_port_list), - StatusIs( - absl::StatusCode::kInternal, - HasSubstr(absl::StrCat( - "Port oper-status match failed for port Ethernet1/1/1. got: \"", - pins_test::kStateDown, - "\", want:", expected_port_info["Ethernet1/1/1"].oper_status)))); + StatusIs(absl::StatusCode::kUnavailable, + HasSubstr("Some interfaces are not in the expected state UP"))); } TEST_F(GNMIThinkitInterfaceUtilityTest, @@ -1847,41 +1819,6 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, expected_port_info["Ethernet1/1/1"] = pins_test::PortBreakoutInfo{"[0,1,2,3]", pins_test::kStateUp}; std::vector non_existing_port_list; - gnmi::GetRequest oper_status_req; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - R"pb(prefix { origin: "openconfig" } - path { - elem { name: "interfaces" } - elem { - name: "interface" - key { key: "name" value: "Ethernet1/1/1" } - } - elem { name: "state" } - elem { name: "oper-status" } - } - type: STATE)pb", - &oper_status_req)); - gnmi::GetResponse oper_status_resp; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - R"pb(notification { - timestamp: 1632102697699213032 - prefix { origin: "openconfig" } - update { - path { - elem { name: "interfaces" } - elem { - name: "interface" - key { key: "name" value: "Ethernet1/1/1" } - } - elem { name: "state" } - elem { name: "oper-status" } - } - val { - json_ietf_val: "{\"openconfig-interfaces:oper-status\":\"UP\"}" - } - } - })pb", - &oper_status_resp)); gnmi::GetRequest physical_channels_req; ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( R"pb(prefix { origin: "openconfig" } @@ -1917,9 +1854,20 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, } })pb", &physical_channels_resp)); - EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(oper_status_req), _)) - .WillOnce( - DoAll(SetArgPointee<2>(oper_status_resp), Return(grpc::Status::OK))); + EXPECT_CALL(*mock_gnmi_stub_ptr, Get) + .WillOnce(DoAll( + SetArgPointee<2>(gutil::ParseProtoOrDie( + R"pb(notification { + timestamp: 1620348032128305716 + prefix { origin: "openconfig" } + update { + path { elem { name: "interfaces" } } + val { + json_ietf_val: "{\"openconfig-interfaces:interfaces\":{\"interface\":[{\"name\":\"Ethernet1/1/1\",\"state\":{\"oper-status\":\"UP\"}}]}}" + } + } + })pb")), + Return(grpc::Status::OK))); EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(physical_channels_req), _)) .WillOnce(DoAll(SetArgPointee<2>(physical_channels_resp), @@ -2012,9 +1960,22 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, } })pb", &physical_channels_resp)); + EXPECT_CALL(*mock_gnmi_stub_ptr, Get) + .WillOnce(DoAll( + SetArgPointee<2>(gutil::ParseProtoOrDie( + R"pb(notification { + timestamp: 1620348032128305716 + prefix { origin: "openconfig" } + update { + path { elem { name: "interfaces" } } + val { + json_ietf_val: "{\"openconfig-interfaces:interfaces\":{\"interface\":[{\"name\":\"Ethernet1/1/1\",\"state\":{\"oper-status\":\"UP\"}}]}}" + } + } + })pb")), + Return(grpc::Status::OK))); EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(oper_status_req), _)) - .Times(2) - .WillRepeatedly( + .WillOnce( DoAll(SetArgPointee<2>(oper_status_resp), Return(grpc::Status::OK))); EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(physical_channels_req), _)) @@ -2035,41 +1996,6 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, TestValidateBreakoutStateSuccess) { expected_port_info["Ethernet1/1/1"] = pins_test::PortBreakoutInfo{"[0,1,2,3,4,5,6,7]", pins_test::kStateUp}; std::vector non_existing_port_list{}; - gnmi::GetRequest oper_status_req; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - R"pb(prefix { origin: "openconfig" } - path { - elem { name: "interfaces" } - elem { - name: "interface" - key { key: "name" value: "Ethernet1/1/1" } - } - elem { name: "state" } - elem { name: "oper-status" } - } - type: STATE)pb", - &oper_status_req)); - gnmi::GetResponse oper_status_resp; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - R"pb(notification { - timestamp: 1632102697699213032 - prefix { origin: "openconfig" } - update { - path { - elem { name: "interfaces" } - elem { - name: "interface" - key { key: "name" value: "Ethernet1/1/1" } - } - elem { name: "state" } - elem { name: "oper-status" } - } - val { - json_ietf_val: "{\"openconfig-interfaces:oper-status\":\"UP\"}" - } - } - })pb", - &oper_status_resp)); gnmi::GetRequest physical_channels_req; ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( R"pb(prefix { origin: "openconfig" } @@ -2105,9 +2031,20 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, TestValidateBreakoutStateSuccess) { } })pb", &physical_channels_resp)); - EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(oper_status_req), _)) - .WillOnce( - DoAll(SetArgPointee<2>(oper_status_resp), Return(grpc::Status::OK))); + EXPECT_CALL(*mock_gnmi_stub_ptr, Get) + .WillOnce(DoAll( + SetArgPointee<2>(gutil::ParseProtoOrDie( + R"pb(notification { + timestamp: 1620348032128305716 + prefix { origin: "openconfig" } + update { + path { elem { name: "interfaces" } } + val { + json_ietf_val: "{\"openconfig-interfaces:interfaces\":{\"interface\":[{\"name\":\"Ethernet1/1/1\",\"state\":{\"oper-status\":\"UP\"}}]}}" + } + } + })pb")), + Return(grpc::Status::OK))); EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(physical_channels_req), _)) .WillOnce(DoAll(SetArgPointee<2>(physical_channels_resp), From c4691eb96fe10976b7f4137482139ca4548eeb55 Mon Sep 17 00:00:00 2001 From: divyagayathri-hcl <159437886+divyagayathri-hcl@users.noreply.github.com> Date: Fri, 10 Jan 2025 21:18:11 +0000 Subject: [PATCH 2/2] [Thinkit] Improve factory reset tests, gNMI breakout tests, Update Finish to read all of the messages in the stream channel before Finishing it, Utility library cleanup & Add support to get counters. (#949) * [Thinkit] Update hard-coded dscp-to-queue mappings to match our configs, Configure switches sequentially with ConfigureSwitchPairAndReturnP4RuntimeSessionPair, Stop masking fixed bug, add helper methods for generating IPv6 and WCMP flows & Vlan tag test, gNMI port breakout tests. * [Thinkit] Improve factory reset tests, gNMI breakout tests, Update Finish to read all of the messages in the stream channel before Finishing it, Utility library cleanup & Add support to get counters. --------- Co-authored-by: smolkaj Co-authored-by: kishanps --- tests/gnmi/BUILD.bazel | 15 +- tests/gnmi/ethcounter_ixia_test.cc | 24 +- tests/gnoi/factory_reset_test.cc | 37 +- tests/lib/switch_test_setup_helpers.expected | 413 +----------------- tests/lib/switch_test_setup_helpers.h | 3 +- ...h_test_setup_helpers_golden_test_runner.cc | 28 +- tests/lib/switch_test_setup_helpers_test.cc | 25 +- tests/qos/qos_test_util.cc | 18 - tests/thinkit_gnmi_interface_tests.cc | 9 + tests/thinkit_gnmi_interface_util.cc | 4 + tests/thinkit_gnmi_interface_util_tests.cc | 21 + 11 files changed, 113 insertions(+), 484 deletions(-) diff --git a/tests/gnmi/BUILD.bazel b/tests/gnmi/BUILD.bazel index 2cc2e9b76..8343bc87f 100644 --- a/tests/gnmi/BUILD.bazel +++ b/tests/gnmi/BUILD.bazel @@ -51,23 +51,20 @@ cc_library( srcs = ["ethcounter_ixia_test.cc"], hdrs = ["ethcounter_ixia_test.h"], deps = [ - "//gutil:collections", - "//gutil:status", + "//gutil:collections", "//gutil:status_matchers", "//gutil:testing", "//lib:ixia_helper", "//lib/gnmi:gnmi_helper", - "//lib/p4rt:packet_listener", - "//lib/validator:validator_lib", "//p4_pdpi:ir", "//p4_pdpi:p4_runtime_session", "//p4_pdpi:pd", - "//p4_pdpi/netaddr:mac_address", - "//p4_pdpi/packetlib", + "//p4_pdpi/netaddr:mac_address", "//p4_pdpi/packetlib:packetlib_cc_proto", + "//sai_p4/instantiations/google:sai_pd_cc_proto", + "//sai_p4/instantiations/google:sai_p4info_cc", "//tests/forwarding:util", "//tests/lib:switch_test_setup_helpers", - "//sai_p4/instantiations/google:sai_p4info_cc", "//thinkit:generic_testbed", "//thinkit:generic_testbed_fixture", "//thinkit:switch", @@ -77,9 +74,11 @@ cc_library( "@com_google_absl//absl/cleanup", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/status", + "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_absl//absl/time", - "@com_google_googletest//:gtest", + "@com_google_absl//absl/types:optional", + "@com_google_googletest//:gtest", ], alwayslink = True, ) diff --git a/tests/gnmi/ethcounter_ixia_test.cc b/tests/gnmi/ethcounter_ixia_test.cc index bed6ec232..595f2fcb7 100644 --- a/tests/gnmi/ethcounter_ixia_test.cc +++ b/tests/gnmi/ethcounter_ixia_test.cc @@ -19,24 +19,30 @@ // (this file) // // TODO: Resolve whole save/restore thing wrt speed? -// + #include "tests/gnmi/ethcounter_ixia_test.h" +#include +#include +#include #include +#include +#include #include "absl/cleanup/cleanup.h" #include "absl/container/flat_hash_map.h" #include "absl/status/status.h" -#include "absl/strings/escaping.h" +#include "absl/status/statusor.h" #include "absl/strings/match.h" +#include "absl/strings/numbers.h" #include "absl/strings/str_cat.h" +#include "absl/strings/str_split.h" #include "absl/strings/string_view.h" #include "absl/strings/substitute.h" #include "absl/time/clock.h" #include "absl/time/time.h" +#include "absl/types/optional.h" #include "glog/logging.h" -#include "gmock/gmock.h" -#include "gtest/gtest.h" #include "gutil/collections.h" #include "gutil/status.h" #include "gutil/status_matchers.h" @@ -44,20 +50,20 @@ #include "include/nlohmann/json.hpp" #include "lib/gnmi/gnmi_helper.h" #include "lib/ixia_helper.h" -#include "lib/p4rt/packet_listener.h" -#include "lib/validator/validator_lib.h" #include "p4_pdpi/ir.h" #include "p4_pdpi/netaddr/mac_address.h" #include "p4_pdpi/p4_runtime_session.h" -#include "p4_pdpi/packetlib/packetlib.h" #include "p4_pdpi/packetlib/packetlib.pb.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/util.h" #include "tests/lib/switch_test_setup_helpers.h" -#include "sai_p4/instantiations/google/sai_p4info.h" #include "thinkit/generic_testbed.h" #include "thinkit/proto/generic_testbed.pb.h" #include "thinkit/switch.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" namespace pins_test { @@ -395,6 +401,8 @@ absl::Status SetPortSpeed(absl::string_view port_speed, absl::string_view iface, return absl::OkStatus(); } +// TODO: Refactor to use gnmi_helper for stats. + absl::StatusOr GetGnmiStat(std::string stat_name, absl::string_view iface, gnmi::gNMI::StubInterface *gnmi_stub) { diff --git a/tests/gnoi/factory_reset_test.cc b/tests/gnoi/factory_reset_test.cc index a0b5b0e9a..77ed72aaa 100644 --- a/tests/gnoi/factory_reset_test.cc +++ b/tests/gnoi/factory_reset_test.cc @@ -38,6 +38,9 @@ namespace factory_reset { constexpr absl::Duration kFactoryResetWaitForDownTime = absl::Seconds(60); +constexpr absl::Duration kPingReachabilityInterval = absl::Seconds(2); +constexpr absl::Duration kPingTimeout = absl::Seconds(1); +constexpr int kConsecutivePingsRequired = 3; constexpr absl::Duration kFactoryResetWaitForUpTime = absl::Minutes(25); constexpr absl::Duration kSshSessionTimeout = absl::Seconds(5); @@ -45,11 +48,16 @@ void IssueGnoiFactoryResetAndValidateStatus( thinkit::Switch& sut, const gnoi::factory_reset::StartRequest& request, gnoi::factory_reset::StartResponse* response, grpc::Status expected_status) { + LOG(INFO) << "Issuing factory reset with parameters: " + << request.DebugString(); ASSERT_OK_AND_ASSIGN(auto sut_gnoi_factory_reset_stub, sut.CreateGnoiFactoryResetStub()); grpc::ClientContext context; grpc::Status status = sut_gnoi_factory_reset_stub->Start(&context, request, response); + LOG(INFO) << "Factory reset status: " << status.error_code() << ", " + << status.error_message(); + LOG(INFO) << "Factory reset response: " << response->DebugString(); if (expected_status.ok()) { EXPECT_OK(status); } else { @@ -66,11 +74,15 @@ void ValidateStackState(thinkit::Switch& sut, // Wait for system to become unreachable via ping - as that's the last thing // that goes down. + LOG(INFO) << "Starting polling for unreachability, now: " << absl::Now() + << " deadline: " << start_time + kFactoryResetWaitForDownTime; while (absl::Now() < (start_time + kFactoryResetWaitForDownTime)) { - if (!pins_test::Pingable(sut).ok()) { + if (!pins_test::Pingable(sut, kPingTimeout).ok()) { system_down = true; break; } + LOG(INFO) << "System still reachable at " << absl::Now() << " sleeping"; + absl::SleepFor(kPingReachabilityInterval); } // Return failure if system did not go down. ASSERT_TRUE(system_down) << "System did not go down in " @@ -82,6 +94,8 @@ void ValidateStackState(thinkit::Switch& sut, system_down = false; break; } + LOG(INFO) << "System still unreachable, sleeping"; + absl::SleepFor(kPingReachabilityInterval); } // Return failure if system did not come up. ASSERT_FALSE(system_down) @@ -120,9 +134,23 @@ void TestGnoiFactoryResetGnoiServerUnreachableFail( IssueGnoiFactoryResetAndValidateStatus(sut, request, &response); EXPECT_TRUE(response.has_reset_success()); - absl::SleepFor(kFactoryResetWaitForDownTime); - ASSERT_TRUE(!pins_test::Pingable(sut).ok()) - << "System did not go down in " << kFactoryResetWaitForDownTime; + LOG(INFO) << "Waiting for device to become unreachable"; + absl::Time start = absl::Now(); + int consecutive_unreachable_count = 0; + while (consecutive_unreachable_count < kConsecutivePingsRequired) { + if (pins_test::Pingable(sut, kPingTimeout).ok()) { + consecutive_unreachable_count = 0; + } else { + consecutive_unreachable_count++; + } + if (absl::Now() - start > kFactoryResetWaitForDownTime) { + FAIL() << "System did not go down in " << kFactoryResetWaitForDownTime; + } + LOG(INFO) << "System unreachable for " << consecutive_unreachable_count + << " consecutive pings"; + absl::SleepFor(kPingReachabilityInterval); + } + LOG(INFO) << "Device became unreachable after: " << absl::Now() - start; // Wait until the switch goes down, send another request and expect a failure // due to gNOI server unreachable. @@ -131,6 +159,7 @@ void TestGnoiFactoryResetGnoiServerUnreachableFail( grpc::Status(grpc::StatusCode::UNAVAILABLE, "failed to connect to all addresses")); + LOG(INFO) << "Waiting for device to become reachable"; absl::SleepFor(kFactoryResetWaitForUpTime); // Return failure if system did not come up. ASSERT_OK(pins_test::SwitchReady(sut, interfaces)) diff --git a/tests/lib/switch_test_setup_helpers.expected b/tests/lib/switch_test_setup_helpers.expected index 75753c17e..2ade910ad 100644 --- a/tests/lib/switch_test_setup_helpers.expected +++ b/tests/lib/switch_test_setup_helpers.expected @@ -317,412 +317,7 @@ action { -- MODIFICATIONS --------------------------------------------------------------- NONE -================================================================================ -Roughly even number of 1, 2, and 'a_port', with existing 'a_port' and port-less entries unchanged -Available ports: 1,2,a_port -================================================================================ --- ORIGINAL ENTRY -------------------------------------------------------------- -table_name: "neighbor_table" -matches { - name: "router_interface_id" - exact { - str: "router-interface-1" - } -} -matches { - name: "neighbor_id" - exact { - str: "fe80::3e28:6dff:fe34:c002" - } -} -action { - name: "set_dst_mac" - params { - name: "dst_mac" - value { - mac: "3c:28:6d:34:c0:02" - } - } -} --- MODIFICATIONS --------------------------------------------------------------- -NONE - --- ORIGINAL ENTRY -------------------------------------------------------------- -table_name: "router_interface_table" -matches { - name: "router_interface_id" - exact { - str: "router-interface-1" - } -} -action { - name: "set_port_and_src_mac" - params { - name: "port" - value { - str: "a_port" - } - } - params { - name: "src_mac" - value { - mac: "00:02:03:04:05:06" - } - } -} --- MODIFICATIONS --------------------------------------------------------------- -NONE - --- ORIGINAL ENTRY -------------------------------------------------------------- -table_name: "wcmp_group_table" -matches { - name: "wcmp_group_id" - exact { - str: "group-4294934545" - } -} -action_set { - actions { - action { - name: "set_nexthop_id" - params { - name: "nexthop_id" - value { - str: "nexthop-1" - } - } - } - weight: 1 - watch_port: "4" - } - actions { - action { - name: "set_nexthop_id" - params { - name: "nexthop_id" - value { - str: "nexthop-2" - } - } - } - weight: 1 - watch_port: "5" - } -} --- MODIFICATIONS --------------------------------------------------------------- -modified: action_set.actions[0].watch_port: "4" -> "a_port" -modified: action_set.actions[1].watch_port: "5" -> "2" - --- ORIGINAL ENTRY -------------------------------------------------------------- -table_name: "mirror_session_table" -matches { - name: "mirror_session_id" - exact { - str: "mirror-session-201326593" - } -} -action { - name: "mirror_as_ipv4_erspan" - params { - name: "port" - value { - str: "6" - } - } - params { - name: "src_ip" - value { - ipv4: "10.206.196.0" - } - } - params { - name: "dst_ip" - value { - ipv4: "172.20.0.202" - } - } - params { - name: "src_mac" - value { - mac: "00:02:03:04:05:06" - } - } - params { - name: "dst_mac" - value { - mac: "00:1a:11:17:5f:80" - } - } - params { - name: "ttl" - value { - hex_str: "0x40" - } - } - params { - name: "tos" - value { - hex_str: "0x00" - } - } -} --- MODIFICATIONS --------------------------------------------------------------- -modified: action.params[0].value.str: "6" -> "1" - --- ORIGINAL ENTRY -------------------------------------------------------------- -table_name: "l3_admit_table" -matches { - name: "dst_mac" - ternary { - value { - mac: "02:32:0a:ce:c4:04" - } - mask { - mac: "ff:ff:ff:ff:ff:ff" - } - } -} -matches { - name: "in_port" - optional { - value { - str: "7" - } - } -} -priority: 2030 -action { - name: "admit_to_l3" -} --- MODIFICATIONS --------------------------------------------------------------- -modified: matches[1].optional.value.str: "7" -> "a_port" - --- ORIGINAL ENTRY -------------------------------------------------------------- -table_name: "acl_ingress_table" -matches { - name: "is_ipv4" - optional { - value { - hex_str: "0x1" - } - } -} -matches { - name: "ip_protocol" - ternary { - value { - hex_str: "0x11" - } - mask { - hex_str: "0xff" - } - } -} -matches { - name: "l4_dst_port" - ternary { - value { - hex_str: "0x0223" - } - mask { - hex_str: "0xffff" - } - } -} -matches { - name: "in_port" - optional { - value { - str: "8" - } - } -} -priority: 3100 -action { - name: "acl_drop" -} --- MODIFICATIONS --------------------------------------------------------------- -modified: matches[3].optional.value.str: "8" -> "2" - --- ORIGINAL ENTRY -------------------------------------------------------------- -table_name: "acl_pre_ingress_table" -matches { - name: "in_port" - optional { - value { - str: "9" - } - } -} -priority: 1151 -action { - name: "set_vrf" - params { - name: "vrf_id" - value { - str: "vrf-210" - } - } -} --- MODIFICATIONS --------------------------------------------------------------- -modified: matches[0].optional.value.str: "9" -> "1" - --- ORIGINAL ENTRY -------------------------------------------------------------- -table_name: "acl_egress_table" -matches { - name: "ether_type" - ternary { - value { - hex_str: "0x0800" - } - mask { - hex_str: "" - } - } -} -matches { - name: "ip_protocol" - ternary { - value { - hex_str: "0x11" - } - mask { - hex_str: "0xff" - } - } -} -matches { - name: "l4_dst_port" - ternary { - value { - hex_str: "0x0223" - } - mask { - hex_str: "0xffff" - } - } -} -matches { - name: "out_port" - optional { - value { - str: "10" - } - } -} -priority: 3101 -action { - name: "acl_drop" -} --- MODIFICATIONS --------------------------------------------------------------- -modified: matches[3].optional.value.str: "10" -> "2" - --- ORIGINAL ENTRY -------------------------------------------------------------- -table_name: "wcmp_group_table" -matches { - name: "wcmp_group_id" - exact { - str: "group-4294934545" - } -} -action_set { - actions { - action { - name: "set_nexthop_id" - params { - name: "nexthop_id" - value { - str: "nexthop-1" - } - } - } - weight: 1 - watch_port: "11" - } - actions { - action { - name: "set_nexthop_id" - params { - name: "nexthop_id" - value { - str: "nexthop-2" - } - } - } - weight: 1 - watch_port: "12" - } - actions { - action { - name: "set_nexthop_id" - params { - name: "nexthop_id" - value { - str: "nexthop-2" - } - } - } - weight: 1 - watch_port: "13" - } - actions { - action { - name: "set_nexthop_id" - params { - name: "nexthop_id" - value { - str: "nexthop-2" - } - } - } - weight: 1 - watch_port: "14" - } - actions { - action { - name: "set_nexthop_id" - params { - name: "nexthop_id" - value { - str: "nexthop-2" - } - } - } - weight: 1 - watch_port: "15" - } - actions { - action { - name: "set_nexthop_id" - params { - name: "nexthop_id" - value { - str: "nexthop-2" - } - } - } - weight: 1 - watch_port: "16" - } - actions { - action { - name: "set_nexthop_id" - params { - name: "nexthop_id" - value { - str: "nexthop-2" - } - } - } - weight: 1 - watch_port: "17" - } -} --- MODIFICATIONS --------------------------------------------------------------- -modified: action_set.actions[0].watch_port: "11" -> "1" -modified: action_set.actions[1].watch_port: "12" -> "a_port" -modified: action_set.actions[2].watch_port: "13" -> "2" -modified: action_set.actions[3].watch_port: "14" -> "1" -modified: action_set.actions[4].watch_port: "15" -> "a_port" -modified: action_set.actions[5].watch_port: "16" -> "2" -modified: action_set.actions[6].watch_port: "17" -> "1" +tests/lib/switch_test_setup_helpers_golden_test_runner.cc:87: Failure +Value of: pins_test::RewritePortsInTableEntries( test_case.info, test_case.new_ports_available, entries_to_rewrite) +Expected: is OK + Actual: NOT_FOUND: Key not found: 'mirror_with_psamp_encapsulation' (of type absl::lts_20230802::Status) diff --git a/tests/lib/switch_test_setup_helpers.h b/tests/lib/switch_test_setup_helpers.h index 78290022d..426459ce2 100644 --- a/tests/lib/switch_test_setup_helpers.h +++ b/tests/lib/switch_test_setup_helpers.h @@ -68,8 +68,7 @@ absl::Status WaitForEnabledInterfacesToBeUp( std::function> on_failure = std::nullopt); -// Gets a count of which P4runtime ports are used in `entries` and how -// frequently. +// Gets the set of P4 Runtime port IDs used in `entries`. absl::StatusOr> GetPortsUsed(const pdpi::IrP4Info &info, std::vector entries); diff --git a/tests/lib/switch_test_setup_helpers_golden_test_runner.cc b/tests/lib/switch_test_setup_helpers_golden_test_runner.cc index b12bf806e..358e0972e 100644 --- a/tests/lib/switch_test_setup_helpers_golden_test_runner.cc +++ b/tests/lib/switch_test_setup_helpers_golden_test_runner.cc @@ -438,35 +438,11 @@ int main(int argc, char** argv) { exact { str: "mirror-session-201326593" } } action { - name: "mirror_as_ipv4_erspan" + name: "mirror_with_psamp_encapsulation" params { - name: "port" + name: "monitor_port" value { str: "6" } } - params { - name: "src_ip" - value { ipv4: "10.206.196.0" } - } - params { - name: "dst_ip" - value { ipv4: "172.20.0.202" } - } - params { - name: "src_mac" - value { mac: "00:02:03:04:05:06" } - } - params { - name: "dst_mac" - value { mac: "00:1a:11:17:5f:80" } - } - params { - name: "ttl" - value { hex_str: "0x40" } - } - params { - name: "tos" - value { hex_str: "0x00" } - } } )pb", R"pb( diff --git a/tests/lib/switch_test_setup_helpers_test.cc b/tests/lib/switch_test_setup_helpers_test.cc index 23182badf..502e114c5 100644 --- a/tests/lib/switch_test_setup_helpers_test.cc +++ b/tests/lib/switch_test_setup_helpers_test.cc @@ -107,14 +107,16 @@ void SetNextReadResponse(p4::v1::MockP4RuntimeStub& mock_p4rt_stub, // Mocks a P4RuntimeSession::Create call with a stub by constructing a // ReaderWriter mock stream and mocking an arbitration handshake. This function // does not perform any of these operations, it only sets up expectations. -void MockP4RuntimeSessionCreate( - p4::v1::MockP4RuntimeStub& stub, - const pdpi::P4RuntimeSessionOptionalArgs& metadata) { +[[nodiscard]] grpc::testing::MockClientReaderWriter< + p4::v1::StreamMessageRequest, p4::v1::StreamMessageResponse> & +MockP4RuntimeSessionCreateAndReturnStreamChannel( + p4::v1::MockP4RuntimeStub &stub, + const pdpi::P4RuntimeSessionOptionalArgs &metadata) { // The ReaderWriter stream constructed from the stub. This needs to be - // malloced as it is automatically freed when the unique pointer that it will - // be wrapped in is freed. The stream is wrapped in StreamChannel, which is - // the method of the stub that calls StreamChannelRaw, but is not itself - // mocked. + // malloced as it is automatically freed when the unique pointer that it + // will be wrapped in is freed. The stream is wrapped in StreamChannel, + // which is the method of the stub that calls StreamChannelRaw, but is not + // itself mocked. auto* stream = new NiceMock>(); EXPECT_CALL(stub, StreamChannelRaw).WillOnce(Return(stream)); @@ -135,6 +137,7 @@ void MockP4RuntimeSessionCreate( master_arbitration_update; return true; }); + return *stream; } // Constructs a table entry using the predefined table id, kTableId, and action @@ -222,8 +225,11 @@ void ExpectCallToClearTableEntries( EXPECT_CALL(mock_switch, CreateP4RuntimeStub()).WillOnce([=] { InSequence s; auto stub = std::make_unique<::p4::v1::MockP4RuntimeStub>(); - MockP4RuntimeSessionCreate(*stub, metadata); + auto &stream_channel = + MockP4RuntimeSessionCreateAndReturnStreamChannel(*stub, metadata); MockClearTableEntries(*stub, p4info, metadata); + // From the call to Finish. + EXPECT_CALL(stream_channel, Read).WillOnce(Return(false)); return stub; }); } @@ -309,7 +315,8 @@ void ExpectCallToCreateP4RuntimeSessionAndOptionallyPushP4Info( EXPECT_CALL(mock_switch, CreateP4RuntimeStub).WillOnce([=] { InSequence s; auto stub = absl::make_unique>(); - MockP4RuntimeSessionCreate(*stub, metadata); + // Ignore returned stream channel mock since we have no use for it. + (void)MockP4RuntimeSessionCreateAndReturnStreamChannel(*stub, metadata); if (p4info.has_value()) { // TODO: Test the path where `GetForwardingPipelineConfig` // returns a non-empty P4Info. diff --git a/tests/qos/qos_test_util.cc b/tests/qos/qos_test_util.cc index 81728cea7..67369046b 100644 --- a/tests/qos/qos_test_util.cc +++ b/tests/qos/qos_test_util.cc @@ -245,24 +245,6 @@ GetQueueToIpv6DscpsMapping(absl::string_view port, return dscps_by_queue; } -absl::Status SetPortLoopbackMode(bool port_loopback, - absl::string_view interface_name, - gnmi::gNMI::StubInterface &gnmi_stub) { - std::string config_path = absl::StrCat( - "interfaces/interface[name=", interface_name, "]/config/loopback-mode"); - std::string config_json; - if (port_loopback) { - config_json = "{\"openconfig-interfaces:loopback-mode\":true}"; - } else { - config_json = "{\"openconfig-interfaces:loopback-mode\":false}"; - } - - RETURN_IF_ERROR(pins_test::SetGnmiConfigPath( - &gnmi_stub, config_path, GnmiSetType::kUpdate, config_json)); - - return absl::OkStatus(); -} - absl::StatusOr GetQueueNameByDscpAndPort(int dscp, absl::string_view port, gnmi::gNMI::StubInterface &gnmi_stub) { diff --git a/tests/thinkit_gnmi_interface_tests.cc b/tests/thinkit_gnmi_interface_tests.cc index 7cfd587cb..a5a4b2768 100644 --- a/tests/thinkit_gnmi_interface_tests.cc +++ b/tests/thinkit_gnmi_interface_tests.cc @@ -180,6 +180,15 @@ void BreakoutDuringPortInUse(thinkit::Switch& sut, // Oper-status is expected to be down as breakout is applied on one side only. for (auto& port : new_breakout_info) { port.second.oper_status = kStateDown; + // For the logical ports that do not change on breakout, expect them to be + // operationally up. + auto it = orig_breakout_info.find(port.first); + if (it != orig_breakout_info.end()) { + if (it->second.physical_channels == port.second.physical_channels && + it->second.breakout_speed == port.second.breakout_speed) { + port.second.oper_status = kStateUp; + } + } } ASSERT_OK(ValidateBreakoutState(sut_gnmi_stub, new_breakout_info, non_existing_port_list)); diff --git a/tests/thinkit_gnmi_interface_util.cc b/tests/thinkit_gnmi_interface_util.cc index 98f84de0c..99e4a300f 100644 --- a/tests/thinkit_gnmi_interface_util.cc +++ b/tests/thinkit_gnmi_interface_util.cc @@ -414,6 +414,10 @@ GetExpectedPortInfoForBreakoutMode(const std::string& port, // Strip quotes from breakout mode string. StripSymbolFromString(mode, '\"'); std::vector values = absl::StrSplit(mode, 'x'); + if (values.size() != 2) { + return gutil::InternalErrorBuilder().LogError() + << "Invalid breakout mode found: " << mode; + } int num_breakouts; if (!absl::SimpleAtoi(values[0], &num_breakouts)) { return gutil::InternalErrorBuilder().LogError() diff --git a/tests/thinkit_gnmi_interface_util_tests.cc b/tests/thinkit_gnmi_interface_util_tests.cc index 514757e33..ec84a7c33 100644 --- a/tests/thinkit_gnmi_interface_util_tests.cc +++ b/tests/thinkit_gnmi_interface_util_tests.cc @@ -1141,6 +1141,7 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, ASSERT_OK(breakout_info.status()); EXPECT_EQ(breakout_info.value()["Ethernet1/1/1"].physical_channels, "[0,1,2,3,4,5,6,7]"); + EXPECT_EQ(breakout_info.value()["Ethernet1/1/1"].breakout_speed, "400G"); } TEST_F(GNMIThinkitInterfaceUtilityTest, @@ -1155,6 +1156,8 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, "[0,1,2,3]"); EXPECT_EQ(breakout_info.value()["Ethernet1/1/5"].physical_channels, "[4,5,6,7]"); + EXPECT_EQ(breakout_info.value()["Ethernet1/1/1"].breakout_speed, "200G"); + EXPECT_EQ(breakout_info.value()["Ethernet1/1/5"].breakout_speed, "200G"); } TEST_F(GNMIThinkitInterfaceUtilityTest, @@ -1169,6 +1172,9 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, "[0,1,2,3]"); EXPECT_EQ(breakout_info.value()["Ethernet1/1/5"].physical_channels, "[4,5]"); EXPECT_EQ(breakout_info.value()["Ethernet1/1/7"].physical_channels, "[6,7]"); + EXPECT_EQ(breakout_info.value()["Ethernet1/1/1"].breakout_speed, "200G"); + EXPECT_EQ(breakout_info.value()["Ethernet1/1/5"].breakout_speed, "100G"); + EXPECT_EQ(breakout_info.value()["Ethernet1/1/7"].breakout_speed, "100G"); } TEST_F( @@ -1183,6 +1189,9 @@ TEST_F( EXPECT_EQ(breakout_info.value()["Ethernet1/1/3"].physical_channels, "[2,3]"); EXPECT_EQ(breakout_info.value()["Ethernet1/1/5"].physical_channels, "[4,5,6,7]"); + EXPECT_EQ(breakout_info.value()["Ethernet1/1/1"].breakout_speed, "100G"); + EXPECT_EQ(breakout_info.value()["Ethernet1/1/3"].breakout_speed, "100G"); + EXPECT_EQ(breakout_info.value()["Ethernet1/1/5"].breakout_speed, "200G"); } TEST_F(GNMIThinkitInterfaceUtilityTest, @@ -1194,6 +1203,7 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, ASSERT_OK(breakout_info.status()); EXPECT_EQ(breakout_info.value()["Ethernet1/1/1"].physical_channels, "[0,1,2,3,4,5,6,7]"); + EXPECT_EQ(breakout_info.value()["Ethernet1/1/1"].breakout_speed, "400G"); } TEST_F(GNMIThinkitInterfaceUtilityTest, @@ -1218,6 +1228,17 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, HasSubstr("Failed to convert string (X) to integer"))); } +TEST_F(GNMIThinkitInterfaceUtilityTest, + TestGetExpectedPortInfoForBreakoutModeInvalidModeFailure) { + const std::string port = "Ethernet1/1/1"; + absl::string_view breakout_mode = "1"; + + EXPECT_THAT( + pins_test::GetExpectedPortInfoForBreakoutMode(port, breakout_mode), + StatusIs(absl::StatusCode::kInternal, + HasSubstr("Invalid breakout mode found: 1"))); +} + TEST_F(GNMIThinkitInterfaceUtilityTest, TestGetExpectedPortInfoForBreakoutModeNonParentPortFailure) { const std::string port = "Ethernet1/2/5";