On Fri, May 05, 2023 at 02:06:12PM -0400, Laine Stump wrote: > On 5/4/23 6:44 AM, Daniel P. Berrangé wrote: > > On Sun, Apr 30, 2023 at 11:19:33PM -0400, Laine Stump wrote: > > > In the past virFirewall required all rollback rules for a group (those > > > commands necessary to "undo" any rules that had been added in that > > > group in case of a later failure) to be manually added by switching > > > into "rollback mode" and then re-calling the inverse of the exact > > > virFirewallAddRule*() APIs that had been called to add the original > > > rules (ie. for each --insert command, for rollback we would need to > > > add a rule with all arguments identical except that "--insert" would > > > be replaced by "--delete"). > > > > > > Because nftables can't search for rules to remove by comparing all the > > > arguments (it instead expects *only* a handle that was issued when the > > > rule was originally added), we want for the backends' vir*ApplyRule() > > > functions to be able to automatically add a single rollback rule to > > > the virFirewall object while applying its existing rules (this > > > automatically added rule would then be able to include the handle > > > returned by "nft add rule"). > > > > I think the mistake here is that we're trying to replicate the > > iptables approach for rules 1:1. > > Well, my idea was to *initially* replicate it 1:1 so that we could more > easily verify we haven't changed behavior in some way that we might miss > during any testing, but in a way that we could also easily change it later. > > > > > This was required in iptables world because there was only a single > > global set of tables. libvirt's rules were mixed in with rules from > > non-libvirt apps. Libvirt's rules for different virtual networks also > > had to be mixed together, as we needed special ordering for the > > forward rules. > > > > With nft we can drastically simplify this all with two changes to > > our approach > > > > * Each virtual network should have a top level chain > > ie instead of > > > > table ip libvirt > > > > we should have > > > > table ip libvirt_net_default > > My understanding has always been that each packet must get an ACCEPT result > from *all* of the tables, and if this was the case, then what you're > suggesting wouldn't work. Hmmm, actually, you might be right. I'll have to think about this some more, as I sure would love to have the vnet rules independant of each other. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
