Skip to content

Commit

Permalink
[Thinkit] Add a helper to return the config disabled interfaces.Add i…
Browse files Browse the repository at this point in the history 
…nterface name to error status to improve debugability.Generalize some functions from string& to string_view. (sonic-net#458)



Co-authored-by: Srikishen Pondicherry Shanmugam <kishanps@google.com>
  • Loading branch information
@bibhuprasad-hcl @kishanps
bibhuprasad-hcl and kishanps authored Aug 10, 2024
1 parent 8145e31 commit 8c5dc98
Show file tree
Hide file tree
Showing 3 changed files with 187 additions and 11 deletions.
26 changes: 20 additions & 6 deletions lib/gnmi/gnmi_helper.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ absl::Status UpdateAndVerifyGnmiConfigLeaf(gnmi::gNMI::StubInterface* gnmi_stub,

absl::Status PushGnmiConfig(gnmi::gNMI::StubInterface& stub,
const std::string& chassis_name,
const std::string& gnmi_config,
absl::string_view gnmi_config,
absl::uint128 election_id) {
gnmi::SetRequest req;
req.mutable_prefix()->set_origin("openconfig");
Expand All @@ -700,7 +700,7 @@ absl::Status PushGnmiConfig(gnmi::gNMI::StubInterface& stub,
}

absl::Status PushGnmiConfig(thinkit::Switch& chassis,
const std::string& gnmi_config) {
absl::string_view gnmi_config) {
ASSIGN_OR_RETURN(auto stub, chassis.CreateGnmiStub());
return pins_test::PushGnmiConfig(
*stub, chassis.ChassisName(),
Expand All @@ -709,7 +709,7 @@ absl::Status PushGnmiConfig(thinkit::Switch& chassis,
}

absl::Status WaitForGnmiPortIdConvergence(gnmi::gNMI::StubInterface& stub,
const std::string& gnmi_config,
absl::string_view gnmi_config,
const absl::Duration& timeout) {
absl::Time deadline = absl::Now() + timeout;

Expand Down Expand Up @@ -751,7 +751,7 @@ absl::Status WaitForGnmiPortIdConvergence(gnmi::gNMI::StubInterface& stub,
}

absl::Status WaitForGnmiPortIdConvergence(thinkit::Switch& chassis,
const std::string& gnmi_config,
absl::string_view gnmi_config,
const absl::Duration& timeout) {
ASSIGN_OR_RETURN(auto stub, chassis.CreateGnmiStub());
return WaitForGnmiPortIdConvergence(*stub, gnmi_config, timeout);
Expand Down Expand Up @@ -1000,6 +1000,19 @@ absl::StatusOr<std::vector<std::string>> GetUpInterfacesOverGnmi(
return up_interfaces;
}

absl::StatusOr<absl::flat_hash_set<std::string>> GetConfigDisabledInterfaces(
gnmi::gNMI::StubInterface& stub) {
absl::flat_hash_set<std::string> disabled_interfaces;
ASSIGN_OR_RETURN(auto all_interfaces, pins_test::GetInterfacesAsProto(
stub, gnmi::GetRequest::CONFIG));
for (const auto& interface : all_interfaces.interfaces()) {
if (interface.config().enabled() == false) {
disabled_interfaces.insert(interface.name());
}
}
return disabled_interfaces;
}

absl::StatusOr<OperStatus> GetInterfaceOperStatusOverGnmi(
gnmi::gNMI::StubInterface& stub, absl::string_view if_name) {
std::string if_req = absl::StrCat("interfaces/interface[name=", if_name,
Expand Down Expand Up @@ -1642,7 +1655,7 @@ absl::StatusOr<std::vector<std::string>> GetInterfacesOnPort(
return FindInterfacesNameFromInterfaceJsonArray(port_number, interface_array);
}

std::string UpdateDeviceIdInJsonConfig(const std::string& gnmi_config,
std::string UpdateDeviceIdInJsonConfig(absl::string_view gnmi_config,
const std::string& device_id) {
LOG(INFO) << "Forcing P4RT device ID to be '" << device_id << "'.";

Expand Down Expand Up @@ -1933,7 +1946,8 @@ GetAllInterfaceCounters(gnmi::gNMI::StubInterface& gnmi_stub) {
continue;
}
Counters& port_counters = counters[name.get<std::string>()];
ASSIGN_OR_RETURN(port_counters, GetCountersForInterface(interface));
ASSIGN_OR_RETURN(port_counters, GetCountersForInterface(interface),
_.SetPrepend() << name.get<std::string>() << " -> ");
port_counters.timestamp_ns = timestamp;
}
return counters;
Expand Down
14 changes: 9 additions & 5 deletions lib/gnmi/gnmi_helper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,20 +243,20 @@ GnmiGetElementFromTelemetryResponse(const gnmi::SubscribeResponse& response);
// configure the arbitration settings for the request.
absl::Status PushGnmiConfig(
gnmi::gNMI::StubInterface& stub, const std::string& chassis_name,
const std::string& gnmi_config,
absl::string_view gnmi_config,
absl::uint128 election_id = pdpi::TimeBasedElectionId());

// Pushes a given gNMI config to a thinkit switch. This method will make
// sensible changes to the config like:
// * Update the P4RT device ID to match the chassis settings.
absl::Status PushGnmiConfig(thinkit::Switch& chassis,
const std::string& gnmi_config);
absl::string_view gnmi_config);

absl::Status WaitForGnmiPortIdConvergence(gnmi::gNMI::StubInterface& stub,
const std::string& gnmi_config,
absl::string_view gnmi_config,
const absl::Duration& timeout);
absl::Status WaitForGnmiPortIdConvergence(thinkit::Switch& chassis,
const std::string& gnmi_config,
absl::string_view gnmi_config,
const absl::Duration& timeout);

// Waits until the interface <-> P4RT port id mappings in the config path of the
Expand Down Expand Up @@ -302,6 +302,10 @@ inline absl::StatusOr<std::vector<std::string>> GetUpInterfacesOverGnmi(
return GetUpInterfacesOverGnmi(stub, InterfaceType::kAny, timeout);
}

// Returns a set of interfaces which are in the disabled state.
absl::StatusOr<absl::flat_hash_set<std::string>> GetConfigDisabledInterfaces(
gnmi::gNMI::StubInterface& stub);

// Gets the operational status of an interface.
absl::StatusOr<OperStatus> GetInterfaceOperStatusOverGnmi(
gnmi::gNMI::StubInterface& stub, absl::string_view if_name);
Expand Down Expand Up @@ -462,7 +466,7 @@ absl::StatusOr<uint64_t> GetDeviceId(gnmi::gNMI::StubInterface& gnmi_stub);

// Takes a gNMI config in JSON format and updates the P4RT Device ID. Adding it
// when it doesn't exist, or updating the value if it does.
std::string UpdateDeviceIdInJsonConfig(const std::string& gnmi_config,
std::string UpdateDeviceIdInJsonConfig(absl::string_view gnmi_config,
const std::string& device_id);

// Return the port id whose breakout mode matches the given input.
Expand Down
158 changes: 158 additions & 0 deletions lib/gnmi/gnmi_helper_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1329,6 +1329,32 @@ TEST(GetInterfacePortIdMap, InvalidGnmiGetResponse) {
StatusIs(absl::StatusCode::kInternal, HasSubstr("Invalid response")));
}

TEST(GetConfigDisabledInterfaces, RpcFails) {
gnmi::MockgNMIStub stub;
EXPECT_CALL(stub, Get).WillOnce(
Return(grpc::Status(grpc::StatusCode::DEADLINE_EXCEEDED, "")));
EXPECT_THAT(GetConfigDisabledInterfaces(stub),
StatusIs(absl::StatusCode::kDeadlineExceeded));
}

TEST(GetConfigDisabledInterfaces, RpcSucceeds) {
gnmi::MockgNMIStub stub;
EXPECT_CALL(stub, Get).WillOnce(DoAll(
SetArgPointee<2>(gutil::ParseProtoOrDie<gnmi::GetResponse>(
R"pb(notification {
prefix { origin: "openconfig" }
update {
path { elem { name: "interfaces" } }
val {
json_ietf_val: "{\"openconfig-interfaces:interfaces\":{\"interface\":[{\"config\":{\"name\":\"CPU\",\"openconfig-p4rt:id\":4294967293,\"type\":\"iana-if-type:ethernetCsmacd\"},\"name\":\"CPU\",\"subinterfaces\":{\"subinterface\":[{\"config\":{\"index\":0},\"index\":0,\"openconfig-if-ip:ipv4\":{\"config\":{\"enabled\":false}},\"openconfig-if-ip:ipv6\":{\"config\":{\"enabled\":false}}}]}},{\"config\":{\"description\":\"ju1u1m2.sqs11.net.google.com:Ethernet1/1/1 [(not a trunk member)]\",\"enabled\":true,\"google-pins-interfaces:ecmp-hash-algorithm\":\"CRC_32LO\",\"google-pins-interfaces:ecmp-hash-offset\":\"8\",\"google-pins-interfaces:fully-qualified-interface-name\":\"ju1u1m1.sqs11.net.google.com:Ethernet1/1/1\",\"mtu\":9216,\"name\":\"Ethernet1/1/1\",\"openconfig-p4rt:id\":1,\"openconfig-pins-interfaces:health-indicator\":\"GOOD\",\"type\":\"iana-if-type:ethernetCsmacd\"},\"hold-time\":{\"config\":{\"down\":0,\"up\":8000}},\"name\":\"Ethernet1/1/1\",\"openconfig-if-ethernet:ethernet\":{\"config\":{\"fec-mode\":\"openconfig-if-ethernet:FEC_RS544_2X_INTERLEAVE\",\"port-speed\":\"openconfig-if-ethernet:SPEED_400GB\"}},\"subinterfaces\":{\"subinterface\":[{\"config\":{\"enabled\":true,\"index\":0},\"index\":0,\"openconfig-if-ip:ipv4\":{\"config\":{\"enabled\":false}},\"openconfig-if-ip:ipv6\":{\"config\":{\"enabled\":false},\"unnumbered\":{\"config\":{\"enabled\":true}}}}]}}]}}"
}
}
})pb")),
Return(grpc::Status::OK)));
EXPECT_THAT(GetConfigDisabledInterfaces(stub),
IsOkAndHolds(UnorderedElementsAre("CPU")));
}

TEST(GetInterfaceOperStatusOverGnmi, RpcFails) {
gnmi::MockgNMIStub stub;
EXPECT_CALL(stub,
Expand Down Expand Up @@ -3044,6 +3070,138 @@ TEST(GetAllInterfaceCounters, WorksWithoutOptionalValues) {
EXPECT_EQ(counters.carrier_transitions, std::nullopt);
}

TEST(GetAllInterfaceCounters, FailedWithMissingFieldAndReportsInterface) {
static constexpr absl::string_view kInterfaceJson = R"(
{
"openconfig-interfaces:interface":[
{
"name":"CPU",
"state":{
"counters":{
"in-broadcast-pkts":"0",
"in-discards":"134",
"in-errors":"0",
"in-fcs-errors":"0",
"in-multicast-pkts":"0",
"in-octets":"0",
"in-pkts":"0",
"in-unicast-pkts":"0",
"in-unknown-protos":"0",
"last-clear":"0",
"out-broadcast-pkts":"0",
"out-discards":"0",
"out-errors":"0",
"out-multicast-pkts":"0",
"out-octets":"0",
"out-pkts":"0",
"out-unicast-pkts":"0"
}
},
"subinterfaces":{
"subinterface":[
{
"index":0,
"openconfig-if-ip:ipv4":{
"state":{
"enabled":false
}
},
"openconfig-if-ip:ipv6":{
"state":{
"enabled":false
}
},
"state":{
"index":0
}
}
]
}
},
{
"name":"Ethernet1/1/1",
"openconfig-if-ethernet:ethernet":{
"state":{
"counters":{
"in-maxsize-exceeded":"1001"
}
}
},
"state":{
"counters":{
"carrier-transitions":"1",
"in-broadcast-pkts":"1003",
"in-discards":"132",
"in-errors":"1004",
"in-fcs-errors":"1005",
"in-multicast-pkts":"132",
"in-octets":"9828",
"in-pkts":"132",
"in-unicast-pkts":"1006",
"in-unknown-protos":"0",
"last-clear":"0",
"out-broadcast-pkts":"1007",
"out-discards":"1008",
"out-errors":"1009",
"out-multicast-pkts":"134",
"out-octets":"9996",
"out-pkts":"134",
"out-unicast-pkts":"1010"
}
},
"subinterfaces":{
"subinterface":[
{
"index":0,
"openconfig-if-ip:ipv4":{
"state":{
"counters":{
"in-pkts":"1011",
"out-pkts":"1012"
}
}
},
"openconfig-if-ip:ipv6":{
"state":{
"counters":{
"in-discarded-pkts":"1013",
"in-pkts":"1014",
"out-discarded-pkts":"1015",
"out-pkts":"1016"
}
}
}
}
]
}
}
]
})";
gnmi::MockgNMIStub stub;
EXPECT_CALL(stub, Get(_,
EqualsProto(gutil::ParseProtoOrDie<gnmi::GetRequest>(
R"pb(prefix { origin: "openconfig" }
path {
elem { name: "interfaces" }
elem { name: "interface" }
}
type: STATE)pb")),
_))
.WillOnce(DoAll(
SetArgPointee<2>(ConstructResponse(
"elem { name: \"interfaces\" } elem { name: \"interface\" }",
kInterfaceJson)),
Return(grpc::Status::OK)));
EXPECT_THAT(
GetAllInterfaceCounters(stub),
StatusIs(
absl::StatusCode::kNotFound,
AllOf(
HasSubstr("Ethernet1/1/1"),
HasSubstr(
"google-pins-interfaces:in-buffer-discards not found in"))));
}

TEST(GetGnmiStateLeafValue, ReturnsStateValue) {
gnmi::MockgNMIStub stub;
gnmi::GetResponse response;
Expand Down

0 comments on commit 8c5dc98

Please sign in to comment.