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.

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

Add to save sai objects which are created during onPostPortCreate #994

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gord1306
Copy link

The ports which is without QUEUE related configurations still created related
SCHEDULER_GROUPs in the onPostPortCreate() during the syncd startup in some
platform - e.g. barefoot montara. But these created objects did NOT be recorded
in the COLDVIDS and m_warmBootDiscoveredVids set. While swss is perform
warmstart, the compareViews() would be called before apply the view, and it
would detect the differences between the current view and temp view. Then syncd
would remove these schedulers groups in the finalize.

This patch adds to record the objects which are created during onPostPortCreate
into the m_warmBootDiscoveredVids set.

Verified by

  1. Applied sonic-mgmt t0 minigraph
  2. SONiC CLI command: config warm_restart enable swss
  3. System CLI command: service swss restart
    Verify the syslog is there any WARNING/ERR in checkInternalObjects

The ports which is without QUEUE related configurations still created related
SCHEDULER_GROUPs in the onPostPortCreate() during the syncd startup in some
platform - e.g. barefoot montara. But these created objects did NOT be recorded
in the `COLDVIDS` and `m_warmBootDiscoveredVids` set. While swss is perform
warmstart, the `compareViews()` would be called before apply the view, and it
would detect the differences between the current view and temp view. Then syncd
would remove these schedulers groups in the finalize.

This patch adds to record the objects which are created during onPostPortCreate
into the `m_warmBootDiscoveredVids` set.
@gord1306 gord1306 marked this pull request as draft January 19, 2022 08:09
Copy link
Collaborator

@kcudnik kcudnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please see comments

Comment on lines +1033 to +1046

/*
* The ports which is without QUEUE related configurations still created
* related SCHEDULER_GROUPs in the onPostPortCreate() during the syncd
* startup in some platform - e.g. montara. But these created objects
* did NOT be recorded in the `COLDVIDS` and `m_warmBootDiscoveredVids`
* set. While swss is perform warmstart, the `compareViews()` would be
* called before apply the view, and it would detect the differences
* between the current view and temp view. Then syncd would remove these
* schedulers groups in the finalize. Therefore, add to record these
* object into the m_warmBootDiscoveredVids
*/
sai_object_id_t vid = m_translator->translateRidToVid(rid, m_switch_vid);
m_warmBootDiscoveredVids.insert(vid);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why this is needed here, why m_warmBootDiscoveredVids is used at all, since onPostPortCreate is called AFTER cold boot or warm boot was performed already, so all discovered objects were already discovered during "create switch" at that phase. i dont quite understand description here, also you don't know whether onPostPortCreate which is called here is after cold boot or after warm boot, but yet you always add vids to m_warmBootDiscoveredVids

this translation is not needed here, if you are missing some vids, then it means problem is somewhere else

Copy link
Author

@gord1306 gord1306 Jan 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reply. The case that I encountered was to perform warmstart swss only, not syncd. And the onPostPortCreate() would not be called again. And it causes some vids which were created previous onPostPortCreate() would be miss, because there is no records for this vids. And in the ComparisonLogic::populateExistingObjects()ComparisonLogic.cpp#L2431, the logical looks like to handle this kind of objects, that why I put these vids in the m_warmBootDiscoveredVids.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants