Skip to content

400zr initial support#228

Merged
prgeor merged 18 commits intosonic-net:masterfrom
qinchuanares:400zr-initial-support
Nov 12, 2021
Merged

400zr initial support#228
prgeor merged 18 commits intosonic-net:masterfrom
qinchuanares:400zr-initial-support

Conversation

@qinchuanares
Copy link
Contributor

This is a sfp-refactor follow-up work: to add 400ZR driver and high level functions into sfp-refactor. To query the keys in state_DB and write keys in config_DB into the module.

Description
The PR aims to provide 400ZR + SONiC support.

Motivation and Context
The motivation is to add 400ZR into the SONiC ecosystem. As we recently started to massively deploy 400ZR optical modules as replacement of InPhi DWDM ColorZ modules in metro links, there is emerging need to use SONiC routers to support 400ZR modules.

How Has This Been Tested?
In .\tests folder,

  1. test_cmis.py to test functions in sonic_xcvr\api\public\cmis.py
  2. test_c_cmis.py to test functions in sonic_xcvr\api\public\c_cmis.py
  3. test_cmisCDB.py to test functions in sonic_xcvr\api\public\cmisCDB.py
  4. test_cmisVDM.py to test functions in sonic_xcvr\api\public\cmisVDM.py

Additional Information (Optional)

@lgtm-com
Copy link

lgtm-com bot commented Nov 3, 2021

This pull request introduces 2 alerts when merging 5e4f6e1 into 26c8346 - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for Clear-text logging of sensitive information

@lgtm-com
Copy link

lgtm-com bot commented Nov 4, 2021

This pull request introduces 1 alert when merging 35177c0 into 26c8346 - view on LGTM.com

new alerts:

  • 1 for Clear-text logging of sensitive information

@lgtm-com
Copy link

lgtm-com bot commented Nov 4, 2021

This pull request introduces 1 alert when merging 068a4dd into 26c8346 - view on LGTM.com

new alerts:

  • 1 for Clear-text logging of sensitive information

@lgtm-com
Copy link

lgtm-com bot commented Nov 5, 2021

This pull request introduces 1 alert when merging 854043d into 26c8346 - view on LGTM.com

new alerts:

  • 1 for Clear-text logging of sensitive information

… and B are valid. 2. exception handling when vdm does not return prefec and postfec errors
@lgtm-com
Copy link

lgtm-com bot commented Nov 5, 2021

This pull request introduces 1 alert when merging 520aefc into 26c8346 - view on LGTM.com

new alerts:

  • 1 for Clear-text logging of sensitive information

@prgeor
Copy link
Collaborator

prgeor commented Nov 7, 2021

This pull request introduces 1 alert when merging 520aefc into 26c8346 - view on LGTM.com

new alerts:

  • 1 for Clear-text logging of sensitive information

@qinchuanares could you fix LGTM warning?

@qinchuanares
Copy link
Contributor Author

This pull request introduces 1 alert when merging 520aefc into 26c8346 - view on LGTM.com
new alerts:

  • 1 for Clear-text logging of sensitive information

@qinchuanares could you fix LGTM warning?

It says in /sonic_platform_base/sonic_xcvr/api/public/cmis.py line 1024 sensitive data (password) is logged. But I am logging password_type, not really password.

Copy link
Collaborator

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@qinchuanares could you please address comments.

Copy link
Contributor

@aravindmani-1 aravindmani-1 left a comment

Choose a reason for hiding this comment

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

please check the comments

@aravindmani-1
Copy link
Contributor

aravindmani-1 commented Nov 9, 2021

get_name() implementation is missing in SFP refactored where it is being used sonic-mgmt script.
https://github.com/Azure/sonic-mgmt/blob/da09953eb7a712c09d652e03f1514574b7cf0d89/tests/platform_tests/api/test_sfp.py#L187
get_name() is supposed to retrieve transceiver name(Identifier name). Please include get_name in the refactored code and also define the same in sfp_base.py

@prgeor
Copy link
Collaborator

prgeor commented Nov 9, 2021

@qinchuanares please make sure sonic-mgt tests are passing

@qinchuanares
Copy link
Contributor Author

get_name() implementation is missing in SFP refactored where it is being used sonic-mgmt script.

