Skip to content

Commit

Permalink
[Thinkit] Update hard-coded dscp-to-queue mappings to match our confi…
Browse files Browse the repository at this point in the history
…gs, Configure switches sequentially with ConfigureSwitchPairAndReturnP4RuntimeSessionPair, Stop masking fixed bug, add helper methods for generating IPv6 and WCMP flows & Vlan tag test, gNMI port breakout tests. (#948)

Co-authored-by: smolkaj <[email protected]>
  • Loading branch information
divyagayathri-hcl and smolkaj authored Jan 10, 2025
1 parent 2a5a114 commit eaf131b
Show file tree
Hide file tree
Showing 9 changed files with 367 additions and 189 deletions.
10 changes: 1 addition & 9 deletions tests/forwarding/l3_admit_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -526,15 +526,7 @@ TEST_P(L3AdmitTestFixture, DISABLED_L3AdmitCanUseInPortToRestrictMacAddresses) {
}
LOG(INFO) << "Done collecting packets.";

if (GetMirrorTestbed().Environment().MaskKnownFailures()) {
// TODO: Reduce expected count by tolerance level.
const int kDropTolerance = 1;
int adjusted_good_packets = kNumberOfTestPacket - kDropTolerance;
EXPECT_GE(good_packet_count, adjusted_good_packets);
EXPECT_LE(good_packet_count, kNumberOfTestPacket);
} else {
EXPECT_EQ(good_packet_count, kNumberOfTestPacket);
}
EXPECT_EQ(good_packet_count, kNumberOfTestPacket);
EXPECT_EQ(bad_packet_count, 0);
}

Expand Down
6 changes: 3 additions & 3 deletions tests/lib/p4rt_fixed_table_programming_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#ifndef GOOGLE_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_
#define GOOGLE_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_
#ifndef PINS_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_
#define PINS_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_
#include <optional>
#include <utility>
#include <vector>
Expand Down Expand Up @@ -110,4 +110,4 @@ WcmpGroupTableUpdate(const pdpi::IrP4Info &ir_p4_info,

} // namespace pins

#endif // GOOGLE_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_
#endif // PINS_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_
87 changes: 84 additions & 3 deletions tests/lib/p4rt_fixed_table_programming_helper_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
// limitations under the License.
#include "tests/lib/p4rt_fixed_table_programming_helper.h"

#include <utility>

