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

[TPID CONFIG] Added TPID configuration CLI support #1618

Merged
merged 3 commits into from
May 26, 2021
Merged

[TPID CONFIG] Added TPID configuration CLI support #1618

merged 3 commits into from
May 26, 2021

Conversation

gechiang
Copy link
Contributor

@gechiang gechiang commented May 17, 2021

Signed-off-by: Gen-Hwa Chiang gechiang@microsoft.com

What I did

This PR is part of the TPID CONFIG SONiC feature set to support port/LAG TPID configuration.
TPID HLD PR: (sonic-net/SONiC#681)

There are total of 3 PRs across different submodules to achieve this feature support.

Please refer to the TPID HLD listed above for more details on the TPID configuration feature support.
Here is a quick summary for this PR changes:

  1. By default TPID for port/LAG is 0x8100. With HW SKU that has capability to handle TPID configuration user is allowed to configure the port/LAG to non default TPID values such as (0x9100, 0x9200, 0x88A8).
  2. Not all HW SKU is capable of supporting TPID attribute configuration so any TPID CLI attempt is always first checked against the DUT TPID capability before the configuration is accepted.
  3. LAG member TPID are not allowed to be modified directly via TPID port configuration, Portchannel interface TPID can be configured and applied to all LAG members by SAI if HW is capable of handling TPID LAG configuration.
  4. Before member ca n be added to a LAG, its TPID must be at default setting (0x8100) or else the LAG member Add operation will not be permitted.

Here is some sample output of the TPID configuration attempts:

admin@SONIC:~$ sudo config interface tpid Ethernet1 0x9100
Ethernet1 is already member of PortChannel0001. Set TPID NOT allowed.
admin@SONIC:~$ sudo config interface tpid Ethernet6 0x8888
TPID 0x8888 is not allowed. Allowed: 0x8100, 9100, 9200, or 88A8.
admin@SONIC:~$ sudo config interface tpid Ethernet6 0x9200
admin@SONIC:~$ sudo config interface tpid Ethernet7 0x88a8

How to verify it

Besides validating this on a HW SKU that is capable of handling TPID attribute setting, new unit testcase was also added to ensure most of the TPID feature code is covered:

cat sonic_utilities-1.2-py3-none-any.whl.log | grep tpid
tests/tpid_test.py::TestTpid::test_tpid_config_bad_tpid PASSED           [ 79%]
tests/tpid_test.py::TestTpid::test_tpid_config_lag_mbr PASSED            [ 79%]
tests/tpid_test.py::TestTpid::test_tpid_add_lag_mbr_with_non_default_tpid PASSED [ 79%]
tests/tpid_test.py::TestTpid::test_tpid_config_port_interface PASSED     [ 79%]
tests/tpid_test.py::TestTpid::test_tpid_config_portchannel_interface PASSED [ 79%]
tests/tpid_test.py::TestTpid::test_show_tpid PASSED                      [ 79%]
tests/tpid_test.py::TestTpid::test_show_tpid_ethernet0 PASSED            [ 79%]
copying build/lib/tests/tpid_test.py -> build/bdist.linux-x86_64/wheel/tests
copying build/lib/tests/tpid_tests.py -> build/bdist.linux-x86_64/wheel/tests
adding 'tests/tpid_test.py'

New command output (if the output of a command-line utility has changed)

As part of this feature, a new show command "show interface tpid" is also added.
Here is some sample output:

admin@SONIC:~$ show interfaces tpid
      Interface            Alias    Oper    Admin    TPID
---------------  ---------------  ------  -------  ------
      Ethernet0   fortyGigE1/1/1      up       up  0x8100
      Ethernet1   fortyGigE1/1/2      up       up  0x8100
      Ethernet2   fortyGigE1/1/3    down     down  0x8100
      Ethernet3   fortyGigE1/1/4    down     down  0x8100
      Ethernet4   fortyGigE1/1/5      up       up  0x8100
      Ethernet5   fortyGigE1/1/6      up       up  0x8100
      Ethernet6   fortyGigE1/1/7      up       up  0x9200
      Ethernet7   fortyGigE1/1/8      up       up  0x88A8
      Ethernet8   fortyGigE1/1/9      up       up  0x8100
      Ethernet9  fortyGigE1/1/10      up       up  0x8100
     Ethernet10  fortyGigE1/1/11      up       up  0x8100
     ...
     Ethernet63  fortyGigE1/4/16    down     down  0x8100
PortChannel0001              N/A      up       up  0x8100
PortChannel0002              N/A      up       up  0x8100
PortChannel0003              N/A      up       up  0x8100
PortChannel0004              N/A      up       up  0x8100
admin@str-s6100-acs-1:~$ show interfaces tpid Ethernet6
  Interface           Alias    Oper    Admin    TPID
-----------  --------------  ------  -------  ------
  Ethernet6  fortyGigE1/1/7      up       up  0x9200
admin@str-s6100-acs-1:~$ show interfaces tpid Ethernet7
  Interface           Alias    Oper    Admin    TPID
-----------  --------------  ------  -------  ------
  Ethernet7  fortyGigE1/1/8      up       up  0x88A8
admin@str-s6100-acs-1:~$

Corresponding command reference doc is also updated to include the new changes for config TPID and show TPID related commands.

Copy link
Contributor

@jleveque jleveque left a comment

Choose a reason for hiding this comment

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

Formatting and alignment of code blocks in rendered Command-Reference.md appear incorrect. Formatting could be due to spaces preceding triple backticks. Please fix.

@gechiang
Copy link
Contributor Author

Formatting and alignment of code blocks in rendered Command-Reference.md appear incorrect. Formatting could be due to spaces preceding triple backticks. Please fix.

Thanks for catching it!
I have fixed it in the latest commit just now.

@jleveque jleveque dismissed their stale review May 17, 2021 21:42

Looks good from my perspective. Please wait for other reviewers.

@gechiang gechiang merged commit b616cd9 into sonic-net:master May 26, 2021
gitsabari pushed a commit to gitsabari/sonic-utilities that referenced this pull request Jun 15, 2021
* [TPID CONFIG] Added TPID configuration CLI support

Signed-off-by: Gen-Hwa Chiang <gechiang@microsoft.com>

* Added command-reference update for the new config TPID and show TPID related commands

* Fixed Reference-command spacing issue that caused the show interfaces tpid output incorrectly
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-utilities that referenced this pull request Aug 10, 2021
* [TPID CONFIG] Added TPID configuration CLI support

Signed-off-by: Gen-Hwa Chiang <gechiang@microsoft.com>

* Added command-reference update for the new config TPID and show TPID related commands

* Fixed Reference-command spacing issue that caused the show interfaces tpid output incorrectly
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.

2 participants