Review PRs opened in the last 48 hours by humans

[Copilot]

Reviews 32 human-authored PRs opened between 2026-03-24 and 2026-03-26 in sonic-net/sonic-mgmt. Bot PRs (Copilot, mssonicbld) excluded.

Note: GitHub API write access is blocked by the network security proxy in this environment; reviews are documented in this PR description rather than posted as individual PR comments.

---

Critical Issues Found (🔴)

PR	Issue
#23323	Teardown bug: if dpuhost.dpu_data_port_ip: guards the wrong route (REMOTE_PA_IP via pe_nexthop_ip); the APPLIANCE_VIP route that actually uses dpu_data_port_ip remains unguarded
#23317 / #23318	Both PRs modify the same lines in tests_mark_conditions.yaml; one must be closed or rebased before the other merges
#23320	garp_enabled fixture changed from arp_accept == 1 → == 2; breaks existing test_arp_garp_enabled which depends on this fixture value
#23292	logger.warning(...) fires in the normal IPv4 path (wrong else branch); should only fire when IPv6 mode is requested but ansible_hostv6 is unset
#23283	Module-level _fixture_failures dict is never cleared — failures from one test run persist and incorrectly skip tests in subsequent runs
#23294	wait_for_rsyslog_ready reads entire /var/log/syslog line-by-line on every poll; will be unacceptably slow on production systems (100s of MB logs)

---

Per-PR Reviews

#23329 — disable northbound route zmq flag for ha topology

• Exact string match self.topo_name == "t1-smartswitch-ha" is fragile — consider "ha" in self.topo_name for future HA variants
• Add inline comment referencing issue Bug: failed to install route to peer switch's loopback when orch_northbond_route_zmq_enabled set to true #23328 since this is an explicit workaround
• Type-of-change checkboxes all unchecked; backport to 202511 not evaluated

#23327 — Add SONiC version precheck for test_buffer_profile_create_remove_rollback

• os_version.split('.')[0][:6] < min_version string comparison is correct for YYYYMM.DD but wrap in try/except for non-standard build strings
• PR body duplicates the Approach/Verification sections

#23326 — Use converged topo in KVM to save PR test CPU

• PR description entirely blank; please fill in motivation and expected CPU savings

#23325 — Fix QOS tests to run on 64p topo

• The else branch adding |Asic0| applies to all single-asic systems, not just VOQ; should guard with switch_type == "voq" check to avoid affecting non-VOQ single-asic platforms

#23324 — iface_namemode: add new port role of Dpc

• ✅ Minimal and correct; Yang model reference confirms Dpc is a valid role

#23323 — [smartswitch] fix errors from command execution on DPU

• 🔴 See critical issues above — teardown guard is on the wrong route

#23322 — vpp: cleanup lldp related skip markers

• Scopes snmp_phy_entity and snmp_psu skips to x86_64-kvm_x86_64-r0 only; confirm these pass on VPP hardware before merging

#23321 — [cherry-pick] configlet/test_add_rack.py for ipv6-only topos

• Backport checkbox for 202511 should be checked (this PR targets 202511)

#23320 — Add GARP out-of-subnet test (DRAFT)

• 🔴 See critical issues above — garp_enabled fixture assertion changed from 1 → 2
• time.sleep(2) should be wait_until(); missing @pytest.mark.topology() decorators; PR description blank

#23319 — [gcu][ecn] Fix test_ecn_config_updates

• When min == max == 0 and both fields updated, determine_delta_values returns no-op deltas — the test then verifies a zero-change patch succeeds; confirm this is the intended behavior

#23318 — Skip QoS ECN on non-j2c platforms

• 🔴 Conflicts with [GIT_PR20544][Qos]Update conditionalmark to platform specific for DscpEcntest #23317
• New condition would also skip cisco-8000; confirm whether that's intentional

#23317 — Update conditionalmark for DscpEcntest

• 🔴 Conflicts with [testQosSaiDscpEcn] [test_ecn_config.py] Skip QoS ECN tests on non-j2c platforms #23318
• Hardcoding a single Nokia platform name won't scale; the asic_subtype approach in [testQosSaiDscpEcn] [test_ecn_config.py] Skip QoS ECN tests on non-j2c platforms #23318 is more maintainable

#23316 — vpp: enable crm tests (DRAFT)

• ✅ Rationale well-supported by sairedis PRs

#23315 — Fix ipfwd/test_dip_sip.py for v6 topo

• switch_arptable['arptable']['v6'][intf['peer_addr']]['macaddress'] can KeyError if neighbor not yet in table — use .get()
• Double-# comment leftover: # # Verify IPv6 route is in the routing table

#23314 — Unskip test_srv6 for TH6 and Q3D

• ✅ Minimal and correct

#23312 — Fix IndexError/KeyError in test_buffer_pg

• ✅ Correct fix; verify callers handle (None, False) return when profile_in_pg is empty

