VLAN packets silently dropped in promiscuous mode

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.

Quoted 5 lines · Roger Luethi

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.

Quoted 7 lines · Jesse Gross

Quoted 5 lines · Roger Luethi

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.

Quoted 6 lines

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.

Hardware VLAN filtering, on the other hand, must be disabled in promiscuous mode. But doing that in the driver makes no difference now as the current VLAN code drops the packets so preserved before they are passed to the pcap interface. That appears to be a bug.

Agreed

Could you try following patch, based on net-next-2.6 ?

Quoted 3 lines · Eric Dumazet

Agreed

Could you try following patch, based on net-next-2.6 ?

Here is the official patch I cooked for net-2.6 (linux-2.6)

I tested it successfully with a tg3 NIC.

Thanks

[PATCH] vlan: dont drop packets from unknown vlans in promiscuous mode

Roger Luethi noticed packets for unknown VLANs getting silently dropped even in promiscuous mode.

Check for promiscuous mode in __vlan_hwaccel_rx() and vlan_gro_common() before drops.

Reported-by Roger Luethi <[email protected]>

Signed-off-by Eric Dumazet <[email protected]>

---
 net/8021q/vlan_core.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

net/8021q/vlan_core.c +1 -1

index 01ddb04..ba502b4 100644
@@ -24,7 +24,7 @@ int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,
 
 	if (vlan_dev)
 		skb->dev = vlan_dev;
-	else if (vlan_id)
+	else if (vlan_id && !(skb->dev->flags & IFF_PROMISC))
 		goto drop;
 
 	return (polling ? netif_receive_skb(skb) : netif_rx(skb));

Quoted 47 lines · Eric Dumazet

Quoted 3 lines · Eric Dumazet

Agreed

Could you try following patch, based on net-next-2.6 ?

Here is the official patch I cooked for net-2.6 (linux-2.6)

I tested it successfully with a tg3 NIC.

Thanks

[PATCH] vlan: dont drop packets from unknown vlans in promiscuous mode

Roger Luethi noticed packets for unknown VLANs getting silently dropped even in promiscuous mode.

Check for promiscuous mode in __vlan_hwaccel_rx() and vlan_gro_common() before drops.

Reported-by Roger Luethi <[email protected]>

Signed-off-by Eric Dumazet <[email protected]>

---
 net/8021q/vlan_core.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

net/8021q/vlan_core.c +2 -2

index 01ddb04..ba502b4 100644
@@ -24,7 +24,7 @@ int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,
 
 	if (vlan_dev)
 		skb->dev = vlan_dev;
-	else if (vlan_id)
+	else if (vlan_id && !(skb->dev->flags & IFF_PROMISC))
 		goto drop;
 
 	return (polling ? netif_receive_skb(skb) : netif_rx(skb));