#include "absl/status/status.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
Expand All @@ -27,6 +29,7 @@ namespace {

using ::gutil::StatusIs;
using ::testing::HasSubstr;
using ::testing::StrEq;

MATCHER_P(HasExactMatch, value, "") {
for (const auto& match_field : arg.entity().table_entry().match()) {
Expand Down Expand Up @@ -145,7 +148,7 @@ TEST_P(L3RouteProgrammingTest, VrfTableAddFailsWithEmptyId) {
StatusIs(absl::StatusCode::kInvalidArgument));
}

TEST_P(L3RouteProgrammingTest, Ipv4TableDoesNotRequireAnAction) {
TEST_P(L3RouteProgrammingTest, IpTableDoesNotRequireAnAction) {
// The helper class will assume a default (e.g. drop).
ASSERT_OK_AND_ASSIGN(
p4::v1::Update pi_update,
Expand All @@ -155,7 +158,7 @@ TEST_P(L3RouteProgrammingTest, Ipv4TableDoesNotRequireAnAction) {
EXPECT_THAT(pi_update, HasExactMatch("vrf-0"));
}

TEST_P(L3RouteProgrammingTest, Ipv4TableWithSetNexthopAction) {
TEST_P(L3RouteProgrammingTest, IpTableWithSetNexthopAction) {
ASSERT_OK_AND_ASSIGN(
p4::v1::Update pi_update,
pins::Ipv4TableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT,
Expand All @@ -171,7 +174,7 @@ TEST_P(L3RouteProgrammingTest, Ipv4TableWithSetNexthopAction) {
EXPECT_THAT(pi_update, HasActionParam("nexthop-0"));
}

TEST_P(L3RouteProgrammingTest, Ipv4TableEntryFailsWihInvalidParameters) {
TEST_P(L3RouteProgrammingTest, IpTableEntryFailsWihInvalidParameters) {
EXPECT_THAT(
pins::Ipv4TableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT,
pins::IpTableOptions{
Expand All @@ -183,6 +186,26 @@ TEST_P(L3RouteProgrammingTest, Ipv4TableEntryFailsWihInvalidParameters) {
HasSubstr("Expected 0 parameters")));
}

TEST_P(L3RouteProgrammingTest, Ipv4TableEntryCannotHaveIPv6Address) {
EXPECT_THAT(Ipv4TableUpdate(sai::GetIrP4Info(GetParam()),
p4::v1::Update::INSERT,
IpTableOptions{
.dst_addr_lpm = std::make_pair("FE80::1", 32),
}),
StatusIs(absl::StatusCode::kInvalidArgument,
HasSubstr("Invalid IPv4 address")));
}

TEST_P(L3RouteProgrammingTest, Ipv6TableEntryCannotHaveIPv4Address) {
EXPECT_THAT(
Ipv6TableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT,
IpTableOptions{
.dst_addr_lpm = std::make_pair("10.1.1.1", 32),
}),
StatusIs(absl::StatusCode::kInvalidArgument,
HasSubstr("invalid IPv6 address")));
}

TEST_P(L3RouteProgrammingTest, L3AdmitTableWithoutInPort) {
ASSERT_OK_AND_ASSIGN(
p4::v1::Update pi_update,
Expand All @@ -197,6 +220,13 @@ TEST_P(L3RouteProgrammingTest, L3AdmitTableWithoutInPort) {
"\377\377\377\377\377\377"));
}

TEST_P(L3RouteProgrammingTest, L3AdmitTableAllPacketsDoesNotSetAMatchKey) {
ASSERT_OK_AND_ASSIGN(p4::v1::Update pi_update,
L3AdmitAllTableUpdate(sai::GetIrP4Info(GetParam()),
p4::v1::Update::INSERT));
EXPECT_TRUE(pi_update.entity().table_entry().match().empty());
}

TEST_P(L3RouteProgrammingTest, L3AdmitTableWithInPort) {
ASSERT_OK_AND_ASSIGN(
p4::v1::Update pi_update,
Expand Down Expand Up @@ -224,6 +254,57 @@ TEST_P(L3RouteProgrammingTest, L3AdmitTableMustSetPriority) {
HasSubstr("require a positive non-zero priority")));
}

TEST_P(L3RouteProgrammingTest, WcmpGroupCanHaveMultipleNextHops) {
ASSERT_OK_AND_ASSIGN(
p4::v1::Update pi_update,
WcmpGroupTableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT,
/*wcmp_group_id=*/"group-1",
{WcmpAction{.nexthop_id = "nh-2", .weight = 1},
WcmpAction{.nexthop_id = "nh-3", .weight = 2}}));

EXPECT_THAT(pi_update, HasExactMatch("group-1"));
EXPECT_EQ(pi_update.entity()
.table_entry()
.action()
.action_profile_action_set()
.action_profile_actions_size(),
2);
}

TEST_P(L3RouteProgrammingTest, WcmpGroupActionCannotHaveWeightZero) {
EXPECT_THAT(
WcmpGroupTableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT,
/*wcmp_group_id=*/"group-1",
{WcmpAction{.nexthop_id = "nh-3", .weight = 0}}),
StatusIs(absl::StatusCode::kInvalidArgument,
HasSubstr("Expected positive action set weight")));
}

TEST_P(L3RouteProgrammingTest, WcmpGroupActionCanSetWatchPort) {
ASSERT_OK_AND_ASSIGN(p4::v1::Update pi_update,
WcmpGroupTableUpdate(sai::GetIrP4Info(GetParam()),
p4::v1::Update::INSERT,
/*wcmp_group_id=*/"group-1",
{WcmpAction{
.nexthop_id = "nh-3",
.weight = 2,
.watch_port = "1",
}}));
ASSERT_EQ(pi_update.entity()
.table_entry()
.action()
.action_profile_action_set()
.action_profile_actions_size(),
1);
EXPECT_THAT(pi_update.entity()
.table_entry()
.action()
.action_profile_action_set()
.action_profile_actions(0)
.watch_port(),
StrEq("1"));
}

INSTANTIATE_TEST_SUITE_P(
L3RouteProgrammingTestInstance, L3RouteProgrammingTest,
testing::Values(sai::Instantiation::kMiddleblock,
Expand Down
36 changes: 24 additions & 12 deletions tests/lib/switch_test_setup_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <deque>
#include <functional>
#include <future> // NOLINT: third_party library.
#include <memory>
#include <optional>
#include <string>
Expand Down Expand Up @@ -177,18 +178,29 @@ ConfigureSwitchPairAndReturnP4RuntimeSessionPair(
std::optional<std::string> gnmi_config,
std::optional<p4::config::v1::P4Info> p4info,
const pdpi::P4RuntimeSessionOptionalArgs& metadata) {
// TODO: Ideally, we would configure these in parallel instead.
ASSIGN_OR_RETURN(std::unique_ptr<pdpi::P4RuntimeSession> session1,
ConfigureSwitchAndReturnP4RuntimeSession(
thinkit_switch1, gnmi_config, p4info, metadata),
_.SetPrepend() << "failed to configure switch '"
<< thinkit_switch1.ChassisName() << "': ");
ASSIGN_OR_RETURN(std::unique_ptr<pdpi::P4RuntimeSession> session2,
ConfigureSwitchAndReturnP4RuntimeSession(
thinkit_switch2, gnmi_config, p4info, metadata),
_.SetPrepend() << "failed to configure switch '"
<< thinkit_switch2.ChassisName() << "': ");
return std::make_pair(std::move(session1), std::move(session2));
// We configure both switches in parallel, since it may require rebooting the
// switch which is costly.
using T = absl::StatusOr<std::unique_ptr<pdpi::P4RuntimeSession>>;
T session1, session2;
{
std::future<T> future1 = std::async(std::launch::async, [&] {
return ConfigureSwitchAndReturnP4RuntimeSession(
thinkit_switch1, gnmi_config, p4info, metadata);
});
std::future<T> future2 = std::async(std::launch::async, [&] {
return ConfigureSwitchAndReturnP4RuntimeSession(
thinkit_switch2, gnmi_config, p4info, metadata);
});
session1 = future1.get();
session2 = future2.get();
}
RETURN_IF_ERROR(session1.status()).SetPrepend()
<< "failed to configure switch '" << thinkit_switch1.ChassisName()
<< "': ";
RETURN_IF_ERROR(session2.status()).SetPrepend()
<< "failed to configure switch '" << thinkit_switch2.ChassisName()
<< "': ";
return std::make_pair(std::move(*session1), std::move(*session2));
}

absl::Status MirrorSutP4rtPortIdConfigToControlSwitch(
Expand Down
Loading

0 comments on commit eaf131b

Please sign in to comment.