[pve-devel] [PATCH proxmox-firewall] firewall: properly cleanup tables when firewall is inactive

Stefan Hanreich s.hanreich at proxmox.com
Tue Apr 23 11:21:39 CEST 2024


When executing multiple nft commands they are transactional, either
all get applied or none. When only the host or guest firewall is
active, only one table exists and this causes the delete commands to
fail. To fix this we need to send the delete commands separately.

It might make sense to support running multiple separate batches in
the NftClient in the future in order to avoid having to call nft
twice.

Signed-off-by: Stefan Hanreich <s.hanreich at proxmox.com>
---
 proxmox-firewall/src/bin/proxmox-firewall.rs |  9 +++++----
 proxmox-firewall/src/firewall.rs             | 10 +++++-----
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs
index 2f4875f..4e07993 100644
--- a/proxmox-firewall/src/bin/proxmox-firewall.rs
+++ b/proxmox-firewall/src/bin/proxmox-firewall.rs
@@ -12,11 +12,12 @@ const RULE_BASE: &str = include_str!("../../resources/proxmox-firewall.nft");
 
 fn remove_firewall() -> Result<(), std::io::Error> {
     log::info!("removing existing firewall rules");
-    let commands = Firewall::remove_commands();
 
-    // can ignore other errors, since it fails when tables do not exist
-    if let Err(NftError::Io(err)) = NftClient::run_json_commands(&commands) {
-        return Err(err);
+    for command in Firewall::remove_commands() {
+        // can ignore other errors, since it fails when tables do not exist
+        if let Err(NftError::Io(err)) = NftClient::run_json_commands(&command) {
+            return Err(err);
+        }
     }
 
     Ok(())
diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs
index 2195a07..b137f58 100644
--- a/proxmox-firewall/src/firewall.rs
+++ b/proxmox-firewall/src/firewall.rs
@@ -157,11 +157,11 @@ impl Firewall {
         }
     }
 
-    pub fn remove_commands() -> Commands {
-        Commands::new(vec![
-            Delete::table(Self::cluster_table()),
-            Delete::table(Self::guest_table()),
-        ])
+    pub fn remove_commands() -> Vec<Commands> {
+        vec![
+            Commands::new(vec![Delete::table(Self::cluster_table())]),
+            Commands::new(vec![Delete::table(Self::guest_table())]),
+        ]
     }
 
     fn create_management_ipset(&self, commands: &mut Commands) -> Result<(), Error> {
-- 
2.39.2




More information about the pve-devel mailing list