Skip to content

Commit

Permalink
[Dvaas] Add parameter to ignore packet-ins in switch output comparison.
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 599944244
  • Loading branch information
@kishanps @VSuryaprasad-HCL
kishanps authored and VSuryaprasad-HCL committed Nov 28, 2024
1 parent d4495ee commit 5b323e7
Show file tree
Hide file tree
Showing 3 changed files with 171 additions and 52 deletions.
114 changes: 62 additions & 52 deletions dvaas/test_run_validation.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,13 @@ bool CompareSwitchOutputs(SwitchOutput actual_output,
return false;
}

if (actual_output.packet_ins_size() != expected_output.packet_ins_size()) {
*listener << "has mismatched number of packet ins (actual: "
<< actual_output.packet_ins_size()
<< " expected: " << expected_output.packet_ins_size() << ")\n";
return false;
if (!params.treat_expected_and_actual_outputs_as_having_no_packet_ins) {
if (actual_output.packet_ins_size() != expected_output.packet_ins_size()) {
*listener << "has mismatched number of packet ins (actual: "
<< actual_output.packet_ins_size()
<< " expected: " << expected_output.packet_ins_size() << ")\n";
return false;
}
}

std::sort(actual_output.mutable_packets()->pointer_begin(),
Expand Down Expand Up @@ -139,58 +141,63 @@ bool CompareSwitchOutputs(SwitchOutput actual_output,
}
}

for (int i = 0; i < expected_output.packet_ins_size(); ++i) {
const PacketIn& actual_packet_in = actual_output.packet_ins(i);
const PacketIn& expected_packet_in = expected_output.packet_ins(i);

MessageDifferencer differ;
for (auto* field : params.ignored_packetlib_fields)
differ.IgnoreField(field);
std::string diff;
differ.ReportDifferencesToString(&diff);
if (!differ.Compare(expected_packet_in.parsed(),
actual_packet_in.parsed())) {
*listener << "has packet in " << i
<< " with mismatched header fields:\n " << Indent(2, diff);
return false;
}

// Check that received packet in has desired metadata (except for ignored
// metadata).
for (const auto& expected_metadata : expected_packet_in.metadata()) {
if (params.ignored_packet_in_metadata.contains(expected_metadata.name()))
continue;

std::optional<pdpi::IrPacketMetadata> actual_metadata =
GetPacketInMetadataByName(actual_packet_in, expected_metadata.name());
if (!actual_metadata.has_value()) {
*listener << "has packet in " << i << " with missing metadata field '"
<< expected_metadata.name() << "':\n " << Indent(2, diff);
return false;
}

if (!differ.Compare(expected_metadata.value(),
actual_metadata->value())) {
if (!params.treat_expected_and_actual_outputs_as_having_no_packet_ins) {
for (int i = 0; i < expected_output.packet_ins_size(); ++i) {
const PacketIn& actual_packet_in = actual_output.packet_ins(i);
const PacketIn& expected_packet_in = expected_output.packet_ins(i);

MessageDifferencer differ;
for (auto* field : params.ignored_packetlib_fields)
differ.IgnoreField(field);
std::string diff;
differ.ReportDifferencesToString(&diff);
if (!differ.Compare(expected_packet_in.parsed(),
actual_packet_in.parsed())) {
*listener << "has packet in " << i
<< " with mismatched value for metadata field '"
<< expected_metadata.name() << "':\n " << Indent(2, diff);
<< " with mismatched header fields:\n " << Indent(2, diff);
return false;
}
}

// Check that received packet in does not have additional metadata (except
// for ignored metadata).
for (const auto& actual_metadata : actual_packet_in.metadata()) {
if (params.ignored_packet_in_metadata.contains(actual_metadata.name()))
continue;
// Check that received packet in has desired metadata (except for ignored
// metadata).
for (const auto& expected_metadata : expected_packet_in.metadata()) {
if (params.ignored_packet_in_metadata.contains(
expected_metadata.name()))
continue;

std::optional<pdpi::IrPacketMetadata> actual_metadata =
GetPacketInMetadataByName(actual_packet_in,
expected_metadata.name());
if (!actual_metadata.has_value()) {
*listener << "has packet in " << i << " with missing metadata field '"
<< expected_metadata.name() << "':\n " << Indent(2, diff);
return false;
}

if (!differ.Compare(expected_metadata.value(),
actual_metadata->value())) {
*listener << "has packet in " << i
<< " with mismatched value for metadata field '"
<< expected_metadata.name() << "':\n " << Indent(2, diff);
return false;
}
}

std::optional<pdpi::IrPacketMetadata> expected_metadata =
GetPacketInMetadataByName(expected_packet_in, actual_metadata.name());
if (!expected_metadata.has_value()) {
*listener << "has packet in " << i
<< " with additional metadata field '"
<< actual_metadata.name() << "':\n " << Indent(2, diff);
return false;
// Check that received packet in does not have additional metadata (except
// for ignored metadata).
for (const auto& actual_metadata : actual_packet_in.metadata()) {
if (params.ignored_packet_in_metadata.contains(actual_metadata.name()))
continue;

std::optional<pdpi::IrPacketMetadata> expected_metadata =
GetPacketInMetadataByName(expected_packet_in,
actual_metadata.name());
if (!expected_metadata.has_value()) {
*listener << "has packet in " << i
<< " with additional metadata field '"
<< actual_metadata.name() << "':\n " << Indent(2, diff);
return false;
}
}
}
}
Expand Down Expand Up @@ -370,6 +377,9 @@ PacketTestValidationResult ValidateTestRun(
}),
")");
}
if (diff_params.treat_expected_and_actual_outputs_as_having_no_packet_ins) {
absl::StrAppend(&expectation, "\n (ignoring packet-ins)");
}
std::string actual = DescribeActual(test_run.test_vector().input(),
actual_output_characterization);
if (actual_matches_expected) {
Expand Down
8 changes: 8 additions & 0 deletions dvaas/test_run_validation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ struct SwitchOutputDiffParams {
// in `ignored_metadata_for_validation`, the field is ignored during
// comparison.
absl::flat_hash_set<std::string> ignored_packet_in_metadata;

// If true, packet-ins (punted packets) are ignored while comparing expected
// and actual output. Note that packet-ins are completely ignored on both
// sides. Effectively the expected and actual outputs are compared assuming
// neither has any packet-ins.
// TODO: Remove and replace with
// `ignore_missing_packet_ins_in_actual_output`.
bool treat_expected_and_actual_outputs_as_having_no_packet_ins = false;
};

// Validates the given `test_vector` using the parameters in `params` and
Expand Down
101 changes: 101 additions & 0 deletions dvaas/test_run_validation_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,5 +132,106 @@ TEST(TestRunValidationTest,
HasSubstr("mismatched ports:"));
}

TEST(TestRunValidationTest,
MissingPacketInsAreIgnoredIfAndOnlyIfIgnorePacketInsIsSet) {
const PacketTestRun test_run = gutil::ParseProtoOrDie<PacketTestRun>(R"pb(
test_vector {
acceptable_outputs {
packets { port: "1" }
packet_ins {
metadata {
name: "target_egress_port"
value { str: "1" }
}
}
}
}
actual_output { packets { port: "1" } }
)pb");

// Without ignoring packet-ins, validation must fail.
ASSERT_THAT(ValidateTestRun(test_run).failure().description(),
HasSubstr("packet in"));

// Ignoring packet-ins, validation must succeed.
ASSERT_THAT(
ValidateTestRun(
test_run,
{
.treat_expected_and_actual_outputs_as_having_no_packet_ins = true,
}),
EqualsProto(R"pb()pb"));
}

TEST(TestRunValidationTest,
PacketInDifferencesAreIgnoredIfAndOnlyIfIgnorePacketInsIsSet) {
const PacketTestRun test_run = gutil::ParseProtoOrDie<PacketTestRun>(R"pb(
test_vector {
acceptable_outputs {
packets { port: "1" }
packet_ins {
metadata {
name: "target_egress_port"
value { str: "1" }
}
}
}
}
actual_output {
packets { port: "1" }
packet_ins {
metadata {
name: "target_egress_port"
value { str: "2" }
}
}
}
)pb");

// Without ignoring packet-ins, validation must fail.
ASSERT_THAT(ValidateTestRun(test_run).failure().description(),
HasSubstr("target_egress_port"));

// Ignoring packet-ins, validation must succeed.
ASSERT_THAT(
ValidateTestRun(
test_run,
{
.treat_expected_and_actual_outputs_as_having_no_packet_ins = true,
}),
EqualsProto(R"pb()pb"));
}

TEST(TestRunValidationTest, IgnorePacketInsHasNoEffectWhenPacketInsMatch) {
ASSERT_THAT(
ValidateTestRun(
gutil::ParseProtoOrDie<PacketTestRun>(R"pb(
test_vector {
acceptable_outputs {
packets { port: "1" }
packet_ins {
metadata {
name: "target_egress_port"
value { str: "1" }
}
}
}
}
actual_output {
packets { port: "1" }
packet_ins {
metadata {
name: "target_egress_port"
value { str: "1" }
}
}
}
)pb"),
{
.treat_expected_and_actual_outputs_as_having_no_packet_ins = true,
}),
EqualsProto(R"pb()pb"));
}

} // namespace
} // namespace dvaas

0 comments on commit 5b323e7

Please sign in to comment.