netdev - Re: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag" (802.1p packet)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 16 Jun 2010 15:28:55 +0200
From:	Pedro Garcia <pedro.netdev@...devamos.com>
To:	<netdev@...r.kernel.org>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	Ben Hutchings <bhutchings@...arflare.com>,
	Patrick McHardy <kaber@...sh.net>
Subject: Re: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag" (802.1p packet)

On Wed, 16 Jun 2010 13:42:16 +0200, Patrick McHardy <kaber@...sh.net>
wrote:
> Eric Dumazet wrote:
>> Le mercredi 16 juin 2010 à 10:49 +0200, Pedro Garcia a écrit :
>>> Here it is again. I added the modifications in
>>> http://kerneltrap.org/mailarchive/linux-netdev/2010/5/23/6277868 for HW
>>> accelerated incoming packets (it did not apply cleanly on the last
>>> version of
>>> the kernel, so I applied manually). Now, if the VLAN 0 is not
>>> explicitly created by the user, VLAN 0 packets will be treated as no
>>> VLAN (802.1p packets), instead of dropping them.
>>>
>>> The patch is now for two files: vlan_core (accel) and vlan_dev (non
>>> accel)
>>>
>>> I can not test on HW accelerated devices, so if someone can check it I
>>> will appreciate (even though in the thread above it looked like yes).
>>> For non accel I tessted in 2.6.26. Now the patch is for
>>> net-next-2.6, and it compiles OK, but I a have to setup a test
>>> environment to check it is still OK (should, but better to test).
>>>
>>> Signed-off-by: Pedro Garcia <pedro.netdev@...devamos.com>
>>>     
>>
>> OK, the patch itself is correct.
>>   
> 
> Yes, looks fine to me as well.
> 
>> Now, could you please send it again with a proper changelog ?
>>
>> In this changelog, please explain why patch is needed, and
>> keep lines short (< 72 chars), like the one you did in your first mail.
>>
>> I'll then add my Signed-off-by, since I wrote the accelerated part ;)
>>
>> Note : I wonder if another patch is needed, in case 8021q module is
>> _not_ loaded. We probably should accept vlan 0 frames in this case ?
>>   
> 
> I agree that this would be best for consistency, but that would mean
> adding more special cases to __netif_receive_skb().

In my understanding, 802.1p is a "subset" of 802.1q, and they share the 
protocol number. We can do a 802.1p module, but in the end it will end
up reusing most of the code in 802.1q.

In any case defining a VLAN 0 ends up usually in problems with which table 
the ARP entries get stored in. This patch solves the problem to whom 
is not using VLAN 0 explicitly, but if somebody is using VLAN 0 tagging
it will work (whatever "work" means) as before.

Probably a definitive fix would be not to allow the definition of VLAN 0 
in 802.1q module and provide some other way to tag priority packets without
using a subinterface (maybe in the same module or a new 8021p one). I am 
having a look at the kernel to see what happens if we load two modules for 
the same protocol. 

By the way, the changelog I have to write is just the text before the 
patch?


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