Skip to content

Commit 647b47b

Browse files
authored
[Port Mirror][CLI] Align ERSPAN sample_rate range with YANG model (#381)
<!-- Please make sure you've read and understood our contributing guidelines: https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md CODE_OF_CONDUCT.md LICENSE README.md SECURITY.md SUPPORT.md azure-pipelines failure_prs.log scripts skip_prs.log Make sure all your commits include a signature generated with `git commit -s` ** If this is a bug fix, make sure your description includes "closes #xxxx", "fixes #xxxx" or "resolves #xxxx" so that GitHub automatically closes the related issue when the PR is merged. If you are adding/modifying/removing any command or utility script, please also make sure to add/modify/remove any unit tests from the tests directory as appropriate. If you are modifying or removing an existing 'show', 'config' or 'sonic-clear' subcommand, or you are adding a new subcommand, please make sure you also update the Command Line Reference Guide (doc/Command-Reference.md) to reflect your changes. Please provide the following information: --> #### What I did Aligned the ERSPAN mirror session `sample_rate` validation range with the YANG model ([sonic-buildimage #26756](sonic-net/sonic-buildimage#26756)), which defines it as uint32 `2..max`. #### How I did it - `config/main.py`: changed from `SAMPLE_RATE_MIN` 256 to 2, changed `SAMPLE_RATE_MAX` 8388608 to `0xFFFFFFFF` (uint32 max). - `tests/config_mirror_session_test.py`: updated assertions and reworked the boundary test. - `doc/Command-Reference.md`: updated the valid range. #### How to verify it pytest tests/config_mirror_session_test.py Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com> #### 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 parent 8058eec commit 647b47b

3 files changed

Lines changed: 24 additions & 23 deletions

File tree

config/main.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,16 @@
128128
asic_type = None
129129

130130

131-
SAMPLE_RATE_MIN = 256
132-
SAMPLE_RATE_MAX = 8388608
131+
SAMPLE_RATE_MIN = 2
132+
SAMPLE_RATE_MAX = 0xFFFFFFFF # uint32 max
133133
TRUNCATE_SIZE_MIN = 64
134134
TRUNCATE_SIZE_MAX = 9216
135135

136136

137137
def validate_sample_rate(ctx, param, value):
138138
if value != 0 and (value < SAMPLE_RATE_MIN or value > SAMPLE_RATE_MAX):
139-
raise click.BadParameter(f"must be 0 or in range {SAMPLE_RATE_MIN}..{SAMPLE_RATE_MAX}")
139+
raise click.BadParameter(
140+
f"must be 0 or in range {SAMPLE_RATE_MIN}..{SAMPLE_RATE_MAX} (uint32 max)")
140141
return value
141142

142143

@@ -3216,7 +3217,7 @@ def erspan(ctx):
32163217
@click.argument('direction', metavar='[direction]', required=False)
32173218
@click.option('--policer')
32183219
@click.option('--sample_rate', type=int, default=0, callback=validate_sample_rate,
3219-
help="Sampling rate (1-in-N), 256..8388608. 0 disables sampling")
3220+
help="Sampling rate (1-in-N), 2..4294967295. 0 disables sampling")
32203221
@click.option('--truncate_size', type=int, default=0, callback=validate_truncate_size,
32213222
help="Truncation size in bytes, 64..9216. 0 disables truncation")
32223223
def erspan_add(session_name, src_ip, dst_ip, dscp, ttl, gre_type, queue, policer, src_port, direction,

doc/Command-Reference.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9327,8 +9327,8 @@ While adding a new ERSPAN session, users need to configure the following fields
93279327
7) optional - Policer which will be used to control the rate at which frames are mirrored.
93289328
8) optional - List of source ports which can have both Ethernet and LAG ports.
93299329
9) optional - Direction - Mirror session direction when configured along with Source port. (Supported rx/tx/both. default direction is both)
9330-
10) optional - Sample rate for sampled mirroring. N means mirror 1-in-N packets. When not specified, full mirroring is used. Valid range: 256..8388608.
9331-
11) optional - Truncate size in bytes for mirrored packets. When not specified, no truncation is applied. Valid range: 64..9216.
9330+
10) optional - Sample rate for sampled mirroring. N means mirror 1-in-N packets. When not specified (or set to 0), full mirroring is used. Valid values: 0 or 2..4294967295 (uint32 max).
9331+
11) optional - Truncate size in bytes for mirrored packets. When not specified (or set to 0), no truncation is applied. Valid values: 0 or 64..9216.
93329332

