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.

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

Add XSight platform and device configs #12160

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

deran1980
Copy link
Contributor

Signed-off-by: Roman Zhurakivskyy rzhurak@larch-networks.com

Why I did it

Adding a new device platform support in sonic-buildimage for Xsight Labs devices. (bases on Accton platforms)

How I did it

Added all Accton relevant platform drivers and code.
Uploading relevant code of Xsight based platform to other repos like sonic-platform-common, sairedis and sonic-swss.
Updating makefiles for Xsight based platforms and uploading pre-compiled relevant Debian files to Xsight Sonic repo.

How to verify it

Run SONiC image compiled for Xsight platform on Xsight switch

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205

Description for the changelog

In order to build a SONiC image for Xsight platform use the following commands:
NOJESSIE=1 NOSTRETCH=1 make PLATFORM=xsight

time NOJESSIE=1 NOSTRETCH=1 make all SONIC_BUILD_JOBS=12 INCLUDE_NAT=n INCLUDE_MACSEC=n INCLUDE_MUX=n

@lgtm-com
Copy link

lgtm-com bot commented Sep 22, 2022

This pull request introduces 330 alerts when merging ee8d6f93086d9a79dc85e3f06c4962f3a9e7c7fd into 8af369a - view on LGTM.com

new alerts:

  • 275 for Module-level cyclic import
  • 29 for Unused import
  • 8 for Variable defined multiple times
  • 7 for Unused local variable
  • 4 for 'import *' may pollute namespace
  • 4 for Use of 'global' at module level
  • 2 for Unreachable code
  • 1 for Wrong number of arguments in a class instantiation

@deran1980
Copy link
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@deran1980
Copy link
Contributor Author

Hi @lguohan
Can you help us with reviewing this PR?
I see the failure of the t1-lag on show techsupport which is not related to any of the changes we made and I can also see it fails on many other PRs and probably preventing their merge.

@deran1980 deran1980 force-pushed the master-xsight-upstream-squashed branch from ee8d6f9 to 79b9505 Compare October 6, 2022 07:30
@lgtm-com
Copy link

lgtm-com bot commented Oct 6, 2022

This pull request introduces 331 alerts when merging 715df114c2e4551686f9446d23a3e95637a424ea into 9251d4b - view on LGTM.com

new alerts:

  • 275 for Module-level cyclic import
  • 29 for Unused import
  • 8 for Variable defined multiple times
  • 7 for Unused local variable
  • 4 for 'import *' may pollute namespace
  • 4 for Use of 'global' at module level
  • 2 for Unreachable code
  • 1 for Except block handles 'BaseException'
  • 1 for Wrong number of arguments in a class instantiation

Signed-off-by: Roman Zhurakivskyy <rzhurak@larch-networks.com>
@deran1980 deran1980 force-pushed the master-xsight-upstream-squashed branch from 715df11 to 22f9e7c Compare November 2, 2022 12:14
@lgtm-com
Copy link

lgtm-com bot commented Nov 2, 2022

This pull request introduces 332 alerts when merging 22f9e7c into e1440f0 - view on LGTM.com

new alerts:

  • 276 for Module-level cyclic import
  • 29 for Unused import
  • 8 for Variable defined multiple times
  • 7 for Unused local variable
  • 4 for 'import *' may pollute namespace
  • 4 for Use of 'global' at module level
  • 2 for Unreachable code
  • 1 for Except block handles 'BaseException'
  • 1 for Wrong number of arguments in a class instantiation

@lguohan
Copy link
Collaborator

lguohan commented Nov 9, 2022

can you check the build failure, and also fix the lgtm errors?

@deran1980
Copy link
Contributor Author

can you check the build failure, and also fix the lgtm errors?

Hi @lguohan - thanks for your response.
I will re-trigger the pipeline for sonic-buildimage as before it only failed on show-techsupport test and all other tests passed.
now it seems to fail more tests, but I can't see the connection between these failures and our changes. (we mostly added specific platform files and not modifying any legacy code.)

Regarding the Alerts from lgtm - all of the alerts found by this tool are in Accton drivers platform code so we can not change it as it was provided to us by Accton team.

@deran1980
Copy link
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@deran1980
Copy link
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@deran1980
Copy link
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@deran1980
Copy link
Contributor Author

hi @lguohan
We want to re-initiate this process to push xsight code to be part of the sonic open source community.

I've re-trigger this PR one more time today just to see its current status.
2 things I cant understand currently from the last report:

  1. Is lgtm bot still active and used?
  2. I see from the report that there are 2 failures jobs, but it didn't seems to be run today. all others jobs passed ok.
    does the report stating "2 failing, 2 expected" means that what needed to pass did pass?

Really appreciate any help from your side to help us merging this PR.

Thanks,
Eran

@deran1980
Copy link
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@msosyak
Copy link
Contributor

msosyak commented Mar 11, 2024

@lguohan @yxieca All checks have passed. Could you merge this?

@msosyak
Copy link
Contributor

msosyak commented Mar 26, 2024

@lguohan @yxieca Could you please merge this PR?
The PR is updated, and the checks are passed. The PR introduced a new platform/device, with no other significant changes.
Please let us know if there are any reasons why it can not be merged.

There 2 power supplies slot at the left/right side of the back.
Once if a PSU is not plugged, the status of it is shown failed.

There are 48 SFP+ and 6 QSFP modules are equipped.
Copy link
Contributor

Choose a reason for hiding this comment

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

This file seems to not be relevant to the platform

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. we will remove these generic readme files.

@@ -0,0 +1,21 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

It probably would make more sense splitting this PR to a platform one and then add the specific device. It's a pretty large PR, hard for someone to actually review all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are actually 2 new platforms in this PR: 9632xx and 9632xq. (accton platforms)
Each platform has several SKUs supported.

The device is a from a new vendor called xsight, So I assume that if we want to reduce the size of the PR, we can separate it to a the new device (xsight) and a single platform (e.g 9632xq) and then add the 2nd platform. (9632xx)

It will still create a pretty big first PR.
Do you have any better suggestions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants