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

[202012][sonic-utilities] CLI support for port auto negotiation #1817

Merged
merged 9 commits into from
Sep 15, 2021

Conversation

vdahiya12
Copy link
Contributor

@vdahiya12 vdahiya12 commented Sep 13, 2021

  1. Add CLI support for port auto negotiation feature.

  2. Add db_migrator change for auto negotiation feature

  3. Add unit test cases for all changes

  4. Add new subcommands to "config interface" command group to allow user configuring port auto negotiation

  5. Add new subcommands to "show interfaces" command group to allow user show auto negotiation status

  6. In db_migrator.py, change auto negotiation related DB field to latest one

Adding this PR because the cherry-pick from original PR #1568
has conflicts

What I did

How I did it

How to verify it

Ran the changes on Arista7050cx3 testbed

admin@sonic:~$ sudo config interface autoneg Ethernet96 enabled
admin@sonic:~$ echo $?
0
  Interface    Auto-Neg Mode    Speed    Oper    Admin
-----------  ---------------  -------  ------  -------
  Ethernet0              N/A     100G    down     down
  Ethernet4              N/A     100G    down       up
  Ethernet8              N/A     100G    down       up
 Ethernet12              N/A     100G    down       up
 Ethernet16              N/A     100G    down       up
 Ethernet20              N/A     100G    down     down
 Ethernet24         disabled     100G    down       up
 Ethernet28              N/A     100G    down       up
 Ethernet32              N/A     100G    down       up
 Ethernet36              N/A     100G    down       up
 Ethernet40              N/A     100G    down     down
 Ethernet44              N/A     100G    down       up
 Ethernet48              N/A     100G    down       up
 Ethernet52              N/A     100G    down       up
 Ethernet56              N/A     100G    down       up
 Ethernet60              N/A     100G    down       up
 Ethernet64              N/A     100G    down     down
 Ethernet68              N/A     100G    down     down
 Ethernet72              N/A     100G    down       up
 Ethernet76              N/A     100G    down       up
 Ethernet80              N/A     100G    down       up
 Ethernet84              N/A     100G    down     down
 Ethernet88              N/A     100G    down       up
 Ethernet92              N/A     100G    down       up
 Ethernet96          enabled     100G      up       up
Ethernet100              N/A     100G    down       up
Ethernet104              N/A     100G    down       up
Ethernet108              N/A     100G    down       up
Ethernet112              N/A     100G    down     down
Ethernet116              N/A     100G    down       up
Ethernet120              N/A     100G    down       up
Ethernet124              N/A     100G    down     down
Ethernet128              N/A      10G    down     down
Ethernet132              N/A      10G    down     down
admin@sonic:~$ sudo config interface autoneg Ethernet96 disabled
admin@sonic:~$ echo $?
0
admin@sonic:~$ show interfaces autoneg status
  Interface    Auto-Neg Mode    Speed    Oper    Admin
-----------  ---------------  -------  ------  -------
  Ethernet0              N/A     100G    down     down
  Ethernet4              N/A     100G    down       up
  Ethernet8              N/A     100G    down       up
 Ethernet12              N/A     100G    down       up
 Ethernet16              N/A     100G    down       up
 Ethernet20              N/A     100G    down     down
 Ethernet24          enabled     100G    down       up
 Ethernet28              N/A     100G    down       up
 Ethernet32              N/A     100G    down       up
 Ethernet36              N/A     100G    down       up
 Ethernet40              N/A     100G    down     down
 Ethernet44              N/A     100G    down       up
 Ethernet48              N/A     100G    down       up
 Ethernet52              N/A     100G    down       up
 Ethernet56              N/A     100G    down       up
 Ethernet60              N/A     100G    down       up
 Ethernet64              N/A     100G    down     down
 Ethernet68              N/A     100G    down     down
 Ethernet72              N/A     100G    down       up
 Ethernet76              N/A     100G    down       up
 Ethernet80              N/A     100G    down       up
 Ethernet84              N/A     100G    down     down
 Ethernet88              N/A     100G    down       up
 Ethernet92              N/A     100G    down       up
 Ethernet96         disabled     100G    down       up
Ethernet100              N/A     100G    down       up
Ethernet104              N/A     100G    down       up
Ethernet108              N/A     100G    down       up
Ethernet112              N/A     100G    down     down
Ethernet116              N/A     100G    down       up
Ethernet120              N/A     100G    down       up
Ethernet124              N/A     100G    down     down
Ethernet128              N/A      10G    down     down
Ethernet132              N/A      10G    down     down

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)

1. Add CLI support for port auto negotiation feature.
2. Add db_migrator change for auto negotiation feature
2. Add unit test cases for all changes

1. Add new subcommands to "config interface" command group to allow user configuring port auto negotiation
2. Add new subcommands to "show interfaces" command group to allow user show auto negotiation status
3. In db_migrator.py, change auto negotiation related DB field to latest one
@lgtm-com
Copy link

lgtm-com bot commented Sep 13, 2021

This pull request introduces 2 alerts when merging 863f1c5 into d03ba4f - view on LGTM.com

new alerts:

  • 2 for Unused import

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@lgtm-com
Copy link

lgtm-com bot commented Sep 14, 2021

This pull request introduces 2 alerts when merging 6297d51 into d03ba4f - view on LGTM.com

new alerts:

  • 2 for Unused import

@liat-grozovik
Copy link
Collaborator

@prsunny @lguohan please don't merge yet.
As the feature is based on Nvidia contribution on 202106 we need to be sure the relevant SAI supports it before merging the capabilities and run all the available tests before that.
Also, can you please confirm the full feature is backported and not just the CLI? Can you please share list of backported PRs which required for this feature?

@vdahiya12 vdahiya12 marked this pull request as draft September 14, 2021 14:12
config/main.py Outdated Show resolved Hide resolved
config/main.py Outdated Show resolved Hide resolved
config/main.py Outdated Show resolved Hide resolved
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@lgtm-com
Copy link

lgtm-com bot commented Sep 14, 2021

This pull request introduces 2 alerts when merging d425dd4 into d03ba4f - view on LGTM.com

new alerts:

  • 2 for Unused import

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@lgtm-com
Copy link

lgtm-com bot commented Sep 14, 2021

This pull request introduces 2 alerts when merging 7cc0b39 into d03ba4f - view on LGTM.com

new alerts:

  • 2 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Sep 14, 2021

This pull request introduces 2 alerts when merging 6127d34 into d03ba4f - view on LGTM.com

new alerts:

  • 2 for Unused import

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@vdahiya12 vdahiya12 marked this pull request as ready for review September 14, 2021 23:14
@lgtm-com
Copy link

lgtm-com bot commented Sep 14, 2021

This pull request introduces 2 alerts when merging 7ef0c31 into d03ba4f - view on LGTM.com

new alerts:

  • 2 for Unused import

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@lgtm-com
Copy link

lgtm-com bot commented Sep 14, 2021

This pull request introduces 2 alerts when merging 65e2bb8 into d03ba4f - view on LGTM.com

new alerts:

  • 2 for Unused import

@vdahiya12 vdahiya12 changed the title [sonic-utilities] CLI support for port auto negotiation [202012][sonic-utilities] CLI support for port auto negotiation Sep 14, 2021
@lguohan
Copy link
Contributor

lguohan commented Sep 15, 2021

are we able to resolve the lgtm alert?

@vdahiya12
Copy link
Contributor Author

are we able to resolve the lgtm alert?

This LGTM, is for mock purposes and needs to be fix.
Because this is in portconfig, not in a test file. We need to eliminate this if os.environ["UTILITIES_UNIT_TESTING"] == "1": check from the source files, and move all of this logic into the test files.
I will work on it separately. For now it should be OK.

@lguohan lguohan merged commit 2a8957d into sonic-net:202012 Sep 15, 2021
Copy link
Contributor

@prsunny prsunny left a comment

Choose a reason for hiding this comment

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

Sorry that I couldn't post the comment earlier.

@@ -371,6 +371,16 @@ def prepare_dynamic_buffer_for_warm_reboot(self, buffer_pools=None, buffer_profi

return True

def migrate_config_db_port_table_for_auto_neg(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need this changes in db_migrator

@@ -196,7 +197,7 @@ def test_mellanox_buffer_migrator_negative_nondefault_for_warm_reboot(self):
self.mellanox_buffer_migrator_warm_reboot_runner(input_config_db, input_appl_db, expected_config_db, expected_appl_db, False)


class TestInitConfigMigrator(object):
class TestAutoNegMigrator(object):
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this test and changes

vdahiya12 added a commit that referenced this pull request Sep 16, 2021
…1823)

* [sonic-utilities]remove db_migrator logic for autoneg

What I did
This PR removes all the logic for db_migrator which went into #1817 , This is required because this will cause changing the logic to "0/1" to "on/off" in swss and cause a problem in upgrade.

How I did it
How to verify it
autoneg enable/disable works with the implementation. Removing the db_migration logic so no check required

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@vdahiya12 vdahiya12 deleted the auto_change branch October 20, 2021 21:09
stepanblyschak pushed a commit to stepanblyschak/sonic-utilities that referenced this pull request Apr 18, 2022
* 2a8957d 2021-09-14 | [202012][sonic-utilities] CLI support for port auto negotiation (sonic-net#1817) (HEAD, origin/202012) [vdahiya12]

Signed-off-by: Guohan Lu <lguohan@gmail.com>
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.

5 participants