sonic-net / sonic-buildimage

Scripts which perform an installable binary image build for SONiC
Other
701 stars 1.35k forks source link

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

Closed raphaelt-nvidia closed 2 years ago

raphaelt-nvidia commented 2 years ago

Description

In June 2021, I opened https://github.com/Azure/sonic-buildimage/issues/7990 [mirroring] Missing type validations in config mirror-session command. This was addressed by https://github.com/Azure/sonic-utilities/pull/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]

[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: 44028723e 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 commented 2 years ago

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.