Skip to content

Commit

Permalink
[counter] Fix port flex counter (sonic-net#1052)
Browse files Browse the repository at this point in the history 
Fix sonic-net#10850.

A fact is there might be different port types on asic, then different port stats capabilities.
Instead of using a cached supported port counter ID list for all ports, it gets supported
port counter list per port.
  • Loading branch information
@jimmyzhai
jimmyzhai authored Jun 9, 2022
1 parent 2231b7a commit 3964cf1
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 32 deletions.
41 changes: 17 additions & 24 deletions syncd/FlexCounter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,14 +239,15 @@ void FlexCounter::setPortCounterList(
{
SWSS_LOG_ENTER();

updateSupportedPortCounters(portId);
PortCountersSet supportedPortCounters;
updateSupportedPortCounters(portId, supportedPortCounters);

// Remove unsupported counters
std::vector<sai_port_stat_t> supportedIds;

for (auto &counter : counterIds)
{
if (isPortCounterSupported(counter))
if (supportedPortCounters.count(counter) != 0)
{
supportedIds.push_back(counter);
}
Expand Down Expand Up @@ -1361,13 +1362,6 @@ bool FlexCounter::allPluginsEmpty() const
m_flowCounterPlugins.empty();
}

bool FlexCounter::isPortCounterSupported(sai_port_stat_t counter) const
{
SWSS_LOG_ENTER();

return m_supportedPortCounters.count(counter) != 0;
}

bool FlexCounter::isQueueCounterSupported(
_In_ sai_queue_stat_t counter) const
{
Expand Down Expand Up @@ -2437,7 +2431,8 @@ void FlexCounter::endFlexCounterThread(void)
}

sai_status_t FlexCounter::querySupportedPortCounters(
_In_ sai_object_id_t portRid)
_In_ sai_object_id_t portRid,
_Out_ PortCountersSet &supportedPortCounters)
{
SWSS_LOG_ENTER();

Expand All @@ -2463,23 +2458,25 @@ sai_status_t FlexCounter::querySupportedPortCounters(

if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_INFO("Unable to get port supported counters for %s",
sai_serialize_object_id(portRid).c_str());
SWSS_LOG_NOTICE("Unable to query port supported counters for %s: %s",
sai_serialize_object_id(portRid).c_str(),
sai_serialize_status(status).c_str());
}
else
{
for (auto statCapability: statCapabilityList)
{
sai_port_stat_t counter = static_cast<sai_port_stat_t>(statCapability.stat_enum);
m_supportedPortCounters.insert(counter);
supportedPortCounters.insert(counter);
}
}
}
return status;
}

void FlexCounter::getSupportedPortCounters(
_In_ sai_object_id_t portRid)
_In_ sai_object_id_t portRid,
_Out_ PortCountersSet &supportedPortCounters)
{
SWSS_LOG_ENTER();

Expand All @@ -2493,34 +2490,30 @@ void FlexCounter::getSupportedPortCounters(

if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_INFO("Counter %s is not supported on port RID %s: %s",
SWSS_LOG_NOTICE("Counter %s is not supported on port RID %s: %s",
sai_serialize_port_stat(counter).c_str(),
sai_serialize_object_id(portRid).c_str(),
sai_serialize_status(status).c_str());

continue;
}

m_supportedPortCounters.insert(counter);
supportedPortCounters.insert(counter);
}
}

void FlexCounter::updateSupportedPortCounters(
_In_ sai_object_id_t portRid)
_In_ sai_object_id_t portRid,
_Out_ PortCountersSet &supportedPortCounters)
{
SWSS_LOG_ENTER();

if (m_supportedPortCounters.size())
{
return;
}

/* Query SAI supported port counters */
sai_status_t status = querySupportedPortCounters(portRid);
sai_status_t status = querySupportedPortCounters(portRid, supportedPortCounters);
if (status != SAI_STATUS_SUCCESS)
{
/* Fallback to legacy approach */
getSupportedPortCounters(portRid);
getSupportedPortCounters(portRid, supportedPortCounters);
}
}

Expand Down
14 changes: 7 additions & 7 deletions syncd/FlexCounter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,6 @@ namespace syncd

private: // is counter supported

bool isPortCounterSupported(
_In_ sai_port_stat_t counter) const;

bool isPriorityGroupCounterSupported(
_In_ sai_ingress_priority_group_stat_t counter) const;

Expand All @@ -237,14 +234,18 @@ namespace syncd

private: // update supported counters

typedef std::set<sai_port_stat_t> PortCountersSet;
sai_status_t querySupportedPortCounters(
_In_ sai_object_id_t portRid);
_In_ sai_object_id_t portRid,
_Out_ PortCountersSet &supportedPortCounters);

void getSupportedPortCounters(
_In_ sai_object_id_t portRid);
_In_ sai_object_id_t portRid,
_Out_ PortCountersSet &supportedPortCounters);

