SfpUtilBase: not all EEPROM data are parsed

[wjaco]

To complete the below get/set function, it is found those information are not parsed from EEPROM though there are parser handlers found.

We are on 202012 branch.

    #    get_rx_los
    #    get_tx_fault
    #    get_tx_disable
    #    get_tx_disable_channel
    #    get_lpmode
    #    get_power_override
    #    tx_disable
    #    tx_disable_channel
    #    set_lpmode
    #    set_power_override

Not found in the DB too - which I think is brought into by the sfputil base class only via xcvrd right ? Are these going to be available ?

  "TRANSCEIVER_DOM_SENSOR|Ethernet2": {
    "expireat": 1550333773.959492,
    "ttl": -0.001,
    "type": "hash",
    "value": {
      "rx1power": "0.4112",
      "rx2power": "0.5173",
      "rx3power": "0.4817",
      "rx4power": "0.5907",
      "rxpowerhighalarm": "5.4000",
      "rxpowerhighwarning": "2.4000",
      "rxpowerlowalarm": "-15.0031",
      "rxpowerlowwarning": "-11.0018",
      "temperature": "20.7305",
      "temphighalarm": "75.0000",
      "temphighwarning": "70.0000",
      "templowalarm": "-5.0000",
      "templowwarning": "0.0000",
      "tx1bias": "7.4940",
      "tx1power": "N/A",
      "tx2bias": "7.4940",
      "tx2power": "N/A",
      "tx3bias": "7.4940",
      "tx3power": "N/A",
      "tx4bias": "7.4940",
      "tx4power": "N/A",
      "txbiashighalarm": "10.0000",
      "txbiashighwarning": "8.5000",
      "txbiaslowalarm": "2.0000",
      "txbiaslowwarning": "3.0000",
      "txpowerhighalarm": "N/A",
      "txpowerhighwarning": "N/A",
      "txpowerlowalarm": "N/A",
      "txpowerlowwarning": "N/A",
      "vcchighalarm": "3.6300",
      "vcchighwarning": "3.4650",
      "vcclowalarm": "2.9700",
      "vcclowwarning": "3.1350",
      "voltage": "3.2939"
    }
  },

  "TRANSCEIVER_INFO|Ethernet2": {
    "expireat": 1550333773.954,
    "ttl": -0.001,
    "type": "hash",
    "value": {
      "application_advertisement": "N/A",
      "cable_length": "50",
      "cable_type": "Length Cable Assembly(m)",
      "connector": "MPOx12",
      "encoding": "64B66B",
      "ext_identifier": "Power Class 4(3.5W max), CLEI present, CDR present in Rx Tx",
      "ext_rateselect_compliance": "QSFP+ Rate Select Version 1",
      "hardware_rev": "05",
      "is_replaceable": "False",
      "manufacturer": "CISCO-AVAGO",
      "model": "AFBR-89CDDZ-CS3",
      "nominal_bit_rate": "255",
      "serial": "AVF2219S0T9",
      "specification_compliance": "{}",
      "type": "QSFP28 or later",
      "vendor_date": "2018-05-11 ",
      "vendor_oui": "00-17-6a"
    }
  },

  "TRANSCEIVER_STATUS|Ethernet2": {
    "expireat": 1550333773.9559891,
    "ttl": -0.001,
    "type": "hash",
    "value": {
      "status": "1"
    }
  },

[wjaco]

One more question - shouldn't the reset/set_xxx() functions be performed via xcvrd since that is the driver which does port initializations, transceiver status change event handlings ? There could be potential contention issues if they are done by 2 separate contexts (depending on the underlying device access).

[jleveque]

Hi @wjaco. Thanks for opening this issue.

To complete the below get/set function, it is found those information are not parsed from EEPROM though there are parser handlers found.

When you refer to "parser handlers," are you referring to these methods in the SfpBase API class?

get_rx_los
get_tx_fault
get_tx_disable
get_tx_disable_channel
get_lpmode
get_power_override
tx_disable
tx_disable_channel
set_lpmode
set_power_override

If so, I do agree that all of the above should be implemented in what is currently sfputilbase.py (however, low-power mode ('get_lpmode(), set_lpmode()`) uses a dedicated pin, and is not part of the EEPROM).

Not found in the DB too - which I think is brought into by the sfputil base class only via xcvrd right ? Are these going to be available ?

True. This data should ultimately get polled by xcvrd and stored in the State DB.

One more question - shouldn't the reset/set_xxx() functions be performed via xcvrd since that is the driver which does port initializations, transceiver status change event handlings ? There could be potential contention issues if they are done by 2 separate contexts (depending on the underlying device access).

Optimally, yes, xcvrd should be the arbiter of the transceivers, which will prevent any contention.

We are currently designing a refactor of the sonic_sfp package in sonic-platform-common, which will ultimately replace sfputilbase.py and create a more complete and well-structured API for handling common SFP transceiver interactions, thus eliminating the need for platform vendors to basically re-invent the wheel by writing similar EEPROM parsing code.

Separate from the sonic_sfp refactor, we also intend to update xcvrd such that it instantiates objects to interact with the transceivers, and all interaction goes through xcvrd, thus eliminating an potential contention. Therefore, the CLI for resetting a transceiver will need to be changed to send a message to xcvrd asking it to reset a transceiver.

I am moving this issue to the sonic-platfrom-common repo for the time being until the sonic_sfp refactor is complete.

[wjaco]

Thanks @jleveque .
Yes, this sounds perfect. We will watch for the enhancement. And yes, I was referring to the SfpBase and DeviceBase classes only - sorry for not being specific.

Thanks and regards,
Wilson Jacob
wjacob@cisco.com

[shyam77git]

Thanks @jleveque for the detail update.
Making xcrvd as the owner (central entity) for all sfp/optics related operations would provide a common base for all underlying platforms.

Just trying to see, tentatively how long these enhancements/refactoring would take?
That would help us determine as to what alternative we can have think/plan at platform's infra layer until this change from xcrvd is in place.

[jleveque]

Just trying to see, tentatively how long these enhancements/refactoring would take?
That would help us determine as to what alternative we can have think/plan at platform's infra layer until this change from xcrvd is in place.

We are still in the early stages of the design for the refactor of sonic_sfp. I don't expect it to be done for a few months. After that we will work on refactoring xcvrd. I don't have any ETAs at the moment.

[sachinv-msft]

@jleveque following up on this, any updates or ETA that you could share with us?

[prgeor]

@sachinv-msft can you please check the latest SFP-refactor code. HLD for the same.

[sachinv-msft]

@prgeor has there been a refactored code to address this issue? how is it that i can verify we are not failing on test cases because of this?

[prgeor]

@sachinv-msft yes this has been addressed in SFP-refactor in latest master. Ref. SFP-refactor HLD. Dell platform (use as an example) changes to use SFP-refactor: sonic-net/sonic-buildimage#9016

[prgeor]

@wjaco @sachinv-msft can this be closed?