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

fix sfp eeprom unreadable after switching from SW to FW control mode #9

Closed
wants to merge 1 commit into from

Conversation

yuazhe
Copy link
Owner

@yuazhe yuazhe commented Jul 12, 2024

Why I did it

Work item tracking
  • Microsoft ADO (number only):

How I did it

How to verify it

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

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

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

@yuazhe yuazhe force-pushed the fix_sfp_eeprom_unreadable branch 4 times, most recently from b1c5927 to 2084eea Compare July 12, 2024 03:28
@yuazhe yuazhe force-pushed the fix_sfp_eeprom_unreadable branch 5 times, most recently from b56fa74 to fe54188 Compare July 12, 2024 07:12
yuazhe pushed a commit that referenced this pull request Jul 12, 2024
Why I did it
Advance dhcpmon to a3c5381 in 202305 branch.

a3c5381 - (HEAD, origin/master, origin/HEAD, master) Merge pull request src: Add libnl3 build.sh script #11 from jcaiMR/dev/jcai_fix_err_log (11 days ago) [StormLiangMS]
c5ef7e7 - Change common_libs dependencies from buster to bullseye (Updating docker-orchagent/syncd Dockerfile and start.sh #9)
824a144 - replace atoi with strtol (Rename hostname #6) (10 weeks ago) [Mai Bui]
32c0c3f - Fix libswsscommon package installation for non-amd64 (README.md leaves out docker-database #7) (10 weeks ago) [Saikrishna Arcot]
Work item tracking
Microsoft ADO (25048723):
How I did it
How to verify it
Run test_dhcp_relay.py, no failure
@@ -488,6 +488,12 @@ def get_presence(self):
return False
eeprom_raw = self._read_eeprom(0, 1, log_on_error=False)
return eeprom_raw is not None

def is_eeprom_ready(self, wait_time):

Choose a reason for hiding this comment

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

Suggest to have a function like this:

        @classmethod
    def wait_sfp_eeprom_ready(cls, sfp_list, wait_time):
        not_ready_list = sfp_list
        
        while wait_time > 0:
            not_ready_list = [s for s in not_ready_list if s.state == STATE_FW_CONTROL and s._read_eeprom(0, 2,False) is None]
            if not_ready_list:
                time.sleep(0.1)
                wait_time -= 0.1
            else:
                return
        
        for s in not_ready_list:
            logger.log_error(f'SFP {s.sdk_index} eeprom is not ready')

@@ -1691,8 +1697,26 @@ def initialize_sfp_modules(cls, sfp_list):
if not s.in_stable_state():
logger.log_error(f'SFP {index} is not in stable state after initializing, state={s.state}')
logger.log_notice(f'SFP {index} is in state {s.state} after module initialization')



Choose a reason for hiding this comment

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

Simple call the function:

cls.wait_sfp_eeprom_ready(sfp_list, 2)

ready_sfp_set = wait_ready_task.get_ready_set()
for sfp_index in ready_sfp_set:
s = self._sfp_list[sfp_index]
s.on_event(sfp.EVENT_RESET_DONE)
if s.in_stable_state():
if s.get_control_type() == SFP_FW_CONTROL and not s.is_eeprom_ready(2):

Choose a reason for hiding this comment

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

Simple call the function:

self.sfp_module.SFP.wait_sfp_eeprom_ready([s], 2)

@yuazhe yuazhe force-pushed the fix_sfp_eeprom_unreadable branch from fe54188 to 673cee2 Compare July 12, 2024 08:21
@yuazhe yuazhe force-pushed the fix_sfp_eeprom_unreadable branch from 673cee2 to 3120e41 Compare July 12, 2024 08:25
@yuazhe yuazhe closed this Jul 12, 2024
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