> On Thu, Aug 01, 2024 at 12:27:03AM +0200, Phil Sutter wrote: > > Add a mechanism providing forward compatibility for the current and > > future versions of iptables-nft (and all other nft-variants) by > > annotating nftnl rules with the extensions they were created for. > > > > Upon nftnl rule parsing failure, warn about the situation and perform a > > second attempt loading the respective compat extensions instead of the > > native expressions which replace them. > > OK, so this is last resort to interpret the rule. It is. I had your concerns regarding crafted compat payload in rules in mind with this. The downside is that we may make subtle changes to VM code which the old binary won't detect and ignore. Maybe I could feature it via flag or env var to prefer the userdata extensions. WDYT? > > The foundational assumption is that libxtables extensions are stable > > and thus the VM code created on their behalf does not need to be. > > OK, this requires xtables API becomes frozen forever. Well, not more than it has been: Take iptables-legacy for instance: An old version may see a newer extension revision than it has support for, so will fail just like iptables-nft with native code it can't parse. So effectively iptables-nft becomes *as compatible as* the same version of iptables-legacy. Another perspective to this: Extension development gradually slows down which we'll leverage while at the same time support increased development in nftables itself and conversion of extensions to VM code. > > Since nftnl rule userdata attributes are restricted to 255 bytes, the > > implementation focusses on low memory consumption. Therefore, extensions > > which remain in the rule as compat expressions are not also added to > > userdata. In turn, extensions in userdata are annotated by start and end > > expression number they are replacing. Also, the actual payload is > > zipped using zlib. > > Binary layout is better than storing text in the userdata area. > > Is this zlib approach sufficient to cope with ebtables among > extension? Maybe that one is excluded because it is using the set > infrastructure since the beginning. Yes, among is not an issue because ebtables-nft never implemented this as extension. > I guess you already checked for worst case to make sure compression > always allows to make things fit into 255 bytes? Well, we don't convert too many extensions to nftables anymore since Florian reverted a bunch as a quick countermeasure against the compat complaints. Things will certainly get worse, but the question is mostly how many different extensions will "the largest rule" have. I added some debug output and in shell testsuite for instance the largest compat_ext userdata I see is 68 bytes. I was able to craft a rule which uses 159 bytes though: iptables-nft -A FORWARD \ -m limit --limit 1000/hour \ -p udp -m udp --sport 1024:65535 --dport 4:65535 \ -m mark --mark 0xfeedcafe/0xfeedcafe \ -j NFLOG --nflog-group 23 \ --nflog-prefix "this is a damn long log prefix" The current implementation calls xtables_error() if nftnl_udata_put() fails. Maybe a better error path would be to only warn the user and not add compat_ext to userdata. Guess it depends on whether this should be enabled by default or only upon request - if user asks for compat_ext, failing to do so should be fatal. Cheers, Phil
