On 10/5/25 2:58 AM, Roman Bogorodskiy wrote:
Current implementation uses a single command to flush the old rules and create new ones. This is not optimal because if flush fails for some non-critical reasons (e.g. because the anchor didn't previously exist), it will block rules creation and network start.
Split this command into two: one for flush, and one for rules creation. Also, don't fail if the flush command fails.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Laine Stump <laine@redhat.com>
--- src/network/network_pf.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/network/network_pf.c b/src/network/network_pf.c index ce4461c999..a39eef5fa2 100644 --- a/src/network/network_pf.c +++ b/src/network/network_pf.c @@ -168,6 +168,7 @@ pfAddNatFirewallRules(virNetworkDef *def, g_autofree const char *forwardIf = g_strdup(virNetworkDefForwardIf(def, 0)); g_auto(virBuffer) pf_rules_buf = VIR_BUFFER_INITIALIZER; g_autoptr(virCommand) cmd = virCommandNew(PFCTL); + g_autoptr(virCommand) flush_cmd = virCommandNew(PFCTL); virPortRange *portRange = &def->forward.port; g_autofree char *portRangeStr = NULL;
@@ -240,13 +241,25 @@ pfAddNatFirewallRules(virNetworkDef *def, "block on %s\n", def->bridge);
- /* pfctl -a libvirt/default -F all -f - */ + /* pfctl -a libvirt/default -f - */ virCommandAddArg(cmd, "-a"); virCommandAddArgFormat(cmd, "libvirt/%s", def->name); - virCommandAddArgList(cmd, "-F", "all", "-f", "-", NULL); + virCommandAddArgList(cmd, "-f", "-", NULL);
virCommandSetInputBuffer(cmd, virBufferContentAndReset(&pf_rules_buf));
+ /* pfctl -a libvirt/default -F all */ + /* Flush rules as a separate command, so when it fails, e.g. because the + * anchor didn't exist, we still proceed with rules creation */ + virCommandAddArg(flush_cmd, "-a"); + virCommandAddArgFormat(flush_cmd, "libvirt/%s", def->name); + virCommandAddArgList(flush_cmd, "-F", "all", NULL); + + if (virCommandRun(flush_cmd, NULL) < 0) { + VIR_WARN("Failed to flush firewall rules for network %s", + def->name); + } + if (virCommandRun(cmd, NULL) < 0) { VIR_WARN("Failed to create firewall rules for network %s", def->name);