[ovs-dev] [PATCH v2] datapath: Fix pop_vlan()

[Jesse Gross]

On Mon, Nov 14, 2011 at 6:39 PM, Pravin B Shelar <pshelar at nicira.com> wrote:
> diff --git a/acinclude.m4 b/acinclude.m4
> index 648132a..8c33690 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -254,6 +254,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
>   OVS_GREP_IFELSE([$KSRC/include/net/netlink.h], [nla_get_be16])
>   OVS_GREP_IFELSE([$KSRC/include/net/netlink.h], [nla_find_nested])
>
> +  OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [vlan_set_encap_proto])

Since this function doesn't yet exist in if_vlan.h in any version of
Linux, I would leave out the check completely and just put a comment
on the implementation that it needs to be upstreamed.

> diff --git a/datapath/actions.c b/datapath/actions.c
> index ac7187b..c3fcece 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> +static void pop_vlan_header(struct sk_buff *skb)

I don't think that there's much benefit to this being a separate
function and it actually makes it more difficult to read because the
code is logically the same but split in two different places.

>  static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
[...]
> +       skb->network_header += VLAN_HLEN;
> +       skb->transport_header += VLAN_HLEN;

On thinking about this further, it's not actually correct to update
these headers - they don't move when the vlan header is removed.