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

[Celestica Seastone2] Build correct platform files #9660

Merged
merged 4 commits into from
Jan 16, 2022

Conversation

fyx
Copy link
Contributor

@fyx fyx commented Dec 29, 2021

Why I did it

I noticed show interfaces transceiver presence was not showing correct state for our inserted transceivers

How I did it

Redis database was missing TRANSCEIVER_INFO keys and found out that xcvrd process was crashing:

Dec 28 22:54:12.246211 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd Traceback (most recent call last):
Dec 28 22:54:12.246246 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd   File "/usr/local/bin/xcvrd", line 8, in <module>
Dec 28 22:54:12.246280 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd     sys.exit(main())
Dec 28 22:54:12.247190 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd   File "/usr/local/lib/python2.7/dist-packages/xcvrd/xcvrd.py", line 1441, in main
Dec 28 22:54:12.247190 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd     xcvrd.run()
Dec 28 22:54:12.247190 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd   File "/usr/local/lib/python2.7/dist-packages/xcvrd/xcvrd.py", line 1389, in run
Dec 28 22:54:12.247190 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd     self.init()
Dec 28 22:54:12.247255 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd   File "/usr/local/lib/python2.7/dist-packages/xcvrd/xcvrd.py", line 1352, in init
Dec 28 22:54:12.247904 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd     post_port_sfp_dom_info_to_db(is_warm_start, self.stop_event)
Dec 28 22:54:12.247904 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd   File "/usr/local/lib/python2.7/dist-packages/xcvrd/xcvrd.py", line 518, in post_port_sfp_dom_info_to_db
Dec 28 22:54:12.247904 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd     post_port_sfp_info_to_db(logical_port_name, int_tbl[asic_index], transceiver_dict, stop_event)
Dec 28 22:54:12.247904 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd   File "/usr/local/lib/python2.7/dist-packages/xcvrd/xcvrd.py", line 303, in post_port_sfp_info_to_db
Dec 28 22:54:12.247968 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd     if not _wrapper_get_presence(physical_port):
Dec 28 22:54:12.248586 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd   File "/usr/local/lib/python2.7/dist-packages/xcvrd/xcvrd.py", line 151, in _wrapper_get_presence
Dec 28 22:54:12.248586 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd     return platform_chassis.get_sfp(physical_port).get_presence()
Dec 28 22:54:12.248586 ixp-kg-sw1 INFO pmon#/supervisord: xcvrd AttributeError: 'NoneType' object has no attribute 'get_presence'
Dec 28 22:54:12.308896 ixp-kg-sw1 INFO pmon#supervisord 2021-12-28 22:54:12,307 INFO exited: xcvrd (exit status 1; not expected)
Dec 28 22:54:13.311306 ixp-kg-sw1 INFO pmon#supervisord 2021-12-28 22:54:13,309 INFO gave up: xcvrd entered FATAL state, too many start retries too quickly

After some investigation I figured out that the platform file /usr/local/lib/python2.7/dist-packages/sonic_platform/sfp.py was not intended for seastone2 but for silverstone. The build process was working in the relative directory for silverstone instead of seastone2.

How to verify it

Check logs for xcvrd and verify that it's not crashing anymore

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

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

Backport to earlier releases as this is a bug fix for critical hardware monitoring

Description for the changelog

[Celestica Seastone2] Build correct platform files

A picture of a cute animal (not mandatory but encouraged)

LRG_DSC02033

Signed-off-by: Viktor Ekmark <viktor@ekmark.se>
@fyx fyx requested a review from lguohan as a code owner December 29, 2021 22:18
@fyx fyx changed the title [Celestica] Build correct platform files for seastone2 [Celestica Seastone2] Build correct platform files for seastone2 Dec 29, 2021
@fyx fyx changed the title [Celestica Seastone2] Build correct platform files for seastone2 [Celestica Seastone2] Build correct platform files Dec 29, 2021
Signed-off-by: Christian Svensson <blue@cmd.nu>
@ghost
Copy link

ghost commented Dec 29, 2021

CLA assistant check
All CLA requirements met.

Earlier logic resulted in the name of SFP1 being SFP33 which is not
correct. The cannonical source is seastone2_fpga module and it calls it
SFP1, so ensure the logic does as well.

Signed-off-by: Christian Svensson <blue@cmd.nu>
Various changes that plumbs the correct port presence and DOM decoding
for the SFP1 port.

Signed-off-by: Christian Svensson <blue@cmd.nu>
@fyx
Copy link
Contributor Author

fyx commented Jan 15, 2022

After fixing the platform files we found some xcvrd issues related to adding SFP1 port in #8267 and we have included those patches in this PR as well

@prgeor
Copy link
Contributor

prgeor commented Jan 16, 2022

Please NOTE SONiC has moved to using Sfp refactored code which is mostly platform independent code. Its advised to move your platform to start using sonic_xcvr https://github.com/Azure/sonic-platform-common/tree/master/sonic_platform_base/sonic_xcvr

Ref: SFP refactoring HLD. SFP refactoring HLD

Example Dell platform moved to using SFP refactored code : - #9016

@prgeor prgeor merged commit 1b657be into sonic-net:master Jan 16, 2022
@bluecmd
Copy link
Contributor

bluecmd commented Jan 16, 2022

@prgeor Got it - that might be a thing we have to defer to Celestica for, we only use the platform. The refactor does look very nice though, so hopefully they will adopt it.

As for backporting - could you add the relevant "Request for XYZ branch" labels? Or do we need somebody else to do that?

Thanks!

@prgeor prgeor added Request for 202106 Branch Request for 202111 Branch For PRs being requested for 202111 branch labels Jan 16, 2022
judyjoseph pushed a commit that referenced this pull request Jan 17, 2022
* fix workdir for seastone2

Signed-off-by: Viktor Ekmark <viktor@ekmark.se>

* seastone2: Add I2C SFP definition for SFP1

Signed-off-by: Christian Svensson <blue@cmd.nu>

* [device/cel_seastone_2] sfputil logic for SFP1

Earlier logic resulted in the name of SFP1 being SFP33 which is not
correct. The cannonical source is seastone2_fpga module and it calls it
SFP1, so ensure the logic does as well.

Signed-off-by: Christian Svensson <blue@cmd.nu>

* [device/cel_seastone_2] sysfs paths for SFP1

Various changes that plumbs the correct port presence and DOM decoding
for the SFP1 port.

Signed-off-by: Christian Svensson <blue@cmd.nu>

Co-authored-by: Christian Svensson <blue@cmd.nu>
@bluecmd bluecmd deleted the patch-1 branch February 4, 2022 17:01
@bluecmd
Copy link
Contributor

bluecmd commented Feb 4, 2022

@prgeor Sorry to annoy, but could you add "Request for 202012 branch" as well? Thanks!

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

Successfully merging this pull request may close these issues.

4 participants