@@ -102,7 +102,7 @@ vlan_gro_common(struct napi_struct *napi, struct vlan_group *grp,
 
 	if (vlan_dev)
 		skb->dev = vlan_dev;
-	else if (vlan_id)
+	else if (vlan_id && !(skb->dev->flags & IFF_PROMISC))
 		goto drop;
 

net/8021q/vlan_core.c vlan_gro_common(struct napi_struct *napi, struct vlan_group *grp, · line 108

Quoted 2 lines

net/8021q/vlan_core.c +0 -0

 	for (p = napi->gro_list; p; p = p->next) {

Hmm, packets are delivered not only to tcpdump but also on other stacks, on ethX.

So this is a domain violation.

This should be fine as long as the packets are properly marked with PACKET_OTHERHOST.

Quoted 11 lines · Jesse Gross

Quoted 5 lines · Roger Luethi

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

Quoted 2 lines

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.

Quoted 12 lines · Roger Luethi

Quoted 11 lines · Jesse Gross

Quoted 5 lines · Roger Luethi

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

Quoted 7 lines

Quoted 2 lines

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)
* Reconstruct the VLAN header when there is no VLAN group (as done by the tg3 driver)

A bunch of drivers do neither (bnx2x, for example) and exhibit this problem. It's getting better but it seems like a common issue.

[PATCH] vlan: dont drop packets from unknown vlans in promiscuous mode

Roger Luethi noticed packets for unknown VLANs getting silently dropped even in promiscuous mode.

Check for promiscuous mode in __vlan_hwaccel_rx() and vlan_gro_common() before drops.

As suggested by Patrick, mark such packets to have skb->pkt_type set to PACKET_OTHERHOST to make sure they are dropped by IP stack.

Reported-by Roger Luethi <[email protected]>

Signed-off-by Eric Dumazet <[email protected]>

CC Patrick McHardy <[email protected]>

Quoted 41 lines · Jesse Gross

Quoted 12 lines · Roger Luethi

Quoted 11 lines · Jesse Gross

Quoted 5 lines · Roger Luethi

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

Quoted 7 lines

Quoted 2 lines

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).

Quoted 8 lines

There are a couple ways to fix this:

* Turn off VLAN stripping when in promiscuous mode (as done by the ixgbe driver)
* Reconstruct the VLAN header when there is no VLAN group (as done by the tg3 driver)

A bunch of drivers do neither (bnx2x, for example) and exhibit this problem.  It's getting better but it seems like a common issue.

tg3 is not perfect, because it does the reconstruction of VLAN header even if device is not in promiscuous mode.

It could drop the frame instead.

I wonder which SNMP counter is incremented in this case.

Apparently, none :(

That's true. Dropping here seems roughly equivalent to the effects of a hardware VLAN filter, which will also not be tracked by a counter, so that seems not too bad to me.

The thing that concerns me though is why so many drivers seem to have this problem with completely dropping the VLAN header. I know that even several of the ones that work now were broken initially and had to be fixed. Seeing as the driver drops the VLAN information before it gets to the general networking code I don't see a generic fix to this as it is currently setup. However, perhaps we could make it so that it is harder to get wrong. Something like this:

* Allow vlan_gro_receive() to take a NULL VLAN group and a tag of 0 (and do the same thing for vlan_hwaccel_rx())
* Now that the vlan functions can deal with non-VLAN packets, merge them into their non-VLAN counterparts.
* We can now demultiplex between the VLAN/non-VLAN case in core networking. This is done anyways, it just prevents every driver from needing that code block I copied above and allows us to fix these types of problems centrally.
* Dump the VLAN tag into the SKB and hand off the packet to the various consumers: VLAN devices, libpcap, bridge hook (not currently done but should be for trunking).

I see a number of advantages of this:
* Fixes all the problems with cards dropping VLAN headers at once.
* Avoids having to disable VLAN acceleration when in promiscuous mode (good for bridging since it always puts devices in promiscuous mode).
* Keeps VLAN tag separate until given to ultimate consumer, which avoids needing to do header reconstruction as in tg3 unless absolutely necessary.
* Consolidates common driver code in core networking.

This seems very reasonable ;)

I'll add a counter, a core generalization of
commit 8990f468a (net: rx_dropped accounting)

Because we can drop packets _after_ netif_rx() if RPS is in action anyway.

Quoted 24 lines · Jesse Gross

That's true. Dropping here seems roughly equivalent to the effects of a hardware VLAN filter, which will also not be tracked by a counter, so that seems not too bad to me.

The thing that concerns me though is why so many drivers seem to have this problem with completely dropping the VLAN header. I know that even several of the ones that work now were broken initially and had to be fixed. Seeing as the driver drops the VLAN information before it gets to the general networking code I don't see a generic fix to this as it is currently setup. However, perhaps we could make it so that it is harder to get wrong. Something like this:

* Allow vlan_gro_receive() to take a NULL VLAN group and a tag of 0 (and do the same thing for vlan_hwaccel_rx())
* Now that the vlan functions can deal with non-VLAN packets, merge them into their non-VLAN counterparts.
* We can now demultiplex between the VLAN/non-VLAN case in core networking. This is done anyways, it just prevents every driver from needing that code block I copied above and allows us to fix these types of problems centrally.
* Dump the VLAN tag into the SKB and hand off the packet to the various consumers: VLAN devices, libpcap, bridge hook (not currently done but should be for trunking).

Quoted 8 lines

I see a number of advantages of this:
* Fixes all the problems with cards dropping VLAN headers at once.
* Avoids having to disable VLAN acceleration when in promiscuous mode (good for bridging since it always puts devices in promiscuous mode).
* Keeps VLAN tag separate until given to ultimate consumer, which avoids needing to do header reconstruction as in tg3 unless absolutely necessary.
* Consolidates common driver code in core networking.

This seems very reasonable ;)

Quoted 35 lines · Eric Dumazet

Quoted 32 lines · Jesse Gross

That's true.  Dropping here seems roughly equivalent to the effects of a hardware VLAN filter, which will also not be tracked by a counter, so that seems not too bad to me.

