[intfsorch] add RIF flex counter group #765
Conversation
Signed-off-by: Mykola Faryma <[email protected]>
|
retest this please |
orchagent/intfsorch.cpp
Outdated
| } | ||
| } | ||
|
|
||
| // for (auto it = m_rifsToRemove.begin(); it != m_rifsToRemove.end();) |
There was a problem hiding this comment.
Commented out by mistake or irrelevant?
Signed-off-by: Mykola Faryma <[email protected]>
|
depend on schema sonic-net/sonic-swss-common#256 |
|
retest this please |
orchagent/intfsorch.cpp
Outdated
| /* Initialize DB connectors */ | ||
| m_counter_db = shared_ptr<DBConnector>(new DBConnector(COUNTERS_DB, DBConnector::DEFAULT_UNIXSOCKET, 0)); | ||
| m_flex_db = shared_ptr<DBConnector>(new DBConnector(FLEX_COUNTER_DB, DBConnector::DEFAULT_UNIXSOCKET, 0)); | ||
| // m_state_db = shared_ptr<DBConnector>(new DBConnector(STATE_DB, DBConnector::DEFAULT_UNIXSOCKET, 0)); |
There was a problem hiding this comment.
Please remove commented code
orchagent/intfsorch.cpp
Outdated
| string value; | ||
| for (auto it = m_rifsToAdd.begin(); it != m_rifsToAdd.end(); ++it) | ||
| { | ||
| SWSS_LOG_NOTICE("Registering begin -> %s ", it->m_alias.c_str()); |
There was a problem hiding this comment.
Why do we have a separate for-loop for this log? With the log in below for-loop, this appears to be redundant.
orchagent/intfsorch.cpp
Outdated
| for (auto it = m_rifsToAdd.begin(); it != m_rifsToAdd.end(); ) | ||
| { | ||
| const auto id = sai_serialize_object_id(it->m_rif_id); | ||
| SWSS_LOG_NOTICE("Registering %s, id %s", it->m_alias.c_str(), id.c_str()); |
There was a problem hiding this comment.
Suggest to change this to INFO level
orchagent/intfsorch.cpp
Outdated
| } | ||
| if (m_vidToRidTable->hget("", id, value)) | ||
| { | ||
| SWSS_LOG_NOTICE("Registering %s it is ready", it->m_alias.c_str()); |
orchagent/intfsorch.cpp
Outdated
| { | ||
| SWSS_LOG_ENTER(); | ||
|
|
||
| SWSS_LOG_NOTICE("Registering %ld new intfs, deleting %ld ", m_rifsToAdd.size(), m_rifsToRemove.size()); |
There was a problem hiding this comment.
Suggest to change to INFO/DEBUG level
orchagent/intfsorch.h
Outdated
|
|
||
| SelectableTimer* m_updateMapsTimer = nullptr; | ||
| std::vector<Port> m_rifsToAdd; | ||
| std::vector<Port> m_rifsToRemove; |
There was a problem hiding this comment.
I don't see this updated any where. Is it missed?
| @@ -466,6 +507,9 @@ bool IntfsOrch::removeRouterIntfs(Port &port) | |||
| return false; | |||
| } | |||
|
|
|||
| const auto id = sai_serialize_object_id(port.m_rif_id); | |||
| removeRifFromFlexCounter(id, port.m_alias); | |||
There was a problem hiding this comment.
Why is this called directly whereas addRifToFlexCounter is based on timer?
There was a problem hiding this comment.
this is due to FC logic.
The Flex counter will try to determine the object's type based on the vid. The VIDTOIRID is not instantly available on create.
For the delete flow it's the opposite.
If there will be some issues with this logic, I believe the FC flow should be changed. Right now it's tricky to dynamically add/remove objects that need to be polled.
There was a problem hiding this comment.
How to ensure deleting flex counter is before deleting rif in syncd? If deleting rif happened before deleting flex counter, rid cannot be found in the VIDTOIRID.
Signed-off-by: Mykola Faryma <[email protected]>
Signed-off-by: Mykola Faryma <[email protected]>
|
retest this please |
…d ports (sonic-net#765) Provided a new ConfigMgmt class for - Config Validation - Adding ports - Deleting ports Signed-off-by: Praveen Chaudhary [email protected]
Signed-off-by: Mykola Faryma [email protected]
What I did
Why I did it
How I verified it
Details if related