From d6e719fc144187d2921b531bc97b302c2009c6ca Mon Sep 17 00:00:00 2001 From: kishanps Date: Tue, 29 Mar 2022 12:50:35 -0700 Subject: [PATCH 1/6] [Thinkit] Do not parameterize tests over gNMI config, Remove test access to sai::GetP4Info, Add some tolerance for ECN marked packet count, Log Ixia warning & Add Ixia and gNMI utilities needed for ECN testing. --- tests/qos/BUILD.bazel | 4 +- tests/qos/frontpanel_qos_test.cc | 37 ++++++++++++------ tests/qos/frontpanel_qos_test.h | 7 ++-- tests/qos/qos_test_util.cc | 66 ++++++++++++++++++++++++++++++++ tests/qos/qos_test_util.h | 16 ++++++++ 5 files changed, 113 insertions(+), 17 deletions(-) diff --git a/tests/qos/BUILD.bazel b/tests/qos/BUILD.bazel index da229159c..11cd574dc 100644 --- a/tests/qos/BUILD.bazel +++ b/tests/qos/BUILD.bazel @@ -94,6 +94,7 @@ cc_library( deps = [ ":packet_in_receiver", ":qos_test_util", + "//dvaas:test_vector_cc_proto", "//gutil:collections", "//gutil:status_matchers", "//gutil:testing", @@ -107,9 +108,8 @@ cc_library( "//p4_pdpi/netaddr:mac_address", "//p4_pdpi/packetlib", "//p4_pdpi/packetlib:packetlib_cc_proto", - "//sai_p4/instantiations/google:sai_p4info_cc", "//sai_p4/instantiations/google:sai_pd_cc_proto", - "//dvaas:test_vector_cc_proto", + "//tests/forwarding:util", "//tests/lib:switch_test_setup_helpers", "//thinkit:generic_testbed", "//thinkit:generic_testbed_fixture", diff --git a/tests/qos/frontpanel_qos_test.cc b/tests/qos/frontpanel_qos_test.cc index 36903ab75..15b0df15a 100644 --- a/tests/qos/frontpanel_qos_test.cc +++ b/tests/qos/frontpanel_qos_test.cc @@ -24,6 +24,7 @@ #include "absl/strings/substitute.h" #include "absl/time/time.h" +#include "dvaas/test_vector.pb.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include "gutil/collections.h" @@ -39,9 +40,8 @@ #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 "dvaas/test_vector.pb.h" +#include "tests/forwarding/util.h" #include "tests/lib/switch_test_setup_helpers.h" #include "tests/qos/packet_in_receiver.h" #include "tests/qos/qos_test_util.h" @@ -132,7 +132,7 @@ absl::Status SetUpForwardingAndCopyEgressToCpu( nexthop_table_entry { match { nexthop_id: "$2" } action { - set_nexthop { router_interface_id: "$0" neighbor_id: "$1" } + set_ip_nexthop { router_interface_id: "$0" neighbor_id: "$1" } } } )pb", @@ -335,7 +335,7 @@ TEST_P(FrontpanelQosTest, TestWredEcnMarking) { // Look up the port IDs used by P4RT for the SUT interfaces absl::flat_hash_map port_id_by_interface; ASSERT_OK_AND_ASSIGN(port_id_by_interface, - GetAllInterfaceNameToPortId(GetParam().gnmi_config)); + GetAllInterfaceNameToPortId(*gnmi_stub)); ASSERT_OK_AND_ASSIGN(const std::string kSutInPort1Id, gutil::FindOrStatus(port_id_by_interface, kSutInPort1)); @@ -538,10 +538,11 @@ TEST_P(FrontpanelQosTest, TestWredEcnMarking) { const QueueCounters queue_counters_before_test_packet, GetGnmiQueueCounters(/*port=*/kSutOutPort, /*queue=*/target_queue, *gnmi_stub)); - - ASSERT_OK(pins_test::ixia::StartTraffic(ixia_setup_result.traffic_refs, - ixia_setup_result.topology_ref, - *testbed)); + ASSERT_OK(pins::TryUpToNTimes(3, /*delay=*/absl::Seconds(1), [&] { + return pins_test::ixia::StartTraffic(ixia_setup_result.traffic_refs, + ixia_setup_result.topology_ref, + *testbed); + })); // Time to allow initial burst and to reach steady state queue usage. constexpr absl::Duration kCongestionTime = absl::Seconds(2); @@ -567,10 +568,22 @@ TEST_P(FrontpanelQosTest, TestWredEcnMarking) { LOG(INFO) << "ECN marked Packets: " << packet_receive_info.num_packets_ecn_marked; - if (is_congestion && is_ect) { - // All egress packets must be ECN marked. - ASSERT_EQ(packet_receive_info.num_packets_punted, - packet_receive_info.num_packets_ecn_marked); + if (is_congestion && is_ect) { + // TODO: Currently we are unable to keep queue usage + // constantly above threshold. The queue usage starts going down in + // a few seconds. Till we understand this, we will allow for + // tolerance in packets marked for ECN. + constexpr float kTolerancePercent = 20.0; + if (testbed->Environment().MaskKnownFailures()) { + // Egress packets must be ECN marked. Tolerance of 20%. + ASSERT_GE(packet_receive_info.num_packets_ecn_marked, + packet_receive_info.num_packets_punted * + (1 - kTolerancePercent / 100)); + } else { + // All egress packets must be ECN marked. + ASSERT_EQ(packet_receive_info.num_packets_punted, + packet_receive_info.num_packets_ecn_marked); + } } else { // No egress packets must be ECN marked. ASSERT_EQ(packet_receive_info.num_packets_ecn_marked, 0); diff --git a/tests/qos/frontpanel_qos_test.h b/tests/qos/frontpanel_qos_test.h index 4bed6d7d1..faa579bd9 100644 --- a/tests/qos/frontpanel_qos_test.h +++ b/tests/qos/frontpanel_qos_test.h @@ -28,10 +28,11 @@ namespace pins_test { -// Parameters used by the tests. +// Parameters used by the tests. The fixture is *not* parameterized over a gNMI +// config and assumes that the switch is preconfigured and the testbed ports +// are up. struct QosTestParams { - thinkit::GenericTestbedInterface *testbed_interface; - std::string gnmi_config; + thinkit::GenericTestbedInterface* testbed_interface; p4::config::v1::P4Info p4info; }; diff --git a/tests/qos/qos_test_util.cc b/tests/qos/qos_test_util.cc index 4543cad8d..1a68cfd12 100644 --- a/tests/qos/qos_test_util.cc +++ b/tests/qos/qos_test_util.cc @@ -1,5 +1,6 @@ #include "tests/qos/qos_test_util.h" +#include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "absl/strings/substitute.h" @@ -88,6 +89,32 @@ absl::Status SetPortSpeed(const std::string &port_speed, return absl::OkStatus(); } +absl::StatusOr GetPortSpeed(const std::string &interface_name, + gnmi::gNMI::StubInterface &gnmi_stub) { + // Map keyed on openconfig speed string to value in bits per second. + // http://ops.openconfig.net/branches/models/master/docs/openconfig-interfaces.html#mod-openconfig-if-ethernet + const auto kPortSpeedTable = + absl::flat_hash_map({ + {"openconfig-if-ethernet:SPEED_100GB", 100000000000}, + {"openconfig-if-ethernet:SPEED_200GB", 200000000000}, + {"openconfig-if-ethernet:SPEED_400GB", 400000000000}, + }); + std::string speed_state_path = + absl::StrCat("interfaces/interface[name=", interface_name, + "]/ethernet/state/port-speed"); + + std::string parse_str = "openconfig-if-ethernet:port-speed"; + ASSIGN_OR_RETURN( + std::string response, + GetGnmiStatePathInfo(&gnmi_stub, speed_state_path, parse_str)); + + auto speed = kPortSpeedTable.find(StripQuotes(response)); + if (speed == kPortSpeedTable.end()) { + return absl::NotFoundError(response); + } + return speed->second; +} + absl::Status SetPortMtu(int port_mtu, const std::string &interface_name, gnmi::gNMI::StubInterface &gnmi_stub) { std::string config_path = absl::StrCat( @@ -162,4 +189,43 @@ ParseIpv6DscpToQueueMapping(absl::string_view gnmi_config) { return ParseIpv4DscpToQueueMapping(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; +} + +absl::StatusOr> GetIpv6DscpToQueueMapping( + absl::string_view port, gnmi::gNMI::StubInterface &gnmi_stub) { + // TODO: Actually parse config -- hard-coded for now. + return GetIpv4DscpToQueueMapping(port, gnmi_stub); +} + +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(); +} + } // namespace pins_test diff --git a/tests/qos/qos_test_util.h b/tests/qos/qos_test_util.h index 46dccc0a6..f0628758e 100644 --- a/tests/qos/qos_test_util.h +++ b/tests/qos/qos_test_util.h @@ -47,6 +47,10 @@ absl::Status SetPortSpeed(const std::string &port_speed, const std::string &iface, gnmi::gNMI::StubInterface &gnmi_stub); +// Get configured port speed. +absl::StatusOr GetPortSpeed(const std::string &interface_name, + gnmi::gNMI::StubInterface &gnmi_stub); + // Set port MTU using gNMI. absl::Status SetPortMtu(int port_mtu, const std::string &interface_name, gnmi::gNMI::StubInterface &gnmi_stub); @@ -77,6 +81,18 @@ ParseIpv4DscpToQueueMapping(absl::string_view gnmi_config); absl::StatusOr> ParseIpv6DscpToQueueMapping(absl::string_view gnmi_config); +// Get IPv4 DSCP to queue mapping from switch. +absl::StatusOr> GetIpv4DscpToQueueMapping( + absl::string_view port, gnmi::gNMI::StubInterface &gnmi_stub); + +// Get IPv6 DSCP to queue mapping from switch. +absl::StatusOr> GetIpv6DscpToQueueMapping( + absl::string_view port, gnmi::gNMI::StubInterface &gnmi_stub); + +// Set a port in loopback mode. +absl::Status SetPortLoopbackMode(bool port_loopback, + absl::string_view interface_name, + gnmi::gNMI::StubInterface &gnmi_stub); } // namespace pins_test #endif // PINS_TESTS_QOS_QOS_TEST_UTIL_H_ From 81677916aff1b5e53432a5c57b7abad187de6f3d Mon Sep 17 00:00:00 2001 From: smolkaj Date: Mon, 9 May 2022 12:27:34 -0700 Subject: [PATCH 2/6] [Thinkit] Add helper libraries for manipulating and reading switch QoS config, Update max MTU & Remove workaround to create L3 ingress interfaces. --- tests/qos/BUILD.bazel | 10 ++ tests/qos/cpu_qos_test.cc | 64 +++-------- tests/qos/frontpanel_qos_test.cc | 8 +- tests/qos/qos_test_util.cc | 186 +++++++++++++++++++++++++++++-- tests/qos/qos_test_util.h | 94 +++++++++++++--- 5 files changed, 285 insertions(+), 77 deletions(-) diff --git a/tests/qos/BUILD.bazel b/tests/qos/BUILD.bazel index 11cd574dc..b98cee722 100644 --- a/tests/qos/BUILD.bazel +++ b/tests/qos/BUILD.bazel @@ -165,15 +165,25 @@ cc_library( srcs = ["qos_test_util.cc"], hdrs = ["qos_test_util.h"], deps = [ + ":gnmi_parsers", + "//gutil:collections", + "//gutil:proto", + "//gutil:status", "//lib/gnmi:gnmi_helper", + "//lib/gnmi:openconfig_cc_proto", + "//lib/utils:json_utils", "//thinkit:generic_testbed", "//thinkit/proto:generic_testbed_cc_proto", "@com_github_gnmi//proto/gnmi:gnmi_cc_proto", "@com_github_gnmi//proto/gnmi:gnmi_cc_grpc_proto", + "@com_github_nlohmann_json//:nlohmann_json", + "@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/strings:str_format", + "@com_google_absl//absl/time", + "@com_google_protobuf//:protobuf", ], ) diff --git a/tests/qos/cpu_qos_test.cc b/tests/qos/cpu_qos_test.cc index 2f246c536..948231959 100644 --- a/tests/qos/cpu_qos_test.cc +++ b/tests/qos/cpu_qos_test.cc @@ -91,12 +91,6 @@ using ::testing::Not; // frames. constexpr int kFrameCheckSequenceSize = 4; -// The maximum time the switch is allowed to take before queue counters read via -// gNMI have to be incremented after a packet hits a queue. -// Empirically, for PINS, queue counters currently seem to get updated every -// 10 seconds. -constexpr absl::Duration kMaxQueueCounterUpdateTime = absl::Seconds(25); - // After pushing gNMI config to a switch, the tests sleep for this duration // assuming that the gNMI config will have been fully applied afterwards. // TODO: Instead of hard-coding this time, tests should dynamically @@ -391,19 +385,6 @@ TEST_P(CpuQosTestWithoutIxia, PerEntryAclCounterIncrementsWhenEntryIsHit) { LOG(INFO) << "Link used to inject test packets: " << link_used_for_test_packets; - // We install a RIF to make this test non-trivial, as all packets are dropped - // by default if no RIF exists (b/190736007). - ASSERT_OK_AND_ASSIGN( - p4::v1::TableEntry router_interface_entry, - MakeRouterInterface( - /*router_interface_id=*/"ingress-rif-to-workaround-b/190736007", - /*p4rt_port_name=*/link_used_for_test_packets.sut_port_p4rt_name, - // An arbitrary MAC address will do. - /*mac=*/netaddr::MacAddress(0x00, 0x07, 0xE9, 0x42, 0xAC, 0x28), - /*ir_p4info=*/ir_p4info)); - ASSERT_OK(pdpi::InstallPiTableEntry(sut_p4rt_session.get(), - router_interface_entry)); - // 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( @@ -677,19 +658,6 @@ TEST_P(CpuQosTestWithoutIxia, LOG(INFO) << "Link used to inject test packets: " << link_used_for_test_packets; - // We install a RIF to make this test non-trivial, as all packets are dropped - // by default if no RIF exists (b/190736007). - ASSERT_OK_AND_ASSIGN( - p4::v1::TableEntry router_interface_entry, - MakeRouterInterface( - /*router_interface_id=*/"ingress-rif-to-workaround-b/190736007", - /*p4rt_port_name=*/link_used_for_test_packets.sut_port_p4rt_name, - // An arbitrary MAC address will do. - /*mac=*/netaddr::MacAddress(0x00, 0x07, 0xE9, 0x42, 0xAC, 0x28), - /*ir_p4info=*/ir_p4info)); - ASSERT_OK(pdpi::InstallPiTableEntry(sut_p4rt_session.get(), - router_interface_entry)); - // Extract loopback IPs from gNMI config, to avoid using them in test packets. using IpAddresses = std::vector>; @@ -748,7 +716,7 @@ TEST_P(CpuQosTestWithoutIxia, } // Purpose: Verify DSCP-to-queue mapping for traffic to switch loopback IP. -TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) { +TEST_P(CpuQosTestWithoutIxia, TrafficToLoopbackIpGetsMappedToCorrectQueues) { LOG(INFO) << "-- START OF TEST ---------------------------------------------"; // Setup: the testbed consists of a SUT connected to a control device @@ -778,6 +746,9 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) { LOG(INFO) << "Link used to inject test packets: " << link_used_for_test_packets; + ASSERT_FALSE(GetParam().test_packets.empty()) + << "No packets to test, maybe no loopback IP is configured on switch?"; + for (PacketAndExpectedTargetQueue test_packet : GetParam().test_packets) { std::string_view target_queue = test_packet.target_queue; SCOPED_TRACE(absl::StrCat("Packet: ", test_packet.packet_name, @@ -831,14 +802,14 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopackIpGetsMappedToCorrectQueues) { *sut_gnmi_stub)); } while ( // It may take several seconds for the queue counters to update. - CumulativeNumPacketsEnqueued(queue_counters_after_test_packet) == - CumulativeNumPacketsEnqueued(queue_counters_before_test_packet) && + TotalPacketsForQueue(queue_counters_after_test_packet) == + TotalPacketsForQueue(queue_counters_before_test_packet) && absl::Now() - time_packet_sent < kMaxQueueCounterUpdateTime); // We terminate early if this fails, as that can cause this loop to get // out of sync when counters increment after a long delay, resulting in // confusing error messages where counters increment by 2. - ASSERT_EQ(CumulativeNumPacketsEnqueued(queue_counters_after_test_packet), - CumulativeNumPacketsEnqueued(queue_counters_before_test_packet) + + ASSERT_EQ(TotalPacketsForQueue(queue_counters_after_test_packet), + TotalPacketsForQueue(queue_counters_before_test_packet) + kPacketCount) << "Counters for queue " << target_queue << " did not increment within " << kMaxQueueCounterUpdateTime @@ -868,7 +839,7 @@ constexpr int kTotalFrames = 10000000; constexpr absl::Duration kTrafficDuration = absl::Seconds(kTotalFrames / kFramesPerSecond); constexpr int kDefaultFrameSize = 1514; -constexpr int kMaxFrameSize = 9000; +constexpr int kMaxFrameSize = 9216; constexpr int kMinFrameSize = 64; struct PacketReceiveInfo { @@ -1008,10 +979,9 @@ TEST_P(CpuQosTestWithIxia, TestPuntFlowRateLimitAndCounters) { const absl::flat_hash_map interface_info = generic_testbed->GetSutInterfaceInfo(); for (const auto &[interface, info] : interface_info) { - if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) - { - ASSERT_OK(SetPortSpeed("\"openconfig-if-ethernet:SPEED_200GB\"", - interface, *gnmi_stub)); + if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) { + ASSERT_OK(SetPortSpeedInBitsPerSecond( + "\"openconfig-if-ethernet:SPEED_200GB\"", interface, *gnmi_stub)); ASSERT_OK(SetPortMtu(kMaxFrameSize, interface, *gnmi_stub)); } } @@ -1032,8 +1002,8 @@ TEST_P(CpuQosTestWithIxia, TestPuntFlowRateLimitAndCounters) { if (ready_links.empty()) { for (const auto &[interface, info] : interface_info) { if (info.interface_modes.contains(thinkit::TRAFFIC_GENERATOR)) - ASSERT_OK(SetPortSpeed("\"openconfig-if-ethernet:SPEED_100GB\"", - interface, *gnmi_stub)); + ASSERT_OK(SetPortSpeedInBitsPerSecond( + "\"openconfig-if-ethernet:SPEED_100GB\"", interface, *gnmi_stub)); } } // Wait to let the links come up. Switch guarantees state paths to reflect @@ -1191,8 +1161,8 @@ TEST_P(CpuQosTestWithIxia, TestPuntFlowRateLimitAndCounters) { delta_counters = { .num_packets_transmitted = final_counters.num_packets_transmitted - initial_counters.num_packets_transmitted, - .num_packet_dropped = final_counters.num_packet_dropped - - initial_counters.num_packet_dropped, + .num_packets_dropped = final_counters.num_packets_dropped - + initial_counters.num_packets_dropped, }; LOG(INFO) << delta_counters; absl::MutexLock lock(&packet_receive_info.mutex); @@ -1203,7 +1173,7 @@ TEST_P(CpuQosTestWithIxia, TestPuntFlowRateLimitAndCounters) { ASSERT_NE(gnmi_counters_check, kIterations - 1) << "GNMI packet count " << delta_counters.num_packets_transmitted + - delta_counters.num_packet_dropped + delta_counters.num_packets_dropped << " != Packets received at controller " << packet_receive_info.num_packets_punted; } diff --git a/tests/qos/frontpanel_qos_test.cc b/tests/qos/frontpanel_qos_test.cc index 15b0df15a..156a5f424 100644 --- a/tests/qos/frontpanel_qos_test.cc +++ b/tests/qos/frontpanel_qos_test.cc @@ -424,7 +424,7 @@ TEST_P(FrontpanelQosTest, TestWredEcnMarking) { // Get port speed in bits per second. ASSERT_OK_AND_ASSIGN(auto in_port2_speed, - GetPortSpeedInBitsPerSecond(kSutInPort2, *gnmi_stub)); + GetPortSpeedInBitsPerSecond(kSutInPort2, *gnmi_stub)); uint64_t frame_rate_at_line_speed_of_in_port2 = in_port2_speed / (kDefaultFrameSizeinBytes * 8); @@ -605,10 +605,10 @@ TEST_P(FrontpanelQosTest, TestWredEcnMarking) { // This test expects WRED config to only mark packets and not drop. // Expect no drops in target queue and queue transmit counter // increments. - EXPECT_EQ(queue_counters_after_test_packet.num_packet_dropped, - queue_counters_before_test_packet.num_packet_dropped); + EXPECT_EQ(queue_counters_after_test_packet.num_packets_dropped, + queue_counters_before_test_packet.num_packets_dropped); - EXPECT_GT(queue_counters_after_test_packet.num_packets_transmitted, + EXPECT_GT(queue_counters_after_test_packet.num_packets_transmitted, queue_counters_before_test_packet.num_packets_transmitted); } } diff --git a/tests/qos/qos_test_util.cc b/tests/qos/qos_test_util.cc index 1a68cfd12..6476facaa 100644 --- a/tests/qos/qos_test_util.cc +++ b/tests/qos/qos_test_util.cc @@ -2,10 +2,23 @@ #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/string_view.h" +#include "absl/strings/strip.h" #include "absl/strings/substitute.h" +#include "absl/time/clock.h" +#include "absl/time/time.h" +#include "google/protobuf/util/json_util.h" +#include "gutil/collections.h" +#include "gutil/proto.h" #include "gutil/status.h" +#include "include/nlohmann/json.hpp" #include "lib/gnmi/gnmi_helper.h" +#include "lib/gnmi/openconfig.pb.h" +#include "lib/utils/json_utils.h" +#include "proto/gnmi/gnmi.pb.h" +#include "tests/qos/gnmi_parsers.h" namespace pins_test { @@ -13,7 +26,7 @@ QueueCounters operator-(const QueueCounters &x, const QueueCounters &y) { return QueueCounters{ .num_packets_transmitted = x.num_packets_transmitted - y.num_packets_transmitted, - .num_packet_dropped = x.num_packet_dropped - y.num_packet_dropped, + .num_packets_dropped = x.num_packets_dropped - y.num_packets_dropped, }; } @@ -49,7 +62,7 @@ absl::StatusOr GetGnmiQueueCounters( "openconfig-qos:dropped-pkts")); if (!absl::SimpleAtoi(StripQuotes(drop_counter_response), - &counters.num_packet_dropped)) { + &counters.num_packets_dropped)) { return absl::InternalError( absl::StrCat("Unable to parse counter from ", drop_counter_response)); } @@ -71,13 +84,13 @@ absl::StatusOr GetGnmiQueueCounterWithTimestamp( // Returns the total number of packets enqueued for the queue with the given // `QueueCounters`. -int64_t CumulativeNumPacketsEnqueued(const QueueCounters &counters) { - return counters.num_packet_dropped + counters.num_packets_transmitted; +int64_t TotalPacketsForQueue(const QueueCounters &counters) { + return counters.num_packets_dropped + counters.num_packets_transmitted; } -absl::Status SetPortSpeed(const std::string &port_speed, - const std::string &iface, - gnmi::gNMI::StubInterface &gnmi_stub) { +absl::Status SetPortSpeedInBitsPerSecond(const std::string &port_speed, + const std::string &iface, + gnmi::gNMI::StubInterface &gnmi_stub) { std::string ops_config_path = absl::StrCat( "interfaces/interface[name=", iface, "]/ethernet/config/port-speed"); std::string ops_val = @@ -89,8 +102,9 @@ absl::Status SetPortSpeed(const std::string &port_speed, return absl::OkStatus(); } -absl::StatusOr GetPortSpeed(const std::string &interface_name, - gnmi::gNMI::StubInterface &gnmi_stub) { +absl::StatusOr +GetPortSpeedInBitsPerSecond(const std::string &interface_name, + gnmi::gNMI::StubInterface &gnmi_stub) { // Map keyed on openconfig speed string to value in bits per second. // http://ops.openconfig.net/branches/models/master/docs/openconfig-interfaces.html#mod-openconfig-if-ethernet const auto kPortSpeedTable = @@ -228,4 +242,158 @@ absl::Status SetPortLoopbackMode(bool port_loopback, return absl::OkStatus(); } +absl::StatusOr +GetQueueNameByDscpAndPort(int dscp, absl::string_view port, + gnmi::gNMI::StubInterface &gnmi_stub) { + absl::flat_hash_map queue_by_dscp; + ASSIGN_OR_RETURN(queue_by_dscp, GetIpv4DscpToQueueMapping(port, gnmi_stub)); + return gutil::FindOrStatus(queue_by_dscp, dscp); +} + +absl::StatusOr +GetSchedulerPolicyNameByEgressPort(absl::string_view egress_port, + gnmi::gNMI::StubInterface &gnmi) { + const std::string kPath = absl::StrFormat( + "qos/interfaces/interface[interface-id=%s]/output/scheduler-policy/" + "config/name", + egress_port); + ASSIGN_OR_RETURN(std::string name, + ReadGnmiPath(&gnmi, kPath, gnmi::GetRequest::CONFIG, + "openconfig-qos:name")); + return std::string(StripQuotes(name)); +} + +static std::string +SchedulerPolicyPath(absl::string_view scheduler_policy_name) { + return absl::StrFormat("qos/scheduler-policies/scheduler-policy[name=%s]", + scheduler_policy_name); +} + +absl::StatusOr +GetSchedulerPolicyConfig(absl::string_view scheduler_policy_name, + gnmi::gNMI::StubInterface &gnmi) { + std::string path = SchedulerPolicyPath(scheduler_policy_name); + return ReadGnmiPath(&gnmi, path, gnmi::GetRequest::CONFIG, ""); +} + +absl::Status +UpdateSchedulerPolicyConfig(absl::string_view scheduler_policy_name, + absl::string_view config, + gnmi::gNMI::StubInterface &gnmi) { + std::string path = SchedulerPolicyPath(scheduler_policy_name); + return SetGnmiConfigPath(&gnmi, path, GnmiSetType::kUpdate, config); +} + +absl::Status SetSchedulerPolicyParameters( + absl::string_view scheduler_policy_name, + absl::flat_hash_map params_by_queue_name, + gnmi::gNMI::StubInterface &gnmi, absl::Duration convergence_timeout) { + // Pull existing config. + const std::string kPath = SchedulerPolicyPath(scheduler_policy_name); + const std::string kRoot = "openconfig-qos:scheduler-policy"; + ASSIGN_OR_RETURN(const std::string kRawConfig, + ReadGnmiPath(&gnmi, kPath, gnmi::GetRequest::CONFIG, kRoot)); + ASSIGN_OR_RETURN( + openconfig::Qos::SchedulerPolicy proto_config, + gutil::ParseJsonAsProto( + StripBrackets(kRawConfig), /*ignore_unknown_fields=*/true)); + + // Updated config. + for (openconfig::Qos::Scheduler &scheduler : + *proto_config.mutable_schedulers()->mutable_scheduler()) { + if (scheduler.inputs().input_size() == 0) + continue; + if (scheduler.inputs().input_size() > 1) { + return gutil::UnimplementedErrorBuilder() + << "scheduler with several inputs unsupported: " + << scheduler.DebugString(); + } + const std::string kQueue = scheduler.inputs().input(0).config().queue(); + const std::string kSchedulerPath = absl::StrFormat( + "%s/schedulers/scheduler[sequence=%d]", + SchedulerPolicyPath(scheduler_policy_name), scheduler.sequence()); + LOG(INFO) << "found scheduler '" << kSchedulerPath << " for queue " + << kQueue; + SchedulerParameters *const params = + gutil::FindOrNull(params_by_queue_name, kQueue); + LOG(INFO) << "-> " << (params == nullptr ? "no " : "") + << "changes requested"; + if (params == nullptr) + continue; + + if (scheduler.config().type() != + "openconfig-qos-types:TWO_RATE_THREE_COLOR") { + return gutil::InvalidArgumentErrorBuilder() + << "scheduler '" << kSchedulerPath << "' of unsupported type: '" + << scheduler.config().type() << "'"; + } + + auto &config = *scheduler.mutable_two_rate_three_color()->mutable_config(); + if (auto pir = params->peak_information_rate; pir.has_value()) { + // OpenConfig uses bits, but our API uses bytes for consistency. + config.set_pir(absl::StrCat(*pir * 8)); + } + if (auto be = params->excess_burst_size; be.has_value()) { + config.set_be(*be); + } + if (auto cir = params->committed_information_rate; cir.has_value()) { + // OpenConfig uses bits, but our API uses bytes for consistency. + config.set_cir(absl::StrCat(*cir * 8)); + } + if (auto bc = params->committed_burst_size; bc.has_value()) { + config.set_bc(*bc); + } + + LOG(INFO) << "modified scheduler: " << scheduler.DebugString(); + + // We update the entire scheduler subtree, instead of applying updates + // incrementally, to work around b/228117691. + { + // Convert proto back to JSON string. + ASSIGN_OR_RETURN(std::string scheduler_json, + gutil::SerializeProtoAsJson(scheduler)); + // Apply updated scheduler. + RETURN_IF_ERROR(SetGnmiConfigPath( + &gnmi, kSchedulerPath, GnmiSetType::kUpdate, + absl::StrFormat(R"({ "scheduler": [%s] })", scheduler_json))); + } + } + + // Wait for convergence. + const absl::Time kDeadline = absl::Now() + convergence_timeout; + std::string config_state_diff; + do { + ASSIGN_OR_RETURN(std::string raw_config, + ReadGnmiPath(&gnmi, kPath, gnmi::GetRequest::ALL, kRoot)); + ASSIGN_OR_RETURN( + openconfig::Qos::SchedulerPolicy proto_config, + gutil::ParseJsonAsProto( + StripBrackets(raw_config), /*ignore_unknown_fields=*/true)); + for (openconfig::Qos::Scheduler &scheduler : + *proto_config.mutable_schedulers()->mutable_scheduler()) { + if (!scheduler.has_two_rate_three_color()) + continue; + auto &config = scheduler.two_rate_three_color().config(); + auto &state = scheduler.two_rate_three_color().state(); + ASSIGN_OR_RETURN(config_state_diff, gutil::ProtoDiff(config, state)); + if (!config_state_diff.empty()) { + absl::StrAppendFormat(&config_state_diff, + "between two-rate-three-color config and state, " + "for scheduler '%s[%d]'", + scheduler_policy_name, scheduler.sequence()); + break; + } + } + } while (!config_state_diff.empty() && absl::Now() < kDeadline); + + if (!config_state_diff.empty()) { + return gutil::DeadlineExceededErrorBuilder() + << "QoS scheduler policy state paths did not converge within " + << convergence_timeout << "; diff:\n" + << config_state_diff; + } + + return absl::OkStatus(); +} + } // namespace pins_test diff --git a/tests/qos/qos_test_util.h b/tests/qos/qos_test_util.h index f0628758e..87d86a4ce 100644 --- a/tests/qos/qos_test_util.h +++ b/tests/qos/qos_test_util.h @@ -1,30 +1,43 @@ #ifndef PINS_TESTS_QOS_QOS_TEST_UTIL_H_ #define PINS_TESTS_QOS_QOS_TEST_UTIL_H_ +#include +#include + +#include "absl/container/flat_hash_map.h" #include "absl/status/statusor.h" #include "absl/strings/str_format.h" +#include "absl/strings/string_view.h" #include "lib/gnmi/gnmi_helper.h" +#include "lib/gnmi/openconfig.pb.h" #include "proto/gnmi/gnmi.grpc.pb.h" #include "proto/gnmi/gnmi.pb.h" #include "thinkit/generic_testbed.h" #include "thinkit/proto/generic_testbed.pb.h" namespace pins_test { + +// The maximum time the switch is allowed to take before queue counters read via +// gNMI have to be incremented after a packet hits a queue. +// Empirically, for PINS, queue counters currently seem to get updated every +// 10 seconds. +constexpr absl::Duration kMaxQueueCounterUpdateTime = absl::Seconds(25); + // These are the counters we track in these tests. struct QueueCounters { int64_t num_packets_transmitted = 0; - int64_t num_packet_dropped = 0; + int64_t num_packets_dropped = 0; }; // Operator to pretty print Queue Counters. inline std::ostream &operator<<(std::ostream &os, const QueueCounters &counters) { - return os << absl::StreamFormat( - "QueueCounters{" - ".num_packets_transmitted = %d, " - ".num_packets_dropped = %d" - "}", - counters.num_packets_transmitted, counters.num_packet_dropped); + return os << absl::StreamFormat("QueueCounters{" + ".num_packets_transmitted = %d, " + ".num_packets_dropped = %d" + "}", + counters.num_packets_transmitted, + counters.num_packets_dropped); } QueueCounters operator-(const QueueCounters &x, const QueueCounters &y); @@ -40,21 +53,27 @@ absl::StatusOr GetGnmiQueueCounterWithTimestamp( absl::string_view statistic, gnmi::gNMI::StubInterface &gnmi_stub); // Get total packets (transmitted + dropped) for port queue. -int64_t CumulativeNumPacketsEnqueued(const QueueCounters &counters); +int64_t TotalPacketsForQueue(const QueueCounters &counters); // Set port speed using gNMI. -absl::Status SetPortSpeed(const std::string &port_speed, - const std::string &iface, - gnmi::gNMI::StubInterface &gnmi_stub); +absl::Status SetPortSpeedInBitsPerSecond(const std::string &port_speed, + const std::string &iface, + gnmi::gNMI::StubInterface &gnmi_stub); // Get configured port speed. -absl::StatusOr GetPortSpeed(const std::string &interface_name, - gnmi::gNMI::StubInterface &gnmi_stub); +absl::StatusOr +GetPortSpeedInBitsPerSecond(const std::string &interface_name, + gnmi::gNMI::StubInterface &gnmi_stub); // Set port MTU using gNMI. absl::Status SetPortMtu(int port_mtu, const std::string &interface_name, gnmi::gNMI::StubInterface &gnmi_stub); +// Set a port in loopback mode. +absl::Status SetPortLoopbackMode(bool port_loopback, + absl::string_view interface_name, + gnmi::gNMI::StubInterface &gnmi_stub); + // Check if switch port link is up. absl::StatusOr CheckLinkUp(const std::string &iface, gnmi::gNMI::StubInterface &gnmi_stub); @@ -89,10 +108,51 @@ absl::StatusOr> GetIpv4DscpToQueueMapping( absl::StatusOr> GetIpv6DscpToQueueMapping( absl::string_view port, gnmi::gNMI::StubInterface &gnmi_stub); -// Set a port in loopback mode. -absl::Status SetPortLoopbackMode(bool port_loopback, - absl::string_view interface_name, - gnmi::gNMI::StubInterface &gnmi_stub); +// Get name of queue configured for the given DSCP. +absl::StatusOr +GetQueueNameByDscpAndPort(int dscp, absl::string_view port, + gnmi::gNMI::StubInterface &gnmi_stub); + +// Reads the name of the scheduler policy applied to the given egress port from +// the appropriate gNMI state path. +absl::StatusOr +GetSchedulerPolicyNameByEgressPort(absl::string_view egress_port, + gnmi::gNMI::StubInterface &gnmi); + +// Reads the config path of the scheduler policy of the given name. +// The config is returned unparsed as a raw JSON string. +absl::StatusOr +GetSchedulerPolicyConfig(absl::string_view scheduler_policy_name, + gnmi::gNMI::StubInterface &gnmi); + +// Updates the config path of the scheduler policy of the given name. +absl::Status +UpdateSchedulerPolicyConfig(absl::string_view scheduler_policy_name, + absl::string_view config, + gnmi::gNMI::StubInterface &gnmi); + +// Two-rate-three-color scheduler parameters. Rates are in bytes/second, sizes +// are in bytes. All parameter are optional, only non-nullopt parameters take +// effect. Documentation: +// - https://datatracker.ietf.org/doc/html/rfc2698 +// - http://ops.openconfig.net/branches/models/master/docs/openconfig-qos.html +struct SchedulerParameters { + std::optional committed_information_rate; // 'cir' in OpenConfig + std::optional committed_burst_size; // 'bc' in OpenConfig + std::optional peak_information_rate; // 'pir' in OpenConfig + std::optional excess_burst_size; // 'be' in OpenConfig +}; + +// Updates parameters of the scheduler policy of the given name according to +// `params_by_queue_name` and waits for the updated config to converge, or times +// out with an Unavailable error if the state does not converge within the given +// `convergence_timeout`. +absl::Status SetSchedulerPolicyParameters( + absl::string_view scheduler_policy_name, + absl::flat_hash_map params_by_queue_name, + gnmi::gNMI::StubInterface &gnmi, + absl::Duration convergence_timeout = absl::Seconds(10)); + } // namespace pins_test #endif // PINS_TESTS_QOS_QOS_TEST_UTIL_H_ From ad4e5892f78437ddc1b3bcc2b41a5f9e55d110d8 Mon Sep 17 00:00:00 2001 From: smolkaj Date: Mon, 9 May 2022 13:11:58 -0700 Subject: [PATCH 3/6] [Thinkit] Add DSCP/scheduling test case. --- tests/qos/BUILD.bazel | 10 +- tests/qos/frontpanel_qos_test.cc | 464 +++++++++++++++++++++++++++++-- 2 files changed, 443 insertions(+), 31 deletions(-) diff --git a/tests/qos/BUILD.bazel b/tests/qos/BUILD.bazel index b98cee722..047b06920 100644 --- a/tests/qos/BUILD.bazel +++ b/tests/qos/BUILD.bazel @@ -96,11 +96,15 @@ cc_library( ":qos_test_util", "//dvaas:test_vector_cc_proto", "//gutil:collections", + "//gutil:proto", + "//gutil:status", "//gutil:status_matchers", "//gutil:testing", "//lib:ixia_helper", "//lib/gnmi:gnmi_helper", + "//lib/utils:json_utils", "//p4_pdpi:ir", + "//p4_pdpi:ir_cc_proto", "//p4_pdpi:p4_runtime_session", "//p4_pdpi:pd", "//p4_pdpi/netaddr:ipv4_address", @@ -113,10 +117,14 @@ cc_library( "//tests/lib:switch_test_setup_helpers", "//thinkit:generic_testbed", "//thinkit:generic_testbed_fixture", + "@com_github_gnmi//proto/gnmi:gnmi_cc_grpc_proto", "@com_github_p4lang_p4runtime//:p4info_cc_proto", - "@com_google_absl//absl/cleanup", + "@com_github_p4lang_p4runtime//:p4runtime_cc_proto", + "@com_google_absl//absl/cleanup", + "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", + "@com_google_absl//absl/strings:str_format", "@com_google_absl//absl/time", "@com_google_googletest//:gtest", ], diff --git a/tests/qos/frontpanel_qos_test.cc b/tests/qos/frontpanel_qos_test.cc index 156a5f424..89977b6e7 100644 --- a/tests/qos/frontpanel_qos_test.cc +++ b/tests/qos/frontpanel_qos_test.cc @@ -14,25 +14,38 @@ #include "tests/qos/frontpanel_qos_test.h" +#include +#include +#include #include +#include +#include #include // NOLINT #include #include "absl/cleanup/cleanup.h" +#include "absl/container/flat_hash_map.h" #include "absl/status/status.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/str_format.h" +#include "absl/strings/str_replace.h" #include "absl/strings/string_view.h" #include "absl/strings/substitute.h" +#include "absl/time/clock.h" #include "absl/time/time.h" #include "dvaas/test_vector.pb.h" -#include "gmock/gmock.h" -#include "gtest/gtest.h" #include "gutil/collections.h" +#include "gutil/proto.h" +#include "gutil/status.h" #include "gutil/status_matchers.h" #include "gutil/testing.h" #include "lib/gnmi/gnmi_helper.h" #include "lib/ixia_helper.h" +#include "lib/utils/json_utils.h" +#include "p4/v1/p4runtime.pb.h" #include "p4_pdpi/ir.h" +#include "p4_pdpi/ir.pb.h" #include "p4_pdpi/netaddr/ipv4_address.h" #include "p4_pdpi/netaddr/ipv6_address.h" #include "p4_pdpi/netaddr/mac_address.h" @@ -40,27 +53,146 @@ #include "p4_pdpi/packetlib/packetlib.h" #include "p4_pdpi/packetlib/packetlib.pb.h" #include "p4_pdpi/pd.h" +#include "proto/gnmi/gnmi.grpc.pb.h" #include "sai_p4/instantiations/google/sai_pd.pb.h" #include "tests/forwarding/util.h" #include "tests/lib/switch_test_setup_helpers.h" #include "tests/qos/packet_in_receiver.h" #include "tests/qos/qos_test_util.h" +#include "thinkit/generic_testbed.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" namespace pins_test { -struct EcnTestPacketCounters { - absl::Mutex mutex; - int num_packets_punted ABSL_GUARDED_BY(mutex) = 0; - int num_packets_ecn_marked ABSL_GUARDED_BY(mutex) = 0; -}; +using ::json_yang::FormatJsonBestEffort; +using ::testing::Eq; +using ::testing::Field; +using ::testing::ResultOf; -void ResetEcnTestPacketCounters(EcnTestPacketCounters &packet_receive_info) { - absl::MutexLock lock(&packet_receive_info.mutex); - packet_receive_info.num_packets_punted = 0; - packet_receive_info.num_packets_ecn_marked = 0; +template std::string ToString(const T &t) { + std::stringstream ss; + ss << t; + return ss.str(); +} + +// Connects to Ixia on the given testbed and returns a string handle identifying +// the connection (aka "topology ref"). +absl::StatusOr ConnectToIxia(thinkit::GenericTestbed &testbed) { + ASSIGN_OR_RETURN(auto gnmi_stub, testbed.Sut().CreateGnmiStub()); + ASSIGN_OR_RETURN(std::vector ready_links, + GetReadyIxiaLinks(testbed, *gnmi_stub)); + if (ready_links.empty()) { + return gutil::UnavailableErrorBuilder() << "no Ixia-to-SUT link up"; + } + absl::string_view ixia_interface = ready_links[0].ixia_interface; + ASSIGN_OR_RETURN(ixia::IxiaPortInfo ixia_port_info, + ixia::ExtractPortInfo(ixia_interface)); + ASSIGN_OR_RETURN( + std::string ixia_connection_handle, + pins_test::ixia::IxiaConnect(ixia_port_info.hostname, testbed)); + return ixia_connection_handle; +} + +// Installs the given table `entries` using the given P4Runtime session, +// respecting dependencies between entries by sequencing them into batches +// according to `p4info`. +absl::Status InstallPdTableEntries(const sai::TableEntries &entries, + const pdpi::IrP4Info &p4info, + pdpi::P4RuntimeSession &p4rt_session) { + std::vector pi_entries; + for (const sai::TableEntry &entry : entries.entries()) { + ASSIGN_OR_RETURN(pi_entries.emplace_back(), + pdpi::PartialPdTableEntryToPiTableEntry(p4info, entry)); + } + return pdpi::InstallPiTableEntries(&p4rt_session, p4info, pi_entries); +} +absl::Status InstallPdTableEntries(const sai::TableEntries &entries, + const p4::config::v1::P4Info &p4info, + pdpi::P4RuntimeSession &p4rt_session) { + ASSIGN_OR_RETURN(pdpi::IrP4Info ir_p4info, pdpi::CreateIrP4Info(p4info)); + return InstallPdTableEntries(entries, ir_p4info, p4rt_session); +} + +// Returns a set of table entries that will cause a switch to forward all L3 +// packets unconditionally out of the given port. +absl::StatusOr +ConstructEntriesToForwardAllTrafficToGivenPort(absl::string_view p4rt_port_id) { + // By convention, we use link local IPv6 addresses as neighbor IDs. + const std::string kNeighborId = + netaddr::MacAddress(2, 2, 2, 2, 2, 2).ToLinkLocalIpv6Address().ToString(); + return gutil::ParseTextProto(absl::Substitute( + R"pb( + entries { + l3_admit_table_entry { + match {} # Wildcard. + action { admit_to_l3 {} } + priority: 1 + } + } + entries { + acl_pre_ingress_table_entry { + match {} # Wildcard. + action { set_vrf { vrf_id: "vrf" } } + priority: 1 + } + } + entries { + vrf_table_entry { + match { vrf_id: "vrf" } + action { no_action {} } + } + } + entries { + ipv4_table_entry { + match { vrf_id: "vrf" } + action { set_nexthop_id { nexthop_id: "nexthop" } } + } + } + entries { + ipv6_table_entry { + match { vrf_id: "vrf" } + action { set_nexthop_id { nexthop_id: "nexthop" } } + } + } + entries { + nexthop_table_entry { + match { nexthop_id: "nexthop" } + action { + set_ip_nexthop { router_interface_id: "rif" neighbor_id: "$0" } + } + } + } + entries { + router_interface_table_entry { + match { router_interface_id: "rif" } + action { + set_port_and_src_mac { port: "$1" src_mac: "66:55:44:33:22:11" } + } + } + } + entries { + neighbor_table_entry { + match { router_interface_id: "rif" neighbor_id: "$0" } + action { set_dst_mac { dst_mac: "02:02:02:02:02:02" } } + } + } + )pb", + kNeighborId, p4rt_port_id)); } -TEST_P(FrontpanelQosTest, PacketsGetMappedToCorrectQueuesBasedOnDscp) { +// The purpose of this test is to verify that: +// - Incoming IP packets are mapped to queues according to their DSCP. +// - Queue peak information rates (PIRs) are enforced. +// - Queue egress counters increment correctly. +// +// We test this (for each DSCP, for IPv4 & IPv6) by sending test packets of a +// fixed DSCP at line rate, observing the rate at which the SUT forwards them, +// and inferring which queue was used by cross checking against the PIRs of the +// queues, which we configure to be exponentially spaced. +TEST_P(FrontpanelQosTest, + PacketsGetMappedToCorrectQueuesBasedOnDscpAndQueuePeakRatesAreEnforced) { + LOG(INFO) << "-- Test started ----------------------------------------------"; // Pick a testbed with SUT connected to an Ixia on 2 ports, so we can // oversubscribe switch egress port. auto requirements = gutil::ParseProtoOrDie( @@ -71,13 +203,283 @@ TEST_P(FrontpanelQosTest, PacketsGetMappedToCorrectQueuesBasedOnDscp) { std::unique_ptr testbed, GetParam().testbed_interface->GetTestbedWithRequirements(requirements)); - // TODO: actual testing. + // Pick 2 SUT ports connected to the Ixia, one for receiving test packets and + // one for forwarding them back. + ASSERT_OK_AND_ASSIGN(auto gnmi_stub, testbed->Sut().CreateGnmiStub()); + ASSERT_OK_AND_ASSIGN(std::vector ready_links, + GetReadyIxiaLinks(*testbed, *gnmi_stub)); + ASSERT_GE(ready_links.size(), 2) + << "Test requires at least 2 SUT ports connected to an Ixia"; + const std::string kIxiaSrcPort = ready_links[0].ixia_interface; + const std::string kIxiaDstPort = ready_links[1].ixia_interface; + const std::string kSutIngressPort = ready_links[0].sut_interface; + const std::string kSutEgressPort = ready_links[1].sut_interface; + LOG(INFO) << absl::StrFormat( + "Test packet route: [Ixia: %s] => [SUT: %s] -> [SUT: %s] => [Ixia: %s]", + kIxiaSrcPort, kSutIngressPort, kSutEgressPort, kIxiaDstPort); + ASSERT_OK_AND_ASSIGN( + const std::string kSutEgressPortScheduler, + GetSchedulerPolicyNameByEgressPort(kSutEgressPort, *gnmi_stub)); + absl::flat_hash_map p4rt_id_by_interface; + ASSERT_OK_AND_ASSIGN(p4rt_id_by_interface, + GetAllInterfaceNameToPortId(*gnmi_stub)); + ASSERT_OK_AND_ASSIGN( + const std::string kSutEgressPortP4rtId, + gutil::FindOrStatus(p4rt_id_by_interface, kSutEgressPort)); + + // Configure the switch to send all incomming packets out of the chosen egress + // port. + ASSERT_OK_AND_ASSIGN( + std::unique_ptr sut_p4rt, + ConfigureSwitchAndReturnP4RuntimeSession( + testbed->Sut(), /*gnmi_config=*/std::nullopt, GetParam().p4info)); + ASSERT_OK_AND_ASSIGN( + const sai::TableEntries kTableEntries, + ConstructEntriesToForwardAllTrafficToGivenPort(kSutEgressPortP4rtId)); + ASSERT_OK(testbed->Environment().StoreTestArtifact("pd_entries.textproto", + kTableEntries)); + ASSERT_OK(InstallPdTableEntries(kTableEntries, GetParam().p4info, *sut_p4rt)); + + // Fix test parameters and PIRs (peak information rates, in bytes + // per second) for each DSCP. + constexpr int64_t kTestFrameSizeInBytes = 1000; // 1 KB + constexpr int64_t kTestFrameCount = 30'000'000; // 30 GB + constexpr int64_t kTestFramesPerSecond = kTestFrameCount / 3; // for 3 s + constexpr int64_t kTestFrameSpeedInBytesPerSecond = // 10 GB/s + kTestFramesPerSecond * kTestFrameSizeInBytes; + // We use exponentially spaced PIRs, with a base rate that's high enough for + // buffers to drain quickly. That way we don't have to drain buffers manually + // between test traffic flows. + constexpr int64_t kPirBaseSpeedInBytesPerSecond = 40'000'000; // 40 MB/s. + const absl::flat_hash_map kPirByQueueName = { + {"BE1", 1 * kPirBaseSpeedInBytesPerSecond}, + {"AF1", 2 * kPirBaseSpeedInBytesPerSecond}, + {"AF2", 4 * kPirBaseSpeedInBytesPerSecond}, + {"AF3", 8 * kPirBaseSpeedInBytesPerSecond}, + {"LLQ1", 16 * kPirBaseSpeedInBytesPerSecond}, + {"LLQ2", 32 * kPirBaseSpeedInBytesPerSecond}, + // TODO: Also test strict queues once reconfiguring them is + // supported. + // {"AF4", 64 * kPirBaseSpeedInBytesPerSecond}, + // {"NC1", 128 * kPirBaseSpeedInBytesPerSecond}, + }; + + // Ensure the test parameters are compatible with the testbed. + ASSERT_OK_AND_ASSIGN( + const int64_t kSutIngressPortSpeedInBitsPerSecond, + GetPortSpeedInBitsPerSecond(kSutIngressPort, *gnmi_stub)); + ASSERT_OK_AND_ASSIGN(const int64_t kSutEgressPortSpeedInBitsPerSecond, + GetPortSpeedInBitsPerSecond(kSutEgressPort, *gnmi_stub)); + ASSERT_LE(kTestFrameSpeedInBytesPerSecond, + kSutIngressPortSpeedInBitsPerSecond / 8) + << "ingress port is too slow to sustain test packet speed"; + ASSERT_LE(kTestFrameSpeedInBytesPerSecond, + kSutEgressPortSpeedInBitsPerSecond / 8) + << "egress port is too slow to sustain test packet speed"; + for (const auto &[queue, pir] : kPirByQueueName) { + ASSERT_GE(kTestFrameSpeedInBytesPerSecond, 2 * pir) + << "test packet speed is too low to meaningfully exceed PIR"; + } + + // Before we update the scheduler config, save the current config and prepare + // to restore it at the end of the test. + ASSERT_OK_AND_ASSIGN( + const std::string kInitialSchedulerConfig, + GetSchedulerPolicyConfig(kSutEgressPortScheduler, *gnmi_stub)); + const auto kRestoreSchedulerConfig = absl::Cleanup([&] { + // TODO: The switch does not currently accept its own config + // on the write path, we need to fix it first. Remove this workaround once + // the issue is addressed. + std::string fixed_config = absl::StrReplaceAll(kInitialSchedulerConfig, + { + {R"(,"weight":"0")", ""}, + }); + EXPECT_OK(UpdateSchedulerPolicyConfig(kSutEgressPortScheduler, fixed_config, + *gnmi_stub)) + << "failed to restore initial scheduler config -- switch config may be " + "corrupted, causing subsequent test to fail"; + }); + // Update scheduler config. + { + absl::flat_hash_map params; + for (auto &[queue, pir] : kPirByQueueName) { + params[queue].peak_information_rate = pir; + // To avoid a large initial burst of forwarded packets when we start the + // test packet flow, we use a minimally-sized token buffer. This is to + // ensure that the observed information rate will closely matche the peak + // information rate. See RFC 2698 for details. + params[queue].excess_burst_size = kTestFrameSizeInBytes; + // TODO: Consider picking this uniformly from [0, PIR] instead. + params[queue].committed_information_rate = 0; + params[queue].committed_burst_size = 0; + } + ASSERT_OK(SetSchedulerPolicyParameters(kSutEgressPortScheduler, params, + *gnmi_stub)); + } + // Dump initial and modified configs, to ease debugging. + ASSERT_OK(testbed->Environment().StoreTestArtifact( + absl::StrCat(kSutEgressPortScheduler, "_before_update.json"), + FormatJsonBestEffort(kInitialSchedulerConfig))); + ASSERT_OK_AND_ASSIGN( + std::string updated_scheduler_config, + GetSchedulerPolicyConfig(kSutEgressPortScheduler, *gnmi_stub)); + ASSERT_OK(testbed->Environment().StoreTestArtifact( + absl::StrCat(kSutEgressPortScheduler, "_after_update.json"), + FormatJsonBestEffort(updated_scheduler_config))); + + // Connect to Ixia and fix global parameters. + ASSERT_OK_AND_ASSIGN(const std::string kIxiaHandle, ConnectToIxia(*testbed)); + ASSERT_OK_AND_ASSIGN(const std::string kIxiaSrcPortHandle, + ixia::IxiaVport(kIxiaHandle, kIxiaSrcPort, *testbed)); + ASSERT_OK_AND_ASSIGN(const std::string kIxiaDstPortHandle, + ixia::IxiaVport(kIxiaHandle, kIxiaDstPort, *testbed)); + + // Actual testing -- inject test IPv4 and IPv6 packets for each DSCP, and + // check the behavior is as eexpted. + constexpr int kMaxDscp = 63; + for (int dscp = 0; dscp <= kMaxDscp; ++dscp) { + SCOPED_TRACE(absl::StrCat("DSCP = ", dscp)); + for (bool is_ipv4 : {true, false}) { + SCOPED_TRACE(absl::StrCat("IP version: ", is_ipv4 ? "IPv4" : "IPv6")); + + // Figure out which queue we will be targeting and some queue parameters. + ASSERT_OK_AND_ASSIGN( + const std::string kTargetQueue, + GetQueueNameByDscpAndPort(dscp, kSutEgressPort, *gnmi_stub)); + // TODO(b/230107242: Test strict queues once that is supported. + if (kTargetQueue == "AF4" || kTargetQueue == "NC1") { + LOG(WARNING) << "skipping test for unuspported queue '" << kTargetQueue + << "'"; + continue; + } + ASSERT_OK_AND_ASSIGN(const int kTargetQueuePir, + gutil::FindOrStatus(kPirByQueueName, kTargetQueue)); + ASSERT_OK_AND_ASSIGN( + const QueueCounters kInitialQueueCounters, + GetGnmiQueueCounters(kSutEgressPort, kTargetQueue, *gnmi_stub)); + + // Configure & start test packet flow. + const std::string kTrafficName = + absl::StrCat((is_ipv4 ? "IPv4" : "IPv6"), " traffic for DSCP ", dscp, + ", targeting queue ", kTargetQueue, + " with PIR = ", kTargetQueuePir, " bytes/second"); + SCOPED_TRACE(kTrafficName); + ASSERT_OK_AND_ASSIGN(const std::string kIxiaTrafficHandle, + ixia::SetUpTrafficItem(kIxiaSrcPortHandle, + kIxiaDstPortHandle, + kTrafficName, *testbed)); + auto delete_traffic_item = absl::Cleanup([&, kIxiaTrafficHandle] { + ASSERT_OK(ixia::DeleteTrafficItem(kIxiaTrafficHandle, *testbed)); + }); + auto traffix_parameters = ixia::TrafficParameters{ + .frame_count = kTestFrameCount, + .frame_size_in_bytes = kTestFrameSizeInBytes, + .traffic_speed = ixia::FramesPerSecond{kTestFramesPerSecond}, + }; + if (is_ipv4) { + traffix_parameters.ip_parameters = ixia::Ipv4TrafficParameters{ + .src_ipv4 = netaddr::Ipv4Address(192, 168, 2, dscp + 1), + .dst_ipv4 = netaddr::Ipv4Address(172, 0, 0, dscp + 1), + // Set ECN 0 to avoid RED drops. + .priority = ixia::IpPriority{.dscp = dscp, .ecn = 1}, + }; + } else { + traffix_parameters.ip_parameters = ixia::Ipv6TrafficParameters{ + .src_ipv6 = + netaddr::Ipv6Address(0x1000, 0, 0, 0, 0, 0, 0, dscp + 1), + .dst_ipv6 = + netaddr::Ipv6Address(0x2000, 0, 0, 0, 0, 0, 0, dscp + 1), + // Set ECN 0 to avoid RED drops. + .priority = ixia::IpPriority{.dscp = dscp, .ecn = 0}, + }; + } + LOG(INFO) << "-- starting " << kTrafficName; + ASSERT_OK(ixia::SetTrafficParameters(kIxiaTrafficHandle, + traffix_parameters, *testbed)); + // Occasionally the Ixia API cannot keep up and starting traffic fails, + // so we try up to 3 times. + ASSERT_OK(pins::TryUpToNTimes(3, /*delay=*/absl::Seconds(1), [&] { + return ixia::StartTraffic(kIxiaTrafficHandle, kIxiaHandle, *testbed); + })); + const absl::Duration kTrafficDuration = + absl::Seconds(kTestFrameCount / kTestFramesPerSecond); + LOG(INFO) << "traffic started, waiting for " << kTrafficDuration + << " to complete"; + absl::SleepFor(kTrafficDuration); + + // Check observed traffic rate against target queue PIR. + ASSERT_OK_AND_ASSIGN( + const ixia::TrafficItemStats kIxiaTrafficStats, + ixia::GetTrafficItemStats(kIxiaHandle, kTrafficName, *testbed)); + const int kObservedTrafficRate = + ixia::BytesPerSecondReceived(kIxiaTrafficStats); + LOG(INFO) << "observed traffic rate (bytes/second): " + << kObservedTrafficRate; + const int kAbsoluteError = kObservedTrafficRate - kTargetQueuePir; + const double kRelativeErrorPercent = + 100. * kAbsoluteError / kTargetQueuePir; + constexpr double kTolerancePercent = 10; // +-10% tolerance. + if (std::abs(kRelativeErrorPercent) <= kTolerancePercent) { + LOG(INFO) + << "observed traffic rate matches expected traffic rate (error: " + << kRelativeErrorPercent << "%)"; + } else { + ADD_FAILURE() << "observed traffic rate of " << kObservedTrafficRate + << " bytes/second is not within " << kTolerancePercent + << "% of the PIR of the queue '" << kTargetQueue + << "' targeted by traffic (PIR = " << kTargetQueuePir + << " bytes/second, error = " << kRelativeErrorPercent + << "%)"; + } + + // Verify that the target egress queue counters incremented. + const absl::Time kDeadline = absl::Now() + kMaxQueueCounterUpdateTime; + LOG(INFO) << "polling queue counters (this may take up to " + << kMaxQueueCounterUpdateTime << ")"; + QueueCounters final_counters, delta_counters; + do { + ASSERT_OK_AND_ASSIGN( + final_counters, + GetGnmiQueueCounters(kSutEgressPort, kTargetQueue, *gnmi_stub)); + delta_counters = final_counters - kInitialQueueCounters; + } while (TotalPacketsForQueue(delta_counters) < + kIxiaTrafficStats.num_tx_frames() && + absl::Now() < kDeadline); + LOG(INFO) << "queue counters incremented by " << delta_counters; + SCOPED_TRACE(absl::StrCat("Counters for queue ", kTargetQueue, + " did not change as expected within ", + ToString(kMaxQueueCounterUpdateTime), + " after injecting/receiving back ", + kIxiaTrafficStats.num_tx_frames(), "/", + kIxiaTrafficStats.num_rx_frames(), + " test packets from/at the Ixia.\nBefore: ", + ToString(kInitialQueueCounters), + "\nAfter :", ToString(final_counters))); + EXPECT_THAT(delta_counters, + ResultOf(TotalPacketsForQueue, + Eq(kIxiaTrafficStats.num_tx_frames()))); + EXPECT_THAT(delta_counters, Field(&QueueCounters::num_packets_transmitted, + Eq(kIxiaTrafficStats.num_rx_frames()))); + } + } + LOG(INFO) << "-- Test done -------------------------------------------------"; } -// Set up the switch to forward inbound packets to the egress port using default -// route in VRF. -// The rules will forward all matching packets matching source MAC address -// to the egress port specified. +struct EcnTestPacketCounters { + absl::Mutex mutex; + int num_packets_punted ABSL_GUARDED_BY(mutex) = 0; + int num_packets_ecn_marked ABSL_GUARDED_BY(mutex) = 0; +}; + +void ResetEcnTestPacketCounters(EcnTestPacketCounters &packet_receive_info) { + absl::MutexLock lock(&packet_receive_info.mutex); + packet_receive_info.num_packets_punted = 0; + packet_receive_info.num_packets_ecn_marked = 0; +} + +// Set up the switch to forward inbound packets to the egress port using +// default route in VRF. The rules will forward all matching packets matching +// source MAC address to the egress port specified. // // Also set up a Copy rule to CPU to punt egress packets to test for // any inspection. @@ -270,16 +672,17 @@ absl::StatusOr SetUpIxiaForEcnTest( ASSIGN_OR_RETURN( traffic_refs.emplace_back(), pins_test::ixia::SetUpTrafficItem(vport2_ref, vport3_ref, testbed)); - - return EcnTestIxiaSetUpResult{.topology_ref = topology_ref, - .traffic_refs = traffic_refs}; -} + return EcnTestIxiaSetUpResult{ + .topology_ref = topology_ref, + .traffic_refs = traffic_refs, + }; +} -// This test verifies ECN marking due to configured WRED profile on port queue. -// The test verifies the following: +// This test verifies ECN marking due to configured WRED profile on port +// queue. The test verifies the following: // 1. Switch marks Congestion Encountered bits in the ECN field for -// ECN capable traffic, when an egress port queue usage exceeds the threshold -// of the queue management profile configured for the queue. +// ECN capable traffic, when an egress port queue usage exceeds the +// threshold of the queue management profile configured for the queue. // 2. No marking when queue usage drops. // 3. The test also verifies that switch does not mark non-ECN capable traffic // even when threshold for queue usage exceeds. @@ -505,8 +908,9 @@ TEST_P(FrontpanelQosTest, TestWredEcnMarking) { traffic_ref1, frame_rate_at_line_speed_of_in_port1 + 1, *testbed)); - // TODO : kWredMaxThresholdInBytes is currently - // hardcoded in test. Get configured threshold configured from switch. + // TODO : kWredMaxThresholdInBytes is currently + // hardcoded in test. Get configured threshold configured from + // switch. constexpr int kWredMaxThresholdInBytes = 80000; // Send additional packets (twice the threshold) from second port, // to ensure threshold is crossed. @@ -556,9 +960,9 @@ TEST_P(FrontpanelQosTest, TestWredEcnMarking) { ResetEcnTestPacketCounters(packet_receive_info); absl::SleepFor(kStatsCollectionTime); - { - // We will be verifying for congestion bit in sampled packets received - // at Receiver. + { + // We will be verifying for congestion bit in sampled packets + // received at Receiver. absl::MutexLock lock(&packet_receive_info.mutex); LOG(INFO) << "Congestion : " << (is_congestion ? "true" : "false"); LOG(INFO) << "IPv4 : " << (is_ipv4 ? "true" : "false"); From 554e5fccd3e1170e7f9d9fca3d9e09b25c2288fa Mon Sep 17 00:00:00 2001 From: smolkaj Date: Mon, 16 May 2022 11:49:27 -0700 Subject: [PATCH 4/6] [Thinkit] Add test of round-robin scheduling weights. --- tests/qos/BUILD.bazel | 8 +- tests/qos/frontpanel_qos_test.cc | 332 ++++++++++++++++++++++++++++++- tests/qos/qos_test_util.cc | 117 ++++++++++- tests/qos/qos_test_util.h | 25 +++ 4 files changed, 477 insertions(+), 5 deletions(-) diff --git a/tests/qos/BUILD.bazel b/tests/qos/BUILD.bazel index 047b06920..4dc61f30e 100644 --- a/tests/qos/BUILD.bazel +++ b/tests/qos/BUILD.bazel @@ -100,13 +100,15 @@ cc_library( "//gutil:status", "//gutil:status_matchers", "//gutil:testing", - "//lib:ixia_helper", + "//lib:ixia_helper", + "//lib:ixia_helper_cc_proto", "//lib/gnmi:gnmi_helper", "//lib/utils:json_utils", "//p4_pdpi:ir", "//p4_pdpi:ir_cc_proto", "//p4_pdpi:p4_runtime_session", - "//p4_pdpi:pd", + "//p4_pdpi:pd", + "//p4_pdpi/internal:ordered_map", "//p4_pdpi/netaddr:ipv4_address", "//p4_pdpi/netaddr:ipv6_address", "//p4_pdpi/netaddr:mac_address", @@ -120,7 +122,9 @@ cc_library( "@com_github_gnmi//proto/gnmi:gnmi_cc_grpc_proto", "@com_github_p4lang_p4runtime//:p4info_cc_proto", "@com_github_p4lang_p4runtime//:p4runtime_cc_proto", + "@com_google_absl//absl/algorithm:container", "@com_google_absl//absl/cleanup", + "@com_google_absl//absl/container:btree", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", diff --git a/tests/qos/frontpanel_qos_test.cc b/tests/qos/frontpanel_qos_test.cc index 89977b6e7..9d9bec990 100644 --- a/tests/qos/frontpanel_qos_test.cc +++ b/tests/qos/frontpanel_qos_test.cc @@ -15,19 +15,25 @@ #include "tests/qos/frontpanel_qos_test.h" #include +#include #include #include +#include #include #include #include #include // NOLINT +#include #include +#include "absl/algorithm/container.h" #include "absl/cleanup/cleanup.h" +#include "absl/container/btree_set.h" #include "absl/container/flat_hash_map.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" #include "absl/strings/str_replace.h" #include "absl/strings/string_view.h" #include "absl/strings/substitute.h" @@ -42,8 +48,10 @@ #include "gutil/testing.h" #include "lib/gnmi/gnmi_helper.h" #include "lib/ixia_helper.h" +#include "lib/ixia_helper.pb.h" #include "lib/utils/json_utils.h" #include "p4/v1/p4runtime.pb.h" +#include "p4_pdpi/internal/ordered_map.h" #include "p4_pdpi/ir.h" #include "p4_pdpi/ir.pb.h" #include "p4_pdpi/netaddr/ipv4_address.h" @@ -66,8 +74,12 @@ namespace pins_test { using ::json_yang::FormatJsonBestEffort; +using ::testing::Contains; using ::testing::Eq; using ::testing::Field; +using ::testing::IsEmpty; +using ::testing::Not; +using ::testing::Pair; using ::testing::ResultOf; template std::string ToString(const T &t) { @@ -193,8 +205,8 @@ ConstructEntriesToForwardAllTrafficToGivenPort(absl::string_view p4rt_port_id) { TEST_P(FrontpanelQosTest, PacketsGetMappedToCorrectQueuesBasedOnDscpAndQueuePeakRatesAreEnforced) { LOG(INFO) << "-- Test started ----------------------------------------------"; - // Pick a testbed with SUT connected to an Ixia on 2 ports, so we can - // oversubscribe switch egress port. + // Pick a testbed with SUT connected to an Ixia on 2 ports, one ingress and + // one egress port. auto requirements = gutil::ParseProtoOrDie( R"pb( interface_requirements { count: 2 interface_mode: TRAFFIC_GENERATOR } @@ -465,6 +477,322 @@ TEST_P(FrontpanelQosTest, LOG(INFO) << "-- Test done -------------------------------------------------"; } +// The purpose of this test is to verify that weighted-round-robin-scheduled +// queues are scheduled proportionally to their weight. To test this, we inject +// two categories of traffic (all forwarded out of a single chosen egress port): +// - IPv{4,6} traffic for all round robin queues, in uniform amounts. +// We expect each round robin queue to forward a portion of traffic that is +// proportional to the queue's weight. +// - Auxilliary IPv4 traffic to a strictly prioritized queue, at 95% line rate. +// This reduces the available bandwith for the round-robin-scheduled queues +// to 5%, which ensures all round robin queues remain nonempty and the +// scheduler is able to schedule packets based on weights assigned. +TEST_P(FrontpanelQosTest, WeightedRoundRobinWeightsAreRespected) { + LOG(INFO) << "-- Test started ----------------------------------------------"; + LOG(INFO) << "obtaining testbed handle"; + // Pick a testbed with SUT connected to an Ixia on 3 ports, so we can + // oversubscribe a switch egress port. + auto requirements = gutil::ParseProtoOrDie( + R"pb( + interface_requirements { count: 3 interface_mode: TRAFFIC_GENERATOR } + )pb"); + ASSERT_OK_AND_ASSIGN( + std::unique_ptr testbed, + GetParam().testbed_interface->GetTestbedWithRequirements(requirements)); + + // Pick 3 SUT ports connected to the Ixia, 2 for receiving test packets and + // 1 for forwarding them back. We use the faster links for injecting packets + // so we can oversubsribe the egress port. We inject the traffic for the + // round-robin queues via one ingress port, and auxilliary traffic for a + // strictly-prioritized queue via another ingress port. + LOG(INFO) << "picking test packet links"; + ASSERT_OK_AND_ASSIGN(auto gnmi_stub, testbed->Sut().CreateGnmiStub()); + ASSERT_OK_AND_ASSIGN(std::vector ready_links, + GetReadyIxiaLinks(*testbed, *gnmi_stub)); + absl::c_sort(ready_links, [&](auto &x, auto &y) -> bool { + return x.sut_interface_bits_per_second < y.sut_interface_bits_per_second; + }); + ASSERT_GE(ready_links.size(), 3) + << "Test requires at least 3 SUT ports connected to an Ixia"; + const auto [kEgressLink, kIngressLink1, kIngressLink2] = + std::make_tuple(ready_links[0], ready_links[1], ready_links[2]); + ASSERT_LE(kEgressLink.sut_interface_bits_per_second, + kIngressLink1.sut_interface_bits_per_second); + ASSERT_LE(kEgressLink.sut_interface_bits_per_second, + kIngressLink2.sut_interface_bits_per_second); + const std::string kIxiaMainSrcPort = kIngressLink1.ixia_interface; + const std::string kIxiaAuxiliarySrcPort = kIngressLink2.ixia_interface; + const std::string kSutMainIngressPort = kIngressLink1.sut_interface; + const std::string kSutAuxiliayIngressPort = kIngressLink2.sut_interface; + const std::string kSutEgressPort = kEgressLink.sut_interface; + const std::string kIxiaDstPort = kEgressLink.ixia_interface; + LOG(INFO) << absl::StrFormat( + "Test packet routes:" + "\n- Main traffic: " + "[Ixia: %s] == %.1f Gbps => [SUT: %s] -> [SUT: %s] == %.1f Gbps => " + "[Ixia: %s]" + "\n- Background traffic: " + "[Ixia: %s] == %.1f Gbps => [SUT: %s] -> [SUT: %s] == %.1f Gbps => " + "[Ixia: %s]", + kIxiaMainSrcPort, + kIngressLink1.sut_interface_bits_per_second / 1'000'000'000., + kSutMainIngressPort, kSutEgressPort, + kEgressLink.sut_interface_bits_per_second / 1'000'000'000., kIxiaDstPort, + kIxiaAuxiliarySrcPort, + kIngressLink2.sut_interface_bits_per_second / 1'000'000'000., + kSutAuxiliayIngressPort, kSutEgressPort, + kEgressLink.sut_interface_bits_per_second / 1'000'000'000., kIxiaDstPort); + ASSERT_OK_AND_ASSIGN( + const std::string kSutEgressPortSchedulerPolicy, + GetSchedulerPolicyNameByEgressPort(kSutEgressPort, *gnmi_stub)); + absl::flat_hash_map p4rt_id_by_interface; + ASSERT_OK_AND_ASSIGN(p4rt_id_by_interface, + GetAllInterfaceNameToPortId(*gnmi_stub)); + ASSERT_OK_AND_ASSIGN( + const std::string kSutEgressPortP4rtId, + gutil::FindOrStatus(p4rt_id_by_interface, kSutEgressPort)); + + // Configure the switch to send all incomming packets out of the chosen egress + // port. + ASSERT_OK_AND_ASSIGN( + std::unique_ptr sut_p4rt, + ConfigureSwitchAndReturnP4RuntimeSession( + testbed->Sut(), /*gnmi_config=*/std::nullopt, GetParam().p4info)); + ASSERT_OK_AND_ASSIGN( + const sai::TableEntries kTableEntries, + ConstructEntriesToForwardAllTrafficToGivenPort(kSutEgressPortP4rtId)); + ASSERT_OK(testbed->Environment().StoreTestArtifact("pd_entries.textproto", + kTableEntries)); + ASSERT_OK(InstallPdTableEntries(kTableEntries, GetParam().p4info, *sut_p4rt)); + + // Figure out which DSCPs to use for each queue. + using DscpsByQueueName = absl::flat_hash_map>; + ASSERT_OK_AND_ASSIGN( + const DscpsByQueueName kIpv4DscpsByQueueName, + GetQueueToIpv4DscpsMapping(kSutMainIngressPort, *gnmi_stub)); + ASSERT_OK_AND_ASSIGN( + const DscpsByQueueName kIpv6DscpsByQueueName, + GetQueueToIpv4DscpsMapping(kSutMainIngressPort, *gnmi_stub)); + + // Identify round-robin queues and their weights. + using WeightByQueueName = absl::flat_hash_map; + ASSERT_OK_AND_ASSIGN(const WeightByQueueName kWeightByQueueName, + GetSchedulerPolicyWeightsByQueue( + kSutEgressPortSchedulerPolicy, *gnmi_stub)); + if (kWeightByQueueName.size() < 2) { + GTEST_SKIP() << "test pre-condition violated: expected at least 2 queues " + "with round-robin schedulers"; + } + absl::btree_set weights; + for (auto &[_, weight] : kWeightByQueueName) + weights.insert(weight); + if (weights.size() < 2) { + GTEST_SKIP() << "test pre-condition violated: expected at least 2 " + "different round-robin weights, but found only " + << weights.size() << ": " << absl::StrJoin(weights, ", "); + } + LOG(INFO) + << "Testing the following queues and associated round-robin weights:\n- " + << absl::StrJoin(kWeightByQueueName, "\n- ", + [](std::string *out, auto &queue_and_weight) { + absl::StrAppendFormat(out, "%s - weight %d", + queue_and_weight.first, + queue_and_weight.second); + }); + + // Pick a queue/DSCP that is strictly prioritized over the round robin queues. + ASSERT_OK_AND_ASSIGN(const std::vector kStrictlyPrioritzedQueues, + GetStrictlyPrioritizedQueuesInDescendingOrderOfPriority( + kSutEgressPortSchedulerPolicy, *gnmi_stub)); + ASSERT_THAT(kStrictlyPrioritzedQueues, Not(IsEmpty())); + const std::string kStrictlyPrioritzedQueue = kStrictlyPrioritzedQueues.at(0); + ASSERT_OK_AND_ASSIGN( + const std::vector kStrictlyPrioritizedIpv4Dscps, + gutil::FindOrStatus(kIpv4DscpsByQueueName, kStrictlyPrioritzedQueue)); + ASSERT_THAT(kStrictlyPrioritizedIpv4Dscps, Not(IsEmpty())); + const int kStrictlyPrioritizedIpv4Dscp = kStrictlyPrioritizedIpv4Dscps.at(0); + + // Before we update the scheduler config, save the current config and + // prepare to restore it at the end of the test. + ASSERT_OK_AND_ASSIGN( + const std::string kInitialSchedulerConfig, + GetSchedulerPolicyConfig(kSutEgressPortSchedulerPolicy, *gnmi_stub)); + const auto kRestoreSchedulerConfig = absl::Cleanup([&] { + EXPECT_OK(UpdateSchedulerPolicyConfig(kSutEgressPortSchedulerPolicy, + kInitialSchedulerConfig, *gnmi_stub)) + << "failed to restore initial scheduler config -- switch config may be " + "corrupted, causing subsequent test to fail"; + }); + + // Set lower & upper bounds (CIRs/PIRs) such that: + // - Round-robin-scheduled queues are not rate limited. + // - Auxilliary traffic to strictly prioritized queue uses at most 95% of + // egress link capacity. + LOG(INFO) << "configuring scheduler parameters"; + // Rates are in bytes/second. + const int64_t kEgressLineRateInBytesPerSecond = + kEgressLink.sut_interface_bits_per_second / 8; + const int64_t kStrictlyPrioritizedPir = .95 * kEgressLineRateInBytesPerSecond; + { + absl::flat_hash_map params_by_queue_name; + for (auto &[queue_name, _] : kWeightByQueueName) { + params_by_queue_name[queue_name].committed_information_rate = 0; + params_by_queue_name[queue_name].peak_information_rate = + kEgressLineRateInBytesPerSecond; + } + params_by_queue_name[kStrictlyPrioritzedQueue].peak_information_rate = + kStrictlyPrioritizedPir; + ASSERT_OK(SetSchedulerPolicyParameters(kSutEgressPortSchedulerPolicy, + params_by_queue_name, *gnmi_stub)); + // Dump initial and modified configs, to ease debugging. + ASSERT_OK(testbed->Environment().StoreTestArtifact( + absl::StrCat(kSutEgressPortSchedulerPolicy, "_before_update.json"), + FormatJsonBestEffort(kInitialSchedulerConfig))); + ASSERT_OK_AND_ASSIGN( + std::string updated_scheduler_config, + GetSchedulerPolicyConfig(kSutEgressPortSchedulerPolicy, *gnmi_stub)); + ASSERT_OK(testbed->Environment().StoreTestArtifact( + absl::StrCat(kSutEgressPortSchedulerPolicy, "_after_update.json"), + FormatJsonBestEffort(updated_scheduler_config))); + } + + // Connect to Ixia and fix some parameters. + LOG(INFO) << "configuring Ixia traffic"; + ASSERT_OK_AND_ASSIGN(const std::string kIxiaHandle, ConnectToIxia(*testbed)); + ASSERT_OK_AND_ASSIGN( + const std::string kIxiaMainSrcPortHandle, + ixia::IxiaVport(kIxiaHandle, kIxiaMainSrcPort, *testbed)); + ASSERT_OK_AND_ASSIGN( + const std::string kIxiaAuxiliarySrcPortHandle, + ixia::IxiaVport(kIxiaHandle, kIxiaAuxiliarySrcPort, *testbed)); + ASSERT_OK_AND_ASSIGN(const std::string kIxiaDstPortHandle, + ixia::IxiaVport(kIxiaHandle, kIxiaDstPort, *testbed)); + constexpr int kFrameSizeInBytes = 1000; + const int kFramesPerSecondAtLineRate = + .99 * kEgressLineRateInBytesPerSecond / kFrameSizeInBytes; + + // Configue IPv4 and IPv6 traffic items to all round-robin queues. + std::vector traffic_items; + absl::flat_hash_map queue_by_traffic_item_name; + const int kNumRoundRobinTrafficItems = 2 * kWeightByQueueName.size(); + for (auto &[queue_name, weight] : kWeightByQueueName) { + for (bool ipv4 : {true, false}) { + const auto &kDscpsByQueueName = + ipv4 ? kIpv4DscpsByQueueName : kIpv6DscpsByQueueName; + ASSERT_THAT(kDscpsByQueueName, + Contains(Pair(Eq(queue_name), Not(IsEmpty())))); + const int kDscp = kDscpsByQueueName.at(queue_name).at(0); + const std::string kTrafficName = absl::StrFormat( + "IPv%d packets with DSCP %d targeting queue '%s' with weight %d", + ipv4 ? 4 : 6, kDscp, queue_name, weight); + ixia::TrafficParameters traffic_params{ + .frame_size_in_bytes = kFrameSizeInBytes, + .traffic_speed = + ixia::FramesPerSecond{.frames_per_second = + kFramesPerSecondAtLineRate / + kNumRoundRobinTrafficItems}, + }; + if (ipv4) { + traffic_params.ip_parameters = ixia::Ipv4TrafficParameters{ + .priority = ixia::IpPriority{.dscp = kDscp}, + }; + } else { + traffic_params.ip_parameters = ixia::Ipv6TrafficParameters{ + .priority = ixia::IpPriority{.dscp = kDscp}, + }; + } + ASSERT_OK_AND_ASSIGN(std::string traffic_item, + ixia::SetUpTrafficItem(kIxiaMainSrcPortHandle, + kIxiaDstPortHandle, + kTrafficName, *testbed)); + ASSERT_OK( + ixia::SetTrafficParameters(traffic_item, traffic_params, *testbed)); + traffic_items.push_back(traffic_item); + queue_by_traffic_item_name[kTrafficName] = queue_name; + } + } + // Set up auxilliary traffic to strictly prioritized queue. + const std::string kAuxilliaryTrafficName = absl::StrFormat( + "Auxiliary IPv4 packets with DSCP %d targeting strictly prioritized " + "queue '%s' with PIR = %d bytes/second", + kStrictlyPrioritizedIpv4Dscp, kStrictlyPrioritzedQueue, + kStrictlyPrioritizedPir); + ASSERT_OK_AND_ASSIGN( + const std::string kAuxilliaryTrafficItem, + ixia::SetUpTrafficItem(kIxiaAuxiliarySrcPortHandle, kIxiaDstPortHandle, + kAuxilliaryTrafficName, *testbed)); + ASSERT_OK(ixia::SetTrafficParameters( + kAuxilliaryTrafficItem, + ixia::TrafficParameters{ + .frame_size_in_bytes = kFrameSizeInBytes, + .traffic_speed = ixia::FramesPerSecond{kFramesPerSecondAtLineRate}, + .ip_parameters = + ixia::Ipv4TrafficParameters{ + .priority = + ixia::IpPriority{.dscp = kStrictlyPrioritizedIpv4Dscp}, + }, + }, + *testbed)); + traffic_items.push_back(kAuxilliaryTrafficItem); + + // Start traffic. + LOG(INFO) << "starting traffic"; + ASSERT_OK(ixia::StartTraffic(traffic_items, kIxiaHandle, *testbed)); + auto stop_traffic = absl::Cleanup( + [&] { ASSERT_OK(ixia::StopTraffic(traffic_items, *testbed)); }); + LOG(INFO) << "traffic started -- sleeping for 3 second"; + absl::SleepFor(absl::Seconds(3)); + LOG(INFO) << "clearing table entries to limit buffer drainage after " + "traffic is stopped"; + ASSERT_OK(pdpi::ClearTableEntries(sut_p4rt.get())); + LOG(INFO) << "table entries cleared; stopping traffic"; + std::move(stop_traffic).Invoke(); + + // Obtain traffic stats, and ensure traffic got forwarded according to + // weights. + ASSERT_OK_AND_ASSIGN(const ixia::TrafficStats kTrafficStats, + ixia::GetAllTrafficItemStats(kIxiaHandle, *testbed)); + absl::flat_hash_map num_rx_frames_by_queue; + for (auto &[traffic_item_name, stats] : + Ordered(kTrafficStats.stats_by_traffic_item())) { + if (traffic_item_name == kAuxilliaryTrafficName) + continue; + ASSERT_OK_AND_ASSIGN( + std::string queue, + gutil::FindOrStatus(queue_by_traffic_item_name, traffic_item_name)); + num_rx_frames_by_queue[queue] += stats.num_rx_frames(); + } + int64_t total_num_rx_frames = 0; + for (auto &[_, rx] : num_rx_frames_by_queue) + total_num_rx_frames += rx; + int64_t total_weight = 0; + for (auto &[_, weight] : kWeightByQueueName) + total_weight += weight; + for (auto &[queue, num_rx_frames] : num_rx_frames_by_queue) { + ASSERT_OK_AND_ASSIGN(int64_t weight, + gutil::FindOrStatus(kWeightByQueueName, queue)); + const double kExpectedFraction = 1. * weight / total_weight; + const double kActualFraction = 1. * num_rx_frames / total_num_rx_frames; + const double kAbsoluteError = kActualFraction - kExpectedFraction; + const double kRelativeErrorPercent = + 100. * kAbsoluteError / kExpectedFraction; + const double kAcceptableErrorPercent = 3; + LOG(INFO) << "'" << queue << "' transmitted " << (kActualFraction * 100) + << "% of forwarded round-robin traffic (expected: " + << (kExpectedFraction * 100) + << "%, error: " << kRelativeErrorPercent << "%)"; + EXPECT_LE(std::abs(kRelativeErrorPercent), kAcceptableErrorPercent) + << "expected '" << queue << "' to transmit " + << (kExpectedFraction * 100) + << "% of the forwarded round-robin traffic; instead, it transmitted " + << (kActualFraction * 100) + << "% of the forwarded traffic (error: " << kRelativeErrorPercent + << "%)"; + } + + LOG(INFO) << "-- Test done -------------------------------------------------"; +} + struct EcnTestPacketCounters { absl::Mutex mutex; int num_packets_punted ABSL_GUARDED_BY(mutex) = 0; diff --git a/tests/qos/qos_test_util.cc b/tests/qos/qos_test_util.cc index 6476facaa..8cad6c4f9 100644 --- a/tests/qos/qos_test_util.cc +++ b/tests/qos/qos_test_util.cc @@ -1,7 +1,9 @@ #include "tests/qos/qos_test_util.h" +#include "absl/container/flat_hash_map.h" #include "absl/status/status.h" #include "absl/status/statusor.h" +#include "absl/strings/numbers.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" @@ -17,6 +19,7 @@ #include "lib/gnmi/gnmi_helper.h" #include "lib/gnmi/openconfig.pb.h" #include "lib/utils/json_utils.h" +#include "proto/gnmi/gnmi.grpc.pb.h" #include "proto/gnmi/gnmi.pb.h" #include "tests/qos/gnmi_parsers.h" @@ -171,9 +174,12 @@ absl::StatusOr> GetReadyIxiaLinks( { ASSIGN_OR_RETURN(sut_link_up, CheckLinkUp(interface, gnmi_stub)); if (sut_link_up) { - links.push_back({ + ASSIGN_OR_RETURN(int64_t bit_per_second, + GetPortSpeedInBitsPerSecond(interface, gnmi_stub)); + links.push_back(IxiaLink{ .ixia_interface = info.peer_interface_name, .sut_interface = interface, + .sut_interface_bits_per_second = bit_per_second, }); } } @@ -224,6 +230,30 @@ absl::StatusOr> GetIpv6DscpToQueueMapping( return GetIpv4DscpToQueueMapping(port, gnmi_stub); } +absl::StatusOr>> +GetQueueToIpv4DscpsMapping(absl::string_view port, + gnmi::gNMI::StubInterface &gnmi_stub) { + absl::flat_hash_map> dscps_by_queue; + absl::flat_hash_map queue_by_dscp; + ASSIGN_OR_RETURN(queue_by_dscp, GetIpv4DscpToQueueMapping(port, gnmi_stub)); + for (auto &[dscp, queue] : queue_by_dscp) { + dscps_by_queue[queue].push_back(dscp); + } + return dscps_by_queue; +} + +absl::StatusOr>> +GetQueueToIpv6DscpsMapping(absl::string_view port, + gnmi::gNMI::StubInterface &gnmi_stub) { + absl::flat_hash_map> dscps_by_queue; + absl::flat_hash_map queue_by_dscp; + ASSIGN_OR_RETURN(queue_by_dscp, GetIpv6DscpToQueueMapping(port, gnmi_stub)); + for (auto &[dscp, queue] : queue_by_dscp) { + dscps_by_queue[queue].push_back(dscp); + } + return dscps_by_queue; +} + absl::Status SetPortLoopbackMode(bool port_loopback, absl::string_view interface_name, gnmi::gNMI::StubInterface &gnmi_stub) { @@ -276,6 +306,20 @@ GetSchedulerPolicyConfig(absl::string_view scheduler_policy_name, return ReadGnmiPath(&gnmi, path, gnmi::GetRequest::CONFIG, ""); } +absl::StatusOr +GetSchedulerPolicyConfigAsProto(absl::string_view scheduler_policy_name, + gnmi::gNMI::StubInterface &gnmi) { + const std::string kPath = SchedulerPolicyPath(scheduler_policy_name); + const std::string kRoot = "openconfig-qos:scheduler-policy"; + ASSIGN_OR_RETURN(const std::string kRawConfig, + ReadGnmiPath(&gnmi, kPath, gnmi::GetRequest::CONFIG, kRoot)); + ASSIGN_OR_RETURN( + openconfig::Qos::SchedulerPolicy proto_config, + gutil::ParseJsonAsProto( + StripBrackets(kRawConfig), /*ignore_unknown_fields=*/true)); + return proto_config; +} + absl::Status UpdateSchedulerPolicyConfig(absl::string_view scheduler_policy_name, absl::string_view config, @@ -396,4 +440,75 @@ absl::Status SetSchedulerPolicyParameters( return absl::OkStatus(); } +absl::StatusOr> +GetSchedulerPolicyWeightsByQueue(absl::string_view scheduler_policy_name, + gnmi::gNMI::StubInterface &gnmi) { + // The mapping we're about to compute. + absl::flat_hash_map weight_by_queue_name; + + ASSIGN_OR_RETURN( + const openconfig::Qos::SchedulerPolicy kSchedulerPolicy, + GetSchedulerPolicyConfigAsProto(scheduler_policy_name, gnmi)); + for (const auto &scheduler : kSchedulerPolicy.schedulers().scheduler()) { + if (scheduler.config().priority() == "STRICT") + continue; + if (scheduler.inputs().input_size() == 0) + continue; + if (scheduler.inputs().input_size() > 1) { + return gutil::UnimplementedErrorBuilder() + << "scheduler with several inputs unsupported: " + << scheduler.DebugString(); + } + const auto &input = scheduler.inputs().input(0); + const std::string &queue = input.config().queue(); + const std::string &weight = input.config().weight(); + if (bool success = absl::SimpleAtoi(weight, &weight_by_queue_name[queue]); + !success) { + return gutil::UnknownErrorBuilder() + << "unable to parse weight '" << weight << "' for queue '" << queue + << "' in scheduler of sequence " << scheduler.config().sequence() + << " in scheduler policy '" << scheduler_policy_name << "'"; + } + } + return weight_by_queue_name; +} + +absl::StatusOr> +GetStrictlyPrioritizedQueuesInDescendingOrderOfPriority( + absl::string_view scheduler_policy_name, gnmi::gNMI::StubInterface &gnmi) { + std::vector strict_queues; + + // Read and sort schedulers. + ASSIGN_OR_RETURN( + const openconfig::Qos::SchedulerPolicy kSchedulerPolicy, + GetSchedulerPolicyConfigAsProto(scheduler_policy_name, gnmi)); + std::vector schedulers( + kSchedulerPolicy.schedulers().scheduler().begin(), + kSchedulerPolicy.schedulers().scheduler().end()); + absl::c_sort(schedulers, [](const auto &a, const auto &b) -> bool { + return a.sequence() < b.sequence(); + }); + + // Extract strictly prioritzed queues. + bool have_seen_weighted_scheduler = false; + for (const openconfig::Qos::Scheduler &scheduler : schedulers) { + if (scheduler.inputs().input_size() != 1) { + return gutil::UnimplementedErrorBuilder() + << "scheduler with none/several inputs unsupported: " + << scheduler.DebugString(); + } + const auto &input = scheduler.inputs().input(0); + if (scheduler.config().priority() == "STRICT") { + if (have_seen_weighted_scheduler) { + return gutil::UnimplementedErrorBuilder() + << "found strict scheduler after weighted scheduler"; + } + strict_queues.push_back(input.config().queue()); + } else { + have_seen_weighted_scheduler = true; + } + } + return strict_queues; +} + } // namespace pins_test diff --git a/tests/qos/qos_test_util.h b/tests/qos/qos_test_util.h index c7726de91..8a2fce172 100644 --- a/tests/qos/qos_test_util.h +++ b/tests/qos/qos_test_util.h @@ -88,6 +88,8 @@ absl::StatusOr CheckLinkUp(const std::string &iface, struct IxiaLink { std::string ixia_interface; std::string sut_interface; + // Speed of the SUT interface in bits/second. + int64_t sut_interface_bits_per_second = 0; }; // Go over the connections and return vector of connections @@ -112,6 +114,16 @@ absl::StatusOr> GetIpv4DscpToQueueMapping( absl::StatusOr> GetIpv6DscpToQueueMapping( absl::string_view port, gnmi::gNMI::StubInterface &gnmi_stub); +// Get queue to IPv4 DSCP mapping from switch. +absl::StatusOr>> +GetQueueToIpv4DscpsMapping(absl::string_view port, + gnmi::gNMI::StubInterface &gnmi_stub); + +// Get queue to IPv6 DSCP mapping from switch. +absl::StatusOr>> +GetQueueToIpv6DscpsMapping(absl::string_view port, + gnmi::gNMI::StubInterface &gnmi_stub); + // Get name of queue configured for the given DSCP. absl::StatusOr GetQueueNameByDscpAndPort(int dscp, absl::string_view port, @@ -157,6 +169,19 @@ absl::Status SetSchedulerPolicyParameters( gnmi::gNMI::StubInterface &gnmi, absl::Duration convergence_timeout = absl::Seconds(10)); +// Reads the weights of all round-robin schedulers belonging to the given +// scheduler policy from the state path, and returns them keyed by the name of +// the queue they apply to. +absl::StatusOr> +GetSchedulerPolicyWeightsByQueue(absl::string_view scheduler_policy_name, + gnmi::gNMI::StubInterface &gnmi); + +// Reads all strictly prioritized queues belonging to the given scheduler policy +// from the state paths, and returns them in descrending order of priority. +absl::StatusOr> +GetStrictlyPrioritizedQueuesInDescendingOrderOfPriority( + absl::string_view scheduler_policy_name, gnmi::gNMI::StubInterface &gnmi); + } // namespace pins_test #endif // PINS_TESTS_QOS_QOS_TEST_UTIL_H_ From 5352f56d40778f491eff0376fc9d945d07ad00f7 Mon Sep 17 00:00:00 2001 From: smolkaj Date: Mon, 16 May 2022 16:45:10 -0700 Subject: [PATCH 5/6] [Thinkit] Add test for strict queues, ensuring they are strictly prioritized. --- lib/ixia_helper.cc | 18 +- tests/qos/BUILD.bazel | 1 + tests/qos/frontpanel_qos_test.cc | 358 +++++++++++++++++++++++++++++++ tests/qos/qos_test_util.cc | 90 +++++--- tests/qos/qos_test_util.h | 23 +- 5 files changed, 451 insertions(+), 39 deletions(-) diff --git a/lib/ixia_helper.cc b/lib/ixia_helper.cc index 42675fb30..0fd624ecb 100644 --- a/lib/ixia_helper.cc +++ b/lib/ixia_helper.cc @@ -342,7 +342,7 @@ absl::StatusOr SetUpTrafficItem( absl::Status DeleteTrafficItem(absl::string_view tref, thinkit::GenericTestbed &testbed) { - RETURN_IF_ERROR(StopTraffic(tref, testbed)); + StopTraffic(tref, testbed).IgnoreError(); LOG(INFO) << "Sending DELETE to '" << tref << "'"; ASSIGN_OR_RETURN( thinkit::HttpResponse response, @@ -507,11 +507,17 @@ absl::Status StartTraffic(absl::Span trefs, for (const Json &traffic_item : response) { RET_CHECK(traffic_item.contains("errors")) << titem_response; RET_CHECK(traffic_item.contains("warnings")) << titem_response; - if (!traffic_item.at("errors").empty() && - !traffic_item.at("warnings").empty()) { - LOG(WARNING) << "Found traffic items with warnings, which may result in " - "unexpected behavior. Dump of traffic items:\n" - << json_yang::DumpJson(response); + if (!traffic_item.at("errors").empty()) { + return gutil::UnknownErrorBuilder() + << "Found traffic items with errors, which may result in " + "unexpected behavior. Dump of traffic items:\n" + << json_yang::DumpJson(response); + } + if (!traffic_item.at("warnings").empty()) { + LOG(INFO) + << "WARNING: Found traffic items with warnings, which may result in " + "unexpected behavior. Dump of traffic items:\n" + << json_yang::DumpJson(response); } } diff --git a/tests/qos/BUILD.bazel b/tests/qos/BUILD.bazel index 4dc61f30e..ca585b1b4 100644 --- a/tests/qos/BUILD.bazel +++ b/tests/qos/BUILD.bazel @@ -126,6 +126,7 @@ cc_library( "@com_google_absl//absl/cleanup", "@com_google_absl//absl/container:btree", "@com_google_absl//absl/container:flat_hash_map", + "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", diff --git a/tests/qos/frontpanel_qos_test.cc b/tests/qos/frontpanel_qos_test.cc index 9d9bec990..4b3de4c4b 100644 --- a/tests/qos/frontpanel_qos_test.cc +++ b/tests/qos/frontpanel_qos_test.cc @@ -14,6 +14,7 @@ #include "tests/qos/frontpanel_qos_test.h" +#include #include #include #include @@ -30,6 +31,7 @@ #include "absl/cleanup/cleanup.h" #include "absl/container/btree_set.h" #include "absl/container/flat_hash_map.h" +#include "absl/container/flat_hash_set.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" @@ -793,6 +795,362 @@ TEST_P(FrontpanelQosTest, WeightedRoundRobinWeightsAreRespected) { LOG(INFO) << "-- Test done -------------------------------------------------"; } +// The purpose of this test is to verify that strict queues are strictly +// prioritized over all lower-priority queues. +// We test one strict "queue under test" at a time by injecting two types of +// traffic: +// - Main traffic: IPv{4,6} packets targetting the strict queue, at >= 100% +// egress line rate. +// - Background traffic: mixed IPv{4,6} packets targetting all queues, at 100% +// egress line rate overall. +// +// We then verify that the queue under test forwards traffic at a rate +// R = egress line rate - background traffic rate to higher priority queues +// - sum of CIRs of other queues +// where the CIRs are chosen low enough to be fully saturated by the background +// traffic. We repeat this for CIRs = 0 and CIRs > 0. +TEST_P(FrontpanelQosTest, StrictQueuesAreStrictlyPrioritized) { + LOG(INFO) << "-- Test started ----------------------------------------------"; + LOG(INFO) << "obtaining testbed handle"; + // Pick a testbed with SUT connected to an Ixia on 3 ports, so we can + // oversubscribe a switch egress port. + auto requirements = gutil::ParseProtoOrDie( + R"pb( + interface_requirements { count: 3 interface_mode: TRAFFIC_GENERATOR } + )pb"); + ASSERT_OK_AND_ASSIGN( + std::unique_ptr testbed, + GetParam().testbed_interface->GetTestbedWithRequirements(requirements)); + + // Pick 3 SUT ports connected to the Ixia, 2 for receiving test packets and + // 1 for forwarding them back. We use the faster links for injecting packets + // so we can oversubsribe the egress port. We inject the traffic for the + // round-robin queues via one ingress port, and auxilliary traffic for a + // strictly-prioritized queue via another ingress port. + LOG(INFO) << "picking test packet links"; + ASSERT_OK_AND_ASSIGN(auto gnmi_stub, testbed->Sut().CreateGnmiStub()); + ASSERT_OK_AND_ASSIGN(std::vector ready_links, + GetReadyIxiaLinks(*testbed, *gnmi_stub)); + absl::c_sort(ready_links, [&](auto &x, auto &y) -> bool { + return x.sut_interface_bits_per_second < y.sut_interface_bits_per_second; + }); + ASSERT_GE(ready_links.size(), 3) + << "Test requires at least 3 SUT ports connected to an Ixia"; + const auto [kEgressLink, kIngressLink1, kIngressLink2] = + std::make_tuple(ready_links[0], ready_links[1], ready_links[2]); + ASSERT_LE(kEgressLink.sut_interface_bits_per_second, + kIngressLink1.sut_interface_bits_per_second); + ASSERT_LE(kEgressLink.sut_interface_bits_per_second, + kIngressLink2.sut_interface_bits_per_second); + const std::string kIxiaMainTrafficSrcPort = kIngressLink1.ixia_interface; + const std::string kIxiaBackgroundTrafficSrcPort = + kIngressLink2.ixia_interface; + const std::string kSutMainTrafficInPort = kIngressLink1.sut_interface; + const std::string kSutBackgroundTrafficInPort = kIngressLink2.sut_interface; + const std::string kSutEgressPort = kEgressLink.sut_interface; + const std::string kIxiaDstPort = kEgressLink.ixia_interface; + LOG(INFO) << absl::StrFormat( + "Test packet routes:" + "\n- Main traffic: " + "[Ixia: %s] == %.1f Gbps => [SUT: %s] -> [SUT: %s] == %.1f Gbps => " + "[Ixia: %s]" + "\n- Background traffic: " + "[Ixia: %s] == %.1f Gbps => [SUT: %s] -> [SUT: %s] == %.1f Gbps => " + "[Ixia: %s]", + kIxiaMainTrafficSrcPort, + kIngressLink1.sut_interface_bits_per_second / 1'000'000'000., + kSutMainTrafficInPort, kSutEgressPort, + kEgressLink.sut_interface_bits_per_second / 1'000'000'000., kIxiaDstPort, + kIxiaBackgroundTrafficSrcPort, + kIngressLink2.sut_interface_bits_per_second / 1'000'000'000., + kSutBackgroundTrafficInPort, kSutEgressPort, + kEgressLink.sut_interface_bits_per_second / 1'000'000'000., kIxiaDstPort); + ASSERT_OK_AND_ASSIGN( + const std::string kSutEgressPortSchedulerPolicy, + GetSchedulerPolicyNameByEgressPort(kSutEgressPort, *gnmi_stub)); + absl::flat_hash_map p4rt_id_by_interface; + ASSERT_OK_AND_ASSIGN(p4rt_id_by_interface, + GetAllInterfaceNameToPortId(*gnmi_stub)); + ASSERT_OK_AND_ASSIGN( + const std::string kSutEgressPortP4rtId, + gutil::FindOrStatus(p4rt_id_by_interface, kSutEgressPort)); + + // Configure the switch to send all incomming packets out of the chosen egress + // port. + ASSERT_OK_AND_ASSIGN( + std::unique_ptr sut_p4rt, + ConfigureSwitchAndReturnP4RuntimeSession( + testbed->Sut(), /*gnmi_config=*/std::nullopt, GetParam().p4info)); + ASSERT_OK_AND_ASSIGN( + const sai::TableEntries kTableEntries, + ConstructEntriesToForwardAllTrafficToGivenPort(kSutEgressPortP4rtId)); + ASSERT_OK(testbed->Environment().StoreTestArtifact("pd_entries.textproto", + kTableEntries)); + ASSERT_OK(InstallPdTableEntries(kTableEntries, GetParam().p4info, *sut_p4rt)); + + // Obtain queues and DSCPs for the queues. + ASSERT_OK_AND_ASSIGN( + const std::vector kQueuesFromHighestToLowestPriority, + GetQueuesForSchedulerPolicyInDescendingOrderOfPriority( + kSutEgressPortSchedulerPolicy, *gnmi_stub)); + std::vector strict_queues_from_highest_to_lowest_priority; + for (auto &queue : kQueuesFromHighestToLowestPriority) { + if (queue.type == QueueType::kStrictlyPrioritized) { + strict_queues_from_highest_to_lowest_priority.push_back(queue); + } + } + if (strict_queues_from_highest_to_lowest_priority.empty()) { + GTEST_SKIP() << "no strict queues configured, nothing to test"; + } + LOG(INFO) + << "Testing the following strict queues (highest to lowest priority): " + << absl::StrJoin(strict_queues_from_highest_to_lowest_priority, " > ", + [](std::string *out, const auto &queue) { + absl::StrAppend(out, queue.name); + }); + using DscpsByQueueName = absl::flat_hash_map>; + ASSERT_OK_AND_ASSIGN( + const DscpsByQueueName kIpv4DscpsByQueueName, + GetQueueToIpv4DscpsMapping(kSutBackgroundTrafficInPort, *gnmi_stub)); + ASSERT_OK_AND_ASSIGN( + const DscpsByQueueName kIpv6DscpsByQueueName, + GetQueueToIpv6DscpsMapping(kSutBackgroundTrafficInPort, *gnmi_stub)); + + // Restore scheduler config at the end of the test, so we can freely modify. + ASSERT_OK_AND_ASSIGN( + const std::string kInitialSchedulerConfig, + GetSchedulerPolicyConfig(kSutEgressPortSchedulerPolicy, *gnmi_stub)); + const auto kRestoreSchedulerConfig = absl::Cleanup([&] { + EXPECT_OK(UpdateSchedulerPolicyConfig(kSutEgressPortSchedulerPolicy, + kInitialSchedulerConfig, *gnmi_stub)) + << "failed to restore initial scheduler config -- switch config may be " + "corrupted, causing subsequent test to fail"; + }); + + // Connect to Ixia and fix constant traffic parameters. + LOG(INFO) << "connecting to Ixia"; + ASSERT_OK_AND_ASSIGN(const std::string kIxiaHandle, ConnectToIxia(*testbed)); + ASSERT_OK_AND_ASSIGN( + const std::string kIxiaMainTrafficSrcPortHandle, + ixia::IxiaVport(kIxiaHandle, kIxiaMainTrafficSrcPort, *testbed)); + ASSERT_OK_AND_ASSIGN( + const std::string kIxiaBackgroundTrafficSrcPortHandle, + ixia::IxiaVport(kIxiaHandle, kIxiaBackgroundTrafficSrcPort, *testbed)); + ASSERT_OK_AND_ASSIGN(const std::string kIxiaDstPortHandle, + ixia::IxiaVport(kIxiaHandle, kIxiaDstPort, *testbed)); + const int kNumQueueus = kQueuesFromHighestToLowestPriority.size(); + const int kNumRoundRobinQueues = + kNumQueueus - strict_queues_from_highest_to_lowest_priority.size(); + const int kNumBackgroundTrafficItems = 2 * kNumQueueus; // IPv4 & IPv6 + constexpr int kFrameSizeInBytes = 1514; + const int64_t kEgressLineRateInBytesPerSecond = + kEgressLink.sut_interface_bits_per_second / 8; + const int kEgressLineRateInFramesPerSecond = + .99 * kEgressLineRateInBytesPerSecond / kFrameSizeInBytes; + const int kFramesPerSecondPerTrafficItem = + kEgressLineRateInFramesPerSecond / kNumBackgroundTrafficItems; + const int64_t kBytesPerSecondPerTrafficItem = + kEgressLineRateInBytesPerSecond / kNumBackgroundTrafficItems; + + // Main test, testing one strict queue under test at a time. + for (const QueueInfo &queue_under_test : + strict_queues_from_highest_to_lowest_priority) { + LOG(INFO) << "== QUEUE UNDER TEST: " << queue_under_test.name << " ======="; + SCOPED_TRACE(absl::StrCat("queue under test: ", queue_under_test.name)); + + // We run the test in two variants: + // - Without CIRs. In this case, the strict queue under test competes + // for egress bandwith only with higher prioritized strict queues. + // - With CIRs. In this case, the strict queue under test competes + // for egress bandwith also with all queues with a CIR, since CIRs get + // served first. We configure CIRs uniformly, for round robin queues only. + for (const double kCirsFractionOfEgressLineRate : {0., .40}) { + SCOPED_TRACE(absl::StrCat("CIR fraction of egress line rate: ", + kCirsFractionOfEgressLineRate * 100, "%")); + // Set lower & upper queue bounds (CIRs/PIRs). + LOG(INFO) << "configuring scheduler parameters: CIRs = " + << kCirsFractionOfEgressLineRate * 100 << "% egress line rate"; + absl::flat_hash_map + params_by_queue_name; + // We split the overall CIR uniformly among all round robin queues. + const int64_t kRoundRobinQueueCir = kCirsFractionOfEgressLineRate * + kEgressLineRateInBytesPerSecond / + kNumRoundRobinQueues; + for (const QueueInfo &queue : kQueuesFromHighestToLowestPriority) { + SchedulerParameters ¶ms = params_by_queue_name[queue.name]; + params.committed_information_rate = + queue.type == QueueType::kRoundRobin ? kRoundRobinQueueCir : 0; + params.peak_information_rate = kEgressLineRateInBytesPerSecond; + } + ASSERT_OK(SetSchedulerPolicyParameters(kSutEgressPortSchedulerPolicy, + params_by_queue_name, *gnmi_stub)); + + // Configure traffic. + LOG(INFO) << "configuring traffic"; + std::vector traffic_items; + absl::flat_hash_map queue_by_traffic_item_name; + auto delete_traffic_items = absl::Cleanup([&] { + LOG(INFO) << "deleting traffic items"; + for (auto &traffic_item : traffic_items) { + EXPECT_OK(pins::TryUpToNTimes(3, absl::Seconds(1), [&] { + return ixia::DeleteTrafficItem(traffic_item, *testbed); + })); + } + }); + for (bool ipv4 : {true, false}) { + const auto &kDscpsByQueueName = + ipv4 ? kIpv4DscpsByQueueName : kIpv6DscpsByQueueName; + + // Configuring main traffic targetting queue under test. + { + ASSERT_THAT( + kDscpsByQueueName, + Contains(Pair(Eq(queue_under_test.name), Not(IsEmpty())))); + const int kDscp = kDscpsByQueueName.at(queue_under_test.name).at(0); + const std::string kTrafficName = absl::StrFormat( + "main traffic: %d MB/s of IPv%d packets/second with DSCP %d " + "targeting %s queue '%s'", + kEgressLineRateInBytesPerSecond / 1'000'000, ipv4 ? 4 : 6, kDscp, + queue_under_test.type == QueueType::kStrictlyPrioritized + ? "strict" + : "round-robin", + queue_under_test.name); + ixia::TrafficParameters traffic_params{ + .frame_size_in_bytes = kFrameSizeInBytes, + // 50/50 for IPv4/IPv6. + .traffic_speed = + ixia::FramesPerSecond{kEgressLineRateInFramesPerSecond / 2}, + }; + if (ipv4) { + traffic_params.ip_parameters = ixia::Ipv4TrafficParameters{ + .priority = ixia::IpPriority{.dscp = kDscp}, + }; + } else { + traffic_params.ip_parameters = ixia::Ipv6TrafficParameters{ + .priority = ixia::IpPriority{.dscp = kDscp}, + }; + } + ASSERT_OK_AND_ASSIGN(std::string traffic_item, + ixia::SetUpTrafficItem( + kIxiaMainTrafficSrcPortHandle, + kIxiaDstPortHandle, kTrafficName, *testbed)); + ASSERT_OK(ixia::SetTrafficParameters(traffic_item, traffic_params, + *testbed)); + traffic_items.push_back(traffic_item); + queue_by_traffic_item_name[kTrafficName] = queue_under_test; + } + + // Configure background traffic, for all queues including the queue + // under test. + for (const QueueInfo &queue : kQueuesFromHighestToLowestPriority) { + const auto &kDscpsByQueueName = + ipv4 ? kIpv4DscpsByQueueName : kIpv6DscpsByQueueName; + ASSERT_THAT(kDscpsByQueueName, + Contains(Pair(Eq(queue.name), Not(IsEmpty())))); + const int kDscp = kDscpsByQueueName.at(queue.name).at(0); + const std::string kTrafficName = absl::StrFormat( + "background traffic: %d MB/s of IPv%d packets/second with DSCP " + "%d targeting %s queue '%s'", + kBytesPerSecondPerTrafficItem / 1'000'000, ipv4 ? 4 : 6, kDscp, + queue.type == QueueType::kStrictlyPrioritized ? "strict" + : "round-robin", + queue.name); + ixia::TrafficParameters traffic_params{ + .frame_size_in_bytes = kFrameSizeInBytes, + .traffic_speed = + ixia::FramesPerSecond{kFramesPerSecondPerTrafficItem}, + }; + if (ipv4) { + traffic_params.ip_parameters = ixia::Ipv4TrafficParameters{ + .priority = ixia::IpPriority{.dscp = kDscp}, + }; + } else { + traffic_params.ip_parameters = ixia::Ipv6TrafficParameters{ + .priority = ixia::IpPriority{.dscp = kDscp}, + }; + } + ASSERT_OK_AND_ASSIGN(std::string traffic_item, + ixia::SetUpTrafficItem( + kIxiaBackgroundTrafficSrcPortHandle, + kIxiaDstPortHandle, kTrafficName, *testbed)); + ASSERT_OK(ixia::SetTrafficParameters(traffic_item, traffic_params, + *testbed)); + traffic_items.push_back(traffic_item); + queue_by_traffic_item_name[kTrafficName] = queue; + } + } + + // Run traffic for a while, then obtain stats. + LOG(INFO) << "starting traffic (" << traffic_items.size() << " items)"; + // Occasionally the Ixia API cannot keep up and starting traffic fails, + // so we try up to 3 times. + ASSERT_OK(pins::TryUpToNTimes(3, /*delay=*/absl::Seconds(1), [&] { + return ixia::StartTraffic(traffic_items, kIxiaHandle, *testbed); + })); + LOG(INFO) << "traffic started; sleeping for 3 seconds"; + absl::SleepFor(absl::Seconds(3)); + LOG(INFO) << "stopping traffic"; + ASSERT_OK(ixia::StopTraffic(traffic_items, *testbed)); + + // Obtain traffic stats, and ensure queue under test got scheduled for + // expected percentage of traffic. + LOG(INFO) << "obtaining traffic stats"; + ASSERT_OK_AND_ASSIGN(const ixia::TrafficStats kTrafficStats, + ixia::GetAllTrafficItemStats(kIxiaHandle, *testbed)); + LOG(INFO) << "validating traffic stats against expectation"; + double bytes_per_second_received = 0; + for (auto &[traffic_item, stats] : + kTrafficStats.stats_by_traffic_item()) { + ASSERT_OK_AND_ASSIGN( + QueueInfo traffic_item_queue, + gutil::FindOrStatus(queue_by_traffic_item_name, traffic_item)); + if (traffic_item_queue.name == queue_under_test.name) { + bytes_per_second_received += ixia::BytesPerSecondReceived(stats); + } + } + std::vector higher_priority_queues; + for (const QueueInfo &queue : + strict_queues_from_highest_to_lowest_priority) { + if (queue.name == queue_under_test.name) + break; + higher_priority_queues.push_back(queue.name); + } + const double kBytesPerSecondToHigherPriorityQueues = + 2 * higher_priority_queues.size() * kBytesPerSecondPerTrafficItem; + LOG(INFO) << "higher priority queues " + << absl::StrJoin(higher_priority_queues, ", ") + << " contest for " << kBytesPerSecondToHigherPriorityQueues + << " bytes/s of the egress line rate (" + << 100. * kBytesPerSecondToHigherPriorityQueues / + kEgressLineRateInBytesPerSecond + << "%)"; + LOG(INFO) << "lower priority queues with CIRs contest for " + << (kCirsFractionOfEgressLineRate * + kEgressLineRateInBytesPerSecond) + << " bytes/s of the egress line rate (" + << kCirsFractionOfEgressLineRate * 100 << "%)"; + const double kBytesPerSecondExpected = + (1 - kCirsFractionOfEgressLineRate) * + kEgressLineRateInBytesPerSecond - + kBytesPerSecondToHigherPriorityQueues; + const double kAbsoluteError = + bytes_per_second_received - kBytesPerSecondExpected; + const double kRelativeErrorPercent = + 100. * kAbsoluteError / kBytesPerSecondExpected; + const double kAcceptableErrorPercent = 3; + LOG(INFO) << "received " << bytes_per_second_received + << " bytes/s from queue " << queue_under_test.name + << " (expected: " << kBytesPerSecondExpected + << "bytes/s, error: " << kRelativeErrorPercent << "%)"; + EXPECT_LE(std::abs(kRelativeErrorPercent), kAcceptableErrorPercent) + << "expected to receive " << kBytesPerSecondExpected + << " bytes/s, but received " << bytes_per_second_received + << " bytes/s"; + } + } + LOG(INFO) << "-- Test done -------------------------------------------------"; +} + struct EcnTestPacketCounters { absl::Mutex mutex; int num_packets_punted ABSL_GUARDED_BY(mutex) = 0; diff --git a/tests/qos/qos_test_util.cc b/tests/qos/qos_test_util.cc index 8cad6c4f9..dabd69569 100644 --- a/tests/qos/qos_test_util.cc +++ b/tests/qos/qos_test_util.cc @@ -446,28 +446,12 @@ GetSchedulerPolicyWeightsByQueue(absl::string_view scheduler_policy_name, // The mapping we're about to compute. absl::flat_hash_map weight_by_queue_name; - ASSIGN_OR_RETURN( - const openconfig::Qos::SchedulerPolicy kSchedulerPolicy, - GetSchedulerPolicyConfigAsProto(scheduler_policy_name, gnmi)); - for (const auto &scheduler : kSchedulerPolicy.schedulers().scheduler()) { - if (scheduler.config().priority() == "STRICT") - continue; - if (scheduler.inputs().input_size() == 0) - continue; - if (scheduler.inputs().input_size() > 1) { - return gutil::UnimplementedErrorBuilder() - << "scheduler with several inputs unsupported: " - << scheduler.DebugString(); - } - const auto &input = scheduler.inputs().input(0); - const std::string &queue = input.config().queue(); - const std::string &weight = input.config().weight(); - if (bool success = absl::SimpleAtoi(weight, &weight_by_queue_name[queue]); - !success) { - return gutil::UnknownErrorBuilder() - << "unable to parse weight '" << weight << "' for queue '" << queue - << "' in scheduler of sequence " << scheduler.config().sequence() - << " in scheduler policy '" << scheduler_policy_name << "'"; + ASSIGN_OR_RETURN(std::vector queues, + GetQueuesForSchedulerPolicyInDescendingOrderOfPriority( + scheduler_policy_name, gnmi)); + for (auto &queue : queues) { + if (queue.type == QueueType::kRoundRobin) { + weight_by_queue_name[queue.name] = queue.weight; } } return weight_by_queue_name; @@ -477,6 +461,24 @@ absl::StatusOr> GetStrictlyPrioritizedQueuesInDescendingOrderOfPriority( absl::string_view scheduler_policy_name, gnmi::gNMI::StubInterface &gnmi) { std::vector strict_queues; + ASSIGN_OR_RETURN(std::vector queues, + GetQueuesForSchedulerPolicyInDescendingOrderOfPriority( + scheduler_policy_name, gnmi)); + for (auto &queue : queues) { + if (queue.type == QueueType::kStrictlyPrioritized) + strict_queues.push_back(queue.name); + } + return strict_queues; +} + +bool IsStrict(const openconfig::Qos::Scheduler &scheduler) { + return scheduler.config().priority() == "STRICT"; +} + +absl::StatusOr> +GetQueuesForSchedulerPolicyInDescendingOrderOfPriority( + absl::string_view scheduler_policy_name, gnmi::gNMI::StubInterface &gnmi) { + std::vector queues; // Read and sort schedulers. ASSIGN_OR_RETURN( @@ -486,11 +488,16 @@ GetStrictlyPrioritizedQueuesInDescendingOrderOfPriority( kSchedulerPolicy.schedulers().scheduler().begin(), kSchedulerPolicy.schedulers().scheduler().end()); absl::c_sort(schedulers, [](const auto &a, const auto &b) -> bool { + // TODO: Remove this temporary workaround once strict queues + // are no longer inverted. + if (IsStrict(a) && IsStrict(b)) + return a.sequence() > b.sequence(); return a.sequence() < b.sequence(); }); - // Extract strictly prioritzed queues. - bool have_seen_weighted_scheduler = false; + // Extract queue info, and ensure strict queues come before round-robin + // queues. + bool have_seen_round_robin_scheduler = false; for (const openconfig::Qos::Scheduler &scheduler : schedulers) { if (scheduler.inputs().input_size() != 1) { return gutil::UnimplementedErrorBuilder() @@ -498,17 +505,36 @@ GetStrictlyPrioritizedQueuesInDescendingOrderOfPriority( << scheduler.DebugString(); } const auto &input = scheduler.inputs().input(0); - if (scheduler.config().priority() == "STRICT") { - if (have_seen_weighted_scheduler) { - return gutil::UnimplementedErrorBuilder() - << "found strict scheduler after weighted scheduler"; + const std::string &queue = input.config().queue(); + const std::string &weight = input.config().weight(); + QueueInfo &info = queues.emplace_back(); + info = QueueInfo{ + .name = queue, + .type = IsStrict(scheduler) ? QueueType::kStrictlyPrioritized + : QueueType::kRoundRobin, + .sequence = static_cast(scheduler.sequence()), + }; + + // Extract weight, if relevant. + if (info.type == QueueType::kRoundRobin) { + have_seen_round_robin_scheduler = true; + bool parsed_weight = absl::SimpleAtoi(weight, &info.weight); + if (!parsed_weight) { + return gutil::UnknownErrorBuilder() + << "unable to parse weight '" << weight << "' for queue '" + << queue << "' in scheduler of sequence " + << scheduler.config().sequence() << " in scheduler policy '" + << scheduler_policy_name << "'"; } - strict_queues.push_back(input.config().queue()); - } else { - have_seen_weighted_scheduler = true; + } + + // Ensure invariant. + if (IsStrict(scheduler) && have_seen_round_robin_scheduler) { + return gutil::UnimplementedErrorBuilder() + << "found strict scheduler after weighted scheduler"; } } - return strict_queues; + return queues; } } // namespace pins_test diff --git a/tests/qos/qos_test_util.h b/tests/qos/qos_test_util.h index 8a2fce172..19e3af8b4 100644 --- a/tests/qos/qos_test_util.h +++ b/tests/qos/qos_test_util.h @@ -176,8 +176,29 @@ absl::StatusOr> GetSchedulerPolicyWeightsByQueue(absl::string_view scheduler_policy_name, gnmi::gNMI::StubInterface &gnmi); +enum class QueueType { + kStrictlyPrioritized, + kRoundRobin, +}; + +struct QueueInfo { + std::string name; + QueueType type; + // Priority -- queues with lower `sequence` number are scheduled first. + int sequence = 0; + // Meaningful only when `type == QueueType::kRoundRobin`. + int64_t weight = 0; +}; + +// Reads all queues belonging to the given scheduler policy and returns their +// names and types in descending order of priority. +absl::StatusOr> +GetQueuesForSchedulerPolicyInDescendingOrderOfPriority( + absl::string_view scheduler_policy_name, gnmi::gNMI::StubInterface &gnmi); + // Reads all strictly prioritized queues belonging to the given scheduler policy -// from the state paths, and returns them in descrending order of priority. +// from the state paths, and returns their names in descending order of +// priority. absl::StatusOr> GetStrictlyPrioritizedQueuesInDescendingOrderOfPriority( absl::string_view scheduler_policy_name, gnmi::gNMI::StubInterface &gnmi); From 716261dcc7f6b798d2498fd0c8797efe2bb3e4bb Mon Sep 17 00:00:00 2001 From: kishanps Date: Tue, 17 May 2022 07:18:31 -0700 Subject: [PATCH 6/6] [Thinkit] Add L3 Admit for watch port testDs, don't ignore fatal failures during switch reboot, Remove obsolete workaround, gNMI port interface tests & gNMI port interface tests. --- tests/BUILD.bazel | 1 + tests/forwarding/BUILD.bazel | 1 + tests/forwarding/watch_port_test.cc | 71 ++++++++ tests/lib/BUILD.bazel | 2 + tests/lib/switch_test_setup_helpers.cc | 18 +- tests/qos/qos_test_util.cc | 4 - tests/thinkit_gnmi_interface_util.cc | 50 +++--- tests/thinkit_gnmi_interface_util.h | 2 + tests/thinkit_gnmi_interface_util_tests.cc | 189 +++++++++------------ 9 files changed, 198 insertions(+), 140 deletions(-) diff --git a/tests/BUILD.bazel b/tests/BUILD.bazel index 9ee6942b4..6bdd1608f 100644 --- a/tests/BUILD.bazel +++ b/tests/BUILD.bazel @@ -74,6 +74,7 @@ cc_library( "@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_googletest//:gtest", ], ) diff --git a/tests/forwarding/BUILD.bazel b/tests/forwarding/BUILD.bazel index 7f9aac800..5255b4ecb 100644 --- a/tests/forwarding/BUILD.bazel +++ b/tests/forwarding/BUILD.bazel @@ -306,6 +306,7 @@ cc_library( "//sai_p4/instantiations/google:sai_p4info_cc", "//sai_p4/instantiations/google:sai_pd_cc_proto", "//tests:thinkit_sanity_tests", + "//tests/lib:p4rt_fixed_table_programming_helper", "//thinkit:mirror_testbed", "//thinkit:mirror_testbed_fixture", "//thinkit:switch", diff --git a/tests/forwarding/watch_port_test.cc b/tests/forwarding/watch_port_test.cc index cfb4d4d03..e5e6176ec 100644 --- a/tests/forwarding/watch_port_test.cc +++ b/tests/forwarding/watch_port_test.cc @@ -64,6 +64,7 @@ #include "tests/forwarding/group_programming_util.h" #include "tests/forwarding/packet_test_util.h" #include "tests/forwarding/util.h" +#include "tests/lib/p4rt_fixed_table_programming_helper.h" #include "tests/thinkit_sanity_tests.h" #include "thinkit/mirror_testbed.h" #include "thinkit/mirror_testbed_fixture.h" @@ -273,6 +274,17 @@ absl::StatusOr> CountNumPacketsPerPort( return num_packets_per_port; } +// Program L3 Admit table for the given mac-address. +absl::Status ProgramL3Admit(pdpi::P4RuntimeSession& p4_session, + const pdpi::IrP4Info& ir_p4info, + const L3AdmitOptions& options) { + p4::v1::WriteRequest write_request; + ASSIGN_OR_RETURN( + *write_request.add_updates(), + L3AdmitTableUpdate(ir_p4info, p4::v1::Update::INSERT, options)); + return pdpi::SetMetadataAndSendPiWriteRequest(&p4_session, write_request); +} + // Sends N packets from the control switch to sut at a rate of 500 packets/sec. absl::Status SendNPacketsToSut(int num_packets, const TestConfiguration& test_config, @@ -581,6 +593,15 @@ TEST_P(WatchPortTestFixture, VerifyBasicWcmpPacketDistribution) { p4::v1::Update::INSERT)) << "Failed to program WCMP group: "; + // Allow the destination mac address to L3. + ASSERT_OK(ProgramL3Admit( + *sut_p4_session_, ir_p4info, + L3AdmitOptions{ + .priority = 2070, + .dst_mac = std ::make_pair(pins::GetIthDstMac(0).ToString(), + "FF:FF:FF:FF:FF:FF"), + })); + // Program default routing for all packets on SUT. ASSERT_OK(ProgramDefaultRoutes(*sut_p4_session_, ir_p4info, kVrfId, p4::v1::Update::INSERT)); @@ -660,6 +681,16 @@ TEST_P(WatchPortTestFixture, VerifyBasicWatchPortAction) { ASSERT_OK(pins::ProgramGroupWithMembers(environment, *sut_p4_session_, ir_p4info, kGroupId, members, p4::v1::Update::INSERT)); + + // Allow the destination mac address to L3. + ASSERT_OK(ProgramL3Admit( + *sut_p4_session_, ir_p4info, + L3AdmitOptions{ + .priority = 2070, + .dst_mac = std ::make_pair(pins::GetIthDstMac(0).ToString(), + "FF:FF:FF:FF:FF:FF"), + })); + // Program default routing for all packets on SUT. ASSERT_OK(ProgramDefaultRoutes(*sut_p4_session_, ir_p4info, kVrfId, p4::v1::Update::INSERT)); @@ -774,6 +805,16 @@ TEST_P(WatchPortTestFixture, VerifyWatchPortActionInCriticalState) { ASSERT_OK(pins::ProgramGroupWithMembers(environment, *sut_p4_session_, ir_p4info, kGroupId, members, p4::v1::Update::INSERT)); + + // Allow the destination mac address to L3. + ASSERT_OK(ProgramL3Admit( + *sut_p4_session_, ir_p4info, + L3AdmitOptions{ + .priority = 2070, + .dst_mac = std ::make_pair(pins::GetIthDstMac(0).ToString(), + "FF:FF:FF:FF:FF:FF"), + })); + // Program default routing for all packets on SUT. ASSERT_OK(ProgramDefaultRoutes(*sut_p4_session_, ir_p4info, kVrfId, p4::v1::Update::INSERT)); @@ -881,6 +922,16 @@ TEST_P(WatchPortTestFixture, VerifyWatchPortActionForSingleMember) { ASSERT_OK(pins::ProgramGroupWithMembers(environment, *sut_p4_session_, ir_p4info, kGroupId, members, p4::v1::Update::INSERT)); + + // Allow the destination mac address to L3. + ASSERT_OK(ProgramL3Admit( + *sut_p4_session_, ir_p4info, + L3AdmitOptions{ + .priority = 2070, + .dst_mac = std ::make_pair(pins::GetIthDstMac(0).ToString(), + "FF:FF:FF:FF:FF:FF"), + })); + // Program default routing for all packets on SUT. ASSERT_OK(ProgramDefaultRoutes(*sut_p4_session_, ir_p4info, kVrfId, p4::v1::Update::INSERT)); @@ -994,6 +1045,16 @@ TEST_P(WatchPortTestFixture, VerifyWatchPortActionForMemberModify) { ASSERT_OK(pins::ProgramGroupWithMembers(environment, *sut_p4_session_, ir_p4info, kGroupId, members, p4::v1::Update::INSERT)); + + // Allow the destination mac address to L3. + ASSERT_OK(ProgramL3Admit( + *sut_p4_session_, ir_p4info, + L3AdmitOptions{ + .priority = 2070, + .dst_mac = std ::make_pair(pins::GetIthDstMac(0).ToString(), + "FF:FF:FF:FF:FF:FF"), + })); + // Program default routing for all packets on SUT. ASSERT_OK(ProgramDefaultRoutes(*sut_p4_session_, ir_p4info, kVrfId, p4::v1::Update::INSERT)); @@ -1123,6 +1184,16 @@ TEST_P(WatchPortTestFixture, VerifyWatchPortActionForDownPortMemberInsert) { ASSERT_OK(pins::ProgramGroupWithMembers(environment, *sut_p4_session_, ir_p4info, kGroupId, members, p4::v1::Update::INSERT)); + + // Allow the destination mac address to L3. + ASSERT_OK(ProgramL3Admit( + *sut_p4_session_, ir_p4info, + L3AdmitOptions{ + .priority = 2070, + .dst_mac = std ::make_pair(pins::GetIthDstMac(0).ToString(), + "FF:FF:FF:FF:FF:FF"), + })); + // Program default routing for all packets on SUT. ASSERT_OK(ProgramDefaultRoutes(*sut_p4_session_, ir_p4info, kVrfId, p4::v1::Update::INSERT)); diff --git a/tests/lib/BUILD.bazel b/tests/lib/BUILD.bazel index 543322d0a..2aa8bb46e 100644 --- a/tests/lib/BUILD.bazel +++ b/tests/lib/BUILD.bazel @@ -85,6 +85,7 @@ cc_library( deps = [ "//gutil:collections", "//gutil:proto", + "//gutil:status", "//lib/gnmi:gnmi_helper", "//lib/gnmi:openconfig_cc_proto", "//lib/validator:validator_lib", @@ -106,6 +107,7 @@ cc_library( "@com_google_absl//absl/time", "@com_google_absl//absl/types:optional", "@com_google_absl//absl/types:span", + "@com_google_googletest//:gtest", ], ) diff --git a/tests/lib/switch_test_setup_helpers.cc b/tests/lib/switch_test_setup_helpers.cc index fc116bc91..114810b10 100644 --- a/tests/lib/switch_test_setup_helpers.cc +++ b/tests/lib/switch_test_setup_helpers.cc @@ -18,6 +18,7 @@ #include "absl/types/optional.h" #include "absl/types/span.h" #include "glog/logging.h" +#include "gtest/gtest.h" #include "gutil/collections.h" #include "gutil/proto.h" #include "gutil/status.h" @@ -54,6 +55,21 @@ absl::Status PushGnmiAndWaitForConvergence(thinkit::Switch& thinkit_switch, gnmi_timeout); } +// Wrapper around `TestGnoiSystemColdReboot` that ensures we don't ignore fatal +// failures. +absl::Status Reboot(thinkit::Switch& thinkit_switch) { + if (::testing::Test::HasFatalFailure()) { + return gutil::UnknownErrorBuilder() + << "skipping switch reboot due to pre-existing, fatal test failure"; + } + TestGnoiSystemColdReboot(thinkit_switch); + if (::testing::Test::HasFatalFailure()) { + return gutil::UnknownErrorBuilder() + << "switch reboot failed with fatal error"; + } + return absl::OkStatus(); +} + absl::StatusOr> CreateP4RuntimeSessionAndOptionallyPushP4Info( thinkit::Switch& thinkit_switch, @@ -75,7 +91,7 @@ CreateP4RuntimeSessionAndOptionallyPushP4Info( "but I am asked to push a P4Info with the following diff:\n" << p4info_diff; RETURN_IF_ERROR(session->Finish()); - TestGnoiSystemColdReboot(thinkit_switch); + RETURN_IF_ERROR(Reboot(thinkit_switch)); // Reconnect after reboot. ASSIGN_OR_RETURN( session, pdpi::P4RuntimeSession::Create(thinkit_switch, metadata)); diff --git a/tests/qos/qos_test_util.cc b/tests/qos/qos_test_util.cc index dabd69569..5092d6795 100644 --- a/tests/qos/qos_test_util.cc +++ b/tests/qos/qos_test_util.cc @@ -488,10 +488,6 @@ GetQueuesForSchedulerPolicyInDescendingOrderOfPriority( kSchedulerPolicy.schedulers().scheduler().begin(), kSchedulerPolicy.schedulers().scheduler().end()); absl::c_sort(schedulers, [](const auto &a, const auto &b) -> bool { - // TODO: Remove this temporary workaround once strict queues - // are no longer inverted. - if (IsStrict(a) && IsStrict(b)) - return a.sequence() > b.sequence(); return a.sequence() < b.sequence(); }); diff --git a/tests/thinkit_gnmi_interface_util.cc b/tests/thinkit_gnmi_interface_util.cc index 26b7cfc9a..bd1df62c5 100644 --- a/tests/thinkit_gnmi_interface_util.cc +++ b/tests/thinkit_gnmi_interface_util.cc @@ -17,6 +17,7 @@ #include "absl/strings/match.h" #include "absl/strings/numbers.h" #include "absl/strings/str_cat.h" +#include "absl/strings/str_format.h" #include "absl/strings/str_join.h" #include "absl/strings/str_replace.h" #include "absl/strings/str_split.h" @@ -354,10 +355,10 @@ absl::StatusOr GetSlotPortLaneForPort( } auto slot_port_lane_str = port.substr(kEthernetLen); std::vector values = absl::StrSplit(slot_port_lane_str, '/'); - if (values.size() != 3) { - return absl::InvalidArgumentError( - absl::StrCat("Requested port (", port, - ") does not have a valid format (EthernetX/Y/Z)")); + if (values.size() < kSlotPortLaneMinValues) { + return absl::InvalidArgumentError(absl::StrCat( + "Requested port (", port, + ") does not have a valid format (EthernetX/Y/Z or EthernetX/Y)")); } SlotPortLane slot_port_lane; if (!absl::SimpleAtoi(values[kSlotIndex], &slot_port_lane.slot)) { @@ -370,10 +371,15 @@ absl::StatusOr GetSlotPortLaneForPort( << "Failed to convert string (" << values[kPortIndex] << ") to integer"; } - if (!absl::SimpleAtoi(values[kLaneIndex], &slot_port_lane.lane)) { - return gutil::InternalErrorBuilder().LogError() - << "Failed to convert string (" << values[kLaneIndex] - << ") to integer"; + // Unchannelizable ports will not contain a lane number. + // Use default lane number of 1. + slot_port_lane.lane = 1; + if (values.size() == kSlotPortLaneMaxValues) { + if (!absl::SimpleAtoi(values[kLaneIndex], &slot_port_lane.lane)) { + return gutil::InternalErrorBuilder().LogError() + << "Failed to convert string (" << values[kLaneIndex] + << ") to integer"; + } } return slot_port_lane; } @@ -507,7 +513,7 @@ absl::StatusOr IsCopperPort(gnmi::gNMI::StubInterface* sut_gnmi_stub, absl::string_view port) { // Get transceiver name for the port. auto state_path = - absl::StrCat("interfaces/interface[name=", port, "]/state/transceiver"); + absl::StrFormat("interfaces/interface[name=%s]/state/transceiver", port); auto resp_parse_str = "openconfig-platform-transceiver:transceiver"; ASSIGN_OR_RETURN( auto xcvrd_name, @@ -517,27 +523,21 @@ absl::StatusOr IsCopperPort(gnmi::gNMI::StubInterface* sut_gnmi_stub, << port); StripSymbolFromString(xcvrd_name, '\"'); - // TODO: Replace with PMD type when supported. - // Get cable length for the port transceiver. - state_path = - absl::StrCat("components/component[name=", xcvrd_name, - "]/transceiver/state/openconfig-platform-ext:cable-length"); - resp_parse_str = "openconfig-platform-ext:cable-length"; + state_path = absl::StrFormat( + "components/component[name=%s]/transceiver/state/ethernet-pmd", + xcvrd_name); + resp_parse_str = "openconfig-platform-transceiver:ethernet-pmd"; ASSIGN_OR_RETURN( - auto cable_length_str, + auto ethernet_pmd, GetGnmiStatePathInfo(sut_gnmi_stub, state_path, resp_parse_str), - _ << "Failed to get GNMI state path value for cable-length for " + _ << "Failed to get GNMI state path value for ethernet-pmd for " "port " << port); - StripSymbolFromString(cable_length_str, '\"'); + StripSymbolFromString(ethernet_pmd, '\"'); - // Only cable lengths of copper ports are a positive value. - float cable_length; - if (!absl::SimpleAtof(cable_length_str, &cable_length)) { - return gutil::InternalErrorBuilder().LogError() - << "Failed to convert string (" << cable_length_str << ") to float"; - } - return (cable_length > 0); + // PMD state path value for copper ports ends in CR2/CR4/CR8. + auto pos = ethernet_pmd.find_last_of('_'); + return (ethernet_pmd.substr(pos + 1, 2) == "CR"); } absl::StatusOr GenerateInterfaceBreakoutConfig( diff --git a/tests/thinkit_gnmi_interface_util.h b/tests/thinkit_gnmi_interface_util.h index 61a592d05..862f1719c 100644 --- a/tests/thinkit_gnmi_interface_util.h +++ b/tests/thinkit_gnmi_interface_util.h @@ -38,6 +38,8 @@ inline constexpr char kEthernet[] = "Ethernet"; const int kSlotIndex = 0; const int kPortIndex = 1; const int kLaneIndex = 2; +const int kSlotPortLaneMinValues = 2; +const int kSlotPortLaneMaxValues = 3; // PortBreakoutInfo contains physical channels and operational status for an // interface. diff --git a/tests/thinkit_gnmi_interface_util_tests.cc b/tests/thinkit_gnmi_interface_util_tests.cc index bb9dd6a13..c666cb8a7 100644 --- a/tests/thinkit_gnmi_interface_util_tests.cc +++ b/tests/thinkit_gnmi_interface_util_tests.cc @@ -26,11 +26,13 @@ namespace pins_test { using gutil::EqualsProto; +using gutil::IsOkAndHolds; using gutil::StatusIs; using ::nlohmann::json; using ::testing::_; using ::testing::ContainerEq; using ::testing::DoAll; +using ::testing::FieldsAre; using ::testing::HasSubstr; using ::testing::Return; using ::testing::ReturnRefOfCopy; @@ -70,7 +72,7 @@ constexpr char get_xcvrd_resp_str[] = } )pb"; -constexpr char cable_len_req_str[] = +constexpr char ethernet_pmd_req_str[] = R"pb(prefix { origin: "openconfig" } path { elem { name: "components" } @@ -80,11 +82,11 @@ constexpr char cable_len_req_str[] = } elem { name: "transceiver" } elem { name: "state" } - elem { name: "openconfig-platform-ext:cable-length" } + elem { name: "ethernet-pmd" } } type: STATE)pb"; -constexpr char cable_len_resp_copper_str[] = +constexpr char ethernet_pmd_resp_copper_str[] = R"pb(notification { timestamp: 1631864194292383538 prefix { origin: "openconfig" } @@ -97,16 +99,16 @@ constexpr char cable_len_resp_copper_str[] = } elem { name: "transceiver" } elem { name: "state" } - elem { name: "openconfig-platform-ext:cable-length" } + elem { name: "ethernet-pmd" } } val { - json_ietf_val: "{\"openconfig-platform-ext:cable-length\":\"10\"}" + json_ietf_val: "{\"openconfig-platform-transceiver:ethernet-pmd\":\"google-pins-transceivers:ETH_2X400GBASE_CR4\"}" } } } )pb"; -constexpr char cable_len_resp_optic_str[] = +constexpr char ethernet_pmd_resp_optic_str[] = R"pb(notification { timestamp: 1631864194292383538 prefix { origin: "openconfig" } @@ -119,10 +121,10 @@ constexpr char cable_len_resp_optic_str[] = } elem { name: "transceiver" } elem { name: "state" } - elem { name: "cable-length" } + elem { name: "ethernet-pmd" } } val { - json_ietf_val: "{\"openconfig-platform-ext:cable-length\":\"0\"}" + json_ietf_val: "{\"openconfig-platform-transceiver:ethernet-pmd\":\"google-pins-transceivers:ETH_2X400GBASE_CDGR4_PLUS\"}" } } } @@ -1476,15 +1478,15 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(get_xcvrd_req), _)) .WillOnce( DoAll(SetArgPointee<2>(get_xcvrd_resp), Return(grpc::Status::OK))); - gnmi::GetRequest cable_len_req; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(cable_len_req_str, - &cable_len_req)); - gnmi::GetResponse cable_len_resp; + gnmi::GetRequest ethernet_pmd_req; ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - cable_len_resp_optic_str, &cable_len_resp)); - EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(cable_len_req), _)) + ethernet_pmd_req_str, ðernet_pmd_req)); + gnmi::GetResponse ethernet_pmd_resp; + ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( + ethernet_pmd_resp_optic_str, ðernet_pmd_resp)); + EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(ethernet_pmd_req), _)) .WillOnce( - DoAll(SetArgPointee<2>(cable_len_resp), Return(grpc::Status::OK))); + DoAll(SetArgPointee<2>(ethernet_pmd_resp), Return(grpc::Status::OK))); ASSERT_OK(pins_test::GetBreakoutModeConfigFromString( req, mock_gnmi_stub_ptr.get(), port_index, intf_name, breakout_mode)); EXPECT_THAT(req, EqualsProto(expected_breakout_config)); @@ -1517,15 +1519,15 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(get_xcvrd_req), _)) .WillOnce( DoAll(SetArgPointee<2>(get_xcvrd_resp), Return(grpc::Status::OK))); - gnmi::GetRequest cable_len_req; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(cable_len_req_str, - &cable_len_req)); - gnmi::GetResponse cable_len_resp; + gnmi::GetRequest ethernet_pmd_req; + ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( + ethernet_pmd_req_str, ðernet_pmd_req)); + gnmi::GetResponse ethernet_pmd_resp; ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - cable_len_resp_optic_str, &cable_len_resp)); - EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(cable_len_req), _)) + ethernet_pmd_resp_optic_str, ðernet_pmd_resp)); + EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(ethernet_pmd_req), _)) .WillOnce( - DoAll(SetArgPointee<2>(cable_len_resp), Return(grpc::Status::OK))); + DoAll(SetArgPointee<2>(ethernet_pmd_resp), Return(grpc::Status::OK))); ASSERT_OK(pins_test::GetBreakoutModeConfigFromString( req, mock_gnmi_stub_ptr.get(), port_index, intf_name, breakout_mode)); EXPECT_THAT(req, EqualsProto(expected_breakout_config)); @@ -1558,15 +1560,15 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(get_xcvrd_req), _)) .WillOnce( DoAll(SetArgPointee<2>(get_xcvrd_resp), Return(grpc::Status::OK))); - gnmi::GetRequest cable_len_req; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(cable_len_req_str, - &cable_len_req)); - gnmi::GetResponse cable_len_resp; + gnmi::GetRequest ethernet_pmd_req; + ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( + ethernet_pmd_req_str, ðernet_pmd_req)); + gnmi::GetResponse ethernet_pmd_resp; ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - cable_len_resp_optic_str, &cable_len_resp)); - EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(cable_len_req), _)) + ethernet_pmd_resp_optic_str, ðernet_pmd_resp)); + EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(ethernet_pmd_req), _)) .WillOnce( - DoAll(SetArgPointee<2>(cable_len_resp), Return(grpc::Status::OK))); + DoAll(SetArgPointee<2>(ethernet_pmd_resp), Return(grpc::Status::OK))); ASSERT_OK(pins_test::GetBreakoutModeConfigFromString( req, mock_gnmi_stub_ptr.get(), port_index, intf_name, breakout_mode)); EXPECT_THAT(req, EqualsProto(expected_breakout_config)); @@ -1599,15 +1601,15 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(get_xcvrd_req), _)) .WillOnce( DoAll(SetArgPointee<2>(get_xcvrd_resp), Return(grpc::Status::OK))); - gnmi::GetRequest cable_len_req; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(cable_len_req_str, - &cable_len_req)); - gnmi::GetResponse cable_len_resp; + gnmi::GetRequest ethernet_pmd_req; ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - cable_len_resp_copper_str, &cable_len_resp)); - EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(cable_len_req), _)) + ethernet_pmd_req_str, ðernet_pmd_req)); + gnmi::GetResponse ethernet_pmd_resp; + ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( + ethernet_pmd_resp_copper_str, ðernet_pmd_resp)); + EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(ethernet_pmd_req), _)) .WillOnce( - DoAll(SetArgPointee<2>(cable_len_resp), Return(grpc::Status::OK))); + DoAll(SetArgPointee<2>(ethernet_pmd_resp), Return(grpc::Status::OK))); ASSERT_OK(pins_test::GetBreakoutModeConfigFromString( req, mock_gnmi_stub_ptr.get(), port_index, intf_name, breakout_mode)); EXPECT_THAT(req, EqualsProto(expected_breakout_config)); @@ -1628,15 +1630,15 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(get_xcvrd_req), _)) .WillOnce( DoAll(SetArgPointee<2>(get_xcvrd_resp), Return(grpc::Status::OK))); - gnmi::GetRequest cable_len_req; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(cable_len_req_str, - &cable_len_req)); - gnmi::GetResponse cable_len_resp; + gnmi::GetRequest ethernet_pmd_req; + ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( + ethernet_pmd_req_str, ðernet_pmd_req)); + gnmi::GetResponse ethernet_pmd_resp; ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - cable_len_resp_optic_str, &cable_len_resp)); - EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(cable_len_req), _)) + ethernet_pmd_resp_optic_str, ðernet_pmd_resp)); + EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(ethernet_pmd_req), _)) .WillOnce( - DoAll(SetArgPointee<2>(cable_len_resp), Return(grpc::Status::OK))); + DoAll(SetArgPointee<2>(ethernet_pmd_resp), Return(grpc::Status::OK))); gnmi::SetRequest req; EXPECT_THAT( pins_test::GetBreakoutModeConfigFromString( @@ -2199,15 +2201,15 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, TestIsCopperPortSuccessOpticPort) { google::protobuf::TextFormat::ParseFromString(get_xcvrd_resp_str, &resp)); EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(req), _)) .WillOnce(DoAll(SetArgPointee<2>(resp), Return(grpc::Status::OK))); - gnmi::GetRequest cable_len_req; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(cable_len_req_str, - &cable_len_req)); - gnmi::GetResponse cable_len_resp; + gnmi::GetRequest ethernet_pmd_req; + ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( + ethernet_pmd_req_str, ðernet_pmd_req)); + gnmi::GetResponse ethernet_pmd_resp; ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - cable_len_resp_optic_str, &cable_len_resp)); - EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(cable_len_req), _)) + ethernet_pmd_resp_optic_str, ðernet_pmd_resp)); + EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(ethernet_pmd_req), _)) .WillOnce( - DoAll(SetArgPointee<2>(cable_len_resp), Return(grpc::Status::OK))); + DoAll(SetArgPointee<2>(ethernet_pmd_resp), Return(grpc::Status::OK))); EXPECT_THAT( pins_test::IsCopperPort(mock_gnmi_stub_ptr.get(), "Ethernet1/1/1"), false); @@ -2223,15 +2225,15 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, TestIsCopperPortSuccessCopperPort) { google::protobuf::TextFormat::ParseFromString(get_xcvrd_resp_str, &resp)); EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(req), _)) .WillOnce(DoAll(SetArgPointee<2>(resp), Return(grpc::Status::OK))); - gnmi::GetRequest cable_len_req; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(cable_len_req_str, - &cable_len_req)); - gnmi::GetResponse cable_len_resp; + gnmi::GetRequest ethernet_pmd_req; ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - cable_len_resp_copper_str, &cable_len_resp)); - EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(cable_len_req), _)) + ethernet_pmd_req_str, ðernet_pmd_req)); + gnmi::GetResponse ethernet_pmd_resp; + ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( + ethernet_pmd_resp_copper_str, ðernet_pmd_resp)); + EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(ethernet_pmd_req), _)) .WillOnce( - DoAll(SetArgPointee<2>(cable_len_resp), Return(grpc::Status::OK))); + DoAll(SetArgPointee<2>(ethernet_pmd_resp), Return(grpc::Status::OK))); EXPECT_THAT( pins_test::IsCopperPort(mock_gnmi_stub_ptr.get(), "Ethernet1/1/1"), true); } @@ -2250,7 +2252,7 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, TestIsCopperPortTransceiverGetFailure) { "port transceiver for port Ethernet1/1/1"))); } -TEST_F(GNMIThinkitInterfaceUtilityTest, TestIsCopperPortCableLengthGetFailure) { +TEST_F(GNMIThinkitInterfaceUtilityTest, TestIsCopperPortEthernetPmdGetFailure) { auto mock_gnmi_stub_ptr = absl::make_unique(); gnmi::GetRequest req; ASSERT_TRUE( @@ -2260,62 +2262,22 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, TestIsCopperPortCableLengthGetFailure) { google::protobuf::TextFormat::ParseFromString(get_xcvrd_resp_str, &resp)); EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(req), _)) .WillOnce(DoAll(SetArgPointee<2>(resp), Return(grpc::Status::OK))); - gnmi::GetRequest cable_len_req; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(cable_len_req_str, - &cable_len_req)); - EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(cable_len_req), _)) + gnmi::GetRequest ethernet_pmd_req; + ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( + ethernet_pmd_req_str, ðernet_pmd_req)); + EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(ethernet_pmd_req), _)) .WillOnce(Return(grpc::Status(grpc::StatusCode::DEADLINE_EXCEEDED, ""))); EXPECT_THAT( pins_test::IsCopperPort(mock_gnmi_stub_ptr.get(), "Ethernet1/1/1"), StatusIs(absl::StatusCode::kDeadlineExceeded, HasSubstr("Failed to get GNMI state path value for " - "cable-length for port Ethernet1/1/1"))); + "ethernet-pmd for port Ethernet1/1/1"))); } TEST_F(GNMIThinkitInterfaceUtilityTest, - TestIsCopperPortFloatConversionFailure) { - auto mock_gnmi_stub_ptr = absl::make_unique(); - gnmi::GetRequest req; - ASSERT_TRUE( - google::protobuf::TextFormat::ParseFromString(get_xcvrd_req_str, &req)); - gnmi::GetResponse resp; - ASSERT_TRUE( - google::protobuf::TextFormat::ParseFromString(get_xcvrd_resp_str, &resp)); - EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(req), _)) - .WillOnce(DoAll(SetArgPointee<2>(resp), Return(grpc::Status::OK))); - gnmi::GetRequest cable_len_req; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(cable_len_req_str, - &cable_len_req)); - gnmi::GetResponse cable_len_resp; - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( - R"pb(notification { - timestamp: 1631864194292383538 - prefix { origin: "openconfig" } - update { - path { - elem { name: "components" } - elem { - name: "component" - key { key: "name" value: "Ethernet1/1/1" } - } - elem { name: "transceiver" } - elem { name: "state" } - elem { name: "openconfig-platform-ext:cable-length" } - } - val { - json_ietf_val: "{\"openconfig-platform-ext:cable-length\":\"XYZ\"}" - } - } - } - )pb", - &cable_len_resp)); - EXPECT_CALL(*mock_gnmi_stub_ptr, Get(_, EqualsProto(cable_len_req), _)) - .WillOnce( - DoAll(SetArgPointee<2>(cable_len_resp), Return(grpc::Status::OK))); - EXPECT_THAT( - pins_test::IsCopperPort(mock_gnmi_stub_ptr.get(), "Ethernet1/1/1"), - StatusIs(absl::StatusCode::kInternal, - HasSubstr("Failed to convert string (XYZ) to float"))); + TestGetSlotPortLaneForPortFrontPanelPortSuccess) { + EXPECT_THAT(pins_test::GetSlotPortLaneForPort("Ethernet1/1/5"), + IsOkAndHolds(FieldsAre(1, 1, 5))); } TEST_F(GNMIThinkitInterfaceUtilityTest, @@ -2349,10 +2311,17 @@ TEST_F(GNMIThinkitInterfaceUtilityTest, TEST_F(GNMIThinkitInterfaceUtilityTest, TestGetSlotPortLaneForPortInvalidPortFormatFailure) { - EXPECT_THAT(pins_test::GetSlotPortLaneForPort("Ethernet1/1"), - StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("Requested port (Ethernet1/1) does not have a " - "valid format (EthernetX/Y/Z)"))); + EXPECT_THAT( + pins_test::GetSlotPortLaneForPort("Ethernet1"), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("Requested port (Ethernet1) does not have a " + "valid format (EthernetX/Y/Z or EthernetX/Y)"))); +} + +TEST_F(GNMIThinkitInterfaceUtilityTest, + TestGetSlotPortLaneForPortUnchannelizedPortSuccess) { + EXPECT_THAT(pins_test::GetSlotPortLaneForPort("Ethernet1/33"), + IsOkAndHolds(FieldsAre(1, 33, 1))); } TEST_F(GNMIThinkitInterfaceUtilityTest,