Skip to content

Commit

Permalink
[Thinkit] Fix the gnoi factory reset test error message matching erro…
Browse files Browse the repository at this point in the history 
…r, Allow some tolerance for protocol packets, Change neighbor entries to ipv6, Skip ACL modify during bcm-sai 7.1 upgrade, Rate limit punt traffic to control switch & Default to qos_queue 0x7 instead of 0x1. (#944)

Co-authored-by: kishanps <[email protected]>
  • Loading branch information
@divyagayathri-hcl @kishanps
divyagayathri-hcl and kishanps authored Jan 10, 2025
1 parent 3720a05 commit dac1d13
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 86 deletions.
108 changes: 55 additions & 53 deletions tests/forwarding/l3_admit_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,9 @@ absl::Status SendUdpPacket(pdpi::P4RuntimeSession& session,
ASSIGN_OR_RETURN(std::string packet,
UdpPacket(dst_mac, dst_ip,
absl::Substitute("[Packet:$0] $1", i, payload)));
RETURN_IF_ERROR(InjectEgressPacket(port_id, packet, ir_p4info, &session));
// Rate limit to 500pps to avoid punt packet drops on the control switch.
RETURN_IF_ERROR(InjectEgressPacket(port_id, packet, ir_p4info, &session,
/*packet_delay=*/absl::Milliseconds(2)));
}
return absl::OkStatus();
}
Expand All @@ -282,7 +284,7 @@ TEST_P(L3AdmitTestFixture,
.switch_ip = std::make_pair("10.0.0.1", 32),
.peer_port = "1",
.peer_mac = "00:00:00:00:00:02",
.peer_ip = "10.0.0.2",
.peer_ip = "fe80::2",
.router_interface_id = "rif-1",
.nexthop_id = "nexthop-1",
};
Expand Down Expand Up @@ -371,7 +373,7 @@ TEST_P(L3AdmitTestFixture, L3AdmitCanUseMaskToAllowMultipleMacAddresses) {
.switch_ip = std::make_pair("10.0.0.1", 32),
.peer_port = "1",
.peer_mac = "00:00:00:00:00:02",
.peer_ip = "10.0.0.2",
.peer_ip = "fe80::2",
.router_interface_id = "rif-1",
.nexthop_id = "nexthop-1",
};
Expand Down Expand Up @@ -450,7 +452,7 @@ TEST_P(L3AdmitTestFixture, DISABLED_L3AdmitCanUseInPortToRestrictMacAddresses) {
.switch_ip = std::make_pair("10.0.0.1", 32),
.peer_port = "1",
.peer_mac = "00:00:00:00:00:02",
.peer_ip = "10.0.0.2",
.peer_ip = "fe80::2",
.router_interface_id = "rif-1",
.nexthop_id = "nexthop-1",
};
Expand Down Expand Up @@ -540,56 +542,56 @@ ASSERT_OK(

// Add an L3 route to enable forwarding, but do not add an explicit L3Admit
// rule.
L3Route l3_route{
.vrf_id = "vrf-1",
.switch_mac = "00:00:00:00:00:01",
.switch_ip = std::make_pair("10.0.0.1", 32),
.peer_port = "1",
.peer_mac = "00:00:00:00:00:02",
.peer_ip = "10.0.0.2",
.router_interface_id = "rif-1",
.nexthop_id = "nexthop-1",
};
ASSERT_OK(AddAndSetDefaultVrf(GetSutP4RuntimeSession(),
sai::GetIrP4Info(sai::Instantiation::kMiddleblock),
l3_route.vrf_id));
ASSERT_OK(AddL3Route(GetSutP4RuntimeSession(),
sai::GetIrP4Info(sai::Instantiation::kMiddleblock),
l3_route));

// Send 1 set of packets to the switch using the switch's MAC address from the
// L3 route.
const int kNumberOfTestPacket = 100;
const std::string kGoodPayload =
"Testing L3 forwarding. This packet should arrive to packet in.";
ASSERT_OK(SendUdpPacket(GetControlP4RuntimeSession(),
sai::GetIrP4Info(sai::Instantiation::kMiddleblock),
/*port_id=*/"1", kNumberOfTestPacket,
/*dst_mac=*/"00:00:00:00:00:01",
/*dst_ip=*/"10.0.0.1", kGoodPayload));

absl::Time timeout = absl::Now() + absl::Minutes(1);
int good_packet_count = 0;
while (good_packet_count < kNumberOfTestPacket) {
if (packetio_control->HasPacketInMessage()) {
ASSERT_OK_AND_ASSIGN(p4::v1::StreamMessageResponse response,
packetio_control->GetNextPacketInMessage());

// Verify this is the packet we expect.
packetlib::Packet packet_in =
packetlib::ParsePacket(response.packet().payload());
if (response.update_case() == p4::v1::StreamMessageResponse::kPacket &&
absl::StrContains(packet_in.payload(), kGoodPayload)) {
++good_packet_count;
} else {
LOG(WARNING) << "Unexpected response: " << response.DebugString();
}
L3Route l3_route{
.vrf_id = "vrf-1",
.switch_mac = "00:00:00:00:00:01",
.switch_ip = std::make_pair("10.0.0.1", 32),
.peer_port = "1",
.peer_mac = "00:00:00:00:00:02",
.peer_ip = "fe80::2",
.router_interface_id = "rif-1",
.nexthop_id = "nexthop-1",
};
ASSERT_OK(AddAndSetDefaultVrf(
GetSutP4RuntimeSession(),
sai::GetIrP4Info(sai::Instantiation::kMiddleblock), l3_route.vrf_id));
ASSERT_OK(AddL3Route(GetSutP4RuntimeSession(),
sai::GetIrP4Info(sai::Instantiation::kMiddleblock),
l3_route));

// Send 1 set of packets to the switch using the switch's MAC address from the
// L3 route.
const int kNumberOfTestPacket = 100;
const std::string kGoodPayload =
"Testing L3 forwarding. This packet should arrive to packet in.";
ASSERT_OK(SendUdpPacket(GetControlP4RuntimeSession(),
sai::GetIrP4Info(sai::Instantiation::kMiddleblock),
/*port_id=*/"1", kNumberOfTestPacket,
/*dst_mac=*/"00:00:00:00:00:01",
/*dst_ip=*/"10.0.0.1", kGoodPayload));

absl::Time timeout = absl::Now() + absl::Minutes(1);
int good_packet_count = 0;
while (good_packet_count < kNumberOfTestPacket) {
if (packetio_control->HasPacketInMessage()) {
ASSERT_OK_AND_ASSIGN(p4::v1::StreamMessageResponse response,
packetio_control->GetNextPacketInMessage());

// Verify this is the packet we expect.
packetlib::Packet packet_in =
packetlib::ParsePacket(response.packet().payload());
if (response.update_case() == p4::v1::StreamMessageResponse::kPacket &&
absl::StrContains(packet_in.payload(), kGoodPayload)) {
++good_packet_count;
} else {
LOG(WARNING) << "Unexpected response: " << response.DebugString();
}
}

if (absl::Now() > timeout) {
LOG(ERROR) << "Reached timeout waiting on packets to arrive.";
break;
}
if (absl::Now() > timeout) {
LOG(ERROR) << "Reached timeout waiting on packets to arrive.";
break;
}
}
LOG(INFO) << "Done collecting packets.";

Expand Down Expand Up @@ -646,7 +648,7 @@ TEST_P(L3AdmitTestFixture, L3PacketsCanBeClassifiedByDestinationMac) {
.switch_ip = std::make_pair("10.0.0.1", 32),
.peer_port = "1",
.peer_mac = "00:00:00:00:00:02",
.peer_ip = "10.0.0.2",
.peer_ip = "fe80::2",
.router_interface_id = "rif-1",
.nexthop_id = "nexthop-1",
};
Expand Down
10 changes: 4 additions & 6 deletions tests/forwarding/smoke_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,10 @@ TEST_P(SmokeTestFixture, AclTableAddDeleteOkButModifyFails) {
}
} while (!pi_read_response.entities(0).table_entry().has_counter_data());

// Many ACL table attribues are NOT modifiable currently due to missing SAI
// implementation. Because there are no production use-cases, these are
// de-prioritized.
ASSERT_THAT(pdpi::SetMetadataAndSendPiWriteRequest(&GetSutP4RuntimeSession(),
pi_modify),
StatusIs(absl::StatusCode::kUnknown));
// To avoid any test failures during the submission process (test running with
// the pre-7.1 image), skip this check for now.
// ASSERT_OK(pdpi::SetMetadataAndSendPiWriteRequest(&GetSutP4RuntimeSession(),
// pi_modify));

// Delete works.
ASSERT_OK(pdpi::SetMetadataAndSendPiWriteRequest(&GetSutP4RuntimeSession(),
Expand Down
2 changes: 1 addition & 1 deletion tests/forwarding/test_data.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ std::vector<sai::TableEntry> CreateUpTo255GenericTableEntries(
ip_protocol { value: "0x11" mask: "0xff" }
l4_dst_port { value: "0x0043" mask: "0xffff" }
}
action { trap { qos_queue: "0x1" } }
action { trap { qos_queue: "0x7" } }
priority: 2040
}
)pb");
Expand Down
5 changes: 3 additions & 2 deletions tests/forwarding/watch_port_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,9 @@ absl::Status SendNPacketsToSut(int num_packets,
pins::GenerateIthPacket(test_config, i));
ASSIGN_OR_RETURN(std::string raw_packet, SerializePacket(packet));
ASSIGN_OR_RETURN(std::string port_string, pdpi::IntToDecimalString(port));
RETURN_IF_ERROR(
pins::InjectEgressPacket(port_string, raw_packet, ir_p4info, &p4_session));
RETURN_IF_ERROR(InjectEgressPacket(port_string, raw_packet, ir_p4info,
&p4_session,
/*packet_delay=*/std::nullopt));

dvaas::Packet p;
p.set_port(port_string);
Expand Down
3 changes: 2 additions & 1 deletion tests/gnoi/factory_reset_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ void IssueGnoiFactoryResetAndValidateStatus(
EXPECT_OK(status);
} else {
EXPECT_EQ(status.error_code(), expected_status.error_code());
EXPECT_EQ(status.error_message(), expected_status.error_message());
EXPECT_THAT(status.error_message(),
testing::HasSubstr(expected_status.error_message()));
}
}

Expand Down
32 changes: 16 additions & 16 deletions tests/lib/p4rt_fixed_table_programming_helper.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,19 +142,19 @@ absl::StatusOr<p4::v1::Update> NeighborTableUpdate(
type: $0
entity {
table_entry {
table_name: "router_interface_table"
table_name: "neighbor_table"
matches {
name: "router_interface_id"
exact { str: "$1" }
}
matches {
name: "neighbor_id"
exact { ipv6: "$2" }
}
action {
name: "set_port_and_src_mac"
params {
name: "port"
value { str: "$2" }
}
name: "set_dst_mac"
params {
name: "src_mac"
name: "dst_mac"
value { mac: "$3" }
}
}
Expand All @@ -177,20 +177,20 @@ absl::StatusOr<p4::v1::Update> NexthopTableUpdate(
type: $0
entity {
table_entry {
table_name: "neighbor_table"
table_name: "nexthop_table"
matches {
name: "router_interface_id"
name: "nexthop_id"
exact { str: "$1" }
}
matches {
name: "neighbor_id"
exact { ipv6: "$2" }
}
action {
name: "set_dst_mac"
name: "set_ip_nexthop"
params {
name: "dst_mac"
value { mac: "$3" }
name: "router_interface_id"
value { str: "$2" }
}
params {
name: "neighbor_id"
value { ipv6: "$3" }
}
}
}
Expand Down
19 changes: 16 additions & 3 deletions tests/lib/p4rt_fixed_table_programming_helper_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
#include "sai_p4/instantiations/google/instantiations.h"
#include "sai_p4/instantiations/google/sai_p4info.h"

namespace gpins {
namespace pins {
namespace {

using ::gutil::StatusIs;
Expand Down Expand Up @@ -112,26 +112,39 @@ TEST_P(L3RouteProgrammingTest, NeighborId) {
/*dst_mac=*/"00:01:02:03:04:05"));

EXPECT_THAT(pi_update, HasExactMatch("rid-1"));
EXPECT_THAT(pi_update, HasExactMatch("\001"));
EXPECT_THAT(pi_update, HasActionParam("\001\002\003\004\005"));
}

TEST_P(L3RouteProgrammingTest, NeighborIdFailsWithInvalidMacAddress) {
EXPECT_THAT(
pins::NeighborTableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT,
/*router_interface_id=*/"rid-1",
/*neighbor_id=*/"peer-1",
/*neighbor_id=*/"fe80::201:02ff:fe03:0405",
/*dst_mac=*/"invalid_format"),
StatusIs(absl::StatusCode::kInvalidArgument,
HasSubstr("Invalid MAC address")));
}

TEST_P(L3RouteProgrammingTest, NexthopId) {
ASSERT_OK_AND_ASSIGN(
p4::v1::Update pi_update,
NexthopTableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT,
/*nexthop_id=*/"nexthop-2",
/*router_interface_id=*/"rid-2",
/*neighbor_id=*/"::1"));
EXPECT_THAT(pi_update, HasExactMatch("nexthop-2"));
EXPECT_THAT(pi_update, HasActionParam("rid-2"));
EXPECT_THAT(pi_update, HasActionParam("\001"));
}

TEST_P(L3RouteProgrammingTest, VrfTableAddFailsWithEmptyId) {
EXPECT_THAT(
pins::VrfTableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT,
/*vrf_id=*/""),
StatusIs(absl::StatusCode::kInvalidArgument));
}

TEST_P(L3RouteProgrammingTest, Ipv4TableDoesNotRequireAnAction) {
// The helper class will assume a default (e.g. drop).
ASSERT_OK_AND_ASSIGN(
Expand Down Expand Up @@ -220,4 +233,4 @@ INSTANTIATE_TEST_SUITE_P(
});

} // namespace
} // namespace gpins
} // namespace pins
9 changes: 6 additions & 3 deletions tests/qos/cpu_qos_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,8 @@ TEST_P(CpuQosTestWithoutIxia, PerEntryAclCounterIncrementsWhenEntryIsHit) {
packetlib::SerializePacket(test_packet));
ASSERT_OK(pins::InjectEgressPacket(
/*port=*/link_used_for_test_packets.control_device_port_p4rt_name,
/*packet=*/raw_packet, ir_p4info, control_p4rt_session.get()));
/*packet=*/raw_packet, ir_p4info, control_p4rt_session.get(),
/*packet_delay=*/std::nullopt));

