-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Using pci device id to identify the ASIC on sai_switch_create #4705
Conversation
… the "asic_id" field in the DEVICE_METADATA. Going forward will use the PCI device id as the asic instance ID passed to syncd/sai while doing create_switch.
…asic.conf files. In that case we use the asic_id itself which is an integer
We need to make any changes for multi ASIC VS to work ? |
src/sonic-config-engine/sonic-cfggen
Outdated
hardware_data['DEVICE_METADATA']['localhost'].update(asic_id=asic_id) | ||
pci_id = get_npu_pci_device_id(asic_id) | ||
# if the pci_id obtained is None, use the asic_id itself. | ||
device_id = asic_id if pci_id is None else pci_id |
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.
When we dont get the pic_id do we want to throw an error instead of using asic_id.
Can SAI understand both pci_id and asic_id ?
With this SAI 3.7.5.1 it understands only PCI device id for multi-asic. I can make this change to error out if we don't get the pci-id + make this change only in 201911 branch.
My thought initially was to share this patch with public/master 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.
We need to make any changes for multi ASIC VS to work ?
Yes, what I find is we will need some work to be done with the multi-asic VS to pass the instance ID to orchagent. Additionaly to use pci-id as the instance id to pass to the sai stub in VS, we need to update the asic.conf file for VS platform.
We could take this as a separate activity ?
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.
lgtm
…d be pci_id for some platforms.
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.
lgtm
…ster branch (sonic-net#7513) Port the fix to get the pci_device ID from asic.conf file and update the "asic_id" field in DEVICE_METADATA with the pci_device_Id. Ref: sonic-net#4705
…ster branch (sonic-net#7513) Port the fix to get the pci_device ID from asic.conf file and update the "asic_id" field in DEVICE_METADATA with the pci_device_Id. Ref: sonic-net#4705
- Why I did it
The broadcom SAI takes in the pci device ID's to identify the ASIC instead of the instance ID going forward from 3.7.5.1. Currently it supports only one particular multi-asic device ( Wider support for more multi-asic platforms is to come later )
Introducing this change in 201911 branch.
- How I did it
To incorporate this, the following changes are proposed.
Update the "asic.conf" to include the PCI device ids for the ASIC's. Sample "asic.conf" is as below. Here in this eg: the ASIC_ID 0 has the PCI id 03:00.0, ASIC_ID 1 has the PCI id 04:00.0 .. etc.
NUM_ASIC=n
DEV_ID_ASIC_0=03:00.0
DEV_ID_ASIC_1=04:00.0
DEV_ID_ASIC_2=05:00.0
Introduced an API get_npu_pci_device_id() which will get the PCI id for a given npu id
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)