* [pve-devel] [PATCH proxmox-firewall 2/2] firewall: delete unused sets
2024-10-30 14:12 [pve-devel] [PATCH proxmox-firewall 1/2] firewall: cargo: use new cargo feature resolver Gabriel Goller
@ 2024-10-30 14:12 ` Gabriel Goller
2024-10-31 10:09 ` [pve-devel] [PATCH proxmox-firewall 1/2] firewall: cargo: use new cargo feature resolver Gabriel Goller
1 sibling, 0 replies; 3+ messages in thread
From: Gabriel Goller @ 2024-10-30 14:12 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>
---
proxmox-firewall/src/bin/proxmox-firewall.rs | 60 +++++++++++++++++++-
proxmox-nftables/src/client.rs | 9 ++-
proxmox-nftables/src/types.rs | 33 +++++++++--
3 files changed, 95 insertions(+), 7 deletions(-)
diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs
index 4732e51984b5..5871da677067 100644
--- a/proxmox-firewall/src/bin/proxmox-firewall.rs
+++ b/proxmox-firewall/src/bin/proxmox-firewall.rs
@@ -1,3 +1,4 @@
+use std::collections::HashSet;
use std::io::Write;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;
@@ -7,6 +8,9 @@ use anyhow::{Context, Error};
use proxmox_firewall::config::{FirewallConfig, PveFirewallConfigLoader, PveNftConfigLoader};
use proxmox_firewall::firewall::Firewall;
+use proxmox_nftables::command::{Add, Commands, Delete, List, ListOutput};
+use proxmox_nftables::types::SetName;
+use proxmox_nftables::Command;
use proxmox_nftables::{client::NftError, NftClient};
const RULE_BASE: &str = include_str!("../../resources/proxmox-firewall.nft");
@@ -26,6 +30,55 @@ fn remove_firewall() -> Result<(), std::io::Error> {
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(commands: &Commands) -> Result<Commands, NftError> {
+ let mut delete_commands = Commands::new(vec![]);
+
+ // 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),
+ }
+}
+
fn handle_firewall() -> Result<(), Error> {
let config = FirewallConfig::new(&PveFirewallConfigLoader::new(), &PveNftConfigLoader::new())?;
@@ -38,13 +91,18 @@ fn handle_firewall() -> Result<(), Error> {
log::info!("creating the firewall skeleton");
NftClient::run_commands(RULE_BASE)?;
- let commands = firewall.full_host_fw()?;
+ let mut commands = firewall.full_host_fw()?;
log::info!("Running proxmox-firewall commands");
for (idx, c) in commands.iter().enumerate() {
log::debug!("cmd #{idx} {}", serde_json::to_string(&c)?);
}
+ match delete_unused_sets(&commands) {
+ Ok(mut delete_commands) => commands.append(&mut delete_commands),
+ Err(err) => log::error!("error deleting unused sets {err:?}"),
+ }
+
let response = NftClient::run_json_commands(&commands)?;
if let Some(output) = response {
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] 3+ messages in thread