[ovs-dev] [PATCH 1/4] datapath: Add vlan acceleration field for older kernels.

Ben Pfaff blp at nicira.com
Mon Feb 7 17:48:29 UTC 2011


On Sat, Feb 05, 2011 at 04:14:46PM -0800, Jesse Gross wrote:
> Kernels prior to 2.6.27 did not have a vlan_tci field in struct
> sk_buff for vlan acceleration.  It's very convenient to use this
> field for manipulating vlan tags, so we would like to use it as
> the primary mechanism.  To enable this, this commit adds similar
> infrastructure to the OVS_CB on the kernels that need it and a
> set of functions to use the correct location.
> 
> Signed-off-by: Jesse Gross <jesse at nicira.com>

The new functions for pre-2.6.27 are all trivial.  Why aren't they
inlined into vlan.h?

The two occurrences of #ifdef X/#undef X/#endif can be simplified to
just #undef X.

I don't understand the name of vlan_copy_skb_tci().  It doesn't copy
anything and only one of two of its invocations happen just after a
(possible) skb copy.  Maybe vlan_init_skb_tci()?

I'd be really tempted to just define one base macro, like this:
        #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,27)
        #define SKB_VLAN_TCI(skb) OVS_CB(skb)->vlan_tci
        #else
        #define SKB_VLAN_TCI(skb) (skb)->vlan_tci
        #endif
or even something like this (although the naming is poor):
        #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,27)
        #define VLAN_STRUCT(skb) OVS_CB(skb)
        #else
        #define VLAN_STRUCT(skb) (skb)
        #endif
and then get rid of the vlan_set_tci() and vlan_get_tci() functions.  I
think that it would be slightly more amenable to search-and-replace for
upstreaming.  But I'll understand if you think that's too clever.

Acked-by: Ben Pfaff <blp at nicira.com>




More information about the dev mailing list