From 0ee9e6511d06bef25176a9bf90575523dfc1a9ca Mon Sep 17 00:00:00 2001 From: kishanps Date: Thu, 9 Oct 2025 11:52:26 +0000 Subject: [PATCH 1/7] Enhance Resource Exhaustion Handling and Test Coverage for Action Profile MODIFY Operations in Switch State. --- p4_fuzzer/fuzz_util.cc | 10 +- p4_fuzzer/fuzzer_config.h | 15 +- p4_fuzzer/switch_state.cc | 73 ++++- p4_fuzzer/switch_state.h | 12 +- p4_fuzzer/switch_state_test.cc | 307 ++++++++++++++++--- sai_p4/instantiations/google/versions.h | 1 + tests/forwarding/BUILD.bazel | 1 + tests/forwarding/fuzzer_tests.cc | 3 +- tests/forwarding/fuzzer_tests.h | 10 +- tests/forwarding/watch_port_test.cc | 4 +- tests/integration/system/nsf/BUILD.bazel | 1 + tests/integration/system/nsf/upgrade_test.cc | 35 ++- tests/integration/system/nsf/upgrade_test.h | 3 +- tests/integration/system/nsf/util.cc | 1 + tests/qos/frontpanel_qos_test.cc | 2 +- tests/qos/pfc_test.cc | 2 +- 16 files changed, 401 insertions(+), 79 deletions(-) diff --git a/p4_fuzzer/fuzz_util.cc b/p4_fuzzer/fuzz_util.cc index f597a94d2..bafba8d2c 100644 --- a/p4_fuzzer/fuzz_util.cc +++ b/p4_fuzzer/fuzz_util.cc @@ -523,7 +523,15 @@ bool IsAccidentallyInvalidUpdate( candidate_update.pi().entity().table_entry().table_id(), num_inserts); } - return config.GetIsBuggyUpdateThatShouldBeSkipped()(candidate_update); + absl::StatusOr is_buggy_update = + config.GetIsBuggyUpdateThatShouldBeSkipped()(config, candidate_update); + if (!is_buggy_update.ok()) { + DLOG(FATAL) << "Failed to determine if update is buggy. Update: " + << gutil::PrintTextProto(candidate_update) + << "\nStatus: " << is_buggy_update.status(); + return true; + } + return *is_buggy_update; } // Returns all valid table ids. diff --git a/p4_fuzzer/fuzzer_config.h b/p4_fuzzer/fuzzer_config.h index e8157a229..385e81844 100644 --- a/p4_fuzzer/fuzzer_config.h +++ b/p4_fuzzer/fuzzer_config.h @@ -46,6 +46,9 @@ absl::StatusOr GetFullyQualifiedMatchFieldName( const pdpi::IrTableDefinition& table_def, const pdpi::IrMatchFieldDefinition& match_field_def); +// Forward declaration to allow for circular dependencies. +class FuzzerConfig; + struct ConfigParams { // -- Required --------------------------------------------------------------- // NOTE: These values are required for correct function. All of them are @@ -109,8 +112,12 @@ struct ConfigParams { // Ignores the constraints on tables listed when fuzzing entries. absl::flat_hash_set ignore_constraints_on_tables; // A function for masking any updates that should not be sent to the switch. - std::function IsBuggyUpdateThatShouldBeSkipped = - [](const AnnotatedUpdate& update) { return false; }; + std::function(const FuzzerConfig& config, + const AnnotatedUpdate&)> + IsBuggyUpdateThatShouldBeSkipped = + [](const FuzzerConfig& config, const AnnotatedUpdate& update) { + return false; + }; }; class FuzzerConfig { @@ -182,7 +189,9 @@ class FuzzerConfig { GetTreatAsEqualDuringReadDueToKnownBug() const { return params_.TreatAsEqualDuringReadDueToKnownBug; } - const std::function& + const std::function(const FuzzerConfig& config, + const AnnotatedUpdate&)>& + GetIsBuggyUpdateThatShouldBeSkipped() const { return params_.IsBuggyUpdateThatShouldBeSkipped; } diff --git a/p4_fuzzer/switch_state.cc b/p4_fuzzer/switch_state.cc index 79cf2caa5..41e7fda8f 100644 --- a/p4_fuzzer/switch_state.cc +++ b/p4_fuzzer/switch_state.cc @@ -44,7 +44,7 @@ #include "p4_pdpi/ir.pb.h" #include "p4_pdpi/translation_options.h" -// TODO: b/316625656 - Remove death behavior from this file (i.e. remove +// Remove death behavior from this file (i.e. remove // statements that crash like FindOrDie). namespace p4_fuzzer { @@ -103,11 +103,28 @@ ActionProfileResources GetAllActionProfileResourceForTableEntry( absl::StatusOr ReasonActionProfileCanAccommodateTableEntry( const p4::config::v1::ActionProfile& action_profile, const UnorderedTableEntries& current_table, - const p4::v1::TableEntry& pi_table_entry) { + const p4::v1::Update& pi_update) { + if (pi_update.type() == Update::DELETE) { + return "The update is a DELETE which can always be accommodated."; + } + + const p4::v1::TableEntry& pi_table_entry = pi_update.entity().table_entry(); ActionProfileResources current_resources = GetAllActionProfileResourceForTable(current_table); ActionProfileResources needed_resources = GetAllActionProfileResourceForTableEntry(pi_table_entry); + // MODIFYs are treated as a DELETE + INSERT which means resources are removed + // before added. INSERTs do not remove resources. + ActionProfileResources removed_resources; + if (pi_update.type() == Update::MODIFY) { + auto it = current_table.find(pdpi::TableEntryKey(pi_table_entry)); + if (it == current_table.end()) { + return gutil::InvalidArgumentErrorBuilder() + << " Cannot MODIFY an entry that does not exist. " + << gutil::PrintTextProto(pi_update); + } + removed_resources = GetAllActionProfileResourceForTableEntry(it->second); + } for (const p4::v1::ActionProfileAction& action : pi_table_entry.action() @@ -146,7 +163,14 @@ absl::StatusOr ReasonActionProfileCanAccommodateTableEntry( "with space for %d, and the new entry needs %d.", current_resources.actions, action_profile.size(), needed_resources.actions); - if (current_resources.actions + needed_resources.actions <= + if (pi_update.type() == Update::MODIFY) { + absl::StrAppend( + &result, + " The update is a MODIFY and will replace an entry that had ", + removed_resources.actions, " members."); + } + if (current_resources.actions + needed_resources.actions - + removed_resources.actions <= action_profile.size()) { return result; } @@ -168,7 +192,14 @@ absl::StatusOr ReasonActionProfileCanAccommodateTableEntry( "of %d with space for %d, and the new entry needs %d.", current_resources.total_weight, action_profile.size(), needed_resources.total_weight); - if (current_resources.total_weight + needed_resources.total_weight <= + if (pi_update.type() == Update::MODIFY) { + absl::StrAppend(&result, + " The update is a MODIFY and will replace an entry that " + "had weight ", + removed_resources.total_weight, "."); + } + if (current_resources.total_weight + needed_resources.total_weight - + removed_resources.total_weight <= action_profile.size()) { return result; } @@ -268,10 +299,25 @@ int64_t SwitchState::GetNumTableEntries() const { return current_entries_; } // * table is full. // * action profile resources are exhausted. (only applies to action sets) absl::Status SwitchState::ResourceExhaustedIsAllowed( - const p4::v1::TableEntry& pi_table_entry) const { + const p4::v1::Update& pi_update) const { + // DELETEs should never trigger resource exhaustion. + if (pi_update.type() == Update::DELETE) { + return gutil::FailedPreconditionErrorBuilder() + << "DELETEs should never trigger resource exhaustion." + << gutil::PrintTextProto(pi_update); + } + + // Now that we have multicast platform properties, check + // for invalid multicast resource exhaustion. + if (!pi_update.entity().has_table_entry()) { + return gutil::UnimplementedErrorBuilder() + << "ResourceExhaustedIsAllowed only supports table entries."; + } + std::vector results; results.reserve(2); + const TableEntry& pi_table_entry = pi_update.entity().table_entry(); uint32_t table_id = pi_table_entry.table_id(); ASSIGN_OR_RETURN(const pdpi::IrTableDefinition* table_def, FindPtrOrStatus(ir_p4info_.tables_by_id(), table_id)); @@ -280,12 +326,19 @@ absl::Status SwitchState::ResourceExhaustedIsAllowed( // If adding this entry would push the table size beyond what is defined then // ResourceExhausted is allowed. - if (table->size() + 1 > table_def->size()) { + if (pi_update.type() == Update::INSERT && + table->size() + 1 > table_def->size()) { return absl::OkStatus(); } - results.push_back( - absl::StrFormat("The table is holding %d entries, but has space for %d.", - table->size(), table_def->size())); + + std::string table_size_result = + absl::StrFormat("The table is holding %d entries and has space for %d.", + table->size(), table_def->size()); + if (pi_update.type() == Update::MODIFY) { + absl::StrAppend(&table_size_result, + " The update is a MODIFY and replaces an existing entry."); + } + results.push_back(table_size_result); // If the table uses an action profile then we also need to verify that the // profile itself has enough space. @@ -300,7 +353,7 @@ absl::Status SwitchState::ResourceExhaustedIsAllowed( // Check if we have exhausted our action profile resources. absl::StatusOr action_profile_has_space = ReasonActionProfileCanAccommodateTableEntry(action_profile, *table, - pi_table_entry); + pi_update); if (!action_profile_has_space.ok()) { // If we've exhausted our action profile resources then ResourceExhausted diff --git a/p4_fuzzer/switch_state.h b/p4_fuzzer/switch_state.h index 9a658d252..31a437cec 100644 --- a/p4_fuzzer/switch_state.h +++ b/p4_fuzzer/switch_state.h @@ -99,15 +99,15 @@ class SwitchState { // or not a table is full, may depend on the entries in other tables as well. bool IsTableFull(const uint32_t table_id) const; - // Returns OK if inserting the `pi_table_entry` into its table could cause a - // resource exhaustion, and a FailedPrecondition error if the entry should fit - // within the table's resources. + // Returns OK if applying the `pi_update` into its table could cause a + // resource exhaustion, and a FailedPrecondition error if the update should + // fit within the table's resources. // // Keep in mind sizes for P4 tables are minimum guarantees. So while this // method may suggest seeing a ResourceExhausted is expected (i.e. return OK), - // an actual switch may still accept the insert. - absl::Status - ResourceExhaustedIsAllowed(const p4::v1::TableEntry &pi_table_entry) const; + // an actual switch may still accept the update. + absl::Status ResourceExhaustedIsAllowed( + const p4::v1::Update& pi_update) const; // Checks whether all P4 tables are empty. bool AllP4TablesEmpty() const; diff --git a/p4_fuzzer/switch_state_test.cc b/p4_fuzzer/switch_state_test.cc index 3ce60f9f2..57ecb6bcb 100644 --- a/p4_fuzzer/switch_state_test.cc +++ b/p4_fuzzer/switch_state_test.cc @@ -20,6 +20,7 @@ #include "absl/container/flat_hash_set.h" #include "absl/status/status.h" #include "absl/status/statusor.h" +#include "absl/strings/string_view.h" #include "absl/strings/substitute.h" #include "absl/types/span.h" #include "gmock/gmock.h" @@ -46,11 +47,13 @@ using ::gutil::StatusIs; using ::p4::config::v1::Action; using ::p4::config::v1::ActionProfile; using ::p4::config::v1::ActionRef; +using ::p4::config::v1::MatchField; using ::p4::config::v1::P4Info; using ::p4::config::v1::Preamble; using ::p4::config::v1::Table; using ::p4::v1::ActionProfileAction; using ::p4::v1::Entity; +using ::p4::v1::FieldMatch; using ::p4::v1::MulticastGroupEntry; using ::p4::v1::TableEntry; using ::p4::v1::Update; @@ -649,7 +652,7 @@ TEST(SwitchStateTest, SetEntitiesSetsEntities) { // * 1 action profile // * 1 table which uses the action profile. // * 1 table which does not use the action profile. -class ResourceLimitsTest : public testing::Test { +class ResourceExhaustionAllowedTest : public testing::Test { protected: // Tests should specify the P4Info values that are relevant. For example, when // verifying behavior around the table size the test should set '.table_size = @@ -698,6 +701,12 @@ class ResourceLimitsTest : public testing::Test { table->mutable_preamble()->set_alias("action_set_table"); table->mutable_preamble()->add_annotations("@oneshot"); table->set_size(options.table_size); + // The table needs a simple match field to create unique entries. + MatchField* match_field = table->add_match_fields(); + match_field->set_id(kActionProfileTableMatchFieldId); + match_field->set_match_type(MatchField::EXACT); + // Set excessively large to hold arbitrary key strings used in testing. + match_field->set_bitwidth(1024); // The table needs to link to the action. ActionRef* action_ref = table->add_action_refs(); action_ref->set_id(action->preamble().id()); @@ -719,12 +728,17 @@ class ResourceLimitsTest : public testing::Test { return CreateIrP4Info(info); } - Update GetUpdateWithWeights(absl::Span weights) { + Update GetUpdateWithWeights(absl::string_view key, + absl::Span weights, + p4::v1::Update::Type type) { Update update; - update.set_type(Update::INSERT); + update.set_type(type); TableEntry* entry = update.mutable_entity()->mutable_table_entry(); entry->set_table_id(TableWithActionProfileId()); + FieldMatch* match_field = entry->add_match(); + match_field->set_field_id(kActionProfileTableMatchFieldId); + match_field->mutable_exact()->set_value(key); for (int32_t weight : weights) { ActionProfileAction* profile = entry->mutable_action() ->mutable_action_profile_action_set() @@ -735,112 +749,327 @@ class ResourceLimitsTest : public testing::Test { return update; } + + private: + const uint32_t kActionProfileTableMatchFieldId = 1; }; -TEST_F(ResourceLimitsTest, ReturnsFailedPreconditionWhenEntryWillFit) { +TEST_F(ResourceExhaustionAllowedTest, + ReturnsFailedPreconditionForDeleteUpdate) { ASSERT_OK_AND_ASSIGN(IrP4Info ir_p4info, GetIrP4Info(P4InfoOptions{ - .table_size = 10, + .table_size = 1, .action_profile_size = 10, .action_profile_max_group_size = 10, .set_selector_size_semantics = true, })); SwitchState state(ir_p4info); - // Insert an entry to use up some space, then check for new space. - ASSERT_OK(state.ApplyUpdate(GetUpdateWithWeights({1, 1, 1}))); - EXPECT_THAT( - state.ResourceExhaustedIsAllowed( - GetUpdateWithWeights({2}).entity().table_entry()), - StatusIs(absl::StatusCode::kFailedPrecondition, - AllOf(HasSubstr("1 entries"), HasSubstr("weight of 3")))); + p4::v1::Update update; + update.set_type(Update::DELETE); + EXPECT_THAT(state.ResourceExhaustedIsAllowed(update), + StatusIs(absl::StatusCode::kFailedPrecondition)); } -TEST_F(ResourceLimitsTest, - ReturnsFailedPreconditionWhenOnlyTableSizeIsChecked) { +TEST_F(ResourceExhaustionAllowedTest, + ReturnsUnimplementedErrorForMulticastUpdate) { ASSERT_OK_AND_ASSIGN(IrP4Info ir_p4info, GetIrP4Info(P4InfoOptions{ - .table_size = 10, + .table_size = 1, .action_profile_size = 10, .action_profile_max_group_size = 10, .set_selector_size_semantics = true, })); SwitchState state(ir_p4info); + p4::v1::Update update; + update.set_type(Update::INSERT); + update.mutable_entity() + ->mutable_packet_replication_engine_entry() + ->mutable_multicast_group_entry(); + EXPECT_THAT(state.ResourceExhaustedIsAllowed(update), + StatusIs(absl::StatusCode::kUnimplemented)); +} + +TEST_F(ResourceExhaustionAllowedTest, + ReturnsInvalidArgumentWhenModifyingNonExistentActionSelectorEntry) { + ASSERT_OK_AND_ASSIGN(IrP4Info ir_p4info, + GetIrP4Info(P4InfoOptions{ + .table_size = 1, + .action_profile_size = 10, + .action_profile_max_group_size = 10, + .set_selector_size_semantics = true, + })); + SwitchState state(ir_p4info); + + EXPECT_THAT(state.ResourceExhaustedIsAllowed(GetUpdateWithWeights( + /*key=*/"non_existent_entry", {2}, Update::MODIFY)), + StatusIs(absl::StatusCode::kInvalidArgument)); +} + +TEST_F(ResourceExhaustionAllowedTest, + ReturnsOkOnUpdateThatWontFitTableSizeOtherwiseError) { + ASSERT_OK_AND_ASSIGN(IrP4Info ir_p4info, GetIrP4Info(P4InfoOptions{ + .table_size = 1, + })); + SwitchState state(ir_p4info); + Update update; update.set_type(Update::INSERT); TableEntry* entry = update.mutable_entity()->mutable_table_entry(); entry->set_table_id(TableWithoutActionProfileId()); + entry->mutable_action()->mutable_action()->set_action_id(ActionId()); - // Insert an entry to use up some space, then check for new space. - EXPECT_THAT(state.ResourceExhaustedIsAllowed(update.entity().table_entry()), + // Initial insert does not cause resource exhaustion. + EXPECT_THAT(state.ResourceExhaustedIsAllowed(update), + StatusIs(absl::StatusCode::kFailedPrecondition)); + ASSERT_OK(state.ApplyUpdate(update)); + + // Resource exhaustion allowed on second insert. + EXPECT_OK(state.ResourceExhaustedIsAllowed(update)); + + update.set_type(Update::MODIFY); + // Resource exhaustion not allowed on modify. + EXPECT_THAT(state.ResourceExhaustedIsAllowed(update), StatusIs(absl::StatusCode::kFailedPrecondition)); } -TEST_F(ResourceLimitsTest, ReturnsOkForTooManyTableResourcesUsed) { +TEST_F(ResourceExhaustionAllowedTest, + ReturnsErrorOnMaxGroupSizeWeightViolation) { ASSERT_OK_AND_ASSIGN(IrP4Info ir_p4info, GetIrP4Info(P4InfoOptions{ - .table_size = 1, + .table_size = 10, .action_profile_size = 10, .action_profile_max_group_size = 10, .set_selector_size_semantics = true, })); SwitchState state(ir_p4info); - // Insert 1 table entry to use up the space. - ASSERT_OK(state.ApplyUpdate(GetUpdateWithWeights({1}))); - EXPECT_OK(state.ResourceExhaustedIsAllowed( - GetUpdateWithWeights({1}).entity().table_entry())); + // Error due to violating max_group_size on INSERT. + EXPECT_THAT(state.ResourceExhaustedIsAllowed(GetUpdateWithWeights( + /*key=*/"entry", {25}, Update::INSERT)), + StatusIs(absl::StatusCode::kInvalidArgument)); + EXPECT_THAT(state.ResourceExhaustedIsAllowed(GetUpdateWithWeights( + /*key=*/"entry", {5, 10, 10}, Update::INSERT)), + StatusIs(absl::StatusCode::kInvalidArgument)); + + // Insert entry for modifies. + ASSERT_OK(state.ApplyUpdate( + GetUpdateWithWeights(/*key=*/"entry", {5}, Update::INSERT))); + + // Error due to violating max_group_size on MODIFY. + EXPECT_THAT(state.ResourceExhaustedIsAllowed(GetUpdateWithWeights( + /*key=*/"entry", {25}, Update::MODIFY)), + StatusIs(absl::StatusCode::kInvalidArgument)); + EXPECT_THAT(state.ResourceExhaustedIsAllowed(GetUpdateWithWeights( + /*key=*/"entry", {5, 10, 10}, Update::MODIFY)), + StatusIs(absl::StatusCode::kInvalidArgument)); } -TEST_F(ResourceLimitsTest, ReturnsOkForTooMuchWeightBeingUsed) { +TEST_F( + ResourceExhaustionAllowedTest, + ReturnsOkOnUpdateThatWontFitUsingSumOfWeightsOtherwiseReturnsErrorWithReason) { ASSERT_OK_AND_ASSIGN(IrP4Info ir_p4info, GetIrP4Info(P4InfoOptions{ .table_size = 10, - .action_profile_size = 3, + .action_profile_size = 10, .action_profile_max_group_size = 10, .set_selector_size_semantics = true, })); SwitchState state(ir_p4info); - // We should expect a resource exhausted for one member using too much, or the - // sum total of all members being too much. - EXPECT_OK(state.ResourceExhaustedIsAllowed( - GetUpdateWithWeights({4}).entity().table_entry())); + // Insert an entry to use up some space. + ASSERT_OK(state.ApplyUpdate( + GetUpdateWithWeights(/*key=*/"entry", {5}, Update::INSERT))); + + // Inserts that DO NOT exhaust weight resources. + EXPECT_THAT( + state.ResourceExhaustedIsAllowed( + GetUpdateWithWeights(/*key=*/"other_entry", {5}, Update::INSERT)), + StatusIs(absl::StatusCode::kFailedPrecondition, + AllOf(HasSubstr("1 entries"), HasSubstr("total weight of 5"), + HasSubstr("needs 5")))); + EXPECT_THAT( + state.ResourceExhaustedIsAllowed(GetUpdateWithWeights( + /*key=*/"other_entry", {1, 2, 2}, Update::INSERT)), + StatusIs(absl::StatusCode::kFailedPrecondition, + AllOf(HasSubstr("1 entries"), HasSubstr("total weight of 5"), + HasSubstr("needs 5")))); + + // Inserts that DO exhaust weight resources. EXPECT_OK(state.ResourceExhaustedIsAllowed( - GetUpdateWithWeights({1, 1, 1, 1}).entity().table_entry())); + GetUpdateWithWeights(/*key=*/"other_entry", {10}, Update::INSERT))); + EXPECT_OK(state.ResourceExhaustedIsAllowed(GetUpdateWithWeights( + /*key=*/"other_entry", {2, 4, 4}, Update::INSERT))); + + // Modifies that DO NOT exhaust weight resources. Note that these entries are + // the same as the ones that caused resource exhaustion on insert. + EXPECT_THAT( + state.ResourceExhaustedIsAllowed( + GetUpdateWithWeights(/*key=*/"entry", {10}, Update::MODIFY)), + StatusIs(absl::StatusCode::kFailedPrecondition, + AllOf(HasSubstr("1 entries"), HasSubstr("total weight of 5"), + HasSubstr("needs 10"), HasSubstr("had weight 5")))); + EXPECT_THAT( + state.ResourceExhaustedIsAllowed( + GetUpdateWithWeights(/*key=*/"entry", {2, 4, 4}, Update::MODIFY)), + StatusIs(absl::StatusCode::kFailedPrecondition, + AllOf(HasSubstr("1 entries"), HasSubstr("total weight of 5"), + HasSubstr("needs 10"), HasSubstr("had weight 5")))); + + // For modifies that exhaust resource exhaustion, see + // `ReturnsOkOnModifyUpdateThatWontFitUsingSumOfWeightsOtherwiseReturnsErrorWithReason`. } -TEST_F(ResourceLimitsTest, ReturnsOkForTooManyActionsBeingUsed) { +TEST_F( + ResourceExhaustionAllowedTest, + ReturnsOkOnModifyUpdateThatWontFitUsingSumOfWeightsOtherwiseReturnsErrorWithReason) { ASSERT_OK_AND_ASSIGN(IrP4Info ir_p4info, GetIrP4Info(P4InfoOptions{ .table_size = 10, - .action_profile_size = 3, + .action_profile_size = 10, .action_profile_max_group_size = 10, .set_selector_size_semantics = true, - .max_member_weight = 4096, })); SwitchState state(ir_p4info); + // Insert an entry to use up some space. + ASSERT_OK(state.ApplyUpdate( + GetUpdateWithWeights(/*key=*/"other_entry", {5}, Update::INSERT))); + // Insert another entry that will be modified. + ASSERT_OK(state.ApplyUpdate( + GetUpdateWithWeights(/*key=*/"mod_entry", {2}, Update::INSERT))); + + // Modifies that DO NOT exhaust weight resources. + EXPECT_THAT( + state.ResourceExhaustedIsAllowed( + GetUpdateWithWeights(/*key=*/"mod_entry", {5}, Update::MODIFY)), + StatusIs(absl::StatusCode::kFailedPrecondition, + AllOf(HasSubstr("2 entries"), HasSubstr("total weight of 7"), + HasSubstr("needs 5"), HasSubstr("had weight 2")))); + EXPECT_THAT( + state.ResourceExhaustedIsAllowed( + GetUpdateWithWeights(/*key=*/"mod_entry", {1, 2, 2}, Update::MODIFY)), + StatusIs(absl::StatusCode::kFailedPrecondition, + AllOf(HasSubstr("2 entries"), HasSubstr("total weight of 7"), + HasSubstr("needs 5"), HasSubstr("had weight 2")))); + + // Modifies that DO exhaust weight resources. + EXPECT_OK(state.ResourceExhaustedIsAllowed( + GetUpdateWithWeights(/*key=*/"mod_entry", {10}, Update::MODIFY))); EXPECT_OK(state.ResourceExhaustedIsAllowed( - GetUpdateWithWeights({1, 10, 4, 2}).entity().table_entry())); + GetUpdateWithWeights(/*key=*/"mod_entry", {2, 4, 4}, Update::MODIFY))); } -// If a group size is too large, the switch must return an InvalidArgumentError, -// so we do the same. -TEST_F(ResourceLimitsTest, ReturnsInvalidArgumentForGroupSizesBeingTooLarge) { +TEST_F(ResourceExhaustionAllowedTest, + ReturnsErrorOnMaxGroupSizeMemberViolation) { ASSERT_OK_AND_ASSIGN(IrP4Info ir_p4info, GetIrP4Info(P4InfoOptions{ .table_size = 10, .action_profile_size = 10, .action_profile_max_group_size = 3, .set_selector_size_semantics = true, + .max_member_weight = 128, })); SwitchState state(ir_p4info); - EXPECT_THAT(state.ResourceExhaustedIsAllowed( - GetUpdateWithWeights({1, 1, 1, 1}).entity().table_entry()), + // Error due to violating max_group_size on INSERT. + EXPECT_THAT(state.ResourceExhaustedIsAllowed(GetUpdateWithWeights( + /*key=*/"entry", {5, 5, 5, 5}, Update::INSERT)), + StatusIs(absl::StatusCode::kInvalidArgument)); + + // Error due to violating max_member_weight on INSERT. + EXPECT_THAT(state.ResourceExhaustedIsAllowed(GetUpdateWithWeights( + /*key=*/"entry", {200}, Update::INSERT)), + StatusIs(absl::StatusCode::kInvalidArgument)); + + // Insert entry for modifies. + ASSERT_OK(state.ApplyUpdate( + GetUpdateWithWeights(/*key=*/"entry", {5}, Update::INSERT))); + + // Error due to violating max_group_size on MODIFY. + EXPECT_THAT(state.ResourceExhaustedIsAllowed(GetUpdateWithWeights( + /*key=*/"entry", {5, 5, 5, 5}, Update::MODIFY)), StatusIs(absl::StatusCode::kInvalidArgument)); + + // Error due to violating max_member_weight on MODIFY. + EXPECT_THAT(state.ResourceExhaustedIsAllowed(GetUpdateWithWeights( + /*key=*/"entry", {200}, Update::MODIFY)), + StatusIs(absl::StatusCode::kInvalidArgument)); +} + +TEST_F( + ResourceExhaustionAllowedTest, + ReturnsOkOnUpdateThatWontFitUsingSumOfMembersOtherwiseReturnsErrorWithReason) { + ASSERT_OK_AND_ASSIGN(IrP4Info ir_p4info, + GetIrP4Info(P4InfoOptions{ + .table_size = 10, + .action_profile_size = 5, + .action_profile_max_group_size = 5, + .set_selector_size_semantics = true, + .max_member_weight = 128, + })); + SwitchState state(ir_p4info); + + // Insert an entry to use up some space. + ASSERT_OK(state.ApplyUpdate( + GetUpdateWithWeights(/*key=*/"entry", {10, 20, 30}, Update::INSERT))); + + // Insert that DOES NOT exhaust weight resources. + EXPECT_THAT(state.ResourceExhaustedIsAllowed(GetUpdateWithWeights( + /*key=*/"other_entry", {40, 50}, Update::INSERT)), + StatusIs(absl::StatusCode::kFailedPrecondition, + AllOf(HasSubstr("1 entries"), HasSubstr("3 members"), + HasSubstr("needs 2")))); + + // Insert that DOES exhaust member resources. + EXPECT_OK(state.ResourceExhaustedIsAllowed(GetUpdateWithWeights( + /*key=*/"other_entry", {40, 50, 60}, Update::INSERT))); + + // Modify that DOES NOT exhaust member resources. Note that this entry is + // the same as the one that caused resource exhaustion on insert. + EXPECT_THAT( + state.ResourceExhaustedIsAllowed( + GetUpdateWithWeights(/*key=*/"entry", {40, 50, 60}, Update::MODIFY)), + StatusIs(absl::StatusCode::kFailedPrecondition, + AllOf(HasSubstr("1 entries"), HasSubstr("3 members"), + HasSubstr("needs 3"), HasSubstr("had 3 members")))); + + // For modifies that exhaust resource exhaustion, see + // `ReturnsOkOnModifyUpdateThatWontFitUsingSumOfMembersOtherwiseReturnsErrorWithReason`. +} + +TEST_F( + ResourceExhaustionAllowedTest, + ReturnsOkOnModifyUpdateThatWontFitUsingSumOfMembersOtherwiseReturnsErrorWithReason) { + ASSERT_OK_AND_ASSIGN(IrP4Info ir_p4info, + GetIrP4Info(P4InfoOptions{ + .table_size = 10, + .action_profile_size = 5, + .action_profile_max_group_size = 5, + .set_selector_size_semantics = true, + .max_member_weight = 128, + })); + SwitchState state(ir_p4info); + + // Insert an entry to use up some space. + ASSERT_OK(state.ApplyUpdate(GetUpdateWithWeights( + /*key=*/"other_entry", {10, 20, 30}, Update::INSERT))); + // Insert another entry that will be modified. + ASSERT_OK(state.ApplyUpdate( + GetUpdateWithWeights(/*key=*/"mod_entry", {40, 50}, Update::INSERT))); + + // Modifies that DO NOT exhaust member resources. + EXPECT_THAT( + state.ResourceExhaustedIsAllowed(GetUpdateWithWeights( + /*key=*/"mod_entry", {60, 70}, Update::MODIFY)), + StatusIs(absl::StatusCode::kFailedPrecondition, + AllOf(HasSubstr("2 entries"), HasSubstr("5 members"), + HasSubstr("needs 2"), HasSubstr("had 2 members")))); + + // Modifies that DO exhaust member resources. + EXPECT_OK(state.ResourceExhaustedIsAllowed( + GetUpdateWithWeights(/*key=*/"mod_entry", {60, 70, 80}, Update::MODIFY))); } } // namespace diff --git a/sai_p4/instantiations/google/versions.h b/sai_p4/instantiations/google/versions.h index b4ea3a006..3bc4892d8 100644 --- a/sai_p4/instantiations/google/versions.h +++ b/sai_p4/instantiations/google/versions.h @@ -66,6 +66,7 @@ // Indicates that the program supports ternary rather than optional route // metadata in the acl_ingress_table. +// Also, indicates the program has the MRIF vlan_id p4 constraint. #define SAI_P4_PKGINFO_VERSION_USES_TERNARY_ROUTE_METADATA "3.0.0" // Indicates that the program supports all valid modifications to the diff --git a/tests/forwarding/BUILD.bazel b/tests/forwarding/BUILD.bazel index 658569221..8417e2b33 100644 --- a/tests/forwarding/BUILD.bazel +++ b/tests/forwarding/BUILD.bazel @@ -119,6 +119,7 @@ cc_library( cc_library( name = "util", + testonly = True, srcs = ["util.cc"], hdrs = ["util.h"], deps = [ diff --git a/tests/forwarding/fuzzer_tests.cc b/tests/forwarding/fuzzer_tests.cc index 78ca20a02..9ad35dde4 100644 --- a/tests/forwarding/fuzzer_tests.cc +++ b/tests/forwarding/fuzzer_tests.cc @@ -361,8 +361,7 @@ TEST_P(FuzzerTestFixture, P4rtWriteAndCheckNoInternalErrors) { // exhaustion is allowed. if (!IsMaskedResource(table.preamble().alias(), current_version)) { // Check that table is allowed to have exhausted resources. - ASSERT_OK( - state.ResourceExhaustedIsAllowed(update.entity().table_entry())) + ASSERT_OK(state.ResourceExhaustedIsAllowed(update)) << "\nUpdate = " << update.DebugString() << "\nState = " << state.SwitchStateSummary(); } diff --git a/tests/forwarding/fuzzer_tests.h b/tests/forwarding/fuzzer_tests.h index d5f995f85..5fb7b6bc5 100644 --- a/tests/forwarding/fuzzer_tests.h +++ b/tests/forwarding/fuzzer_tests.h @@ -23,10 +23,12 @@ #include "absl/container/btree_set.h" #include "absl/container/flat_hash_set.h" #include "absl/status/status.h" +#include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "gtest/gtest.h" #include "p4/config/v1/p4info.pb.h" #include "p4_fuzzer/fuzzer.pb.h" +#include "p4_fuzzer/fuzzer_config.h" #include "p4_fuzzer/switch_state.h" #include "p4_pdpi/ir.pb.h" #include "thinkit/mirror_testbed_fixture.h" @@ -104,8 +106,12 @@ struct FuzzerTestFixtureParams { // parameter. bool do_not_enforce_fail_on_first_switch_ordering; // A function for masking any updates that should not be sent to the switch. - std::function IsBuggyUpdateThatShouldBeSkipped = - [](const AnnotatedUpdate& update) { return false; }; + std::function(const FuzzerConfig&, + const AnnotatedUpdate&)> + IsBuggyUpdateThatShouldBeSkipped = + [](const FuzzerConfig&, const AnnotatedUpdate& update) { + return false; + }; // A function for modifying any `TableEntry` produced by the Fuzzer. Note that // mutations can override modified values. diff --git a/tests/forwarding/watch_port_test.cc b/tests/forwarding/watch_port_test.cc index a5e03a527..3707c900f 100644 --- a/tests/forwarding/watch_port_test.cc +++ b/tests/forwarding/watch_port_test.cc @@ -355,9 +355,9 @@ void PrettyPrintDistribution( } // Creates the port_names_per_port_id map from GNMI config. -absl::StatusOr> +absl::StatusOr> GetPortNamePerPortId(gnmi::gNMI::StubInterface& gnmi_stub) { - absl::flat_hash_map port_name_per_port_id; + absl::flat_hash_map port_name_per_port_id; ASSIGN_OR_RETURN(auto port_id_per_port_name, pins_test::GetAllInterfaceNameToPortId(gnmi_stub)); for (const auto& [name, port_id] : port_id_per_port_name) { diff --git a/tests/integration/system/nsf/BUILD.bazel b/tests/integration/system/nsf/BUILD.bazel index 8de1c78bc..996322e53 100644 --- a/tests/integration/system/nsf/BUILD.bazel +++ b/tests/integration/system/nsf/BUILD.bazel @@ -113,6 +113,7 @@ cc_library( "@com_github_google_glog//:glog", "@com_github_p4lang_p4runtime//:p4info_cc_proto", "@com_github_p4lang_p4runtime//:p4runtime_cc_proto", + "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/flags:flag", "@com_google_absl//absl/random", "@com_google_absl//absl/status", diff --git a/tests/integration/system/nsf/upgrade_test.cc b/tests/integration/system/nsf/upgrade_test.cc index 6ea62dc1b..331d74d74 100644 --- a/tests/integration/system/nsf/upgrade_test.cc +++ b/tests/integration/system/nsf/upgrade_test.cc @@ -15,10 +15,12 @@ #include "tests/integration/system/nsf/upgrade_test.h" #include +#include #include #include #include +#include "absl/container/flat_hash_map.h" #include "absl/flags/flag.h" #include "absl/random/random.h" #include "absl/status/status.h" @@ -66,6 +68,14 @@ NsfUpgradeScenario GetRandomNsfUpgradeScenario() { return static_cast(random_index); } + +// Checks for any changes in the expected interfaces and their operational +// states. +// @interface_to_oper_status_map is used as a baseline to compare it with the +// data returned by GetInterfaceToOperStatusMapOverGnmi during this check. +// @trigger is to state the actions/events performed prior to calling +// this helper. As we add this to the overall status we can clearly identify the +// cause of this checks failure. } // namespace void NsfUpgradeTest::SetUp() { @@ -83,18 +93,20 @@ void NsfUpgradeTest::SetUp() { void NsfUpgradeTest::TearDown() { TearDownTestbed(testbed_interface_); } absl::Status NsfUpgradeTest::PushConfigAndValidate( - const ImageConfigParams& image_config_param, + const ImageConfigParams& curr_image_config_params, + const ImageConfigParams& next_image_config_params, bool enable_interface_validation_during_nsf) { - RETURN_IF_ERROR(PushConfig(image_config_param, GetSut(testbed_), *ssh_client_, + RETURN_IF_ERROR(PushConfig(next_image_config_params, GetSut(testbed_), + *ssh_client_, /*clear_config=*/false)); std::vector interfaces_to_check; RETURN_IF_ERROR(ValidateTestbedState( - testbed_, *ssh_client_, &image_config_param, + testbed_, *ssh_client_, &next_image_config_params, enable_interface_validation_during_nsf, interfaces_to_check)); return ValidateComponents( &ComponentValidator::OnConfigPush, component_validators_, - image_config_param.image_label, testbed_, *ssh_client_); + next_image_config_params.image_label, testbed_, *ssh_client_); } absl::Status NsfUpgradeTest::NsfUpgradeOrReboot( @@ -109,8 +121,12 @@ absl::Status NsfUpgradeTest::NsfUpgradeOrReboot( curr_image_config.image_label, next_image_config.image_version, next_image_config.image_label); LOG(INFO) << "Initiating " << upgrade_path; - + thinkit::Switch& sut = GetSut(testbed_); std::vector interfaces_to_check; + + ASSIGN_OR_RETURN(auto sut_gnmi_stub, sut.CreateGnmiStub()); + LOG(INFO) << "Capture interfaces and their port status prior to NSF reboot."; + RETURN_IF_ERROR(ValidateTestbedState( testbed_, *ssh_client_, &curr_image_config, enable_interface_validation_during_nsf, interfaces_to_check)); @@ -156,7 +172,6 @@ absl::Status NsfUpgradeTest::NsfUpgradeOrReboot( LOG(INFO) << "Programming flows before starting the traffic"; RETURN_IF_ERROR(flow_programmer_->ProgramFlows(curr_image_config, testbed_, *ssh_client_)); - thinkit::Switch& sut = GetSut(testbed_); RETURN_IF_ERROR(ValidateTestbedState( testbed_, *ssh_client_, &curr_image_config, enable_interface_validation_during_nsf, interfaces_to_check)); @@ -218,8 +233,6 @@ absl::Status NsfUpgradeTest::NsfUpgradeOrReboot( } } - ASSIGN_OR_RETURN(auto sut_gnmi_stub, sut.CreateGnmiStub()); - ASSIGN_OR_RETURN( PinsSoftwareComponentInfo pins_component_info_before_upgrade_reboot, GetPinsSoftwareComponentInfo(*sut_gnmi_stub), @@ -336,7 +349,7 @@ absl::Status NsfUpgradeTest::NsfUpgradeOrReboot( case NsfUpgradeScenario::kOnlyConfigPush: LOG(INFO) << upgrade_path << ": Proceeding with only config push"; - status = PushConfigAndValidate(next_image_config, + status = PushConfigAndValidate(curr_image_config, next_image_config, enable_interface_validation_during_nsf); if (!status.ok()) { AppendErrorStatus(overall_status, @@ -360,7 +373,7 @@ absl::Status NsfUpgradeTest::NsfUpgradeOrReboot( status.message()))); } - status = PushConfigAndValidate(next_image_config, + status = PushConfigAndValidate(curr_image_config, next_image_config, enable_interface_validation_during_nsf); if (!status.ok()) { AppendErrorStatus(overall_status, @@ -375,7 +388,7 @@ absl::Status NsfUpgradeTest::NsfUpgradeOrReboot( case NsfUpgradeScenario::kConfigPushBeforeAclFlowProgram: LOG(INFO) << upgrade_path << ": Proceeding with config push before ACL flow program"; - status = PushConfigAndValidate(next_image_config, + status = PushConfigAndValidate(curr_image_config, next_image_config, enable_interface_validation_during_nsf); if (!status.ok()) { AppendErrorStatus(overall_status, diff --git a/tests/integration/system/nsf/upgrade_test.h b/tests/integration/system/nsf/upgrade_test.h index 401bbbd83..98cc4fc53 100644 --- a/tests/integration/system/nsf/upgrade_test.h +++ b/tests/integration/system/nsf/upgrade_test.h @@ -38,7 +38,8 @@ class NsfUpgradeTest : public testing::TestWithParam { void TearDown() override; absl::Status PushConfigAndValidate( - const ImageConfigParams& image_config_params, + const ImageConfigParams& curr_image_config_params, + const ImageConfigParams& next_image_config_params, bool enable_interface_validation_during_nsf); // Assumption: Valid config (gNMI and P4Info) has been pushed (to avoid diff --git a/tests/integration/system/nsf/util.cc b/tests/integration/system/nsf/util.cc index 5f44f0642..bafd28cfa 100644 --- a/tests/integration/system/nsf/util.cc +++ b/tests/integration/system/nsf/util.cc @@ -747,6 +747,7 @@ absl::Status DoNsfRebootAndWaitForSwitchReadyOrRecover( ADD_FAILURE() << "NSF reboot failed with: " << nsf_reboot_status; LOG(ERROR) << "Attempting to recover the switch through cold " "reboot."; + thinkit::TestEnvironment& env = GetTestEnvironment(testbed); RETURN_IF_ERROR(Reboot(RebootMethod::COLD, sut, env, /*collect_debug_artifacts_before_reboot=*/false)); RETURN_IF_ERROR( diff --git a/tests/qos/frontpanel_qos_test.cc b/tests/qos/frontpanel_qos_test.cc index 04747e0d4..93159d825 100644 --- a/tests/qos/frontpanel_qos_test.cc +++ b/tests/qos/frontpanel_qos_test.cc @@ -1275,7 +1275,7 @@ TEST_P(FrontpanelQosTest, WeightedRoundRobinWeightsAreRespected) { const double kAbsoluteError = kActualFraction - kExpectedFraction; const double kRelativeErrorPercent = 100. * kAbsoluteError / kExpectedFraction; - const double kAcceptableErrorPercent = 7; + const double kAcceptableErrorPercent = 10; LOG(INFO) << "'" << queue << "' transmitted " << (kActualFraction * 100) << "% of forwarded round-robin traffic (expected: " << (kExpectedFraction * 100) diff --git a/tests/qos/pfc_test.cc b/tests/qos/pfc_test.cc index 3cb284f2c..72b590dea 100644 --- a/tests/qos/pfc_test.cc +++ b/tests/qos/pfc_test.cc @@ -147,7 +147,7 @@ void PfcTestWithIxia::SetUp() { // Configure SUT. ASSERT_OK_AND_ASSIGN(sut_p4rt_session_, pins_test::ConfigureSwitchAndReturnP4RuntimeSession( - sut, std::nullopt, std::nullopt)); + sut, std::nullopt, GetParam().p4info)); // Flow details. const auto dest_mac = netaddr::MacAddress(02, 02, 02, 02, 02, 02); From 3f933ef52e7f1e1ac5e6a4fd8f7800c11308532d Mon Sep 17 00:00:00 2001 From: kishanps Date: Thu, 19 Sep 2024 09:13:42 -0700 Subject: [PATCH 2/7] Adding mirror fail over test and gNMI Get() calls on root path to increase current switch operations that potentially impact quiescence time. --- lib/pins_control_device.cc | 4 +- p4_pdpi/utils/annotation_parser.cc | 17 +- .../config_db_queue_table_event_test.cc | 8 +- .../sonic/app_db_acl_def_table_manager.cc | 4 +- p4rt_app/sonic/packetio_port_test.cc | 48 ++- .../tests/forwarding_pipeline_config_test.cc | 3 +- tests/forwarding/l3_admit_test.cc | 4 - tests/forwarding/watch_port_test.cc | 2 +- tests/gnmi/BUILD.bazel | 1 + tests/gnmi/host_link_flap_tests.cc | 8 +- tests/gnmi/util.cc | 5 +- tests/gnmi/util.h | 3 +- .../nsf/component_validators/BUILD.bazel | 1 - .../component_validators/syncd_validator.cc | 2 +- .../system/nsf/interfaces/test_params.h | 1 + ...rrent_config_push_flow_programming_test.cc | 81 +++++ .../system/nsf/nsf_link_flap_test.cc | 2 +- tests/integration/system/nsf/util.cc | 1 - tests/integration/system/nsf/util.h | 3 + tests/qos/BUILD.bazel | 1 + tests/qos/frontpanel_qos_test.cc | 4 +- tests/qos/punt_qos_test.cc | 276 +++++++++++++++++- tests/qos/punt_qos_test.h | 4 + tests/qos/qos_test_util.cc | 21 +- tests/qos/qos_test_util.h | 10 +- 25 files changed, 435 insertions(+), 79 deletions(-) diff --git a/lib/pins_control_device.cc b/lib/pins_control_device.cc index 0d2f88e5c..f432f0ad1 100644 --- a/lib/pins_control_device.cc +++ b/lib/pins_control_device.cc @@ -141,9 +141,9 @@ PinsControlDevice::CollectPackets() { type: INSERT table_entry { acl_ingress_table_entry { - match {} # Wildcard match. + match {} # Wildcard match. action { acl_trap { qos_queue: "0x7" } } # Action: punt. - priority: 1 # Highest priority. + priority: 1 # Highest priority. } } })pb"))); diff --git a/p4_pdpi/utils/annotation_parser.cc b/p4_pdpi/utils/annotation_parser.cc index 2472b5df1..7c3340d95 100644 --- a/p4_pdpi/utils/annotation_parser.cc +++ b/p4_pdpi/utils/annotation_parser.cc @@ -55,8 +55,8 @@ absl::StatusOr ParseAnnotation( // (label-only): '@label ' --> 'label ' if (annotation_iter == annotation.end() || *annotation_iter != '@') { return gutil::InvalidArgumentErrorBuilder() - << "Annotation \"" << annotation - << "\" is malformed: " << "Annotations must begin with '@'"; + << "Annotation \"" << annotation << "\" is malformed: " + << "Annotations must begin with '@'"; } ++annotation_iter; @@ -71,8 +71,8 @@ absl::StatusOr ParseAnnotation( } if (annotation_iter == label_start_iter) { return gutil::InvalidArgumentErrorBuilder() - << "Annotation \"" << annotation - << "\" is malformed: " << "Annotation contains no label text"; + << "Annotation \"" << annotation << "\" is malformed: " + << "Annotation contains no label text"; } std::string label(label_start_iter, annotation_iter); @@ -91,9 +91,8 @@ absl::StatusOr ParseAnnotation( // '(body) ' --> 'body) ' if (*annotation_iter != '(') { return gutil::InvalidArgumentErrorBuilder() - << "Annotation \"" << annotation - << "\" is malformed: " << "Expected '(' but found '" - << *annotation_iter << "'"; + << "Annotation \"" << annotation << "\" is malformed: " + << "Expected '(' but found '" << *annotation_iter << "'"; } ++annotation_iter; const auto body_start_iter = annotation_iter; @@ -112,8 +111,8 @@ absl::StatusOr ParseAnnotation( // 'body)' --> 'body' if (*annotation_iter != ')') { return gutil::InvalidArgumentErrorBuilder() - << "Annotation \"" << annotation - << "\" is malformed: " << "Missing ')' at the end of the annotation"; + << "Annotation \"" << annotation << "\" is malformed: " + << "Missing ')' at the end of the annotation"; } std::string body(body_start_iter, annotation_iter); diff --git a/p4rt_app/event_monitoring/config_db_queue_table_event_test.cc b/p4rt_app/event_monitoring/config_db_queue_table_event_test.cc index 778513825..552a77689 100644 --- a/p4rt_app/event_monitoring/config_db_queue_table_event_test.cc +++ b/p4rt_app/event_monitoring/config_db_queue_table_event_test.cc @@ -35,8 +35,8 @@ MATCHER_P2(BidirectionallyMaps, name, id, bool success = true; auto id_lookup = arg.NameToId(name); if (!id_lookup.ok()) { - *result_listener << "arg.NameToId(" << name << ")" << " returned status " - << id_lookup.status(); + *result_listener << "arg.NameToId(" << name << ")" + << " returned status " << id_lookup.status(); success = false; } else if (*id_lookup != id) { *result_listener << "arg.NameToId(" << name << ")" @@ -45,8 +45,8 @@ MATCHER_P2(BidirectionallyMaps, name, id, } auto name_lookup = arg.IdToName(id); if (!name_lookup.ok()) { - *result_listener << "arg.IdToName(" << id << ")" << " returned status " - << name_lookup.status(); + *result_listener << "arg.IdToName(" << id << ")" + << " returned status " << name_lookup.status(); success = false; } else if (*name_lookup != name) { *result_listener << "arg.IdToName(" << id << ")" diff --git a/p4rt_app/sonic/app_db_acl_def_table_manager.cc b/p4rt_app/sonic/app_db_acl_def_table_manager.cc index b17dafa1e..40478eae4 100644 --- a/p4rt_app/sonic/app_db_acl_def_table_manager.cc +++ b/p4rt_app/sonic/app_db_acl_def_table_manager.cc @@ -447,8 +447,8 @@ StatusOr GenerateCompositeMatchFieldJson( return gutil::InvalidArgumentErrorBuilder() << "Composite field bitwidth (" << json["bitwidth"] << ") does not match total element bitwidth (" - << total_element_bitwidth << "). Annotation: " << "@" - << annotation.label << "(" << annotation.body << ")"; + << total_element_bitwidth << "). Annotation: " + << "@" << annotation.label << "(" << annotation.body << ")"; } return json; } diff --git a/p4rt_app/sonic/packetio_port_test.cc b/p4rt_app/sonic/packetio_port_test.cc index 108b8bd1f..8c6c5e5b1 100644 --- a/p4rt_app/sonic/packetio_port_test.cc +++ b/p4rt_app/sonic/packetio_port_test.cc @@ -53,11 +53,9 @@ using ::testing::Test; TEST(PacketIoTest, SendPacketOutOk) { MockSystemCallAdapter mock_call_adapter; - struct ifreq if_resp { /*ifr_name=*/ - {""}, /*ifr_flags=*/{ - { IFF_UP | IFF_RUNNING } - } - }; + struct ifreq if_resp{/*ifr_name=*/ + {""}, + /*ifr_flags=*/{{IFF_UP | IFF_RUNNING}}}; EXPECT_CALL(mock_call_adapter, ioctl) .WillOnce(DoAll(SetArgPointee<2>(if_resp), Return(0))); EXPECT_CALL(mock_call_adapter, write).WillOnce(Return(sizeof(kTestPacket))); @@ -82,11 +80,9 @@ TEST(PacketIoTest, SendPacketOutIoctlError) { TEST(PacketIoTest, SendPacketOutLinkDown) { MockSystemCallAdapter mock_call_adapter; - struct ifreq if_resp { /*ifr_name=*/ - {""}, /*ifr_flags=*/{ - { IFF_UP } - } - }; + struct ifreq if_resp{/*ifr_name=*/ + {""}, + /*ifr_flags=*/{{IFF_UP}}}; EXPECT_CALL(mock_call_adapter, ioctl) .WillOnce(DoAll(SetArgPointee<2>(if_resp), Return(0))); @@ -99,11 +95,9 @@ TEST(PacketIoTest, SendPacketOutLinkDown) { TEST(PacketIoTest, SendPacketOutWriteError) { MockSystemCallAdapter mock_call_adapter; - struct ifreq if_resp { /*ifr_name=*/ - {""}, /*ifr_flags=*/{ - { IFF_UP | IFF_RUNNING } - } - }; + struct ifreq if_resp{/*ifr_name=*/ + {""}, + /*ifr_flags=*/{{IFF_UP | IFF_RUNNING}}}; EXPECT_CALL(mock_call_adapter, ioctl) .WillOnce(DoAll(SetArgPointee<2>(if_resp), Return(0))); EXPECT_CALL(mock_call_adapter, write).WillOnce(Return(-1)); @@ -118,11 +112,9 @@ TEST(PacketIoTest, SendPacketOutWriteError) { TEST(PacketIoTest, SendPacketOutFailsOnGetSockOptError) { MockSystemCallAdapter mock_call_adapter; - struct ifreq if_resp { /*ifr_name=*/ - {""}, /*ifr_flags=*/{ - { IFF_UP | IFF_RUNNING } - } - }; + struct ifreq if_resp{/*ifr_name=*/ + {""}, + /*ifr_flags=*/{{IFF_UP | IFF_RUNNING}}}; EXPECT_CALL(mock_call_adapter, ioctl) .WillOnce(DoAll(SetArgPointee<2>(if_resp), Return(0))); EXPECT_CALL(mock_call_adapter, getsockopt).WillOnce(Return(-1)); @@ -137,11 +129,9 @@ TEST(PacketIoTest, SendPacketOutFailsOnGetSockOptError) { TEST(PacketIoTest, SendPacketOutOkWithClearPendingError) { MockSystemCallAdapter mock_call_adapter; - struct ifreq if_resp { /*ifr_name=*/ - {""}, /*ifr_flags=*/{ - { IFF_UP | IFF_RUNNING } - } - }; + struct ifreq if_resp{/*ifr_name=*/ + {""}, + /*ifr_flags=*/{{IFF_UP | IFF_RUNNING}}}; EXPECT_CALL(mock_call_adapter, ioctl) .WillOnce(DoAll(SetArgPointee<2>(if_resp), Return(0))); int optval = 100; @@ -158,11 +148,9 @@ TEST(PacketIoTest, SendPacketOutOkWithClearPendingError) { TEST(PacketIoTest, SendPacketOutSplitWrite) { MockSystemCallAdapter mock_call_adapter; - struct ifreq if_resp { /*ifr_name=*/ - {""}, /*ifr_flags=*/{ - { IFF_UP | IFF_RUNNING } - } - }; + struct ifreq if_resp{/*ifr_name=*/ + {""}, + /*ifr_flags=*/{{IFF_UP | IFF_RUNNING}}}; EXPECT_CALL(mock_call_adapter, ioctl) .WillOnce(DoAll(SetArgPointee<2>(if_resp), Return(0))); EXPECT_CALL(mock_call_adapter, write) diff --git a/p4rt_app/tests/forwarding_pipeline_config_test.cc b/p4rt_app/tests/forwarding_pipeline_config_test.cc index 8054fe3b0..a730b0ed2 100644 --- a/p4rt_app/tests/forwarding_pipeline_config_test.cc +++ b/p4rt_app/tests/forwarding_pipeline_config_test.cc @@ -230,7 +230,8 @@ TEST_F(VerifyTest, DoesNotUpdateDatabases) { // this failure with suggestions for fixing verification issues. EXPECT_OK(p4rt_session_->SetForwardingPipelineConfig(request)) << "Is the p4info_verification_schema updated? If not, run: " - << "p4rt_app/scripts/" << "update_p4info_verification_schema.sh"; + << "p4rt_app/scripts/" + << "update_p4info_verification_schema.sh"; EXPECT_THAT(p4rt_service_->GetP4rtAppDbTable().GetAllKeys(), IsEmpty()); EXPECT_THAT(p4rt_service_->GetHostStatsStateDbTable().GetAllKeys(), IsEmpty()); diff --git a/tests/forwarding/l3_admit_test.cc b/tests/forwarding/l3_admit_test.cc index 7ec03dbc7..8cbf885e0 100644 --- a/tests/forwarding/l3_admit_test.cc +++ b/tests/forwarding/l3_admit_test.cc @@ -826,10 +826,6 @@ TEST_P(L3AdmitTestFixture, VlanOverrideAdmitsAllPacketsToL3Routing) { table_entry { table_name: "acl_pre_ingress_vlan_table" priority: 10 - matches { - name: "is_ipv4" - optional { value { hex_str: "0x1" } } - } action { name: "set_outer_vlan_id", params { diff --git a/tests/forwarding/watch_port_test.cc b/tests/forwarding/watch_port_test.cc index 3707c900f..c0078ca95 100644 --- a/tests/forwarding/watch_port_test.cc +++ b/tests/forwarding/watch_port_test.cc @@ -183,7 +183,7 @@ absl::Status SetUpSut(pdpi::P4RuntimeSession& p4_session, ir_p4info, gutil::ParseProtoOrDie(absl::Substitute( R"pb( acl_pre_ingress_table_entry { - match {} # Wildcard match + match {} # Wildcard match action { set_vrf { vrf_id: "$0" } } # Default vrf priority: 1129 })pb", diff --git a/tests/gnmi/BUILD.bazel b/tests/gnmi/BUILD.bazel index e69741c10..76dfbb67c 100644 --- a/tests/gnmi/BUILD.bazel +++ b/tests/gnmi/BUILD.bazel @@ -85,6 +85,7 @@ cc_library( "@com_github_google_glog//:glog", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/strings:str_format", + "@com_google_absl//absl/time", "@com_google_absl//absl/types:span", "@com_google_googletest//:gtest", ], diff --git a/tests/gnmi/host_link_flap_tests.cc b/tests/gnmi/host_link_flap_tests.cc index 5a30c978a..a3a23f65c 100644 --- a/tests/gnmi/host_link_flap_tests.cc +++ b/tests/gnmi/host_link_flap_tests.cc @@ -16,6 +16,7 @@ #include "absl/container/flat_hash_map.h" #include "absl/strings/str_format.h" +#include "absl/time/time.h" #include "absl/types/span.h" #include "glog/logging.h" #include "gmock/gmock.h" @@ -36,6 +37,8 @@ using ::gutil::IsOkAndHolds; using ::testing::Contains; using ::testing::IsEmpty; +constexpr absl::Duration kFlapTimeout = absl::Minutes(2); + TEST_P(ExampleTestFixture, LinkFlapTest) { LOG(INFO) << "Get testbed requirements."; thinkit::TestRequirements requirements = @@ -86,7 +89,8 @@ TEST_P(ExampleTestFixture, LinkFlapTest) { // device detects it. // Set admin-status DOWN through gNMI. LOG(INFO) << absl::StrFormat("Set SUT port %s admin DOWN", sut_interface); - EXPECT_OK(SetAdminStatus(gnmi_stub.get(), sut_interface, "DOWN")); + EXPECT_OK( + SetAdminStatus(gnmi_stub.get(), sut_interface, "DOWN", kFlapTimeout)); LOG(INFO) << absl::StrFormat("Validate SUT port %s state: DOWN", sut_interface); EXPECT_THAT(GetInterfaceOperStatusOverGnmi(*gnmi_stub, sut_interface), @@ -94,7 +98,7 @@ TEST_P(ExampleTestFixture, LinkFlapTest) { // Set admin-status UP through gNMI. LOG(INFO) << absl::StrFormat("Set SUT port %s admin UP", sut_interface); - EXPECT_OK(SetAdminStatus(gnmi_stub.get(), sut_interface, "UP")); + EXPECT_OK(SetAdminStatus(gnmi_stub.get(), sut_interface, "UP", kFlapTimeout)); LOG(INFO) << absl::StrFormat("Validate control device port %s state: UP", peer_interface); EXPECT_THAT(generic_testbed->ControlDevice(peer_device_index) diff --git a/tests/gnmi/util.cc b/tests/gnmi/util.cc index d68c81e53..d880dbec4 100644 --- a/tests/gnmi/util.cc +++ b/tests/gnmi/util.cc @@ -92,7 +92,8 @@ CheckControlDeviceInterfaceLinkState(thinkit::GenericTestbed &generic_testbed, absl::Status SetAdminStatus(gnmi::gNMI::StubInterface* gnmi_stub, absl::string_view if_name, - absl::string_view if_status) { + absl::string_view if_status, + absl::Duration timeout) { std::string enable_status; if (if_status == "UP") { enable_status = kEnabledTrue; @@ -108,7 +109,7 @@ absl::Status SetAdminStatus(gnmi::gNMI::StubInterface* gnmi_stub, absl::StrCat("interfaces/interface[name=", if_name, "]/config/enabled"); RETURN_IF_ERROR(pins_test::SetGnmiConfigPath( gnmi_stub, if_enabled_config_path, GnmiSetType::kUpdate, enable_status)); - absl::SleepFor(absl::Seconds(15)); + absl::SleepFor(timeout); // Verifies /interfaces/interface[name=]/state/admin-status = UP/DOWN. std::string if_state_path = diff --git a/tests/gnmi/util.h b/tests/gnmi/util.h index 759be80be..28e434f56 100644 --- a/tests/gnmi/util.h +++ b/tests/gnmi/util.h @@ -10,7 +10,8 @@ namespace pins_test { absl::Status SetAdminStatus(gnmi::gNMI::StubInterface* gnmi_stub, absl::string_view if_name, - absl::string_view if_status); + absl::string_view if_status, + absl::Duration timeout = absl::Seconds(15)); absl::Status FlapLink(gnmi::gNMI::StubInterface &gnmi_stub, thinkit::GenericTestbed &generic_testbed, diff --git a/tests/integration/system/nsf/component_validators/BUILD.bazel b/tests/integration/system/nsf/component_validators/BUILD.bazel index 801c08e2a..26080eaa8 100644 --- a/tests/integration/system/nsf/component_validators/BUILD.bazel +++ b/tests/integration/system/nsf/component_validators/BUILD.bazel @@ -86,7 +86,6 @@ cc_library( srcs = ["syncd_validator.cc"], hdrs = ["syncd_validator.h"], deps = [ - "//lib/gnmi:gnmi_helper", "//tests/integration/system/nsf:util", "//tests/integration/system/nsf/interfaces:component_validator", "//tests/integration/system/nsf/interfaces:testbed", diff --git a/tests/integration/system/nsf/component_validators/syncd_validator.cc b/tests/integration/system/nsf/component_validators/syncd_validator.cc index 1cb1b6a26..50a7cd011 100644 --- a/tests/integration/system/nsf/component_validators/syncd_validator.cc +++ b/tests/integration/system/nsf/component_validators/syncd_validator.cc @@ -18,10 +18,10 @@ #include "absl/container/flat_hash_set.h" #include "absl/status/status.h" #include "absl/strings/match.h" +#include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" #include "glog/logging.h" -#include "lib/gnmi/gnmi_helper.h" #include "tests/integration/system/nsf/interfaces/testbed.h" #include "tests/integration/system/nsf/util.h" #include "thinkit/ssh_client.h" diff --git a/tests/integration/system/nsf/interfaces/test_params.h b/tests/integration/system/nsf/interfaces/test_params.h index 72602b438..f10fcf80d 100644 --- a/tests/integration/system/nsf/interfaces/test_params.h +++ b/tests/integration/system/nsf/interfaces/test_params.h @@ -45,6 +45,7 @@ struct NsfTestParams { std::function()> create_ssh_client; std::function(NsfUpgradeScenario)> get_test_case_ids; bool enable_interface_validation_during_nsf = true; + bool enable_dynamic_replay = false; }; } // namespace pins_test diff --git a/tests/integration/system/nsf/nsf_concurrent_config_push_flow_programming_test.cc b/tests/integration/system/nsf/nsf_concurrent_config_push_flow_programming_test.cc index fce029620..d3184cdf4 100644 --- a/tests/integration/system/nsf/nsf_concurrent_config_push_flow_programming_test.cc +++ b/tests/integration/system/nsf/nsf_concurrent_config_push_flow_programming_test.cc @@ -25,6 +25,8 @@ #include "absl/time/time.h" #include "glog/logging.h" #include "gmock/gmock.h" +#include "google/protobuf/message.h" +#include "grpcpp/client_context.h" #include "gtest/gtest.h" #include "gutil/status.h" #include "gutil/status_matchers.h" @@ -51,6 +53,9 @@ constexpr int kMinNsfDelayDuration = 1; constexpr int kMaxNsfDelayDuration = 15; constexpr absl::Duration kTurnUpTimeout = absl::Minutes(6); constexpr char kInterfaceToRemove[] = "Ethernet1/10/1"; +constexpr int kMaxGnmiGetClients = 15; +constexpr int kMaxGnmiSubscribeClients = 10; +constexpr absl::Duration kSubscribeWaitTime = absl::Seconds(100); void NsfConcurrentConfigPushFlowProgrammingTestFixture::SetUp() { flow_programmer_ = GetParam().create_flow_programmer(); @@ -146,6 +151,72 @@ TEST_P(NsfConcurrentConfigPushFlowProgrammingTestFixture, std::thread flow_programming_thread( [&]() { flow_programming_status = flow_programming_func(); }); + // Get gNMI info thread. Fetch gNMI information multiple times periodically + // from different clients to increase stress from Get() clients. + auto get_gnmi_func = [&nsf_delay_duration, &sut]() -> absl::Status { + // Wait for sometime before initiating the gNMI connection. The order of + // test execution is as follows: + // 1) Create Get() threads. + // 2) Create NSF reboot thread. + // In each Get() thread, wait for sometime and then start Get() operation. + // This increases the chances of hitting the race condition between gNMI + // operation and NSF reboot request. + absl::SleepFor(absl::Seconds(nsf_delay_duration)); + LOG(INFO) << "Fetching gNMI information on " << sut.ChassisName(); + ASSIGN_OR_RETURN(auto gnmi_stub, sut.CreateGnmiStub()); + auto status = GetGnmiStatePathInfo(gnmi_stub.get(), ""); + LOG(INFO) << "gNMI Get() operation completed"; + return absl::OkStatus(); + }; + std::vector gnmi_get_threads; + for (int i = 0; i < kMaxGnmiGetClients; i++) { + gnmi_get_threads.emplace_back(get_gnmi_func); + } + + // gNMI subscribe thread to increase stress from gNMI subscribe clients. + auto subscribe_gnmi_func = [&nsf_delay_duration, &sut]() -> absl::Status { + // Wait for sometime before initiating the gNMI connection (same reason as + // gNMI Get() thread above). + absl::SleepFor(absl::Seconds(nsf_delay_duration)); + ASSIGN_OR_RETURN(auto gnmi_stub, sut.CreateGnmiStub()); + grpc::ClientContext context; + auto stream = gnmi_stub->Subscribe(&context); + if (stream == nullptr) { + return absl::InternalError("Cannot create gNMI stream"); + } + gnmi::SubscribeRequest subscribe_request; + gnmi::SubscriptionList* subscription_list = + subscribe_request.mutable_subscribe(); + subscription_list->set_mode(gnmi::SubscriptionList::STREAM); + subscription_list->mutable_prefix()->set_origin(kOpenconfigStr); + subscription_list->mutable_prefix()->set_target(kTarget); + AddSubtreeToGnmiSubscription("", *subscription_list, gnmi::SAMPLE, + /*suppress_redundant=*/false, + absl::Seconds(1)); + LOG(INFO) << "Sending subscription request: " + << google::protobuf::ShortFormat(subscribe_request); + if (!stream->Write(subscribe_request)) { + return absl::InternalError("Failed to write subscribe request"); + } + LOG(INFO) << "Write subscribe request successful. Reading from stream.."; + const auto start_time = absl::Now(); + while (absl::Now() < (start_time + kSubscribeWaitTime)) { + gnmi::SubscribeResponse response; + if (!stream->Read(&response)) { + // Stream to server is broken possibly due to warm reboot. + LOG(INFO) << "Subscribe client cannot read from the stream"; + return absl::OkStatus(); + } + LOG(INFO) << "Subscribe response received"; + } + return absl::InternalError( + "subscribe client didn't break stream with server"); + }; + std::vector gnmi_subscribe_threads; + for (int i = 0; i < kMaxGnmiSubscribeClients; i++) { + gnmi_subscribe_threads.emplace_back(subscribe_gnmi_func); + } + // NSF Reboot thread. absl::Status nsf_reboot_status = absl::UnknownError("Yet to do nsf reboot"); ReadResponse p4flow_snapshot2; @@ -181,6 +252,16 @@ TEST_P(NsfConcurrentConfigPushFlowProgrammingTestFixture, // SaveP4FlowSnapshot function before the NSF reboot. EXPECT_OK(SaveP4FlowSnapshot(p4flow_snapshot2, "p4flow_snapshot2_before_nsf.txt", environment)); + // Since gNMI Get and Subscribe clients are read-only and are being called to + // add delay during quiescence, we don't really care about the result of these + // calls. Therefore, joining these threads after all information has been + // fetched from the switch to perform validations. + for (auto& gnmi_get_thread : gnmi_get_threads) { + gnmi_get_thread.join(); + } + for (auto& gnmi_subscribe_thread : gnmi_subscribe_threads) { + gnmi_subscribe_thread.join(); + } EXPECT_OK(config_push_status) << "Failed to push config"; EXPECT_OK(flow_programming_status) << "Failed to program flows"; ASSERT_OK(nsf_reboot_status) << "Failed to initiate NSF reboot"; diff --git a/tests/integration/system/nsf/nsf_link_flap_test.cc b/tests/integration/system/nsf/nsf_link_flap_test.cc index afa0161f6..96ecfdb7f 100644 --- a/tests/integration/system/nsf/nsf_link_flap_test.cc +++ b/tests/integration/system/nsf/nsf_link_flap_test.cc @@ -43,7 +43,7 @@ namespace pins_test { namespace { -constexpr absl::Duration kLinkFlapTimeout = absl::Seconds(60); +constexpr absl::Duration kLinkFlapTimeout = absl::Seconds(90); // Flaps all the links in parallel and returns the total time taken to flap all // the links. diff --git a/tests/integration/system/nsf/util.cc b/tests/integration/system/nsf/util.cc index bafd28cfa..e2bb93153 100644 --- a/tests/integration/system/nsf/util.cc +++ b/tests/integration/system/nsf/util.cc @@ -77,7 +77,6 @@ using ::p4::v1::Entity; using ::p4::v1::ReadRequest; using ::p4::v1::ReadResponse; -constexpr absl::Duration kNsfRebootWaitTime = absl::Minutes(11); constexpr absl::Duration kPollingInterval = absl::Seconds(10); constexpr absl::Duration kTurnUpTimeout = absl::Minutes(6); constexpr absl::Duration kTurnDownTimeout = absl::Minutes(2); diff --git a/tests/integration/system/nsf/util.h b/tests/integration/system/nsf/util.h index c3185f073..e9d2322a7 100644 --- a/tests/integration/system/nsf/util.h +++ b/tests/integration/system/nsf/util.h @@ -21,6 +21,7 @@ #include #include "absl/base/nullability.h" +#include "absl/flags/declare.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_format.h" @@ -37,6 +38,8 @@ namespace pins_test { +constexpr absl::Duration kNsfRebootWaitTime = absl::Minutes(11); + struct PinsSoftwareInfo { std::string name; std::string oper_status; diff --git a/tests/qos/BUILD.bazel b/tests/qos/BUILD.bazel index 9f40969d2..f9f88c4c2 100644 --- a/tests/qos/BUILD.bazel +++ b/tests/qos/BUILD.bazel @@ -122,6 +122,7 @@ cc_library( "//sai_p4/instantiations/google:sai_pd_cc_proto", "//sai_p4/instantiations/google/test_tools:test_entries", "//tests/forwarding:util", + "//tests/gnmi: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 93159d825..893f9f245 100644 --- a/tests/qos/frontpanel_qos_test.cc +++ b/tests/qos/frontpanel_qos_test.cc @@ -2319,9 +2319,9 @@ TEST_P(FrontpanelBufferTest, BufferCarving) { params_by_queue_name[queue_name].weight = 1; } params_by_queue_name[queue_name].committed_information_rate = 0; - // Limit peak rate to 10% of line rate so queue is always full. + // Limit peak rate to 1% of line rate so queue is always full. params_by_queue_name[queue_name].peak_information_rate = - 0.1 * links.egress_link.sut_interface_bits_per_second / 8; + 0.01 * links.egress_link.sut_interface_bits_per_second / 8; } ASSERT_OK(SetSchedulerPolicyParameters(kSutEgressPortSchedulerPolicy, params_by_queue_name, *gnmi_stub)); diff --git a/tests/qos/punt_qos_test.cc b/tests/qos/punt_qos_test.cc index 8b1f01aa3..320cf81e5 100644 --- a/tests/qos/punt_qos_test.cc +++ b/tests/qos/punt_qos_test.cc @@ -54,6 +54,7 @@ #include "sai_p4/instantiations/google/sai_pd.pb.h" #include "sai_p4/instantiations/google/test_tools/test_entries.h" #include "tests/forwarding/util.h" +#include "tests/gnmi/util.h" #include "tests/lib/switch_test_setup_helpers.h" #include "tests/qos/packet_in_receiver.h" #include "tests/qos/qos_test_util.h" @@ -141,11 +142,20 @@ void PuntQoSTestWithIxia::SetUp() { ixia_sut_link_.sut_rx_interface = ready_links[0].sut_interface; ixia_sut_link_.ixia_rx_interface = ready_links[1].ixia_interface; ixia_sut_link_.sut_tx_interface = ready_links[1].sut_interface; + ixia_sut_link_.ixia_mirror_interface = ready_links[2].ixia_interface; + ixia_sut_link_.sut_mirror_interface = ready_links[2].sut_interface; + ixia_sut_link_.ixia_mirror_backup_interface = ready_links[3].ixia_interface; + ixia_sut_link_.sut_mirror_backup_interface = ready_links[3].sut_interface; LOG(INFO) << absl::StrFormat( - "Test packet route: [Ixia: %s] => [SUT: %s] -> [SUT: %s] => [Ixia: %s]", + "Test packet route: [Ixia: %s] => [SUT: %s] -> [SUT: %s] => [Ixia: %s] " + "SUT Mirror port %s => Ixia port %s, SUT Mirror backup port %s => Ixia " + "port %s", ixia_sut_link_.ixia_tx_interface, ixia_sut_link_.sut_rx_interface, - ixia_sut_link_.sut_rx_interface, ixia_sut_link_.ixia_rx_interface); + ixia_sut_link_.sut_tx_interface, ixia_sut_link_.ixia_rx_interface, + ixia_sut_link_.sut_mirror_interface, ixia_sut_link_.ixia_mirror_interface, + ixia_sut_link_.sut_mirror_backup_interface, + ixia_sut_link_.ixia_mirror_backup_interface); ASSERT_OK_AND_ASSIGN(p4rt_id_by_interface_, GetAllInterfaceNameToPortId(*sut_gnmi_stub_)); @@ -196,6 +206,7 @@ const sai::NexthopRewriteOptions kNextHopRewriteOptions = { // some amount of variance in rate measured end to end. // Level of tolerance for packet rate verification. constexpr float kTolerancePercent = 5.0; +constexpr float kQueueCountersTolerancePercent = 10.0; // Ixia configurations: // 1. Frames sent per second by Ixia. @@ -207,7 +218,15 @@ constexpr int kFramesPerSecond = 1000000; constexpr int kTotalFrames = 10000000; constexpr absl::Duration kTrafficDuration = absl::Seconds(kTotalFrames / kFramesPerSecond); -constexpr int kFrameSize = 9212; +constexpr int kFrameSize = 8000; + +// Constants for lower rate traffic. +const int kTotalFramesTrafficLowTrafficRate = 500'000; +const int kBytesPerSecondLowTrafficRate = 200'000'000; // 200 MBps +const int kFramesPerSecondLowTrafficRate = + kBytesPerSecondLowTrafficRate / (kFrameSize); +constexpr absl::Duration kTrafficDurationLowTrafficRate = absl::Seconds( + kTotalFramesTrafficLowTrafficRate / kFramesPerSecondLowTrafficRate); struct PacketReceiveInfo { absl::Mutex mutex; @@ -244,6 +263,17 @@ TEST_P(PuntQoSTestWithIxia, SetDscpAndQueuesAndDenyAboveRateLimit) { ASSERT_OK_AND_ASSIGN(const std::string kSutEgressPortP4rtId, gutil::FindOrStatus(p4rt_id_by_interface_, ixia_sut_link_.sut_tx_interface)); + ASSERT_OK_AND_ASSIGN(const std::string kSutIngressPortP4rtId, + gutil::FindOrStatus(p4rt_id_by_interface_, + ixia_sut_link_.sut_rx_interface)); + ASSERT_OK_AND_ASSIGN( + const std::string kSutMirrorToPortP4rtId, + gutil::FindOrStatus(p4rt_id_by_interface_, + ixia_sut_link_.sut_mirror_interface)); + ASSERT_OK_AND_ASSIGN( + const std::string kSutMirrorToBackupPortP4rtId, + gutil::FindOrStatus(p4rt_id_by_interface_, + ixia_sut_link_.sut_mirror_backup_interface)); // Listen for punted packets from the SUT. PacketReceiveInfo packet_receive_info; { @@ -294,6 +324,20 @@ TEST_P(PuntQoSTestWithIxia, SetDscpAndQueuesAndDenyAboveRateLimit) { .burst_bytes = kFrameSize, }; + // Add forwarding rule and mirror rule. + sai::MirrorSessionParams mirror_session_params = { + .mirror_session_id = "1", + .monitor_port = kSutMirrorToPortP4rtId, + .monitor_backup_port = kSutMirrorToBackupPortP4rtId, + .mirror_encap_src_mac = "02:02:02:02:02:01", + .mirror_encap_dst_mac = "02:02:02:02:02:02", + .mirror_encap_vlan_id = "0x001", + .mirror_encap_src_ip = "2000::1", + .mirror_encap_dst_ip = "2000::2", + .mirror_encap_udp_src_port = "0x1000", + .mirror_encap_udp_dst_port = "0x1001", + }; + ASSERT_OK_AND_ASSIGN( std::vector entities, sai::EntryBuilder() @@ -305,6 +349,10 @@ TEST_P(PuntQoSTestWithIxia, SetDscpAndQueuesAndDenyAboveRateLimit) { dest_mac.ToString(), sai::PuntAction::kCopy, kDefaultCosQueue) .AddEntryToSetDscpAndQueuesAndDenyAboveRateLimit( queue_assignments, meter_configuration) + .AddMirrorSessionTableEntry(mirror_session_params) + .AddMarkToMirrorAclEntry( + {.ingress_port = kSutIngressPortP4rtId, + .mirror_session_id = mirror_session_params.mirror_session_id}) .LogPdEntries() .GetDedupedPiEntities(ir_p4info)); @@ -334,6 +382,24 @@ TEST_P(PuntQoSTestWithIxia, SetDscpAndQueuesAndDenyAboveRateLimit) { ASSERT_OK(ixia::SetTrafficParameters( ixia_traffic_handle_, traffic_parameters, *generic_testbed_)); + // Get packet count for Mirror-To-Port. + ASSERT_OK_AND_ASSIGN( + uint64_t mirror_packets_pre, + GetInterfaceCounter("out-unicast-pkts", + ixia_sut_link_.sut_mirror_interface, + sut_gnmi_stub_.get())); + LOG(INFO) << "Mirror-To-Port packets pre: " << mirror_packets_pre; + // Get queue counters for Mirror-To-Port. + ASSERT_OK_AND_ASSIGN( + QueueCounters initial_mirror_green_queue_counters, + GetGnmiQueueCounters(ixia_sut_link_.sut_mirror_interface, + GetParam().multicast_green_queue, + *sut_gnmi_stub_)); + + ASSERT_OK_AND_ASSIGN( + QueueCounters initial_mirror_red_queue_counters, + GetGnmiQueueCounters(ixia_sut_link_.sut_mirror_interface, + GetParam().multicast_red_queue, *sut_gnmi_stub_)); // 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), [&] { @@ -446,6 +512,71 @@ TEST_P(PuntQoSTestWithIxia, SetDscpAndQueuesAndDenyAboveRateLimit) { (1 + kTolerancePercent / 100)); EXPECT_GE(kObservedTrafficRate, flow_rate_limit_in_bytes_per_second * (1 - kTolerancePercent / 100)); + + ASSERT_OK_AND_ASSIGN( + auto mirror_packets_post, + GetInterfaceCounter("out-unicast-pkts", + ixia_sut_link_.sut_mirror_interface, + sut_gnmi_stub_.get())); + + LOG(INFO) << "Mirror-To-Port packets post: " << mirror_packets_post; + + ASSERT_OK_AND_ASSIGN( + QueueCounters final_mirror_green_queue_counters, + GetGnmiQueueCounters(ixia_sut_link_.sut_mirror_interface, + GetParam().multicast_green_queue, + *sut_gnmi_stub_)); + + ASSERT_OK_AND_ASSIGN( + QueueCounters final_mirror_red_queue_counters, + GetGnmiQueueCounters(ixia_sut_link_.sut_mirror_interface, + GetParam().multicast_red_queue, + *sut_gnmi_stub_)); + + QueueCounters delta_mirror_green = final_mirror_green_queue_counters - + initial_mirror_green_queue_counters; + QueueCounters delta_mirror_red = + final_mirror_red_queue_counters - initial_mirror_red_queue_counters; + LOG(INFO) << "Mirror-To-Port green queue delta: " << delta_mirror_green; + LOG(INFO) << "Mirror-To-Port red queue delta: " << delta_mirror_red; + + ASSERT_OK_AND_ASSIGN( + auto mirror_queue_info, + ExtractQueueInfoViaGnmiConfig(ixia_sut_link_.sut_mirror_interface, + sut_gnmi_config_, false)); + int64_t mirror_red_queue_rate_limit = + mirror_queue_info[GetParam().multicast_red_queue] + .rate_packets_per_second; + LOG(INFO) << "Mirror-To-Port red queue rate limit (bps): " + << mirror_red_queue_rate_limit; + int64_t expected_green_mirror_packets = + (absl::ToDoubleSeconds(kTrafficDuration) * + flow_rate_limit_in_bytes_per_second) / + kFrameSize; + int64_t expected_red_mirror_packets = + (absl::ToDoubleSeconds(kTrafficDuration) * + (mirror_red_queue_rate_limit / (8 * kFrameSize))); + int64_t expected_mirror_packets = + expected_green_mirror_packets + expected_red_mirror_packets; + LOG(INFO) << "Expected mirror packets: " << expected_mirror_packets; + // Verify Mirror-To-Port counters. + EXPECT_GE(mirror_packets_post - mirror_packets_pre, + expected_mirror_packets * (1 - kTolerancePercent / 100)); + EXPECT_LE(mirror_packets_post - mirror_packets_pre, + expected_mirror_packets * (1 + kTolerancePercent / 100)); + + EXPECT_GE(delta_mirror_green.num_packets_transmitted, + expected_green_mirror_packets * + (1 - kQueueCountersTolerancePercent / 100)); + EXPECT_LE(delta_mirror_green.num_packets_transmitted, + expected_green_mirror_packets * + (1 + kQueueCountersTolerancePercent / 100)); + EXPECT_GE(delta_mirror_red.num_packets_transmitted, + expected_red_mirror_packets * + (1 - kQueueCountersTolerancePercent / 100)); + EXPECT_LE(delta_mirror_red.num_packets_transmitted, + expected_red_mirror_packets * + (1 + kQueueCountersTolerancePercent / 100)); } } // for each queue. @@ -453,5 +584,144 @@ TEST_P(PuntQoSTestWithIxia, SetDscpAndQueuesAndDenyAboveRateLimit) { receiver.Destroy(); } +// TODO: Disabled till failover is fixed. +TEST_P(PuntQoSTestWithIxia, DISABLED_MirrorFailover) { + // Flow details. + const auto dest_mac = netaddr::MacAddress(02, 02, 02, 02, 02, 02); + const auto source_mac = netaddr::MacAddress(00, 01, 02, 03, 04, 05); + const auto source_ip = netaddr::Ipv4Address(192, 168, 10, 1); + const auto dest_ip = netaddr::Ipv4Address(172, 0, 0, 1); + + auto traffic_parameters = ixia::TrafficParameters{ + .frame_count = kTotalFramesTrafficLowTrafficRate, + .frame_size_in_bytes = kFrameSize, + .traffic_speed = ixia::FramesPerSecond{kFramesPerSecondLowTrafficRate}, + .src_mac = source_mac, + .dst_mac = dest_mac, + .ip_parameters = ixia::Ipv4TrafficParameters{ + .src_ipv4 = source_ip, + .dst_ipv4 = dest_ip, + // Set ECN 0 to avoid RED drops. + .priority = ixia::IpPriority{.dscp = 0, .ecn = 0}, + }}; + + ASSERT_OK_AND_ASSIGN(pdpi::IrP4Info ir_p4info, + pdpi::CreateIrP4Info(GetParam().p4info)); + ASSERT_OK_AND_ASSIGN(const std::string kSutIngressPortP4rtId, + gutil::FindOrStatus(p4rt_id_by_interface_, + ixia_sut_link_.sut_rx_interface)); + ASSERT_OK_AND_ASSIGN(const std::string kSutEgressPortP4rtId, + gutil::FindOrStatus(p4rt_id_by_interface_, + ixia_sut_link_.sut_tx_interface)); + ASSERT_OK_AND_ASSIGN( + const std::string kSutMirrorToPortP4rtId, + gutil::FindOrStatus(p4rt_id_by_interface_, + ixia_sut_link_.sut_mirror_interface)); + ASSERT_OK_AND_ASSIGN( + const std::string kSutMirrorToBackupPortP4rtId, + gutil::FindOrStatus(p4rt_id_by_interface_, + ixia_sut_link_.sut_mirror_backup_interface)); + + ASSERT_OK(pdpi::ClearEntities(*sut_p4_session_)); + // Add forwarding rule and mirror rule. + sai::MirrorSessionParams mirror_session_params = { + .mirror_session_id = "1", + .monitor_port = kSutMirrorToPortP4rtId, + .monitor_backup_port = kSutMirrorToBackupPortP4rtId, + .mirror_encap_src_mac = "02:02:02:02:02:01", + .mirror_encap_dst_mac = "02:02:02:02:02:02", + .mirror_encap_vlan_id = "0x001", + .mirror_encap_src_ip = "2000::1", + .mirror_encap_dst_ip = "2000::2", + .mirror_encap_udp_src_port = "0x1000", + .mirror_encap_udp_dst_port = "0x1001", + }; + + ASSERT_OK( + sai::EntryBuilder() + .AddEntriesForwardingIpPacketsToGivenPort( + /*egress_port=*/kSutEgressPortP4rtId, + /*ip_version=*/sai::IpVersion::kIpv4And6, + /*rewrite_options*/ kNextHopRewriteOptions) + .AddMirrorSessionTableEntry(mirror_session_params) + .AddMarkToMirrorAclEntry( + {.ingress_port = kSutIngressPortP4rtId, + .mirror_session_id = mirror_session_params.mirror_session_id}) + .LogPdEntries() + .InstallDedupedEntities(*sut_p4_session_)); + + generic_testbed_->Environment().SetTestCaseID( + "46255587-1f7c-4254-8bd2-17b50e7f68f3"); + + ASSERT_OK(ixia::SetTrafficParameters(ixia_traffic_handle_, traffic_parameters, + *generic_testbed_)); + + // Get packet count for Mirror-To-Port. + ASSERT_OK_AND_ASSIGN(uint64_t mirror_packets_mtp_pre, + GetInterfaceCounter("out-unicast-pkts", + ixia_sut_link_.sut_mirror_interface, + sut_gnmi_stub_.get())); + ASSERT_OK_AND_ASSIGN( + uint64_t mirror_packets_backup_mtp_pre, + GetInterfaceCounter("out-unicast-pkts", + ixia_sut_link_.sut_mirror_backup_interface, + sut_gnmi_stub_.get())); + LOG(INFO) << "Mirror-To-Port packets pre: " << mirror_packets_mtp_pre; + LOG(INFO) << "Mirror-To-Backup Port packets pre: " + << mirror_packets_backup_mtp_pre; + + // 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(ixia_traffic_handle_, ixia_handle_, + *generic_testbed_); + })); + + LOG(INFO) << "Toggle interface started"; + ASSERT_OK(SetAdminStatus(sut_gnmi_stub_.get(), + ixia_sut_link_.sut_mirror_interface, "DOWN", + absl::Minutes(2))); + LOG(INFO) << "Toggle interface ended"; + LOG(INFO) << "Total Traffic duration: " << kTrafficDurationLowTrafficRate; + // Wait for Traffic to be sent. + absl::SleepFor(kTrafficDurationLowTrafficRate); + + // Verify GNMI queue stats match packets received. + absl::SleepFor(absl::Seconds(10)); + + // Get packet count for Mirror-To-Port. + ASSERT_OK_AND_ASSIGN(auto mirror_packets_mtp_post, + GetInterfaceCounter("out-unicast-pkts", + ixia_sut_link_.sut_mirror_interface, + sut_gnmi_stub_.get())); + + LOG(INFO) << "Mirror-To-Port packets post: " << mirror_packets_mtp_post; + // Get packet count for Mirror-To-Port. + ASSERT_OK_AND_ASSIGN( + auto mirror_packets_backup_post, + GetInterfaceCounter("out-unicast-pkts", + ixia_sut_link_.sut_mirror_backup_interface, + sut_gnmi_stub_.get())); + + LOG(INFO) << "Mirror-To-Backup-Port packets post: " + << mirror_packets_backup_post; + + int64_t total_mirrored_packets = + (mirror_packets_mtp_post - mirror_packets_mtp_pre) + + (mirror_packets_backup_post - mirror_packets_backup_mtp_pre); + LOG(INFO) << "Total packets:" << kTotalFramesTrafficLowTrafficRate; + LOG(INFO) << "Total mirrored packets: " << total_mirrored_packets; + LOG(INFO) << "Packets not mirrored: " + << kTotalFramesTrafficLowTrafficRate - total_mirrored_packets; + // Verify drop duration is within acceptable limits. + int drop_duration_ms = + (kTotalFramesTrafficLowTrafficRate - total_mirrored_packets) * + kFrameSize * 1000 / kBytesPerSecondLowTrafficRate; + LOG(INFO) << "Mirror Failover duration (ms): " << drop_duration_ms; + EXPECT_LE(drop_duration_ms, 500) << "Drop duration exceeds 500ms"; + EXPECT_OK(SetAdminStatus(sut_gnmi_stub_.get(), + ixia_sut_link_.sut_mirror_interface, "UP", + absl::Minutes(2))); +} } // namespace } // namespace pins_test diff --git a/tests/qos/punt_qos_test.h b/tests/qos/punt_qos_test.h index 07017704e..da962fc16 100644 --- a/tests/qos/punt_qos_test.h +++ b/tests/qos/punt_qos_test.h @@ -54,8 +54,12 @@ struct ParamsForTestsWithIxia { struct IxiaSutLink { std::string ixia_tx_interface; std::string ixia_rx_interface; + std::string ixia_mirror_interface; + std::string ixia_mirror_backup_interface; std::string sut_tx_interface; std::string sut_rx_interface; + std::string sut_mirror_interface; + std::string sut_mirror_backup_interface; }; class PuntQoSTestWithIxia diff --git a/tests/qos/qos_test_util.cc b/tests/qos/qos_test_util.cc index b2de8a354..21b2cc133 100644 --- a/tests/qos/qos_test_util.cc +++ b/tests/qos/qos_test_util.cc @@ -672,7 +672,7 @@ absl::StatusOr GetGnmiPortEcnCounter( ASSIGN_OR_RETURN( std::string ecn_counter_response, GetGnmiStatePathInfo(&gnmi_stub, openconfig_transmit_count_state_path, - "google-pins-interfaces:out-ecn-marked-pkts")); + "interfaces:out-ecn-marked-pkts")); int64_t ecn_counters; if (!absl::SimpleAtoi(StripQuotes(ecn_counter_response), &ecn_counters)) { @@ -714,7 +714,8 @@ absl::StatusOr GetGnmiPortIngressCounter( // the test. absl::StatusOr> ExtractQueueInfoViaGnmiConfig(absl::string_view port, - absl::string_view gnmi_config) { + absl::string_view gnmi_config, + bool is_rate_mode_in_packets) { nlohmann::json config = nlohmann::json::parse(gnmi_config); if (!config.is_object()) { return absl::InvalidArgumentError("Could not parse gnmi configuration."); @@ -744,9 +745,11 @@ ExtractQueueInfoViaGnmiConfig(absl::string_view port, .get(); queue_info[queue_name].gnmi_queue_name = queue_name; queue_info[queue_name].p4_queue_name = queue_name; - std::string peak_rate = scheduler["two-rate-three-color"]["config"] - ["google-pins-qos:pir-pkts"] - .get(); + std::string peak_rate = + scheduler["two-rate-three-color"]["config"] + [is_rate_mode_in_packets ? "qos:pir-pkts" + : "pir"] + .get(); if (!absl::SimpleAtoi( peak_rate, &queue_info[queue_name].rate_packets_per_second)) { return absl::InternalError( @@ -755,9 +758,11 @@ ExtractQueueInfoViaGnmiConfig(absl::string_view port, } LOG(INFO) << "Queue: " << queue_name << ", configured rate:" << peak_rate; - int be_pkts = scheduler["two-rate-three-color"]["config"] - ["google-pins-qos:be-pkts"] - .get(); + int be_pkts = + scheduler["two-rate-three-color"]["config"] + [is_rate_mode_in_packets ? "qos:be-pkts" + : "be"] + .get(); queue_info[queue_name].scheduler_be_pkts = be_pkts; } break; diff --git a/tests/qos/qos_test_util.h b/tests/qos/qos_test_util.h index a72943f14..31a2c7992 100644 --- a/tests/qos/qos_test_util.h +++ b/tests/qos/qos_test_util.h @@ -211,9 +211,10 @@ struct BufferParameters { // queue info. struct QueueInfoByQueueName { - std::string gnmi_queue_name; // Openconfig queue name. - std::string p4_queue_name; // P4 queue name. - int rate_packets_per_second = 0; // Rate of packets in packets per second. + std::string gnmi_queue_name; // Openconfig queue name. + std::string p4_queue_name; // P4 queue name. + int64_t rate_packets_per_second = + 0; // Rate of packets in packets per second. double shared_buffer_static_limit; // Statically configured shared buffer // limit of queue. int scheduler_be_pkts; // Burst excess packets. @@ -259,7 +260,8 @@ absl::StatusOr> GetQueuesByEgressPort( // Extract CPU queues and their bursty traffic info from the gNMI config. absl::StatusOr> ExtractQueueInfoViaGnmiConfig(absl::string_view port, - absl::string_view gnmi_config); + absl::string_view gnmi_config, + bool is_rate_mode_in_packets = true); absl::StatusOr> ExtractCPUQueuesViaGnmiConfig( absl::string_view gnmi_config); From f4099bfc3085c8a5fa5ed8e544fa74b6d68bca2a Mon Sep 17 00:00:00 2001 From: kishanps Date: Fri, 4 Oct 2024 14:48:12 -0700 Subject: [PATCH 3/7] Adding a test to meassure watchport prunning rate and Ensuring that multicast RIFs can't be deleted when referenced. --- tests/forwarding/BUILD.bazel | 2 + tests/forwarding/smoke_test.cc | 74 ++++++- tests/forwarding/watch_port_test.cc | 180 +++++++++++++++++- tests/forwarding/watch_port_test.h | 10 + tests/integration/system/nsf/BUILD.bazel | 1 + .../component_validators/syncd_validator.cc | 1 + ...rrent_config_push_flow_programming_test.cc | 9 + .../nsf/nsf_dynamic_state_update_test.cc | 13 +- .../system/nsf/traffic_helpers/otg_helper.cc | 99 +++++----- .../system/nsf/traffic_helpers/otg_helper.h | 9 + tests/packet_capture/packet_capture_test.cc | 1 + tests/qos/punt_qos_test.cc | 9 +- 12 files changed, 351 insertions(+), 57 deletions(-) diff --git a/tests/forwarding/BUILD.bazel b/tests/forwarding/BUILD.bazel index 8417e2b33..8f5f27480 100644 --- a/tests/forwarding/BUILD.bazel +++ b/tests/forwarding/BUILD.bazel @@ -69,6 +69,8 @@ cc_library( "//p4_pdpi:p4_runtime_session", "//p4_pdpi:p4_runtime_session_extras", "//p4_pdpi:pd", + "//p4_pdpi/netaddr:ipv6_address", + "//p4_pdpi/netaddr:mac_address", "//sai_p4/instantiations/google:sai_p4info_cc", "//sai_p4/instantiations/google:sai_pd_cc_proto", "//sai_p4/instantiations/google/test_tools:test_entries", diff --git a/tests/forwarding/smoke_test.cc b/tests/forwarding/smoke_test.cc index 4fcbe722d..488f1d86d 100644 --- a/tests/forwarding/smoke_test.cc +++ b/tests/forwarding/smoke_test.cc @@ -36,6 +36,8 @@ #include "p4/v1/p4runtime.pb.h" #include "p4_pdpi/ir.h" #include "p4_pdpi/ir.pb.h" +#include "p4_pdpi/netaddr/ipv6_address.h" +#include "p4_pdpi/netaddr/mac_address.h" #include "p4_pdpi/p4_runtime_session.h" #include "p4_pdpi/p4_runtime_session_extras.h" #include "p4_pdpi/pd.h" @@ -491,7 +493,7 @@ ASSERT_OK_AND_ASSIGN (pdpi:: IrP4Info ir_p4info, ASSERT_OK(pins_test::PushGnmiConfig(testbed.Sut(), sut_gnmi_config)); } -TEST_P(SmokeTestFixture, DeleteReferencedEntryNotOk) { +TEST_P(SmokeTestFixture, DeleteReferencedNeighborNotOk) { thinkit::MirrorTestbed& testbed = GetParam().mirror_testbed->GetMirrorTestbed(); @@ -555,5 +557,75 @@ ASSERT_OK_AND_ASSIGN (pdpi:: IrP4Info ir_p4info, EXPECT_OK(pdpi::InstallIrTableEntries(*sut_p4rt_session, read_entries)); } +TEST_P(SmokeTestFixture, DeleteReferencedMulticastRifNotOk) { + thinkit::MirrorTestbed& testbed = + GetParam().mirror_testbed->GetMirrorTestbed(); + + // Initialize the connection, clear table entries, and push GNMI + // configuration (if given) for the SUT and Control switch. + ASSERT_OK_AND_ASSIGN( + std::unique_ptr sut_p4rt_session, + pins_test::ConfigureSwitchAndReturnP4RuntimeSession( + testbed.Sut(), GetParam().gnmi_config, GetParam().p4info)); + + constexpr absl::string_view kVrf = "vrf"; + constexpr int kMulticastGroupId = 1; + const sai::Replica kReplica = sai::Replica{ + .egress_port = "1", + .instance = 0, + }; + + // Install a multicast route with: + // - A multicast RIF + // - A multicast group referring to that RIF. + // - An Ipv6 multicast table entry referring to that group. + // - Other auxiliary entries + ASSERT_OK(sai::EntryBuilder() + .AddVrfEntry(kVrf) + // Makes the ipv6_dst "ff05::". + .AddMulticastRoute(kVrf, netaddr::Ipv6Address(0xff05), + kMulticastGroupId) + .AddMulticastGroupEntry(kMulticastGroupId, {kReplica}) + .AddMrifEntryRewritingSrcMac( + kReplica.egress_port, kReplica.instance, + netaddr::MacAddress(0x22, 0x22, 0x22, 0x22, 0x22, 0x22)) + .InstallDedupedEntities(*sut_p4rt_session)); + + // Try to delete entities that are referenced by other entities. + pdpi::IrEntity multicast_rif_entity; + pdpi::IrEntity multicast_group_entity; + ASSERT_OK_AND_ASSIGN(const pdpi::IrEntities entities, + pdpi::ReadIrEntities(*sut_p4rt_session)); + for (const pdpi::IrEntity& entity : entities.entities()) { + if (entity.has_packet_replication_engine_entry()) { + multicast_group_entity = entity; + } else if (entity.has_table_entry() && + entity.table_entry().table_name() == + "multicast_router_interface_table") { + multicast_rif_entity = entity; + } + } + ASSERT_TRUE(multicast_rif_entity.has_table_entry()) + << "Could not find multicast RIF on switch after successfully installing " + "it."; + ASSERT_TRUE(multicast_group_entity.has_packet_replication_engine_entry()) + << "Could not find multicast group on switch after successfully " + "installing it."; + + // Cannot delete the multicast RIF because it is used by the multicast group. + EXPECT_THAT(pdpi::DeleteIrEntity(*sut_p4rt_session, multicast_rif_entity), + Not(IsOk())); + // Cannot delete the multicast group because it is used by the IPv6 route. + EXPECT_THAT(pdpi::DeleteIrEntity(*sut_p4rt_session, multicast_group_entity), + Not(IsOk())); + + // We should always be able to delete and re-install entities read from the + // switch. Otherwise, the switch is in a corrupted state. + ASSERT_OK_AND_ASSIGN(const pdpi::IrEntities read_entities, + pdpi::ReadIrEntities(*sut_p4rt_session)); + ASSERT_OK(pdpi::ClearEntities(*sut_p4rt_session)); + EXPECT_OK(pdpi::InstallIrEntities(*sut_p4rt_session, read_entities)); +} + } // namespace } // namespace pins diff --git a/tests/forwarding/watch_port_test.cc b/tests/forwarding/watch_port_test.cc index c0078ca95..a9272101b 100644 --- a/tests/forwarding/watch_port_test.cc +++ b/tests/forwarding/watch_port_test.cc @@ -14,11 +14,12 @@ #include "tests/forwarding/watch_port_test.h" +#include #include #include #include +#include // NOLINT(build/c++11) #include -#include #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" @@ -111,6 +112,12 @@ constexpr int kNumWcmpMembersForTest = 5; // Number of packets used in the test. constexpr int kNumTestPackets = 5000; +// Number of packets used in the test for watch port pruning. +constexpr int kNumTestPacketsForWatchPortPruning = 12000; + +// Rate of packets in packets per seconds. +constexpr int kPacketRateInSecondsForPruning = 1200; + // Default input port index of the group members vector, on which packets // arrive. constexpr int kDefaultInputPortIndex = 0; @@ -289,12 +296,14 @@ absl::Status SendNPacketsToSut(int num_packets, absl::Span port_ids, const pdpi::IrP4Info& ir_p4info, pdpi::P4RuntimeSession& p4_session, - thinkit::TestEnvironment& test_environment) { + thinkit::TestEnvironment& test_environment, + int packets_rate = 500) { const absl::Time start_time = absl::Now(); int packet_index = 0; for (int i = 0; i < num_packets; i++) { // Rate limit to 500 packets per second. - auto earliest_send_time = start_time + (i * absl::Seconds(1) / 500); + auto earliest_send_time = + start_time + (i * absl::Seconds(1) / packets_rate); absl::SleepFor(earliest_send_time - absl::Now()); // Vary the port on which to send the packet if the hash field selected is @@ -617,6 +626,171 @@ void WatchPortTestFixture::TearDown() { namespace { +// Measure watchport pruning duration by sending packets to a port and then +// setting the port to down/up, and then measure the number of packets received +// by the SUT to calculate the watch port pruning rate. +TEST_P(WatchPortTestFixture, MeasureWatchPortPruningDuration) { + thinkit::TestEnvironment& environment = + GetParam().testbed->GetMirrorTestbed().Environment(); + if (!GetParam().check_and_export_duration.has_value()) { + GTEST_SKIP() << "Watchport pruning duration test skipped because threshold" + "and export vector is not defined"; + } + + ASSERT_OK_AND_ASSIGN( + std::vector controller_port_ids, + pins_test::GetMatchingP4rtPortIds(*sut_gnmi_stub_, + pins_test::IsEnabledEthernetInterface)); + const int group_size = kNumWcmpMembersForTest; + ASSERT_OK_AND_ASSIGN(std::vector members, + CreateGroupMembers(group_size, controller_port_ids)); + ASSERT_OK_AND_ASSIGN(const pdpi::IrP4Info ir_p4info, + pdpi::CreateIrP4Info(GetParam().p4_info)); + + // Programs the required router interfaces, nexthops for wcmp group. + ASSERT_OK(pins::ProgramNextHops(environment, *sut_p4_session_, ir_p4info, + members)); + 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)); + + // Generate test configuration, pick any field used by hashing to vary for + // every packet so that it gets sent to all the members. + TestConfiguration test_config = { + .field = PacketField::kIpDst, + .ipv4 = true, + .encapped = false, + .inner_ipv4 = false, + .decap = false, + }; + ASSERT_TRUE(IsValidTestConfiguration(test_config)); + + // Create test data entry. + std::string test_config_key = TestConfigurationToPayload(test_config); + { + absl::MutexLock lock(&test_data_.mutex); + test_data_.input_output_per_packet[test_config_key] = TestInputOutput{ + .config = test_config, + }; + } + + // Select one random member of the group to toggle. + absl::BitGen gen; + const int random_member_index = + absl::Uniform(absl::IntervalClosedOpen, gen, 0, members.size()); + const int selected_port_id = members[random_member_index].port; + // Get port_name to port id mapping for the control switch. + ASSERT_OK_AND_ASSIGN(const auto port_name_per_port_id, + GetPortNamePerPortId(*control_gnmi_stub_)); + // Get port_name for the selected port. + ASSERT_OK_AND_ASSIGN(const auto& port_name, + gutil::FindOrStatus(port_name_per_port_id, + absl::StrCat(selected_port_id))); + InterfaceState current_port_state = InterfaceState::kUp; + + int64_t total_packets_sent; + int64_t total_packets_received; + int64_t total_packets_lost; + double watchport_pruning_duration; + absl::flat_hash_map + port_state_to_pruning_duration; + + for (auto port_desired_state : {InterfaceState::kDown, InterfaceState::kUp}) { + absl::string_view port_final_state = + port_desired_state == InterfaceState::kDown ? "DOWN" : "UP"; + + // Verify the oper status is reflected on the SUT. + ASSERT_OK(VerifyInterfaceOperState(*sut_gnmi_stub_, + GetParam().testbed->GetMirrorTestbed(), + port_name, current_port_state)); + + // Start a new thread to send packets to the SUT. This is to ensure that + // the packets are being sent to the SUT while the port changes state. + std::thread send_packets_thread([&]() { + ASSERT_OK(SendNPacketsToSut(kNumTestPacketsForWatchPortPruning, + test_config, members, controller_port_ids, + ir_p4info, *control_p4_session_, environment, + kPacketRateInSecondsForPruning)); + }); + + double delay_before_watchport_pruning = + (kNumTestPacketsForWatchPortPruning / kPacketRateInSecondsForPruning) * + 0.25; + LOG(INFO) << "Waiting for " << delay_before_watchport_pruning + << " seconds to change the port state to: " << port_final_state; + absl::SleepFor(absl::Seconds(delay_before_watchport_pruning)); + // Set the selected port to new state. + ASSERT_OK(SetInterfaceAdminState(*control_gnmi_stub_, port_name, + port_desired_state)); + // Verify the oper status is reflected on the SUT. + LOG(INFO) << "Setting port " << port_name + << " to state: " << port_final_state; + current_port_state = port_desired_state; + ASSERT_OK(VerifyInterfaceOperState(*sut_gnmi_stub_, + GetParam().testbed->GetMirrorTestbed(), + port_name, current_port_state)); + // Join the thread to ensure that all packets are sent to the SUT after the + // port is changed to new state. + send_packets_thread.join(); + + test_data_.ClearReceivedPackets(); + test_data_.total_packets_sent = kNumTestPacketsForWatchPortPruning; + // Wait for packets from the SUT to arrive. Since some packets are expected + // to be lost due to the port being in new state, we don't verify the number + // of packets received. + absl::Status status = control_p4_session_->HandleNextNStreamMessages( + [&](const p4::v1::StreamMessageResponse& message) { + return HandleStreamMessage(ir_p4info, + &GetParam().testbed->GetMirrorTestbed(), + message, &test_data_); + }, + kNumTestPacketsForWatchPortPruning, kDurationToWaitForPackets); + if (!status.ok() && !absl::IsDeadlineExceeded(status)) { + FAIL() << "Failed to receive packets from SUT: " << status; + } + + { + absl::MutexLock lock(&test_data_.mutex); + TestInputOutput& test = + test_data_.input_output_per_packet[test_config_key]; + + total_packets_sent = test_data_.total_packets_sent; + total_packets_received = test.output.size(); + total_packets_lost = total_packets_sent - total_packets_received; + watchport_pruning_duration = + 1000 * total_packets_lost / kPacketRateInSecondsForPruning; + } + + LOG(INFO) << "Watchport packet rate(pps): " + << kPacketRateInSecondsForPruning << "\n" + << "Total Packets sent: " << total_packets_sent << "\n" + << "Total Packets received: " << total_packets_received << "\n" + << "Total Packets lost: " << total_packets_lost << "\n" + << "Watchport pruning duration: " << watchport_pruning_duration + << " msecs."; + port_state_to_pruning_duration[port_final_state] = + absl::Milliseconds(watchport_pruning_duration); + } + LOG(INFO) << "Checking watch port pruning (Port Up/Down) duration against " + "threshold and exporting data to perfgate storage"; + ASSERT_OK((*GetParam().check_and_export_duration)( + GetParam().testbed->GetMirrorTestbed().Sut().ChassisName(), + port_state_to_pruning_duration)); +} + // Verifies basic WCMP behavior by programming a group with multiple members // with random weights and ensuring that all members receive some part of // the sent traffic. diff --git a/tests/forwarding/watch_port_test.h b/tests/forwarding/watch_port_test.h index 60b27f23b..709baabf0 100644 --- a/tests/forwarding/watch_port_test.h +++ b/tests/forwarding/watch_port_test.h @@ -20,7 +20,10 @@ #include #include +#include "absl/container/flat_hash_map.h" #include "absl/status/status.h" +#include "absl/strings/string_view.h" +#include "absl/time/time.h" #include "gtest/gtest.h" #include "lib/gnmi/openconfig.pb.h" #include "p4/config/v1/p4info.pb.h" @@ -46,6 +49,13 @@ struct WatchPortTestParams { // critical state related tests will be skipped. absl::optional> set_critical_alarm; + // Optional function that checks the watchport pruning duration against the + // threshold and exports the data to perfgate. + absl::optional& + port_state_to_pruning_duration)>> + check_and_export_duration; }; // WatchPortTestFixture for testing watch port action. diff --git a/tests/integration/system/nsf/BUILD.bazel b/tests/integration/system/nsf/BUILD.bazel index 996322e53..a861d4a32 100644 --- a/tests/integration/system/nsf/BUILD.bazel +++ b/tests/integration/system/nsf/BUILD.bazel @@ -97,6 +97,7 @@ cc_library( ":compare_p4flows", ":milestone", ":util", + "//gutil:proto", "//gutil:status_matchers", "//lib/gnmi:gnmi_helper", "//tests/integration/system/nsf/interfaces:component_validator", diff --git a/tests/integration/system/nsf/component_validators/syncd_validator.cc b/tests/integration/system/nsf/component_validators/syncd_validator.cc index 50a7cd011..ac9dca2c8 100644 --- a/tests/integration/system/nsf/component_validators/syncd_validator.cc +++ b/tests/integration/system/nsf/component_validators/syncd_validator.cc @@ -20,6 +20,7 @@ #include "absl/strings/match.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" #include "absl/strings/string_view.h" #include "glog/logging.h" #include "tests/integration/system/nsf/interfaces/testbed.h" diff --git a/tests/integration/system/nsf/nsf_concurrent_config_push_flow_programming_test.cc b/tests/integration/system/nsf/nsf_concurrent_config_push_flow_programming_test.cc index d3184cdf4..82211b4d4 100644 --- a/tests/integration/system/nsf/nsf_concurrent_config_push_flow_programming_test.cc +++ b/tests/integration/system/nsf/nsf_concurrent_config_push_flow_programming_test.cc @@ -67,6 +67,15 @@ void NsfConcurrentConfigPushFlowProgrammingTestFixture::SetUp() { ASSERT_OK_AND_ASSIGN(testbed_, GetTestbed(testbed_interface_)); } void NsfConcurrentConfigPushFlowProgrammingTestFixture::TearDown() { + if (HasFailure()) { + thinkit::Switch& sut = GetSut(testbed_); + LOG(INFO) << "Performing cold reboot to recover the switch."; + thinkit::TestEnvironment& env = GetTestEnvironment(testbed_); + ASSERT_OK( + pins_test::Reboot(gnoi::system::RebootMethod::COLD, sut, env, + /*collect_debug_artifacts_before_reboot=*/false)); + ASSERT_OK(WaitForReboot(testbed_, *ssh_client_)); + } if (!GetParam().image_config_params.empty()) { LOG(INFO) << "Restoring the original config"; ASSERT_OK(PushConfig(GetParam().image_config_params[0], GetSut(testbed_), diff --git a/tests/integration/system/nsf/nsf_dynamic_state_update_test.cc b/tests/integration/system/nsf/nsf_dynamic_state_update_test.cc index 4b456b6c8..ec8238ae1 100644 --- a/tests/integration/system/nsf/nsf_dynamic_state_update_test.cc +++ b/tests/integration/system/nsf/nsf_dynamic_state_update_test.cc @@ -85,17 +85,20 @@ TEST_P(NsfDynamicStateUpdateTestFixture, NsfDynamicStateUpdateTest) { ssh_client)); EXPECT_OK( SetAdminStatus(control_switch_gnmi_stub.get(), intf_to_check, "DOWN")); - if (!WaitForNsfReboot(&mirror_testbed, ssh_client, - /*image_config_param=*/nullptr, - /*check_interfaces_up =*/false) - .ok()) { + + absl::Status reboot_status = WaitForNsfReboot(&mirror_testbed, ssh_client, + /*image_config_param=*/nullptr, + /*check_interfaces_up =*/false); + if (!reboot_status.ok()) { // Cold reboot the testbed as the failed NSF reboot could leave the testbed // in unhealthy state - LOG(INFO) << "NSF reboot failed. Cold rebooting the switch."; + LOG(INFO) << "NSF reboot failed. " << reboot_status.message() + << "Cold rebooting the switch."; EXPECT_OK(Reboot(RebootMethod::COLD, sut, mirror_testbed.Environment())); EXPECT_OK(WaitForReboot(&mirror_testbed, ssh_client, false)); FAIL() << "Failure in NSF reboot."; } + EXPECT_THAT(GetInterfaceOperStatusOverGnmi(*control_switch_gnmi_stub.get(), intf_to_check), gutil::IsOkAndHolds(OperStatus::kDown)); diff --git a/tests/integration/system/nsf/traffic_helpers/otg_helper.cc b/tests/integration/system/nsf/traffic_helpers/otg_helper.cc index a7299180f..b22885d1c 100644 --- a/tests/integration/system/nsf/traffic_helpers/otg_helper.cc +++ b/tests/integration/system/nsf/traffic_helpers/otg_helper.cc @@ -21,6 +21,7 @@ #include #include "absl/algorithm/container.h" +#include "absl/log/log.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_join.h" @@ -29,7 +30,6 @@ #include "absl/time/time.h" #include "artifacts/otg.grpc.pb.h" #include "artifacts/otg.pb.h" -#include "glog/logging.h" #include "grpcpp/client_context.h" #include "gutil/status.h" #include "lib/utils/generic_testbed_utils.h" @@ -92,6 +92,53 @@ TrafficMetrics GetTrafficMetrics(const otg::FlowMetric& flow_metric, } // namespace +void OtgHelper::ConfigureFlowBaseSettings(::otg::Flow* flow, + absl::string_view src_port, + absl::string_view dst_port) { + // Set Tx / Rx ports. + flow->mutable_tx_rx()->set_choice(otg::FlowTxRx::Choice::port); + flow->mutable_tx_rx()->mutable_port()->set_tx_name(src_port); + flow->mutable_tx_rx()->mutable_port()->set_rx_name(dst_port); + + // Set packet size. + flow->mutable_size()->set_choice(otg::FlowSize::Choice::fixed); + flow->mutable_size()->set_fixed(kDefaultPacketSize); + + // Set transmission duration. + flow->mutable_duration()->set_choice(otg::FlowDuration::Choice::continuous); + + // Set transmission rate. + if (enable_linerate_) { + flow->mutable_rate()->set_choice(otg::FlowRate::Choice::percentage); + flow->mutable_rate()->set_percentage(kFlowRateAtLinerate / 2); + } else { + flow->mutable_rate()->set_choice(otg::FlowRate::Choice::pps); + flow->mutable_rate()->set_pps(kDefaultPacketsPerSecond); + } + + // Set capture metrics. + flow->mutable_metrics()->set_enable(true); + flow->mutable_metrics()->set_loss(false); + flow->mutable_metrics()->set_timestamps(true); + flow->mutable_metrics()->mutable_latency()->set_enable(true); + flow->mutable_metrics()->mutable_latency()->set_mode( + otg::FlowLatencyMetrics::Mode::cut_through); +} + +void OtgHelper::ConfigureFlowEthernetHeader(::otg::Flow* flow, + absl::string_view src_mac, + absl::string_view dst_mac) { + // Set ethernet header. + auto* eth_packet = flow->add_packet(); + eth_packet->set_choice(otg::FlowHeader::Choice::ethernet); + eth_packet->mutable_ethernet()->mutable_src()->set_choice( + otg::PatternFlowEthernetSrc::Choice::value); + eth_packet->mutable_ethernet()->mutable_dst()->set_choice( + otg::PatternFlowEthernetDst::Choice::value); + eth_packet->mutable_ethernet()->mutable_src()->set_value(src_mac); + eth_packet->mutable_ethernet()->mutable_dst()->set_value(dst_mac); +} + absl::Status OtgHelper::StartTraffic(const Testbed &testbed, absl::string_view config_label) { ASSIGN_OR_RETURN(thinkit::GenericTestbed * generic_testbed, @@ -153,51 +200,15 @@ absl::Status OtgHelper::StartTraffic(const Testbed &testbed, // Set MTU. layer1->set_mtu(kDefaultMtu); - // Create flow. - auto *flow = config->add_flows(); - flow->set_name(absl::StrCat(otg_src_port, "->", otg_dst_port)); + // Create TCP flow. + auto* tcp_flow = config->add_flows(); + tcp_flow->set_name(absl::StrCat(otg_src_port, "->", otg_dst_port, "_TCP")); - // Set Tx / Rx ports. - flow->mutable_tx_rx()->set_choice(otg::FlowTxRx::Choice::port); - flow->mutable_tx_rx()->mutable_port()->set_tx_name(otg_src_port); - flow->mutable_tx_rx()->mutable_port()->set_rx_name(otg_dst_port); - - // Set packet size. - flow->mutable_size()->set_choice(otg::FlowSize::Choice::fixed); - flow->mutable_size()->set_fixed(kDefaultPacketSize); - - // Set transmission duration. - flow->mutable_duration()->set_choice(otg::FlowDuration::Choice::continuous); - - // Set transmission rate. - if (enable_linerate_) { - flow->mutable_rate()->set_choice(otg::FlowRate::Choice::percentage); - flow->mutable_rate()->set_percentage(kFlowRateAtLinerate); - } else { - flow->mutable_rate()->set_choice(otg::FlowRate::Choice::pps); - flow->mutable_rate()->set_pps(kDefaultPacketsPerSecond); - } - - // Set capture metrics. - flow->mutable_metrics()->set_enable(true); - flow->mutable_metrics()->set_loss(false); - flow->mutable_metrics()->set_timestamps(true); - flow->mutable_metrics()->mutable_latency()->set_enable(true); - flow->mutable_metrics()->mutable_latency()->set_mode( - otg::FlowLatencyMetrics::Mode::cut_through); - - // Set ethernet header. - auto *eth_packet = flow->add_packet(); - eth_packet->set_choice(otg::FlowHeader::Choice::ethernet); - eth_packet->mutable_ethernet()->mutable_src()->set_choice( - otg::PatternFlowEthernetSrc::Choice::value); - eth_packet->mutable_ethernet()->mutable_dst()->set_choice( - otg::PatternFlowEthernetDst::Choice::value); - eth_packet->mutable_ethernet()->mutable_src()->set_value(otg_src_mac); - eth_packet->mutable_ethernet()->mutable_dst()->set_value(otg_dst_mac); + ConfigureFlowBaseSettings(tcp_flow, otg_src_port, otg_dst_port); + ConfigureFlowEthernetHeader(tcp_flow, otg_src_mac, otg_dst_mac); // Set IP header. - auto *ip_packet = flow->add_packet(); + auto* ip_packet = tcp_flow->add_packet(); ip_packet->set_choice(otg::FlowHeader::Choice::ipv6); ip_packet->mutable_ipv6()->mutable_src()->set_choice( otg::PatternFlowIpv6Src::Choice::value); @@ -207,7 +218,7 @@ absl::Status OtgHelper::StartTraffic(const Testbed &testbed, ip_packet->mutable_ipv6()->mutable_dst()->set_value(otg_dst_ip); // Set TCP header. - auto *tcp_packet = flow->add_packet(); + auto* tcp_packet = tcp_flow->add_packet(); tcp_packet->set_choice(otg::FlowHeader::Choice::tcp); auto *tcp_dst_port = tcp_packet->mutable_tcp()->mutable_dst_port(); tcp_dst_port->set_choice(otg::PatternFlowTcpDstPort::Choice::increment); diff --git a/tests/integration/system/nsf/traffic_helpers/otg_helper.h b/tests/integration/system/nsf/traffic_helpers/otg_helper.h index b1c09ea59..0cd5ca1a7 100644 --- a/tests/integration/system/nsf/traffic_helpers/otg_helper.h +++ b/tests/integration/system/nsf/traffic_helpers/otg_helper.h @@ -26,6 +26,15 @@ namespace pins_test { class OtgHelper : public TrafficHelper { public: OtgHelper(bool enable_linerate = false) : enable_linerate_(enable_linerate) {} + // Configures the base settings for the flow: + // - Packet size + // - Transmission duration + // - Transmission rate + // - Capture metrics + void ConfigureFlowBaseSettings(::otg::Flow* flow, absl::string_view src_port, + absl::string_view dst_port); + void ConfigureFlowEthernetHeader(::otg::Flow* flow, absl::string_view src_mac, + absl::string_view dst_mac); absl::Status StartTraffic(const Testbed &testbed, absl::string_view config_label) override; absl::Status StopTraffic(const Testbed &testbed) override; diff --git a/tests/packet_capture/packet_capture_test.cc b/tests/packet_capture/packet_capture_test.cc index 1bad3ebc4..99a2bf78e 100644 --- a/tests/packet_capture/packet_capture_test.cc +++ b/tests/packet_capture/packet_capture_test.cc @@ -160,6 +160,7 @@ TEST_P(PacketCaptureTestWithoutIxia, PsampEncapsulatedMirroringTest) { auto mirror_session_params = sai::MirrorSessionParams{ .mirror_session_id = "psamp_mirror", .monitor_port = kSutEgressPortP4rtId, + .monitor_backup_port = kSutEgressPortP4rtId, .mirror_encap_src_mac = "00:00:00:22:22:22", .mirror_encap_dst_mac = "00:00:00:44:44:44", .mirror_encap_vlan_id = "0x0fe", diff --git a/tests/qos/punt_qos_test.cc b/tests/qos/punt_qos_test.cc index 320cf81e5..70cc3141c 100644 --- a/tests/qos/punt_qos_test.cc +++ b/tests/qos/punt_qos_test.cc @@ -521,6 +521,8 @@ TEST_P(PuntQoSTestWithIxia, SetDscpAndQueuesAndDenyAboveRateLimit) { LOG(INFO) << "Mirror-To-Port packets post: " << mirror_packets_post; + absl::SleepFor(kMaxQueueCounterUpdateTime); + ASSERT_OK_AND_ASSIGN( QueueCounters final_mirror_green_queue_counters, GetGnmiQueueCounters(ixia_sut_link_.sut_mirror_interface, @@ -584,8 +586,7 @@ TEST_P(PuntQoSTestWithIxia, SetDscpAndQueuesAndDenyAboveRateLimit) { receiver.Destroy(); } -// TODO: Disabled till failover is fixed. -TEST_P(PuntQoSTestWithIxia, DISABLED_MirrorFailover) { +TEST_P(PuntQoSTestWithIxia, MirrorFailover) { // Flow details. const auto dest_mac = netaddr::MacAddress(02, 02, 02, 02, 02, 02); const auto source_mac = netaddr::MacAddress(00, 01, 02, 03, 04, 05); @@ -680,7 +681,7 @@ TEST_P(PuntQoSTestWithIxia, DISABLED_MirrorFailover) { LOG(INFO) << "Toggle interface started"; ASSERT_OK(SetAdminStatus(sut_gnmi_stub_.get(), ixia_sut_link_.sut_mirror_interface, "DOWN", - absl::Minutes(2))); + absl::Seconds(0))); LOG(INFO) << "Toggle interface ended"; LOG(INFO) << "Total Traffic duration: " << kTrafficDurationLowTrafficRate; // Wait for Traffic to be sent. @@ -721,7 +722,7 @@ TEST_P(PuntQoSTestWithIxia, DISABLED_MirrorFailover) { EXPECT_LE(drop_duration_ms, 500) << "Drop duration exceeds 500ms"; EXPECT_OK(SetAdminStatus(sut_gnmi_stub_.get(), ixia_sut_link_.sut_mirror_interface, "UP", - absl::Minutes(2))); + absl::Seconds(0))); } } // namespace } // namespace pins_test From 14801a7a7c4fcb9351e92d6e0198b398a3044f83 Mon Sep 17 00:00:00 2001 From: kishanps Date: Thu, 24 Oct 2024 17:25:15 -0700 Subject: [PATCH 4/7] Add VLAN Support to Multicast test --- tests/forwarding/l3_multicast_test.cc | 1017 ++++++++++++++++++------- tests/qos/qos_test_util.cc | 2 +- tests/qos/qos_test_util.h | 3 +- 3 files changed, 764 insertions(+), 258 deletions(-) diff --git a/tests/forwarding/l3_multicast_test.cc b/tests/forwarding/l3_multicast_test.cc index 5f26f1e42..8393a8695 100644 --- a/tests/forwarding/l3_multicast_test.cc +++ b/tests/forwarding/l3_multicast_test.cc @@ -57,6 +57,7 @@ #include "sai_p4/instantiations/google/test_tools/test_entries.h" #include "tests/lib/p4info_helper.h" #include "tests/lib/switch_test_setup_helpers.h" +#include "thinkit/mirror_testbed.h" #include "thinkit/switch.h" #include "util/gtl/value_or_die.h" @@ -86,6 +87,32 @@ struct ReplicaPair { enum class IpmcGroupAssignmentMechanism { kAclRedirect, kIpMulticastTable }; +struct VlanForwardingParams { + bool disable_ingress_vlan_checks = false; + bool disable_egress_vlan_checks = false; + // input_vlan_id indicates vlan membership of the ingress port + std::optional input_vlan_id = std::nullopt; + // input_vlan_tagging_mode indicates mode of vlan membership of the ingress + // port + std::optional input_vlan_tagging_mode = std::nullopt; + // out_ports_vlan_id indicates vlan membership of the output ports + std::optional out_ports_vlan_id = std::nullopt; + // out_ports_vlan_tagging_mode indicates mode of vlan membership of the output + // ports + std::optional out_ports_vlan_tagging_mode = + std::nullopt; +}; + +struct MulticastForwardingParams { + // SUT port IDs used as ingress and egress ports for multicast replicas. + std::vector sut_port_ids; + netaddr::MacAddress next_hop_dst_mac; + IpmcGroupAssignmentMechanism assignment_mechanism; + int number_replicas_per_group; + int number_multicast_groups; + VlanForwardingParams vlan_forwarding_params; +}; + // Multicast IPv4 addresses of the form 226.10.#.#. The last two bytes // are computed based on the multicast group ID. absl::StatusOr GetIpv4AddressForReplica( @@ -164,16 +191,44 @@ absl::StatusOr> GetNUpInterfaceIDs( return result; } +struct MrifTableEntryParams { + std::string egress_port; + // multicast replica instance + int instance; + netaddr::MacAddress src_mac; + std::optional next_hop_dst_mac = std::nullopt; + std::optional preserve_ingress_vlan_id = std::nullopt; + std::optional egress_vlan_id = std::nullopt; +}; + // Add table entries for multicast_router_interface_table. absl::StatusOr> CreateRifTableEntities( - const pdpi::IrP4Info& ir_p4info, const std::string& port_id, - const int instance, const netaddr::MacAddress& src_mac) { - ASSIGN_OR_RETURN(std::vector entities, - sai::EntryBuilder() - .AddMrifEntryRewritingSrcMac(port_id, instance, src_mac) - .LogPdEntries() - .GetDedupedPiEntities(ir_p4info, - /*allow_unsupported=*/true)); + const pdpi::IrP4Info& ir_p4info, const MrifTableEntryParams& params) { + std::vector entities; + sai::EntryBuilder entry_builder; + if (params.egress_vlan_id.has_value()) { + if (params.preserve_ingress_vlan_id.has_value() && + (params.preserve_ingress_vlan_id.value() == + params.egress_vlan_id.value())) { + entry_builder.AddMrifEntryRewritingSrcMacAndPreservingIngressVlanId( + params.egress_port, params.instance, params.src_mac); + } else { + if (params.next_hop_dst_mac.has_value()) { + entry_builder.AddMrifEntryRewritingSrcMacDstMacAndVlanId( + params.egress_port, params.instance, params.src_mac, + params.next_hop_dst_mac.value(), params.egress_vlan_id.value()); + } else { + entry_builder.AddMrifEntryRewritingSrcMacAndVlanId( + params.egress_port, params.instance, params.src_mac, + params.egress_vlan_id.value()); + } + } + } else { + entry_builder.AddMrifEntryRewritingSrcMac(params.egress_port, + params.instance, params.src_mac); + } + ASSIGN_OR_RETURN( + entities, entry_builder.LogPdEntries().GetDedupedPiEntities(ir_p4info)); return entities; } @@ -251,18 +306,56 @@ absl::Status ClearEntities(pdpi::P4RuntimeSession& session, return absl::OkStatus(); } +std::string GetVlanIdHexStr(int val) { + return absl::StrCat("0x", absl::Hex(val, absl::kZeroPad3)); +} + +absl::Status InstallVlanMembershipEntries( + pdpi::P4RuntimeSession& switch_session, pdpi::IrP4Info& ir_p4info, + const std::vector& sut_port_ids, + const VlanForwardingParams& test_params) { + sai::EntryBuilder entry_builder; + + // ingress/egress disable checks do it only for the first multicast group + // for scale setups + if (test_params.disable_ingress_vlan_checks) { + entry_builder.AddDisableIngressVlanChecksEntry(); + } + if (test_params.disable_egress_vlan_checks) { + entry_builder.AddDisableEgressVlanChecksEntry(); + } + + if (test_params.input_vlan_id.has_value()) { + entry_builder.AddVlanEntry( + GetVlanIdHexStr(test_params.input_vlan_id.value())); + entry_builder.AddVlanMembershipEntry( + GetVlanIdHexStr(test_params.input_vlan_id.value()), sut_port_ids[0], + test_params.input_vlan_tagging_mode.value()); + } + + if (test_params.out_ports_vlan_id.has_value()) { + entry_builder.AddVlanEntry( + GetVlanIdHexStr(test_params.out_ports_vlan_id.value())); + for (int r = 1; r < sut_port_ids.size(); ++r) { + entry_builder.AddVlanMembershipEntry( + GetVlanIdHexStr(test_params.out_ports_vlan_id.value()), + sut_port_ids[r], test_params.out_ports_vlan_tagging_mode.value()); + } + } + + return entry_builder.LogPdEntries().InstallDedupedEntities(switch_session); +} + // Setup multicast and other related tables for forwarding multicast packets. absl::Status SetupDefaultMulticastProgramming( pdpi::P4RuntimeSession& session, const pdpi::IrP4Info& ir_p4info, - const p4::v1::Update_Type& update_type, int number_multicast_groups, - int replicas_per_group, const std::vector& port_ids, - IpmcGroupAssignmentMechanism assignment_mechanism, + const p4::v1::Update_Type& update_type, MulticastForwardingParams params, std::vector& entities_created) { - if (port_ids.size() < replicas_per_group) { + if (params.sut_port_ids.size() < params.number_replicas_per_group) { return gutil::InternalErrorBuilder() << "Not enough port IDs provided to setup multicast programming:" - << " expected: " << replicas_per_group - << " received: " << port_ids.size(); + << " expected: " << params.number_replicas_per_group + << " received: " << params.sut_port_ids.size(); } // Setup admission for all L3 packets, a default VRF, @@ -281,15 +374,25 @@ absl::Status SetupDefaultMulticastProgramming( acl_entities.end()); // Setup multicast RIF table. std::vector rif_entities; - for (int m = 0; m < number_multicast_groups; ++m) { - for (int r = 0; r < replicas_per_group; ++r) { - const std::string& port_id = port_ids.at(r + 1); + for (int m = 0; m < params.number_multicast_groups; ++m) { + for (int r = 0; r < params.number_replicas_per_group; ++r) { + const std::string& port_id = params.sut_port_ids[r + 1]; // Unique Ether src mac base address. - ASSIGN_OR_RETURN(netaddr::MacAddress src_mac, - GetSrcMacForReplica(m, replicas_per_group, r)); + ASSIGN_OR_RETURN( + netaddr::MacAddress src_mac, + GetSrcMacForReplica(m, params.number_replicas_per_group, r)); + int instance = params.number_replicas_per_group * m + r; + const MrifTableEntryParams rif_params = { + .egress_port = port_id, + .instance = instance, + .src_mac = src_mac, + .next_hop_dst_mac = params.next_hop_dst_mac, + .preserve_ingress_vlan_id = + params.vlan_forwarding_params.input_vlan_id.has_value() ? true + : false, + .egress_vlan_id = params.vlan_forwarding_params.out_ports_vlan_id}; ASSIGN_OR_RETURN(auto rifs, - CreateRifTableEntities(ir_p4info, port_id, - kDefaultInstance, src_mac)); + CreateRifTableEntities(ir_p4info, rif_params)); rif_entities.insert(rif_entities.end(), rifs.begin(), rifs.end()); } } @@ -299,11 +402,12 @@ absl::Status SetupDefaultMulticastProgramming( // Setup multicast groups and group members. std::vector mc_entities; - for (int m = 0; m < number_multicast_groups; ++m) { + for (int m = 0; m < params.number_multicast_groups; ++m) { std::vector replicas; - for (int r = 0; r < replicas_per_group; ++r) { - const std::string& port_id = port_ids.at(r + 1); - replicas.push_back({port_id, kDefaultInstance}); + for (int r = 0; r < params.number_replicas_per_group; ++r) { + const std::string& port_id = params.sut_port_ids[r + 1]; + int instance = params.number_replicas_per_group * m + r; + replicas.push_back({port_id, instance}); } // Note: multicast group ID 0 is not valid. int multicast_group_id = m + 1; @@ -315,11 +419,12 @@ absl::Status SetupDefaultMulticastProgramming( entities_created.insert(entities_created.end(), mc_entities.begin(), mc_entities.end()); - if (assignment_mechanism == IpmcGroupAssignmentMechanism::kAclRedirect) { + if (params.assignment_mechanism == + IpmcGroupAssignmentMechanism::kAclRedirect) { // Setup multicast group assignment (ACL redirect). // In the default traffic test setup, we only send traffic on one port // (port_index 0), so we only need to add one ACL entry. - const std::string& port_id = port_ids[0]; + const std::string& port_id = params.sut_port_ids[0]; ASSIGN_OR_RETURN(std::vector acl_entities, sai::EntryBuilder() .AddIngressAclEntryRedirectingToMulticastGroup( @@ -335,7 +440,7 @@ absl::Status SetupDefaultMulticastProgramming( // Setup multicast group assignment (IPMC entries). std::vector ipmc_entities; - for (int m = 0; m < number_multicast_groups; ++m) { + for (int m = 0; m < params.number_multicast_groups; ++m) { ASSIGN_OR_RETURN(const netaddr::Ipv4Address ipv4_address, GetIpv4AddressForReplica(m)); uint8_t multicast_group_id = m + 1; @@ -359,99 +464,186 @@ absl::Status SetupDefaultMulticastProgramming( return absl::OkStatus(); } -// Build test packets that match the multicast table entries +struct TestVectorParams { + // sut_port_ids is the list of ports that are used for multicast replication. + // The first port is the ingress port and the rest are the egress ports. + std::vector sut_port_ids; + // The index of the multicast group that the input packets are expected to + // match. + int input_group_index; + // The index of the multicast group that the output packets are expected to + // match. + int output_group_index; + int replicas_per_group; + int output_replica_start_index = 0; + // input_vlan_id indicates vlan membership of the ingress port + std::optional input_vlan_id = std::nullopt; + // out_ports_vlan_id indicates vlan membership of the output ports + std::optional out_ports_vlan_id = std::nullopt; +}; + +// Build a test vector that injects one IPv4 and one IPv6 test packet. The +// input packets are formatted such that they are expected to match the +// multicast group specified by the `input_group_index`. The multicast group +// is expected to be active, and the output packets are expected to be formatted +// according to the multicast group specified by the `output_group_index`. absl::StatusOr> BuildTestVectors( - const std::vector& port_ids, int number_multicast_groups, - int replicas_per_group, bool expect_output_packets = true) { + const TestVectorParams& params) { // Test packets injected and expected results. std::vector expectations; // All packets will be injected on the same port. - const std::string& in_port = port_ids.at(0); - int total_packets = 0; - int unique_payload_ids = 1; - for (int m = 0; m < number_multicast_groups; ++m) { - ASSIGN_OR_RETURN(const auto ipv4_address, GetIpv4AddressForReplica(m)); - ASSIGN_OR_RETURN(const auto ipv6_address, GetIpv6AddressForReplica(m)); - // Construct the input packets. - // We will inject 2 packets to touch each multicast group, one using IPv4 - // and one using IPv6. - ProtoFixtureRepository repo; - repo.RegisterValue("@ingress_port", in_port) - .RegisterValue("@egress_src_mac", kOriginalSrcMacAddress.ToString()) - .RegisterValue("@ttl", "0x40") - .RegisterValue("@hop_limit", "0x50") - .RegisterValue("@decremented_hop_limit", "0x4f") - .RegisterValue("@decremented_ttl", "0x3f") - .RegisterValue("@ipv4_dst", ipv4_address.ToString()) - .RegisterValue("@ipv6_dst", ipv6_address.ToString()) - .RegisterValue( - "@payload_ipv4", - dvaas::MakeTestPacketPayloadFromUniqueId(unique_payload_ids++)) - .RegisterValue( - "@payload_ipv6", - dvaas::MakeTestPacketPayloadFromUniqueId(unique_payload_ids++)); - // Build headers. - repo.RegisterSnippetOrDie("@ethernet_ipv4", R"pb( - ethernet_header { - ethernet_destination: "01:00:5e:01:01:01" - ethernet_source: @egress_src_mac - ethertype: "0x0800" # IPv4 - } - )pb") - .RegisterSnippetOrDie("@ethernet_ipv6", R"pb( - ethernet_header { - ethernet_destination: "33:33:00:00:00:01" - ethernet_source: @egress_src_mac - ethertype: "0x86dd" # IPv6 - } - )pb") - .RegisterSnippetOrDie("@ipv4", R"pb( - ipv4_header { - version: "0x4" - ihl: "0x5" - dscp: "0x00" - ecn: "0x0" - # total_length: filled in automatically. - identification: "0x0000" - flags: "0x0" - fragment_offset: "0x0000" - ttl: @ttl - protocol: "0x11" - # checksum: filled in automatically - ipv4_source: "128.252.7.36" - ipv4_destination: @ipv4_dst - } - )pb") - .RegisterSnippetOrDie("@ipv6", R"pb( - ipv6_header { - version: "0x6" - dscp: "0x00" - ecn: "0x1" - flow_label: "0x12345" - # payload_length: filled in automatically. - next_header: "0x11" - hop_limit: @hop_limit - ipv6_source: "2002:ad12:4100:3::" - ipv6_destination: @ipv6_dst - } - )pb") - .RegisterSnippetOrDie("@udp", R"pb( - udp_header { - source_port: "0x0567" # 1383 - destination_port: "0x1234" # 4660 - # length: filled in automatically - # checksum: filled in automatically - } - )pb") - .RegisterMessage("@input_packet_ipv4", ParsePacketAndPadToMinimumSize( + const std::string& in_port = params.sut_port_ids[0]; + int unique_payload_ids = 2 * params.input_group_index + 1; + + // We will inject 2 packets to touch each multicast group, one using IPv4 + // and one using IPv6. + ASSIGN_OR_RETURN(const auto ipv4_address, + GetIpv4AddressForReplica(params.input_group_index)); + ASSIGN_OR_RETURN(const auto ipv6_address, + GetIpv6AddressForReplica(params.input_group_index)); + ProtoFixtureRepository repo; + repo.RegisterValue("@ingress_port", in_port) + .RegisterValue("@egress_src_mac", kOriginalSrcMacAddress.ToString()) + .RegisterValue("@ttl", "0x40") + .RegisterValue("@hop_limit", "0x50") + .RegisterValue("@decremented_hop_limit", "0x4f") + .RegisterValue("@decremented_ttl", "0x3f") + .RegisterValue("@ipv4_dst", ipv4_address.ToString()) + .RegisterValue("@ipv6_dst", ipv6_address.ToString()) + .RegisterValue("@payload_ipv4", + dvaas::MakeTestPacketTagFromUniqueId(unique_payload_ids++, + "IPv4 UDP packet")) + .RegisterValue("@payload_ipv6", + dvaas::MakeTestPacketTagFromUniqueId(unique_payload_ids++, + "IPv6 UDP packet")); + if (params.input_vlan_id.has_value()) { + repo.RegisterValue("@input_vlan_id", + GetVlanIdHexStr(params.input_vlan_id.value())); + } + if (params.out_ports_vlan_id.has_value()) { + repo.RegisterValue("@out_ports_vlan_id", + GetVlanIdHexStr(params.out_ports_vlan_id.value())); + } + + // Build headers. + repo.RegisterSnippetOrDie("@ethernet_ipv4", R"pb( + ethernet_header { + ethernet_destination: "01:00:5e:01:01:01", + ethernet_source: @egress_src_mac + ethertype: "0x0800" # IPv4 + } + )pb") + .RegisterSnippetOrDie("@ethernet_ipv6", R"pb( + ethernet_header { + ethernet_destination: "33:33:00:00:00:01" + ethernet_source: @egress_src_mac + ethertype: "0x86dd" # IPv6 + } + )pb") + .RegisterSnippetOrDie("@ethernet_ipv4_vlan", R"pb( + ethernet_header { + ethernet_destination: "01:00:5e:01:01:01", + ethernet_source: @egress_src_mac + ethertype: "0x8100" # vlan + } + )pb") + .RegisterSnippetOrDie("@ethernet_ipv6_vlan", R"pb( + ethernet_header { + ethernet_destination: "33:33:00:00:00:01", + ethernet_source: @egress_src_mac + ethertype: "0x8100" # vlan + } + )pb") + .RegisterSnippetOrDie("@vlan_ipv4", R"pb( + vlan_header { + priority_code_point: "0x0", + drop_eligible_indicator: "0x0", + vlan_identifier: "0x0", + ethertype: "0x0800" # IPv4 + } + )pb") + .RegisterSnippetOrDie("@vlan_ipv6", R"pb( + vlan_header { + priority_code_point: "0x0", + drop_eligible_indicator: "0x0", + vlan_identifier: "0x0", + ethertype: "0x86dd" # IPv6 + } + )pb") + .RegisterSnippetOrDie("@ipv4", R"pb( + ipv4_header { + version: "0x4" + ihl: "0x5" + dscp: "0x00" + ecn: "0x0" + # total_length: filled in automatically. + identification: "0x0000" + flags: "0x0" + fragment_offset: "0x0000" + ttl: @ttl + protocol: "0x11" + # checksum: filled in automatically + ipv4_source: "128.252.7.36" + ipv4_destination: @ipv4_dst + } + )pb") + .RegisterSnippetOrDie("@ipv6", R"pb( + ipv6_header { + version: "0x6" + dscp: "0x00" + ecn: "0x1" + flow_label: "0x12345" + # payload_length: filled in automatically. + next_header: "0x11" + hop_limit: @hop_limit + ipv6_source: "2002:ad12:4100:3::" + ipv6_destination: @ipv6_dst + } + )pb") + .RegisterSnippetOrDie("@udp", R"pb( + udp_header { + source_port: "0x0567" # 1383 + destination_port: "0x1234" # 4660 + # length: filled in automatically + # checksum: filled in automatically + } + )pb"); + + if (params.input_vlan_id.has_value()) { + repo.RegisterMessage( + "@input_packet_ipv4", + ParsePacketAndPadToMinimumSize( + repo, + R"pb( + headers: @ethernet_ipv4_vlan + headers: + @vlan_ipv4 { vlan_header { vlan_identifier: @input_vlan_id } } + headers: @ipv4 + headers: @udp + payload: @payload_ipv4 + )pb")); + repo.RegisterMessage( + "@input_packet_ipv6", + ParsePacketAndPadToMinimumSize( + repo, + R"pb( + headers: @ethernet_ipv6_vlan + headers: + @vlan_ipv6 { vlan_header { vlan_identifier: @input_vlan_id } } + headers: @ipv6 + headers: @udp + payload: @payload_ipv6 + )pb")); + } else { + repo.RegisterMessage("@input_packet_ipv4", ParsePacketAndPadToMinimumSize( repo, R"pb( headers: @ethernet_ipv4 headers: @ipv4 headers: @udp payload: @payload_ipv4 - )pb")) - .RegisterMessage("@input_packet_ipv6", ParsePacketAndPadToMinimumSize( + )pb")); + repo.RegisterMessage("@input_packet_ipv6", ParsePacketAndPadToMinimumSize( repo, R"pb( headers: @ethernet_ipv6 @@ -459,21 +651,30 @@ absl::StatusOr> BuildTestVectors( headers: @udp payload: @payload_ipv6 )pb")); - // Build up acceptable_outputs string, to account for each replica. - dvaas::SwitchOutput expected_ipv4_output; - dvaas::SwitchOutput expected_ipv6_output; - for (int r = 0; r < replicas_per_group; ++r) { - ASSIGN_OR_RETURN(const auto egress_src_mac, - GetSrcMacForReplica(m, replicas_per_group, r)); + } + + // Build up acceptable_outputs string, to account for each replica. + dvaas::SwitchOutput expected_ipv4_output; + dvaas::SwitchOutput expected_ipv6_output; + for (int r = params.output_replica_start_index; r < params.replicas_per_group; + ++r) { + ASSIGN_OR_RETURN(const auto egress_src_mac, + GetSrcMacForReplica(params.output_group_index, + params.replicas_per_group, r)); + + if (params.out_ports_vlan_id.has_value()) { // IPv4 *expected_ipv4_output.add_packets() = - repo.RegisterValue("@egress_port", port_ids.at(r + 1)) + repo.RegisterValue("@egress_port", params.sut_port_ids[r + 1]) .RegisterValue("@egress_src_mac", egress_src_mac.ToString()) .RegisterMessage( "@output_packet", ParsePacketAndPadToMinimumSize(repo, R"pb( - headers: @ethernet_ipv4 { + headers: @ethernet_ipv4_vlan { ethernet_header { ethernet_source: @egress_src_mac } } + headers: @vlan_ipv4 { + vlan_header { vlan_identifier: @out_ports_vlan_id } + } headers: @ipv4 { ipv4_header { ttl: @decremented_ttl } } headers: @udp payload: @payload_ipv4 @@ -484,13 +685,16 @@ absl::StatusOr> BuildTestVectors( )pb"); // IPv6 *expected_ipv6_output.add_packets() = - repo.RegisterValue("@egress_port", port_ids.at(r + 1)) + repo.RegisterValue("@egress_port", params.sut_port_ids[r + 1]) .RegisterValue("@egress_src_mac", egress_src_mac.ToString()) .RegisterMessage( "@output_packet", ParsePacketAndPadToMinimumSize(repo, R"pb( - headers: @ethernet_ipv6 { + headers: @ethernet_ipv6_vlan { ethernet_header { ethernet_source: @egress_src_mac } } + headers: @vlan_ipv6 { + vlan_header { vlan_identifier: @out_ports_vlan_id } + } headers: @ipv6 { ipv6_header { hop_limit: @decremented_hop_limit } } @@ -501,32 +705,49 @@ absl::StatusOr> BuildTestVectors( port: @egress_port parsed: @output_packet )pb"); - } // for replica - LOG(INFO) << "Packets will be sent on port " << in_port; - LOG(INFO) << "Expected outputs should be " << total_packets << " packets"; - - if (expect_output_packets) { - expectations.emplace_back() = - repo.RegisterMessage("@expected_ipv4_output", expected_ipv4_output) - .ParseTextOrDie(R"pb( - input { - type: DATAPLANE - packet { port: @ingress_port parsed: @input_packet_ipv4 } - } - acceptable_outputs: @expected_ipv4_output + } else { + *expected_ipv4_output.add_packets() = + repo.RegisterValue("@egress_port", params.sut_port_ids[r + 1]) + .RegisterValue("@egress_src_mac", egress_src_mac.ToString()) + .RegisterMessage( + "@output_packet", ParsePacketAndPadToMinimumSize(repo, R"pb( + headers: @ethernet_ipv4 { + ethernet_header { ethernet_source: @egress_src_mac } + } + headers: @ipv4 { ipv4_header { ttl: @decremented_ttl } } + headers: @udp + payload: @payload_ipv4 + )pb")) + .ParseTextOrDie(R"pb( + port: @egress_port + parsed: @output_packet )pb"); - expectations.emplace_back() = - repo.RegisterMessage("@expected_ipv6_output", expected_ipv6_output) - .ParseTextOrDie(R"pb( - input { - type: DATAPLANE - packet { port: @ingress_port parsed: @input_packet_ipv6 } - } - acceptable_outputs: @expected_ipv6_output + // IPv6 + *expected_ipv6_output.add_packets() = + repo.RegisterValue("@egress_port", params.sut_port_ids[r + 1]) + .RegisterValue("@egress_src_mac", egress_src_mac.ToString()) + .RegisterMessage( + "@output_packet", ParsePacketAndPadToMinimumSize(repo, R"pb( + headers: @ethernet_ipv6 { + ethernet_header { ethernet_source: @egress_src_mac } + } + headers: @ipv6 { + ipv6_header { hop_limit: @decremented_hop_limit } + } + headers: @udp + payload: @payload_ipv6 + )pb")) + .ParseTextOrDie(R"pb( + port: @egress_port + parsed: @output_packet )pb"); - } else { - expectations.push_back(repo.ParseTextOrDie( - R"pb( + } // for out_ports_vlan_id + } // for replica + LOG(INFO) << "Packets will be sent on port " << in_port; + + expectations.emplace_back() = + repo.RegisterMessage("@expected_ipv4_output", expected_ipv4_output) + .ParseTextOrDie(R"pb( input { type: DATAPLANE packet { port: @ingress_port parsed: @input_packet_ipv4 } @@ -538,8 +759,32 @@ absl::StatusOr> BuildTestVectors( type: DATAPLANE packet { port: @ingress_port parsed: @input_packet_ipv6 } } - )pb")); - } + acceptable_outputs: @expected_ipv6_output + )pb"); + + return expectations; +} + +// Build test packets that match the multicast table entries +absl::StatusOr> BuildTestVectors( + const MulticastForwardingParams& params) { + // Test packets injected and expected results. + std::vector expectations; + for (int m = 0; m < params.number_multicast_groups; ++m) { + ASSIGN_OR_RETURN( + auto group_expectations, + BuildTestVectors({ + .sut_port_ids = params.sut_port_ids, + .input_group_index = m, + .output_group_index = m, + .replicas_per_group = params.number_replicas_per_group, + .output_replica_start_index = 0, + .input_vlan_id = params.vlan_forwarding_params.input_vlan_id, + .out_ports_vlan_id = + params.vlan_forwarding_params.out_ports_vlan_id, + })); + expectations.insert(expectations.end(), group_expectations.begin(), + group_expectations.end()); } // for multicast group return expectations; } @@ -602,10 +847,11 @@ TEST_P(L3MulticastTestFixture, TEST_P(L3MulticastTestFixture, DeleteNonexistentRifEntryFails) { // Unable to delete RIF entry that was not added. - ASSERT_OK_AND_ASSIGN( - const auto entities, - CreateRifTableEntities(ir_p4info_, /*port_id=*/"1", kDefaultInstance, - kOriginalSrcMacAddress)); + const MrifTableEntryParams rif_params = {.egress_port = "1", + .instance = kDefaultInstance, + .src_mac = kOriginalSrcMacAddress}; + ASSERT_OK_AND_ASSIGN(const auto entities, + CreateRifTableEntities(ir_p4info_, rif_params)); EXPECT_THAT(ClearEntities(*sut_p4rt_session_, ir_p4info_, entities), StatusIs(absl::StatusCode::kUnknown, @@ -673,7 +919,6 @@ TEST_P(L3MulticastTestFixture, DeleteNonexistentIpmcEntryFails) { TEST_P(L3MulticastTestFixture, BasicReplicationProgramming) { thinkit::MirrorTestbed& testbed = GetParam().mirror_testbed->GetMirrorTestbed(); - const int kNumberMulticastGroupsInTest = 1; const int kPortsToUseInTest = 2; // Collect port IDs. // Get SUT and control ports to test on. @@ -684,20 +929,21 @@ TEST_P(L3MulticastTestFixture, BasicReplicationProgramming) { // -------------------------------------------------------------------------- // Add multicast programming. // -------------------------------------------------------------------------- + + const MulticastForwardingParams test_params = { + .sut_port_ids = sut_ports_ids, + .assignment_mechanism = IpmcGroupAssignmentMechanism::kIpMulticastTable, + .number_replicas_per_group = kPortsToUseInTest, + .number_multicast_groups = 1, + }; LOG(INFO) << "Adding multicast programming."; std::vector entities_created; - ASSERT_OK(SetupDefaultMulticastProgramming( - *sut_p4rt_session_, ir_p4info_, p4::v1::Update::INSERT, - kNumberMulticastGroupsInTest, /*replicas_per_group=*/kPortsToUseInTest, - sut_ports_ids, IpmcGroupAssignmentMechanism::kIpMulticastTable, - entities_created)); + ASSERT_OK(SetupDefaultMulticastProgramming(*sut_p4rt_session_, ir_p4info_, + p4::v1::Update::INSERT, + test_params, entities_created)); LOG(INFO) << "Added " << entities_created.size() << " entities."; // Build test packets. - ASSERT_OK_AND_ASSIGN( - auto vectors, - BuildTestVectors(sut_ports_ids, kNumberMulticastGroupsInTest, - /*replicas_per_group=*/kPortsToUseInTest, - /*expect_output_packets=*/true)); + ASSERT_OK_AND_ASSIGN(auto vectors, BuildTestVectors(test_params)); // Send test packets. LOG(INFO) << "Sending traffic to verify added multicast programming."; dvaas::DataplaneValidationParams dvaas_params = @@ -753,7 +999,6 @@ TEST_P(L3MulticastTestFixture, BasicReplicationProgrammingWithAclRedirect) { thinkit::MirrorTestbed& testbed = GetParam().mirror_testbed->GetMirrorTestbed(); - constexpr int kNumberMulticastGroupsInTest = 1; constexpr int kPortsToUseInTest = 2; // Get set of ports on the SUT and control switch to test on. @@ -761,21 +1006,21 @@ TEST_P(L3MulticastTestFixture, BasicReplicationProgrammingWithAclRedirect) { const std::vector sut_ports_ids, GetNUpInterfaceIDs(GetParam().mirror_testbed->GetMirrorTestbed().Sut(), kPortsToUseInTest + 1)); + const MulticastForwardingParams test_params = { + .sut_port_ids = sut_ports_ids, + .assignment_mechanism = IpmcGroupAssignmentMechanism::kAclRedirect, + .number_replicas_per_group = kPortsToUseInTest, + .number_multicast_groups = 1, + }; std::vector entities_created; - ASSERT_OK(SetupDefaultMulticastProgramming( - *sut_p4rt_session_, ir_p4info_, p4::v1::Update::INSERT, - kNumberMulticastGroupsInTest, /*replicas_per_group=*/kPortsToUseInTest, - sut_ports_ids, IpmcGroupAssignmentMechanism::kAclRedirect, - entities_created)); + ASSERT_OK(SetupDefaultMulticastProgramming(*sut_p4rt_session_, ir_p4info_, + p4::v1::Update::INSERT, + test_params, entities_created)); LOG(INFO) << "Added " << entities_created.size() << " entities."; // Build test packets. - ASSERT_OK_AND_ASSIGN( - auto vectors, - BuildTestVectors(sut_ports_ids, kNumberMulticastGroupsInTest, - /*replicas_per_group=*/kPortsToUseInTest, - /*expect_output_packets=*/true)); + ASSERT_OK_AND_ASSIGN(auto vectors, BuildTestVectors(test_params)); // Send test packets. LOG(INFO) << "Sending traffic to verify added multicast programming."; @@ -798,6 +1043,131 @@ TEST_P(L3MulticastTestFixture, BasicReplicationProgrammingWithAclRedirect) { EXPECT_OK(validation_result.HasSuccessRateOfAtLeast(1.0)); } +// This test confirms that when a participant drops out of a multicast group, +// there is is no longer an externally replicated packet received. +TEST_P(L3MulticastTestFixture, UnregisteredParticipantProgramming) { + + if (!pins::TableHasMatchField(ir_p4info_, "acl_egress_l2_table", + "src_mac")) { + GTEST_SKIP() + << "Skipping because match field 'src_mac' is not available in table " + << "'acl_egress_l2_table'"; + } + + thinkit::MirrorTestbed& testbed = + GetParam().mirror_testbed->GetMirrorTestbed(); + constexpr int kPortsToUseInTest = 2; + + // Get set of ports on the SUT and control switch to test on. + ASSERT_OK_AND_ASSIGN( + const std::vector sut_ports_ids, + GetNUpInterfaceIDs(GetParam().mirror_testbed->GetMirrorTestbed().Sut(), + kPortsToUseInTest + 1)); + + const MulticastForwardingParams test_params = { + .sut_port_ids = sut_ports_ids, + .assignment_mechanism = IpmcGroupAssignmentMechanism::kIpMulticastTable, + .number_replicas_per_group = kPortsToUseInTest, + .number_multicast_groups = 1, + }; + + // Setup 3 RIFs, 2 with valid source MAC addresses and 1 with the drop MAC + // address. Have multicast group begin expecting replication to all members. + std::vector entities_created; + ASSERT_OK(SetupDefaultMulticastProgramming(*sut_p4rt_session_, ir_p4info_, + p4::v1::Update::INSERT, + test_params, entities_created)); + LOG(INFO) << "Added " << entities_created.size() << " entities."; + + // Add additional drop RIF. + constexpr int kDropInstance = 33; + const MrifTableEntryParams rif_params = {.egress_port = sut_ports_ids[1], + .instance = kDropInstance, + .src_mac = kDropSrcMacAddress}; + ASSERT_OK_AND_ASSIGN(auto rif_entities, + CreateRifTableEntities(ir_p4info_, rif_params)); + ASSERT_OK(pdpi::InstallPiEntities(sut_p4rt_session_.get(), ir_p4info_, + rif_entities)); + + // Create the Egress ACL entry to drop relevant Ethernet source MAC. + ASSERT_OK_AND_ASSIGN(auto proto_entry, + gutil::ParseTextProto( + R"pb(table_name: "acl_egress_l2_table" + priority: 1 + matches { + name: "src_mac" + ternary { + value { mac: "02:2a:10:00:00:02" } + mask { mac: "ff:ff:ff:ff:ff:ff" } + } + } + action { name: "acl_drop" } + )pb")); + + EXPECT_OK(pdpi::InstallIrTableEntry(*sut_p4rt_session_.get(), proto_entry)); + + // Build test packets expecting 2 replicas received per packet sent. + ASSERT_OK_AND_ASSIGN(auto vectors, BuildTestVectors(test_params)); + LOG(INFO) << "Sending traffic to verify added multicast programming."; + dvaas::DataplaneValidationParams dvaas_params = + dvaas::DefaultpinsDataplaneValidationParams(); + // Ensure the port map for the control switch can map to the SUT (for + // situations where the config differs for SUT and control switch). + auto interface_to_peer_entity_map = gtl::ValueOrDie( + pins::ControlP4rtPortIdBySutP4rtPortIdFromSwitchConfig()); + dvaas_params.mirror_testbed_port_map_override = gtl::ValueOrDie( + dvaas::MirrorTestbedP4rtPortIdMap::CreateFromControlSwitchToSutPortMap( + interface_to_peer_entity_map)); + dvaas_params.packet_test_vector_override = vectors; + + ASSERT_OK_AND_ASSIGN( + dvaas::ValidationResult validation_result, + GetParam().dvaas->ValidateDataplane(testbed, dvaas_params)); + // Validate traffic. + validation_result.LogStatistics(); + EXPECT_OK(validation_result.HasSuccessRateOfAtLeast(1.0)); + + // Reestablish primary-ship of P4RT connection. + ASSERT_OK_AND_ASSIGN(sut_p4rt_session_, + pdpi::P4RuntimeSession::Create(testbed.Sut())); + + // ---------------------------------------------------------------------- + // Update multicast group to change 1 member to unsubscribe. + // ---------------------------------------------------------------------- + std::vector replicas; + replicas.push_back({sut_ports_ids[1], kDropInstance}); + replicas.push_back({sut_ports_ids[2], /*.instance=*/1}); // unchanged + ASSERT_OK_AND_ASSIGN(auto update_multicast_group_entities, + CreateMulticastGroupEntities( + ir_p4info_, /*multicast_group_id=*/1, replicas)); + ASSERT_OK( + pdpi::SendPiUpdates(sut_p4rt_session_.get(), + pdpi::CreatePiUpdates(update_multicast_group_entities, + p4::v1::Update::MODIFY))); + + // Send traffic and confirm only 1 replica received (instead of 2). + ASSERT_OK_AND_ASSIGN( + auto update_vectors, + BuildTestVectors({.sut_port_ids = sut_ports_ids, + .input_group_index = 0, + .output_group_index = 0, + .replicas_per_group = kPortsToUseInTest, + .output_replica_start_index = 1})); + LOG(INFO) << "Sending traffic to verify added multicast programming."; + dvaas_params.packet_test_vector_override = update_vectors; + + ASSERT_OK_AND_ASSIGN( + dvaas::ValidationResult update_validation_result, + GetParam().dvaas->ValidateDataplane(testbed, dvaas_params)); + // Validate traffic. + update_validation_result.LogStatistics(); + EXPECT_OK(update_validation_result.HasSuccessRateOfAtLeast(1.0)); +} + +// This test confirms that fixed delay programming is achievable by adding +// group members where the replications are always dropped. +// Part of our use case requires us to replicate packets that must be dropped, +// which adds a fixed amount of time before a "real" replicated packet emerges. TEST_P(L3MulticastTestFixture, ConfirmFixedDelayProgramming) { if (!pins::TableHasMatchField(ir_p4info_, "acl_egress_l2_table", "src_mac")) { @@ -808,7 +1178,6 @@ TEST_P(L3MulticastTestFixture, ConfirmFixedDelayProgramming) { thinkit::MirrorTestbed& testbed = GetParam().mirror_testbed->GetMirrorTestbed(); - constexpr int kNumberMulticastGroupsInTest = 1; // We'll use 4 "drop" replicas and 2 expected replications. constexpr int kPortsToUseInTest = 6; @@ -818,23 +1187,30 @@ TEST_P(L3MulticastTestFixture, ConfirmFixedDelayProgramming) { GetNUpInterfaceIDs(GetParam().mirror_testbed->GetMirrorTestbed().Sut(), kPortsToUseInTest + 1)); + const MulticastForwardingParams test_params = { + .sut_port_ids = sut_ports_ids, + .assignment_mechanism = IpmcGroupAssignmentMechanism::kIpMulticastTable, + .number_replicas_per_group = kPortsToUseInTest, + .number_multicast_groups = 1, + }; + // Setup 6 RIFs, 2 with valid source MAC addresses and 4 with the drop MAC // address. std::vector entities_created; - ASSERT_OK(SetupDefaultMulticastProgramming( - *sut_p4rt_session_, ir_p4info_, p4::v1::Update::INSERT, - kNumberMulticastGroupsInTest, /*replicas_per_group=*/kPortsToUseInTest, - sut_ports_ids, IpmcGroupAssignmentMechanism::kIpMulticastTable, - entities_created)); + ASSERT_OK(SetupDefaultMulticastProgramming(*sut_p4rt_session_, ir_p4info_, + p4::v1::Update::INSERT, + test_params, entities_created)); LOG(INFO) << "Added " << entities_created.size() << " entities."; // Add the 4 drop RIFs using the first 4 ports for replication. constexpr int kDropInstance = 33; for (int port_index = 1; port_index <= 4; ++port_index) { - ASSERT_OK_AND_ASSIGN( - auto rif_entities, - CreateRifTableEntities(ir_p4info_, sut_ports_ids[port_index], - kDropInstance, kDropSrcMacAddress)); + const MrifTableEntryParams rif_params = { + .egress_port = sut_ports_ids[port_index], + .instance = kDropInstance, + .src_mac = kDropSrcMacAddress}; + ASSERT_OK_AND_ASSIGN(auto rif_entities, + CreateRifTableEntities(ir_p4info_, rif_params)); ASSERT_OK(pdpi::InstallPiEntities(sut_p4rt_session_.get(), ir_p4info_, rif_entities)); } @@ -879,11 +1255,11 @@ TEST_P(L3MulticastTestFixture, ConfirmFixedDelayProgramming) { // 4 of the "prefix" replicas should have been dropped. ASSERT_OK_AND_ASSIGN( auto update_vectors, - BuildInputOutputVectors(sut_ports_ids, - /*input_group_index=*/0, - /*output_group_index=*/0, - /*replicas_per_group=*/kPortsToUseInTest, - /*output_replica_start_index=*/4)); + BuildTestVectors({.sut_port_ids = sut_ports_ids, + .input_group_index = 0, + .output_group_index = 0, + .replicas_per_group = kPortsToUseInTest, + .output_replica_start_index = 4})); LOG(INFO) << "Sending traffic to verify added multicast programming."; dvaas::DataplaneValidationParams dvaas_params = dvaas::DefaultpinsDataplaneValidationParams(); @@ -907,10 +1283,8 @@ TEST_P(L3MulticastTestFixture, ConfirmFixedDelayProgramming) { // This test confirms replicating N times to the same port (using different // replica instances) will produce N output packets. TEST_P(L3MulticastTestFixture, ReplicatingNTimesToSamePortProducesNCopies) { - thinkit::MirrorTestbed& testbed = GetParam().mirror_testbed->GetMirrorTestbed(); - constexpr int kNumberMulticastGroupsInTest = 1; constexpr int kOutputPortsToUseInTest = 1; constexpr int kInitialReplicasPerGroup = 1; @@ -920,16 +1294,21 @@ TEST_P(L3MulticastTestFixture, ReplicatingNTimesToSamePortProducesNCopies) { GetNUpInterfaceIDs(GetParam().mirror_testbed->GetMirrorTestbed().Sut(), kOutputPortsToUseInTest + 1)); + const MulticastForwardingParams test_params = { + .sut_port_ids = sut_ports_ids, + .assignment_mechanism = IpmcGroupAssignmentMechanism::kIpMulticastTable, + .number_replicas_per_group = kInitialReplicasPerGroup, + .number_multicast_groups = 1, + }; + // Setup 3 RIFs, all on same port but that use different instances. // We will expect that the multicast group will produce three copies. // Setup default programming assuming only 1 replica to start, since default // programming wants to output on different ports. std::vector entities_created; - ASSERT_OK(SetupDefaultMulticastProgramming( - *sut_p4rt_session_, ir_p4info_, p4::v1::Update::INSERT, - kNumberMulticastGroupsInTest, - /*replicas_per_group=*/kInitialReplicasPerGroup, sut_ports_ids, - IpmcGroupAssignmentMechanism::kIpMulticastTable, entities_created)); + ASSERT_OK(SetupDefaultMulticastProgramming(*sut_p4rt_session_, ir_p4info_, + p4::v1::Update::INSERT, + test_params, entities_created)); LOG(INFO) << "Added " << entities_created.size() << " entities."; // Add additional active RIF that will replicate to the same port. @@ -939,13 +1318,17 @@ TEST_P(L3MulticastTestFixture, ReplicatingNTimesToSamePortProducesNCopies) { /*replicas_number=*/1)); constexpr int kExtraInstance = 1; constexpr int kExtraInstanceTwo = 2; + const MrifTableEntryParams rif_params = {.egress_port = sut_ports_ids[1], + .instance = kExtraInstance, + .src_mac = kExtraSrcMac}; ASSERT_OK_AND_ASSIGN(auto rif_entities, - CreateRifTableEntities(ir_p4info_, sut_ports_ids[1], - kExtraInstance, kExtraSrcMac)); + CreateRifTableEntities(ir_p4info_, rif_params)); // Create another RIF with different instance but same src MAC. + const MrifTableEntryParams rif_params2 = {.egress_port = sut_ports_ids[1], + .instance = kExtraInstanceTwo, + .src_mac = kExtraSrcMac}; ASSERT_OK_AND_ASSIGN(auto rif_entities2, - CreateRifTableEntities(ir_p4info_, sut_ports_ids[1], - kExtraInstanceTwo, kExtraSrcMac)); + CreateRifTableEntities(ir_p4info_, rif_params2)); ASSERT_OK(pdpi::InstallPiEntities(sut_p4rt_session_.get(), ir_p4info_, rif_entities)); ASSERT_OK(pdpi::InstallPiEntities(sut_p4rt_session_.get(), ir_p4info_, @@ -1140,7 +1523,6 @@ TEST_P(L3MulticastTestFixture, ConfirmAclRedirectOverridesIpMulticastTable) { thinkit::MirrorTestbed& testbed = GetParam().mirror_testbed->GetMirrorTestbed(); - constexpr int kNumberMulticastGroupsInTest = 2; constexpr int kPortsToUseInTest = 2; // Get set of ports on the SUT and control switch to test on. @@ -1149,12 +1531,17 @@ TEST_P(L3MulticastTestFixture, ConfirmAclRedirectOverridesIpMulticastTable) { GetNUpInterfaceIDs(GetParam().mirror_testbed->GetMirrorTestbed().Sut(), kPortsToUseInTest + 1)); + const MulticastForwardingParams test_params = { + .sut_port_ids = sut_ports_ids, + .assignment_mechanism = IpmcGroupAssignmentMechanism::kIpMulticastTable, + .number_replicas_per_group = kPortsToUseInTest, + .number_multicast_groups = 2, + }; + std::vector entities_created; - ASSERT_OK(SetupDefaultMulticastProgramming( - *sut_p4rt_session_, ir_p4info_, p4::v1::Update::INSERT, - kNumberMulticastGroupsInTest, /*replicas_per_group=*/kPortsToUseInTest, - sut_ports_ids, IpmcGroupAssignmentMechanism::kIpMulticastTable, - entities_created)); + ASSERT_OK(SetupDefaultMulticastProgramming(*sut_p4rt_session_, ir_p4info_, + p4::v1::Update::INSERT, + test_params, entities_created)); // Setup the ACL redirect path to use multicast group 2. constexpr int kMulticastGroup2 = 2; @@ -1180,11 +1567,13 @@ TEST_P(L3MulticastTestFixture, ConfirmAclRedirectOverridesIpMulticastTable) { // assign to multicast group 1. However, expect the ACL redirect path to // override the IPMC table assignment such that multicast group 2 will be // the outputs. - ASSERT_OK_AND_ASSIGN( - auto vectors, - BuildInputOutputVectors(sut_ports_ids, /*input_group_index=*/0, - /*output_group_index=*/1, - /*replicas_per_group=*/kPortsToUseInTest)); + ASSERT_OK_AND_ASSIGN(auto vectors, + BuildTestVectors({ + .sut_port_ids = sut_ports_ids, + .input_group_index = 0, + .output_group_index = 1, + .replicas_per_group = kPortsToUseInTest, + })); // Send test packets. LOG(INFO) << "Sending traffic to verify added multicast programming."; @@ -1210,10 +1599,11 @@ TEST_P(L3MulticastTestFixture, ConfirmAclRedirectOverridesIpMulticastTable) { TEST_P(L3MulticastTestFixture, AddMulticastRifForUnknownPortFails) { // Unable to add an entry if the port does not exist. const std::string kUnknownPortId = "20000"; - ASSERT_OK_AND_ASSIGN( - const auto entities, - CreateRifTableEntities(ir_p4info_, kUnknownPortId, kDefaultInstance, - kOriginalSrcMacAddress)); + const MrifTableEntryParams rif_params = {.egress_port = kUnknownPortId, + .instance = kDefaultInstance, + .src_mac = kOriginalSrcMacAddress}; + ASSERT_OK_AND_ASSIGN(const auto entities, + CreateRifTableEntities(ir_p4info_, rif_params)); EXPECT_THAT(InstallPiEntities(sut_p4rt_session_.get(), ir_p4info_, entities), StatusIs(absl::StatusCode::kUnknown, @@ -1228,11 +1618,11 @@ TEST_P(L3MulticastTestFixture, AddMulticastReplicaForUnknownPortInstanceFails) { const auto sut_ports_ids, GetNUpInterfaceIDs(GetParam().mirror_testbed->GetMirrorTestbed().Sut(), kPortsToUseInTest)); - - ASSERT_OK_AND_ASSIGN( - const auto rif_entities, - CreateRifTableEntities(ir_p4info_, sut_ports_ids.at(0), kDefaultInstance, - kOriginalSrcMacAddress)); + const MrifTableEntryParams rif_params = {.egress_port = sut_ports_ids[0], + .instance = kDefaultInstance, + .src_mac = kOriginalSrcMacAddress}; + ASSERT_OK_AND_ASSIGN(const auto rif_entities, + CreateRifTableEntities(ir_p4info_, rif_params)); // Purposefully do not create a RIF for the second port ID. EXPECT_OK(pdpi::InstallPiEntities(sut_p4rt_session_.get(), ir_p4info_, rif_entities)); @@ -1268,10 +1658,11 @@ TEST_P(L3MulticastTestFixture, AddIpmcEntryForUnknownMulticastGroupFails) { std::vector rif_entities; for (int r = 0; r < kPortsToUseInTest; ++r) { - ASSERT_OK_AND_ASSIGN( - const auto rifs, - CreateRifTableEntities(ir_p4info_, sut_ports_ids.at(r), - kDefaultInstance, kOriginalSrcMacAddress)); + const MrifTableEntryParams rif_params = {.egress_port = sut_ports_ids[r], + .instance = kDefaultInstance, + .src_mac = kOriginalSrcMacAddress}; + ASSERT_OK_AND_ASSIGN(const auto rifs, + CreateRifTableEntities(ir_p4info_, rif_params)); rif_entities.insert(rif_entities.end(), rifs.begin(), rifs.end()); } EXPECT_OK(pdpi::InstallPiEntities(sut_p4rt_session_.get(), ir_p4info_, @@ -1331,10 +1722,11 @@ TEST_P(L3MulticastTestFixture, AddIpmcEntryForUnknownVrfFails) { std::vector rif_entities; for (int r = 0; r < kPortsToUseInTest; ++r) { - ASSERT_OK_AND_ASSIGN( - const auto rifs, - CreateRifTableEntities(ir_p4info_, sut_ports_ids.at(r), - kDefaultInstance, kOriginalSrcMacAddress)); + const MrifTableEntryParams rif_params = {.egress_port = sut_ports_ids[r], + .instance = kDefaultInstance, + .src_mac = kOriginalSrcMacAddress}; + ASSERT_OK_AND_ASSIGN(const auto rifs, + CreateRifTableEntities(ir_p4info_, rif_params)); rif_entities.insert(rif_entities.end(), rifs.begin(), rifs.end()); } EXPECT_OK(pdpi::InstallPiEntities(sut_p4rt_session_.get(), ir_p4info_, @@ -1384,10 +1776,11 @@ TEST_P(L3MulticastTestFixture, AddIpmcEntryWithInvalidIPv4AddressFails) { std::vector rif_entities; for (int r = 0; r < kPortsToUseInTest; ++r) { - ASSERT_OK_AND_ASSIGN( - const auto rifs, - CreateRifTableEntities(ir_p4info_, sut_ports_ids[r], kDefaultInstance, - kOriginalSrcMacAddress)); + const MrifTableEntryParams rif_params = {.egress_port = sut_ports_ids[r], + .instance = kDefaultInstance, + .src_mac = kOriginalSrcMacAddress}; + ASSERT_OK_AND_ASSIGN(const auto rifs, + CreateRifTableEntities(ir_p4info_, rif_params)); rif_entities.insert(rif_entities.end(), rifs.begin(), rifs.end()); } EXPECT_OK(pdpi::InstallPiEntities(sut_p4rt_session_.get(), ir_p4info_, @@ -1443,10 +1836,11 @@ TEST_P(L3MulticastTestFixture, DeleteRifWhileInUseFails) { std::vector rif_entities; for (int r = 0; r < kPortsToUseInTest; ++r) { - ASSERT_OK_AND_ASSIGN( - const auto rifs, - CreateRifTableEntities(ir_p4info_, sut_ports_ids[r], kDefaultInstance, - kOriginalSrcMacAddress)); + const MrifTableEntryParams rif_params = {.egress_port = sut_ports_ids[r], + .instance = kDefaultInstance, + .src_mac = kOriginalSrcMacAddress}; + ASSERT_OK_AND_ASSIGN(const auto rifs, + CreateRifTableEntities(ir_p4info_, rif_params)); rif_entities.insert(rif_entities.end(), rifs.begin(), rifs.end()); } EXPECT_OK(pdpi::InstallPiEntities(sut_p4rt_session_.get(), ir_p4info_, @@ -1489,10 +1883,11 @@ TEST_P(L3MulticastTestFixture, DeleteMulticastGroupWhileInUseFails) { std::vector rif_entities; for (int r = 0; r < kPortsToUseInTest; ++r) { - ASSERT_OK_AND_ASSIGN( - const auto rifs, - CreateRifTableEntities(ir_p4info_, sut_ports_ids[r], kDefaultInstance, - kOriginalSrcMacAddress)); + const MrifTableEntryParams rif_params = {.egress_port = sut_ports_ids[r], + .instance = kDefaultInstance, + .src_mac = kOriginalSrcMacAddress}; + ASSERT_OK_AND_ASSIGN(const auto rifs, + CreateRifTableEntities(ir_p4info_, rif_params)); rif_entities.insert(rif_entities.end(), rifs.begin(), rifs.end()); } EXPECT_OK(pdpi::InstallPiEntities(sut_p4rt_session_.get(), ir_p4info_, @@ -1562,10 +1957,13 @@ TEST_P(L3MulticastTestFixture, AbleToProgramExpectedMulticastRifCapacity) { GetSrcMacForReplica(/*multicast_group_index=*/r, /*replicas_per_group=*/1, /*replicas_number=*/0)); - ASSERT_OK_AND_ASSIGN( - auto rifs, - CreateRifTableEntities(ir_p4info_, port_id, - /*instance=*/r / kPortsToUseInTest, src_mac)); + const MrifTableEntryParams rif_params = { + .egress_port = port_id, + .instance = r / kPortsToUseInTest, + .src_mac = src_mac, + }; + ASSERT_OK_AND_ASSIGN(auto rifs, + CreateRifTableEntities(ir_p4info_, rif_params)); absl::Status add_status = pdpi::InstallPiEntities(sut_p4rt_session_.get(), ir_p4info_, rifs); @@ -1655,19 +2053,126 @@ TEST_P(L3MulticastTestFixture, AbleToProgramExpectedMulticastGroupCapacity) { EXPECT_OK(ClearEntities(*sut_p4rt_session_, ir_p4info_, rif_entities)); } -// TEST_P(L3MulticastTestFixture, PerformanceInitializationTime) { -// GTEST_SKIP() << "Skipping because this test is not implemented yet."; -// } -// TEST_P(L3MulticastTestFixture, PerformanceMulticastGroupAdjustmentRate) { -// GTEST_SKIP() << "Skipping because this test is not implemented yet."; -// } -// TEST_P(L3MulticastTestFixture, PerformanceReplicaArrivalTimeWithFixedDelay) { -// GTEST_SKIP() << "Skipping because this test is not implemented yet."; -// } -// TEST_P(L3MulticastTestFixture, -// PerformanceReplicaArrivalTimeWithUnregisteredParticipants) { -// GTEST_SKIP() << "Skipping because this test is not implemented yet."; -// } +TEST_P(L3MulticastTestFixture, ReplicatePacketWithVlanAndSrcMacRewrite) { + thinkit::MirrorTestbed& testbed = + GetParam().mirror_testbed->GetMirrorTestbed(); + const int kPortsToUseInTest = 3; + // Collect port IDs. + // Get SUT and control ports to test on. + ASSERT_OK_AND_ASSIGN( + const std::vector sut_ports_ids, + GetNUpInterfaceIDs(GetParam().mirror_testbed->GetMirrorTestbed().Sut(), + kPortsToUseInTest + 1)); + + const MulticastForwardingParams test_params = { + .sut_port_ids = sut_ports_ids, + .assignment_mechanism = IpmcGroupAssignmentMechanism::kIpMulticastTable, + .number_replicas_per_group = kPortsToUseInTest, + .number_multicast_groups = 1, + .vlan_forwarding_params = { + .disable_ingress_vlan_checks = true, + .disable_egress_vlan_checks = true, + .input_vlan_id = 0x100, + .input_vlan_tagging_mode = sai::VlanTaggingMode::kTagged, + .out_ports_vlan_id = 0x100, + .out_ports_vlan_tagging_mode = sai::VlanTaggingMode::kTagged, + }}; + // -------------------------------------------------------------------------- + // Add multicast programming. + // -------------------------------------------------------------------------- + LOG(INFO) << "Adding multicast programming."; + std::vector entities_created; + ASSERT_OK(SetupDefaultMulticastProgramming(*sut_p4rt_session_, ir_p4info_, + p4::v1::Update::INSERT, + test_params, entities_created)); + LOG(INFO) << "Added " << entities_created.size() << " entities."; + LOG(INFO) << "Adding VLAN Membership."; + ASSERT_OK(InstallVlanMembershipEntries(*sut_p4rt_session_, ir_p4info_, + test_params.sut_port_ids, + test_params.vlan_forwarding_params)); + // Build test packets. + ASSERT_OK_AND_ASSIGN(auto vectors, BuildTestVectors(test_params)); + // Send test packets. + LOG(INFO) << "Sending traffic to verify added multicast programming."; + dvaas::DataplaneValidationParams dvaas_params = + dvaas::DefaultpinsDataplaneValidationParams(); + // Ensure the port map for the control switch can map to the SUT (for + // situations where the config differs for SUT and control switch). + auto interface_to_peer_entity_map = gtl::ValueOrDie( + pins::ControlP4rtPortIdBySutP4rtPortIdFromSwitchConfig()); + dvaas_params.mirror_testbed_port_map_override = gtl::ValueOrDie( + dvaas::MirrorTestbedP4rtPortIdMap::CreateFromControlSwitchToSutPortMap( + interface_to_peer_entity_map)); + dvaas_params.packet_test_vector_override = vectors; + + ASSERT_OK_AND_ASSIGN( + dvaas::ValidationResult validation_result, + GetParam().dvaas->ValidateDataplane(testbed, dvaas_params)); + // Validate traffic. + validation_result.LogStatistics(); + EXPECT_OK(validation_result.HasSuccessRateOfAtLeast(1.0)); +} + +TEST_P(L3MulticastTestFixture, + SwitchAclRedirectAndReplicatePacketWithVlanAndSrcMacRewrite) { + thinkit::MirrorTestbed& testbed = + GetParam().mirror_testbed->GetMirrorTestbed(); + const int kPortsToUseInTest = 3; + // Collect port IDs. + // Get SUT and control ports to test on. + ASSERT_OK_AND_ASSIGN( + const std::vector sut_ports_ids, + GetNUpInterfaceIDs(GetParam().mirror_testbed->GetMirrorTestbed().Sut(), + kPortsToUseInTest + 1)); + + const MulticastForwardingParams test_params = { + .sut_port_ids = sut_ports_ids, + .assignment_mechanism = IpmcGroupAssignmentMechanism::kAclRedirect, + .number_replicas_per_group = kPortsToUseInTest, + .number_multicast_groups = 1, + .vlan_forwarding_params = { + .disable_ingress_vlan_checks = true, + .disable_egress_vlan_checks = true, + .input_vlan_id = 0x100, + .input_vlan_tagging_mode = sai::VlanTaggingMode::kTagged, + .out_ports_vlan_id = 0x100, + .out_ports_vlan_tagging_mode = sai::VlanTaggingMode::kTagged, + }}; + // -------------------------------------------------------------------------- + // Add multicast programming. + // -------------------------------------------------------------------------- + LOG(INFO) << "Adding multicast programming."; + std::vector entities_created; + ASSERT_OK(SetupDefaultMulticastProgramming(*sut_p4rt_session_, ir_p4info_, + p4::v1::Update::INSERT, + test_params, entities_created)); + LOG(INFO) << "Added " << entities_created.size() << " entities."; + LOG(INFO) << "Adding VLAN Membership."; + ASSERT_OK(InstallVlanMembershipEntries(*sut_p4rt_session_, ir_p4info_, + sut_ports_ids, + test_params.vlan_forwarding_params)); + // Build test packets. + ASSERT_OK_AND_ASSIGN(auto vectors, BuildTestVectors(test_params)); + // Send test packets. + LOG(INFO) << "Sending traffic to verify added multicast programming."; + dvaas::DataplaneValidationParams dvaas_params = + dvaas::DefaultpinsDataplaneValidationParams(); + // Ensure the port map for the control switch can map to the SUT (for + // situations where the config differs for SUT and control switch). + auto interface_to_peer_entity_map = gtl::ValueOrDie( + pins::ControlP4rtPortIdBySutP4rtPortIdFromSwitchConfig()); + dvaas_params.mirror_testbed_port_map_override = gtl::ValueOrDie( + dvaas::MirrorTestbedP4rtPortIdMap::CreateFromControlSwitchToSutPortMap( + interface_to_peer_entity_map)); + dvaas_params.packet_test_vector_override = vectors; + + ASSERT_OK_AND_ASSIGN( + dvaas::ValidationResult validation_result, + GetParam().dvaas->ValidateDataplane(testbed, dvaas_params)); + // Validate traffic. + validation_result.LogStatistics(); + EXPECT_OK(validation_result.HasSuccessRateOfAtLeast(1.0)); +} } // namespace void L3MulticastTestFixture::SetUp() { diff --git a/tests/qos/qos_test_util.cc b/tests/qos/qos_test_util.cc index 21b2cc133..6f0b9a9b3 100644 --- a/tests/qos/qos_test_util.cc +++ b/tests/qos/qos_test_util.cc @@ -632,7 +632,7 @@ absl::Status EffectivelyDisablePuntLimitsForSwitch( break; } } - std::string switch_role_name = SwtichRoleToDisableQoSToString(role); + std::string switch_role_name = SwitchRoleToDisableQoSToString(role); ASSIGN_OR_RETURN(auto gnmi_config_before_qos_and_buffer_change, pins_test::GetGnmiConfig(*gnmi_stub)); diff --git a/tests/qos/qos_test_util.h b/tests/qos/qos_test_util.h index 31a2c7992..f87ef528b 100644 --- a/tests/qos/qos_test_util.h +++ b/tests/qos/qos_test_util.h @@ -23,6 +23,7 @@ #include "absl/status/statusor.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" +#include "absl/time/time.h" #include "lib/gnmi/gnmi_helper.h" #include "lib/gnmi/openconfig.pb.h" #include "proto/gnmi/gnmi.grpc.pb.h" @@ -44,7 +45,7 @@ enum class SwitchRoleToDisablePuntFlowQoS { kSwitchUnderTest, }; -inline std::string SwtichRoleToDisableQoSToString( +inline std::string SwitchRoleToDisableQoSToString( SwitchRoleToDisablePuntFlowQoS role) { switch (role) { case SwitchRoleToDisablePuntFlowQoS::kControlSwitch: From a64d1fdb40794f75d32ff617603b82035f377937 Mon Sep 17 00:00:00 2001 From: kishanps Date: Mon, 7 Oct 2024 12:44:37 -0700 Subject: [PATCH 5/7] Adding of P4RT component validator and verifying to make sure the CPU punt on SUT switch works before enabling the sflow tests, to ensure FuzzNonKeyFields return a Status. --- p4_fuzzer/fuzz_util.cc | 86 ++++++------ p4_fuzzer/fuzzer_config.h | 12 +- .../instantiations/google/tests/BUILD.bazel | 2 +- .../tests/p4_fuzzer_integration_test.cc | 13 +- tests/forwarding/fuzzer_tests.cc | 5 + tests/forwarding/l3_multicast_test.cc | 10 +- tests/forwarding/tunnel_decap_test.cc | 12 +- .../nsf/component_validators/BUILD.bazel | 13 +- .../component_validators/p4rt_validator.cc | 127 ++++++++++++++++++ .../nsf/component_validators/p4rt_validator.h | 36 +++++ .../component_validators/swss_validator.cc | 14 +- ...rrent_config_push_flow_programming_test.cc | 8 +- tests/integration/system/nsf/util.h | 2 + tests/sflow/sflow_test.cc | 54 ++++++++ 14 files changed, 320 insertions(+), 74 deletions(-) create mode 100644 tests/integration/system/nsf/component_validators/p4rt_validator.cc create mode 100644 tests/integration/system/nsf/component_validators/p4rt_validator.h diff --git a/p4_fuzzer/fuzz_util.cc b/p4_fuzzer/fuzz_util.cc index bafba8d2c..7d0a39745 100644 --- a/p4_fuzzer/fuzz_util.cc +++ b/p4_fuzzer/fuzz_util.cc @@ -716,9 +716,9 @@ absl::StatusOr FuzzReplica(absl::BitGen* gen, // Randomly changes the `multicast_group_entry` without affecting the key // fields. -void FuzzNonKeyFields(absl::BitGen* gen, const FuzzerConfig& config, - const SwitchState& switch_state, - MulticastGroupEntry* multicast_group_entry) { +absl::Status FuzzNonKeyFields(absl::BitGen* gen, const FuzzerConfig& config, + const SwitchState& switch_state, + MulticastGroupEntry* multicast_group_entry) { multicast_group_entry->clear_replicas(); // Fuzz 0-8 unique replicas // TODO: - Replace how iteration count is chosen with information @@ -730,27 +730,27 @@ void FuzzNonKeyFields(absl::BitGen* gen, const FuzzerConfig& config, // replicas generated can be less than `replica_fuzz_iterations`. for (int i = 0; i < replica_fuzz_iterations; ++i) { // Generate replica. - absl::StatusOr replica = - FuzzReplica(gen, config, switch_state, - config.GetIrP4Info().built_in_tables().at( - pdpi::GetMulticastGroupTableName())); + ASSIGN_OR_RETURN(Replica replica, + FuzzReplica(gen, config, switch_state, + config.GetIrP4Info().built_in_tables().at( + pdpi::GetMulticastGroupTableName()))); // Within a given multicast entry, replicas must be unique. - if (replica.ok() && - unique_replicas - .insert(std::make_pair(replica->instance(), replica->port())) + if (unique_replicas + .insert(std::make_pair(replica.instance(), replica.port())) .second) { - *multicast_group_entry->add_replicas() = std::move(*replica); + *multicast_group_entry->add_replicas() = std::move(replica); } } + return absl::OkStatus(); } // Randomly changes the table_entry, without affecting the key fields. -void FuzzNonKeyFields(absl::BitGen* gen, const FuzzerConfig& config, - const SwitchState& switch_state, - TableEntry* table_entry) { +absl::Status FuzzNonKeyFields(absl::BitGen* gen, const FuzzerConfig& config, + const SwitchState& switch_state, + TableEntry* table_entry) { // With some probability, don't modify the element at all. - if (absl::Bernoulli(*gen, 0.2)) return; + if (absl::Bernoulli(*gen, 0.2)) return absl::OkStatus(); if (absl::Bernoulli(*gen, 0.25)) { if (absl::Bernoulli(*gen, 0.5)) { @@ -758,17 +758,16 @@ void FuzzNonKeyFields(absl::BitGen* gen, const FuzzerConfig& config, // IsWellformedUpdate // will think the update is no longer well-formed. } else { - if (auto table_definition = gutil::FindOrStatus( - config.GetIrP4Info().tables_by_id(), table_entry->table_id()); - table_definition.ok()) { - // Try up to 3 times to create a new action. - for (int i = 0; i < 3; ++i) { - if (auto action = - FuzzAction(gen, config, switch_state, *table_definition); - action.ok()) { - *table_entry->mutable_action() = *action; - break; - } + ASSIGN_OR_RETURN(auto table_definition, + gutil::FindOrStatus(config.GetIrP4Info().tables_by_id(), + table_entry->table_id())); + // Try up to 3 times to create a new action. + for (int i = 0; i < 3; ++i) { + if (auto action = + FuzzAction(gen, config, switch_state, table_definition); + action.ok()) { + *table_entry->mutable_action() = *action; + break; } } } @@ -778,31 +777,29 @@ void FuzzNonKeyFields(absl::BitGen* gen, const FuzzerConfig& config, if (absl::Bernoulli(*gen, 0.25)) { if (absl::Bernoulli(*gen, 0.5)) { table_entry->clear_metadata(); - } else { - // TODO: Currently, we only want to use metadata that looks - // similar to our expectation. Eventually, we should fuzz just a random ID - // with e.g. `FuzzRandomId(gen)`. - table_entry->set_metadata( - absl::StrCat("orion_cookie: ", FuzzUint64(gen, /*bits=*/64))); } } // TODO: also fuzz meters + return absl::OkStatus(); } // Randomly changes the entity, without affecting the key fields. -void FuzzNonKeyFields(absl::BitGen* gen, const FuzzerConfig& config, - const SwitchState& switch_state, Entity* entity) { +absl::Status FuzzNonKeyFields(absl::BitGen* gen, const FuzzerConfig& config, + const SwitchState& switch_state, Entity* entity) { // This function only supports table entries and multicast group entries. CHECK(entity->has_table_entry() || // Crash OK entity->packet_replication_engine_entry().has_multicast_group_entry()); if (entity->has_table_entry()) { - FuzzNonKeyFields(gen, config, switch_state, entity->mutable_table_entry()); + RETURN_IF_ERROR(FuzzNonKeyFields(gen, config, switch_state, + entity->mutable_table_entry())); } if (entity->packet_replication_engine_entry().has_multicast_group_entry()) { - FuzzNonKeyFields(gen, config, switch_state, - entity->mutable_packet_replication_engine_entry() - ->mutable_multicast_group_entry()); + RETURN_IF_ERROR( + FuzzNonKeyFields(gen, config, switch_state, + entity->mutable_packet_replication_engine_entry() + ->mutable_multicast_group_entry())); } + return absl::OkStatus(); } // Generates `WeightedItems` for all valid table_ids where weight is equal to @@ -979,7 +976,13 @@ AnnotatedUpdate FuzzUpdate(absl::BitGen* gen, const FuzzerConfig& config, *entity.mutable_table_entry() = UniformValueFromMap(gen, switch_state.GetTableEntries(table_id)); } - FuzzNonKeyFields(gen, config, switch_state, &entity); + if (!FuzzNonKeyFields(gen, config, switch_state, &entity).ok()) { + // Retry. + return FuzzUpdate(gen, config, switch_state); + } + // TODO: After this we may need to apply the ModifyFuzzedX + // functions to ensure that modified entities have their non-key fields + // correctly modified. break; } default: @@ -1528,7 +1531,7 @@ absl::StatusOr FuzzValidMulticastGroupEntry( } // Fills in replicas randomly. - FuzzNonKeyFields(gen, config, switch_state, &entry); + RETURN_IF_ERROR(FuzzNonKeyFields(gen, config, switch_state, &entry)); return entry; }; @@ -1592,9 +1595,6 @@ absl::StatusOr FuzzValidTableEntry( FuzzAction(gen, config, switch_state, ir_table_info), _.SetPrepend() << "while fuzzing action: "); - // Set cookie and priority. - table_entry.set_metadata( - absl::StrCat("orion_cookie: ", FuzzUint64(gen, /*bits=*/64))); if (ir_table_info.requires_priority()) { uint64_t priority; do { diff --git a/p4_fuzzer/fuzzer_config.h b/p4_fuzzer/fuzzer_config.h index 385e81844..976fcaee3 100644 --- a/p4_fuzzer/fuzzer_config.h +++ b/p4_fuzzer/fuzzer_config.h @@ -52,13 +52,13 @@ class FuzzerConfig; struct ConfigParams { // -- Required --------------------------------------------------------------- // NOTE: These values are required for correct function. All of them are - // initialized to values that should usually work for GPINs switches. + // initialized to values that should usually work for PINs switches. // --------------------------------------------------------------------------- - // The set of valid port names. 1 tends to be mapped on most GPINs switches. + // The set of valid port names. 1 tends to be mapped on most PINs switches. std::vector ports = pins_test::P4rtPortId::MakeVectorFromOpenConfigEncodings({1}); // The set of valid QOS queues. CONTROLLER_PRIORITY_5 tends to be mapped on - // most GPINs switches. + // most PINs switches. std::vector qos_queues = {"CONTROLLER_PRIORITY_5"}; // The P4RT role the fuzzer should use. std::string role = "sdn_controller"; @@ -68,7 +68,7 @@ struct ConfigParams { // field. float match_field_wildcard_probability = 0.05; // The probability of fuzzing a multicast group entry when fuzzing an update. - // TODO: b/316926338 - Remove once switch state transitions to entities and + // Remove once switch state transitions to entities and // fuzzer can use weighted distribution for multicast. float fuzz_multicast_group_entry_probability = 0; @@ -214,7 +214,7 @@ class FuzzerConfig { ConfigParams params_; - // TODO: b/276461175 - Support P4RT translated types. + // Support P4RT translated types. // Checks the following assumptions made about p4 constraints that aren't // marked as ignored in `params_`: // 1) No constraint includes a match field that has a P4Runtime translated @@ -222,7 +222,7 @@ class FuzzerConfig { // on this type and EXACT fields are required, so this combination is // forbidden. // Also logs the following information: - // TODO: b/324083270 - Remove once references+constraints are handled. + // Remove once references+constraints are handled. // 1) A field that is both constrained and has a reference. The fuzzer will // choose to satisfy references over constraints, which means the resulting // entry may not satisfy constraints. This is a current weakness of the diff --git a/sai_p4/instantiations/google/tests/BUILD.bazel b/sai_p4/instantiations/google/tests/BUILD.bazel index b42774ba0..93f03dea3 100644 --- a/sai_p4/instantiations/google/tests/BUILD.bazel +++ b/sai_p4/instantiations/google/tests/BUILD.bazel @@ -54,7 +54,7 @@ cc_test( "@com_github_p4lang_p4_constraints//p4_constraints/backend:interpreter", "@com_github_p4lang_p4runtime//:p4info_cc_proto", "@com_github_p4lang_p4runtime//:p4runtime_cc_proto", - "@com_google_absl//absl/container:flat_hash_set", + "@com_google_absl//absl/log", "@com_google_absl//absl/random", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", diff --git a/sai_p4/instantiations/google/tests/p4_fuzzer_integration_test.cc b/sai_p4/instantiations/google/tests/p4_fuzzer_integration_test.cc index 91f16b686..bf587d02e 100644 --- a/sai_p4/instantiations/google/tests/p4_fuzzer_integration_test.cc +++ b/sai_p4/instantiations/google/tests/p4_fuzzer_integration_test.cc @@ -14,10 +14,9 @@ // ============================================================================= // Integration test between SAI-P4 and the P4-Fuzzer library. -#include #include -#include "absl/container/flat_hash_set.h" +#include "absl/log/log.h" #include "absl/random/random.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" @@ -68,6 +67,16 @@ absl::Status AddReferenceableEntries(const FuzzerConfig& config, referable_entities.push_back(mirror_session_entity); } + ASSIGN_OR_RETURN(pdpi::IrTableDefinition multicast_router_interface_table, + gutil::FindOrStatus(config.GetIrP4Info().tables_by_name(), + "multicast_router_interface_table")); + p4::v1::Update mrif_update; + mrif_update.set_type(p4::v1::Update::INSERT); + ASSIGN_OR_RETURN(*mrif_update.mutable_entity()->mutable_table_entry(), + FuzzValidTableEntry(&gen, config, switch_state, + multicast_router_interface_table)); + RETURN_IF_ERROR(switch_state.ApplyUpdate(mrif_update)); + p4::v1::Entity multicast_group_entity; ASSIGN_OR_RETURN( *multicast_group_entity.mutable_packet_replication_engine_entry() diff --git a/tests/forwarding/fuzzer_tests.cc b/tests/forwarding/fuzzer_tests.cc index 9ad35dde4..63401eb78 100644 --- a/tests/forwarding/fuzzer_tests.cc +++ b/tests/forwarding/fuzzer_tests.cc @@ -435,6 +435,11 @@ TEST_P(FuzzerTestFixture, P4rtWriteAndCheckNoInternalErrors) { << " seconds."; } + ASSERT_OK_AND_ASSIGN(std::vector table_entries, + pdpi::ReadPiTableEntries(session.get())); + EXPECT_OK(state.AssertEntriesAreEqualToState( + table_entries, config.GetTreatAsEqualDuringReadDueToKnownBug())); + LOG(INFO) << "Finished " << iteration << " iterations."; LOG(INFO) << " num_updates: " << num_updates; // Expected value is 50, so if it's very far from that, we probably have a diff --git a/tests/forwarding/l3_multicast_test.cc b/tests/forwarding/l3_multicast_test.cc index 8393a8695..2dcb633bf 100644 --- a/tests/forwarding/l3_multicast_test.cc +++ b/tests/forwarding/l3_multicast_test.cc @@ -364,8 +364,9 @@ absl::Status SetupDefaultMulticastProgramming( sai::EntryBuilder() .AddEntryAdmittingAllPacketsToL3() .AddVrfEntry(kDefaultMulticastVrf) - .AddPreIngressAclEntryAssigningVrfForGivenIpType( - kDefaultMulticastVrf, sai::IpVersion::kIpv4And6) + .AddPreIngressAclTableEntry( + kDefaultMulticastVrf, + sai::AclPreIngressMatchFields{.is_ip = true}) .LogPdEntries() .GetDedupedPiEntities(ir_p4info)); @@ -2053,7 +2054,8 @@ TEST_P(L3MulticastTestFixture, AbleToProgramExpectedMulticastGroupCapacity) { EXPECT_OK(ClearEntities(*sut_p4rt_session_, ir_p4info_, rif_entities)); } -TEST_P(L3MulticastTestFixture, ReplicatePacketWithVlanAndSrcMacRewrite) { +TEST_P(L3MulticastTestFixture, + DISABLED_ReplicatePacketWithVlanAndSrcMacRewrite) { thinkit::MirrorTestbed& testbed = GetParam().mirror_testbed->GetMirrorTestbed(); const int kPortsToUseInTest = 3; @@ -2114,7 +2116,7 @@ TEST_P(L3MulticastTestFixture, ReplicatePacketWithVlanAndSrcMacRewrite) { } TEST_P(L3MulticastTestFixture, - SwitchAclRedirectAndReplicatePacketWithVlanAndSrcMacRewrite) { + DISABLED_SwitchAclRedirectAndReplicatePacketWithVlanAndSrcMacRewrite) { thinkit::MirrorTestbed& testbed = GetParam().mirror_testbed->GetMirrorTestbed(); const int kPortsToUseInTest = 3; diff --git a/tests/forwarding/tunnel_decap_test.cc b/tests/forwarding/tunnel_decap_test.cc index 0efea3a3f..3fe29d285 100644 --- a/tests/forwarding/tunnel_decap_test.cc +++ b/tests/forwarding/tunnel_decap_test.cc @@ -398,8 +398,8 @@ absl::Status InstallL3Route(pdpi::P4RuntimeSession& switch_session, sai::EntryBuilder entry_builder = sai::EntryBuilder() .AddVrfEntry("vrf-1") - .AddPreIngressAclEntryAssigningVrfForGivenIpType( - "vrf-1", sai::IpVersion::kIpv6) + .AddPreIngressAclTableEntry( + "vrf-1", sai::AclPreIngressMatchFields{.is_ipv6 = true}) .AddDefaultRouteForwardingAllPacketsToGivenPort(given_port, ip_version, "vrf-1"); if (l3_admit) { @@ -504,7 +504,7 @@ TEST_P(TunnelDecapTestFixture, BasicTunnelTermDecapv4Inv6) { GetParam().tunnel_type)); LOG(INFO) << "Sending IPv4-in-IPv6 Packet from ingress:" << in_port - << " to engress:" << out_port; + << " to egress:" << out_port; // Send IPv4-in-IPv6 Packet and Verify positive testcase dst-ipv6 is matching pins_test::TunnelDecapTestVectorParams tunnel_v6_match{ .in_port = in_port, @@ -659,7 +659,7 @@ TEST_P(TunnelDecapTestFixture, BasicTunnelTermDecapNoAdmit) { GetParam().tunnel_type)); LOG(INFO) << "Sending IPv4-in-IPv6 Packet from ingress:" << in_port - << " to engress:" << out_port; + << " to egress:" << out_port; // Send IPv4-in-IPv6 Packet and verify that packet is getting // dropped as l3-admit is not configured pins_test::TunnelDecapTestVectorParams tunnel_v6_match{ @@ -731,7 +731,7 @@ TEST_P(TunnelDecapTestFixture, NoAdmitTableAclRedirectTunnelTermNoDecapForV4) { GetParam().tunnel_type)); LOG(INFO) << "Sending IPv4-in-IPv6 Packet from ingress:" << in_port - << " to engress:" << out_port; + << " to egress:" << out_port; // Send IPv4-in-IPv6 Packet and Verify positive testcase dst-ipv6 is matching pins_test::TunnelDecapTestVectorParams tunnel_v6_match{ .in_port = in_port, @@ -797,7 +797,7 @@ TEST_P(TunnelDecapTestFixture, AdmitTableAclRedirectTunnelTermDecapForV4) { GetParam().tunnel_type)); LOG(INFO) << "Sending IPv4-in-IPv6 Packet from ingress:" << in_port - << " to engress:" << out_port; + << " to egress:" << out_port; // Send IPv4-in-IPv6 Packet and Verify positive testcase dst-ipv6 is matching pins_test::TunnelDecapTestVectorParams tunnel_v6_match{ .in_port = in_port, diff --git a/tests/integration/system/nsf/component_validators/BUILD.bazel b/tests/integration/system/nsf/component_validators/BUILD.bazel index 26080eaa8..36aa4e0dc 100644 --- a/tests/integration/system/nsf/component_validators/BUILD.bazel +++ b/tests/integration/system/nsf/component_validators/BUILD.bazel @@ -24,22 +24,17 @@ package( ) cc_library( - name = "platform_validator", + name = "p4rt_validator", testonly = True, - srcs = ["platform_validator.cc"], - hdrs = ["platform_validator.h"], + srcs = ["p4rt_validator.cc"], + hdrs = ["p4rt_validator.h"], deps = [ - "//gutil:status", - "//lib/gnmi:gnmi_helper", "//tests/integration/system/nsf:util", "//tests/integration/system/nsf/interfaces:component_validator", "//tests/integration/system/nsf/interfaces:testbed", "//thinkit:ssh_client", - "//thinkit:switch", - "@com_github_gnmi//proto/gnmi:gnmi_cc_grpc_proto", - "@com_github_google_glog//:glog", - "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/container:flat_hash_set", + "@com_google_absl//absl/log", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:string_view", diff --git a/tests/integration/system/nsf/component_validators/p4rt_validator.cc b/tests/integration/system/nsf/component_validators/p4rt_validator.cc new file mode 100644 index 000000000..ff298e86b --- /dev/null +++ b/tests/integration/system/nsf/component_validators/p4rt_validator.cc @@ -0,0 +1,127 @@ +// Copyright 2025 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#include "tests/integration/system/nsf/component_validators/p4rt_validator.h" + +#include + +#include "absl/container/flat_hash_set.h" +#include "absl/log/log.h" +#include "absl/status/status.h" +#include "absl/strings/ascii.h" +#include "absl/strings/match.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/str_split.h" +#include "absl/strings/string_view.h" +#include "absl/strings/strip.h" +#include "absl/strings/substitute.h" +#include "absl/time/time.h" +#include "gutil/status.h" +#include "tests/integration/system/nsf/interfaces/testbed.h" +#include "tests/integration/system/nsf/util.h" +#include "thinkit/ssh_client.h" + +namespace pins_test { +namespace { + +constexpr absl::string_view k2SeptemberRelease = "pins_release_20240902"; +constexpr char kSyslogFile[] = "/var/log/tmp/sandcastle.log"; +constexpr char kGrepAclRecarvedTablesCmd[] = + "grep \'Removing modified nonessential ACL table\' $0 | cut -d ' ' -f 16"; +constexpr absl::Duration kRunCommandTimeout = absl::Minutes(1); + +absl::Status VerifyAclRecarvingTelemetry(absl::string_view version, + const Testbed& testbed, + thinkit::SSHClient& ssh_client) { + ASSIGN_OR_RETURN(auto sut_gnmi_stub, GetSut(testbed).CreateGnmiStub()); + ASSIGN_OR_RETURN(PinsSoftwareComponentInfo software_info, + GetPinsSoftwareComponentInfo(*sut_gnmi_stub)); + + if (software_info.primary_network_stack.version == + software_info.secondary_network_stack.version) { + // There shouldn't be any ACL recarving when a config is pushed between same + // versions of NSF upgrade. + return absl::OkStatus(); + } + + // Add release specific objects in the allow list. + absl::flat_hash_set allowlist; + if (absl::StrContains(software_info.secondary_network_stack.version, + k2SeptemberRelease)) { + // NSF upgrading from 2 september release --> new image. + allowlist = absl::flat_hash_set({ + "acl_ingress_qos_table", + "acl_ingress_table", + }); + } + + std::string allowlist_str; + for (const auto obj : allowlist) { + absl::StrAppend(&allowlist_str, obj, ", "); + } + if (!allowlist.empty()) { + LOG(INFO) << absl::Substitute("ACL recarving allow list for $0 -> $1: $2", + software_info.secondary_network_stack.version, + software_info.primary_network_stack.version, + allowlist_str); + } + + // TODO: Replace fetching recarved ACL tables from syslog via SSH + // with blackbox interfaces. + // Fetch recarved ACL tables from syslog. + ASSIGN_OR_RETURN(auto recarved_acl_tables, + ssh_client.RunCommand( + GetSut(testbed).ChassisName(), + absl::Substitute(kGrepAclRecarvedTablesCmd, kSyslogFile), + kRunCommandTimeout)); + absl::StripAsciiWhitespace(&recarved_acl_tables); + if (recarved_acl_tables.empty()) { + // No ACL recarving occurred during config push. + return absl::OkStatus(); + } + LOG(INFO) << "Recarved ACL tables: " << recarved_acl_tables; + + std::string error_msg; + for (absl::string_view acl_table : + absl::StrSplit(recarved_acl_tables, '\n')) { + // ACL tables returned from syslog are enclosed within single quotes. For + // example: 'acl_ingress_table', 'acl_ingress_qos_table' etc. + absl::ConsumePrefix(&acl_table, "'"); + absl::ConsumeSuffix(&acl_table, "'"); + if (acl_table.empty()) { + continue; + } + if (!allowlist.contains(acl_table)) { + absl::StrAppend( + &error_msg, + absl::Substitute( + "Recarved ACL table $0 is not in the allow list: $1\n", acl_table, + allowlist_str)); + } + } + if (!error_msg.empty()) { + LOG(ERROR) << "ACL recarving validation failed: " << error_msg; + return absl::InternalError(error_msg); + } + return absl::OkStatus(); +} + +} // namespace + +absl::Status P4rtValidator::OnConfigPush(absl::string_view version, + const Testbed& testbed, + thinkit::SSHClient& ssh_client) { + return VerifyAclRecarvingTelemetry(version, testbed, ssh_client); +} + +} // namespace pins_test diff --git a/tests/integration/system/nsf/component_validators/p4rt_validator.h b/tests/integration/system/nsf/component_validators/p4rt_validator.h new file mode 100644 index 000000000..9f28570e6 --- /dev/null +++ b/tests/integration/system/nsf/component_validators/p4rt_validator.h @@ -0,0 +1,36 @@ +// Copyright 2025 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef PINS_TESTS_INTEGRATION_SYSTEM_NSF_COMPONENT_VALIDATORS_P4RT_VALIDATOR_H_ +#define PINS_TESTS_INTEGRATION_SYSTEM_NSF_COMPONENT_VALIDATORS_P4RT_VALIDATOR_H_ + +#include "absl/container/flat_hash_set.h" +#include "absl/log/log.h" +#include "absl/status/status.h" +#include "absl/strings/string_view.h" +#include "tests/integration/system/nsf/interfaces/component_validator.h" +#include "tests/integration/system/nsf/interfaces/testbed.h" +#include "thinkit/ssh_client.h" + +namespace pins_test { + +class P4rtValidator : public ComponentValidator { + public: + absl::Status OnConfigPush(absl::string_view version, const Testbed& testbed, + thinkit::SSHClient& ssh_client) override; +}; + +} // namespace pins_test + +#endif // PINS_TESTS_INTEGRATION_SYSTEM_NSF_COMPONENT_VALIDATORS_P4RT_VALIDATOR_H_ diff --git a/tests/integration/system/nsf/component_validators/swss_validator.cc b/tests/integration/system/nsf/component_validators/swss_validator.cc index a6f3c2ffa..dfddf7755 100644 --- a/tests/integration/system/nsf/component_validators/swss_validator.cc +++ b/tests/integration/system/nsf/component_validators/swss_validator.cc @@ -35,6 +35,7 @@ namespace pins_test { namespace { constexpr absl::string_view k31JulyRelease = "pins_release_20240731"; +constexpr absl::string_view k02SeptRelease = "pins_release_20240902"; // The following objects will always be modified after APPLY_VIEW. const absl::flat_hash_set kAllowList = @@ -190,7 +191,18 @@ absl::Status SwssValidator::OnImageCopy(absl::string_view version, "SAI_OBJECT_TYPE_HOSTIF_TABLE_ENTRY", }); } - + if (absl::StrContains(software_info.primary_network_stack.version, + k02SeptRelease) && + software_info.primary_network_stack.version != + software_info.secondary_network_stack.version) { + LOG(INFO) << "Allowing additional APPLY_VIEW object operations during NSF " + "upgrade from " + << k02SeptRelease << " to " + << software_info.secondary_network_stack.version; + allowlist_ = absl::flat_hash_set({ + "SAI_OBJECT_TYPE_SWITCH", + }); + } return absl::OkStatus(); } diff --git a/tests/integration/system/nsf/nsf_concurrent_config_push_flow_programming_test.cc b/tests/integration/system/nsf/nsf_concurrent_config_push_flow_programming_test.cc index 82211b4d4..df6e42df8 100644 --- a/tests/integration/system/nsf/nsf_concurrent_config_push_flow_programming_test.cc +++ b/tests/integration/system/nsf/nsf_concurrent_config_push_flow_programming_test.cc @@ -49,8 +49,8 @@ constexpr int kIsolatedLacpSystemPriority = 512; // 2. Config push takes ~10s to complete. Hence the maximum delay is set to 15s. // In future, if the config or the flows are modified, consider calculating the // time needed for both and set the NSF delay duration window accordingly. -constexpr int kMinNsfDelayDuration = 1; -constexpr int kMaxNsfDelayDuration = 15; +constexpr int kMinNsfDelayDuration = 10; +constexpr int kMaxNsfDelayDuration = 20; constexpr absl::Duration kTurnUpTimeout = absl::Minutes(6); constexpr char kInterfaceToRemove[] = "Ethernet1/10/1"; constexpr int kMaxGnmiGetClients = 15; @@ -152,6 +152,10 @@ TEST_P(NsfConcurrentConfigPushFlowProgrammingTestFixture, absl::Status flow_programming_status = absl::UnknownError("Yet to program flows"); auto flow_programming_func = [&sut, &image_config_param]() -> absl::Status { + // ACL flows used in the test takes ~1s to get programmed. + if (kMinNsfDelayDuration > 1) { + absl::SleepFor(absl::Seconds(kMinNsfDelayDuration - 1)); + } LOG(INFO) << "Programming ACL flows"; RETURN_IF_ERROR(ProgramAclFlows(sut, image_config_param.p4_info)); LOG(INFO) << "Successfully programmed ACL flows on " << sut.ChassisName(); diff --git a/tests/integration/system/nsf/util.h b/tests/integration/system/nsf/util.h index e9d2322a7..998a7be4f 100644 --- a/tests/integration/system/nsf/util.h +++ b/tests/integration/system/nsf/util.h @@ -21,6 +21,8 @@ #include #include "absl/base/nullability.h" +#include "absl/container/flat_hash_map.h" +#include "absl/container/flat_hash_set.h" #include "absl/flags/declare.h" #include "absl/status/status.h" #include "absl/status/statusor.h" diff --git a/tests/sflow/sflow_test.cc b/tests/sflow/sflow_test.cc index 16560e37f..d0f452392 100644 --- a/tests/sflow/sflow_test.cc +++ b/tests/sflow/sflow_test.cc @@ -146,6 +146,9 @@ constexpr absl::string_view kSrcIp6Address = "2001:db8:0:12::1"; // This could be parameterized in future if this is platform dependent. constexpr double kTolerance = 0.20; +// Thredshold percentage of packets punted to CPU to be considered passing +constexpr int kPassingPercentageCpuPunted = 80; + // Vrf prefix used in the test. constexpr absl::string_view kVrfIdPrefix = "vrf-"; @@ -3666,6 +3669,8 @@ TEST_P(SflowMirrorTestFixture, TestIp2MePacketsAreSampledAndPunted) { thinkit::MirrorTestbed& testbed = GetParam().testbed_interface->GetMirrorTestbed(); + LOG(INFO) << "Starting TestIp2MePacketsAreSampledAndPunted test"; + const absl::string_view kSrcIp6Address = "2001:db8:0:12::1"; const int packets_num = 3000; // Use 100 for traffic speed since we want to verify the punted packets number @@ -3678,6 +3683,11 @@ TEST_P(SflowMirrorTestFixture, TestIp2MePacketsAreSampledAndPunted) { auto port_id_per_port_name, pins_test::GetAllUpInterfacePortIdsByName(*sut_gnmi_stub_)); ASSERT_GE(port_id_per_port_name.size(), 0) << "No up interfaces."; + + LOG(INFO) << "Up interface numbers: " << port_id_per_port_name.size(); + LOG(INFO) << "Traffic port on SUT: " << traffic_port.interface_name << " / " + << traffic_port.port_id; + ASSERT_OK_AND_ASSIGN( auto control_switch_port_id_per_port_name, pins_test::GetAllUpInterfacePortIdsByName(*control_gnmi_stub_)); @@ -3685,6 +3695,8 @@ TEST_P(SflowMirrorTestFixture, TestIp2MePacketsAreSampledAndPunted) { auto control_switch_port_id, GetPortIdFromInterfaceName(control_switch_port_id_per_port_name, traffic_port.interface_name)); + LOG(INFO) << "Traffic port on control: " << traffic_port.interface_name + << " / " << control_switch_port_id; absl::flat_hash_map sflow_enabled_interfaces; for (const auto& [interface_name, unused] : port_id_per_port_name) { @@ -3693,6 +3705,48 @@ TEST_P(SflowMirrorTestFixture, TestIp2MePacketsAreSampledAndPunted) { std::vector> collector_address_and_port{ {kLocalLoopbackIpv6, collector_port_}}; + + // Before starting Sflow testing, make sure punting packets to CPU works. + bool verify_first = true; + if (verify_first) { + LOG(INFO) << "Start verifying punting packets to CPU works"; + Counters initial_cpu_counter; + ASSERT_OK_AND_ASSIGN(initial_cpu_counter, + ReadCounters("CPU", sut_gnmi_stub_.get())); + absl::Time start_time = absl::Now(); + auto control_switch_port = Port{ + .interface_name = traffic_port.interface_name, + .port_id = control_switch_port_id, + }; + ASSERT_OK(SendNSshPacketsFromSwitch( + packets_num, traffic_speed, control_switch_port, kSrcIp6Address, + agent_address_, sut_gnmi_stub_.get(), GetControlIrP4Info(), + *control_p4_session_, testbed.Environment())); + + // Display the difference for CPU counter during test dev. + ASSERT_OK_AND_ASSIGN(auto final_cpu_counter, + ReadCounters("CPU", sut_gnmi_stub_.get())); + auto delta = DeltaCounters(initial_cpu_counter, final_cpu_counter); + LOG(INFO) << "Ingress Deltas (CPU):\n total time: " + << (absl::Now() - start_time); + ShowCounters(delta); + LOG(INFO) << "End verifying punting packets to CPU works"; + // Make sure at least a certain percent of packets are punted to CPU. + if (delta.in_pkts < packets_num * kPassingPercentageCpuPunted / 100) { + GTEST_SKIP() + << "The SUT switch does not punt expected number of packets to CPU." + << " Expected: " << packets_num << ". Actual: " << delta.in_pkts; + } + } + + // Sflow testing starting from here. + LOG(INFO) << "Agent_address: " << agent_address_ + << " collector_address_and_port: " << kLocalLoopbackIpv6 << "/" + << collector_port_ << " sflow_enabled_interfaces size: " + << sflow_enabled_interfaces.size() + << " kInbandSamplingRate:" << kInbandSamplingRate + << " kSampleHeaderSize: " << kSampleHeaderSize; + ASSERT_OK_AND_ASSIGN( auto sut_gnmi_config_with_sflow, UpdateSflowConfig(GetParam().sut_gnmi_config, agent_address_, From 82a222f6bd09ee054e7ce3e051e90940ef6fdde5 Mon Sep 17 00:00:00 2001 From: jonathan-dilorenzo Date: Wed, 25 Sep 2024 10:10:40 -0700 Subject: [PATCH 6/7] Restore gNMI interfaces to their original state at the end of tests and Attempt to modify every installed entity if it supports modification. --- p4_fuzzer/fuzz_util.cc | 6 + p4_fuzzer/fuzz_util.h | 7 +- tests/forwarding/l3_admit_test.h | 28 +- .../forwarding/match_action_coverage_test.cc | 257 +++++++++--------- .../component_perf_validator.cc | 16 +- .../component_perf_validator.h | 9 + 6 files changed, 191 insertions(+), 132 deletions(-) diff --git a/p4_fuzzer/fuzz_util.cc b/p4_fuzzer/fuzz_util.cc index 7d0a39745..9f1dbd521 100644 --- a/p4_fuzzer/fuzz_util.cc +++ b/p4_fuzzer/fuzz_util.cc @@ -1658,4 +1658,10 @@ AnnotatedWriteRequest FuzzWriteRequest(absl::BitGen* gen, return request; } +absl::Status ModifyEntity(absl::BitGen* gen, const FuzzerConfig& config, + const SwitchState& switch_state, + p4::v1::Entity& entity) { + return FuzzNonKeyFields(gen, config, switch_state, &entity); +} + } // namespace p4_fuzzer diff --git a/p4_fuzzer/fuzz_util.h b/p4_fuzzer/fuzz_util.h index 37117de48..b489be281 100644 --- a/p4_fuzzer/fuzz_util.h +++ b/p4_fuzzer/fuzz_util.h @@ -267,6 +267,11 @@ FuzzWriteRequest(absl::BitGen *gen, const FuzzerConfig &config, const SwitchState &switch_state, absl::optional max_batch_size = absl::nullopt); -} // namespace p4_fuzzer +// Modifies non-key fields of `entity`. +absl::Status ModifyEntity(absl::BitGen* gen, const FuzzerConfig& config, + const SwitchState& switch_state, + p4::v1::Entity& entity); + +} // namespace p4_fuzzer #endif // P4_FUZZER_FUZZ_UTIL_H_ diff --git a/tests/forwarding/l3_admit_test.h b/tests/forwarding/l3_admit_test.h index ccb68a4bc..82d08d3e3 100644 --- a/tests/forwarding/l3_admit_test.h +++ b/tests/forwarding/l3_admit_test.h @@ -54,6 +54,15 @@ class L3AdmitTestFixture : public testing::TestWithParam { testbed.Sut(), testbed.ControlSwitch(), /*gnmi_config=*/std::nullopt, /*p4_info=*/GetParam().p4info)); + // Store the original control switch gNMI interface config before changing + // it. + ASSERT_OK_AND_ASSIGN( + std::unique_ptr control_gnmi_stub, + testbed.ControlSwitch().CreateGnmiStub()); + ASSERT_OK_AND_ASSIGN(original_control_interfaces_, + pins_test::GetInterfacesAsProto( + *control_gnmi_stub, gnmi::GetRequest::CONFIG)); + // The L3Admit tests assume identical P4RT port IDs are used between the SUT // and control switch. So sending a packet from a given port ID on the // control switch means it will arrive on the same port ID on the SUT. To @@ -65,7 +74,19 @@ class L3AdmitTestFixture : public testing::TestWithParam { ASSERT_OK_AND_ASSIGN(ir_p4info_, pdpi::GetIrP4Info(*sut_p4rt_session_)); } - void TearDown() override { GetParam().testbed_interface->TearDown(); } + void TearDown() override { + // Restore the original control switch gNMI interface config's P4RT IDs. + ASSERT_OK_AND_ASSIGN( + std::unique_ptr control_gnmi_stub, + GetParam() + .testbed_interface->GetMirrorTestbed() + .ControlSwitch() + .CreateGnmiStub()); + ASSERT_OK(pins_test::SetInterfaceP4rtIds(*control_gnmi_stub, + original_control_interfaces_)); + + GetParam().testbed_interface->TearDown(); + } ~L3AdmitTestFixture() override { delete GetParam().testbed_interface; } @@ -75,6 +96,11 @@ class L3AdmitTestFixture : public testing::TestWithParam { std::unique_ptr control_switch_p4rt_session_; pdpi::IrP4Info ir_p4info_; + + // Captures the control interfaces at the start of the test. These may be + // changed during testing to mirror those of the SUT. They will then be + // returned to their previous state during teardown. + pins_test::openconfig::Interfaces original_control_interfaces_; }; } // namespace pins diff --git a/tests/forwarding/match_action_coverage_test.cc b/tests/forwarding/match_action_coverage_test.cc index 5d0d0fba3..5ea8205e7 100644 --- a/tests/forwarding/match_action_coverage_test.cc +++ b/tests/forwarding/match_action_coverage_test.cc @@ -1,5 +1,6 @@ #include "tests/forwarding/match_action_coverage_test.h" +#include #include #include #include @@ -52,6 +53,7 @@ namespace { using ::p4::v1::Entity; using ::p4::v1::TableEntry; using ::p4::v1::Update; +using ::pdpi::GetMulticastGroupTableName; using EntityPredicate = std::function; @@ -141,6 +143,17 @@ absl::StatusOr EntityTriggersKnownBug(const pdpi::IrP4Info& info, return true; } + // Remove once tunnel encap entries can refer to RIFs with + // Condition on NON-DELETE once we cover deletion. + // Empty multicast group entries are not supported by the switch. + if (ir_entity.packet_replication_engine_entry().has_multicast_group_entry() && + ir_entity.packet_replication_engine_entry() + .multicast_group_entry() + .replicas() + .empty()) { + return true; + } + // Remove once tunnel encap entries can refer to RIFs with // the `set_port_and_src_mac_and_vlan_id` action. if (entity.table_entry().table_id() == ROUTING_TUNNEL_TABLE_ID && @@ -210,6 +223,55 @@ absl::Status ModifyEntityToAvoidKnownBug(const pdpi::IrP4Info& info, return absl::OkStatus(); } +absl::Status ModifyDeleteAndReinstallEntity( + const FuzzerConfig& config, const SwitchState& state, + const Entity& initial_entity, bool skip_modify, bool mask_known_failures, + absl::BitGen& gen, pdpi::P4RuntimeSession& session) { + p4::v1::Update update; + update.set_type(p4::v1::Update::MODIFY); + *update.mutable_entity() = initial_entity; + pdpi::IrEntity entity_to_modify_to; + + if (!skip_modify) { + const absl::Time kDeadline = absl::Now() + absl::Seconds(10); + bool entry_triggers_known_bug = false; + do { + RETURN_IF_ERROR(p4_fuzzer::ModifyEntity(&gen, config, state, + *update.mutable_entity())); + if (mask_known_failures) { + RETURN_IF_ERROR(ModifyEntityToAvoidKnownBug(config.GetIrP4Info(), + *update.mutable_entity())); + } + ASSIGN_OR_RETURN( + entry_triggers_known_bug, + EntityTriggersKnownBug(config.GetIrP4Info(), update.entity(), state)); + } while (mask_known_failures && entry_triggers_known_bug && + absl::Now() < kDeadline); + // If we timeout, we return an error. + if (mask_known_failures && entry_triggers_known_bug) { + return gutil::DeadlineExceededErrorBuilder() + << "failed to generate a modification within 10s that doesn't " + "trigger a known bug for entity: " + << initial_entity.DebugString(); + } + + // Perform the modification if it was correctly generated. + ASSIGN_OR_RETURN(entity_to_modify_to, + pdpi::PiEntityToIr(config.GetIrP4Info(), update.entity())); + RETURN_IF_ERROR(pdpi::SendPiUpdates(&session, {update})) + << "during modification from original entity to new entity:\n" + << entity_to_modify_to.DebugString(); + } + + if (!skip_modify) { + *update.mutable_entity() = initial_entity; + RETURN_IF_ERROR(pdpi::SendPiUpdates(&session, {update})) + << "during modification back to original entity from new entity:\n" + << entity_to_modify_to.DebugString(); + } + return absl::OkStatus(); +} + // Generates valid entries for `table` until one meets the given // `predicate`. Unless an entry with the same keys already exists on the switch, // installs the generated entry and updates `state`. @@ -227,7 +289,7 @@ absl::Status GenerateAndInstallEntryThatMeetsPredicate( // This means that an empty optional also signals that we have the multicast // group table. std::optional p4_table_def = std::nullopt; - if (table_name != pdpi::GetMulticastGroupTableName()) { + if (table_name != GetMulticastGroupTableName()) { ASSIGN_OR_RETURN( p4_table_def, gutil::FindOrStatus(config.GetIrP4Info().tables_by_name(), table_name)); @@ -237,6 +299,7 @@ absl::Status GenerateAndInstallEntryThatMeetsPredicate( // 10 seconds. const absl::Time kDeadline = absl::Now() + absl::Seconds(10); bool entry_triggers_known_bug = false; + bool mask_known_failures = environment.MaskKnownFailures(); do { if (p4_table_def.has_value()) { ASSIGN_OR_RETURN(*entity.mutable_table_entry(), @@ -248,7 +311,7 @@ absl::Status GenerateAndInstallEntryThatMeetsPredicate( ->mutable_multicast_group_entry(), FuzzValidMulticastGroupEntry(&gen, config, state)); } - if (environment.MaskKnownFailures()) { + if (mask_known_failures) { RETURN_IF_ERROR( ModifyEntityToAvoidKnownBug(config.GetIrP4Info(), entity)); } @@ -256,11 +319,10 @@ absl::Status GenerateAndInstallEntryThatMeetsPredicate( entry_triggers_known_bug, EntityTriggersKnownBug(config.GetIrP4Info(), entity, state)); } while ((!predicate(entity) || - (environment.MaskKnownFailures() && entry_triggers_known_bug)) && + (mask_known_failures && entry_triggers_known_bug)) && absl::Now() < kDeadline); // If we timeout, we return an error. - if (!predicate(entity) || - (environment.MaskKnownFailures() && entry_triggers_known_bug)) { + if (!predicate(entity) || (mask_known_failures && entry_triggers_known_bug)) { return gutil::DeadlineExceededErrorBuilder() << absl::Substitute( "failed to generate an entry meeting the given predicate and " "doesn't trigger a known bug for table '$0' within 10s", @@ -276,20 +338,31 @@ absl::Status GenerateAndInstallEntryThatMeetsPredicate( ASSIGN_OR_RETURN(pdpi::IrEntity ir_entity, pdpi::PiEntityToIr(config.GetIrP4Info(), entity)); RETURN_IF_ERROR(environment.AppendToTestArtifact( - "requests_and_responses.txt", + "install_requests_and_responses.txt", absl::StrCat("# IR Entity:\n", ir_entity.DebugString()))); if (absl::Status status = pdpi::InstallPiEntity(&session, entity); status.ok()) { RETURN_IF_ERROR(environment.AppendToTestArtifact( - "requests_and_responses.txt", "# Successfully installed!\n")); + "install_requests_and_responses.txt", "# Successfully installed!\n")); } else { RETURN_IF_ERROR(environment.AppendToTestArtifact( - "requests_and_responses.txt", + "install_requests_and_responses.txt", absl::StrCat("# Installation failed:\n", status.ToString()))); RETURN_IF_ERROR(status) << "IR entity:\n" << ir_entity.DebugString(); } + // Don't modify non-modifiable tables. + absl::string_view fully_qualified_table_name = + p4_table_def.has_value() ? p4_table_def->preamble().name() : table_name; + bool skip_modify = + config.GetNonModifiableTables().contains(fully_qualified_table_name); + // Test if modification and deletion works as expected for this entity. + RETURN_IF_ERROR(ModifyDeleteAndReinstallEntity( + config, state, entity, skip_modify, mask_known_failures, gen, session)) + << absl::StrCat("When attempting to ", skip_modify ? "" : "modify, then ", + "delete and reinstall IR entity:\n", ir_entity); + // Update state. Update update; update.set_type(Update::INSERT); @@ -307,21 +380,15 @@ absl::Status AddMulticastGroupEntryWithAndWithoutReplicas( const p4::config::v1::P4Info& p4info) { LOG(INFO) << "INSTALLING ENTRIES INTO BUILT-IN MULTICAST GROUP TABLE"; + // Ensure both a L2 and L3 multicast entry are + // installed. L2 groups are currently disabled, so all groups are L3. RETURN_IF_ERROR(GenerateAndInstallEntryThatMeetsPredicate( gen, session, config, state, environment, - pdpi::GetMulticastGroupTableName(), + GetMulticastGroupTableName(), /*predicate=*/ - [&](const Entity& entity) { - // Checks that the multicast entry has replicas. - return entity.packet_replication_engine_entry() - .has_multicast_group_entry() && - !entity.packet_replication_engine_entry() - .multicast_group_entry() - .replicas() - .empty(); - })) + [&](const Entity& entity) { return true; })) .SetPrepend() - << "while generating entry for '" << pdpi::GetMulticastGroupTableName() + << "while generating entry for '" << GetMulticastGroupTableName() << "': "; LOG(INFO) << absl::Substitute(" - With $0: Present", pdpi::GetReplicaActionName()); @@ -329,7 +396,7 @@ absl::Status AddMulticastGroupEntryWithAndWithoutReplicas( // Uncomment once empty multicast groups are supported. // RETURN_IF_ERROR(GenerateAndInstallEntryThatMeetsPredicate( // gen, session, config, state, environment, - // pdpi::GetMulticastGroupTableName(), + // GetMulticastGroupTableName(), // /*predicate=*/ // [&](const Entity& entity) { // // Checks that the multicast entry has no replicas. @@ -341,7 +408,7 @@ absl::Status AddMulticastGroupEntryWithAndWithoutReplicas( // .empty(); // })) // .SetPrepend() - // << "while generating entry for '" << pdpi::GetMulticastGroupTableName() + // << "while generating entry for '" << GetMulticastGroupTableName() // << "': "; // LOG(INFO) << absl::Substitute(" - With $0: Absent", // pdpi::GetReplicaActionName()); @@ -375,6 +442,10 @@ absl::StatusOr FieldPresenceConstraintString( // - Each omittable match field is omitted and included in at least one table // entry. // - Each action is included in at least one table entry. +// Tables are installed in the order of their dependency rank so that all +// references can be satisfied. In other words, if Table A refers to Table B, +// then Table B will be installed first. +// Remove p4info param and use config to get P4Info. absl::Status AddTableEntryForEachMatchAndEachAction( absl::BitGen& gen, pdpi::P4RuntimeSession& session, const FuzzerConfig& config, SwitchState& state, @@ -383,8 +454,44 @@ absl::Status AddTableEntryForEachMatchAndEachAction( ASSIGN_OR_RETURN(const p4_constraints::ConstraintInfo constraints_by_table_id, p4_constraints::P4ToConstraintInfo(p4info)); - for (const pdpi::IrTableDefinition& table : - AllValidTablesForP4RtRole(config)) { + // Order tables so that tables being referenced have valid entries. + std::vector table_names_by_dependency_rank; + for (const auto& table_def : AllValidTablesForP4RtRole(config)) { + table_names_by_dependency_rank.push_back(table_def.preamble().alias()); + } + + // Remove once built-in tables can be treated like any + // other table. + // Add built-in tables that are not disabled. + for (const auto& [name, unused] : config.GetIrP4Info().built_in_tables()) { + if (!config.GetDisabledFullyQualifiedNames().contains(name)) { + table_names_by_dependency_rank.push_back(name); + } + } + + // Move function to sequencing library. + std::sort( + table_names_by_dependency_rank.begin(), + table_names_by_dependency_rank.end(), + [&](absl::string_view a, absl::string_view b) -> bool { + int rank_a = config.GetIrP4Info().dependency_rank_by_table_name().at(a); + int rank_b = config.GetIrP4Info().dependency_rank_by_table_name().at(b); + // If the ranks are the same, sort alphabetically. Otherwise, + // larger rank (i.e. table w/ smaller dependency chain) comes first. + return rank_a == rank_b ? a < b : rank_a > rank_b; + }); + + for (absl::string_view table_name : table_names_by_dependency_rank) { + // Built-in multicast group table is handled separately. + if (table_name == GetMulticastGroupTableName()) { + RETURN_IF_ERROR(AddMulticastGroupEntryWithAndWithoutReplicas( + gen, session, config, state, environment, p4info)); + continue; + } + + ASSIGN_OR_RETURN( + const pdpi::IrTableDefinition& table, + gutil::FindOrStatus(config.GetIrP4Info().tables_by_name(), table_name)); LOG(INFO) << absl::Substitute("For table '$0', installing entries with:", table.preamble().alias()); std::vector required_match_descriptions; @@ -546,96 +653,6 @@ absl::Status AddTableEntryForEachMatchAndEachAction( return absl::OkStatus(); } -// Installs a table entry per supported, programmable table in a particular -// order. This ensures that we can generate valid table entries for every table. -// It is required due to the possibility of references between tables. -// Remove this function once auxiliary entries are installed -// via dependency rank in P4Info. -absl::Status AddAuxiliaryTableEntries(absl::BitGen& gen, - pdpi::P4RuntimeSession& session, - const FuzzerConfig& config, - SwitchState& state, - thinkit::TestEnvironment& environment) { - std::vector kOrderedTablesToInsertEntriesInto = { - "mirror_session_table", - "l3_admit_table", - "vrf_table", - "router_interface_table", - "multicast_router_interface_table", - "builtin::multicast_group_table", - "neighbor_table", - "tunnel_table", - "nexthop_table", - "wcmp_group_table", - "ipv4_table", - "ipv6_table", - }; - - for (const auto& table_name : kOrderedTablesToInsertEntriesInto) { - // mirror_session_table is only present in a few instantiations, so the test - // needs to skip it if it is not present. - if (table_name == "mirror_session_table" && - !config.GetIrP4Info().tables_by_name().contains(table_name)) { - continue; - } - std::string fully_qualified_name = table_name; - if (table_name != pdpi::GetMulticastGroupTableName()) { - ASSIGN_OR_RETURN(const pdpi::IrTableDefinition& table, - gutil::FindOrStatus( - config.GetIrP4Info().tables_by_name(), table_name)); - fully_qualified_name = table.preamble().name(); - - if (table.is_unsupported()) { - LOG(WARNING) - << table_name - << " is currently unsupported and will not be fuzzed. DO NOT add " - "aux entries until the table is supported. If the table was " - "previously supported and will not be supported in the near " - "future, consider removing the aux entry. If support will be " - "added again soon disregard this message."; - continue; - } - } - - if (IsDisabledForFuzzing(config, fully_qualified_name)) { - LOG(INFO) << absl::StrCat(table_name, - " was skipped due to being disabled."); - continue; - } - - LOG(INFO) << absl::StrCat("Adding auxiliary entry to ", table_name); - RETURN_IF_ERROR( - GenerateAndInstallEntryThatMeetsPredicate( - gen, session, config, state, environment, table_name, - /*predicate=*/ - [](const Entity& entity) { - // Checks that the multicast entry has replicas. - bool multicast_has_no_replicas = - entity.has_packet_replication_engine_entry() && - entity.packet_replication_engine_entry() - .has_multicast_group_entry() && - entity.packet_replication_engine_entry() - .multicast_group_entry() - .replicas() - .empty(); - - // Check that the RIF entry does not have VLAN. - // Remove once tunnel encap entries can refer - // to RIFs with the `set_port_and_src_mac_and_vlan_id` action. - bool rif_has_vlan = - entity.table_entry().table_id() == - ROUTING_ROUTER_INTERFACE_TABLE_ID && - entity.table_entry().action().action().action_id() == - ROUTING_SET_PORT_AND_SRC_MAC_AND_VLAN_ID_ACTION_ID; - - return !rif_has_vlan && !multicast_has_no_replicas; - })) - .SetPrepend() - << "while trying to generate entry for '" << table_name << "': "; - } - return absl::OkStatus(); -} - TEST_P(MatchActionCoverageTestFixture, InsertEntriesForEveryTableAndMatchConfiguration) { thinkit::MirrorTestbed& testbed = @@ -668,25 +685,11 @@ TEST_P(MatchActionCoverageTestFixture, config.SetMutateUpdateProbability(0); SwitchState state(config.GetIrP4Info()); - // Sets up the switch such that there is a possible valid entry per table. - // This is required due to the possibility of references between tables. - ASSERT_OK(AddAuxiliaryTableEntries(gen, *p4rt_session, config, state, - testbed.Environment())); // Generates and installs entries that use every match field and action. EXPECT_OK(AddTableEntryForEachMatchAndEachAction(gen, *p4rt_session, config, state, testbed.Environment(), GetParam().p4info)); - - // Remove function once built-in multicast group table can - // be treated like any other table. - // Generates and installs entries for the built-in multicast group table. - if (!config.GetDisabledFullyQualifiedNames().contains( - pdpi::GetMulticastGroupTableName())) { - EXPECT_OK(AddMulticastGroupEntryWithAndWithoutReplicas( - gen, *p4rt_session, config, state, testbed.Environment(), - GetParam().p4info)); - } } } // namespace diff --git a/tests/integration/system/nsf/component_validators/component_perf_validator.cc b/tests/integration/system/nsf/component_validators/component_perf_validator.cc index 057c68178..19c8597a9 100644 --- a/tests/integration/system/nsf/component_validators/component_perf_validator.cc +++ b/tests/integration/system/nsf/component_validators/component_perf_validator.cc @@ -33,15 +33,25 @@ #include "thinkit/switch.h" namespace pins_test { -namespace { +namespace {} // namespace -absl::Status ValidateComponentWarmbootPerformance(thinkit::Switch& sut) { +absl::Status ComponentPerfValidator::ValidateComponentWarmbootPerformance( + thinkit::Switch& sut) { absl::Status ret_status; return ret_status; } -} // namespace +absl::Status ComponentPerfValidator::FetchPreWarmRebootRegistrationInformation( + thinkit::Switch& sut) { + return absl::OkStatus(); +} + +absl::Status ComponentPerfValidator::OnImageCopy( + absl::string_view version, const Testbed& testbed, + thinkit::SSHClient& ssh_client) { + return FetchPreWarmRebootRegistrationInformation(GetSut(testbed)); +} absl::Status ComponentPerfValidator::OnNsfReboot( absl::string_view version, const Testbed& testbed, diff --git a/tests/integration/system/nsf/component_validators/component_perf_validator.h b/tests/integration/system/nsf/component_validators/component_perf_validator.h index fc54354f0..32352616c 100644 --- a/tests/integration/system/nsf/component_validators/component_perf_validator.h +++ b/tests/integration/system/nsf/component_validators/component_perf_validator.h @@ -25,8 +25,17 @@ namespace pins_test { class ComponentPerfValidator : public ComponentValidator { + absl::Status OnImageCopy(absl::string_view version, const Testbed& testbed, + thinkit::SSHClient& ssh_client) override; absl::Status OnNsfReboot(absl::string_view version, const Testbed& testbed, thinkit::SSHClient& ssh_client) override; + + private: + absl::Status FetchPreWarmRebootRegistrationInformation(thinkit::Switch& sut); + absl::Status ValidateComponentWarmbootPerformance(thinkit::Switch& sut); + + absl::flat_hash_set freeze_apps_; + absl::flat_hash_set checkpoint_apps_; }; } // namespace pins_test From 8366a98e4ec5ec3602823ad553f692f9ceb0ee58 Mon Sep 17 00:00:00 2001 From: kishanps Date: Tue, 19 Nov 2024 01:58:34 -0800 Subject: [PATCH 7/7] Extend packet capture tests with controller packet capture. --- tests/forwarding/BUILD.bazel | 1 + tests/forwarding/l3_admit_test.h | 4 +- .../system/nsf/traffic_helpers/otg_helper.cc | 1 + tests/packet_capture/packet_capture_test.cc | 472 +++++++++++++++++- tests/packet_capture/packet_capture_test.h | 25 +- 5 files changed, 476 insertions(+), 27 deletions(-) diff --git a/tests/forwarding/BUILD.bazel b/tests/forwarding/BUILD.bazel index 8f5f27480..acea86901 100644 --- a/tests/forwarding/BUILD.bazel +++ b/tests/forwarding/BUILD.bazel @@ -434,6 +434,7 @@ cc_library( "//p4_pdpi:ir_cc_proto", "//p4_pdpi:p4_runtime_session", "//p4_pdpi:p4_runtime_session_extras", + "//p4_pdpi/netaddr:ipv4_address", "//p4_pdpi/packetlib", "//p4_pdpi/packetlib:packetlib_cc_proto", "//tests/lib:p4info_helper", diff --git a/tests/forwarding/l3_admit_test.h b/tests/forwarding/l3_admit_test.h index 82d08d3e3..7335f84a8 100644 --- a/tests/forwarding/l3_admit_test.h +++ b/tests/forwarding/l3_admit_test.h @@ -18,6 +18,7 @@ #include #include +#include "gmock/gmock.h" #include "gutil/status_matchers.h" #include "lib/gnmi/gnmi_helper.h" #include "lib/gnmi/openconfig.pb.h" @@ -25,10 +26,11 @@ #include "p4_pdpi/ir.pb.h" #include "p4_pdpi/p4_runtime_session.h" #include "p4_pdpi/p4_runtime_session_extras.h" +#include "proto/gnmi/gnmi.grpc.pb.h" +#include "proto/gnmi/gnmi.pb.h" #include "tests/lib/switch_test_setup_helpers.h" #include "thinkit/mirror_testbed.h" #include "thinkit/mirror_testbed_fixture.h" -#include "gmock/gmock.h" namespace pins { diff --git a/tests/integration/system/nsf/traffic_helpers/otg_helper.cc b/tests/integration/system/nsf/traffic_helpers/otg_helper.cc index b22885d1c..33abb767d 100644 --- a/tests/integration/system/nsf/traffic_helpers/otg_helper.cc +++ b/tests/integration/system/nsf/traffic_helpers/otg_helper.cc @@ -285,6 +285,7 @@ absl::Status OtgHelper::ValidateTraffic(const Testbed &testbed, metrics_req.mutable_metrics_request()->set_choice( otg::MetricsRequest::Choice::flow); + metrics_req.mutable_metrics_request()->mutable_flow(); RETURN_IF_ERROR(gutil::GrpcStatusToAbslStatus( stub->GetMetrics(&metrics_ctx, metrics_req, &metrics_res))); diff --git a/tests/packet_capture/packet_capture_test.cc b/tests/packet_capture/packet_capture_test.cc index 99a2bf78e..bc1c8841d 100644 --- a/tests/packet_capture/packet_capture_test.cc +++ b/tests/packet_capture/packet_capture_test.cc @@ -14,6 +14,7 @@ #include "tests/packet_capture/packet_capture_test.h" +#include #include #include #include @@ -22,6 +23,8 @@ #include "absl/container/flat_hash_map.h" #include "absl/flags/declare.h" +#include "absl/log/check.h" +#include "absl/log/log.h" #include "absl/status/statusor.h" #include "absl/strings/numbers.h" #include "absl/strings/str_cat.h" @@ -29,11 +32,18 @@ #include "absl/time/clock.h" #include "absl/time/time.h" #include "absl/types/optional.h" -#include "glog/logging.h" +#include "dvaas/dataplane_validation.h" +#include "dvaas/test_vector.h" +#include "dvaas/test_vector.pb.h" +#include "dvaas/validation_result.h" +#include "gmock/gmock.h" +#include "google/protobuf/descriptor.h" +#include "gtest/gtest.h" #include "gutil/collections.h" #include "gutil/status.h" #include "lib/gnmi/gnmi_helper.h" #include "lib/gnmi/openconfig.pb.h" +#include "net/google::protobuf/contrib/fixtures/proto-fixture-repository.h" #include "p4/config/v1/p4info.pb.h" #include "p4/v1/p4runtime.pb.h" #include "p4_pdpi/ir.h" @@ -42,7 +52,9 @@ #include "p4_pdpi/p4_runtime_session_extras.h" #include "p4_pdpi/packetlib/packetlib.h" #include "p4_pdpi/packetlib/packetlib.pb.h" +#include "p4_pdpi/string_encodings/decimal_string.h" #include "p4_pdpi/string_encodings/hex_string.h" +#include "p4_pdpi/ternary.h" #include "proto/gnmi/gnmi.pb.h" #include "sai_p4/instantiations/google/instantiations.h" #include "sai_p4/instantiations/google/sai_pd.pb.h" @@ -53,8 +65,6 @@ #include "thinkit/mirror_testbed.h" #include "thinkit/proto/generic_testbed.pb.h" #include "thinkit/switch.h" -#include "gmock/gmock.h" -#include "gtest/gtest.h" ABSL_DECLARE_FLAG(std::optional, switch_instantiation); @@ -68,9 +78,9 @@ using pctutil::SutToControlLinks; // on an incoming port to a mirror-to-port using PSAMP encapsulation and // adding a specified Vlan tag. absl::StatusOr> -ConstructEntriesToMirrorTrafficWithVlanTag( - const pdpi::IrP4Info &ir_p4info, const std::string &p4rt_src_port_id, - const sai::MirrorSessionParams &mirror_session) { +CreateEntriesToMirrorTrafficWithVlanTag( + const pdpi::IrP4Info& ir_p4info, const std::string& p4rt_src_port_id, + const sai::MirrorSessionParams& mirror_session) { ASSIGN_OR_RETURN( std::vector pi_entities, sai::EntryBuilder() @@ -91,8 +101,8 @@ TEST_P(PacketCaptureTestWithoutIxia, PsampEncapsulatedMirroringTest) { // Setup: the testbed consists of a SUT connected to a control device // that allows us to send and receive packets to/from the SUT. - thinkit::Switch &sut = Testbed().Sut(); - thinkit::Switch &control_device = Testbed().ControlSwitch(); + thinkit::Switch& sut = Testbed().Sut(); + thinkit::Switch& control_device = Testbed().ControlSwitch(); // Configure mirror testbed. std::unique_ptr sut_p4rt_session, @@ -105,9 +115,9 @@ TEST_P(PacketCaptureTestWithoutIxia, PsampEncapsulatedMirroringTest) { pins_test::ConfigureSwitchAndReturnP4RuntimeSession( control_device, std::nullopt, std::nullopt)); - ASSERT_OK_AND_ASSIGN(const P4Info &p4info, + ASSERT_OK_AND_ASSIGN(const P4Info& p4info, pdpi::GetP4Info(*sut_p4rt_session)); - ASSERT_OK_AND_ASSIGN(const P4Info &control_p4info, + ASSERT_OK_AND_ASSIGN(const P4Info& control_p4info, pdpi::GetP4Info(*control_p4rt_session)); ASSERT_OK_AND_ASSIGN(const pdpi::IrP4Info ir_p4info, pdpi::CreateIrP4Info(p4info)); @@ -170,7 +180,7 @@ TEST_P(PacketCaptureTestWithoutIxia, PsampEncapsulatedMirroringTest) { .mirror_encap_udp_dst_port = "0x1283"}; // Install ACL table entry to match on inject port on Control switch // and mirror-to-port on SUT. - ASSERT_OK_AND_ASSIGN(auto entries, ConstructEntriesToMirrorTrafficWithVlanTag( + ASSERT_OK_AND_ASSIGN(auto entries, CreateEntriesToMirrorTrafficWithVlanTag( ir_p4info, kSutIngressPortP4rtId, mirror_session_params)); ASSERT_OK( @@ -209,9 +219,8 @@ TEST_P(PacketCaptureTestWithoutIxia, PsampEncapsulatedMirroringTest) { // Read packets mirrored back to Control switch. std::vector received_packets; EXPECT_OK(control_p4rt_session->HandleNextNStreamMessages( - [&](const p4::v1::StreamMessageResponse &message) { - if (!message.has_packet()) - return false; + [&](const p4::v1::StreamMessageResponse& message) { + if (!message.has_packet()) return false; packetlib::Packet received_packet = packetlib::ParsePacket(message.packet().payload()); received_packets.push_back(received_packet); @@ -225,7 +234,7 @@ TEST_P(PacketCaptureTestWithoutIxia, PsampEncapsulatedMirroringTest) { int mirrored_packets_received = 0; std::optional prev_obs_time, curr_obs_time; std::optional prev_sequence, curr_sequence; - for (const packetlib::Packet &received_packet : received_packets) { + for (const packetlib::Packet& received_packet : received_packets) { // Header count sanity check. LOG(INFO) << absl::StrCat("Packet: ", received_packet.DebugString()); if (received_packet.headers().size() != 6) { @@ -233,7 +242,7 @@ TEST_P(PacketCaptureTestWithoutIxia, PsampEncapsulatedMirroringTest) { } // Parse Ethernet header. ASSERT_EQ(received_packet.headers(0).has_ethernet_header(), true); - const auto ð_header = received_packet.headers(0).ethernet_header(); + const auto& eth_header = received_packet.headers(0).ethernet_header(); EXPECT_EQ(eth_header.ethernet_source(), mirror_session_params.mirror_encap_src_mac); EXPECT_EQ(eth_header.ethernet_destination(), @@ -244,14 +253,14 @@ TEST_P(PacketCaptureTestWithoutIxia, PsampEncapsulatedMirroringTest) { mirror_session_params.mirror_encap_vlan_id); // Parse IPv6 header. ASSERT_EQ(received_packet.headers(2).has_ipv6_header(), true); - const auto &ip_header = received_packet.headers(2).ipv6_header(); + const auto& ip_header = received_packet.headers(2).ipv6_header(); EXPECT_EQ(ip_header.ipv6_source(), mirror_session_params.mirror_encap_src_ip); EXPECT_EQ(ip_header.ipv6_destination(), mirror_session_params.mirror_encap_dst_ip); // Parse UDP header. ASSERT_EQ(received_packet.headers(3).has_udp_header(), true); - const auto &udp_header = received_packet.headers(3).udp_header(); + const auto& udp_header = received_packet.headers(3).udp_header(); EXPECT_EQ(udp_header.source_port(), mirror_session_params.mirror_encap_udp_src_port); @@ -271,7 +280,7 @@ TEST_P(PacketCaptureTestWithoutIxia, PsampEncapsulatedMirroringTest) { prev_sequence = curr_sequence; // Parse PSAMP header. ASSERT_EQ(received_packet.headers(5).has_psamp_header(), true); - const auto &psamp_header = received_packet.headers(5).psamp_header(); + const auto& psamp_header = received_packet.headers(5).psamp_header(); // Validate observation times increment within expected range. ASSERT_OK_AND_ASSIGN(curr_obs_time, pdpi::HexStringToUint64( psamp_header.observation_time())); @@ -339,5 +348,426 @@ TEST_P(PacketCaptureTestWithoutIxia, PsampEncapsulatedMirroringTest) { ", out_packets_pre: ", out_packets_pre); } -} // anonymous namespace -} // namespace pins_test +using ::google::protobuf::contrib::fixtures::ProtoFixtureRepository; + +struct ControllerPacketCaptureParams { + std::string ingress_port; + std::string egress_port; + // Ingress VLAN IDs to match on. + std::vector ingress_vlan_ids; + // Egress VLAN ID to be set on packets. + int forwarded_packet_vlan_id; + std::string mirror_session_id; +}; + +std::string GetVlanIdHexStr(int val) { + return absl::StrCat("0x", absl::Hex(val, absl::kZeroPad3)); +} + +std::string GetAclMetadataHexStr(int val) { + return absl::StrCat("0x", absl::Hex(val, absl::kZeroPad2)); +} + +// Returns a set of table entries that will cause a switch to match on Ingress +// VLAN ID and Ingress Port and set ACL Metadata and Egress VLAN ID. +absl::StatusOr> CreatePreIngressAclEntries( + const pdpi::IrP4Info& ir_p4info, const std::string& ingress_port, + const std::vector& ingress_vlan_ids, + const int forwarded_packet_vlan_id) { + std::vector entities; + sai::EntryBuilder entry_builder; + for (const int ingress_vlan_id : ingress_vlan_ids) { + ASSIGN_OR_RETURN(std::bitset ingress_vlan_id_bitset, + pdpi::HexStringToBitset( + GetVlanIdHexStr(ingress_vlan_id))); + entry_builder.AddPreIngressAclEntrySettingVlanAndAclMetadata( + GetVlanIdHexStr(forwarded_packet_vlan_id), + /*set ingress vlan id as acl metadata*/ + GetAclMetadataHexStr(ingress_vlan_id), + sai::AclPreIngressVlanTableMatchFields{ + .vlan_id = pdpi::Ternary>( + ingress_vlan_id_bitset), + .in_port = ingress_port, + }, + /*priority=*/1); + } + ASSIGN_OR_RETURN( + std::vector builder_entities, + entry_builder.LogPdEntries().GetDedupedPiEntities(ir_p4info)); + entities.insert(entities.end(), builder_entities.begin(), + builder_entities.end()); + return entities; +} + +// Returns a set of table entries that will cause a switch to match on ACL +// metadata and mirror all packets on an incoming port to a mirror-to-port +// using PSAMP encapsulation and adding a specified Vlan tag. +absl::StatusOr> +CreateEntriesToMatchOnAclMetadataAndMirrorTrafficAndRedirectToPort( + const pdpi::IrP4Info& ir_p4info, + const sai::MirrorSessionParams& mirror_session_params, + const std::vector& ingress_vlan_ids, const std::string& egress_port, + const std::string& mirror_session_id) { + std::vector pi_entities; + ASSIGN_OR_RETURN(std::vector builder_entities, + sai::EntryBuilder() + .AddDisableIngressVlanChecksEntry() + .AddMirrorSessionTableEntry(mirror_session_params) + .LogPdEntries() + .GetDedupedPiEntities(ir_p4info)); + pi_entities.insert(pi_entities.end(), builder_entities.begin(), + builder_entities.end()); + for (const int ingress_vlan_id : ingress_vlan_ids) { + /* Set ACL metadata to be the ingress VLAN ID. */ + ASSIGN_OR_RETURN(std::bitset acl_metadata, + pdpi::HexStringToBitset( + GetAclMetadataHexStr(ingress_vlan_id))); + ASSIGN_OR_RETURN( + std::vector pre_ingress_acl_entries, + sai::EntryBuilder() + .AddIngressAclEntryMirroringAndRedirectingToPort( + egress_port, mirror_session_id, + sai::MirrorAndRedirectMatchFields{ + .acl_metadata = + pdpi::Ternary>( + acl_metadata), + }, + /*priority=*/1) + .LogPdEntries() + .GetDedupedPiEntities(ir_p4info)); + pi_entities.insert(pi_entities.end(), pre_ingress_acl_entries.begin(), + pre_ingress_acl_entries.end()); + } + + return pi_entities; +} + +packetlib::Packet ParsePacketAndPadToMinimumSize( + const ProtoFixtureRepository& repo, absl::string_view packet_pb) { + packetlib::Packet packet = repo.ParseTextOrDie(packet_pb); + CHECK_OK(packetlib::PadPacketToMinimumSize(packet)); + return packet; +} + +absl::StatusOr> +CreateControllerPacketCaptureTestVectors( + const ControllerPacketCaptureParams& params, + const sai::MirrorSessionParams& mirror_session_params) { + // Test packets injected and expected results. + std::vector test_vectors; + + for (int i = 0; i < params.ingress_vlan_ids.size(); ++i) { + dvaas::PacketTestVector test_vector; + ProtoFixtureRepository repo; + + ASSIGN_OR_RETURN(int mirror_port, pdpi::DecimalStringToInt( + mirror_session_params.monitor_port)); + + std::string mirror_port_hex = + absl::StrCat("0x", absl::Hex(mirror_port, absl::kZeroPad4)); + repo.RegisterValue("@ingress_port", ""); + + repo.RegisterValue("@egress_port", params.egress_port); + repo.RegisterValue("@mirror_port_hex", mirror_port_hex); + repo.RegisterValue("@mirror_port", mirror_session_params.monitor_port); + repo.RegisterValue("@ingress_vlan_id", + GetVlanIdHexStr(params.ingress_vlan_ids[i])); + repo.RegisterValue("@mirror_ethernet_source", + mirror_session_params.mirror_encap_src_mac); + repo.RegisterValue("@mirror_ethernet_destination", + mirror_session_params.mirror_encap_dst_mac); + repo.RegisterValue("@mirror_vlan_id", + mirror_session_params.mirror_encap_vlan_id); + repo.RegisterValue("@mirror_encap_udp_src_port", + mirror_session_params.mirror_encap_udp_src_port); + repo.RegisterValue("@mirror_encap_udp_dst_port", + mirror_session_params.mirror_encap_udp_dst_port); + repo.RegisterValue("@mirror_encap_ip_src", + mirror_session_params.mirror_encap_src_ip); + repo.RegisterValue("@mirror_encap_ip_dst", + mirror_session_params.mirror_encap_dst_ip); + repo.RegisterValue("@ttl", "0x10"); + repo.RegisterValue("@payload", + dvaas::MakeTestPacketTagFromUniqueId( + i, + "SwitchCapturesControlPacketsWithVlanTagAn" + "dRedirectsToPortAndMirrorsPackets")); + // Build headers. + repo.RegisterSnippetOrDie("@ethernet", R"pb( + ethernet_header { + ethernet_destination: "08:00:20:86:35:4b" + ethernet_source: "00:1a:11:17:5f:80" + ethertype: "0x8100" + } + )pb") + .RegisterSnippetOrDie("@vlan", R"pb( + vlan_header { + priority_code_point: "0x0", + drop_eligible_indicator: "0x0", + vlan_identifier: @ingress_vlan_id, + ethertype: "0x0800" + } + )pb") + .RegisterSnippetOrDie("@ipv4", R"pb( + ipv4_header { + version: "0x4" + dscp: "0x00" + ecn: "0x0" + # total_length: filled in automatically. + identification: "0xaafb" + flags: "0x2" + fragment_offset: "0x0000" + ttl: @ttl + protocol: "0x01" + # checksum: filled in automatically + ipv4_source: "10.0.0.1" + ipv4_destination: "10.0.0.2" + } + )pb") + .RegisterSnippetOrDie("@ipv6", R"pb( + ipv6_header { + version: "0x6" + dscp: "0x00" + ecn: "0x0" + flow_label: "0x00000" + # payload_length: filled in automatically. + next_header: "0x11" + hop_limit: "0x00" + ipv6_source: @mirror_encap_ip_src + ipv6_destination: @mirror_encap_ip_dst + } + )pb") + .RegisterSnippetOrDie("@icmp", R"pb( + icmp_header { type: "0x00" code: "0x00" rest_of_header: "0x1e600000" } + )pb") + .RegisterSnippetOrDie("@udp", R"pb( + udp_header { + source_port: @mirror_encap_udp_src_port + destination_port: @mirror_encap_udp_dst_port + # length: filled in automatically + # checksum: filled in automatically + } + )pb") + .RegisterSnippetOrDie("@ipfix", R"pb( + ipfix_header { + version: "0x000a" + # length: filled in automatically + export_time: "0x00002edf" + sequence_number: "0x00000000" + observation_domain_id: "0x00000000" + } + )pb") + .RegisterSnippetOrDie("@psamp_header", R"pb( + psamp_header { + template_id: "0x0000" + # length: filled in automatically + observation_time: "0x00002edf27a5cb40" + flowset: "0xe72d" + next_hop_index: "0x0000" + epoch: "0x0000" + ingress_port: "0x0000" + egress_port: @mirror_port_hex + user_meta_field: "0x0000" + dlb_id: "0x00" + } + )pb") + .RegisterMessage("@input_packet", + ParsePacketAndPadToMinimumSize(repo, + R"pb( + headers: @ethernet + headers: @vlan + headers: @ipv4 + headers: @icmp + payload: @payload + )pb")); + + // Build up acceptable_outputs string, to account for each replica. + dvaas::SwitchOutput expected_output; + *expected_output.add_packets() = + repo.RegisterValue("@egress_port", params.egress_port) + .RegisterMessage( + "@output_packet", ParsePacketAndPadToMinimumSize(repo, R"pb( + headers: @ethernet { ethernet_header { ethertype: "0x0800" } } + headers: @ipv4 + headers: @icmp + payload: @payload + )pb")) + .ParseTextOrDie(R"pb( + port: @egress_port + parsed: @output_packet + )pb"); + *expected_output.add_packets() = + repo.RegisterValue("@egress_port", mirror_session_params.monitor_port) + .RegisterMessage( + "@output_packet", ParsePacketAndPadToMinimumSize(repo, R"pb( + headers: @ethernet { + ethernet_header { + ethernet_destination: @mirror_ethernet_destination + ethernet_source: @mirror_ethernet_source + ethertype: "0x8100" + } + } + headers: @vlan { + vlan_header { + vlan_identifier: @mirror_vlan_id + ethertype: "0x86dd" + } + } + headers: @ipv6 + headers: @udp + headers: @ipfix + headers: @psamp_header + payload: @payload + )pb")) + .ParseTextOrDie(R"pb( + port: @egress_port + parsed: @output_packet + )pb"); + + test_vector = repo.RegisterMessage("@expected_output", expected_output) + .ParseTextOrDie(R"pb( + input { + type: DATAPLANE + packet { port: @ingress_port parsed: @input_packet } + } + acceptable_outputs: @expected_output + )pb"); + test_vector.mutable_input()->set_type( + dvaas::SwitchInput::SUBMIT_TO_INGRESS); + test_vectors.push_back(test_vector); + } + return test_vectors; +} + +TEST_P(PacketCaptureTestWithoutIxia, + SwitchCapturesControlPacketWithVlanTagAndRedirectsToPort) { + const int kPortsToUseInTest = 3; + + dvaas::DataplaneValidationParams dvaas_params = GetParam().validation_params; + thinkit::MirrorTestbed& testbed = + GetParam().testbed_interface->GetMirrorTestbed(); + + // Initialize the connection, clear all entities, and (for the SUT) push + // P4Info. + ASSERT_OK_AND_ASSIGN( + auto sut_p4rt_session, + pins_test::ConfigureSwitchAndReturnP4RuntimeSession( + testbed.Sut(), + /*gnmi_config=*/std::nullopt, GetParam().sut_p4info)); + + ASSERT_OK_AND_ASSIGN(auto sut_ir_p4info, + pdpi::GetIrP4Info(*sut_p4rt_session)); + ASSERT_OK(pdpi::ClearEntities(*sut_p4rt_session)); + + // Collect port IDs. + // Get SUT and control ports to test on. + ASSERT_OK_AND_ASSIGN(auto gnmi_stub, testbed.Sut().CreateGnmiStub()); + ASSERT_OK_AND_ASSIGN(std::vector sut_port_ids, + GetNUpInterfacePortIds(*gnmi_stub, kPortsToUseInTest)); + + const std::string kMirrorSessionId = "psamp_mirror"; + const ControllerPacketCaptureParams controller_packet_capture_params = { + .ingress_port = absl::StrCat(GetParam().cpu_port_id), + .egress_port = sut_port_ids[0], + .ingress_vlan_ids = GetParam().vlans_to_be_tested, + .forwarded_packet_vlan_id = 4095, + .mirror_session_id = kMirrorSessionId, + }; + + ASSERT_OK_AND_ASSIGN( + std::vector acl_entities, + CreatePreIngressAclEntries( + sut_ir_p4info, controller_packet_capture_params.ingress_port, + controller_packet_capture_params.ingress_vlan_ids, + controller_packet_capture_params.forwarded_packet_vlan_id)); + ASSERT_OK(pdpi::InstallPiEntities(sut_p4rt_session.get(), sut_ir_p4info, + acl_entities)); + + // Configure mirror session attributes. + const sai::MirrorSessionParams mirror_session_params = + sai::MirrorSessionParams{ + .mirror_session_id = kMirrorSessionId, + .monitor_port = sut_port_ids[1], + .monitor_backup_port = sut_port_ids[1], + .mirror_encap_src_mac = "00:00:00:22:22:22", + .mirror_encap_dst_mac = "00:00:00:44:44:44", + .mirror_encap_vlan_id = "0x0fe", + .mirror_encap_src_ip = "2222:2222:2222:2222:2222:2222:2222:2222", + .mirror_encap_dst_ip = "4444:4444:4444:4444:4444:4444:4444:4444", + .mirror_encap_udp_src_port = "0x08ae", + .mirror_encap_udp_dst_port = "0x1283"}; + // Install ACL table entry to match on inject port on Control switch + // and mirror-to-port on SUT. + ASSERT_OK_AND_ASSIGN( + const std::vector entries, + CreateEntriesToMatchOnAclMetadataAndMirrorTrafficAndRedirectToPort( + sut_ir_p4info, mirror_session_params, + controller_packet_capture_params.ingress_vlan_ids, + controller_packet_capture_params.egress_port, kMirrorSessionId)); + ASSERT_OK( + pdpi::InstallPiEntities(sut_p4rt_session.get(), sut_ir_p4info, entries)); + // Build test vectors. + ASSERT_OK_AND_ASSIGN( + std::vector test_vector, + CreateControllerPacketCaptureTestVectors(controller_packet_capture_params, + mirror_session_params)); + + dvaas_params.packet_test_vector_override = test_vector; + + const google::protobuf::FieldDescriptor* ipfix_header_descriptor = + packetlib::Header::descriptor()->FindFieldByName("ipfix_header"); + if (ipfix_header_descriptor == nullptr) { + FAIL() << "IPFIX header not found in packetlib"; + } + const google::protobuf::FieldDescriptor* psamp_header_descriptor = + packetlib::Header::descriptor()->FindFieldByName("psamp_header"); + if (psamp_header_descriptor == nullptr) { + FAIL() << "PSAMP header not found in packetlib"; + } + dvaas_params.switch_output_diff_params.ignored_packetlib_fields.push_back( + ipfix_header_descriptor); + dvaas_params.switch_output_diff_params.ignored_packetlib_fields.push_back( + psamp_header_descriptor); + const google::protobuf::FieldDescriptor* + ipv6_header_payload_length_descriptor = + packetlib::Ipv6Header::descriptor()->FindFieldByName( + "payload_length"); + if (ipv6_header_payload_length_descriptor == nullptr) { + FAIL() << "IPv6 header field payload_length not found in packetlib"; + } + dvaas_params.switch_output_diff_params.ignored_packetlib_fields.push_back( + ipv6_header_payload_length_descriptor); + const google::protobuf::FieldDescriptor* udp_header_length_descriptor = + packetlib::UdpHeader::descriptor()->FindFieldByName("length"); + if (udp_header_length_descriptor == nullptr) { + FAIL() << "UDP header field length not found in packetlib"; + } + const google::protobuf::FieldDescriptor* udp_header_checksum_descriptor = + packetlib::UdpHeader::descriptor()->FindFieldByName("checksum"); + if (udp_header_checksum_descriptor == nullptr) { + FAIL() << "UDP header field checksum not found in packetlib"; + } + dvaas_params.switch_output_diff_params.ignored_packetlib_fields.push_back( + udp_header_length_descriptor); + dvaas_params.switch_output_diff_params.ignored_packetlib_fields.push_back( + udp_header_checksum_descriptor); + const google::protobuf::FieldDescriptor* payload_descriptor = + packetlib::Packet::descriptor()->FindFieldByName("payload"); + if (payload_descriptor == nullptr) { + FAIL() << "Payload field not found in packetlib"; + } + dvaas_params.switch_output_diff_params.ignored_packetlib_fields.push_back( + payload_descriptor); + + // Send test packets. + LOG(INFO) << "Sending test packets."; + ASSERT_OK_AND_ASSIGN( + dvaas::ValidationResult validation_result, + GetParam().validator->ValidateDataplane(testbed, dvaas_params)); + // Validate traffic. + validation_result.LogStatistics(); + EXPECT_OK(validation_result.HasSuccessRateOfAtLeast(1.0)); +} + +} // anonymous namespace +} // namespace pins_test diff --git a/tests/packet_capture/packet_capture_test.h b/tests/packet_capture/packet_capture_test.h index 4342f4ae6..c07b22ec9 100644 --- a/tests/packet_capture/packet_capture_test.h +++ b/tests/packet_capture/packet_capture_test.h @@ -14,26 +14,41 @@ #ifndef PINS_TESTS_PACKET_CAPTURE_PACKET_CAPTURE_TEST_H_ #define PINS_TESTS_PACKET_CAPTURE_PACKET_CAPTURE_TEST_H_ -#include "gutil/status_matchers.h" + +#include +#include +#include +#include + +#include "dvaas/dataplane_validation.h" +#include "gtest/gtest.h" #include "p4_pdpi/packetlib/packetlib.pb.h" +#include "sai_p4/instantiations/google/instantiations.h" #include "thinkit/mirror_testbed.h" #include "thinkit/mirror_testbed_fixture.h" -#include "gtest/gtest.h" namespace pins_test { // Parameters used by tests that don't require an Ixia. -struct ParamsForPacketCaptureTestsWithoutIxia { +struct PacketCaptureTestWithoutIxiaParams { // Using a shared_ptr because parameterized tests require objects to be // copyable. std::shared_ptr testbed_interface; packetlib::Packet test_packet; + std::vector vlans_to_be_tested; // Ingress Packet VLAN IDs to be tested. + uint32_t cpu_port_id; + // If provided, installs the P4Info on the SUT. Otherwise, uses the P4Info + // already on the SUT. + std::optional sut_p4info; + sai::Instantiation sut_instantiation; + std::shared_ptr validator; + dvaas::DataplaneValidationParams validation_params; }; // These tests must be run on a testbed where the SUT is connected // to a "control device" that can send and received packets. class PacketCaptureTestWithoutIxia - : public testing::TestWithParam { -protected: + : public testing::TestWithParam { + protected: void SetUp() override { GetParam().testbed_interface->SetUp(); } thinkit::MirrorTestbed &Testbed() {