On Tue, May 21, 2024 at 03:40:54PM -0400, Laine Stump wrote: > On 5/17/24 1:30 PM, Laine Stump wrote: > > + virFirewallAddCmd(fw, layer, "insert", "rule", > > + nftablesLayerTypeToString(layer), > > + VIR_NFTABLES_PRIVATE_TABLE, > > + VIR_NFTABLES_FWD_X_CHAIN, > > + "iifname", iface, > > + "oifname", iface, > > + "counter", "accept", > > + NULL); > > I just found out that if we use "iif" and "oif" rather than "iifname" and > "oifname", the runtime operation will be a direct comparison if the ifindex > (stored with the packet data) rather than a string comparison with the > interface name (which needs to be separately retrieved). Right, that's one of the valuable benefits of NFT avoiding the O(n) performance degradation we have with ipt > The only potential downside would be in the case that an interface was > deleted and then re-added - it would have a new ifindex, and the ifindex in > the rule is determined when the rule is installed, so the new ifindex would > never match. In our case this would never happen though, since all the rules > for a network are recreating (possibly multiple times just after the bridge > device is created, then deleted when the bridge device is deleted. So, you're saying that nft doesn't have something that automatically purges the rules when an indexed interface is deleted ? > > I've tried this, and everything works fine. Should I squash that change into > this patch, or make a separate patch with the change so that this patch > remains (nearly) identical to what is produced by iptables-restore-translate > on the old iptables rules? Lets just do it as a followup. Since you now record the historical rules, we should "do the right thing" in upgrades, so we don't need to be perfect first time out. 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 :|