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.

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

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

Closed
wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions platform/mellanox/mlnx-platform-api/sonic_platform/chassis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,29 @@ def get_change_event(self, timeout=0):
status = self.sfp_event.check_sfp_status(port_dict, timeout)

if status:
self.reinit_sfps(port_dict)
return True, {'sfp':port_dict}
else:
return True, {'sfp':{}}

def reinit_sfps(self, port_dict):
"""
Re-initialize SFP if there is any newly inserted SFPs
:param port_dict: SFP event data
:return:
"""
# SFP not initialize yet, do nothing
if not self.sfp_module_initialized:
return

from . import sfp
for index, status in port_dict.items():
if status == sfp.SFP_STATUS_INSERTED:
try:
self.get_sfp(index).reinit()
except Exception as e:
logger.log_error("Fail to re-initialize SFP {} - {}".format(index, repr(e)))

def get_thermal_manager(self):
from .thermal_manager import ThermalManager
return ThermalManager
Expand Down
10 changes: 10 additions & 0 deletions platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 •
edited
Loading

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


# Global logger class instance
logger = Logger()

Expand All @@ -296,6 +299,13 @@ def __init__(self, sfp_index, sfp_type):
self.sdk_handle = None
self.sdk_index = sfp_index

def reinit(self):
"""
Re-initialize this SFP object when a new SFP inserted
:return:
"""
self._detect_sfp_type(self.sfp_type)
self._dom_capability_detect()

#SDK initializing stuff
def _initialize_sdk_handle(self):
Expand Down