On 04.10.2025 15:17, Thomas Lamprecht wrote:
Am 16.09.25 um 11:32 schrieb Gabriel Goller:
+        if let Ok(index) = IPTABLES_ICMP_TYPES_MAPPING.binary_search_by(|v| 
v.0.cmp(s)) {
+            if let Some(mapped_nftables_type) = 
IPTABLES_ICMP_TYPES_MAPPING[index].1 {
+                return Ok(Self::Named(mapped_nftables_type));
+            } else {
+                bail!("icmp_type {s:?} is unsupported in nftables");

How is this Err handled on use sites? As for the bail below I'd be fine
if it causes some further failure down the road, but if it is a supported
type in pve-firewall above should not cause any actual error or rule
generations to get skipped, besides skipping the nftables-unsupported
ones, obviously.

Yeah you're right, I forgot that erroring here in the firewall is not
good; i.e. you only get the error in the journal and no firewall code is
generated.

Also, the linked wiki spots "Last update: Mar/2022" at the top, so a bit
dated. Did you actually try if this is still the case for all of the
listed ones?

Yep, it's still just the listed ones. But the other ones can actually be
implemented as a custom type/code combination, so that we have full coverage.

To be exact, iptables has a few types that can be specified as strings,
but actually correspond to a type and a specific code, e.g.:
'port-unreachable' corresponds to type 3 and code 3,
'destination-unreachable' corresponds to type 3 (and no code), which
means it contains 'port-unreachable'. In nftables they did it right and
completely separated the type and code properties.

For more info see here: 
https://www.iana.org/assignments/icmpv6-parameters/icmpv6-parameters.xhtml#icmpv6-parameters-2

+            }
+        }
+
         bail!("{s:?} is not a valid icmpv6 type");
     }


Thanks for the review!


_______________________________________________
pve-devel mailing list
[email protected]
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to