Subject: Re: [PATCH 2/2] network: Allow af_packet to transmit +4 bytes for VLAN packets.

From: Ben Greear <[email-masked]>
Date: Fri, 11 Feb 2011 09:38:26 -0800

On 02/10/2011 10:57 PM, Eric Dumazet wrote:
> Le jeudi 10 février 2011 à 13:59 -0800, [email-masked] a
> écrit :
>> From: Ben Greear<[email-masked]>
>>
>> This allows user-space to send a '1500' MTU VLAN packet on a
>> 1500 MTU ethernet frame.  The extra 4 bytes of a VLAN header is
>> not usually charged against the MTU when other parts of the
>> network stack is transmitting vlans...
>>
>> Signed-off-by: Ben Greear<[email-masked]>
>> ---
>> :100644 100644 91cb1d7... ef7f378... M	net/packet/af_packet.c
>>   net/packet/af_packet.c |   31 +++++++++++++++++++++++++++++--
>>   1 files changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
>> index 91cb1d7..ef7f378 100644
>> --- a/net/packet/af_packet.c
>> +++ b/net/packet/af_packet.c
>> @@ -466,7 +466,7 @@ retry:
>>   	 */
>>
>>   	err = -EMSGSIZE;
>> -	if (len>  dev->mtu + dev->hard_header_len)
>> +	if (len>  dev->mtu + dev->hard_header_len + VLAN_HLEN)
>>   		goto out_unlock;
>>
>>   	if (!skb) {
>> @@ -497,6 +497,19 @@ retry:
>>   		goto retry;
>>   	}
>>
>> +	if (len>  (dev->mtu + dev->hard_header_len)) {
>> +		/* Earlier code assumed this would be a VLAN pkt,
>> +		 * double-check this now that we have the actual
>> +		 * packet in hand.
>> +		 */
>> +		struct ethhdr *ehdr;
>> +		skb_reset_mac_header(skb);
>> +		ehdr = eth_hdr(skb);
>> +		if (ehdr->h_proto != htons(ETH_P_8021Q)) {
>> +			err = -EMSGSIZE;
>> +			goto out_unlock;
>
> This would leak skb.
>
>> +		}
>> +	}
>>
>>   	skb->protocol = proto;
>>   	skb->dev = dev;

Can you double-check that?  Seems to me that in that method the out_unlock
falls through to the out_free case.  The other method is the opposite, funny
enough...

Thanks,
Ben

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

-- 
Ben Greear <[email-masked]>
Candela Technologies Inc  http://www.candelatech.com