void updateSupportedPortCounters(
_In_ sai_object_id_t portRid);
_In_ sai_object_id_t portRid,
_Out_ PortCountersSet &supportedPortCounters);

std::vector<sai_port_stat_t> saiCheckSupportedPortDebugCounters(
_In_ sai_object_id_t portRid,
Expand Down Expand Up @@ -545,7 +546,6 @@ namespace syncd

private: // supported counters

std::set<sai_port_stat_t> m_supportedPortCounters;
std::set<sai_ingress_priority_group_stat_t> m_supportedPriorityGroupCounters;
std::set<sai_queue_stat_t> m_supportedQueueCounters;
std::set<sai_router_interface_stat_t> m_supportedRifCounters;
Expand Down
2 changes: 1 addition & 1 deletion unittest/syncd/MockableSaiInterface.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ sai_status_t MockableSaiInterface::queryStatsCapability(
return mock_queryStatsCapability(switch_id, object_type, stats_capability);
}

return SAI_STATUS_SUCCESS;
return SAI_STATUS_NOT_SUPPORTED;
}

sai_status_t MockableSaiInterface::getStatsExt(
Expand Down
90 changes: 90 additions & 0 deletions unittest/syncd/TestFlexCounter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,93 @@ TEST(FlexCounter, addRemoveCounterForMACsecSA)

}

TEST(FlexCounter, addRemoveCounterForPort)
{
std::shared_ptr<MockableSaiInterface> sai(new MockableSaiInterface());
FlexCounter fc("test", sai, "COUNTERS_DB");

sai_object_id_t counterVid{100};
sai_object_id_t counterRid{100};
std::vector<swss::FieldValueTuple> values;
values.emplace_back(PORT_COUNTER_ID_LIST, "SAI_PORT_STAT_IF_IN_OCTETS,SAI_PORT_STAT_IF_IN_ERRORS");

test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_PORT);
sai->mock_getStats = [](sai_object_type_t, sai_object_id_t, uint32_t number_of_counters, const sai_stat_id_t *ids, uint64_t *counters) {
for (uint32_t i = 0; i < number_of_counters; i++)
{
if (ids[i] == SAI_PORT_STAT_IF_IN_OCTETS)
{
counters[i] = 100;
}
else if (ids[i] == SAI_PORT_STAT_IF_IN_ERRORS)
{
counters[i] = 200;
}
else
{
return SAI_STATUS_FAILURE;
}
}
return SAI_STATUS_SUCCESS;
};

fc.addCounter(counterVid, counterRid, values);
EXPECT_EQ(fc.isEmpty(), false);

values.clear();
values.emplace_back(POLL_INTERVAL_FIELD, "1000");
values.emplace_back(FLEX_COUNTER_STATUS_FIELD, "enable");
fc.addCounterPlugin(values);

usleep(1000*1000);
swss::DBConnector db("COUNTERS_DB", 0);
swss::RedisPipeline pipeline(&db);
swss::Table countersTable(&pipeline, COUNTERS_TABLE, false);

std::vector<std::string> keys;
countersTable.getKeys(keys);
EXPECT_EQ(keys.size(), size_t(1));
EXPECT_EQ(keys[0], "oid:0x64");

std::string value;
countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_OCTETS", value);
EXPECT_EQ(value, "100");
countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_ERRORS", value);
EXPECT_EQ(value, "200");

fc.removeCounter(counterVid);
EXPECT_EQ(fc.isEmpty(), true);
countersTable.del("oid:0x64");
countersTable.getKeys(keys);
ASSERT_TRUE(keys.empty());

// Test again with queryStatsCapability support
sai->mock_queryStatsCapability = [](sai_object_id_t, sai_object_type_t, sai_stat_capability_list_t *capability) {
if (capability->count < 2)
{
capability->count = 2;
return SAI_STATUS_BUFFER_OVERFLOW;
}

capability->list[0].stat_enum = SAI_PORT_STAT_IF_IN_OCTETS;
capability->list[1].stat_enum = SAI_PORT_STAT_IF_IN_ERRORS;
return SAI_STATUS_SUCCESS;
};

values.clear();
values.emplace_back(PORT_COUNTER_ID_LIST, "SAI_PORT_STAT_IF_IN_OCTETS,SAI_PORT_STAT_IF_IN_ERRORS");
fc.addCounter(counterVid, counterRid, values);
EXPECT_EQ(fc.isEmpty(), false);

usleep(1000*1000);
countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_OCTETS", value);
EXPECT_EQ(value, "100");
countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_ERRORS", value);
EXPECT_EQ(value, "200");

fc.removeCounter(counterVid);
EXPECT_EQ(fc.isEmpty(), true);
countersTable.del("oid:0x64");
countersTable.getKeys(keys);
ASSERT_TRUE(keys.empty());
}

0 comments on commit 3964cf1

Please sign in to comment.