The thing that concerns me though is why so many drivers seem to have this problem with completely dropping the VLAN header.  I know that even several of the ones that work now were broken initially and had to be fixed.  Seeing as the driver drops the VLAN information before it gets to the general networking code I don't see a generic fix to this as it is currently setup.  However, perhaps we could make it so that it is harder to get wrong.  Something like this:

* Allow vlan_gro_receive() to take a NULL VLAN group and a tag of 0 (and do the same thing for vlan_hwaccel_rx())
* Now that the vlan functions can deal with non-VLAN packets, merge them into their non-VLAN counterparts.
* We can now demultiplex between the VLAN/non-VLAN case in core networking.  This is done anyways, it just prevents every driver from needing that code block I copied above and allows us to fix these types of problems centrally.
* Dump the VLAN tag into the SKB and hand off the packet to the various consumers: VLAN devices, libpcap, bridge hook (not currently done but should be for trunking).

I see a number of advantages of this:
* Fixes all the problems with cards dropping VLAN headers at once.
* Avoids having to disable VLAN acceleration when in promiscuous mode (good for bridging since it always puts devices in promiscuous mode).
* Keeps VLAN tag separate until given to ultimate consumer, which avoids needing to do header reconstruction as in tg3 unless absolutely necessary.
* Consolidates common driver code in core networking.

Quoted 1 line

This seems very reasonable ;)

Jesse, do you plan to work on this stuff yourself in a near future ?

Quoted 9 lines · Eric Dumazet

This seems very reasonable ;)

I'll add a counter, a core generalization of
commit 8990f468a (net: rx_dropped accounting)

Because we can drop packets _after_ netif_rx() if RPS is in action anyway.

In this patch I fold the additional dev->rx_dropped into get_stats() structure. We might chose to not fold it, and provides this counter in a new /proc/net/dev column, a new rtnetlink attribute (and appropriate iproute2 change)

What do you think ?

[PATCH net-next] net: add a core netdev->rx_dropped counter

In various situations, a device provides a packet to our stack and we drop it before it enters protocol stack :
- softnet backlog full (accounted in /proc/net/softnet_stat)
- bad vlan tag (not accounted)
- unknown/unregistered protocol (not accounted)

We can handle a per-device counter of such dropped frames at core level, and automatically adds it to the device provided stats (rx_dropped), so that standard tools can be used (ifconfig, ip link, cat /proc/net/dev)

This is a generalization of commit 8990f468a (net: rx_dropped accounting), thus reverting it.

Signed-off-by Eric Dumazet <[email protected]>

Quoted 14 lines · Roger Luethi

Quoted 12 lines · Jesse Gross

Quoted 6 lines · Roger Luethi

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

Quoted 8 lines

Quoted 3 lines

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)

* Reconstruct the VLAN header when there is no VLAN group (as done by the tg3 driver)

A bunch of drivers do neither (bnx2x, for example) and exhibit this problem. It's getting better but it seems like a common issue. --
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 53 lines · Jesse Gross

Quoted 23 lines · Roger Luethi

Quoted 11 lines · Jesse Gross

Quoted 7 lines · Roger Luethi

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.

Quoted 8 lines

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.

Quoted 6 lines

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).

Quoted 5 lines

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?

Quoted 68 lines · Guillaume Gaudonville

Quoted 52 lines · Jesse Gross

Quoted 22 lines · Roger Luethi

Quoted 10 lines · Jesse Gross

Quoted 7 lines · Roger Luethi

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.

Quoted 9 lines

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.

Quoted 7 lines

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).

Quoted 6 lines

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.

Quoted 8 lines

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 protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 104 lines · Jesse Gross

Quoted 83 lines · Guillaume Gaudonville

Quoted 65 lines · Jesse Gross

Quoted 31 lines · Roger Luethi

Quoted 16 lines · Jesse Gross

Quoted 11 lines · Roger Luethi

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.

Quoted 10 lines

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.

Quoted 9 lines

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).

Quoted 6 lines

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.

Quoted 9 lines

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:

Quoted 6 lines

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 protected]
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.

Quoted 112 lines · Guillaume Gaudonville

Quoted 110 lines · Jesse Gross

Quoted 82 lines · Guillaume Gaudonville

Quoted 64 lines · Jesse Gross

Quoted 30 lines · Roger Luethi

Quoted 15 lines · Jesse Gross

Quoted 11 lines · Roger Luethi

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.

Quoted 11 lines

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.

Quoted 10 lines

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).

Quoted 7 lines

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.

Quoted 10 lines

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 protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 3 lines

Thank you, I'm going to check these patches and try to apply them in our kernel.

An updated set of patches has been merged into net-2.6, so you might want to try that instead.