// Check that the counters increment within kMaxQueueCounterUpdateTime.
absl::Time time_packet_sent = absl::Now();
Expand Down Expand Up @@ -702,7 +703,8 @@ TEST_P(CpuQosTestWithoutIxia,
packetlib::SerializePacket(packet));
ASSERT_OK(pins::InjectEgressPacket(
/*port=*/link_used_for_test_packets.control_device_port_p4rt_name,
/*packet=*/raw_packet, ir_p4info, control_p4rt_session.get()));
/*packet=*/raw_packet, ir_p4info, control_p4rt_session.get(),
/*packet_delay=*/std::nullopt));

LOG(INFO) << "Sleeping for " << kMaxQueueCounterUpdateTime
<< " before checking for queue counter increment.";
Expand Down Expand Up @@ -791,7 +793,8 @@ TEST_P(CpuQosTestWithoutIxia, TrafficToLoopbackIpGetsMappedToCorrectQueues) {
for (int iter = 0; iter < kPacketCount; iter++) {
ASSERT_OK(pins::InjectEgressPacket(
/*port=*/link_used_for_test_packets.control_device_port_p4rt_name,
/*packet=*/raw_packet, ir_p4info, control_p4rt_session.get()));
/*packet=*/raw_packet, ir_p4info, control_p4rt_session.get(),
/*packet_delay=*/std::nullopt));
}
// Read counter of the target queue again.
absl::Time time_packet_sent = absl::Now();
Expand Down
11 changes: 10 additions & 1 deletion tests/qos/frontpanel_qos_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ using ::json_yang::FormatJsonBestEffort;
using ::testing::Contains;
using ::testing::Eq;
using ::testing::Field;
using ::testing::Ge;
using ::testing::IsEmpty;
using ::testing::Le;
using ::testing::Not;
using ::testing::Pair;
using ::testing::ResultOf;
Expand Down Expand Up @@ -472,7 +474,14 @@ TEST_P(FrontpanelQosTest,
"\nAfter :", ToString(final_counters)));
EXPECT_THAT(delta_counters,
ResultOf(TotalPacketsForQueue,
Eq(kIxiaTrafficStats.num_tx_frames())));
Ge(kIxiaTrafficStats.num_tx_frames())));
// Protocol packets such as LLDP can be transmitted via queue during test.
// Add some tolerance to account for these packets.
constexpr int kMaxToleratedExtraPackets = 5;
EXPECT_THAT(
delta_counters,
ResultOf(TotalPacketsForQueue, Le(kIxiaTrafficStats.num_tx_frames() +
kMaxToleratedExtraPackets)));
EXPECT_THAT(delta_counters, Field(&QueueCounters::num_packets_transmitted,
Eq(kIxiaTrafficStats.num_rx_frames())));
}
Expand Down

0 comments on commit dac1d13

Please sign in to comment.