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

Using SFP refactoring framework in PDDF sfp class #10047

Merged
merged 2 commits into from
Mar 16, 2022

Conversation

FuzailBrcm
Copy link
Contributor

Why I did it

All the SFP platform API classes need to use SFP refactoring framework. The platforms which use PDDF, derive their SFP API class from a common pddf_sfp.py. Hence pddf_sfp.py needs to comply with SFP refactoring.

How I did it

Used sfpOptoeBase class in PddfSfp

How to verify it

Verified with the following logs on AS7326 platform.

root@sonic:/home/admin# sfputil show presence
Port        Presence
----------  -----------
Ethernet0   Not present
Ethernet1   Not present
Ethernet2   Not present
Ethernet3   Present
Ethernet4   Present
Ethernet5   Not present
Ethernet6   Present
Ethernet7   Present
Ethernet8   Not present
Ethernet9   Not present
Ethernet10  Not present
Ethernet11  Not present
Ethernet12  Not present
Ethernet13  Not present
Ethernet14  Not present
Ethernet15  Not present
Ethernet16  Not present
Ethernet17  Not present
Ethernet18  Not present
Ethernet19  Not present
Ethernet20  Present
Ethernet21  Not present
Ethernet22  Not present
Ethernet23  Not present
Ethernet24  Not present
Ethernet25  Not present
Ethernet26  Not present
Ethernet27  Not present
Ethernet28  Not present
Ethernet29  Not present
Ethernet30  Not present
Ethernet31  Not present
Ethernet32  Not present
Ethernet33  Not present
Ethernet34  Not present
Ethernet35  Not present
Ethernet36  Not present
Ethernet37  Not present
Ethernet38  Not present
Ethernet39  Not present
Ethernet40  Present
Ethernet41  Present
Ethernet42  Present
Ethernet43  Present
Ethernet44  Present
Ethernet45  Present
Ethernet46  Present
Ethernet47  Present
Ethernet48  Present
Ethernet52  Present
Ethernet56  Present
Ethernet60  Present
Ethernet64  Present
Ethernet68  Present
Ethernet72  Present
Ethernet76  Present
root@sonic:/home/admin#
root@sonic:/home/admin# sfputil show eeprom

Ethernet0: SFP EEPROM not detected

Ethernet1: SFP EEPROM not detected

Ethernet2: SFP EEPROM not detected

Ethernet3: SFP EEPROM detected
        Application Advertisement: N/A
        Connector: Copper pigtail
        Encoding: Unspecified
        Extended Identifier: GBIC/SFP defined by two-wire interface ID
        Extended RateSelect Compliance: Unknown
        Identifier: SFP/SFP+/SFP28
        Length OM3(10m): 32.0
        Nominal Bit Rate(100Mbs): 103
        Specification compliance:
                10G Ethernet Compliance: Unknown
                ESCON Compliance: Unknown
                Ethernet Compliance: 1000BASE-CX
                Fibre Channel Link Length: Medium (M)
                Fibre Channel Speed: Unknown
                Fibre Channel Transmission Media: Twin Axial Pair (TW)
                Fibre Channel Transmitter Technology: Unknown
                Infiniband Compliance: 1X Copper Passive
                SFP+CableTechnology: Passive Cable
                SONET Compliance Codes: Unknown
        Vendor Date Code(YYYY-MM-DD Lot): 2018-01-24
        Vendor Name: Fiberstore
        Vendor OUI: 00-05-1e
        Vendor PN: Q-4SPC03
        Vendor Rev: A
        Vendor SN: CAN0812400121-4

Ethernet4: SFP EEPROM detected
        Application Advertisement: N/A
        Connector: Copper pigtail
        Encoding: Unspecified
        Extended Identifier: GBIC/SFP defined by two-wire interface ID
        Extended RateSelect Compliance: Unknown
        Identifier: SFP/SFP+/SFP28
        Length OM3(10m): 32.0
        Nominal Bit Rate(100Mbs): 103
        Specification compliance:
                10G Ethernet Compliance: Unknown
                ESCON Compliance: Unknown
                Ethernet Compliance: 1000BASE-CX
                Fibre Channel Link Length: Medium (M)
                Fibre Channel Speed: Unknown
                Fibre Channel Transmission Media: Twin Axial Pair (TW)
                Fibre Channel Transmitter Technology: Unknown
                Infiniband Compliance: 1X Copper Passive
                SFP+CableTechnology: Passive Cable
                SONET Compliance Codes: Unknown
        Vendor Date Code(YYYY-MM-DD Lot): 2018-01-24
        Vendor Name: Fiberstore
        Vendor OUI: 00-05-1e
        Vendor PN: Q-4SPC03
        Vendor Rev: A
        Vendor SN: CAN0812400121-2

Ethernet5: SFP EEPROM not detected

