Skip to content

Fix route_check.py: replace ijson with vtysh text streaming#4651

Open
mike-dubrovsky wants to merge 3 commits into
sonic-net:masterfrom
mike-dubrovsky:route-check-text-streaming
Open

Fix route_check.py: replace ijson with vtysh text streaming#4651
mike-dubrovsky wants to merge 3 commits into
sonic-net:masterfrom
mike-dubrovsky:route-check-text-streaming

Conversation

@mike-dubrovsky

Copy link
Copy Markdown
Contributor

Summary

Split from #4576 per @deepak-singhal0408's reviewBucket C (performance, needs discussion).

Depends on #4649 and the correctness PR (Bucket B). Replaces the ijson JSON parser with a line-by-line scan of vtysh show ip/ipv6 route plain-text output.

Why text over JSON: vtysh show ip route json serializes the full in-memory RIB before emitting any bytes; plain-text output is streamed incrementally and scanned with r'^B>([qo])\s+(\S+)\s', which is significantly faster on large T2 route tables (see #4576 measurements).

Returns missing routes as {'prefix', 'protocol'} dicts for mitigation compatibility.

Test plan

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@mike-dubrovsky
mike-dubrovsky force-pushed the route-check-text-streaming branch from 5284229 to 7e4567e Compare June 29, 2026 19:22
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Collaborator

Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks!

---Powered by SONiC BuildBot

mitigate_installed_not_offloaded_frr_routes() expects missed_frr_rt entries
with 'prefix' and 'protocol' keys, but fetch_routes() appended bare prefix
strings after the ijson refactor (sonic-net#4205), causing TypeError whenever
suppress-fib-pending triggers mitigation.

Adopt the 202511 approach (sonic-net#4509): append
{'prefix': prefix, 'protocol': route_entry.get('protocol', '')} so the
mitigation contract is preserved without changing callers.

Update tests to match the dict format, ported from 202511 (sonic-net#4509):
- fetch_routes_chunk_test.py expectations
- route_check_test.py mock_fetch_routes now emits dict entries so the
  end-to-end path mirrors production
- route_check_test_data.py missed_FRR_routes golden entries

Signed-off-by: mike-dubrovsky <mdubrovs@cisco.com>
Replace the overwrite-on-each-iteration retry loop with an intersection
accumulator so that only routes stuck in every poll window are mitigated,
avoiding premature mitigation of routes that are still converging.

Add unit tests for the intersection logic with dict-format missing routes.

Signed-off-by: mike-dubrovsky <mdubrovs@cisco.com>
Replace the ijson-based JSON parser in fetch_routes() with a line-by-line
scan of vtysh plain-text output. vtysh streams text incrementally, avoiding
the full-RIB JSON serialization cost on large route tables.

Return missing routes as {'prefix', 'protocol'} dicts so mitigation remains
compatible with the dict-format contract from the bugfix PR.

Add tests/fetch_routes_test.py and remove ijson-based chunk tests that no
longer apply to the text-streaming implementation.

Signed-off-by: mike-dubrovsky <mdubrovs@cisco.com>
@mike-dubrovsky
mike-dubrovsky force-pushed the route-check-text-streaming branch from 7e4567e to f3c2f48 Compare July 6, 2026 18:18
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants