On Thu, Jan 23, 2014 at 04:04:24PM -0800, Luigi Rizzo wrote: > 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__.
Feature tests are always nice, but a lot of the stuff protected by LINUX_DATAPATH is really Linux specific, like the structure of /proc. > 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. Why don't you just send along the patches and if they make sense we'll apply them. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