Ethernet6: SFP EEPROM detected
        Application Advertisement: N/A
        Connector: Copper pigtail
        Encoding: Unspecified
        Extended Identifier: GBIC/SFP defined by two-wire interface ID
        Extended RateSelect Compliance: Unknown
        Identifier: SFP/SFP+/SFP28
        Length OM4(10m): 3.0
        Nominal Bit Rate(100Mbs): 255
        Specification compliance:
                10G Ethernet Compliance: Unknown
                ESCON Compliance: Unknown
                Ethernet Compliance: Unknown
                Fibre Channel Link Length: Unknown
                Fibre Channel Speed: Unknown
                Fibre Channel Transmission Media: Unknown
                Fibre Channel Transmitter Technology: Unknown
                Infiniband Compliance: Unknown
                SFP+CableTechnology: Passive Cable
                SONET Compliance Codes: Unknown
        Vendor Date Code(YYYY-MM-DD Lot): 2018-06-06
        Vendor Name: Fiberstore
        Vendor OUI: 00-02-c9
        Vendor PN: Q-4S2803
        Vendor Rev:
        Vendor SN: C1805137653-3
..
..
root@sonic:/home/admin# 

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

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

Description for the changelog

Link to config_db schema for YANG module changes

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

@FuzailBrcm FuzailBrcm requested a review from lguohan as a code owner February 21, 2022 09:11
@FuzailBrcm
Copy link
Contributor Author

@ds952811 , @prgeor
Please review this PR

ds952811
ds952811 previously approved these changes Feb 23, 2022
@lguohan lguohan requested a review from prgeor March 1, 2022 21:11
@FuzailBrcm
Copy link
Contributor Author

@prgeor
Will you please review this change?

@@ -1109,61 +264,11 @@ def tx_disable(self, tx_disable):
"""
# find out a generic implementation of tx_disable for SFP, QSFP and OSFP
status = False
if not self.get_presence():
return tx_disable

device = 'PORT{}'.format(self.port_index)
path = self.pddf_obj.get_path(device, 'xcvr_txdisable')

# TODO: put the optic based reset logic using EEPROM
Copy link
Contributor

Choose a reason for hiding this comment

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

why override tx_disable() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, if CPLD setting is present for tx_disable then that would be used. Else common SfpOptoeBase implementation is used. I think this common implementation does the same eeprom register set operations which was removed from here.

return [tx1_pw, tx2_pw, tx3_pw, tx4_pw]
else:
return None
return lpmode

def get_intr_status(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be removed if NOT used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some platforms do have CPLD reg setting to read the xcvr interrupt status, hence it was added.

Comment on lines 195 to +197
else:
return None
# Use common SfpOptoeBase implementation for get_lpmode
lpmode = super().get_lpmode()
Copy link
Contributor

Choose a reason for hiding this comment

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

will the 'else' part be executed for QSFP-DD optics?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, given that lpmode setting cant be done via CPLD for any given platform.

status = int(output['status'].rstrip())

if status == 1:
tx_fault = True
else:
tx_fault = False
else:
# Use common SfpOptoeBase implementation for get_tx_dault
Copy link
Contributor

Choose a reason for hiding this comment

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

typo error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will correct

rx_los = (sffbase().test_bit(data, 1) != 0)

else:
if output:
Copy link
Contributor

Choose a reason for hiding this comment

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

why override sfp_optoe_base class's get_rx_los ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In some platforms, rxlos can be set via CPLD. In such cases, PDDF JSON file would have the details of CPLD address, offset etc. If it is found, then we use PDDF parsing to write to the CPLD reg. If no such CPLD setting is present, then we use sf_optoe_base class's implementation.

@@ -749,42 +136,19 @@ def get_tx_fault(self):
Note : TX fault status is lached until a call to get_tx_fault or a reset.
Copy link
Contributor

Choose a reason for hiding this comment

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

why override get_tx_fault() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if CPLD setting for get_tx_fault() is present for any platform, that needs to be used. Else the common get_tx_fault() implementation of SfpOptoeBase

else:
# SFP doesnt support this
return 0

def get_lpmode(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

why override sfpopote's get_lpmode. ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if CPLD setting for lpmode is present for any platform, that needs to be used. Else the common get_lpmode implementation of SfpOptoeBase

Copy link
Contributor Author

@FuzailBrcm FuzailBrcm left a comment

Choose a reason for hiding this comment

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

replied to the comments

@prgeor prgeor merged commit 98cfec2 into sonic-net:master Mar 16, 2022
@FuzailBrcm
Copy link
Contributor Author

This PR is required in 202111 branch. Can someone cherry-pick it?
@prgeor @lguohan @xumia

@prgeor prgeor added Request for 202111 Branch For PRs being requested for 202111 branch Included in 202111 Branch labels Apr 13, 2022
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.

3 participants