From c8d75f88f078c8c2c2a1f6a9a62c937424279d4b Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Wed, 7 Oct 2026 19:59:13 +0200 Subject: frrender: T9368: reconcile FRR after a DHCP lease without the commit socket Reconciling the FRR configuration after a DHCP lease event was requested from the configuration daemon over the socket the commit pipeline uses. The daemon serves one client at a time and assumes it is the running commit, so the request either landed in the middle of the handshake which hands over the configuration, or kept the daemon busy past the 10ms a commit waits for it. Either way the commit fell back to running the conf-mode scripts directly. Those never render FRR, only the daemon does, so the commit succeeded while FRR kept the configuration rendered before it, until the next lease event. A lease event now renders FRR in its own process, and a commit which could not use the daemon renders FRR itself instead of skipping the reload. Both take the same lock. A commit post-hook would remove the second case, for a full render on every commit. --- .../98-vyos-static-routes-dhclient-hook | 5 +- .../98-vyos-static-routes-dhclient-hook | 4 +- src/helpers/vyos-frr-render.py | 58 ++++++++++++++++++++ src/helpers/vyos-request-configd-update.py | 31 ----------- src/services/vyos-commitd | 12 +++-- src/services/vyos-configd | 12 +++-- src/shim/vyshim.c | 63 ++++++++++++++++++++-- 7 files changed, 135 insertions(+), 50 deletions(-) create mode 100755 src/helpers/vyos-frr-render.py delete mode 100755 src/helpers/vyos-request-configd-update.py (limited to 'src') diff --git a/src/etc/dhcp/dhclient-enter-hooks.d/98-vyos-static-routes-dhclient-hook b/src/etc/dhcp/dhclient-enter-hooks.d/98-vyos-static-routes-dhclient-hook index 439a8dd87..f3b1b35f1 100755 --- a/src/etc/dhcp/dhclient-enter-hooks.d/98-vyos-static-routes-dhclient-hook +++ b/src/etc/dhcp/dhclient-enter-hooks.d/98-vyos-static-routes-dhclient-hook @@ -28,6 +28,7 @@ fi # - RELEASE: lease released, remove routes # - STOP: dhclient stopped, remove routes if [ "$reason" == "PREINIT" ] || [ "$reason" == "EXPIRE" ] || [ "$reason" == "FAIL" ] || [ "$reason" == "RELEASE" ] || [ "$reason" == "STOP" ]; then - # Re-generate static routes config to remove routes that depend on this interface - sudo /usr/libexec/vyos/vyos-request-configd-update.py + # Reconcile the rendered FRR configuration - routes depending on this + # interface must be removed + sudo /usr/libexec/vyos/vyos-frr-render.py fi diff --git a/src/etc/dhcp/dhclient-exit-hooks.d/98-vyos-static-routes-dhclient-hook b/src/etc/dhcp/dhclient-exit-hooks.d/98-vyos-static-routes-dhclient-hook index 4f207411e..2c895ffd6 100755 --- a/src/etc/dhcp/dhclient-exit-hooks.d/98-vyos-static-routes-dhclient-hook +++ b/src/etc/dhcp/dhclient-exit-hooks.d/98-vyos-static-routes-dhclient-hook @@ -34,5 +34,5 @@ elif [ "$reason" != "BOUND" ] && [ "$reason" != "EXPIRE" ] && [ "$reason" != "RE return 0 fi -# Re-generate the static routes config -sudo /usr/libexec/vyos/vyos-request-configd-update.py +# Reconcile the rendered FRR configuration with the current lease +sudo /usr/libexec/vyos/vyos-frr-render.py diff --git a/src/helpers/vyos-frr-render.py b/src/helpers/vyos-frr-render.py new file mode 100755 index 000000000..739b56410 --- /dev/null +++ b/src/helpers/vyos-frr-render.py @@ -0,0 +1,58 @@ +#!/usr/bin/env python3 +# +# Copyright (C) VyOS Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 2 or later as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +"""Render the FRR configuration and reload FRR without vyos-configd. + +Only vyos-configd renders FRR, and only at the end of a commit it handled +itself. Two paths need a render without it: a DHCP lease event, which changes +the rendered configuration without any CLI change, and a commit which fell back +to running the conf-mode scripts directly.""" + +import sys + +from vyos.config import Config +from vyos.frrender import FRRender +from vyos.frrender import frr_applied_config_file +from vyos.frrender import frr_config_file +from vyos.frrender import frr_render_lock +from vyos.frrender import get_frrender_dict +from vyos.utils.file import read_file +from vyos import ConfigError + + +def render() -> None: + """Reload FRR if the rendered configuration differs from the one FRR was + last reloaded with. + + A fresh instance has no cached configuration, thus its own change detection + can not be used here. The rendered file is no substitute for it - a render + which failed before the reload leaves it unapplied.""" + frr = FRRender() + frr.generate(get_frrender_dict(Config())) + + if read_file(frr_config_file) == read_file(frr_applied_config_file, None): + return + + frr.apply() + + +if __name__ == '__main__': + try: + with frr_render_lock(): + render() + except ConfigError as e: + print(e) + sys.exit(1) diff --git a/src/helpers/vyos-request-configd-update.py b/src/helpers/vyos-request-configd-update.py deleted file mode 100755 index 55ed0eef9..000000000 --- a/src/helpers/vyos-request-configd-update.py +++ /dev/null @@ -1,31 +0,0 @@ -#!/usr/bin/env python3 - -import json -import zmq - -from vyos.utils.commit import wait_for_commit_lock -from vyos.defaults import vyos_configd_socket_path - -context = zmq.Context() - -request = { - 'type': 'node', - 'last': True, - 'data': '/usr/libexec/vyos/conf_mode/protocols_static.py', -} -request = json.dumps(request) - -print("Waiting for commit lock...") -wait_for_commit_lock() - -print("Connecting to vyos-configd server...") -socket = context.socket(zmq.REQ) -socket.connect(vyos_configd_socket_path) - -print(f"Sending request {request}...") -socket.send_string(request) - -message = socket.recv() -print(f"Received reply {request} [ {message} ]") - -print("All done") diff --git a/src/services/vyos-commitd b/src/services/vyos-commitd index b8e430b93..8a515446b 100755 --- a/src/services/vyos-commitd +++ b/src/services/vyos-commitd @@ -45,6 +45,7 @@ from vyos.configsource import ConfigSourceError from vyos.configdiff import get_commit_scripts from vyos.config import Config from vyos.frrender import FRRender +from vyos.frrender import frr_render_lock from vyos.frrender import get_frrender_dict from vyos import ConfigError @@ -293,11 +294,12 @@ def call_frr_render(frr, config): def _call_frr_render(frr, config): # pylint: disable=broad-exception-caught try: - tmp = get_frrender_dict(config) - if frr.generate(tmp): - # only apply a new FRR configuration if anything changed - # in comparison to the previous applied configuration - frr.apply() + with frr_render_lock(): + tmp = get_frrender_dict(config) + if frr.generate(tmp): + # only apply a new FRR configuration if anything changed + # in comparison to the previous applied configuration + frr.apply() except ConfigError as e: logger.error(e) diff --git a/src/services/vyos-configd b/src/services/vyos-configd index 32d773f78..8e0b1adec 100755 --- a/src/services/vyos-configd +++ b/src/services/vyos-configd @@ -39,6 +39,7 @@ from vyos.configsource import ConfigSourceError from vyos.configdiff import get_commit_scripts from vyos.config import Config from vyos.frrender import FRRender +from vyos.frrender import frr_render_lock from vyos.frrender import get_frrender_dict from vyos import ConfigError from vyos.configmanager import ConfigManager @@ -250,11 +251,12 @@ def call_frr_render(frr, config): def _call_frr_render(frr, config): # pylint: disable=broad-exception-caught try: - tmp = get_frrender_dict(config) - if frr.generate(tmp): - # only apply a new FRR configuration if anything changed - # in comparison to the previous applied configuration - frr.apply() + with frr_render_lock(): + tmp = get_frrender_dict(config) + if frr.generate(tmp): + # only apply a new FRR configuration if anything changed + # in comparison to the previous applied configuration + frr.apply() except ConfigError as e: logger.error(e) diff --git a/src/shim/vyshim.c b/src/shim/vyshim.c index 7b516cd46..81a1f8b49 100644 --- a/src/shim/vyshim.c +++ b/src/shim/vyshim.c @@ -53,6 +53,8 @@ #define COMMIT_MARKER "/var/tmp/initial_in_commit" #define QUEUE_MARKER "/var/tmp/last_in_queue" +#define FRR_RENDER "/usr/libexec/vyos/vyos-frr-render.py" + enum { SUCCESS = 1 << 0, ERROR_COMMIT = 1 << 1, @@ -66,6 +68,7 @@ volatile int timeout = 0; int initialization(void *, char *); int pass_through(char **, int); +int frr_render(char *); void timer_handler(int); void leave_hint(char *); @@ -107,6 +110,13 @@ int main(int argc, char* argv[]) strsep(&pid_str, "_"); debug_print("config session pid: %s\n", pid_str); + // Consume the marker before talking to the daemon, else it is left behind + // for the next commit when the daemon can not be used here + if (access(QUEUE_MARKER, F_OK) != -1) { + last = 1; + remove(QUEUE_MARKER); + } + if (access(COMMIT_MARKER, F_OK) != -1) { init_timeout = initialization(requester, pid_str); if (!init_timeout) remove(COMMIT_MARKER); @@ -115,14 +125,12 @@ int main(int argc, char* argv[]) // if initial communication failed, pass through execution of script if (init_timeout) { int ret = pass_through(argv, ex_index); + // The conf-mode scripts do not render FRR, only the daemon does - + // without this the FRR reload of this commit is silently skipped + if (last && frr_render(pid_str) != 0) ret = -1; return ret; } - if (access(QUEUE_MARKER, F_OK) != -1) { - last = 1; - remove(QUEUE_MARKER); - } - char error_code[1]; debug_print("Sending node data ...\n"); char *string_node_data_msg = mkjson(MKJSON_OBJ, 3, @@ -159,6 +167,11 @@ int main(int argc, char* argv[]) ret = pass_through(argv, ex_index); } + // The daemon renders FRR only for a last script it handled itself + if (last && (err & (PASS | ERROR_DAEMON))) { + if (frr_render(pid_str) != 0) ret = -1; + } + if (err & ERROR_COMMIT) { debug_print("Received ERROR_COMMIT\n"); ret = -1; @@ -344,6 +357,46 @@ int pass_through(char **argv, int ex_index) return 0; } +int frr_render(char *pid_val) +{ + pid_t child_pid; + int status; + + // Nothing to fall back to, e.g. during a partial package upgrade + if (access(FRR_RENDER, X_OK) == -1) { + debug_print("%s is not available\n", FRR_RENDER); + return 0; + } + + debug_print("rendering FRR configuration without the daemon\n"); + + if ((child_pid=fork()) < 0) { + debug_print("frr_render fork() failed\n"); + return -1; + } else if (child_pid == 0) { + char *newargv[] = { FRR_RENDER, NULL }; + if (-1 == execv(FRR_RENDER, newargv)) { + fprintf(stderr, "frr_render execv failed %s: %s\n", + FRR_RENDER, strerror(errno)); + exit(EXIT_FAILURE); + } + } + + if (waitpid(child_pid, &status, 0) != child_pid) { + debug_print("frr_render waitpid() failed\n"); + return -1; + } + + if (WIFEXITED(status) && WEXITSTATUS(status) == 0) { + return 0; + } + + // Same treatment as an apply error reported by the daemon + leave_hint(pid_val); + + return -1; +} + void timer_handler(int signum) { debug_print("timer_handler invoked\n"); -- cgit v1.2.3 From 8cfc171402bbe8fbc67bea3b579a27d7e0d78ac6 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Wed, 7 Oct 2026 19:59:15 +0200 Subject: pppoe: T9395: apply the configuration being committed when a session returns A PPPoE session is restarted from within the commit that changes it and comes back up milliseconds later, while that commit is still running. The hook which then configures the interface reads the running configuration, which until the commit finishes is still the one from before it. Everything the commit adds is missing at that point: for an interface newly set to IPv6 autoconf the hook disabled router advertisements and autoconfiguration on the link, and nothing turned them back on. The link kept its link-local address and never received a global one, until the next commit or reconnect. Which interface is hit varies with the order the commit happens to process them, so this surfaced as an intermittent failure. The hook now waits for the commit to finish before reading the configuration. --- src/etc/ppp/ip-up.d/99-vyos-pppoe-callback | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'src') diff --git a/src/etc/ppp/ip-up.d/99-vyos-pppoe-callback b/src/etc/ppp/ip-up.d/99-vyos-pppoe-callback index f3efb9ac0..9decfcbdd 100755 --- a/src/etc/ppp/ip-up.d/99-vyos-pppoe-callback +++ b/src/etc/ppp/ip-up.d/99-vyos-pppoe-callback @@ -26,6 +26,7 @@ from sys import exit from vyos.configquery import ConfigTreeQuery from vyos.configdict import get_interface_dict from vyos.ifconfig import PPPoEIf +from vyos.utils.commit import wait_for_commit_lock # When the ppp link comes up, this script is called with the following # parameters @@ -41,6 +42,12 @@ if (len(argv) < 7): interface = argv[6] +# The session is re-established from within a commit and can come back up +# before that commit finished. The configuration read below is the running +# one, which until then still is the configuration from before the commit - +# applying it would undo what is being committed right now +wait_for_commit_lock() + conf = ConfigTreeQuery() _, pppoe = get_interface_dict(conf.config, ['interfaces', 'pppoe'], interface) -- cgit v1.2.3