Skip to content

Commit 3964cf1

Browse files
jimmyzhaijimmyzhai
authored
[counter] Fix port flex counter (#1052)
Fix #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.
1 parent 2231b7a commit 3964cf1

File tree

4 files changed

+115
-32
lines changed

4 files changed

+115
-32
lines changed

‎syncd/FlexCounter.cpp

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -239,14 +239,15 @@ void FlexCounter::setPortCounterList(
239239
{
240240
SWSS_LOG_ENTER();
241241

242-
updateSupportedPortCounters(portId);
242+
PortCountersSet supportedPortCounters;
243+
updateSupportedPortCounters(portId, supportedPortCounters);
243244

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

247248
for (auto &counter : counterIds)
248249
{
249-
if (isPortCounterSupported(counter))
250+
if (supportedPortCounters.count(counter) != 0)
250251
{
251252
supportedIds.push_back(counter);
252253
}
@@ -1361,13 +1362,6 @@ bool FlexCounter::allPluginsEmpty() const
13611362
m_flowCounterPlugins.empty();
13621363
}
13631364

1364-
bool FlexCounter::isPortCounterSupported(sai_port_stat_t counter) const
1365-
{
1366-
SWSS_LOG_ENTER();
1367-
1368-
return m_supportedPortCounters.count(counter) != 0;
1369-
}
1370-
13711365
bool FlexCounter::isQueueCounterSupported(
13721366
_In_ sai_queue_stat_t counter) const
13731367
{
@@ -2437,7 +2431,8 @@ void FlexCounter::endFlexCounterThread(void)
24372431
}
24382432

24392433
sai_status_t FlexCounter::querySupportedPortCounters(
2440-
_In_ sai_object_id_t portRid)
2434+
_In_ sai_object_id_t portRid,
2435+
_Out_ PortCountersSet &supportedPortCounters)
24412436
{
24422437
SWSS_LOG_ENTER();
24432438

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

24642459
if (status != SAI_STATUS_SUCCESS)
24652460
{
2466-
SWSS_LOG_INFO("Unable to get port supported counters for %s",
2467-
sai_serialize_object_id(portRid).c_str());
2461+
SWSS_LOG_NOTICE("Unable to query port supported counters for %s: %s",
2462+
sai_serialize_object_id(portRid).c_str(),
2463+
sai_serialize_status(status).c_str());
24682464
}
24692465
else
24702466
{
24712467
for (auto statCapability: statCapabilityList)
24722468
{
24732469
sai_port_stat_t counter = static_cast<sai_port_stat_t>(statCapability.stat_enum);
2474-
m_supportedPortCounters.insert(counter);
2470+
supportedPortCounters.insert(counter);
24752471
}
24762472
}
24772473
}
24782474
return status;
24792475
}
24802476

24812477
void FlexCounter::getSupportedPortCounters(
2482-
_In_ sai_object_id_t portRid)
2478+
_In_ sai_object_id_t portRid,
2479+
_Out_ PortCountersSet &supportedPortCounters)
24832480
{
24842481
SWSS_LOG_ENTER();
24852482

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

24942491
if (status != SAI_STATUS_SUCCESS)
24952492
{
2496-
SWSS_LOG_INFO("Counter %s is not supported on port RID %s: %s",
2493+
SWSS_LOG_NOTICE("Counter %s is not supported on port RID %s: %s",
24972494
sai_serialize_port_stat(counter).c_str(),
24982495
sai_serialize_object_id(portRid).c_str(),
24992496
sai_serialize_status(status).c_str());
25002497

25012498
continue;
25022499
}
25032500

2504-
m_supportedPortCounters.insert(counter);
2501+
supportedPortCounters.insert(counter);
25052502
}
25062503
}
25072504

25082505
void FlexCounter::updateSupportedPortCounters(
2509-
_In_ sai_object_id_t portRid)
2506+
_In_ sai_object_id_t portRid,
2507+
_Out_ PortCountersSet &supportedPortCounters)
25102508
{
25112509
SWSS_LOG_ENTER();
25122510

2513-
if (m_supportedPortCounters.size())
2514-
{
2515-
return;
2516-
}
2517-
25182511
/* Query SAI supported port counters */
2519-
sai_status_t status = querySupportedPortCounters(portRid);
2512+
sai_status_t status = querySupportedPortCounters(portRid, supportedPortCounters);
25202513
if (status != SAI_STATUS_SUCCESS)
25212514
{
25222515
/* Fallback to legacy approach */
2523-
getSupportedPortCounters(portRid);
2516+
getSupportedPortCounters(portRid, supportedPortCounters);
25242517
}
25252518
}
25262519

‎syncd/FlexCounter.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,6 @@ namespace syncd
213213

214214
private: // is counter supported
215215

216-
bool isPortCounterSupported(
217-
_In_ sai_port_stat_t counter) const;
218-
219216
bool isPriorityGroupCounterSupported(
220217
_In_ sai_ingress_priority_group_stat_t counter) const;
221218

@@ -237,14 +234,18 @@ namespace syncd
237234

238235
private: // update supported counters
239236

237+
typedef std::set<sai_port_stat_t> PortCountersSet;
240238
sai_status_t querySupportedPortCounters(
241-
_In_ sai_object_id_t portRid);
239+
_In_ sai_object_id_t portRid,
240+
_Out_ PortCountersSet &supportedPortCounters);
242241

243242
void getSupportedPortCounters(
244-
_In_ sai_object_id_t portRid);
243+
_In_ sai_object_id_t portRid,
244+
_Out_ PortCountersSet &supportedPortCounters);
245245

