[show/vnet]: Fix ECMP scale formatting and per-endpoint MAC/VNI wrapping#2
Open
miatttao wants to merge 17 commits intoalawing:dev/davizhang/addshowmetricfrom
Open
Conversation
…4407) Updated fast-reboot script (which is soft-linked to express-reboot script) to allow Marvell Teralynx platform for express boot. Signed-off-by: Chris Nitin Adonis Petrus <cpetrus@marvell.com>
…t#4404) CLI does not allow the empty '' global namespace and this causes MGMT test failures. Signed-off-by: Peter <peterbailey@arista.com>
Remove the IJSON_BACKEND=python override. The pure-Python ijson backend is slow, causing route_check to take 21 minutes on chassis platforms with 100K+ routes. With the companion sonic-buildimage PR installing python3-ijson from Debian repos, the fast yajl2_c backend (C extension) becomes available and is auto-detected by ijson. Fixes sonic-net/sonic-buildimage#23442 Signed-off-by: deepak-singhal0408 <deepak-singhal0408@users.noreply.github.com> Signed-off-by: Deepak Singhal <deepsinghal@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…validators (sonic-net#4223) * Add th6 broadcom ASIC entry with all Nokia IXR7220-H6 HWSKUs: Nokia-IXR7220-H6-64, Nokia-IXR7220-H6-P128, Nokia-IXR7220-H6-O256 * Add unit test for th6 ASIC name resolution Signed-off-by: dygodwin <dylan.godwin@nokia.com>
miatttao
force-pushed
the
dev/mia/pretty-print-fix-addshowmetric
branch
from
fddbb94 to
2259766
Compare
What I did Certain bgp CLI commands like show ip bgp sum,show ip bgp neighbors <neighbor-ip> and show ip bgp neighbors <neighbor-ip> routes do not work for dynamic neighbors. Fixed the CLI by parsing dynamic BGP neighbors correctly. These fixes also result in the neighborName being displayed correctly for dynamic neighbors in the bgp summary. In addition, fixed a bug for static neighbors where the vrf name was not being checked (when specified). A neighbor IP match would return true even if the neighbor was configured in a different vrf. Fixed vrf all show commands, eg show ip bgp vrf all sum. Earlier it would combine the summaries for different vrfs into one table. Separated them out for clarity and to match VTYSH output. How I did it Removed dependency on src_address from dynamic neighbor parsing for bgp summary, which is an optional field. Added dynamic neighbor parsing for show ip bgp neighbors <neighbor-ip>. Earlier dynamic neighbors were not being checked at all. enhanced existing static neighbor parsing to also compare the vrf name instead of partial check on IP How to verify it Run show ip bgp sum,show ip bgp neighbors <neighbor-ip>,show ip bgp neighbors <neighbor-ip> routes on a DUT with dynamic neighbors, verify CLI output
…e sonic-kdump-config (sonic-net#4437) What I did Enforce the YANG allowlist patterns for ssh_string and ssh_path inside sonic-kdump-config itself. hostcfgd reads these fields from CONFIG_DB and passes them directly to sonic-kdump-config --ssh_string / --ssh_path. Values written to CONFIG_DB via raw redis or programmatic ConfigDB writes do not go through YANG validation, so the script must validate its own inputs. These values are interpolated into sed scripts and written into /etc/default/kdump-tools (which is sourced by /usr/sbin/kdump-config), so the input character set must be tightly constrained. How I did it Add SSH_STRING_RE / SSH_PATH_RE matching the patterns in sonic-kdump.yang, anchored with \Z (not $, which would permit a trailing newline). The user portion of ssh_string additionally requires an alphanumeric first character to avoid argv option confusion when passed positionally to kdump-config set-remote. write_ssh_path / write_ssh_string reject non-matching input with exit 1 before invoking any external command. Convert write_ssh_path and the kdump-config set-remote call from formatted-string commands to argv lists. Drop the no-longer-needed / escaping in write_ssh_string (the validated character set contains no /). How to verify it python3 -m pytest tests/sonic_kdump_config_test.py -v New tests test_write_ssh_path_validation and test_write_ssh_string_validation cover: Accepted: /root/.ssh/id_rsa, admin@10.0.0.1, user.name+tag@host.example.com, etc. Rejected: characters outside the allowlist (' " | ; $ ` space newline etc.), non-absolute paths, missing @, trailing newline, leading -. In all rejection cases run_command is asserted not called. Existing tests updated to expect list-form argv.
… number of seconds (sonic-net#4388) - What I did Add stop-time to manifest options, and pass the stop-time to docker_image_ctl.j2. + sonic-buildimage PR - sonic-net/sonic-buildimage#26366 - How to verify it Run Application extension with stop-time in manifest and make sure it is stopping after the wanted stop-time. Signed-off-by: noaOrMlnx <noaor@nvidia.com>
- What I did
Fixed config mirror_session and show mirror_session commands for multi-ASIC platforms. The previous implementation wrote SPAN/ERSPAN sessions to all front-panel ASIC namespaces, which caused failures when port-based arguments (dst_port, src_port) were only valid in one namespace — the loop would succeed on the port's own ASIC but fail on others, leaving partial writes in ConfigDB. This PR changes the storage model so that SPAN sessions are written only to the destination port's namespace, and ERSPAN src_port fields are distributed only to the namespace that owns each port. The show mirror_session (acl_loader) is updated accordingly to read and merge from all namespaces. Additionally, several robustness improvements were made: two-phase validate-then-write to prevent partial writes, exact port-token matching to avoid substring false positives, alias-aware port resolution, and cross-namespace duplicate session name detection.
- How I did it
config/main.py:
SPAN (add_span): Instead of looping through all front-panel ASIC namespaces (which fails because the destination port only exists in one namespace), the new code auto-detects the destination port's namespace via get_port_namespace() and writes the session only to that single namespace. It also validates that all source ports are on the same ASIC as the destination port before writing. All namespace ConfigDBs are connected upfront so that validate_mirror_session_config can perform cross-namespace duplicate session name checks.
ERSPAN (add_erspan): Source ports are distributed per-namespace — only the namespace that owns a given source port gets the src_port/direction fields. All other namespaces still receive the base ERSPAN session (without src_port) so ACL-based mirroring remains available everywhere. A two-phase approach (validate all namespaces first, then write) prevents partial writes when validation fails on any namespace.
remove: Added a get_entry check before set_entry(None) so that only namespaces that actually contain the session attempt to delete it, avoiding unnecessary JsonPatchConflict errors.
validate_mirror_session_config: Added front_asic_configdbs parameter to check for duplicate session names across all namespaces (not just the current one). Validation now uses split_mirror_ports() for exact port-token matching and normalize_mirror_src_port() for alias-to-canonical-name conversion.
get_port_namespace: Improved to resolve physical ports by both canonical name and alias regardless of CLI naming mode, and to fall back to scanning PORT aliases when the port name does not match a known table prefix.
New helper functions: split_mirror_ports() (exact comma-separated token parsing), normalize_mirror_src_port() (alias-to-canonical conversion for ConfigDB storage), mirror_entry_has_port() (exact port match replacing re.search substring matching), mirror_session_exists() (extracted for reuse).
interface_has_mirror_config: Replaced re.search substring matching with exact token matching via mirror_entry_has_port() to prevent false positives (e.g., Ethernet4 incorrectly matching Ethernet40).
gather_session_info: Removed inline alias conversion for src_port; alias handling is now deferred to normalize_mirror_src_port() which runs after validation, ensuring validation sees the original user-supplied port names.
acl_loader/main.py:
Since this PR changes SPAN sessions to be stored in only the destination port's namespace (instead of all namespaces), show session must now read from all front-panel ASIC namespace ConfigDBs and merge results (de-duplicated by session key), instead of reading from only the first namespace. Without this change, SPAN sessions could be invisible if the first namespace happens not to be the one containing the session.
Tracks which namespaces each session belongs to (session_namespaces), so state_db queries only target relevant namespaces — eliminating false error status for SPAN sessions on ASICs where they don't exist.
tests/config_mirror_session_test.py:
Fixed front_ns mock values from 'sample_ns' (string) to ['sample_ns'] (list) to match the actual get_all_namespaces() return format.
Added get_port_namespace mock for SPAN multi-ASIC test.
Added get_entry mock for remove multi-ASIC test.
New unit tests:
test_interface_has_mirror_config_matches_exact_port_tokens: verifies Ethernet4 does not false-match Ethernet40,Ethernet48.
test_add_span_validates_before_src_port_alias_conversion: verifies validation sees original alias names, ConfigDB gets canonical names.
test_add_erspan_validates_before_src_port_alias_conversion: same for ERSPAN path.
test_mirror_session_span_add_multi_asic_writes_only_destination_namespace: verifies SPAN writes only to dst_port's ASIC namespace.
test_mirror_session_span_add_multi_asic_rejects_cross_asic_source_port: verifies cross-ASIC src_port is rejected with no writes.
test_mirror_session_erspan_add_multi_asic_splits_source_ports_by_namespace: verifies ERSPAN distributes src_ports per-namespace correctly.
test_mirror_session_remove_multi_asic_skips_missing_sessions: verifies remove only deletes from namespaces that have the session.
- How to verify it
On multi-ASIC platform:
ERSPAN with src_port — previously caused partial-write:
# Old behavior: fails with "Source Interface Ethernet0 is invalid", but asic0 already has data
# New behavior: succeeds, asic0 gets src_port, asic1-3 get base ERSPAN session
sudo config mirror_session erspan add TEST_ERSPAN 192.0.2.31 198.51.100.31 23 34 35006 9 Ethernet0
show mirror_session TEST_ERSPAN
# Verify: all 4 ASICs show 'inactive' (not 'error')
sudo sonic-db-cli -n asic0 CONFIG_DB hgetall "MIRROR_SESSION|TEST_ERSPAN"
# Should have src_port=Ethernet0, direction=BOTH
sudo sonic-db-cli -n asic1 CONFIG_DB hgetall "MIRROR_SESSION|TEST_ERSPAN"
# Should have base ERSPAN fields only (no src_port)
sudo config mirror_session remove TEST_ERSPAN
SPAN with src/dst port — previously caused partial-write:
# Old behavior: fails with "Destination Interface Ethernet0 is invalid", but asic0 already has data
# New behavior: succeeds, only asic0 has the session
sudo config mirror_session span add TEST_SPAN Ethernet0 Ethernet1 both 10
show mirror_session TEST_SPAN
# Verify: only shows {'asic0': 'active'}, no error for other ASICs
sudo sonic-db-cli -n asic0 CONFIG_DB hgetall "MIRROR_SESSION|TEST_SPAN"
# Should have dst_port, src_port, direction, queue
sudo sonic-db-cli -n asic1 CONFIG_DB hgetall "MIRROR_SESSION|TEST_SPAN"
# Should be empty {}
sudo config mirror_session remove TEST_SPAN
ERSPAN cross-ASIC src_ports with conflict — verify no partial-write:
# Create a pre-existing session using Ethernet513 on asic1
sudo config mirror_session span add PRE_EXIST Ethernet512 Ethernet513 rx
# Attempt ERSPAN with src_ports spanning asic0 and asic1 (Ethernet513 conflicts)
sudo config mirror_session erspan add TEST_PARTIAL 192.0.2.1 198.51.100.1 8 64 0x6558 0 Ethernet0,Ethernet513 rx
# Should fail with "Source Interface Ethernet513 already has mirror config in same direction"
# Verify NO partial writes on any ASIC:
sudo sonic-db-cli -n asic0 CONFIG_DB hgetall "MIRROR_SESSION|TEST_PARTIAL"
# Should be empty {}
sudo sonic-db-cli -n asic1 CONFIG_DB hgetall "MIRROR_SESSION|TEST_PARTIAL"
# Should be empty {}
sudo config mirror_session remove PRE_EXIST
SPAN cross-ASIC src_port rejection:
sudo config mirror_session span add TEST_CROSS Ethernet0 Ethernet512 rx
# Should fail with "Source Interface Ethernet512 is not on the same ASIC as Destination Interface Ethernet0"
On single-ASIC platform:
# All commands should behave identically to before the change
sudo config mirror_session span add TEST1 Ethernet4 Ethernet8 rx
sudo config mirror_session erspan add TEST2 10.0.0.1 10.0.0.2 8 64 0x6558 3 Ethernet8 rx
sudo config mirror_session add TEST3 10.0.0.3 10.0.0.4 16 128
show mirror_session
sudo config mirror_session remove TEST1
sudo config mirror_session remove TEST2
sudo config mirror_session remove TEST3
Previous command output (if the output of a command-line utility has changed)
On multi-ASIC, show mirror_session for a SPAN session showed false errors:
SPAN Sessions
Name Status DST Port SRC Port Direction Queue Policer
---------- ------------------------------------------------------------------------- ---------- ---------- ----------- ------- ---------
TSPAN_FULL {'asic0': 'active', 'asic1': 'error', 'asic2': 'error', 'asic3': 'error'} Ethernet0 Ethernet1 both 10
New command output (if the output of a command-line utility has changed)
On multi-ASIC, show mirror_session now only shows relevant ASICs:
SPAN Sessions
Name Status DST Port SRC Port Direction Queue Policer
---------- ------------------- ---------- ---------- ----------- ------- ---------
TSPAN_FULL {'asic0': 'active'} Ethernet0 Ethernet1 both 10
On multi-ASIC, ERSPAN with src_port now shows all ASICs healthy:
ERSPAN Sessions
Name Status SRC IP DST IP GRE DSCP TTL Queue Policer Monitor Port SRC Port Direction
----------- ------------------------------------------------------------------------------------ ---------- ------------- ----- ------ ----- ------- --------- ---------------------------------------------------- ---------- -----------
TERSPAN_DEF {'asic0': 'inactive', 'asic1': 'inactive', 'asic2': 'inactive', 'asic3': 'inactive'} 192.0.2.31 198.51.100.31 35006 23 34 9 {'asic0': '', 'asic1': '', 'asic2': '', 'asic3': ''} Ethernet0 both
Signed-off-by: William Tsai <willtsai@nvidia.com>
miatttao
force-pushed
the
dev/mia/pretty-print-fix-addshowmetric
branch
4 times, most recently
from
33d71e6 to
9f09ec0
Compare
* add summary option to cli show queue wredcounters Signed-off-by: Zhixin Zhu <zhixzhu@cisco.com> * update UT for show queue wredcounters --summary Signed-off-by: Zhixin Zhu <zhixzhu@cisco.com> * update UT Signed-off-by: Zhixin Zhu <zhixzhu@cisco.com> --------- Signed-off-by: Zhixin Zhu <zhixzhu@cisco.com>
* restart mgmt over-usb interface if not RUNNING
PROBLEM:
Sometimes after "config reload" command the management
interface (eth0) connected over ETH-USB device is
UP/RUNNING on a remote peer side, but
stays UP/not-running on the local side.
Well-known negotiation problem of ETH-over-USB devices (ASIX).
SOLUTION:
If the management interface is a USB device and is not RUNNING
(/sys/class/net/${if}/operstate is not "up") at the end of the
"config reload" command
bring it down and up with delay to trigger renegotiation.
Before bringing it down, save default route and restore them
after bringing the interface up again
Signed-off-by: Yan Markman <ymarkman@marvell.com>
* Nokia-armhf config-reload with swss and sync restart
Fix for issue reported by the: sonic-net/sonic-buildimage#24766
Guaranty "config reload" by a swss.service restart "inside" the command implementation.
The restart is done ONLY for the device Nokia-7215 and does not impact other devices and platforms
Signed-off-by: Yan Markman <ymarkman@marvell.com>
---------
Signed-off-by: Yan Markman <ymarkman@marvell.com>
What I did Mux neighbor_mode and prober_type config is preserved when muxcable.py configures mode, etc. How I did it Retrieve the optional config and set them back in config_db along with changed parameters of mux cable. How to verify it Verified on hardware by changing mux mode. Verified using muxcable tests.
Why I did it FWPackage.untar_fwpackage() called tarfile.extractall() without validating member paths. A crafted firmware package with path traversal sequences (e.g. ../../) in member names could write files outside the intended extraction directory. How I did it Added os.path.realpath confinement check on each tar member before extraction. Members that resolve outside FWUPDATE_FWPACKAGE_DIR raise a ValueError. How to verify it Unit tests added in tests/fwutil_test.py::TestFWPackageUntar: Valid tar members extract successfully Members with ../../ path sequences are rejected Members with absolute paths are rejected
… watchdogutil_test (sonic-net#4456) (sonic-net#4456) What I did Fix issues in pytests related to import sonic_platform. That module may or may not be available on the python search path when the tests are run, and generally tests are written in a way to work when it is not present (or incorrectly assume it isn't present), using different approaches. This change fixes the way this is handled in two cases. test_module.py: Fix the way sonic_platform module is substituted with MagicMock, such that it now cleans up / doesn't leak to the test cases after. watchdogutil_test.py: The test case TestWatchdog.test_import_fails assumes the module is not present. However, it is possible that it is present. This also interacts in weird ways with the test_module.py leaking import (bullet point above), to cause failures. This change fixes that issue by mocking the import statement to simulate the module not being found, whether it exists or not, and adds an assertion that import sonic_platform actually gets called. Without these changes, its possible that if the sonic_platform module is available on the search path, the following test failure can occur: FAILED tests/watchdogutil_test.py::TestWatchdog::test_import_fails - assert 0 != 0 With these details: _____________________ TestWatchdog.test_import_fails _____________________ self = <tests.watchdogutil_test.TestWatchdog object at 0x7f73a36a42d0> @patch('os.geteuid', MagicMock(return_value=0)) def test_import_fails(self): runner = CliRunner() result = runner.invoke(watchdogutil.watchdogutil, ["version"]) > assert result.exit_code != 0 E assert 0 != 0 E + where 0 = <Result okay>.exit_code tests/watchdogutil_test.py:18: AssertionError How I did it Using monkeypatches to enforce and clean up the special-case behaviors for sonic_platform. How to verify it Without these changes, you can reproduce by ensuring that module sonic_platform is available. You can do this in sonic-buildimage when building with PLATFORM=mellanox by manually installing the wheel mlnx_platform_api-1.0-py3-none-any.whl in the slave container, or altering the sonic-utilities.mk file to add $(SONIC_PLATFORM_API_PY3) to the $(SONIC_UTILITIES_PY3)_DEPENDS list so it builds and installs before the test cases run: $(SONIC_UTILITIES_PY3)_DEPENDS += $(SONIC_PY_COMMON_PY3) \ $(SONIC_CONFIG_ENGINE_PY3) \ $(SONIC_PLATFORM_COMMON_PY3) \ $(SONIC_YANG_MGMT_PY3) \ - $(SONIC_YANG_MODELS_PY3) + $(SONIC_YANG_MODELS_PY3) \ + $(SONIC_PLATFORM_API_PY3) and then building sonic_utilities, which will run the test case: rm target/python-wheels/trixie/sonic_utilities-1.2-py3-none-any.whl NOBOOKWORM=1 KEEP_SLAVE_ON=yes SONIC_BUILD_JOBS=64 \ DOCKER_EXTRA_OPTS="--registry-mirror=https://dockerhub.nvidia.com" \ make target/python-wheels/trixie/sonic_utilities-1.2-py3-none-any.whl If you want a much quicker reproduction, you can add BUILD_SKIP_TEST=y to skip the many long tests that normally run, and then manually run the two tests in the slave container after the wheels are built. You must run both of these tests in one go to trigger the failure: tests/test_module.py tests/watchdogutil_test.py by running: cd src/sonic-utilities python -m pytest tests/test_module.py tests/watchdogutil_test.py
What I did Updating the show vnet cli: 1. showing new filed "metric" in show vnet route all/tunnel 2. added <vnet_name> for specific vnet name on vnet show route all/local/tunnel commands How I did it added metric field in show vnet route changed show vnet show route all/tunnel, and added vnet show route local updated command reference document Added mock test cases and related unit tests How to verify it added unit tests and passed.
miatttao
force-pushed
the
dev/mia/pretty-print-fix-addshowmetric
branch
3 times, most recently
from
e77bdd3 to
79d23a2
Compare
Signed-off-by: miatao <miatao@microsoft.com>
miatttao
force-pushed
the
dev/mia/pretty-print-fix-addshowmetric
branch
7 times, most recently
from
1cd7c06 to
cd32c04
Compare
miatttao
force-pushed
the
dev/mia/pretty-print-fix-addshowmetric
branch
from
cd32c04 to
ee40942
Compare
Signed-off-by: miatao <miatao@microsoft.com>
miatttao
force-pushed
the
dev/mia/pretty-print-fix-addshowmetric
branch
from
ee40942 to
5b7296f
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: miatao <miatao@microsoft.com>
miatttao
force-pushed
the
dev/mia/pretty-print-fix-addshowmetric
branch
from
6855f7b to
3bfce61
Compare
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
15 participants
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
How I did it
How to verify it
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)