Subject: Re: VLAN packets silently dropped in promiscuous mode

From: Guillaume Gaudonville <[email-masked]>
Date: Mon, 25 Oct 2010 15:48:41 +0200

Jesse Gross wrote:
> On Fri, Oct 15, 2010 at 2:16 AM, Guillaume Gaudonville
> <[email-masked]> wrote:
>   
>> Jesse Gross wrote:
>>     
>>> On Thu, Sep 30, 2010 at 1:07 AM, Roger Luethi <[email-masked]> wrote:
>>>
>>>       
>>>> On Wed, 29 Sep 2010 10:44:26 -0700, Jesse Gross wrote:
>>>>
>>>>         
>>>>> On Wed, Sep 29, 2010 at 4:37 AM, Roger Luethi <[email-masked]> wrote:
>>>>>
>>>>>           
>>>>>> I noticed packets for unknown VLANs getting silently dropped even in
>>>>>> promiscuous mode (this is true only for the hardware accelerated path).
>>>>>> netif_nit_deliver was introduced specifically to prevent that, but the
>>>>>> function gets called only _after_ packets from unknown VLANs have been
>>>>>> dropped.
>>>>>>
>>>>>>             
>>>>> Some drivers are fixing this on a case by case basis by disabling
>>>>> hardware accelerated VLAN stripping when in promiscuous mode, i.e.:
>>>>>
>>>>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=5f6c01819979afbfec7e0b15fe52371b8eed87e8
>>>>>
>>>>> However, at this point it is more or less random which drivers do
>>>>> this.  It would obviously be much better if it were consistent.
>>>>>
>>>>>           
>>>> My understanding is this. Hardware VLAN tagging and stripping can always
>>>> be
>>>> enabled. The kernel passes 802.1Q information along with the stripped
>>>> header to libpcap which reassembles the original header where necessary.
>>>> Works for me.
>>>>
>>>>         
>>> Sorry, I misread your original post as saying that the VLAN header
>>> gets dropped, rather than the entire packet.  I agree that this is how
>>> it should work but not necessarily how it does work (again, depending
>>> on the driver).  Here's the problem that I was talking about:
>>>
>>> Most drivers have a snippet of code that looks something like this
>>> (taken from ixgbe):
>>>
>>> if (adapter->vlgrp && is_vlan && (tag & VLAN_VID_MASK))
>>>        vlan_gro_receive(napi, adapter->vlgrp, tag, skb);
>>> else
>>>        napi_gro_receive(napi, skb);
>>>
>>> At this point the VLAN has already been stripped in hardware.  If
>>> there is no VLAN group configured on the device then we hit the second
>>> case.  The VLAN header was removed from the SKB and the tag variable
>>> is unused.  It is no longer possible for libpcap to reconstruct the
>>> header because the information was thrown away (even the fact that
>>> there was a VLAN tag at all).
>>>
>>> There are a couple ways to fix this:
>>>
>>> * Turn off VLAN stripping when in promiscuous mode (as done by the ixgbe
>>> driver)
>>>
>>>       
>> This is not totally true: if changing the MTU ixgbe_change_mtu will call:
>> ixgbe_reinit_locked--> ixgbe_up --> ixgbe_configure:
>>                --> ixgbe_set_rx_mode: flag IFF_PROMISC is tested
>> ixgbe_vlan_filter_enable is not called
>>                --> ixgbe_restore_vlan --> ixgbe_vlan_rx_register: flag
>> IFF_PROMISC is not tested ixgbe_vlan_filter_enable
>>                     will be called.
>>
>> In fact it should happen each time we configure something which needs a
>> reset of the device. Why don't add a test
>> on flag promiscuous directly in ixgbe_vlan_filter_enable? Or do it on each
>> call, if we want to allow a device in promiscuous
>> mode to enable this feature.
>>
>> What do you think?
>>     
>
> I can believe that there are paths that lead to this not working
> correctly.  That was actually my larger point: this is something that
> is commonly not implemented correctly in drivers.  Rather than try to
> study every driver my goal is to just avoid the problem completely by
> handling vlan acceleration centrally in the networking core.  I sent
> out an RFC patch series a few days ago that should solve this problem:
>
> http://marc.info/?l=linux-netdev&m=128700022614170&w=3
> --
> 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
>   
Thank you, I'm going to check these patches and try to apply them in our 
kernel.

Best Regards,

-- 
Guillaume Gaudonville
6WIND
Software Engineer

Tel: +33 1 39 30 92 63
Mob: +33 6 47 85 34 33
Fax: +33 1 39 30 92 11
[email-masked]
www.6wind.com
Join the Multicore Packet Processing Forum: www.multicorepacketprocessing.com

Ce courriel ainsi que toutes les pièces jointes, est uniquement destiné à son ou ses destinataires. Il contient des informations confidentielles qui sont la propriété de 6WIND. Toute révélation, distribution ou copie des informations qu'il contient est strictement interdite. Si vous avez reçu ce message par erreur, veuillez immédiatement le signaler à l'émetteur et détruire toutes les données reçues

This e-mail message, including any attachments, is for the sole use of the intended recipient(s) and contains information that is confidential and proprietary to 6WIND. All unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply e-mail and destroy all copies of the original message.