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

Switch Port Modes and VLAN CLI Enhancement #2419

Merged
merged 72 commits into from
Aug 24, 2023
Merged

Switch Port Modes and VLAN CLI Enhancement #2419

merged 72 commits into from
Aug 24, 2023

Conversation

MuhammadUmarAsad
Copy link
Contributor

@MuhammadUmarAsad MuhammadUmarAsad commented Oct 1, 2022

What I did

  • Modified "/config/vlan.py" to add support for multiple vids, range of vids in vlan add|del and vlan member add|del

  • Creation of "/config/switchport.py" which will deal with port modes for interfaces i.e. access, trunk and routed

  • Created the show interface switchport status and show interface switchport config commands so that it shows the status of port either being access, trunk or routed.

  • Modified Command-Reference.md to add new commands in it with usage and examples

  • Added new utility functions in "/utilities_common/cli.py" to use in vlan.py and switchport.py commands

  • Modified test cases and added new test cases for the new commands

How I did it

  • Added new commands and modified the existing commands in "/config/vlan.py" to

    • accept multiple vids
    • accept range of vids
    • except_flag and all option in vlan member command
  • Creation of "/config/switchport.py" file to add commands which will deal with port modes i.e. access, trunk or routed which we were unable to define or assign in SONiC before

  • New utility functions in "/utilities_common/cli.py"

  • Using show interface switchport status and show interface switchport config commands, it will display if an interface is in access or trunk or routed mode.

How to verify it

New commands have been added in Command-Reference.md All the syntax and examples have been added there and they can be verified by running the specific command

Signed-off-by: Muhammad Umar Asad umarasad20@gmail.com [ xFlow Research Inc. ]

- What I did

Modified config/vlan.py to add new commands with multiple and range
options

Creation of config/switchport.py which will deal with default Vlan1

Modified the show interface command so it will show the status of port
being access or trunk. Before it did not show about access mode only
trunk.

Modified the Command-Reference.md file to add new commands in it with
usage and examples

Added new utility functions in utilities_common/cli.py for new Vlan.py
and switchport.py commands

- How I did

Added new commands and modified the existing commands in config/vlan.py
for example to accept multiple vids and range of vids in add|del vlan
and add|del vlan member as well as except flag in vlan member command

Creation of config/switchport.py file to add commands which will deal
only with Default Vlan1 which was unable to be defined or assigned in
SONiC before

New utility functions in utilities_common/cli.py

changes in scripts/intfutil. Modified the existing function and now it
will display if an interface is in access mode or trunk mode with routed
mode.

- How to verify it

New commands have been added in Command-Reference.md
All the syntax and examples have been added there and they can be
verified by running the specific command

Signed-off-by: Muhammad Umar Asad <umarasad20@gmail.com> < xFlow
Research Inc. >
@MuhammadUmarAsad MuhammadUmarAsad changed the title Modification of Vlan.py and creation of switchport.py L2 IEEE 802.1Q Tunneling Support in SONiC Oct 3, 2022
@lgtm-com
Copy link

lgtm-com bot commented Oct 14, 2022

This pull request introduces 2 alerts when merging 6b3ac13 into abd5eba - view on LGTM.com

new alerts:

  • 1 for Except block handles 'BaseException'
  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 14, 2022

This pull request introduces 2 alerts when merging 45fe3b8 into abd5eba - view on LGTM.com

new alerts:

  • 1 for Except block handles 'BaseException'
  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 14, 2022

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

new alerts:

  • 1 for Except block handles 'BaseException'
  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 20, 2022

This pull request introduces 3 alerts when merging 1d9daf1 into aedc05e - view on LGTM.com

new alerts:

  • 3 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 20, 2022

This pull request introduces 2 alerts when merging fb2921d into aedc05e - view on LGTM.com

new alerts:

  • 2 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request introduces 4 alerts when merging 15587ab into aedc05e - view on LGTM.com

new alerts:

  • 2 for Unused local variable
  • 2 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request introduces 4 alerts when merging 4e660a2 into aedc05e - view on LGTM.com

new alerts:

  • 2 for Unused local variable
  • 2 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request introduces 5 alerts when merging a507786 into aedc05e - view on LGTM.com

new alerts:

  • 2 for Unused local variable
  • 2 for Unused import
  • 1 for Variable defined multiple times

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request introduces 5 alerts when merging 5cf9b60 into aedc05e - view on LGTM.com

new alerts:

  • 2 for Unused local variable
  • 2 for Unused import
  • 1 for Variable defined multiple times

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request introduces 5 alerts when merging 63086ab into aedc05e - view on LGTM.com

new alerts:

  • 2 for Unused local variable
  • 2 for Unused import
  • 1 for Variable defined multiple times

Modified './config/vlan.py' and added switchport mode checks in it as well as other small checks

Modified './scripts/intfutil/' to show correct port mode with access option for 'show interfaces status' command and its subfunctions

Added utilities functions in './utilities_common/cli.py' for './config/switchport.py'

Updated mocktables according to the tests and addded 'Mode' attribute in PORT table as well as 'Vlan' to 'Mode' in intfutil and multi_asic tests expected outputs of vlan_test.py
@MuhammadUmarAsad MuhammadUmarAsad changed the title L2 IEEE 802.1Q Tunneling Support in SONiC Switch Port Modes and VLAN CLI Enhancement Oct 26, 2022
@ridahanif96
Copy link
Contributor

We have modified our code along with HLD. Suggestions have been incorporated. @venkatmahalingam pls review.

Fixed config_db_vlan_port_keys_get() in intfutil to show correct port mode

updated test cases in '/tests/vlan_test.py' for enhanced vlan commands
and new switchport commands which are used with vlan member commands

Signed-off-by: Muhammad Umar Asad <umarasad20@gmail.com> xFlow Research
Inc.
@zhangyanzhao
Copy link
Collaborator

@venkatmahalingam can you please help to review this PR? Thanks.

@ridahanif96
Copy link
Contributor

@qiluo-msft can you please help merge this PR , thanks.

@ridahanif96
Copy link
Contributor

@qiluo-msft can you please help merge this PR? thanks

Copy link
Contributor

@theasianpianist theasianpianist left a comment

Choose a reason for hiding this comment

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

VLAN related changes lgtm

@ridahanif96
Copy link
Contributor

@gechiang @venkatmahalingam can you please guide in this PR. Its reviewed and approved but still awaiting for merge, please help, thanks.
@vaibhavhd & @prsunny can you please help with this PR, thanks.

@gechiang
Copy link
Contributor

@gechiang Do you have more concern?

I don't have any more concerns. Please help review and approve/merge this PR.
Thanks!

@qiluo-msft qiluo-msft merged commit a0b1cdc into sonic-net:master Aug 24, 2023
4 checks passed
@liushilongbuaa
Copy link
Contributor

@gechiang , @theasianpianist , @MuhammadUmarAsad ,
Can you please check kvmtest failure? This change is blocked by PR checker.
sonic-net/sonic-buildimage#16267

@ridahanif96
Copy link
Contributor

@gechiang , @theasianpianist , @MuhammadUmarAsad , Can you please check kvmtest failure? This change is blocked by PR checker. sonic-net/sonic-buildimage#16267

@liushilongbuaa I, have checked kvmtest failure. As of my understanding this PR has not made any change that blocked/cause failure in sonic-net/sonic-buildimage#16267 Updating sonic-utilites submodule.

@liushilongbuaa
Copy link
Contributor

liushilongbuaa commented Sep 1, 2023

