Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[counter] Fix port flex counter #1052

Merged
merged 5 commits into from
Jun 9, 2022
Merged

[counter] Fix port flex counter #1052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6e7995a
Debug flex counter
jimmyzhai May 30, 2022
7e8f088
Get supportedPortCounters per port
jimmyzhai May 30, 2022
5628744
Remove some debug logs
jimmyzhai Jun 3, 2022
ad1a46c
Add unit test addRemoveCounterForPort
jimmyzhai Jun 3, 2022
02f6844
Fix comments
jimmyzhai Jun 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 17 additions & 24 deletions syncd/FlexCounter.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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());
}