get_name() implementation is missing in SFP refactored where it is being used sonic-mgmt script. https://github.com/Azure/sonic-mgmt/blob/da09953eb7a712c09d652e03f1514574b7cf0d89/tests/platform_tests/api/test_sfp.py#L187 get_name() is supposed to retrieve transceiver name(Identifier name). Please include get_name in the refactored code and also define the same in sfp_base.py

I include get_name() function in CmisApi class.

…ps\public\c_cmis.py to include mem_maps only associated with c-cmis 2. change cmd0001h() to module_enter_password(). 3. Enter password before start downloading (cmd0101h) regardless of module type and module vendor
@lgtm-com
Copy link

lgtm-com bot commented Nov 9, 2021

This pull request introduces 3 alerts when merging 1dc4652 into 26c8346 - view on LGTM.com

new alerts:

  • 2 for Unused import
  • 1 for Clear-text logging of sensitive information

@lgtm-com
Copy link

lgtm-com bot commented Nov 9, 2021

This pull request introduces 1 alert when merging 20f6b72 into 26c8346 - view on LGTM.com

new alerts:

  • 1 for Clear-text logging of sensitive information

@andywongarista
Copy link
Contributor

get_name() implementation is missing in SFP refactored where it is being used sonic-mgmt script.

get_name() implementation is missing in SFP refactored where it is being used sonic-mgmt script. https://github.com/Azure/sonic-mgmt/blob/da09953eb7a712c09d652e03f1514574b7cf0d89/tests/platform_tests/api/test_sfp.py#L187 get_name() is supposed to retrieve transceiver name(Identifier name). Please include get_name in the refactored code and also define the same in sfp_base.py

I include get_name() function in CmisApi class.

get_name() is implemented per-platform and was not meant to be implemented in the new design as part of any XcvrApi-derived classes.

…. removed get_name() function in cmis.py for now.
@lgtm-com
Copy link

lgtm-com bot commented Nov 10, 2021

This pull request introduces 1 alert when merging add1040 into 26c8346 - view on LGTM.com

new alerts:

  • 1 for Clear-text logging of sensitive information

@lgtm-com
Copy link

lgtm-com bot commented Nov 10, 2021

This pull request introduces 1 alert when merging a93d90f into 26c8346 - view on LGTM.com

new alerts:

  • 1 for Clear-text logging of sensitive information

Copy link
Collaborator

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

can you please address comments.


return self.xcvr_eeprom.write(consts.TX_DISABLE_FIELD, channel_state)

def get_power_override(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you add TODO, to remove this for CMIS as its not supported

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you need get_power_override() to be removed in this PR?

@lgtm-com
Copy link

lgtm-com bot commented Nov 11, 2021

This pull request introduces 16 alerts when merging 0b9aaf9 into ef55364 - view on LGTM.com

new alerts:

  • 9 for Unused local variable
  • 7 for Mismatch in multiple assignment

@lgtm-com
Copy link

lgtm-com bot commented Nov 11, 2021

This pull request introduces 16 alerts when merging 2de0878 into ef55364 - view on LGTM.com

new alerts:

  • 9 for Unused local variable
  • 7 for Mismatch in multiple assignment

@lgtm-com
Copy link

lgtm-com bot commented Nov 12, 2021

This pull request introduces 16 alerts when merging 9989804 into ef55364 - view on LGTM.com

new alerts:

  • 9 for Unused local variable
  • 7 for Mismatch in multiple assignment

@lgtm-com
Copy link

lgtm-com bot commented Nov 12, 2021

This pull request introduces 8 alerts when merging 073b445 into ef55364 - view on LGTM.com

new alerts:

  • 7 for Mismatch in multiple assignment
  • 1 for Unused local variable

@prgeor prgeor self-assigned this Nov 12, 2021
@prgeor prgeor merged commit c8eceec into sonic-net:master Nov 12, 2021
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-platform-common that referenced this pull request Oct 25, 2024
Description
Clear all data from DB table when the daemon stops

Motivation and Context
Clean up when the daemon stops

How Has This Been Tested?
Check Thermal and physical info data if they are cleared after stopping the thermalctld.
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.

4 participants