From: Stefan Hanreich <s.hanreich@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH proxmox-firewall 2/2] firewall: fix ipset lookup for auto-generated ipam ipsets
Date: Thu, 12 Feb 2026 09:48:30 +0100 [thread overview]
Message-ID: <20260212084832.63278-3-s.hanreich@proxmox.com> (raw)
In-Reply-To: <20260212084832.63278-1-s.hanreich@proxmox.com>
In commit 1a917517 the ipset lookup function was adapted such that it
can support both legacy ipset names as well as the new format that
includes scopes. During this change, the function that performs the
lookup for the SDN scope only included ipsets from the SDN
configuration, but not the IPAM configuration. While the IPAM ipsets
were correctly generated, the firewall refused to generate rules that
included the IPAM ipsets, since they were not found in the lookup
function. Adapt the lookup function to consider the IPAM ipsets as
well to fix this issue.
Also add respective rules to the integration tests, which would have
caught this issue.
Reported in the forum by a user [1]
[1] https://forum.proxmox.com/threads/sdn-aliases-not-found-by-firewall.180549/
Fixes: 1a917517
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
proxmox-firewall/src/config.rs | 40 ++-
proxmox-firewall/src/firewall.rs | 5 +-
proxmox-firewall/tests/input/host.fw | 2 +
.../integration_tests__firewall.snap | 250 +++++++++++++++++-
4 files changed, 286 insertions(+), 11 deletions(-)
diff --git a/proxmox-firewall/src/config.rs b/proxmox-firewall/src/config.rs
index 3854f71..11d9119 100644
--- a/proxmox-firewall/src/config.rs
+++ b/proxmox-firewall/src/config.rs
@@ -274,6 +274,21 @@ impl FirewallSdnConfig {
}
}
+pub struct FirewallIpamConfig {
+ _config: Ipam,
+ ipsets: BTreeMap<String, Ipset>,
+}
+
+impl FirewallIpamConfig {
+ pub fn ipsets(&self) -> &BTreeMap<String, Ipset> {
+ &self.ipsets
+ }
+
+ pub fn ipset(&self, name: &str) -> Option<&Ipset> {
+ self.ipsets.get(name)
+ }
+}
+
pub struct FirewallConfig {
cluster_config: ClusterConfig,
host_config: HostConfig,
@@ -281,7 +296,7 @@ pub struct FirewallConfig {
bridge_config: BTreeMap<BridgeName, BridgeConfig>,
nft_config: BTreeMap<String, ListChain>,
sdn_config: Option<FirewallSdnConfig>,
- ipam_config: Option<Ipam>,
+ ipam_config: Option<FirewallIpamConfig>,
interface_mapping: AltnameMapping,
}
@@ -362,11 +377,23 @@ impl FirewallConfig {
})
}
- pub fn parse_ipam(firewall_loader: &dyn FirewallConfigLoader) -> Result<Option<Ipam>, Error> {
+ pub fn parse_ipam(
+ firewall_loader: &dyn FirewallConfigLoader,
+ ) -> Result<Option<FirewallIpamConfig>, Error> {
Ok(match firewall_loader.ipam()? {
Some(data) => {
let raw_ipam: IpamJson = serde_json::from_reader(data)?;
- Some(Ipam::try_from(raw_ipam)?)
+ let ipam = Ipam::try_from(raw_ipam)?;
+
+ let ipsets = ipam
+ .ipsets(None)
+ .map(|ipset| (ipset.name().name().to_string(), ipset))
+ .collect();
+
+ Some(FirewallIpamConfig {
+ _config: ipam,
+ ipsets,
+ })
}
_ => None,
})
@@ -446,7 +473,7 @@ impl FirewallConfig {
self.sdn_config.as_ref()
}
- pub fn ipam(&self) -> Option<&Ipam> {
+ pub fn ipam(&self) -> Option<&FirewallIpamConfig> {
self.ipam_config.as_ref()
}
@@ -497,7 +524,10 @@ impl FirewallConfig {
match name {
RuleIpsetName::Scoped(ipset_name) => match ipset_name.scope() {
- IpsetScope::Sdn => self.sdn()?.ipset(ipset_name.name()),
+ IpsetScope::Sdn => self
+ .sdn()?
+ .ipset(ipset_name.name())
+ .or_else(|| self.ipam()?.ipset(ipset_name.name())),
IpsetScope::Datacenter => self.cluster().ipset(ipset_name.name()),
IpsetScope::Guest => {
vmid.and_then(|vmid| self.guest_ipset(ipset_name.name(), vmid))
diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs
index 58f4aec..65da889 100644
--- a/proxmox-firewall/src/firewall.rs
+++ b/proxmox-firewall/src/firewall.rs
@@ -249,10 +249,7 @@ impl Firewall {
}
if let Some(ipam_config) = self.config.ipam() {
- let ipsets = ipam_config
- .ipsets(None)
- .map(|ipset| (ipset.name().to_string(), ipset))
- .collect();
+ let ipsets = ipam_config.ipsets();
self.create_ipsets(&mut commands, &ipsets, &cluster_host_table, None)?;
self.create_ipsets(&mut commands, &ipsets, &guest_table, None)?;
diff --git a/proxmox-firewall/tests/input/host.fw b/proxmox-firewall/tests/input/host.fw
index 7b89aad..6b117f6 100644
--- a/proxmox-firewall/tests/input/host.fw
+++ b/proxmox-firewall/tests/input/host.fw
@@ -24,4 +24,6 @@ IN ACCEPT --icmp-type neighbor-solicitation --proto ipv6-icmp --log info
IN Ping(REJECT)
IN REJECT -p udp --dport 443
OUT REJECT -p udp --dport 443
+FORWARD DROP --source +sdn/guest-ipam-101 --dest +sdn/guest-ipam-101
+FORWARD DROP --source +sdn/public-all --dest +sdn/public-gateway
diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
index 79cb882..3157473 100644
--- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
+++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
@@ -1,8 +1,6 @@
---
source: proxmox-firewall/tests/integration_tests.rs
-assertion_line: 127
expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
-snapshot_kind: text
---
{
"nftables": [
@@ -4073,6 +4071,254 @@ snapshot_kind: text
}
}
},
+ {
+ "add": {
+ "rule": {
+ "family": "inet",
+ "table": "proxmox-firewall",
+ "chain": "host-forward",
+ "expr": [
+ {
+ "match": {
+ "op": "==",
+ "left": {
+ "payload": {
+ "protocol": "ip",
+ "field": "saddr"
+ }
+ },
+ "right": "@v4-sdn/guest-ipam-101"
+ }
+ },
+ {
+ "match": {
+ "op": "!=",
+ "left": {
+ "payload": {
+ "protocol": "ip",
+ "field": "saddr"
+ }
+ },
+ "right": "@v4-sdn/guest-ipam-101-nomatch"
+ }
+ },
+ {
+ "match": {
+ "op": "==",
+ "left": {
+ "payload": {
+ "protocol": "ip",
+ "field": "daddr"
+ }
+ },
+ "right": "@v4-sdn/guest-ipam-101"
+ }
+ },
+ {
+ "match": {
+ "op": "!=",
+ "left": {
+ "payload": {
+ "protocol": "ip",
+ "field": "daddr"
+ }
+ },
+ "right": "@v4-sdn/guest-ipam-101-nomatch"
+ }
+ },
+ {
+ "drop": null
+ }
+ ]
+ }
+ }
+ },
+ {
+ "add": {
+ "rule": {
+ "family": "inet",
+ "table": "proxmox-firewall",
+ "chain": "host-forward",
+ "expr": [
+ {
+ "match": {
+ "op": "==",
+ "left": {
+ "payload": {
+ "protocol": "ip6",
+ "field": "saddr"
+ }
+ },
+ "right": "@v6-sdn/guest-ipam-101"
+ }
+ },
+ {
+ "match": {
+ "op": "!=",
+ "left": {
+ "payload": {
+ "protocol": "ip6",
+ "field": "saddr"
+ }
+ },
+ "right": "@v6-sdn/guest-ipam-101-nomatch"
+ }
+ },
+ {
+ "match": {
+ "op": "==",
+ "left": {
+ "payload": {
+ "protocol": "ip6",
+ "field": "daddr"
+ }
+ },
+ "right": "@v6-sdn/guest-ipam-101"
+ }
+ },
+ {
+ "match": {
+ "op": "!=",
+ "left": {
+ "payload": {
+ "protocol": "ip6",
+ "field": "daddr"
+ }
+ },
+ "right": "@v6-sdn/guest-ipam-101-nomatch"
+ }
+ },
+ {
+ "drop": null
+ }
+ ]
+ }
+ }
+ },
+ {
+ "add": {
+ "rule": {
+ "family": "inet",
+ "table": "proxmox-firewall",
+ "chain": "host-forward",
+ "expr": [
+ {
+ "match": {
+ "op": "==",
+ "left": {
+ "payload": {
+ "protocol": "ip",
+ "field": "saddr"
+ }
+ },
+ "right": "@v4-sdn/public-all"
+ }
+ },
+ {
+ "match": {
+ "op": "!=",
+ "left": {
+ "payload": {
+ "protocol": "ip",
+ "field": "saddr"
+ }
+ },
+ "right": "@v4-sdn/public-all-nomatch"
+ }
+ },
+ {
+ "match": {
+ "op": "==",
+ "left": {
+ "payload": {
+ "protocol": "ip",
+ "field": "daddr"
+ }
+ },
+ "right": "@v4-sdn/public-gateway"
+ }
+ },
+ {
+ "match": {
+ "op": "!=",
+ "left": {
+ "payload": {
+ "protocol": "ip",
+ "field": "daddr"
+ }
+ },
+ "right": "@v4-sdn/public-gateway-nomatch"
+ }
+ },
+ {
+ "drop": null
+ }
+ ]
+ }
+ }
+ },
+ {
+ "add": {
+ "rule": {
+ "family": "inet",
+ "table": "proxmox-firewall",
+ "chain": "host-forward",
+ "expr": [
+ {
+ "match": {
+ "op": "==",
+ "left": {
+ "payload": {
+ "protocol": "ip6",
+ "field": "saddr"
+ }
+ },
+ "right": "@v6-sdn/public-all"
+ }
+ },
+ {
+ "match": {
+ "op": "!=",
+ "left": {
+ "payload": {
+ "protocol": "ip6",
+ "field": "saddr"
+ }
+ },
+ "right": "@v6-sdn/public-all-nomatch"
+ }
+ },
+ {
+ "match": {
+ "op": "==",
+ "left": {
+ "payload": {
+ "protocol": "ip6",
+ "field": "daddr"
+ }
+ },
+ "right": "@v6-sdn/public-gateway"
+ }
+ },
+ {
+ "match": {
+ "op": "!=",
+ "left": {
+ "payload": {
+ "protocol": "ip6",
+ "field": "daddr"
+ }
+ },
+ "right": "@v6-sdn/public-gateway-nomatch"
+ }
+ },
+ {
+ "drop": null
+ }
+ ]
+ }
+ }
+ },
{
"add": {
"set": {
--
2.47.3
next prev parent reply other threads:[~2026-02-12 8:48 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-12 8:48 [PATCH proxmox-firewall 0/2] Fix auto-generated IPAM ipsets in firewall Stefan Hanreich
2026-02-12 8:48 ` [PATCH proxmox-firewall 1/2] firewall: chore: autoformat imports Stefan Hanreich
2026-02-12 8:48 ` Stefan Hanreich [this message]
2026-02-16 19:22 ` applied: [PATCH proxmox-firewall 0/2] Fix auto-generated IPAM ipsets in firewall Thomas Lamprecht
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260212084832.63278-3-s.hanreich@proxmox.com \
--to=s.hanreich@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.