On Thu, Jan 23, 2014 at 3:39 PM, Ben Pfaff <b...@nicira.com> wrote: > On Thu, Jan 23, 2014 at 11:51:26PM +0100, Luigi Rizzo wrote: > > following the port of the OVS kernel code to FreeBSD (most of it > > done by Daniele Di Proietto), the varia that control feature > > inclusion may benefit from some rediscussion. > > > > At the moment ovs uses LINUX_DATAPATH for different purposes: > > > > 1. indicate that an in-kernel datapath is available > > 2. indicate the availability of certain linux features > > (rtnetlink, traffic control, vlandev, stats collection) > > that interact with the in-kernel datapath. > > > > #1 and #2 are now decoupled (and #2 could be split even further at > > some point), so in places where we want to check for #2 we added > > an extra test on HAVE_IF_DL, which at the moment basically discriminates > > between Linux and *BSD. > > Case #2 occurs in openvswitch/lib/automake.mk, openvswitch/lib/vlandev.c > > and openvswitch/vswitchd/system-stats.c whereas there are over 100 > > instances of LINUX_DATAPATH > > > > Before submitting a patch we'd like to know which of the following > > sounds more suitable: > > > > a) use " #if defined(LINUX_DATAPATH) && !defined(HAVE_IF_DL)" > > to test for case 2. > > > > b) define some other indentifiers for specific features to be used in > > case 2 (say HAVE_RTNETLINK ...), same as it is done for HAVE_IF_DL > > > > We have (a) ready, but for readability maybe (b) would be preferable. > > > > And as an orthogonal issue, it may make sense to rename LINUX_DATAPATH > > into KERNEL_DATAPATH (though it would be just for aesthetic reasons, > > and i am not sure if it is worthwhile considering the number > > of files and places affected) > > After review, I think we can just change them all from LINUX_DATAPATH > to just __linux__. I guess there are a few where you'll want to > either change them to __linux__ || __FreeBSD__ when the port is > available, or to something like KERNEL_DATAPATH if you prefer, but it > seems like a clean way forward. > > hmmm... I believe we should look for features, not assume their presence based on OS name. so I'd rather go for KERNEL_DATAPATH and HAVE_* rather than __linux__ or __FreeBSD__.
Also, our porting approach (which we found very effective in many cases) remaps linux APIs into FreeBSD equivalent ones, and for the most part there is a 1-1 mapping with no significant performance hit at runtime. Using a handful of private, fine-grained HAVE_* names makes the porting simpler. Daniele can probably send our current diff to see what are the components involved. > (How did you find over 100 instances of LINUX_DATAPATH? I only see > about 30, total.) > my bad, i had the autotools-generated files in the tree... thanks for the feedback luigi
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