@ridahanif96 , it failed on vlan related command:
{"changed": false, "cmds": ["config vlan add 108", "config interface ip add Vlan108 192.168.8.1/24", "config vlan add 109", "config interface ip add Vlan109 192.168.9.1/24", "config vlan member add 108 Ethernet4", "config vlan member add 109 Ethernet4"], "delta": "0:00:02.836740", "end": "2023-08-31 16:11:34.191502", "failed": true, "msg": "At least running one of the commands failed", "results": [
{"cmd": "config vlan add 108", "cmd_with_timeout": "", "err_msg": "", "rc": 0, "stderr": "", "stderr_lines": [], "stdout": "", "stdout_lines": [], "timeout": 0},
{"cmd": "config interface ip add Vlan108 192.168.8.1/24", "cmd_with_timeout": "", "err_msg": "", "rc": 0, "stderr": "", "stderr_lines": [], "stdout": "", "stdout_lines": [], "timeout": 0},
{"cmd": "config vlan add 109", "cmd_with_timeout": "", "err_msg": "", "rc": 0, "stderr": "", "stderr_lines": [], "stdout": "", "stdout_lines": [], "timeout": 0},
{"cmd": "config interface ip add Vlan109 192.168.9.1/24", "cmd_with_timeout": "", "err_msg": "", "rc": 0, "stderr": "", "stderr_lines": [], "stdout": "", "stdout_lines": [], "timeout": 0},
{"cmd": "config vlan member add 108 Ethernet4", "cmd_with_timeout": "", "err_msg": "", "rc": 2, "stderr": "Usage: config vlan member add [OPTIONS] port\nTry "config vlan member add -h" for help.\n\nError: Ethernet4 is in routed mode!\nUse switchport mode command to change port mode\n", "stderr_lines": ["Usage: config vlan member add [OPTIONS] port", "Try "config vlan member add -h" for help.", "", "Error: Ethernet4 is in routed mode!", "Use switchport mode command to change port mode"], "stdout": "", "stdout_lines": [], "timeout": 0},
{"cmd": "config vlan member add 109 Ethernet4", "cmd_with_timeout": "", "err_msg": "", "rc": 2, "stderr": "Usage: config vlan member add [OPTIONS] port\nTry "config vlan member add -h" for help.\n\nError: Ethernet4 is in routed mode!\nUse switchport mode command to change port mode\n", "stderr_lines": ["Usage: config vlan member add [OPTIONS] port", "Try "config vlan member add -h" for help.", "", "Error: Ethernet4 is in routed mode!", "Use switchport mode command to change port mode"], "stdout": "", "stdout_lines": [], "timeout": 0}
], "start": "2023-08-31 16:11:31.354762"}

@ridahanif96
Copy link
Contributor

All vlan related failure issues resolved.

elif not is_port:
port_data = config_db.get_entry('PORTCHANNEL',port)

if "mode" not in port_data:
Copy link
Contributor

Choose a reason for hiding this comment

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

port_data

PORT/mode is not yang modelled. So this PR will become breaking change, and will block this submodule move. I will raise a PR to revert this PR.

Copy link
Contributor

@ridahanif96 ridahanif96 Nov 30, 2023

Choose a reason for hiding this comment

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

@qiluo-msft ,
@venkatmahalingam @gechiang We have fixed YANG Model for this PR. The YANG PR is failing as it needs supported utilities sourcecode. One possible solution is that we merge #2419 utilities code and then we merge this PR. All dependencies will be solved.
Other solution is that we remove MUST condition from this PR for instance in sonic-vlan.yang for Vlan membership for only on access and trunk port and merge this PR and merge utilities source code. After that i will add a new PR with must condition check on sonic-vlan membership.It is requested to please help in this matter. Thanks!

qiluo-msft added a commit to qiluo-msft/sonic-utilities that referenced this pull request Oct 6, 2023
qiluo-msft added a commit that referenced this pull request Oct 7, 2023
#### What I did
Revert "Switch Port Modes and VLAN CLI Enhancement (#2419)".
The reason is PORT/mode is not yang modelled.
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.

8 participants