diff options
| author | Robert Navarro <crshman@gmail.com> | 2026-06-19 06:02:27 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2026-06-19 14:02:27 +0100 |
| commit | 5b80cd0cbddd014e54b01e7fa1cd0c0777d94724 (patch) | |
| tree | dd29f3ef378d43abf85a627d16aee841148c2ad8 /plugins/filter | |
| parent | 05a852be0687eee5264c190942da66c7139465bd (diff) | |
| download | vyos.vyos-5b80cd0cbddd014e54b01e7fa1cd0c0777d94724.tar.gz vyos.vyos-5b80cd0cbddd014e54b01e7fa1cd0c0777d94724.zip | |
rm_templates: T8609: fix slow parse() from group-quantifier patterns (#470)
* rm_templates: T8609: fix slow parse() from group-quantifier patterns
Running an Ansible playbook that manages BGP route-maps on my VyOS 1.4
edge routers, the vyos.vyos.vyos_route_maps task (state=gathered or
state=merged) takes ~50 seconds per host, sometimes 100s+, against
devices with only a handful of route-maps configured. The persistent
connection times out before the module finishes and the failure
surfaces as a misleading "socket path does not exist".
Three related quantifier shapes in nine rm_templates files cause O(2^n)
regex backtracking on inputs that share a parser's prefix but don't
match overall. parse() time on a representative 12-line route-map
config drops from ~50s to <1ms post-fix.
* Trailing form (188 sites): `(?P<X>\S+)\n *$"""` -> `(?P<X>\S+)\s*$"""`.
Under re.VERBOSE the literal newline+indent between `\S+` and `*$`
is stripped at compile time, so the source compiled to `(\S+)*$`
with the `*` quantifying the named group.
* Mid-pattern `(group containing \S+)*` (22 sites in snmp_server.py
with a couple in bgp_global*.py): e.g.
`(?P<protocol>protocol\s\S+)*` -> `(?P<protocol>protocol\s\S+)?`.
Different position from the trailing form, but the same shape
underneath (a group whose content includes \S+, quantified with
`*`), so the same O(2^n) backtracking on prefix-sharing inputs.
* Mid-pattern `(literal-only group)*` (14 sites in snmp_server.py,
bgp_address_family*.py, bgp_global*.py): e.g.
`(?P<as_set>as-set)*` -> `(?P<as_set>as-set)?`. No \S+ inside the
quantified group, so the backtracking exposure is much smaller.
On the inputs these patterns actually receive (no device line
carries duplicate flags) `*` and `?` accept identical input sets,
so changing to `?` is behavior-preserving.
The first draft of this fix made set_comm_list_delete's
`(?P<delete>\S+)` group required. The parser's setval emits `set
comm-list delete` with no token after `delete`, so the parser stopped
matching its own output (no existing fixture covered this case, which
is why the regression initially shipped). CodeRabbit caught it
during review. Replaced `delete(?P<delete>\S+)\s*$` with
`\s(?P<delete>delete)\s*$` so the named group captures the literal
word `delete`; the result template `{{True if delete is defined}}`
continues to evaluate True.
Affects: route_maps[/_14], bgp_global[/_14], bgp_address_family[/_14],
snmp_server, ospf_interfaces[/_14].
Adds tests/unit/modules/network/vyos/test_rm_templates_perf.py with
a 1-second budget against fixture inputs that have realistic-length
identifiers, plus a round-trip test asserting set_comm_list_delete
matches its own setval-generated line, plus a Bgp_address_familyTemplate14
budget test for symmetry with the other "hot" template families.
* Fix regex backtracking issues in parse() method
Fix slow parse() to prevent regex backtracking on prefix-sharing inputs across multiple files. Add unit test for the bugfixes.
* Refactor bug fixes for regex backtracking improvements
Updated bug fixes to include details on regex backtracking issues in multiple files.
* Fix typo in bugfixes section of changelog
* Update bugfixes for regex backtracking issues
* rm_templates: T8609: route_maps.py: fix remaining trailing `*$` patterns
Five sites in route_maps.py still had the `(?P<name>...)\n *$` shape that collapses to `(?P<name>...)*$` under `re.VERBOSE`, parsers `sequence`, `on_match_next`, `set_atomic_aggregate`, `set_extcommunity_bandwidth_non_transitive`, and `match_community_exact_match`. Collapsed each to `\s*$` on the same line as the named group, matching the rest of the PR.
* rm_templates: T8609: route_maps.py: split overlong getval to satisfy E501
Line 517 was 161 chars (`set_extcommunity_bandwidth_non_transitive` parser) and tripped pycodestyle's E501 sanity test in CI. Split the regex source across two lines at the `\d+)` boundary; under `re.VERBOSE` the literal newline and indent between regex tokens are stripped at compile time, so the engine sees the same pattern. `\s*$` stays on the same line as the closing capture group, so the group-quantifier shape this PR fixes elsewhere isn't reintroduced.
---------
Co-authored-by: omnom62 <75066712+omnom62@users.noreply.github.com>
Diffstat (limited to 'plugins/filter')
0 files changed, 0 insertions, 0 deletions
