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

set cable length to 1m for backplane ports #13572

Merged
merged 2 commits into from
Feb 11, 2023

Conversation

zhixzhu
Copy link
Contributor

@zhixzhu zhixzhu commented Jan 31, 2023

Signed-off-by: Zhixin Zhu zhixzhu@cisco.com

Why I did it

backplane ports cable length need to be specified.

How I did it

separated handling for the specific port name.

How to verify it

Used "sonic-cfggen" to generate buffer config, and verified cable length of backplane ports is 1m, other ports default length.

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
    --> This is required for T2.
  • 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)

Signed-off-by: Zhixin Zhu <zhixzhu@cisco.com>
@zhixzhu zhixzhu requested a review from lguohan as a code owner January 31, 2023 18:17
@zhixzhu
Copy link
Contributor Author

zhixzhu commented Jan 31, 2023

@lguohan There is 1 workflow awaiting approval, could you help approve it?

@zhixzhu
Copy link
Contributor Author

zhixzhu commented Feb 1, 2023

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

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

@zhixzhu
Copy link
Contributor Author

zhixzhu commented Feb 1, 2023

@lguohan There is 1 workflow awaiting approval, could you help approve it?
The failure happened when running script test_pretest.py, changing the cable length of backplane ports should not have any impact on it, could you help rerun the test? I don't have privilege to do rerun.

@zhixzhu
Copy link
Contributor Author

zhixzhu commented Feb 3, 2023

@yxieca @lguohan There is 1 workflow awaiting approval, could you help approve it? And rerun the pipeline?

@gechiang
Copy link
Collaborator

gechiang commented Feb 3, 2023

/azp run

@azure-pipelines
Copy link

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

@zhixzhu
Copy link
Contributor Author

zhixzhu commented Feb 4, 2023

The failure is rsyslogd restart error:
"cmd": "docker exec -i teamd0 supervisorctl restart rsyslogd", "rc": 7, "stderr": "", "stderr_lines": [], "stdout": "rsyslogd: ERROR (not running)\nrsyslogd: ERROR (spawn error)\n"

Change cable length should not have any impact on it.

@gechiang Could you help run the pipeline again?

@lguohan
Copy link
Collaborator

lguohan commented Feb 6, 2023

why no unit test added for this case?

@lguohan
Copy link
Collaborator

lguohan commented Feb 6, 2023

@zhixzhu

@zhixzhu
Copy link
Contributor Author

zhixzhu commented Feb 6, 2023

why no unit test added for this case?

@lguohan To render buffer template, it requires minigraph, device/vendor/platform/hwsku/, there is no such directory and files for vendor cisco, and the change here is for cisco specific backplane interface name "Ethernet-BPxxx", so can't add UT for it.
cisco hwsku files are in seperate repo platform-cisco-8000 which is not open sourced.

I had verified the change locally, used "sonic-cfggen" to generate buffer config, and verified cable length of backplane ports is 1m, other ports default length.

@rlhui rlhui added the P0 Priority of the issue label Feb 8, 2023
files/build_templates/buffers_config.j2 Outdated Show resolved Hide resolved
Signed-off-by: Zhixin Zhu <zhixzhu@cisco.com>
@zhixzhu
Copy link
Contributor Author

zhixzhu commented Feb 9, 2023

@abdosi @lguohan I have updated the default cable length of internal links to 5m and added UT. Please help review again, thanks.

@abdosi
Copy link
Contributor

abdosi commented Feb 11, 2023

LGTM.

@abdosi
Copy link
Contributor

abdosi commented Feb 11, 2023

@rlhui please help merge this.

@rlhui rlhui merged commit f0f7639 into sonic-net:master Feb 11, 2023
@zhixzhu zhixzhu deleted the master-buffer-config branch February 11, 2023 04:24
StormLiangMS pushed a commit to StormLiangMS/sonic-buildimage that referenced this pull request Mar 28, 2023
Related work items: sonic-net#276, sonic-net#305, sonic-net#332, sonic-net#338, sonic-net#339, sonic-net#1188, sonic-net#1192, sonic-net#1197, sonic-net#1206, sonic-net#1685, sonic-net#1690, sonic-net#1696, sonic-net#1699, sonic-net#1709, sonic-net#1727, sonic-net#1737, sonic-net#1741, sonic-net#1742, sonic-net#2511, sonic-net#2512, sonic-net#2532, sonic-net#2559, sonic-net#2626, sonic-net#2638, sonic-net#2645, sonic-net#2649, sonic-net#2660, sonic-net#2669, sonic-net#2670, sonic-net#2678, sonic-net#10084, sonic-net#11442, sonic-net#11873, sonic-net#12047, sonic-net#12110, sonic-net#12207, sonic-net#12529, sonic-net#12678, sonic-net#13235, sonic-net#13287, sonic-net#13372, sonic-net#13395, sonic-net#13456, sonic-net#13497, sonic-net#13522, sonic-net#13545, sonic-net#13547, sonic-net#13552, sonic-net#13569, sonic-net#13572, sonic-net#13578, sonic-net#13591, sonic-net#13611, sonic-net#13647, sonic-net#13649, sonic-net#13660, sonic-net#13710, sonic-net#13716, sonic-net#13724, sonic-net#13726, sonic-net#13732, sonic-net#13735, sonic-net#13739, sonic-net#13757, sonic-net#13786, sonic-net#13792, sonic-net#13800, sonic-net#13801, sonic-net#13802, sonic-net#13805, sonic-net#13806, sonic-net#13812, sonic-net#13814, sonic-net#13822, sonic-net#13831, sonic-net#13834, sonic-net#13847, sonic-net#13870, sonic-net#13882, sonic-net#13884, sonic-net#13885, sonic-net#13894, sonic-net#13895, sonic-net#13926, sonic-net#13932, sonic-net#13935, sonic-net#13942, sonic-net#13951, sonic-net#13953, sonic-net#13964
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 Priority of the issue
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants