* [pve-devel] [PATCH proxmox-firewall v2 2/2] firewall: delete unused sets
2024-10-31 10:10 [pve-devel] [PATCH proxmox-firewall v2 1/2] firewall: cargo: use new cargo feature resolver Gabriel Goller
@ 2024-10-31 10:10 ` Gabriel Goller
2024-10-31 13:04 ` [pve-devel] applied: [PATCH proxmox-firewall v2 1/2] firewall: cargo: use new cargo feature resolver Fabian Grünbichler
1 sibling, 0 replies; 4+ messages in thread
From: Gabriel Goller @ 2024-10-31 10:10 UTC (permalink / raw)
To: pve-devel
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.goller@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
^ permalink raw reply [flat|nested] 4+ messages in thread