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.

Sign up for GitHub

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

Jump to bottom

[ERSPAN] Hexadecimal values are not accepted for GRE parameter #10096

Closed
raphaelt-nvidia opened this issue Feb 27, 2022 · 1 comment · Fixed by sonic-net/sonic-utilities#2095
Closed

[ERSPAN] Hexadecimal values are not accepted for GRE parameter #10096

raphaelt-nvidia opened this issue Feb 27, 2022 · 1 comment · Fixed by sonic-net/sonic-utilities#2095
Assignees
@bingwang-ms
Labels
MSFT Triaged this issue has been triaged

Comments

@raphaelt-nvidia
Copy link
Contributor

Description

In June 2021, I opened #7990 [mirroring] Missing type validations in config mirror-session command. This was addressed by sonic-net/sonic-utilities#1825 Validate input of config mirror_session add. In particular, that PR introduced

GRE_TYPE_RANGE = click.IntRange(min=0, max=65535)

The result of this change is that whereas previously, any value was accepted for the GRE parameter, including garbage, now only a DECIMAL integer between 0 and 65535 is accepted.

The command reference https://github.com/Azure/sonic-utilities/blob/master/doc/Command-Reference.md#mirroring-config-commands doesn't define it explicitly, but the following text suggests that hex format should at least be allowed, if not preferred:

optional - GRE Type in case if user wants to send the packet via GRE tunnel. GRE type could be anything; it could also be left as empty; by default, it is 0x8949 for Mellanox; and 0x88be for the rest of the chips.

@bingwang-ms What do you think the correct validation should be? Accept decimal, hexadecimal, both?

Steps to reproduce the issue:

sudo config mirror_session erspan add ERSPAN 100.0.0.1 100.0.0.2 0 11 0x8949

Describe the results you received:

Usage: config mirror_session erspan add [OPTIONS] <session_name> <src_ip>
<dst_ip> [gre_type]
[queue] [src_port] [direction]
Try "config mirror_session erspan add -h" for help.

Error: Invalid value for "[gre_type]": 0x8949 is not a valid integer

Describe the results you expected:

No error, command accepted.

Output of show version:

SONiC Software Version: SONiC.master.289-44028723e_Internal
Distribution: Debian 11.2
Kernel: 5.10.0-8-2-amd64
Build commit: 4402872
Build date: Thu Feb 24 09:14:41 UTC 2022
Built by: sw-r2d2-bot@r-build-sonic-ci02-243

Platform: x86_64-nvidia_sn5600_simx-r0
HwSKU: ACS-SN5600
ASIC: mellanox
ASIC Count: 1
Serial Number: MT2022X08597
Model Number: MSN4700-WS2FO
Hardware Revision: N/A
Uptime: 13:55:31 up 11:41, 1 user, load average: 0.00, 0.00, 0.00

Docker images:
REPOSITORY TAG IMAGE ID SIZE
docker-dhcp-relay latest edd72bdf8147 449MB
docker-sonic-telemetry latest 5d09e3b72d4e 528MB
docker-sonic-telemetry master.289-44028723e_Internal 5d09e3b72d4e 528MB
docker-database latest b32db879bff8 443MB
docker-database master.289-44028723e_Internal b32db879bff8 443MB
docker-teamd latest 99ee886ffabe 441MB
docker-teamd master.289-44028723e_Internal 99ee886ffabe 441MB
docker-syncd-mlnx latest 36e87f37db14 920MB
docker-syncd-mlnx master.289-44028723e_Internal 36e87f37db14 920MB
docker-sonic-mgmt-framework latest 947e18f39843 582MB
docker-sonic-mgmt-framework master.289-44028723e_Internal 947e18f39843 582MB
docker-snmp latest e2e4751658d6 470MB
docker-snmp master.289-44028723e_Internal e2e4751658d6 470MB
docker-sflow latest 986258e62a19 442MB
docker-sflow master.289-44028723e_Internal 986258e62a19 442MB
docker-router-advertiser latest f5899e1f887a 427MB
docker-router-advertiser master.289-44028723e_Internal f5899e1f887a 427MB
docker-platform-monitor latest ecd515f89ac3 660MB
docker-platform-monitor master.289-44028723e_Internal ecd515f89ac3 660MB
docker-orchagent latest 77b5c7010337 462MB
docker-orchagent master.289-44028723e_Internal 77b5c7010337 462MB
docker-nat latest 7c35e73a9045 444MB
docker-nat master.289-44028723e_Internal 7c35e73a9045 444MB
docker-mux latest 1bf1009fce5a 479MB
docker-mux master.289-44028723e_Internal 1bf1009fce5a 479MB
docker-macsec latest 8dbdb424ecd2 444MB
docker-macsec master.289-44028723e_Internal 8dbdb424ecd2 444MB
docker-lldp latest b173e8340da2 467MB
docker-lldp master.289-44028723e_Internal b173e8340da2 467MB
docker-fpm-frr latest a8a83cbeb8e4 460MB
docker-fpm-frr master.289-44028723e_Internal a8a83cbeb8e4 460MB
urm.nvidia.com/sw-nbu-sws-sonic-docker/sonic-wjh 1.0.0-master-internal-30 59effeefff34 472MB

