Re: [pve-devel] [PATCH ve-rs 1/2] fix: firewall: introduce iptables to nftables mapping for icmpv6-types

[Gabriel Goller <g.goller@proxmox.com>]

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
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel