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 get_aux_mon_type xcvr API #339

Merged

Conversation

dmytroxIntel
Copy link
Contributor

Description

Add error handling to get_aux_mon_type API

Motivation and Context

get_aux_mon_type reads field consts.AUX_MON_TYPE in eeprom on page 1 address 145
If memory model is flat, there is no page 1 in eeprom, only 0_lower and 0_upper
Here get_aux_mon_type doesn't check whether memory model is not flat, this causes errors in running xcvrd

How Has This Been Tested?

Tested on testbed, assured that logs of failed eeprom reading disappeared

Additional Information (Optional)

@dmytroxIntel
Copy link
Contributor Author

@prgeor please review and merge

@dmytroxIntel
Copy link
Contributor Author

@prgeor please review and merge

1 similar comment
@dmytroxIntel
Copy link
Contributor Author

@prgeor please review and merge

@dmytroxIntel
Copy link
Contributor Author

@prgeor please review and merge

@dmytroxIntel
Copy link
Contributor Author

@richardyu-ms, @StormLiangMS, please review and merge

Copy link

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

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

@dmytroxIntel Is there CLI we can check if it is flat memory? Or any output in the log?

@StormLiangMS StormLiangMS merged commit 9df998b into sonic-net:master Jan 5, 2023
@dmytroxIntel
Copy link
Contributor Author

dmytroxIntel commented Jan 5, 2023

@StormLiangMS didn't find one, I just put such code snipped in sonic_platform.sfp custom plugin to check this

    from sonic_platform_base.sonic_xcvr.mem_maps.public.cmis import CmisMemMap
    from sonic_platform_base.sonic_xcvr.codes.public.cmis import CmisCodes
    from sonic_platform_base.sonic_xcvr.xcvr_eeprom import XcvrEeprom
    from sonic_platform_base.sonic_xcvr.fields import consts

    def flatness(self):
        id_byte_raw = self.read_eeprom(0, 1)
        if id_byte_raw is None:
            return None
        id_byte = id_byte_raw[0]
        if id_byte == 0x18 or id_byte == 0x19 or id_byte == 0x1e:
            eeprom_wrapper = XcvrEeprom(self.read_eeprom, None, CmisMemMap(CmisCodes))
            return eeprom_wrapper.read(consts.FLAT_MEM_FIELD)
        return None

@zhangyanzhao
Copy link

@StormLiangMS is this for 202211 release? Otherwise, this need be approved and merged by the maintainer. Suggest to revert this one.

@dmytroxIntel
Copy link
Contributor Author

dmytroxIntel commented Jan 6, 2023

@zhangyanzhao , who is maintainer for this repo and file ?
It's not clear from this info https://github.com/sonic-net/sonic-buildimage/blob/master/.github/CODEOWNERS who exactly is in this group "@Azure/sonic-platform"

keboliu pushed a commit to keboliu/sonic-platform-common that referenced this pull request Jan 12, 2023
Description
Add error handling to get_aux_mon_type API

Motivation and Context
get_aux_mon_type reads field consts.AUX_MON_TYPE in eeprom on page 1 address 145
If memory model is flat, there is no page 1 in eeprom, only 0_lower and 0_upper
Here get_aux_mon_type doesn't check whether memory model is not flat, this causes errors in running xcvrd

How Has This Been Tested?
Tested on testbed, assured that logs of failed eeprom reading disappeared
StormLiangMS pushed a commit that referenced this pull request Jan 23, 2023
Description
Add error handling to get_aux_mon_type API

Motivation and Context
get_aux_mon_type reads field consts.AUX_MON_TYPE in eeprom on page 1 address 145
If memory model is flat, there is no page 1 in eeprom, only 0_lower and 0_upper
Here get_aux_mon_type doesn't check whether memory model is not flat, this causes errors in running xcvrd

How Has This Been Tested?
Tested on testbed, assured that logs of failed eeprom reading disappeared
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.

6 participants