93339333
- Usage:
93349334
```

tests/config_mirror_session_test.py

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -810,7 +810,7 @@ def test_mirror_session_erspan_add_with_invalid_sample_rate():
810810
["test_session", "1.1.1.1", "2.2.2.2", "8", "64",
811811
"--sample_rate", "-1"])
812812
assert result.exit_code != 0
813-
assert 'must be 0 or in range 256..8388608' in result.output
813+
assert 'must be 0 or in range 2..4294967295' in result.output
814814

815815
# Verify invalid truncate_size (negative)
816816
result = runner.invoke(
@@ -824,45 +824,45 @@ def test_mirror_session_erspan_add_with_invalid_sample_rate():
824824
def test_mirror_session_erspan_add_sample_rate_boundary():
825825
runner = CliRunner()
826826

827-
# sample_rate=1 (in the 1-255 hole, should fail)
827+
# sample_rate=1 (below minimum of 2, should fail)
828828
result = runner.invoke(
829829
config.config.commands["mirror_session"].commands["erspan"].commands["add"],
830830
["test_session", "1.1.1.1", "2.2.2.2", "8", "64",
831831
"--sample_rate", "1"])
832832
assert result.exit_code != 0
833-
assert 'must be 0 or in range 256..8388608' in result.output
833+
assert 'must be 0 or in range 2..4294967295' in result.output
834834

835-
# sample_rate=255 (upper boundary of hole, should fail)
836-
result = runner.invoke(
837-
config.config.commands["mirror_session"].commands["erspan"].commands["add"],
838-
["test_session", "1.1.1.1", "2.2.2.2", "8", "64",
839-
"--sample_rate", "255"])
840-
assert result.exit_code != 0
841-
assert 'must be 0 or in range 256..8388608' in result.output
835+
# sample_rate=2 (minimum valid, should pass)
836+
with mock.patch('config.main.add_erspan') as _:
837+
result = runner.invoke(
838+
config.config.commands["mirror_session"].commands["erspan"].commands["add"],
839+
["test_session", "1.1.1.1", "2.2.2.2", "8", "64",
840+
"--sample_rate", "2"])
841+
assert result.exit_code == 0
842842

843-
# sample_rate=256 (minimum valid, should pass)
843+
# sample_rate=100 (in the valid range, should pass)
844844
with mock.patch('config.main.add_erspan') as _:
845845
result = runner.invoke(
846846
config.config.commands["mirror_session"].commands["erspan"].commands["add"],
847847
["test_session", "1.1.1.1", "2.2.2.2", "8", "64",
848-
"--sample_rate", "256"])
848+
"--sample_rate", "100"])
849849
assert result.exit_code == 0
850850

851-
# sample_rate=8388608 (maximum valid, should pass)
851+
# sample_rate=4294967295 (maximum valid, should pass)
852852
with mock.patch('config.main.add_erspan') as _:
853853
result = runner.invoke(
854854
config.config.commands["mirror_session"].commands["erspan"].commands["add"],
855855
["test_session", "1.1.1.1", "2.2.2.2", "8", "64",
856-
"--sample_rate", "8388608"])
856+
"--sample_rate", "4294967295"])
857857
assert result.exit_code == 0
858858

859-
# sample_rate=8388609 (above maximum, should fail)
859+
# sample_rate=4294967296 (above maximum, should fail)
860860
result = runner.invoke(
861861
config.config.commands["mirror_session"].commands["erspan"].commands["add"],
862862
["test_session", "1.1.1.1", "2.2.2.2", "8", "64",
863-
"--sample_rate", "8388609"])
863+
"--sample_rate", "4294967296"])
864864
assert result.exit_code != 0
865-
assert 'must be 0 or in range 256..8388608' in result.output
865+
assert 'must be 0 or in range 2..4294967295' in result.output
866866

867867

868868
def test_mirror_session_erspan_add_truncate_size_boundary():

0 commit comments

Comments
 (0)