#23310 — skip reboot_cause for sn6600_ld

• 'sn6600_ld' in platform is a substring match; prefer platform in ['x86_64-nvidia_sn6600_ld-r0'] for explicitness and consistency

#23309 — Add sn6600_ld and sn6600_simx platform data

• fans: 0, psus: 0, cpu_cores: 0 for sn6600_ld — confirm these are correct for LD form factor, not placeholders
• sn6600_simx missing hot_swappable and capabilities in psus

#23307 — Everflow queue counters fix 202511

• Unrelated bgp/reliable_tsa skip removal bundled in; consider splitting into a separate PR

#23306 — add spc6 memory utilization configuration

• JSON key "Mellanox-SN6600":["..."] missing space after colon; inconsistent with surrounding entries

#23305 — Add simx SPC6 hwsku to support platform tests

• ✅ Minimal and correct; title typo: "platfrom" → "platform"

#23304 — Add LogAnalyzer ignore pattern for SAI FEC stat error

• ✅ Correct regex, follows existing pattern format
• Comment says "on 202511" but this is a master PR — remove branch qualifier from comment

#23303 — skip test_reboot on SN5640

• ✅ Clean and well-described

#23302 — xfail decap tests on SN5640

• New top-level test_vnet_decap xfail overlaps with existing parameterized variant entry; harmless but redundant

#23301 — pfcwd: scale restore timeout by port count

• Verify stormed_ports_list is in scope at the num_ports assignment for both storm and restore code paths

#23298 — Scope Ansible connection vars per host (DRAFT)

• Private vm._hostvars fallback is fragile across Ansible versions
• Hardcoding ansible_connection='multi_passwd_ssh' in fanout.py may break EOS/Onyx fanout initialization

#23296 — System health LED fix

• Confirm check_system_health_led_info() returns a truthy value on success (required for wait_until to work)
• delay=0 in wait_until polls immediately; small initial delay (2–5s) would reduce noise

#23295 — fix console login extra newline

• ✅ Correct and well-analyzed

#23294 — [loganalyzer] Wait for rsyslog readiness (DRAFT)

• 🔴 See critical issues above — full syslog scan is too slow
• Verify self.flush_rsyslogd() and import os are available in this context

#23292 — Add IPv6-only management mode support

• 🔴 See critical issues above — spurious warning in IPv4 path
• _ipv6_only_mgmt_mode class-level state is problematic for parallel execution; pass via fixture instead

#23291 — fix po_cleanup loganalyzer marker loss

• Syslog drain poll (30 × 10s = 300s) + config_reload wait=240s could push total runtime to 540s+; document expected max
• killall yes should be kill <PID> per codebase guidelines

#23288 — re-enable vpp pipeline pr test

• ✅ Correctly removes duplicate not(canceled()), succeeded() from t2 condition
• condition: not(canceled()) on VPP job intentionally omits succeeded() — confirm this is desired

#23283 — test_qos_sai: prevent cascading failures (DRAFT)

• 🔴 See critical issues above — module-level dict persists across runs
• params.split('-')[0] is fragile for hyphenated parameter names like single-asic

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• https://api.github.com/
  • Triggering command: /usr/bin/curl curl -s --connect-timeout 5 REDACTED -o /dev/null -w %{http_code} (http block)
• https://api.github.com/graphql
  • Triggering command: /usr/bin/gh gh auth status (http block)
• https://api.github.com/repos/sonic-net/sonic-mgmt/issues/23329/comments
  • Triggering command: /usr/bin/curl curl -s -X POST -H Authorization: token ****** -H Content-Type: application/json REDACTED -d { "body": "## Code Review\n\nThank you for the workaround fix. A few observations:\n\n1. **PR description incomplete**: The \"Type of change\" checkboxes are all unchecked. This should be marked as at least \"Bug fix\" or \"Testbed and Framework\" dep (http block)
  • Triggering command: /usr/bin/curl curl -v -s -X POST -H Authorization: ****** -H Content-Type: application/json -H X-GitHub-Api-Version: 2022-11-28 REDACTED -d {"body":"test comment"} (http block)
  • Triggering command: /usr/bin/curl curl -s -X POST -H Authorization: ****** -H Content-Type: application/json -H X-GitHub-Api-Version: 2022-11-28 REDACTED -d {"body":"test comment"} (http block)
• https://api.github.com/repos/sonic-net/sonic-mgmt/pulls/23329
  • Triggering command: /usr/bin/curl curl -s -o /dev/null -w %{http_code} -H Authorization: token ****** REDACTED (http block)
• https://api.github.com/user
  • Triggering command: /usr/bin/curl curl -s -H Authorization: token ****** REDACTED (http block)
  • Triggering command: /usr/bin/curl curl -sI -H Authorization: token ****** REDACTED (http block)
  • Triggering command: /usr/bin/curl curl -v -s --max-time 10 -H Authorization: token ****** REDACTED (http block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

---

📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.