Tune consumer order in BufferOrch::doTask() based on dependency for warm start#590
Conversation
Signed-off-by: Qi Luo <qiluo-msft@users.noreply.github.com>
Signed-off-by: Qi Luo <qiluo-msft@users.noreply.github.com>
qiluo-msft
changed the title
| if (!suc) | ||
| { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
from line 265 - 274 it could be ine one line return !WarmStart::isWarmStart || warmRestoreAndSyncUp(); #Resolved
There was a problem hiding this comment.
It is equivalent. I prefer no change for easy reading. #Closed
| if (consumer == pool_consumer) | ||
| continue; | ||
| consumer->drain(); | ||
| } |
There was a problem hiding this comment.
Do you mean the configuration for other buffer tables like CFG_BUFFER_QUEUE_TABLE_NAME, CFG_BUFFER_PG_TABLE_NAME, CFG_BUFFER_PORT_INGRESS_PROFILE_LIST_NAME and CFG_BUFFER_PORT_EGRESS_PROFILE_LIST_NAME have dependency on CFG_BUFFER_PROFILE_TABLE_NAME & CFG_BUFFER_POOL_TABLE_NAME? CFG_BUFFER_PROFILE_TABLE_NAME has dependency on CFG_BUFFER_POOL_TABLE_NAME?
I didn't see existing code checking that dependency. #Resolved
There was a problem hiding this comment.
Exactly, so I added comments. The ground truth is inside SAI design doc.
For code, task_process_status::task_need_retry possibly indicates a dependency missing. They don't show the dependency relationship as specific as SAI design doc.
In reply to: 212513136 [](ancestors = 212513136)
There was a problem hiding this comment.
If there is dependency, the exact dependency chain should be clarified and the problem should be fixed in normal code logic which applies to cold boot too.
#Resolved
There was a problem hiding this comment.
it is difficult to do this in the normal boot since the input sequence is unknown. we need to get the dependency at the entry level and then dispatched to task. in warmboot, since we have all the data, we can just solve the dependency at the table level.
There was a problem hiding this comment.
During cold start, the task_process_status::task_need_retry handles dependency correctly, ie. anything missing will postpone task processing. So there is no problem to fix.
I agree this code is not explicit in dependency relationship. This should be improved in future.
In reply to: 212527615 [](ancestors = 212527615)
orchagent/bufferorch.cpp
Outdated
| // 3.1 Ingress priority group (PG) configuration | ||
| // 3.2.1 Buffer profile configuration | ||
| // | ||
| // buffer poll |
sonic-net#590) Signed-off-by: Guohan Lu <lguohan@gmail.com>
…arm start (sonic-net#590) * initBufferReadyLists will pops consumers, so warm start will not redo * revert initBufferReadyLists() * (comment) * (typo)
[PINS] Fix bit width of MAC address in SAI ACL schema
4 participants
Tested in VS: