Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
swss-common: Changes to support SONiC Gearbox Manager #347
swss-common: Changes to support SONiC Gearbox Manager #347
Changes from 4 commits
d9ce0e8
c7bec86
7ccec8b
ba5cfe3
a9d5f9f
648b48f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is ASIC_DB2? it is unclear what it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Guohan;
The new ASIC_DB2 (as well as counters, flex, and state) database designations are being used by Gearbox and the new PHY SYNCD DOCKER.
These definitions are also specified in the specific HWSKU context_config.json files, such as in the following excerpt;
{
"guid" : 1,
"name" : "phy1",
"dbAsic" : "ASIC_DB2",
"dbCounters" : "COUNTERS_DB2",
"dbFlex": "FLEX_COUNTER_DB2",
"dbState" : "STATE_DB2",
"switches": [
{
"index" : 1,
"hwinfo" : ""
}
]
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should rename it to PHY_DB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thinking ahead here...
The design supports multiple phys for which each phy "could" have their own set of databases.
So I'm guessing the naming scheme made something like?
PHY_DB
PHY_DB2
etc...
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We used asic_db2 because we antiipate multiple switches in the future, and phy and switch are not differentiated in sairedis, so I believe we should give them all the same name. In the future, we may have multiredis support and we might then consider not needing this (redis then become namespace for the table names, one redis per switch). Please accept the change as it currently is proposed, future this might change but for now, asic_db2 is most consistent naming for switch related tables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all the phy db can be put into the same db since there are not much query/config activities. I think PHY_DB is a better name and give user more information on the nature of this db.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to clarify, a added "2" for each of second db names since i didn't have any better name, we could add PHY or GB (as gearbox) prefixx or something more explanatory then just "2"