246246
void updateSupportedPortCounters(
247-
_In_ sai_object_id_t portRid);
247+
_In_ sai_object_id_t portRid,
248+
_Out_ PortCountersSet &supportedPortCounters);
248249

249250
std::vector<sai_port_stat_t> saiCheckSupportedPortDebugCounters(
250251
_In_ sai_object_id_t portRid,
@@ -545,7 +546,6 @@ namespace syncd
545546

546547
private: // supported counters
547548

548-
std::set<sai_port_stat_t> m_supportedPortCounters;
549549
std::set<sai_ingress_priority_group_stat_t> m_supportedPriorityGroupCounters;
550550
std::set<sai_queue_stat_t> m_supportedQueueCounters;
551551
std::set<sai_router_interface_stat_t> m_supportedRifCounters;

‎unittest/syncd/MockableSaiInterface.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ sai_status_t MockableSaiInterface::queryStatsCapability(
165165
return mock_queryStatsCapability(switch_id, object_type, stats_capability);
166166
}
167167

168-
return SAI_STATUS_SUCCESS;
168+
return SAI_STATUS_NOT_SUPPORTED;
169169
}
170170

171171
sai_status_t MockableSaiInterface::getStatsExt(

‎unittest/syncd/TestFlexCounter.cpp

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,3 +186,93 @@ TEST(FlexCounter, addRemoveCounterForMACsecSA)
186186

187187
}
188188

189+
TEST(FlexCounter, addRemoveCounterForPort)
190+
{
191+
std::shared_ptr<MockableSaiInterface> sai(new MockableSaiInterface());
192+
FlexCounter fc("test", sai, "COUNTERS_DB");
193+
194+
sai_object_id_t counterVid{100};
195+
sai_object_id_t counterRid{100};
196+
std::vector<swss::FieldValueTuple> values;
197+
values.emplace_back(PORT_COUNTER_ID_LIST, "SAI_PORT_STAT_IF_IN_OCTETS,SAI_PORT_STAT_IF_IN_ERRORS");
198+
199+
test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_PORT);
200+
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) {
201+
for (uint32_t i = 0; i < number_of_counters; i++)
202+
{
203+
if (ids[i] == SAI_PORT_STAT_IF_IN_OCTETS)
204+
{
205+
counters[i] = 100;
206+
}
207+
else if (ids[i] == SAI_PORT_STAT_IF_IN_ERRORS)
208+
{
209+
counters[i] = 200;
210+
}
211+
else
212+
{
213+
return SAI_STATUS_FAILURE;
214+
}
215+
}
216+
return SAI_STATUS_SUCCESS;
217+
};
218+
219+
fc.addCounter(counterVid, counterRid, values);
220+
EXPECT_EQ(fc.isEmpty(), false);
221+
222+
values.clear();
223+
values.emplace_back(POLL_INTERVAL_FIELD, "1000");
224+
values.emplace_back(FLEX_COUNTER_STATUS_FIELD, "enable");
225+
fc.addCounterPlugin(values);
226+
227+
usleep(1000*1000);
228+
swss::DBConnector db("COUNTERS_DB", 0);
229+
swss::RedisPipeline pipeline(&db);
230+
swss::Table countersTable(&pipeline, COUNTERS_TABLE, false);
231+
232+
std::vector<std::string> keys;
233+
countersTable.getKeys(keys);
234+
EXPECT_EQ(keys.size(), size_t(1));
235+
EXPECT_EQ(keys[0], "oid:0x64");
236+
237+
std::string value;
238+
countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_OCTETS", value);
239+
EXPECT_EQ(value, "100");
240+
countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_ERRORS", value);
241+
EXPECT_EQ(value, "200");
242+
243+
fc.removeCounter(counterVid);
244+
EXPECT_EQ(fc.isEmpty(), true);
245+
countersTable.del("oid:0x64");
246+
countersTable.getKeys(keys);
247+
ASSERT_TRUE(keys.empty());
248+
249+
// Test again with queryStatsCapability support
250+
sai->mock_queryStatsCapability = [](sai_object_id_t, sai_object_type_t, sai_stat_capability_list_t *capability) {
251+
if (capability->count < 2)
252+
{
253+
capability->count = 2;
254+
return SAI_STATUS_BUFFER_OVERFLOW;
255+
}
256+
257+
capability->list[0].stat_enum = SAI_PORT_STAT_IF_IN_OCTETS;
258+
capability->list[1].stat_enum = SAI_PORT_STAT_IF_IN_ERRORS;
259+
return SAI_STATUS_SUCCESS;
260+
};
261+
262+
values.clear();
263+
values.emplace_back(PORT_COUNTER_ID_LIST, "SAI_PORT_STAT_IF_IN_OCTETS,SAI_PORT_STAT_IF_IN_ERRORS");
264+
fc.addCounter(counterVid, counterRid, values);
265+
EXPECT_EQ(fc.isEmpty(), false);
266+
267+
usleep(1000*1000);
268+
countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_OCTETS", value);
269+
EXPECT_EQ(value, "100");
270+
countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_ERRORS", value);
271+
EXPECT_EQ(value, "200");
272+
273+
fc.removeCounter(counterVid);
274+
EXPECT_EQ(fc.isEmpty(), true);
275+
countersTable.del("oid:0x64");
276+
countersTable.getKeys(keys);
277+
ASSERT_TRUE(keys.empty());
278+
}

0 commit comments

Comments
 (0)