Skip to content

Add platform_asic file to each platform folder in sonic-device-data based package #8542

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

Jump to bottom
Merged
merged 9 commits into from
Oct 9, 2021
Merged

Add platform_asic file to each platform folder in sonic-device-data based package #8542

merged 9 commits into from
Oct 9, 2021

Conversation

qiluo-msft
Copy link
Collaborator

@qiluo-msft qiluo-msft commented Aug 20, 2021 •
edited
Loading

Why I did it

Add platform_asic file to each platform folder in sonic-device-data package. The file content will be used as the ground truth of mapping from PLATFORM_STRING to switch ASIC family.

One use case of the mapping is to prevent installing a wrong image, which targets for other ASIC platforms. For example, currently we have several ONIE images naming as sonic-*.bin, it's easy to mistakenly install the wrong image. With this mapping built into image, we could fetch the ONIE platform string, and figure out which ASIC it is using, and check we are installing the correct image.

After this PR merged, each platform vendor has to add one mandatory text file device/PLATFORM_VENDOR/PLATFORM_STRING/platform_asic, with the content of the platform's switch ASIC family.

You can get a list of the ASIC platforms by ls -b platform | cat. Currently the options are

barefoot
broadcom
cavium
centec
centec-arm64
generic
innovium
marvell
marvell-arm64
marvell-armhf
mellanox
nephos
p4
vs

Also support

broadcom-dnx

How I did it

How to verify it

Test one image on DUT. And check the folders under /usr/share/sonic/device

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

@qiluo-msft qiluo-msft changed the title Qiluo/devicefolder Reorganize the folder structure under /device Aug 20, 2021
@lgtm-com
Copy link

lgtm-com bot commented Aug 23, 2021 •
edited by qiluo-msft
Loading

This pull request introduces 5 alerts and fixes 5 when merging 06ca952ea9f36f9734cb3c4a785de4175b5ceb44 into 26c6e7f - view on LGTM.com

new alerts:

  • 2 for Module imports itself
  • 2 for 'import *' may pollute namespace
  • 1 for Unused import

fixed alerts:

  • 2 for Module imports itself
  • 2 for 'import *' may pollute namespace
  • 1 for Unused import

In reply to: 903559173

@sonic-net sonic-net deleted a comment from lgtm-com bot Aug 23, 2021
@lgtm-com
Copy link

lgtm-com bot commented Aug 24, 2021 •
edited by qiluo-msft
Loading

This pull request introduces 5 alerts and fixes 5 when merging d2d518f1c3f2e75ed3d473e14ffaf9b2a17a2df6 into 5ff87e1 - view on LGTM.com

new alerts:

  • 2 for Module imports itself
  • 2 for 'import *' may pollute namespace
  • 1 for Unused import

fixed alerts:

  • 2 for Module imports itself
  • 2 for 'import *' may pollute namespace
  • 1 for Unused import

In reply to: 904557767

@lguohan
Copy link
Collaborator

lguohan commented Aug 24, 2021 •
edited by qiluo-msft
Loading

i do not think asic vendor and platform vendor are strict hiercharical,
a platform vendor can provide platform driver for two different asic.

suggest not to introduce such hierachy.

In reply to: 904871309

@qiluo-msft qiluo-msft marked this pull request as ready for review August 25, 2021 01:18
@qiluo-msft qiluo-msft requested a review from lguohan as a code owner August 25, 2021 01:18
@qiluo-msft qiluo-msft marked this pull request as draft August 30, 2021 10:16
@qiluo-msft qiluo-msft marked this pull request as ready for review August 31, 2021 12:10
@qiluo-msft qiluo-msft changed the title Reorganize the folder structure under /device Stripe the content in the package of sonic-device-data Aug 31, 2021
@qiluo-msft qiluo-msft changed the title Stripe the content in the package of sonic-device-data Reduce the content in the package of sonic-device-data Sep 1, 2021
lguohan
lguohan reviewed Sep 1, 2021
View reviewed changes
device/arista/x86_64-arista_7280cr3_32d4/platform_asic Outdated
@@ -0,0 +1 @@
broadcom
Copy link
Collaborator

@lguohan lguohan Sep 1, 2021 •
edited by qiluo-msft
Loading

Choose a reason for hiding this comment

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

broadcom

this is available for sonic-broadcom-dnx.bin not in sonic-broadcom.bin

can you differ? #Resolved

Copy link
Collaborator Author

@qiluo-msft qiluo-msft Sep 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Technically they are using the same CONFIGURED_PLATFORM. So I cannot differentiate them by this dimention. We could improve in future.

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 a must fix

Copy link
Collaborator

Choose a reason for hiding this comment

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

please sync with judy

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I fixed the files under dnx platforms. I decided not reduce package content and keep the build/installation behavior as before.

lguohan
lguohan reviewed Sep 1, 2021
View reviewed changes
device/arista/x86_64-arista_7050_qx32/platform_asic
@@ -0,0 +1 @@
broadcom
Copy link
Collaborator

@lguohan lguohan Sep 1, 2021 •
edited by qiluo-msft
Loading

Choose a reason for hiding this comment

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

all arista devices are only available for sonic-broadcom.swi image, not sonic-broadcom.bin image, can you differentiate that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They are using the same CONFIGURED_PLATFORM (broadcom). So I cannot differentiate them by this dimention. If we have ground truth about bootloader for each platform, we could improve in future.

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 a must fix

