[ovs-dev] [PATCH 04/16] datapath: Don't read net namespace on kernels that don't use them.

[Jesse Gross]

On Tue, Apr 13, 2010 at 1:04 PM, Ben Pfaff <blp at nicira.com> wrote:

> On Tue, Apr 13, 2010 at 10:41:06AM -0400, Jesse Gross wrote:
> > Use macros to eliminate the network namespace argument before it
> > gets to the compiler.  This allows us to specify a namespace on
> > kernels that know about them and prevent the compiler from complaining
> > on kernels that don't.
>
> I'm surprised that this is necessary.  I didn't think that the kernel
> compiler flags were set up to complain about unused arguments.  But the
> change seems OK.
>

This actually isn't to suppress a warning about unused arguments - it avoids
an error about an undefined symbol.

The problem is that when we call the function it look like this:

rtnl_notify(skb, &init_net, ...);

init_net isn't defined on kernels before net namespaces were introduced so
the compiler barfs when it sees it, even though it isn't used.


> One nit: The cast to (void) below should be unnecessary since
> rtnl_notify() as we define it already has a "void" return type.
>
> >  /* No 'net' parameter in these versions. */
> >  #define rtnl_notify(skb, net, pid, group, nlh, flags) \
> > -     ((void) (net), (void) rtnl_notify(skb, pid, group, nlh, flags))
> > +                 ((void) rtnl_notify((skb), (pid), (group), (nlh),
> (flags)))
>

This macro is also used on later kernels where this function is defined but
not in the form we want (2.6.19-24) so we need the cast to void.


> Another nit: I know that paranoid parenthesization is customary for C
> macros, but it really isn't necessary for function arguments, because
> macro arguments cannot contain commas (outside parentheses)[1] and the
> extra parentheses make everything harder to read:
>
> >  /* Make the return type effectively 'void' to match Linux 2.6.30+. */
> >  #define rtnl_notify(skb, net, pid, group, nlh, flags) \
> > -     ((void) rtnl_notify(skb, net, pid, group, nlh, flags))
> > +     ((void) rtnl_notify((skb), (net), (pid), (group), (nlh), (flags)))
> >  #endif
> >
> >  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,26)
>
> [1] Well, you can do something like
>       #define COMMA ,
>       #define f(a,b) a
>       f(1 COMMA 2, 3)
>    which will expand to
>       1 , 2
>    but anyone who does that deserves what he gets in my opinion.
>

Hmm, I guess you're right.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20100413/be88c218/attachment-0003.html>