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

[Mellanox] Re-initialize SFP object when detecting a new SFP insertion #31

Closed
wants to merge 1 commit into from

Conversation

Junchao-Mellanox
Copy link
Owner

- Why I did it

SFP object will be initialized to a certain type even if no SFP present. A case could be:

  1. A SFP object is initialized to QSFP type by default when there is no SFP present
  2. User insert a SFP with an adapter to this QSFP port
  3. The SFP object fail to read EEPROM because it still treats itself as QSFP.

This PR fixes this issue.

- How I did it

When detecting a new SFP insertion, read its SFP type and DOM capability from EEPROM again.

- How to verify it

Manual test

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

  • 201811
  • 201911
  • 202006

- Description for the changelog

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

@@ -280,6 +280,9 @@
NVE_MASK = PORT_TYPE_MASK & (PORT_TYPE_NVE << PORT_TYPE_OFFSET)
CPU_MASK = PORT_TYPE_MASK & (PORT_TYPE_CPU << PORT_TYPE_OFFSET)

# parameters for SFP presence
SFP_STATUS_INSERTED = '1'
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it possible to reuse the "presence" status?

Copy link
Owner Author

@Junchao-Mellanox Junchao-Mellanox Oct 16, 2020

Choose a reason for hiding this comment

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

could u share more detail? Do u mean use the get_presence method?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes

Copy link
Owner Author

Choose a reason for hiding this comment

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

In get_presence, it uses ethtool to get the SFP presence status which is another process. Not sure how ethtool is implemented, but I suppose there might be risks that:

  • We get the port insert event from select, but ethtool still "think" the port is absence. (Not sure what the flow is)
  • As we tested before, ethtool has a performance issue

Based on that, I suppose we should directly use the status returned by the select, xcvrd also use it this way. Any suggestion?

Copy link
Collaborator

Choose a reason for hiding this comment

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

LGTM

@Junchao-Mellanox
Copy link
Owner Author

sonic-net#5695

@Junchao-Mellanox Junchao-Mellanox deleted the fix_insert_sfp branch November 18, 2020 01:06
Junchao-Mellanox pushed a commit that referenced this pull request Dec 14, 2020
This update brings in the following commits.

86c1108 Enable arm architecture to build in addition to amd64 (#37)
4acb2c3 fix bugs and enhance Transformer (#35)
49e5a22 ygot related enhancements and fixes (#34)
51224de Fix ietf yang search path for cvl schema builds (#32)
3c6cdb3 CVL Changes #8: 'must' and 'when' expression evaluation (#31)
dabf231 CVL Changes #7: 'leafref' evaluation (#28)
6f9535f CVL Changes #6: Customized Xpath Engine integration (#27)
5e2466b DB-Layer fixes/enhancements (#26)
9a27302 CVL Changes #4: Implementation of new CVL APIs (#22)
dbf1093 Translib support for authorization, yang versioning and Delete flag (#21)
80f369e CVL Changes #5: YParser enhancement (#23)
904ce18 CVL Changes #3: Multi-db instance support (#20)
9d24a34 CVL Changes #2:  YValidator infra changes for evaluating xpath expression (#19)
f3fc40f CVL Changes #1: Initial CVL code reorganization and common infra changes (#18)
4922601 Bulk and RPC API support in translib (#16)
1d730df RFC7895 yang module library implementation (#15)
Junchao-Mellanox pushed a commit that referenced this pull request Mar 29, 2022
f00efef Longxiang Lyu Wed Mar 16 09:12:46 2022 +0800 Add a command line option to store logs into a separate file (#41)
ff2e67d Longxiang Lyu Tue Mar 15 09:10:59 2022 +0800 Add default port cable type (#39)
ebbb4d8 Jing Zhang Mon Mar 14 15:41:11 2022 -0700 Prevent switching MUX to "Unknown" (#36)
c779b8f Longxiang Lyu Thu Mar 10 21:35:11 2022 +0800 [nonfunctional] Use LinkProberStateMachineBase (#38)
b9fedd0 Longxiang Lyu Wed Mar 9 13:03:58 2022 +0800 [NONFUNCTIONAL] Add LinkProberStateMachineBase (#37)
bedd42b Longxiang Lyu Wed Mar 9 10:03:00 2022 +0800 Add .clang-format file to format code (#28)
9fe4fc6 Guohan Lu Thu Mar 3 17:51:43 2022 -0800 [doc]: add lgtm badge in README.md
c1249d9 Longxiang Lyu Wed Mar 2 18:05:18 2022 +0800 Enable lgtm (#33)
b8514c6 Longxiang Lyu Wed Mar 2 13:34:39 2022 +0800 Collect port cable type to use corresponding state machine (#31)
9b59ef9 Longxiang Lyu Wed Mar 2 07:19:33 2022 +0800 Improve make clean (#32)
Junchao-Mellanox pushed a commit that referenced this pull request Oct 18, 2022
* [BFN] Canceling PSU platform API calls on SIGTERM

Signed-off-by: Andriy Kokhan <andriyx.kokhan@intel.com>

* [BFN] Fixed SONiC fwutil exec time (#31)

Signed-off-by: Taras Keryk <tarasx.keryk@intel.com>

Signed-off-by: Andriy Kokhan <andriyx.kokhan@intel.com>
Signed-off-by: Taras Keryk <tarasx.keryk@intel.com>
Co-authored-by: Taras Keryk <tarasx.keryk@intel.com>
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.

2 participants