[multi-asic] Add multi asic support for VRF and clear flowcnt-trap#4383
Open
DavidZagury wants to merge 5 commits into
Open
[multi-asic] Add multi asic support for VRF and clear flowcnt-trap#4383DavidZagury wants to merge 5 commits into
DavidZagury wants to merge 5 commits into
Conversation
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: david.zagury <davidza@nvidia.com>
91eaf2b to
9852e41
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
86a62ef to
d754747
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: david.zagury <davidza@nvidia.com>
d754747 to
14dc447
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
oleksandrivantsiv
previously approved these changes
May 5, 2026
Collaborator
|
@DavidZagury i assume that these commands are on the command reference doc, should you update it with the updated CLI as well? |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…ace support Signed-off-by: david.zagury <davidza@nvidia.com>
7c4ae2f to
61c00b0
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
Updated command reference |
oleksandrivantsiv
previously approved these changes
May 18, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds multi-ASIC namespace handling to VRF configuration and trap flow counter clearing commands in sonic-utilities.
Changes:
- Adds
-n/--namespacesupport toconfig vrf. - Adds
-n/--namespacesupport tosonic-clear flowcnt-trap. - Updates tests and command reference documentation for the new namespace behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
config/main.py |
Adds namespace option and namespaced CONFIG_DB connection for config vrf. |
clear/main.py |
Passes namespace through to flow_counters_stat for trap flow counter clears. |
tests/vrf_test.py |
Adds VRF namespace behavior tests. |
tests/clear_test.py |
Adds flowcnt-trap namespace tests. |
doc/Command-Reference.md |
Documents namespace usage for VRF and flow counter clear commands. |
| ``` | ||
|
|
||
| - Details: | ||
| - -n, --namespace: (Multi-ASIC) Namespace name (e.g. asic0). When omitted, the default namespace is targeted. |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
bd11708 to
174e371
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
174e371 to
3339459
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
The `config vrf` group now treats `-n/--namespace` as optional and validates it per-subcommand: `add mgmt`/`del mgmt` reject `-n` and write to the global CONFIG_DB, while data-VRF subcommands (`add Vrf-*`, `del Vrf-*`, `add_vrf_vni_map`, `del_vrf_vni_map`) require `-n` on multi-ASIC platforms. Doc updated to match. Multi-ASIC vrf tests now use context-manager patches with the module reload inside a try/finally so the patched multi-ASIC state can't leak into later tests. Signed-off-by: david.zagury <davidza@nvidia.com>
3339459 to
2875a23
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: david.zagury <davidza@nvidia.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
oleksandrivantsiv
approved these changes
Jun 4, 2026
Member
|
@qiluo-msft could you please help review and approve the change? |
Comment on lines
+341
to
+342
| @patch('clear.main.run_command') | ||
| @patch('utilities_common.multi_asic.multi_asic_ns_choices', MagicMock(return_value=[''])) |
Comment on lines
+350
to
+353
| @patch('clear.main.run_command') | ||
| @patch('utilities_common.multi_asic.multi_asic_ns_choices', | ||
| MagicMock(return_value=['asic0', 'asic1', 'asic2', 'asic3'])) | ||
| @patch.object(click.Choice, 'convert', MagicMock(return_value='asic0')) |
Comment on lines
+686
to
+688
| @click.option('--namespace', '-n', 'namespace', default=None, | ||
| type=click.Choice(multi_asic_util.multi_asic_ns_choices()), | ||
| show_default=True, help='Namespace name') |
Member
|
@saiarcot895 could you please review the change? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I did
Added multi asic support for commands:
How I did it
Modified the vrf and clear flowcnt-trap commands to add muti-asic support
How to verify it
Verified the commands in a multi asic device using the namespace flag
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)