-
Notifications
You must be signed in to change notification settings - Fork 163
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
Collect asic info and store in CHASSIS_STATE_DB #175
Conversation
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@@ -163,6 +170,9 @@ class ModuleUpdater(logger.Logger): | |||
CHASSIS_MODULE_INFO_SLOT_FIELD, | |||
CHASSIS_MODULE_INFO_OPERSTATUS_FIELD] | |||
|
|||
chassis_state_db = daemon_base.db_connect("CHASSIS_STATE_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.
This new table is kept in CHASSIS_STATE_DB, unlike the other tables like MIDPLANE_TABLE etc that is kept in STATE_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.
See this comment in PR description that the info is kept in CHASSIS_STATE_DB because it's accessible by swss containers. Does the swss use this asic present info ?
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.
There are a couple of places we can keep asic info like STATE_DB (redis instance) or CHASSIS_STATE_DB (redis chassis instance).
CHASSIS_STATE_DB is accessible from every where in swss like/usr/bin/swss.sh or /usr/bin/local/swss.sh
- script creating swss containerand
docker_init.sh` inside swss container. Inside swss container, STATE_DB is not accessible (asic database is mapped, instead of database). So CHASSIS_STATE_DB is a safer choice.
@judyjoseph - can you please merge asap please, needed for 202106 release |
Add a generic function that returns a list of asics on module. Vendors will implement their own for their platform. The function could be used for collecting fabric asic info in each module and populating into CHASSIS_STATE_DB (sonic-net/sonic-platform-daemons#175). To simplify the change, all fabric cards in system should be identical in terms of number of asics in a card.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: ngocdo <ngocdo@arista.com>
Signed-off-by: ngocdo <ngocdo@arista.com>
@judyjoseph I don't think I have permissions to merge. Could you please help with that? I have approved. |
Chassisd inside pmon queries asic information from platform modules and stores it into CHASSIS_STATE_DB of redis_chassis. The reason we keep information in CHASSIS_STATE_DB because it's accessible by swss containers. Asic information includes asic pci address, module name, and asic id in the module. For example, CHASSIS_STATE_DB will look like below: 127.0.0.1:6379[6]> HGETALL "CHASSIS_ASIC_TABLE|ASIC0" 1) "asic_pci_address" 2) "0000:09:00.0" 3) "module_name" 4) "FABRIC-CARD0" 5) "asic_id_in_module" 6) "0"
asics = list(self.asic_table.getKeys()) | ||
for asic in asics: | ||
fvs = self.asic_table.get(asic) | ||
if fvs[CHASSIS_MODULE_INFO_NAME_FIELD] in notOnlineModules: |
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.
@ngoc-do
Looks like there is a bug here? Are you seeing this as well? fvs is in the format below:
[True, (('asic_pci_address', 'n/a'), ('name', 'FABRIC-CARD6'), ('asic_id_in_module', '1'))]
However, if the code is changed to below, it works:
fvp = dict(fvs[1])
if fvp[CHASSIS_MODULE_INFO_NAME_FIELD] in notOnlineModules:
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.
Hmm I don't see that. But if this happens, the unit test will fail, I think?
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.
unit-tests use mock_swsscommon.py, where it uses a simple dictionary. So, unit-tests are passing. However, I am seeing the behavior I mentioned previously in the real hw environment. Are you testing on the near latest github? Will try and investigate. Will help if you can confirm/investigate as well.
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.
Ah OK.
Yes, I tested on our chassis when committing the change. But it could be that there were new changes at the upstream, and I didn't have latest upstream in our local repos by the time I tested. I will double check, but I'm afraid that our local repos hasn't been updated at this moment. So if you see it needs to be fixed, free to do that.
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.
OK, I found it happened on our latest update.
I have a fix in #203. Please review.
Add a generic function that returns a list of asics on module. Vendors will implement their own for their platform. The function could be used for collecting fabric asic info in each module and populating into CHASSIS_STATE_DB (sonic-net/sonic-platform-daemons#175). To simplify the change, all fabric cards in system should be identical in terms of number of asics in a card.
This commit is already present in 202106 sonic-buildimage submodule ptr. |
Fix a bug coming from PR #175 when asic is removed due to card getting offline (before it was online).
Fix a bug coming from PR #175 when asic is removed due to card getting offline (before it was online).
…t#175) Fix `get_status_led()` API definition to conform to the description. Signed-off-by: Volodymyr Boyko <volodymyrx.boiko@intel.com>
Asic PCI ID (PCI address) is collected by chassisd (inside pmon - sonic-net/sonic-platform-daemons#175) and saved in CHASSIS_STATE_DB (in redis_chassis). CHASSIS_STATE_DB is accessible by swss containers. At docker-init.sh (script is called after swss container is created and before anything that could run in swss like orchagent...), we wait until asic PCI ID of the corresponding asic is populated by chassisd. We then update asic_id in CONFIG_DB of asic's database. A system supporting dynamic asic PCI ID identification requires to have a file (empty) use_pci_id_chassis in its platform dir. When orchagent runs, it has correct asic PCI ID in its CONFIG_DB. Together with this PR: sonic-net/sonic-platform-daemons#175 sonic-net/sonic-platform-common#185 Signed-off-by: Maxime Lorrillere <mlorrillere@arista.com>
… CONFIG_DB (#9681) Asic PCI ID (PCI address) is collected by chassisd (inside pmon - sonic-net/sonic-platform-daemons#175) and saved in CHASSIS_STATE_DB (in redis_chassis). CHASSIS_STATE_DB is accessible by swss containers. At docker-init.sh (script is called after swss container is created and before anything that could run in swss like orchagent...), we wait until asic PCI ID of the corresponding asic is populated by chassisd. We then update asic_id in CONFIG_DB of asic's database. A system supporting dynamic asic PCI ID identification requires to have a file (empty) use_pci_id_chassis in its platform dir. When orchagent runs, it has correct asic PCI ID in its CONFIG_DB. Together with this PR: sonic-net/sonic-platform-daemons#175 sonic-net/sonic-platform-common#185 Signed-off-by: Maxime Lorrillere <mlorrillere@arista.com> Co-authored-by: Maxime Lorrillere <mlorrillere@arista.com>
Signed-off-by: ngocdo ngocdo@arista.com
chassisd inside pmon queries asic information from platform modules and stores it into CHASSIS_STATE_DB of redis_chassis.
The reason we keep information in CHASSIS_STATE_DB because it's accessible by swss containers.
Asic information includes asic pci address, module name, and asic id in the module. For example, CHASSIS_STATE_DB will look like below:
This PR will merge only after sonic-net/sonic-platform-common#185 merges since this PR calls
get_all_asics
that is implemented in the other one.