On 01.10.2025 18:46, Stefan Hanreich wrote:
On 9/16/25 11:32 AM, Gabriel Goller wrote:
nftables changed the names of the icmpv6-types and they don't overlap
completely with the old iptables names. Introduce a mapping that
converts old names into the new ones. A few of these are not supported,
see here for more info:
https://wiki.nftables.org/wiki-nftables/index.php/Supported_features_compared_to_xtables#icmp6

Did you find a reasoning for that? Are they not in use anymore /
deprecated? Then I guess we should not make that a hard error, but
possibly a warning and soft failure? In the other case (still in use), I
think we should still try to generate rules for them.

Since those are configurations that users can have pre-existing, we
should handle them gracefully instead of just erroring out on
encountering them.

You're right I forgot erroring here in the firewall is not good.

There are even other possible values that are still not considered here
like 'TOS-network-unreachable'. Since they are all mappable to a numeric
type/code combo - we should take all possible values for the field [1]
[2] to preserve compatibility with existing configurations?

Not sure if they're accurate, but pve-manager seems to have the
respective information on type / code combinations [3]. Can take a
closer look at it and send a follow-up.

Yep, I introced a mapping to type/code for icmpv6 and icmp types as
well!

Not sure if this is a blocker, it might be a bit too obscure / niche to
prevent this series from getting merged... - can always just do a follow-up.

[1]
https://git.proxmox.com/?p=pve-firewall.git;a=blob;f=src/PVE/Firewall.pm;h=49430b174bb2fdd56ce586f90bf929c5648f9060;hb=HEAD#l785
[2]
https://git.proxmox.com/?p=pve-firewall.git;a=blob;f=src/PVE/Firewall.pm;h=49430b174bb2fdd56ce586f90bf929c5648f9060;hb=HEAD#l826
[3]
https://git.proxmox.com/?p=pve-manager.git;a=blob;f=www/manager6/grid/FirewallRules.js;h=0db817ebce0e9254d18f172a6e02a7a12e7a481c;hb=HEAD#l83


Thanks for the review!


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

Reply via email to