Output of show techsupport:

Additional information you deem important (e.g. issue happens only occasionally):
@bingwang-ms
Copy link
Contributor

Hi @raphaelt-nvidia
Thanks for the comments. Supporting both hexadecimal and decimal makes sense to me. I'll self-assign this issue and post a fix.

@bingwang-ms bingwang-ms self-assigned this Mar 2, 2022
@zhangyanzhao zhangyanzhao added Triaged this issue has been triaged MSFT labels Mar 2, 2022
This was referenced Mar 3, 2022
Merged
Merged
qiluo-msft pushed a commit that referenced this issue Mar 19, 2022
@bingwang-ms
[yang] Update YANG model for mirror session to support decimal value …
fb7f046 
…for GRE type (#10140)

#### Why I did it
PR  sonic-net/sonic-utilities#1825 added validation for the input of `config mirror session add`, and only decimal value is accepted.
An issue #10096 was raised to suggest accepting HEX value as well, and the suggestion makes sense to me.

To accept HEX value for GRE type, and keep backward compatibility as well, I updated the YANG model to support both decimal and hexadecimal input for GRE type.

#### How I did it
Update the regex for GRE type.

#### How to verify it
Verified by UT
```
platform linux -- Python 3.9.2, pytest-6.0.2, py-1.10.0, pluggy-0.13.0
rootdir: /sonic/src/sonic-yang-models
plugins: pyfakefs-4.5.4, cov-2.10.1
collected 3 items                                                                                                                                                                                     

tests/test_sonic_yang_models.py ..                                                                                                                                                              [ 66%]
tests/yang_model_tests/test_yang_model.py .                                                                                                                                                     [100%]

========================================================================================== 3 passed in 2.53s ==========================================================================================
```

#### Description for the changelog
Update YANG model for mirror session to support decimal value for GRE type.
judyjoseph pushed a commit that referenced this issue Mar 20, 2022
@bingwang-ms @judyjoseph
[yang] Update YANG model for mirror session to support decimal value …
ef9df9e 
…for GRE type (#10140)

#### Why I did it
PR  sonic-net/sonic-utilities#1825 added validation for the input of `config mirror session add`, and only decimal value is accepted.
An issue #10096 was raised to suggest accepting HEX value as well, and the suggestion makes sense to me.

To accept HEX value for GRE type, and keep backward compatibility as well, I updated the YANG model to support both decimal and hexadecimal input for GRE type.

#### How I did it
Update the regex for GRE type.

#### How to verify it
Verified by UT
```
platform linux -- Python 3.9.2, pytest-6.0.2, py-1.10.0, pluggy-0.13.0
rootdir: /sonic/src/sonic-yang-models
plugins: pyfakefs-4.5.4, cov-2.10.1
collected 3 items                                                                                                                                                                                     

tests/test_sonic_yang_models.py ..                                                                                                                                                              [ 66%]
tests/yang_model_tests/test_yang_model.py .                                                                                                                                                     [100%]

========================================================================================== 3 passed in 2.53s ==========================================================================================
```

#### Description for the changelog
Update YANG model for mirror session to support decimal value for GRE type.
praveen-li pushed a commit to praveen-li/sonic-buildimage that referenced this issue Dec 23, 2022
@bingwang-ms
[yang] Update YANG model for mirror session to support decimal value …
ef07f58 
…for GRE type (sonic-net#10140)

#### Why I did it
PR  sonic-net/sonic-utilities#1825 added validation for the input of `config mirror session add`, and only decimal value is accepted.
An issue sonic-net#10096 was raised to suggest accepting HEX value as well, and the suggestion makes sense to me.

To accept HEX value for GRE type, and keep backward compatibility as well, I updated the YANG model to support both decimal and hexadecimal input for GRE type.

#### How I did it
Update the regex for GRE type.

#### How to verify it
Verified by UT
```
platform linux -- Python 3.9.2, pytest-6.0.2, py-1.10.0, pluggy-0.13.0
rootdir: /sonic/src/sonic-yang-models
plugins: pyfakefs-4.5.4, cov-2.10.1
collected 3 items                                                                                                                                                                                     

tests/test_sonic_yang_models.py ..                                                                                                                                                              [ 66%]
tests/yang_model_tests/test_yang_model.py .                                                                                                                                                     [100%]

========================================================================================== 3 passed in 2.53s ==========================================================================================
```

#### Description for the changelog
Update YANG model for mirror session to support decimal value for GRE type.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bingwang-ms bingwang-ms

Labels
MSFT Triaged this issue has been triaged
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update config mirror_session CLI to support heximal gre type value bingwang-ms/sonic-utilities
3 participants
@zhangyanzhao @bingwang-ms @raphaelt-nvidia