Skip to content

Commit

Permalink
Merge branch 'main' of https://github.com/VSuryaprasad-HCL/sonic-pins …
Browse files Browse the repository at this point in the history 
…into symbolic_import_branch_126
  • Loading branch information
@VSuryaprasad-HCL
VSuryaprasad-HCL committed Dec 6, 2024
2 parents ebcc1a0 + 756827b commit a071857
Show file tree
Hide file tree
Showing 38 changed files with 2,469 additions and 520 deletions.
4 changes: 3 additions & 1 deletion dvaas/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,9 @@ cc_library(
":test_vector_cc_proto",
"//gutil:proto",
"//gutil:status",
"//p4_pdpi:ir",
"//p4_pdpi/packetlib",
"@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
Expand All @@ -274,6 +276,7 @@ cc_test(
"//gutil:status_matchers",
"//gutil:testing",
"//p4_pdpi/packetlib:packetlib_cc_proto",
"//p4_pdpi/testing:test_p4info_cc",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
Expand Down Expand Up @@ -376,7 +379,6 @@ cc_library(
"//lib/gnmi:gnmi_helper",
"//lib/gnmi:openconfig_cc_proto",
"//lib/p4rt:p4rt_port",
"//p4_pdpi:ir",
"//p4_pdpi:ir_cc_proto",
"//p4_pdpi:p4_runtime_session",
"//p4_pdpi:p4_runtime_session_extras",
Expand Down
7 changes: 4 additions & 3 deletions dvaas/dataplane_validation.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
#include "lib/gnmi/openconfig.pb.h"
#include "lib/p4rt/p4rt_port.h"
#include "p4/v1/p4runtime.pb.h"
#include "p4_pdpi/ir.h"
#include "p4_pdpi/ir.pb.h"
#include "p4_pdpi/p4_runtime_session.h"
#include "p4_pdpi/p4_runtime_session_extras.h"
Expand Down Expand Up @@ -254,8 +253,10 @@ absl::StatusOr<ValidationResult> DataplaneValidator::ValidateDataplane(
GenerateTestVectors(params, sut, *backend_, *writer));
} else {
LOG(INFO) << "Checking user-provided test vectors for well-formedness";
ASSIGN_OR_RETURN(test_vectors, LegitimizeUserProvidedTestVectors(
params.packet_test_vector_override));
ASSIGN_OR_RETURN(pdpi::IrP4Info ir_info, pdpi::GetIrP4Info(*sut.p4rt));
ASSIGN_OR_RETURN(test_vectors,
LegitimizeUserProvidedTestVectors(
params.packet_test_vector_override, ir_info));
}
RETURN_IF_ERROR(
writer->AppendToTestArtifact("test_vectors.txt", ToString(test_vectors)));
Expand Down
13 changes: 11 additions & 2 deletions dvaas/packet_injection.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,19 @@ absl::StatusOr<PacketTestRuns> SendTestPacketsAndCollectOutputs(
if (packet_test_vector.input().type() == SwitchInput::DATAPLANE) {
const Packet& packet = packet_test_vector.input().packet();

// Get corresponding control switch port for the packet's ingress port.
ASSIGN_OR_RETURN(const P4rtPortId sut_ingress_port,
P4rtPortId::MakeFromP4rtEncoding(packet.port()));
ASSIGN_OR_RETURN(
const P4rtPortId control_switch_port,
parameters.mirror_testbed_port_map
.GetControlSwitchPortConnectedToSutPort(sut_ingress_port));

// Inject to egress of control switch.
RETURN_IF_ERROR(pins::InjectEgressPacket(
packet.port(), absl::HexStringToBytes(packet.hex()),
control_ir_p4info, &control_switch, injection_delay));
control_switch_port.GetP4rtEncoding(),
absl::HexStringToBytes(packet.hex()), control_ir_p4info,
&control_switch, injection_delay));
} else {
return absl::UnimplementedError(
absl::StrCat("Test vector input type not supported\n",
Expand Down
23 changes: 21 additions & 2 deletions dvaas/port_id_map.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,28 @@ MirrorTestbedP4rtPortIdMap::GetSutPortConnectedToControlSwitchPort(
absl::Substitute("Control port '$0' was not found in control switch "
"to SUT P4RT port ID map.",
control_port));
} else {
return it->second;
}
return it->second;
}

absl::StatusOr<pins_test::P4rtPortId>
MirrorTestbedP4rtPortIdMap::GetControlSwitchPortConnectedToSutPort(
const pins_test::P4rtPortId& sut_port) const {
// Handle implicit identity map.
if (!control_to_sut_port_map_.has_value()) return sut_port;

// Handle explicit map.
const auto it = absl::c_find_if(*control_to_sut_port_map_,
[&](const auto& control_sut_port) {
return control_sut_port.second == sut_port;
});
if (it == control_to_sut_port_map_->end()) {
return absl::NotFoundError(
absl::Substitute("SUT port '$0' was not found in control switch "
"to control switch P4RT port ID map.",
sut_port));
}
return it->first;
}

absl::Status CheckAndStoreMappedAndUnmappedPortIds(
Expand Down
6 changes: 6 additions & 0 deletions dvaas/port_id_map.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ class MirrorTestbedP4rtPortIdMap {
absl::StatusOr<pins_test::P4rtPortId> GetSutPortConnectedToControlSwitchPort(
const pins_test::P4rtPortId& control_port) const;

// Returns the P4RT port ID of the control switch interface connected to the
// interface on the SUT with the given P4RT port ID according to the port
// mapping.
absl::StatusOr<pins_test::P4rtPortId> GetControlSwitchPortConnectedToSutPort(
const pins_test::P4rtPortId& sut_port) const;

private:
MirrorTestbedP4rtPortIdMap(
absl::flat_hash_map<pins_test::P4rtPortId, pins_test::P4rtPortId>
Expand Down
57 changes: 30 additions & 27 deletions dvaas/port_id_map_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,17 @@ TEST(MirrorTestbedP4rtPortIdMap,
MirrorTestbedP4rtPortIdMap::CreateFromControlSwitchToSutPortMap(
{{port_1, port_2}}));

// Control -> SUT.
ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_2),
StatusIs(absl::StatusCode::kNotFound));
ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_3),
StatusIs(absl::StatusCode::kNotFound));

// SUT -> Control.
ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_1),
StatusIs(absl::StatusCode::kNotFound));
ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_3),
StatusIs(absl::StatusCode::kNotFound));
}

