Re: [libvirt] [RFC] Refactoring bridge driver for portability
On 12/23/2012 07:54 AM, Roman Bogorodskiy wrote:
Hi,
Few weeks ago when I have submitted my all-in-one huge FreeBSD,
Eric made some comments on the networking part:
http://www.redhat.com/archives/libvir-list/2012-December/msg00432.html
Now when we're done with most of the other things, I'd like
to discuss networking in more detail.
Basically, the main question that bothers me is what would be the
best way to refactor network/bridge_driver.c?
Currently it contains a number of things:
- dnsmasq routines, static, but cross-platform, doesn't need
any changes (at least major ones)
- the same for radvd
- minor firewalld bits, that are linux specifc
- iptables routines. There are quite a lot of that code
and it's also gets called from many places in the file
Note that there is already viriptables.c in src/util. Of course those
are lower level primitives, but they were really created with the needs
of bridge_driver.c in mind.
I'm thinking about an approach that Eric mentioned for virthread.c:
- keep dnsmasq and radvd in bridge_driver.c
- move out all linux specific code to bridge_driver_linux.c and
#include it in a way similar to virthread.c
I must have missed that message. I dislike #including .c files, if
that's what you're talking about.
- in a similar way create bridge_driver_freebsd.c
Does it sound reasonable?
I don't mind the idea of bridge_driver_xxx.c and bridge_driver.c, but I
think that the appropriate bridge_driver_xxx.c should be added to the
Makefile rather than #included in bridge_driver.c.
I would do it by having a common .h file bridge_driver_platform.h which
had a set of functions that must be defined for any supported platform,
then rewrite bridge_driver.c so that everything that isn't
platform-independent is turned into a call to a function or functions
declared in bridge_driver_platform.h and defined in
bridge_driver_(linux|freebsd).c
For example, the firewalld/dbus specific stuff can be considered as "an
opaque chunk of Linux specific code that must be executed during
networkStartup()". I think that can be taken care of by declaring a
function networkStartupPlatform() in bridge_driver_platform.h, then
defining it in both bridge_driver_linux.c and bridge_driver_freebsd.c
(where it may simply return success.)
All of the seemingly iptables-specific functions bridge_driver.c should
be renamed to use a more generic term in place of "iptables":
networkAddIpSpecificIptablesRules
networkAddMasqueradingFilterRules
networkAddRoutingFilterRules
networkReloadFilterRules
networkRemoveFilterRules
networkRemoveIpSPecificFilterRules
networkRemoveGeneralFilterRules
[etc]
Almost all of those functions should be able to remain in
bridge_driver.c; only the bottom-level functions that are directly
calling iptables(Add|Remove)*() functions should need to be moved to
bridge_driver_xxx.c (and re-written appropriately for FreeBSD). (Or
maybe the concepts behind the functions declared in
src/util/viriptables.h are generic enough that those functions should
also be renamed, and a new src/util/virpf.c (or whatever filtering
package is used on FreeeBSD these days - I haven't kept up with BSD for
a long time) defined that implements these filter*() functions for that
platform.