Laine Stump wrote: > 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. That sounds reasonable, I'll start moving this way. Thanks Roman Bogorodskiy
Attachment:
pgpP98IVCGY1J.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list