From 5b80cd0cbddd014e54b01e7fa1cd0c0777d94724 Mon Sep 17 00:00:00 2001 From: Robert Navarro Date: Fri, 19 Jun 2026 06:02:27 -0700 Subject: 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\S+)\n *$"""` -> `(?P\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. `(?Pprotocol\s\S+)*` -> `(?Pprotocol\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. `(?Pas-set)*` -> `(?Pas-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\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\S+)\s*$` with `\s(?Pdelete)\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...)\n *$` shape that collapses to `(?P...)*$` 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> --- .../fragments/T8609_rm_templates_regex_backtracking.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 changelogs/fragments/T8609_rm_templates_regex_backtracking.yml (limited to 'changelogs') diff --git a/changelogs/fragments/T8609_rm_templates_regex_backtracking.yml b/changelogs/fragments/T8609_rm_templates_regex_backtracking.yml new file mode 100644 index 00000000..b02dbf19 --- /dev/null +++ b/changelogs/fragments/T8609_rm_templates_regex_backtracking.yml @@ -0,0 +1,12 @@ +--- +bugfixes: + - bgp_address_family.py - Fix slow parse() to stop regex backtracking on prefix-sharing inputs. + - bgp_address_family_14.py - Fix slow parse() to stop regex backtracking on prefix-sharing inputs. + - bgp_global.py - Fix slow parse() to stop regex backtracking on prefix-sharing inputs. + - bgp_global_14.py - Fix slow parse() to stop regex backtracking on prefix-sharing inputs. + - ospf_interfaces.py - Fix slow parse() to stop regex backtracking on prefix-sharing inputs. + - ospf_interfaces_14.py - Fix slow parse() to stop regex backtracking on prefix-sharing inputs. + - route_maps.py - Fix slow parse() to stop regex backtracking on prefix-sharing inputs. + - route_maps_14.py - Fix slow parse() to stop regex backtracking on prefix-sharing inputs. + - snmp_server.py - Fix slow parse() to stop regex backtracking on prefix-sharing inputs. + - test_rm_templates_perf.py - Add unit test for the bugfixes -- cgit v1.2.3