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.

Sign up for GitHub

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

Jump to bottom

[multiasic][sfputil]Fix the sftpuitlhelper on mutilasic platform issue #286

Merged
merged 1 commit into from
Jun 9, 2022
Merged

[multiasic][sfputil]Fix the sftpuitlhelper on mutilasic platform issue #286

merged 1 commit into from
Jun 9, 2022

Conversation

mlok-nokia
Copy link
Contributor

@mlok-nokia mlok-nokia commented May 27, 2022 •
edited
Loading

Description

read_porttab_mappings() is not able to get the multiasic port config. Modified function read_porttab_mapings() to pass the asic_name to allow the get_port_config() to get the port info for each asic. Also use the extend() and update() method to update the logical, logical_to_physical, phyiscal_to_logical lists. Also add the unit test case test_read_all_porttab_mappings() .

Motivation and Context

This change is required to handle the mutliasic platform. Fixes sonic-net/sonic-buildimage#10569

How Has This Been Tested?

The following output should be displayed without any error when using CLI command: sudo sfpulti show presence

admin@sonic:~$ sudo sfputil show presence
Port        Presence
----------  -----------
Ethernet0   Not present
Ethernet1   Not present
Ethernet2   Present
Ethernet3   Present
Ethernet4   Not present
Ethernet5   Not present
Ethernet6   Present
Ethernet7   Not 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  Not present
Ethernet21  Not present
Ethernet22  Not present
Ethernet23  Not present
Ethernet24  Present
Ethernet25  Present
Ethernet26  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
admin@sonic:~$

Additional Information (Optional)

@mlok-nokia
Copy link
Contributor Author

@rlhui @judyjoseph @prgeor Please review this PR for sfputilhelper on multiasic platform issue. Thanks!

@mlok-nokia mlok-nokia force-pushed the sfputil_helper branch 3 times, most recently from 5b6d71d to 8698c75 Compare May 31, 2022 22:11
@mlok-nokia
Copy link
Contributor Author

@rlhui @judyjoseph @prgeor Unit test case has been also added. Please review this PR.

@rlhui rlhui added the Chassis label Jun 8, 2022
judyjoseph
judyjoseph previously approved these changes Jun 8, 2022
View reviewed changes
@judyjoseph judyjoseph self-requested a review June 8, 2022 20:33
@prgeor
Copy link
Collaborator

prgeor commented Jun 9, 2022

@dgsudharsan can you review?

@prgeor
Copy link
Collaborator

prgeor commented Jun 9, 2022

@mlok-nokia how are the changes in bcmshell.py related to the sfputil issue?

prgeor
prgeor reviewed Jun 9, 2022
View reviewed changes
sonic_platform_base/sonic_sfp/sfputilhelper.py
self.logical.extend(logical)
self.logical = list(set(self.logical))
self.logical_to_physical.update(logical_to_physical)
self.physical_to_logical.update(physical_to_logical)

return None


def read_all_porttab_mappings(self, platform_dir, num_asic_inst):
Copy link
Collaborator

Choose a reason for hiding this comment

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

@mlok-nokia can you test this in non-multi-asic platform?

Copy link
Contributor Author

@mlok-nokia mlok-nokia Jun 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

We don't have any non-multi-asic platform to test this change. But there is an existing UT test_read_port_mappings() for the non-multi-asic case. Not sure if it is enough

@prgeor prgeor self-assigned this Jun 9, 2022
@mlok-nokia
Copy link
Contributor Author

@mlok-nokia how are the changes in bcmshell.py related to the sfputil issue?

This change has nothing to do with the sfputil. During the build/UT and PEP8/flake8 verification, it complains the syntax as warning. I just fix them in the same commit.

@judyjoseph judyjoseph merged commit be04b80 into sonic-net:master Jun 9, 2022
yxieca pushed a commit that referenced this pull request Jul 5, 2022
@mlok-nokia @yxieca
[multiasic][sfputil]Fix the sftpuitlhelper on mutilasic platform issue (
4fe38c7 
#286)

Signed-off-by: mlok <marty.lok@nokia.com>
itamar-talmon pushed a commit to itamar-talmon/sonic-platform-common that referenced this pull request Jan 12, 2023
@mlok-nokia
[multiasic][sfputil]Fix the sftpuitlhelper on mutilasic platform issue (
8437a60 
sonic-net#286)

Signed-off-by: mlok <marty.lok@nokia.com>
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-platform-common that referenced this pull request Oct 25, 2024
@anamehra
Use get() to fetch default value from dictionary for port admin_status …
8ff5f37 
…sonic-net#286

Fixes sonic-net/sonic-buildimage#11707
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prgeor prgeor prgeor left review comments

@rlhui rlhui rlhui left review comments

@judyjoseph judyjoseph judyjoseph approved these changes

Assignees

@prgeor prgeor

Labels
Chassis Included in 202205 Branch Request for 202205 Branch
Projects
None yet
Milestone
No milestone
5 participants
@mlok-nokia @prgeor @rlhui @judyjoseph @yxieca