[ovs-dev] [patch 1/5] datapath: dont use non-existent receive hooks

[Simon Horman]

On Sun, Aug 08, 2010 at 05:49:30PM -0400, Jesse Gross wrote:
> On Fri, Aug 6, 2010 at 11:05 AM, Simon Horman <horms at verge.net.au> wrote:
> 
> > This adds compatibility with kernel changeset
> > of changeset "net: replace hooks in __netif_receive_skb V5",
> > which was also added between 2.6.35 and 2.6.35-rc1
> >
> 
> The upper bound on the version numbers should be 2.6.36-rc1, right?
>  Actually, I see this on all the commit messages.

Sorry, it was late. 2.6.36-rc1 is correct.

> >  /*
> >  * Used as br_handle_frame_hook.  (Cannot run bridge at the same time, even
> > on
> >  * different set of devices!)
> >  */
> > +#ifdef HAVE_BR_HANDLE_FRAME_HOOK
> >  #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,22)
> >
> 
> Rather than having nested #if ... it might be cleaner to just have #if >=
> 2.6.36; #elif >= 2.6.22; #else

I've removed HAVE_BR_HANDLE_FRAME_HOOK and used the scheme you suggest.

> > +static int netdev_init(void) { return 0; }
> > +static void netdev_exit(void) { ; }
> >
> 
> You shouldn't need a semicolon here.

Fixed

> >        dev_set_promiscuity(netdev_vport->dev, 1);
> >        dev_disable_lro(netdev_vport->dev);
> > -       rcu_assign_pointer(netdev_vport->dev->br_port, (struct
> > net_bridge_port *)vport);
> > -
> > -       return 0;
> > +       rcu_assign_pointer(netdev_vport->dev->br_port,
> > +                          (struct net_bridge_port *)vport);
> > +#ifndef HAVE_BR_HANDLE_FRAME_HOOK
> > +       err = netdev_rx_handler_register(netdev_vport->dev,
> > netdev_frame_hook);
> > +#endif
> >
> 
> We need to do some rearranging here in case netdev_rx_handler_register()
> fails.  I think we can just move dev_set_prosmiscuity() and
> dev_disable_lro() after registering the rx_handler().
> 
> 
> > +  OVS_GREP_IFELSE([$KSRC26/include/linux/if_bridge.h],
> > +                  [br_handle_frame_hook],
> > +                  [OVS_DEFINE([HAVE_BR_HANDLE_FRAME_HOOK])])
> >
> 
> Usually we just use these configure tests on an as-needed basis for
> functions that have been backported to older kernels and directly check the
> version number in other cases.  This is obviously complicated by the fact
> that 2.6.36-rc1 doesn't exist yet and therefore the version number will
> still show up as 2.6.35 but that will change in the relatively near future.
>  For tests that are repeated we can just define a symbol based on the
> version, without actually having to do a test.

Understood. I've put dummy versions of netdev_rx_handler_register()
and netdev_rx_handler_unregister into
datapath/linux-2.6/compat-2.6/include/linux/netdevice.h. I have used
LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,36) (which conveniently now
works as 2.6.36-rc1 has been released) in netdevice.h and vport-netdev.c.

I will repost the entire series once I have reworked the other patches.