Thread (35 messages) 35 messages, 11 authors, 2010-07-20

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

From: Pedro Garcia <hidden>
Date: 2010-06-16 13:28:58

On Wed, 16 Jun 2010 13:42:16 +0200, Patrick McHardy [off-list ref]
wrote:
Eric Dumazet wrote:
quoted
Le mercredi 16 juin 2010 à 10:49 +0200, Pedro Garcia a écrit :
quoted
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 <redacted>
    
OK, the patch itself is correct.
  
Yes, looks fine to me as well.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help