Copy link
Collaborator

Choose a reason for hiding this comment

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

please sync with ying.

@qiluo-msft qiluo-msft changed the title Reduce the content in the package of sonic-device-data Reduce the content in the package of sonic-device-data based on asic Sep 2, 2021
@qiluo-msft qiluo-msft marked this pull request as draft September 17, 2021 02:51
@qiluo-msft qiluo-msft force-pushed the qiluo/devicefolder branch 2 times, most recently from 3f29360 to 54d7d12 Compare September 17, 2021 14:28
@qiluo-msft qiluo-msft changed the title Reduce the content in the package of sonic-device-data based on asic Add platform_asic file to each platform folder in sonic-device-data based package Sep 17, 2021
@qiluo-msft qiluo-msft marked this pull request as ready for review September 20, 2021 07:20
@akokhan
Copy link
Contributor

akokhan commented Sep 20, 2021 •
edited by qiluo-msft
Loading

Could you please describe a use-case or intention behind this? Also, are you planning to update platform porting guide with this info? Thank you in advance.

In reply to: 922882822

@qiluo-msft
Copy link
Collaborator Author

qiluo-msft commented Sep 20, 2021 •
edited
Loading

Updated PR description.

In reply to: 922888780

@wen587 wen587 mentioned this pull request Sep 22, 2021
Merged
5 tasks
@akokhan
Copy link
Contributor

akokhan commented Sep 22, 2021 •
edited by qiluo-msft
Loading

@qiluo-msft , to avoid adding platform_asic file into the each and every device folder, can't this information be injected into the final image at build stage? Looks like it's the same as PLATFORM configuration option. Or it's something different?

In reply to: 924671625

@qiluo-msft
Copy link
Collaborator Author

Thanks for the feedback!
My understanding is that the content is almost same as PLATFORM. However there is no ground truth of the mapping from ONIE platform to ASIC anywhere. And currently all the platforms are built into the image so nothing is missing.

broadcom-dnx is a special ASIC, and PLATFORM is still broadcom.

In reply to: 924671625

lguohan
lguohan reviewed Sep 28, 2021
View reviewed changes
device/arista/x86_64-arista_7800r3_48cqm2_lc/platform_asic Outdated
@@ -0,0 +1 @@
broadcom
Copy link
Collaborator

@lguohan lguohan Sep 28, 2021 •
edited by qiluo-msft
Loading

Choose a reason for hiding this comment

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

should be dnx #Closed

lguohan
lguohan reviewed Sep 28, 2021
View reviewed changes
device/arista/x86_64-arista_7280cr3mk_32p4/platform_asic Outdated
@@ -0,0 +1 @@
broadcom
Copy link
Collaborator

@lguohan lguohan Sep 28, 2021 •
edited by qiluo-msft
Loading

Choose a reason for hiding this comment

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

dnx #Closed

lguohan
lguohan reviewed Sep 28, 2021
View reviewed changes
device/arista/x86_64-arista_7280cr3mk_32d4/platform_asic Outdated
@@ -0,0 +1 @@
broadcom
Copy link
Collaborator

@lguohan lguohan Sep 28, 2021 •
edited by qiluo-msft
Loading

Choose a reason for hiding this comment

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

dnx #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I fixed all 3. Please check.

@qiluo-msft
Copy link
Collaborator Author

@arunlk-dell Could you help review?

lguohan
lguohan reviewed Sep 28, 2021
View reviewed changes
device/arista/x86_64-arista_7800_sup/platform_asic Outdated
@@ -0,0 +1 @@
broadcom
Copy link
Collaborator

Choose a reason for hiding this comment

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

broadcom-dnx

lguohan
lguohan reviewed Sep 28, 2021
View reviewed changes
device/arista/x86_64-arista_common/platform_asic
@@ -0,0 +1 @@
broadcom
Copy link
Collaborator

Choose a reason for hiding this comment

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

i do not know what this is. this is common, it should be included in both broadcom and broadcom-dnx

@lguohan
Copy link
Collaborator

lguohan commented Sep 28, 2021

others lgtm.

@qiluo-msft
Copy link
Collaborator Author

I just updated https://github.com/Azure/SONiC/wiki/Porting-Guide

@wen587 wen587 self-requested a review September 30, 2021 02:22
wen587
wen587 approved these changes Sep 30, 2021
View reviewed changes
Copy link
Contributor

@wen587 wen587 left a comment

Choose a reason for hiding this comment

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

lgtm

@qiluo-msft
Copy link
Collaborator Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

lguohan
lguohan approved these changes Oct 8, 2021
View reviewed changes
Copy link
Collaborator

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

:shipit:

@qiluo-msft qiluo-msft merged commit add9b65 into sonic-net:master Oct 9, 2021
@qiluo-msft qiluo-msft deleted the qiluo/devicefolder branch October 9, 2021 02:27
@smaheshm smaheshm mentioned this pull request Mar 8, 2022
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msosyak msosyak msosyak left review comments

@lguohan lguohan lguohan approved these changes

@wen587 wen587 wen587 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@qiluo-msft @lguohan @akokhan @wen587 @msosyak @smaheshm @arlakshm