[ovs-dev] [PATCH 2/3] Do not seemingly #include Linux-specific headers on other platforms.

Ben Pfaff blp at nicira.com
Sat Aug 2 04:38:49 UTC 2014


On Fri, Aug 01, 2014 at 06:58:46PM -0700, Jesse Gross wrote:
> On Thu, Jul 31, 2014 at 7:52 PM, Ben Pfaff <blp at nicira.com> wrote:
> > On Mon, Jun 23, 2014 at 03:39:32PM -0700, Jesse Gross wrote:
> >> On Fri, Jun 13, 2014 at 3:28 PM, Ben Pfaff <blp at nicira.com> wrote:
> >> > Until now, the OVS source tree has had a whole maze of header files that
> >> > make "#include <linux/openvswitch.h>" work OK regardless of platform, but
> >> > this confuses everyone new to the tree, at first glance, and is difficult
> >> > to understand at second glance too.
> >> >
> >> > This commit moves the core of the Netlink definitions that were in
> >> > include/linux/openvswitch.h into a new header
> >> > include/odp-netlink-internal.h, and then adds two wrappers:
> >> >
> >> >     * datapath/linux/compat/include/linux/openvswitch.h: This wrapper
> >> >       allows the Linux datapath code to continue using "#include
> >> >       <linux/openvswitch.h>".
> >> >
> >> >     * lib/odp-netlink.h: Userspace code uses this wrapper.
> >> >
> >> > This change allows all of the include/linux/* files to be deleted.
> >> >
> >> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> >>
> >> I'm somewhat worried that diverging our copy of linux/openvswitch.h
> >> will make things harder to maintain, especially since we've tried to
> >> reduce differences between upstream and the out-of-tree module as much
> >> as possible.
> >>
> >> I was imagining that we would model this as if we were compiling
> >> purely against the upstream kernel and openvswitch.h was an exported
> >> header (since it is, we just happen to have a compatibility backport).
> >> In that case, we wouldn't be able to change the original header and
> >> presumably Linux-specific code would translate to something
> >> OVS-internal.
> >
> > I've been starting to think about this again since (as you pointed out
> > elsewhere) it came up in the hyperv port.
> >
> > I'm not too sure what you're envisioning in the model that you're
> > thinking of.  Here are my thoughts on how to do this in terms of
> > backports:
> >
> >         * We'd need some base version of the whole header file, because
> >           not every build environment would have any version at all
> >           (given that we build on non-Linux and often system Linux
> >           header files are for kernels older than the one actually in
> >           use).
> >
> >         * We'd need to detect the presence of numerous "enum" values and
> >           define them if not present (or we could unconditionally
> >           #define them as backports whether present or not, since using
> >           #defines as overlays for enums is mostly harmless).
> >
> >         * We'd need to detect the presence of numerous structs and
> >           define them if not present.
> >
> > It might be more work to maintain a set of backports than a slightly
> > different header file.  (It's definitely more work than what we're doing
> > now.)
> 
> When I said "backport", I didn't really mean that we need to take
> things piece-by-piece. It was really more of a way to view things.
> 
> Let's pretend that all of the features from the out of tree kernel
> module are available in the currently running kernel and the userspace
> header is up to date, since this is our goal. In this case, we would
> just use the installed header and running kernel. This is what we do
> for all other non-OVS Linux features.
> 
> Now obviously this is not really the case because the OVS kernel
> module is still relatively young and fast moving and therefore has not
> reached the point we can safely assume that the features available in
> the current kernel will satisfy the majority of users. For the kernel
> code, this means that we have a backported module. For the interface,
> we also have a backport currently (even if it happens to completely
> replace the exported header file, I still consider this a backport -
> we do a full replacement for some kernel code too).
> 
> Back to the original point, it seems best to write the userspace code
> as if we were running against the kernel's exported header file and
> the out-of-tree module/interface is just a temporary backport. This is
> essentially what is happening today. However, it means that we
> shouldn't modify/refactor this file to include OVS-native types,
> Windows code, etc. because we wouldn't be able to do that if we were
> running against the native kernel.

I posted a v2 that I hope solves both the problems that you want to
solve and the ones that I do:
        http://openvswitch.org/pipermail/dev/2014-August/043396.html
        http://openvswitch.org/pipermail/dev/2014-August/043397.html
        http://openvswitch.org/pipermail/dev/2014-August/043398.html
        



More information about the dev mailing list