TEST(MirrorTestbedP4rtPortIdMap,
Expand Down Expand Up @@ -79,10 +86,17 @@ TEST(MirrorTestbedP4rtPortIdMap,
MirrorTestbedP4rtPortIdMap::CreateFromSutToControlSwitchPortMap(
{{port_1, port_2}}));

// Control -> SUT.
ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_1),
StatusIs(absl::StatusCode::kNotFound));
ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_3),
StatusIs(absl::StatusCode::kNotFound));

// SUT -> Control.
ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_2),
StatusIs(absl::StatusCode::kNotFound));
ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_3),
StatusIs(absl::StatusCode::kNotFound));
}

TEST(MirrorTestbedP4rtPortIdMap,
Expand All @@ -101,32 +115,7 @@ TEST(MirrorTestbedP4rtPortIdMap,
StatusIs(absl::StatusCode::kInvalidArgument));
}

TEST(MirrorTestbedP4rtPortIdMap,
RetrunsCorrectSutPortGivenControlPortGivenControlToSutMapping) {
ASSERT_OK_AND_ASSIGN(const auto port_1,
P4rtPortId::MakeFromP4rtEncoding("1"));
ASSERT_OK_AND_ASSIGN(const auto port_2,
P4rtPortId::MakeFromP4rtEncoding("2"));
ASSERT_OK_AND_ASSIGN(const auto port_3,
P4rtPortId::MakeFromP4rtEncoding("3"));
ASSERT_OK_AND_ASSIGN(const auto port_4,
P4rtPortId::MakeFromP4rtEncoding("4"));

ASSERT_OK_AND_ASSIGN(
const auto port_id_map,
MirrorTestbedP4rtPortIdMap::CreateFromControlSwitchToSutPortMap({
{port_1, port_2},
{port_3, port_4},
}));

ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_1),
IsOkAndHolds(Eq(port_2)));
ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_3),
IsOkAndHolds(Eq(port_4)));
}

TEST(MirrorTestbedP4rtPortIdMap,
RetrunsCorrectSutPortGivenControlPortGivenSutToControlMapping) {
TEST(MirrorTestbedP4rtPortIdMap, ReturnsCorrectPortGivenSutToControlMapping) {
ASSERT_OK_AND_ASSIGN(const auto port_1,
P4rtPortId::MakeFromP4rtEncoding("1"));
ASSERT_OK_AND_ASSIGN(const auto port_2,
Expand All @@ -143,23 +132,37 @@ TEST(MirrorTestbedP4rtPortIdMap,
{port_3, port_4},
}));

// Control -> SUT.
ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_2),
IsOkAndHolds(Eq(port_1)));
ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_4),
IsOkAndHolds(Eq(port_3)));

// SUT -> Control.
ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_1),
IsOkAndHolds(Eq(port_2)));
ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_3),
IsOkAndHolds(Eq(port_4)));
}

TEST(MirrorTestbedP4rtPortIdMap, RetrunsCorrectSutPortWithImplicitIdentityMap) {
TEST(MirrorTestbedP4rtPortIdMap, ReturnsCorrectPortWithImplicitIdentityMap) {
const auto port_id_map = MirrorTestbedP4rtPortIdMap::CreateIdentityMap();
ASSERT_OK_AND_ASSIGN(const auto port_1,
P4rtPortId::MakeFromP4rtEncoding("1"));
ASSERT_OK_AND_ASSIGN(const auto port_2,
P4rtPortId::MakeFromP4rtEncoding("2"));

// Control -> SUT.
ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_1),
IsOkAndHolds(Eq(port_1)));
ASSERT_THAT(port_id_map.GetSutPortConnectedToControlSwitchPort(port_2),
IsOkAndHolds(Eq(port_2)));

// SUT -> Control.
ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_1),
IsOkAndHolds(Eq(port_1)));
ASSERT_THAT(port_id_map.GetControlSwitchPortConnectedToSutPort(port_2),
IsOkAndHolds(Eq(port_2)));
}

} // namespace
Expand Down
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
Loading

0 comments on commit a071857

Please sign in to comment.