.conf file setting(s) for packet filtering backend(s)

[Laine Stump]

(this probably will make no sense to anyone who hasn't spent time 
looking at daemonConfig*, in which case you can go ahead and hit Delete 
now. At any rate I'm just tossing this out into the void to see if 
anyone has any ideas/opinions, so in *any* case feel free to hit delete!)

Happy New Year! and time for another bit of confused ramblings trying to 
figure out how to do something that ends up being non-confused and 
straightforward.


I'm currently working on switching the backend of the network driver 
from using iptables to using nftables. Due to some functionality that is 
not available with nftables (the rule that fixes up the checksum of DHCP 
packets which, btw, is only relevant for *very* old guests, e.g. RHEL5), 
this needs to be opt-in via a config file setting. In the meantime, in 
order to make this doable in a reasonable amount of time, I am *not* 
converting the nwfilter driver right away, and when I do it will need 
its own config file setting for opt-in.

I've never before looked at the code for the .conf file settings at all. 
I had assumed there would be some sort of "pull" API, where code in the 
drivers could call, e.g. virConfGetString("filter_backend") and it would 
return the config setting to the caller. But when I look at it, I see 
that all daemons use the same daemonConfigLoadFile() called from 
remote_daemon.c:main() (which is shared by all the daemons) and the 
daemonConfig object that is created to hold the config settings that are 
read is only visible within main() - the only way that a config setting 
is used is by main() "pushing" it out to a static variable somewhere 
else where it is later retrieved by the interested party, e.g. the way 
that main() calls daemonSetupNetDevOpenvswitch(config), which then sets 
the static virNetDevOpenvswitchTimeout in util/virnetdevopenvswitch.c.

(NB: util/virnetdevopenvswitch.c is linked into every deamon, so even 
for the daemons that don't use it, calls to virnetdevopenvswitch.c 
functions still compile properly (and calling them is harmless), so 
virNetDevOpenvswitchTimeout is set even for daemons that never call 
openvswitch APIs).


If I could count on all builds using split daemons (i.e. separate 
virtnetworkd and virtnwfilterd) then I could add a similar API in 
virfirewall.c that remote_daemon.c:main() could use to set 
"filter_backend" into a static in virfirewall.c (which is used by both 
drivers) and everything would just happily work:

    virtnetworkd.conf:
       filter_backend = nftables

    virtnwfilterd.conf
       filter_backend = iptables

However, I need to also deal with the possibility that the nwfilter and 
network drivers are in the same unified libvirtd binary, and in that 
case both drivers would have the same virfirewall.c:filter_backend 
setting, thus making it impossible to use the iptables backend for the 
nwfilter driver and nftables backend for the network driver. For that 
case I would need separate settings in the config for each driver, e.g.

    libvirtd.conf:
       network_filter_backend = nftables
       nwfilter_backend = iptables

and then those would need to be stored off somewhere different for each 
driver, then they would use it to set the backend for each virFirewall 
object as it is created. Organizationally, it would make the most sense 
for these settings (and the API to set them) to be located in the 
drivers that use them (so, for example, network_filter_backend could 
live in network/bridge_driver_linux.c and nwfilter_backend could live in 
nwfilter/nwfilter_driver.c). But that would mean that 
remote_daemon.c:main() would need to directly call functions in those 
files, which is a no-no (because, in the case of split daemons, you 
either have one or the other at build time, but never both).

So should I perhaps declare the nftables backend for nwfilter to be a 
lost cause until everyone moves to split daemons, add a "filter_backend" 
setting that is directly set in virfirewall.c (by 
remote_daemon.c:main()), and then provide some sort of override in 
virFirewallNew so calls from the nwfilter driver can say "ignore the 
filter_backend setting and use iptables"?

Or should we make the virConf APIs beefier, and add facilities to save 
off the entire daemonConfig object and make its contents available via 
something like virConfGetString("network_filter_backend")?

But if I did that, it would mean two differently-named config entries, 
and it would certainly be nice if I didn't have to introduce 
daemon-specific names like this that would need to be carried over from 
libvirtd.conf into virtnwfilterd.conf and virtnetworkd.conf (where 
differing names would no longer be required). I suppose I could go "full 
MS" and introduce the concept of sections to the conf file, so 
libvirtd.conf could have something like this:

     [network]
        filter_backend = nftables
     [nwfilter]
        filter_backend = iptables

but that seems like a lot of work for something that will be obsolete in 
a year or so anyway (when the unified daemon is a thing of the past).

Am I thinking about details too much and missing an obvious simple solution?

(I think until/unless I or someone else comes up with a better idea, I'm 
going to go with a single filter_backend setting set in virfirewall.c 
that is somehow overridden when the virFirewall calls come from the 
nwfilter driver. Then if everyone moves to split daemons before I get 
around to making the nwfilter driver support nftables, the problem will 
end up just solving itself; otherwise I can think of some other piece of 
ugliness to make it work).