align watermark flow with port configuration #2
Conversation
dbarashinvd
force-pushed
the
dbarashi_align_watermark
branch
3 times, most recently
from
ae96260
to
4191a64
Compare
|
I see it's only 2 commits behind. |
Yes. This is a significant change in this part. Originally, counters were created for all queues and PGs. Now, they are created for queues and PGs with buffer configured only |
dbarashinvd
force-pushed
the
dbarashi_align_watermark
branch
2 times, most recently
from
425bca2
to
8579c62
Compare
| @@ -250,14 +266,24 @@ bool FlexCounterOrch::getPortBufferDropCountersState() const | |||
| return m_port_buffer_drop_counter_enabled; | |||
| } | |||
|
|
|||
| bool FlexCounterOrch::getPgWatermarkCountersState() const | |||
| bool FlexCounterOrch::getQueueCountersState() const | |||
There was a problem hiding this comment.
Better not to change position of the functions that are not modified.
| { | ||
| string queueType; | ||
| uint8_t queueRealIndex = 0; | ||
| if (getQueueTypeAndIndex(port.m_queue_ids[queueIndex], queueType, queueRealIndex)) |
There was a problem hiding this comment.
can we check (!queuesState.isQueueCounterEnabled(queueRealIndex)) first and then (getQueueTypeAndIndex(port.m_queue_ids[queueIndex], queueType, queueRealIndex))?
AFAIU, the former is light and the latter is a bit heavier. If the former returns false, we do not need to call the latter.
There was a problem hiding this comment.
how can this be done ?
we first get the queueRealIIndex using the getQueueTypeAndIndex and then use in for queuesState.isQueueCounterEnabled(queueRealIndex).
this is also the implementation of the PR sonic-net#2473 [counters] Improve performance by polling only configured ports buffer queue/pg counters that uses it in generateQueueMapPerPort function.
can I use the queueIndex rather than the queueRealIndex ? if so I can ditch the whole getQueueTypeAndIndex if statement.
There was a problem hiding this comment.
Ok. You are correct. I didn’t realize it depends on the output argument of the former function call.
dbarashinvd
force-pushed
the
dbarashi_align_watermark
branch
from
8579c62
to
49f744b
Compare
What I did
Align watermark flow with port configuration
correct the queue, watermark and pg-drop counterpoll functionality according to cli commands
Why I did it
the flow before this commit was wrong, when watermark counterpoll was enabled, stats wasn't created in FLEX_COUNTERS_DB
and COUNTERS_DB unless enabling queue counterpoll as well.
in the same way when pg-drop counterpoll was enabled stats weren't added until watermark was enabled.
This is due to the wrong flow where queue and pg maps were generated only when queue or watermark (pg-watermark)
key was detected in flexcountersorch.cpp
How I verified it
manual testing
VS test (updated to reflect current flow) - before this commit only queue was tested
Details if related