-
Notifications
You must be signed in to change notification settings - Fork 666
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
sonic-utils: initial support for link-training #2071
Conversation
This pull request introduces 3 alerts when merging 65e43cd into 9e50124 - view on LGTM.com new alerts:
|
This pull request introduces 2 alerts when merging 188c561 into f71ef64 - view on LGTM.com new alerts:
|
config/main.py
Outdated
@click.argument('mode', metavar='<mode>', required=True, type=click.Choice(["on", "off"])) | ||
@click.option('-v', '--verbose', is_flag=True, help="Enable verbose output") | ||
def link_training(ctx, interface_name, mode, verbose): | ||
"""Set interface auto negotiation mode""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo. Should be link training instead of auto negotiation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
The changes look good to me. Can you please resolve the conflicts? I will approve once the conflicts are resolved |
scripts/portconfig
Outdated
if self.verbose: | ||
print("Setting link-training %s on port %s" % (mode, port)) | ||
if mode not in ['on', 'off']: | ||
mode = 'off' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe print an error and exit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted, and it's done
@zhangyanzhao @yxieca pls help signoff and merge |
8091c9e
- What I did Add CLI support for link-trainig - How I did it portconfig: initial support for link-training config config/main.py: initial support for link-training config intfutil: initial support for link-training show command show/interfaces/init.py: initial support for link-training show command - How to verify it Manual test Ran the Unit-tests to the corresponding changes Signed-off-by: Dante Su <dante.su@broadcom.com>
Signed-off-by: Dante Su <dante.su@broadcom.com>
Signed-off-by: Dante Su <dante.su@broadcom.com>
Signed-off-by: Dante Su <dante.su@broadcom.com>
Signed-off-by: Dante Su <dante.su@broadcom.com>
@@ -639,3 +639,36 @@ def autoneg_status(interfacename, namespace, display, verbose): | |||
cmd += " -n {}".format(namespace) | |||
|
|||
clicommon.run_command(cmd, display_cmd=verbose) | |||
|
|||
# | |||
# link-training group (show interfaces link-training ...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea, this is a legacy command, and my best guess is that it supports both signle interface and multi-interface at the same time, hence the original author adopted 'show interfaces' instead of 'show interface'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you fix the PR description? I see many wrong subcommand.
sudo config interface link-training Ethernet0 on
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm not following you, is this about 'interfaces' v.s. 'interface' in show and config command?
If that's the case, this is a legacy command format since day 1, while the link-training is actually a plug-in to these legacy CLICK based CLI commands, hence it's not expected to change the upper level command syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. I see the difference between show
and config
.
Update sonic-utilities submodule pointer to include the following: * [GCU] Handling type1 lists ([sonic-net#2171](sonic-net/sonic-utilities#2171)) * [yang] extend ConfigMgmt constructor to pass YANG options ([sonic-net#2118](sonic-net/sonic-utilities#2118)) * [dump] implement ACL modules ([sonic-net#2153](sonic-net/sonic-utilities#2153)) * show commands for SYSTEM READY ([sonic-net#1851](sonic-net/sonic-utilities#1851)) * [GCU] Handling non-compliant leaf-list with string values ([sonic-net#2174](sonic-net/sonic-utilities#2174)) * Add sonic-delayed.target to Application Extension .timer file generator ([sonic-net#2176](sonic-net/sonic-utilities#2176)) * [portconfig] Allow to configure interface mtu for physical ports ([#l](https://github.com/Azure/sonic-utilities/pull/l)) * Broadcast Unknown-multicast and Unknown-unicast Storm-control ([sonic-net#928](sonic-net/sonic-utilities#928)) * sonic-utils: initial support for link-training ([sonic-net#2071](sonic-net/sonic-utilities#2071)) * [portchannel] Added ACL/PBH binding checks to the port before getting added to portchannel ([sonic-net#2151](sonic-net/sonic-utilities#2151)) * Modify override testcase to cover PORT admin_status ([sonic-net#2165](sonic-net/sonic-utilities#2165)) * [GCU] Validate peer_group_range ip_range are correct ([sonic-net#2145](sonic-net/sonic-utilities#2145)) * [auto-ts] add memory check ([sonic-net#2116](sonic-net/sonic-utilities#2116)) * support new interface types CR8/SR8/KR8/LR8 which are brougnt by SAI V.1.10.2 ([sonic-net#2167](sonic-net/sonic-utilities#2167)) * [scripts/fast-reboot] Add option to include ssd-upgrader-part boot option with SONiC partition ([sonic-net#2150](sonic-net/sonic-utilities#2150)) * [config reload] Fix invalid rstrip. ([sonic-net#2157](sonic-net/sonic-utilities#2157)) * Accept 0 for queue and dscp ([sonic-net#2162](sonic-net/sonic-utilities#2162)) Signed-off-by: dprital <drorp@nvidia.com>
@ds952811 can you help update CLI doc as well? |
HLD: sonic-net/SONiC#925 #### What I did Add CLI support for link-trainig #### How I did it 1. portconfig: initial support for link-training config 2. config/main.py: initial support for link-training config 3. intfutil: initial support for link-training show command 4. show/interfaces/__init__.py: initial support for link-training show command #### How to verify it 1. Manual test 2. Ran the Unit-tests to the corresponding changes #### Previous command output (if the output of a command-line utility has changed) #### New command output (if the output of a command-line utility has changed) ``` admin@sonic:~$ sudo config interface link-training Ethernet0 on admin@sonic:~$ sudo config interface link-training Ethernet8 on admin@sonic:~$ sudo config interface link-training Ethernet16 off admin@sonic:~$ sudo config interface link-training Ethernet24 on admin@sonic:~$ sudo config interface link-training Ethernet32 off admin@sonic:~$ show interfaces link-training status Interface LT Oper LT Admin Oper Admin ----------- ----------- ---------- ------ ------- Ethernet0 trained on up up Ethernet8 trained on up up Ethernet16 off off down up Ethernet24 not trained on down up Ethernet32 off off down up Ethernet40 off - down up Ethernet48 off - down up Ethernet56 off - down up Ethernet64 off - down up Ethernet72 off - down up Ethernet80 off - down up Ethernet88 off - down up Ethernet96 off - down up Ethernet104 off - down up Ethernet112 off - down up Ethernet120 off - down up Ethernet128 off - down up Ethernet136 off - down up Ethernet144 off - down up Ethernet152 off - down up Ethernet160 off - down up Ethernet168 off - down up Ethernet176 off - down up Ethernet184 off - down up Ethernet192 off - down up Ethernet200 off - down up Ethernet208 off - down up Ethernet216 off - down up Ethernet224 off - down up Ethernet232 off - down up Ethernet240 off - down up Ethernet248 off - down up admin@sonic:~$ ```
Update sonic-utilities submodule pointer to include the following: * Fix test failure in dump table test in 202205 ([sonic-net#2307](sonic-net/sonic-utilities#2307)) * Convert IPv6 addresses to lowercase in apply-patch ([sonic-net#2299](sonic-net/sonic-utilities#2299)) * [config][muxcable] add support to enable/disable ycable telemetry ([sonic-net#2297](sonic-net/sonic-utilities#2297)) * Fix GCU bug when backend service modifying config ([sonic-net#2295](sonic-net/sonic-utilities#2295)) * [intfutil] Check whether the FEC mode is supported on the platform before configuring it to CONFIG_DB ([sonic-net#2223](sonic-net/sonic-utilities#2223)) * Improve the way to check port type of RJ45 port ([sonic-net#2249](sonic-net/sonic-utilities#2249)) * sonic-utils: initial support for link-training ([sonic-net#2071](sonic-net/sonic-utilities#2071)) * Support to enable fips for the command sonic_installer (sonic-net#2154) ([sonic-net#2303](sonic-net/sonic-utilities#2303)) Signed-off-by: dprital <drorp@nvidia.com>
HLD: sonic-net/SONiC#925
What I did
Add CLI support for link-trainig
How I did it
How to verify it
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)