Re: [PATCH v5 24/30] network: add an nftables backend for network driver's firewall construction

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.