Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Fix port index for multi-asic #13042

Merged
merged 2 commits into from
Dec 16, 2022
Merged

Fix port index for multi-asic #13042

merged 2 commits into from
Dec 16, 2022

Conversation

ysmanman
Copy link
Contributor

@ysmanman ysmanman commented Dec 13, 2022 •
edited by rlhui
Loading

Why I did it

Port indexes of front panel ports are not contiguous in multi-asic because we didn't distinguish between front panel and internal ports, e.g., recycle ports. This could cause xcvrd crash seen in aristanetworks/sonic#56

How I did it

Fix this by assigning index to front panel port first and then internal ports.

How to verify it

Loaded the image with the fix in multi-asic LC and verified that xcvrd was not crashing in pmon.

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211

Description for the changelog

Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.

Link to config_db schema for YANG module changes

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

@ysmanman
Fix port index for multi-asic
02a265d 
Port indexes of front panel ports are not contiguous in multi-asic because we didn't distiguish between
front panel and internal ports, e.g., recycle ports. Fix this by assigning index to front panel port first
and then internal ports.
@ysmanman
Copy link
Contributor Author

Add @arlakshm for visibility.

@ysmanman
Copy link
Contributor Author

Add @wenyiz2021 who reported issue aristanetworks/sonic#56 .

@rlhui rlhui added the Chassis 🤖 Modular chassis support label Dec 14, 2022
@rlhui rlhui requested a review from arlakshm December 14, 2022 05:58
@wenyiz2021
Copy link
Contributor

wenyiz2021 commented Dec 14, 2022 •
edited
Loading

thanks @ysmanman for changing port_config.ini, can you please also add the change in mingiraph.py, as I mentioned in the issue?

(Pdb) interfaces.keys()
[u'Ethernet152', u'Ethernet200', u'Ethernet168', u'Ethernet192', u'Ethernet184', u'Ethernet232', u'Ethernet208', u'Ethernet160', u'Ethernet224', u'Ethernet176', u'Ethernet144', u'Ethernet248', u'Ethernet216', u'Ethernet240']
(Pdb) mg_ports.keys()
[u'Ethernet-Rec1', u'Ethernet152', u'Ethernet200', u'Ethernet168', u'Ethernet192', u'Ethernet184', u'Ethernet232', u'Ethernet208', u'Ethernet160', u'Ethernet-IB1', u'Ethernet176', u'Ethernet224', u'Ethernet144', u'Ethernet216']
(Pdb) len(mg_ports)
14
(Pdb) len(interfaces)
14
(Pdb) intf
u'Ethernet248'
(Pdb) intf in mg_ports
False

@wenyiz2021
Copy link
Contributor

wenyiz2021 commented Dec 14, 2022 •
edited
Loading

@ysmanman
Can you change in this sku instead: x86_64-arista_7800r3a_36dm2_lc/Arista-7800R3A-36DM2-C36, not current one

@ysmanman
Copy link
Contributor Author

@ysmanman Can you change in this sku instead: x86_64-arista_7800r3a_36dm2_lc/Arista-7800R3A-36DM2-C36, not current one

@wenyiz2021 Arista-7800R3A-36DM2-C36 is just a symbol link of 36D2-C36.

@wenyiz2021
Copy link
Contributor

thanks @ysmanman for changing port_config.ini, can you please also add the change in mingiraph.py, as I mentioned in the issue?

(Pdb) interfaces.keys()
[u'Ethernet152', u'Ethernet200', u'Ethernet168', u'Ethernet192', u'Ethernet184', u'Ethernet232', u'Ethernet208', u'Ethernet160', u'Ethernet224', u'Ethernet176', u'Ethernet144', u'Ethernet248', u'Ethernet216', u'Ethernet240']
(Pdb) mg_ports.keys()
[u'Ethernet-Rec1', u'Ethernet152', u'Ethernet200', u'Ethernet168', u'Ethernet192', u'Ethernet184', u'Ethernet232', u'Ethernet208', u'Ethernet160', u'Ethernet-IB1', u'Ethernet176', u'Ethernet224', u'Ethernet144', u'Ethernet216']
(Pdb) len(mg_ports)
14
(Pdb) len(interfaces)
14
(Pdb) intf
u'Ethernet248'
(Pdb) intf in mg_ports
False

This issue is not in minigraph.py, it was resolved after re-generating minigraph on the device. The change LGTM

@wenyiz2021
Copy link
Contributor

wenyiz2021 commented Dec 14, 2022 •
edited
Loading

@ysmanman can you please resolve the conflict and merge PR? thanks
Please also add a label request for 202205 branch, somehow I could not add it

@ysmanman
Copy link
Contributor Author

@ysmanman can you please resolve the conflict and merge PR? thanks Please also add a label request for 202205 branch, somehow I could not add it

Hi @wenyiz2021 resolved conflicts just now. I don't think I have permission to add label neither. @rlhui may have the permission.

@wenyiz2021
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 13042 in repo sonic-net/sonic-buildimage

@ysmanman
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 13042 in repo sonic-net/sonic-buildimage

@wenyiz2021
Copy link
Contributor

@ysmanman can you please merge this? thanks

@ysmanman
Copy link
Contributor Author

@ysmanman can you please merge this? thanks

@wenyiz2021 I don't think I have write access to the repo. so I am not able to merge it.

@rlhui rlhui merged commit 1fd2395 into sonic-net:master Dec 16, 2022
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Dec 16, 2022
@ysmanman @mssonicbld
Fix port index for multi-asic (sonic-net#13042)
a030c98 
Port indexes of front panel ports are not contiguous in multi-asic because we didn't distiguish between
front panel and internal ports, e.g., recycle ports. Fix this by assigning index to front panel port first
and then internal ports.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202205: #13086

@mssonicbld mssonicbld mentioned this pull request Dec 16, 2022
Merged
yxieca pushed a commit that referenced this pull request Dec 16, 2022
@mssonicbld @ysmanman
Fix port index for multi-asic (#13042) (#13086)
fd6523b 
Port indexes of front panel ports are not contiguous in multi-asic because we didn't distiguish between
front panel and internal ports, e.g., recycle ports. Fix this by assigning index to front panel port first
and then internal ports.

Co-authored-by: Song Yuan <64041228+ysmanman@users.noreply.github.com>
@wenyiz2021 wenyiz2021 self-assigned this May 4, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@arlakshm arlakshm arlakshm approved these changes

@wenyiz2021 wenyiz2021 wenyiz2021 approved these changes

Assignees

@arlakshm arlakshm

@ysmanman ysmanman

@wenyiz2021 wenyiz2021

Labels
Chassis 🤖 Modular chassis support Included in 202205 Branch Request for 202205 Branch
Projects
SONiC Chassis
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ysmanman @wenyiz2021 @mssonicbld @arlakshm @yxieca @rlhui