On Mon, Nov 25, 2024 at 11:56:31AM -0500, Laine Stump wrote:
On 11/25/24 5:44 AM, Daniel P. Berrangé wrote:
On Fri, Nov 22, 2024 at 04:16:38PM -0500, Laine Stump wrote:
If the layer of a FirewallCmd is "raw", then the first arg is the name of an arbitrary binary to exec, and the rest are the arguments to that binary.
raw layer doesn't support auto-rollback command creation (any rollback needs to be added manually with virFirewallAddRollbackCmd()), and also raw layer isn't supported by the iptables backend (it would have been straightforward to add, but the iptables backend doesn't need it, and I didn't want to take the chance of causing a regression in that code for no good reason).
I guess the obvious question to ask is why you chose to define a "raw" layer, as opposed to defining a "tc" layer ? Being more targetted about the anticipated usage feels better IMHO.
I thought about that, but while layer is used to figure out the binary name for the iptables backend (because the different layers use ebtables, iptables, or ip6tables), in the case of the nftables backend all of the different layers use "nft" as the binary, and the layer indicates changes in a few of the arguments to that command (and really both your suggestion and mine are technically messed up, since the layer in the case of this checksum-fix filter should really be "ipv4").
Maybe we just shouldn't be pretending this is a firewall command at all ? Even with iptables, this really isn't anything to do with traffic filtering. iptables just happened to be a convenient place to put the logic in the kernel at the time. 'tc' is the new "convenient" place to put the logic today. How about putting a virNetDevFixDHCPChecksum() in virnetdev.h/c ? and just invoking this API after we've setup nftables rules ? 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 :|