Delete unused sets in nftables. We check if there are existing sets which will not be recreated on the next iteration and delete them.
Signed-off-by: Gabriel Goller <g.gol...@proxmox.com> --- v2, thanks @Stefan: - moved delete_unused_sets function and invocation into Firewall struct – this makes the bin files much cleaner proxmox-firewall/src/firewall.rs | 61 ++++++++++++++++++++++++++++++-- proxmox-nftables/src/client.rs | 9 +++-- proxmox-nftables/src/types.rs | 33 ++++++++++++++--- 3 files changed, 94 insertions(+), 9 deletions(-) diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs index bb5402393a1f..a457de7cd88d 100644 --- a/proxmox-firewall/src/firewall.rs +++ b/proxmox-firewall/src/firewall.rs @@ -1,9 +1,10 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; use std::fs; use anyhow::{bail, Error}; -use proxmox_nftables::command::{Add, Commands, Delete, Flush}; +use proxmox_nftables::client::NftError; +use proxmox_nftables::command::{Add, Commands, Delete, Flush, List, ListOutput}; use proxmox_nftables::expression::{Meta, Payload}; use proxmox_nftables::helper::NfVec; use proxmox_nftables::statement::{AnonymousLimit, Log, LogLevel, Match, Set, SetOperation}; @@ -11,7 +12,7 @@ use proxmox_nftables::types::{ AddElement, AddRule, ChainPart, MapValue, RateTimescale, SetName, TableFamily, TableName, TablePart, Verdict, }; -use proxmox_nftables::{Expression, Statement}; +use proxmox_nftables::{Command, Expression, NftClient, Statement}; use proxmox_ve_config::host::types::BridgeName; @@ -201,6 +202,55 @@ impl Firewall { Ok(()) } + /// Fetch all the sets that currently exist, then get all the sets that will be added in the next + /// interation. Make the difference between them and remove the unused ones. + fn delete_unused_sets(&self, commands: &Commands) -> Result<Commands, NftError> { + let mut delete_commands = Commands::default(); + + // get sets that will be added + let new_sets = commands + .iter() + .filter_map(|w| match w { + Command::Add(Add::Set(x)) => Some(x.config.name()), + _ => None, + }) + .collect::<HashSet<&SetName>>(); + + // get existing sets + let list_commands = Commands::new(vec![List::sets()]); + let existing_sets = NftClient::run_json_commands(&list_commands); + match existing_sets { + Ok(Some(existing_sets)) => { + let existing_sets = existing_sets + .nftables + .iter() + .filter_map(|item| { + if let ListOutput::Set(e) = item { + Some(e.name()) + } else { + None + } + }) + .collect::<HashSet<&SetName>>(); + let to_delete = existing_sets.difference(&new_sets); + + log::debug!("existing sets: {:#?}", existing_sets); + log::debug!("new sets: {:#?}", new_sets); + log::debug!("these sets are going to be deleted: {:#?}", to_delete); + + for set in to_delete.into_iter().cloned().cloned() { + delete_commands.push(Command::Delete(Delete::Set(set))); + } + Ok(delete_commands) + } + Ok(None) => { + log::debug!("no sets exist"); + Ok(delete_commands) + } + Err(err) => Err(err), + } + } + pub fn full_host_fw(&self) -> Result<Commands, Error> { let mut commands = Commands::default(); @@ -344,6 +394,11 @@ impl Firewall { self.create_bridge_chain(&mut commands, bridge_name, bridge_config)?; } + match self.delete_unused_sets(&commands) { + Ok(mut delete_commands) => commands.append(&mut delete_commands), + Err(err) => log::error!("error deleting unused sets {err:?}"), + } + Ok(commands) } diff --git a/proxmox-nftables/src/client.rs b/proxmox-nftables/src/client.rs index eaa3dd2136ee..0fd950cdc1e1 100644 --- a/proxmox-nftables/src/client.rs +++ b/proxmox-nftables/src/client.rs @@ -52,8 +52,13 @@ impl NftClient { let output = Self::execute_nft_commands(true, &json)?; if !output.is_empty() { - let parsed_output: Option<CommandOutput> = serde_json::from_str(&output).ok(); - return Ok(parsed_output); + return match serde_json::from_str::<CommandOutput>(&output) { + Ok(output) => Ok(Some(output)), + Err(err) => { + log::error!("Error deserializing output: {err:#?}"); + Ok(None) + } + }; } Ok(None) diff --git a/proxmox-nftables/src/types.rs b/proxmox-nftables/src/types.rs index 320c757c7cba..258bfc899734 100644 --- a/proxmox-nftables/src/types.rs +++ b/proxmox-nftables/src/types.rs @@ -19,7 +19,7 @@ use proxmox_ve_config::guest::types::Vmid; #[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] pub struct Handle(i32); -#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize, Hash)] #[serde(rename_all = "lowercase")] pub enum TableFamily { Ip, @@ -210,7 +210,7 @@ impl TableName { } } -#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct TablePart { family: TableFamily, table: String, @@ -564,7 +564,7 @@ impl DerefMut for AddMap { #[derive(Clone, Debug, Deserialize, Serialize)] pub struct AddSet { #[serde(flatten)] - config: SetConfig, + pub config: SetConfig, #[serde(default, skip_serializing_if = "Vec::is_empty")] elem: NfVec<SetElem>, @@ -602,7 +602,7 @@ impl AddSet { } } -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, Hash)] pub struct SetName { #[serde(flatten)] table: TablePart, @@ -909,9 +909,34 @@ impl ListChain { } #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(rename_all = "kebab-case")] pub struct ListSet { #[serde(flatten)] name: SetName, + + #[serde(rename = "type", default, skip_serializing_if = "Vec::is_empty")] + ty: NfVec<ElementType>, + + #[serde(skip_serializing_if = "Option::is_none")] + policy: Option<SetPolicy>, + + #[serde(skip_serializing_if = "Vec::is_empty", default)] + flags: Vec<SetFlag>, + + #[serde(skip_serializing_if = "Option::is_none")] + handle: Option<i64>, + + #[serde(skip_serializing_if = "Option::is_none")] + elem: Option<NfVec<Expression>>, + + #[serde(skip_serializing_if = "Option::is_none")] + timeout: Option<i64>, + + #[serde(skip_serializing_if = "Option::is_none")] + gc_interval: Option<i64>, + + #[serde(skip_serializing_if = "Option::is_none")] + size: Option<i64>, } impl ListSet { -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel