Skip to content

Commit

Permalink
[Fuzzer] Allow multiples of the same next hop in a single Action Prof…
Browse files Browse the repository at this point in the history 
…ile Action Set. Improve summary message. Add additional information to state summary for WCMP tables. Update Oracle to use IrUpdateStatus instead of Error. Update Oracle to use AnnotatedUpdates instead of p4 updates. Update test_utils to use the pre-designed test p4 program. (#575)

Co-authored-by: kishanps <kishanps@google.com>
Co-authored-by: jonathan-dilorenzo <dilo@google.com>
Co-authored-by: rhalstea <rhalstea@google.com>
  • Loading branch information
@VSuryaprasad-HCL @kishanps
@jonathan-dilorenzo @rhalstea
4 people committed Sep 25, 2024
1 parent 5fc1da0 commit 59c5911
Show file tree
Hide file tree
Showing 13 changed files with 364 additions and 281 deletions.
15 changes: 15 additions & 0 deletions p4_fuzzer/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ cc_library(
"//gutil:collections",
"//gutil:status",
"//p4_pdpi:ir_cc_proto",
"//p4_pdpi/internal:ordered_map",
"@com_github_google_glog//:glog",
"@com_github_p4lang_p4runtime//:p4info_cc_proto",
"@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
Expand All @@ -45,9 +46,15 @@ cc_test(
srcs = ["switch_state_test.cc"],
deps = [
":switch_state",
":test_utils",
"//gutil:collections",
"//gutil:status_matchers",
"//p4_pdpi:ir",
"//p4_pdpi/testing:main_p4_pd_cc_proto",
"//p4_pdpi/testing:test_p4info_cc",
"@com_github_google_glog//:glog",
"@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
"@com_google_absl//absl/strings",
"@com_google_googletest//:gtest_main",
],
)
Expand Down Expand Up @@ -180,10 +187,12 @@ cc_library(
],
hdrs = ["oracle_util.h"],
deps = [
":fuzzer_cc_proto",
":switch_state",
":table_entry_key",
"//gutil:status",
"//p4_pdpi:ir",
"//p4_pdpi:ir_cc_proto",
"@com_github_p4lang_p4runtime//:p4info_cc_proto",
"@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
"@com_google_absl//absl/algorithm:container",
Expand All @@ -198,6 +207,7 @@ cc_test(
name = "oracle_util_test",
srcs = ["oracle_util_test.cc"],
deps = [
":fuzzer_cc_proto",
":mutation_and_fuzz_util",
":oracle_util",
":test_utils",
Expand All @@ -214,7 +224,9 @@ cc_test(
"@com_github_p4lang_p4runtime//:p4info_cc_proto",
"@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
"@com_google_googleapis//google/rpc:code_cc_proto",
"@com_google_googletest//:gtest_main",
],
)
Expand Down Expand Up @@ -304,9 +316,12 @@ cc_library(
":fuzzer_config",
":mutation_and_fuzz_util",
":switch_state",
"//gutil:collections",
"//gutil:testing",
"//p4_pdpi:ir",
"//p4_pdpi:ir_cc_proto",
"//p4_pdpi/internal:ordered_map",
"//p4_pdpi/testing:test_p4info_cc",
"//sai_p4/instantiations/google:sai_p4info_cc",
"@com_github_p4lang_p4runtime//:p4info_cc_proto",
"@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
Expand Down
25 changes: 7 additions & 18 deletions p4_fuzzer/fuzz_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ std::vector<uint32_t> TablesUsedByFuzzer(const FuzzerConfig& config) {
if (table.role() != config.role) continue;
// TODO: the switch is currently having issues with this table.
if (table.preamble().alias() == "mirror_session_table") continue;
// Tables without actions cannot have valid table entries.
if (table.entry_actions().empty()) continue;
table_ids.push_back(key);
}
return table_ids;
Expand Down Expand Up @@ -789,13 +791,6 @@ absl::StatusOr<p4::v1::ActionProfileActionSet> FuzzActionProfileActionSet(
absl::IntervalClosedClosed, *gen, 0,
std::min(unallocated_weight, kActionProfileActionSetMaxCardinality));

// TODO: Repeated nexthop members should be supported. Remove
// this workaround once the bug on the switch has been fixed.
// Action sets in GPINS cannot have repeated nexthop members. We hard-code
// this restriction here in the fuzzer.
absl::flat_hash_set<std::string> used_nexthops;
bool is_wcmp_table =
ir_table_info.preamble().id() == ROUTING_WCMP_GROUP_TABLE_ID;
for (int i = 0; i < number_of_actions; i++) {
// Since each action must have at least weight 1, we need to take the number
// of remaining actions into account to determine the acceptable max weight.
Expand All @@ -805,16 +800,6 @@ absl::StatusOr<p4::v1::ActionProfileActionSet> FuzzActionProfileActionSet(
ASSIGN_OR_RETURN(auto action,
FuzzActionProfileAction(gen, config, switch_state,
ir_table_info, max_weight));

bool is_set_nexthop_action =
action.action().action_id() == ROUTING_SET_NEXTHOP_ID_ACTION_ID;
// If this nexthop has already been used, skip. This will generate fewer
// actions, but that's fine.
if (is_wcmp_table && is_set_nexthop_action &&
action.action().params_size() == 1 &&
used_nexthops.insert(action.action().params()[0].value()).second) {
continue;
}
*action_set.add_action_profile_actions() = action;
unallocated_weight -= action.weight();
}
Expand Down Expand Up @@ -958,10 +943,14 @@ std::vector<AnnotatedTableEntry> ValidForwardingEntries(

AnnotatedWriteRequest FuzzWriteRequest(absl::BitGen* gen,
const FuzzerConfig& config,
const SwitchState& switch_state) {
const SwitchState& switch_state,absl::optional<int> max_batch_size) {
AnnotatedWriteRequest request;

while (absl::Bernoulli(*gen, kAddUpdateProbability)) {
if (max_batch_size.has_value() &&
request.updates_size() >= *max_batch_size) {
break;
}
*request.add_updates() = FuzzUpdate(gen, config, switch_state);
}

Expand Down
7 changes: 4 additions & 3 deletions p4_fuzzer/fuzz_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,10 @@ std::vector<AnnotatedTableEntry> ValidForwardingEntries(
absl::BitGen* gen, const FuzzerConfig& config, const int num_entries);

// Randomly generates a set of updates, both valid and invalid.
AnnotatedWriteRequest FuzzWriteRequest(absl::BitGen* gen,
const FuzzerConfig& config,
const SwitchState& switch_state);
AnnotatedWriteRequest FuzzWriteRequest(
absl::BitGen* gen, const FuzzerConfig& config,
const SwitchState& switch_state,
absl::optional<int> max_batch_size = absl::nullopt);

// Takes a P4 Runtime table and returns randomly chosen action ref from the
// action refs that are not in default only scope.
Expand Down
121 changes: 65 additions & 56 deletions p4_fuzzer/fuzz_util_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,57 +147,44 @@ TEST(FuzzUtilTest, FuzzUint64LargeInRange) {
}
}

TEST(FuzzUtilTest, FuzzWriteRequestAreReproducible) {
ASSERT_OK_AND_ASSIGN(const FuzzerTestState fuzzer_state,
ConstructFuzzerTestState(TestP4InfoOptions()));

// Use the same sequence seed for both generators.
absl::SeedSeq seed;
absl::BitGen gen_0(seed);
absl::BitGen gen_1(seed);

// Create 20 instances (of, in expectation, ~50 updates each), and verify that
// they are identical.
for (int i = 0; i < 20; ++i) {
ASSERT_THAT(FuzzWriteRequest(&gen_0, fuzzer_state.config,
fuzzer_state.switch_state),
EqualsProto(FuzzWriteRequest(&gen_1, fuzzer_state.config,
fuzzer_state.switch_state)));
}
}

// Test that FuzzActionProfileActionSet correctly generates an ActionProfile
// Action Set of acceptable weights and size (derived from max_group_size and
// kActionProfileActionSetMaxCardinality).
TEST(FuzzActionProfileActionSetTest,
StaysWithinMaxGroupSizeAndCardinalityParameters) {
absl::BitGen gen;
// As implied by the name, 1000 is basically arbitrarily chosen and anything
// above that would be just as good. Lower is probably worse though.
const int kGroupSizeArbitraryUpperBound = 1000;
FuzzerTestState fuzzer_state = ConstructStandardFuzzerTestState();
ASSERT_OK_AND_ASSIGN(const pdpi::IrTableDefinition& table_definition,
GetAOneShotTableDefinition(fuzzer_state.config.info));
ASSERT_OK_AND_ASSIGN(
pdpi::IrActionProfileDefinition action_profile_definition,
GetActionProfileImplementingTable(fuzzer_state.config.info,
table_definition));
for (int i = 0; i < 1000; ++i) {
// Tests a broad enough band of max weights to give us interesting coverage
// while being narrow enough to likely catch issues when they happen.
int max_group_size =
absl::Uniform<int>(gen, kActionProfileActionSetMaxCardinality, 10000);
auto options =
TestP4InfoOptions{.action_profile_max_group_size = max_group_size};
ASSERT_OK_AND_ASSIGN(FuzzerTestState fuzzer_state,
ConstructFuzzerTestState(options));
const pdpi::IrTableDefinition& table_definition =
fuzzer_state.config.info.tables_by_id().at(
options.action_selector_table_id);
int max_group_size = absl::Uniform<int>(
fuzzer_state.gen, kActionProfileActionSetMaxCardinality,
kGroupSizeArbitraryUpperBound);

SetMaxGroupSizeInActionProfile(fuzzer_state.config.info,
action_profile_definition, max_group_size);

// Fuzz an ActionProfileActionSet.
ASSERT_OK_AND_ASSIGN(auto action_profile_set,
ASSERT_OK_AND_ASSIGN(auto action_profile_action_set,
FuzzActionProfileActionSet(
&fuzzer_state.gen, fuzzer_state.config,
fuzzer_state.switch_state, table_definition));

// The number of actions should always be less than or equal to the max
// cardinality.
EXPECT_LE(action_profile_set.action_profile_actions_size(),
EXPECT_LE(action_profile_action_set.action_profile_actions_size(),
kActionProfileActionSetMaxCardinality);

int total_weight = 0;
for (auto& action : action_profile_set.action_profile_actions()) {
for (auto& action : action_profile_action_set.action_profile_actions()) {
total_weight += action.weight();
}
EXPECT_LE(total_weight, max_group_size);
Expand All @@ -207,67 +194,89 @@ TEST(FuzzActionProfileActionSetTest,
// Test that FuzzActionProfileActionSet correctly generates an ActionProfile
// Action Set of acceptable weights and size when max_group_size is set to 0.
TEST(FuzzActionProfileActionSetTest, HandlesZeroMaxGroupSizeCorrectly) {
auto options = TestP4InfoOptions{.action_profile_max_group_size = 0};
FuzzerTestState fuzzer_state = ConstructStandardFuzzerTestState();
ASSERT_OK_AND_ASSIGN(const pdpi::IrTableDefinition& table_definition,
GetAOneShotTableDefinition(fuzzer_state.config.info));
ASSERT_OK_AND_ASSIGN(
pdpi::IrActionProfileDefinition action_profile_definition,
GetActionProfileImplementingTable(fuzzer_state.config.info,
table_definition));
SetMaxGroupSizeInActionProfile(fuzzer_state.config.info,
action_profile_definition,
/*max_group_size=*/0);
for (int i = 0; i < 1000; ++i) {
ASSERT_OK_AND_ASSIGN(FuzzerTestState fuzzer_state,
ConstructFuzzerTestState(options));
const pdpi::IrTableDefinition& table_definition =
fuzzer_state.config.info.tables_by_id().at(
options.action_selector_table_id);

// Fuzz an ActionProfileActionSet.
ASSERT_OK_AND_ASSIGN(auto action_profile_set,
ASSERT_OK_AND_ASSIGN(auto action_profile_action_set,
FuzzActionProfileActionSet(
&fuzzer_state.gen, fuzzer_state.config,
fuzzer_state.switch_state, table_definition));

// The number of actions should always be less than or equal to the max
// cardinality.
EXPECT_LE(action_profile_set.action_profile_actions_size(),
EXPECT_LE(action_profile_action_set.action_profile_actions_size(),
kActionProfileActionSetMaxCardinality);

int total_weight = 0;
for (auto& action : action_profile_set.action_profile_actions()) {
for (auto& action : action_profile_action_set.action_profile_actions()) {
total_weight += action.weight();
}
// When max_group_size is set to 0, size is the upperbound for weight.
EXPECT_LE(total_weight, options.action_profile_size);
EXPECT_LE(total_weight, action_profile_definition.action_profile().size());
}
}

// Test that FuzzActionProfileActionSet correctly handles a request with low max
// group size (in particular, lower than the max number of actions).
TEST(FuzzActionProfileActionSetTest, HandlesLowMaxGroupSizeCorrectly) {
absl::BitGen gen;
FuzzerTestState fuzzer_state = ConstructStandardFuzzerTestState();
ASSERT_OK_AND_ASSIGN(const pdpi::IrTableDefinition& table_definition,
GetAOneShotTableDefinition(fuzzer_state.config.info));
ASSERT_OK_AND_ASSIGN(
pdpi::IrActionProfileDefinition action_profile_definition,
GetActionProfileImplementingTable(fuzzer_state.config.info,
table_definition));
for (int i = 0; i < 1000; ++i) {
// Set up.
int max_group_size =
absl::Uniform<int>(gen, 1, kActionProfileActionSetMaxCardinality);
auto options =
TestP4InfoOptions{.action_profile_max_group_size = max_group_size};
ASSERT_OK_AND_ASSIGN(FuzzerTestState fuzzer_state,
ConstructFuzzerTestState(options));
const pdpi::IrTableDefinition& table_definition =
fuzzer_state.config.info.tables_by_id().at(
options.action_selector_table_id);
const int max_group_size = absl::Uniform<int>(
fuzzer_state.gen, 1, kActionProfileActionSetMaxCardinality);

ASSERT_OK_AND_ASSIGN(auto action_profile_set,
SetMaxGroupSizeInActionProfile(fuzzer_state.config.info,
action_profile_definition, max_group_size);

ASSERT_OK_AND_ASSIGN(auto action_profile_action_set,
FuzzActionProfileActionSet(
&fuzzer_state.gen, fuzzer_state.config,
fuzzer_state.switch_state, table_definition));

// The number of actions must be less than max_group_size since every
// action has at least weight 1.
EXPECT_LE(action_profile_set.action_profile_actions_size(), max_group_size);
EXPECT_LE(action_profile_action_set.action_profile_actions_size(),
max_group_size);

int total_weight = 0;
for (auto& action : action_profile_set.action_profile_actions()) {
for (auto& action : action_profile_action_set.action_profile_actions()) {
total_weight += action.weight();
}
EXPECT_LE(total_weight, max_group_size);
}
}

TEST(FuzzUtilTest, FuzzWriteRequestRespectMaxBatchSize) {
FuzzerTestState fuzzer_state = ConstructStandardFuzzerTestState();

// Create 200 instances of, in expectation, ~50 updates each without the
// max_batch_size parameter and verify that they all have batches smaller than
// max_batch_size.
for (int i = 0; i < 200; ++i) {
int max_batch_size = absl::Uniform<int>(fuzzer_state.gen, 0, 50);
EXPECT_LE(FuzzWriteRequest(&fuzzer_state.gen, fuzzer_state.config,
fuzzer_state.switch_state, max_batch_size)
.updates_size(),
max_batch_size)
<< absl::StrCat(" using max_batch_size=", max_batch_size);
}
}

// TODO: Add a direct test for FuzzValue that either sometimes
// generates something for non-standard match fields, or, if that is never
// correct, makes sure it still works with that possibility removed.
Expand Down
13 changes: 9 additions & 4 deletions p4_fuzzer/fuzzer.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ enum Mutation {

// Attempts to modify a table entry that does not exist in the switch.
NONEXISTING_MODIFY = 15;

// TODO: New mutation: Table entry without any actions.
// TODO: New mutation: Table entry with an action that exists,
// but is invalid for the table.
// TODO: New mutation: Delete/Insert with invalid references.
}

// Human readable version of a (fuzzed) table entry. Contains the original
Expand All @@ -96,14 +101,14 @@ message AnnotatedTableEntry {

// Like AnnotatedTableEntry, but for updates instead.
message AnnotatedUpdate {
p4.v1.Update pi = 1;

oneof pd {
string error = 2;
string error = 1;

pdpi.IrUpdate ir = 3;
pdpi.IrUpdate ir = 2;
}

p4.v1.Update pi = 3;

repeated Mutation mutations = 4;
}

Expand Down
Loading

0 comments on commit 59c5911

Please sign in to comment.