[net-2.6,1/3] vlan: Add function to retrieve EtherType from vlan packets.

From: Hao Zheng <hzheng@nicira.com>

Depending on how a packet is vlan tagged (i.e. hardware accelerated or
not), the encapsulated protocol is stored in different locations.  This
provides a consistent method of accessing that protocol, which is needed
by drivers, security checks, etc.

Signed-off-by: Hao Zheng <hzheng@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
 include/linux/if_vlan.h |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

On Tue,  9 Nov 2010 17:09:02 -0800
Jesse Gross <jesse@nicira.com> wrote:

> From: Hao Zheng <hzheng@nicira.com>
> 
> Depending on how a packet is vlan tagged (i.e. hardware accelerated or
> not), the encapsulated protocol is stored in different locations.  This
> provides a consistent method of accessing that protocol, which is needed
> by drivers, security checks, etc.
> 
> Signed-off-by: Hao Zheng <hzheng@nicira.com>
> Signed-off-by: Jesse Gross <jesse@nicira.com>
> ---
>  include/linux/if_vlan.h |   20 ++++++++++++++++++++
>  1 files changed, 20 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index c2f3a72..ee06c52 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -339,6 +339,26 @@ static inline int vlan_get_tag(const struct sk_buff *skb, u16 *vlan_tci)
>  	}
>  }
>  
> +/**
> + * vlan_get_protocol - get protocol EtherType.
> + * @skb: skbuff to query
> + *
> + * Returns the EtherType of the packet, regardless of whether it is
> + * vlan encapsulated (normal or hardware accelerated) or not.
> + */
> +static inline __be16 vlan_get_protocol(struct sk_buff *skb)
> +{
> +	__be16 protocol = 0;
> +
> +	if (vlan_tx_tag_present(skb) ||
> +	     skb->protocol != cpu_to_be16(ETH_P_8021Q))
> +		protocol = skb->protocol;
> +	else if (likely(pskb_may_pull(skb, VLAN_ETH_HLEN)))
> +		protocol = ((const struct vlan_ethhdr *)skb->data)->
> +			   h_vlan_encapsulated_proto;
> +
> +	return protocol;
> +}

This this calls pskb_may_pull, which modifies the skb data
offsets and therefore could invalidate any callers pointers
to ip header or other fields.
Therefore you will need to audit all callers of this function!

Also, your code doesn't handle the case of too small a frame (VLAN header only).

On Tue, Nov 9, 2010 at 9:54 PM, Stephen Hemminger <shemminger@vyatta.com> wrote:
> On Tue,  9 Nov 2010 17:09:02 -0800
> Jesse Gross <jesse@nicira.com> wrote:
>
>> From: Hao Zheng <hzheng@nicira.com>
>>
>> Depending on how a packet is vlan tagged (i.e. hardware accelerated or
>> not), the encapsulated protocol is stored in different locations.  This
>> provides a consistent method of accessing that protocol, which is needed
>> by drivers, security checks, etc.
>>
>> Signed-off-by: Hao Zheng <hzheng@nicira.com>
>> Signed-off-by: Jesse Gross <jesse@nicira.com>
>> ---
>>  include/linux/if_vlan.h |   20 ++++++++++++++++++++
>>  1 files changed, 20 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
>> index c2f3a72..ee06c52 100644
>> --- a/include/linux/if_vlan.h
>> +++ b/include/linux/if_vlan.h
>> @@ -339,6 +339,26 @@ static inline int vlan_get_tag(const struct sk_buff *skb, u16 *vlan_tci)
>>       }
>>  }
>>
>> +/**
>> + * vlan_get_protocol - get protocol EtherType.
>> + * @skb: skbuff to query
>> + *
>> + * Returns the EtherType of the packet, regardless of whether it is
>> + * vlan encapsulated (normal or hardware accelerated) or not.
>> + */
>> +static inline __be16 vlan_get_protocol(struct sk_buff *skb)
>> +{
>> +     __be16 protocol = 0;
>> +
>> +     if (vlan_tx_tag_present(skb) ||
>> +          skb->protocol != cpu_to_be16(ETH_P_8021Q))
>> +             protocol = skb->protocol;
>> +     else if (likely(pskb_may_pull(skb, VLAN_ETH_HLEN)))
>> +             protocol = ((const struct vlan_ethhdr *)skb->data)->
>> +                        h_vlan_encapsulated_proto;
>> +
>> +     return protocol;
>> +}
>
> This this calls pskb_may_pull, which modifies the skb data
> offsets and therefore could invalidate any callers pointers
> to ip header or other fields.
> Therefore you will need to audit all callers of this function!

That's a good point.  I switched it to use skb_header_pointer()
instead, which is probably more efficient anyways and avoids the
potential for a problem.

>
> Also, your code doesn't handle the case of too small a frame (VLAN header only).

The goal is to get equivalence to checking skb->protocol, except to
handle vlan accelerated vs non-accelerated consistently.  In this
case, the caller would need to check the length of the protocol header
as appropriate.  If the packet claims to be a vlan frame and the
length is less than the size of a vlan header then we'll return 0,
which should be sufficient to avoid any protocol processing.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html