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.

Sign up for GitHub

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

[vs] Initialization of VOQ switch objects #702

Merged
merged 5 commits into from
Nov 13, 2020
Merged

[vs] Initialization of VOQ switch objects #702

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
03dbeb0
[vstest]SAI emulation for voq switch init
Nov 9, 2020
129c61f
[vs]VOQ Switch initialization
Nov 10, 2020
99189b4
Merge remote-tracking branch 'origin/master' into voq-vstest-pr6
Nov 13, 2020
259a851
[vs]VOQ Switch Initialization
Nov 13, 2020
9937954
[vs]VOQ Switch Initialization
Nov 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions vslib/src/VirtualSwitchSaiInterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,12 @@ sai_status_t VirtualSwitchSaiInterface::create(
return SAI_STATUS_FAILURE;
}

// Initialize switch for VOQ attributes
if ((ss->initialize_voq_switch_objects(attr_count, attr_list)) != SAI_STATUS_SUCCESS)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is in the wrong place, it should be inside SwitchStateBase.cpp class in initialize objects

Copy link
Contributor Author

@vganesan-nokia vganesan-nokia Nov 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

As discussed in PR #701, this is unchanged and going to be called from here as it is now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

agree, but there are places init_default_objects inside _init_switch and warm_boot_initialize_objects,
current implementation you want to execute only for default objects, and this place here is actually executing it for both, which will cause to duplicate system port existence in virtual switch for warm boot,
so i would propose to add attr_count/list to init_switch and move this initialization in line 604

Copy link
Collaborator

Choose a reason for hiding this comment

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

also when you will want to support warm boot scenario for this, then you will need to add warm boot case in init_switch also

Copy link
Contributor Author

@vganesan-nokia vganesan-nokia Nov 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

For comment 1: o.k. I'll fix proposed.
For comment 2: o.k. when we support warm boot (only when system ports become dynamic config. Not sure when), this place will be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Moved initialize_voq_switch_objects() to init_switch() as proposed.

{
SWSS_LOG_THROW("VOQ switch initialization failed!");
}

if (warmBootState != nullptr)
{
update_local_metadata(switchId);
Expand Down