Re: [PATCH] vlan: Fix the b0rked ingress VLAN_FLAG_REORDER_HDR check.

[ebiederm@xmission.com (Eric W. Biederman)]

David Miller <davem@davemloft.net> writes:

> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Mon, 23 May 2011 17:11:51 -0700
>
>> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
>> index 41495dc..c08dae7 100644
>> --- a/net/8021q/vlan_core.c
>> +++ b/net/8021q/vlan_core.c
>> @@ -23,6 +23,11 @@ bool vlan_do_receive(struct sk_buff **skbp)
>>  		return false;
>>  
>>  	skb->dev = vlan_dev;
>> +	if (unlikely(!(vlan_dev_info(vlan_dev)->flags & VLAN_FLAG_REORDER_HDR))) {
>> +		skb = *skbp = vlan_insert_tag(skb, skb->vlan_tci);
>> +		if (!skb)
>> +			return false;
>> +	}
>>  	skb->priority = vlan_get_ingress_priority(vlan_dev, skb->vlan_tci);
>>  	skb->vlan_tci = 0;
>>  
>
> Below we do a eth_hdr(skb) based check on the ethernet address in the
> PACKET_OTHERHOST case.
>
> Won't this VLAN tag insertion change skb->mac_header and thus screw up
> that test?

Yes the vlan tag insertion does in fact change the skb->mac_header
index.  However we also move the location of the data as well, so
I fail to see any reason to be concerned about the PACKET_OTHERHOST
case.  Things were moved around and we updated the appropriate references.

Feel free to read through the code, to convince yourself it is correct.
In addition the code is untouched from the vlan header insertion for
emulation of vlan header acceleration in dev_hard_start_xmit() which
presumably has been working for quite awhile.

> Touching this code requires surgical precision and long audits, because
> there are so many subtble dependencies all over the place like this.

Certainly.

I think you will find that I have applied great precision and restraint
in this case. 

Eric