: Unclassified vlan packet
Subsystems (2)
PATCH 1/5 : Unclassified vlan packet
To be polite to the PACKET,
Don't kill the unclassified & hardware accelerated vlan packets if netdev is in promiscuous, set packet type with PACKET_OTHERHOST.
Put the vlan tag into skb->cb for all hardware accelerated vlan packets.
--- include/linux/if_vlan.h | 56 +++++++++++++++++++++++++++++----------------- 1 files changed, 35 insertions(+), 21 deletions(-)
include/linux/if_vlan.h +35 -21
@@ -62,6 +62,9 @@ struct vlan_ethhdr {#include <linux/skbuff.h> +static inline struct sk_buff *__vlan_hwaccel_put_tag (struct sk_buff *skb, +unsigned short tag); +static inline struct vlan_ethhdr *vlan_eth_hdr (const struct sk_buff *skb) {return (struct vlan_ethhdr *)skb_mac_header (skb);
@@ -175,14 +178,19 @@ static inline int __vlan_hwaccel_rx(struct sk_buff *skb, unsigned short vlan_tag, int polling) {struct net_device_stats *stats; +struct net_device *vlan_dev;if (skb_bond_should_drop (skb)) {dev_kfree_skb_any (skb);return NET_RX_DROP ; } - skb->dev =vlan_group_get_device (grp, vlan_tag &VLAN_VID_MASK ); -if (skb->dev ==NULL ) { + vlan_dev =vlan_group_get_device (grp, vlan_tag &VLAN_VID_MASK ); +if (vlan_dev) + skb->dev = vlan_dev; +else if (skb->dev->flags &IFF_PROMISC ) + skb->pkt_type =PACKET_OTHERHOST ; +else {dev_kfree_skb_any (skb);/* Not NET_RX_DROP, this is not being dropped
@@ -191,31 +199,37 @@ static inline int __vlan_hwaccel_rx(struct sk_buff *skb,return 0 ; } +/* Deliever vlan_tag to PACKET */ +__vlan_hwaccel_put_tag (skb, vlan_tag); + skb->dev->last_rx = jiffies; stats = &skb->dev->stats; stats->rx_packets++; stats->rx_bytes += skb->len; - skb->priority =vlan_get_ingress_priority (skb->dev, vlan_tag); -switch (skb->pkt_type) { -case PACKET_BROADCAST : -break ; - -case PACKET_MULTICAST : - stats->multicast++; -break ; - -case PACKET_OTHERHOST : -/* Our lower layer thinks this is not local, let's make sure. -* This allows the VLAN to have a different MAC than the underlying -* device, and still route correctly. -*/ -if (!compare_ether_addr (eth_hdr (skb)->h_dest, - skb->dev->dev_addr)) - skb->pkt_type =PACKET_HOST ; -break ; - }; +if (vlan_dev) { + skb->priority =vlan_get_ingress_priority (skb->dev, vlan_tag); +switch (skb->pkt_type) { +case PACKET_BROADCAST : +break ; + +case PACKET_MULTICAST : + stats->multicast++; +break ; + +case PACKET_OTHERHOST : +/* Our lower layer thinks this is not local, let's +* make sure. +* This allows the VLAN to have a different MAC than +* the underlying device, and still route correctly. +*/ +if (!compare_ether_addr (eth_hdr (skb)->h_dest, + skb->dev->dev_addr)) + skb->pkt_type =PACKET_HOST ; +break ; + }; + }return (polling ?netif_receive_skb (skb) :netif_rx (skb)); }
Signature · 1 line
1.5.4.3
Quoted 3 lines · Joonwoo Park
To be polite to the PACKET,
Don't kill the unclassified & hardware accelerated vlan packets if netdev
is in promiscuous, set packet type withPACKET_OTHERHOST. Put the vlan tag intoskb->cbfor all hardware accelerated vlan packets.
Conceptually I think this patch goes in the right direction, one question remaining is when to invalidate the VLAN tag again.
The only solution I could come up with is invalidating it in netif_receive_skb() when the receiving device is not a VLAN device and additionally invalidating it in all callers of dev_queue_xmit except VLAN itself, but I would really prefer something less error prone without touching netif_receive_skb().
BTW, I already have a patch queued to move the VLAN tag from skb->cb to a seperate skb member to fix the the conflict with qdiscs (this should also allow to use vlan accel through virtual network devices later on). So please don't resend, I'll integrate the patch on top of this change once we find a good spot for invalidation.
Quoted 99 lines
Signed-off-by Joonwoo Park <[email protected]>--- include/linux/if_vlan.h | 56 +++++++++++++++++++++++++++++----------------- 1 files changed, 35 insertions(+), 21 deletions(-)include/linux/if_vlan.h +35
-21@@ -62,6 +62,9 @@ struct vlan_ethhdr {#include <linux/skbuff.h> +static inline struct sk_buff *__vlan_hwaccel_put_tag (struct sk_buff *skb, +unsigned short tag); +static inline struct vlan_ethhdr *vlan_eth_hdr (const struct sk_buff *skb) {return (struct vlan_ethhdr *)skb_mac_header (skb);@@ -175,14 +178,19 @@ static inline int __vlan_hwaccel_rx(struct sk_buff *skb, unsigned short vlan_tag, int polling) {struct net_device_stats *stats; +struct net_device *vlan_dev;if (skb_bond_should_drop (skb)) {dev_kfree_skb_any (skb);return NET_RX_DROP ; } - skb->dev =vlan_group_get_device (grp, vlan_tag &VLAN_VID_MASK ); -if (skb->dev ==NULL ) { + vlan_dev =vlan_group_get_device (grp, vlan_tag &VLAN_VID_MASK ); +if (vlan_dev) + skb->dev = vlan_dev; +else if (skb->dev->flags &IFF_PROMISC ) + skb->pkt_type =PACKET_OTHERHOST ; +else {dev_kfree_skb_any (skb);/* Not NET_RX_DROP, this is not being dropped @@ -191,31 +199,37 @@ static inline int __vlan_hwaccel_rx(struct sk_buff *skb,return 0 ; } +/* Deliever vlan_tag to PACKET */ +__vlan_hwaccel_put_tag (skb, vlan_tag); + skb->dev->last_rx = jiffies; stats = &skb->dev->stats; stats->rx_packets++; stats->rx_bytes += skb->len; - skb->priority =vlan_get_ingress_priority (skb->dev, vlan_tag); -switch (skb->pkt_type) { -case PACKET_BROADCAST : -break ; - -case PACKET_MULTICAST : - stats->multicast++; -break ; - -case PACKET_OTHERHOST : -/* Our lower layer thinks this is not local, let's make sure. -* This allows the VLAN to have a different MAC than the underlying -* device, and still route correctly. -*/ -if (!compare_ether_addr (eth_hdr (skb)->h_dest, - skb->dev->dev_addr)) - skb->pkt_type =PACKET_HOST ; -break ; - }; +if (vlan_dev) { + skb->priority =vlan_get_ingress_priority (skb->dev, vlan_tag); +switch (skb->pkt_type) { +case PACKET_BROADCAST : +break ; + +case PACKET_MULTICAST : + stats->multicast++; +break ; + +case PACKET_OTHERHOST : +/* Our lower layer thinks this is not local, let's +* make sure. +* This allows the VLAN to have a different MAC than +* the underlying device, and still route correctly. +*/ +if (!compare_ether_addr (eth_hdr (skb)->h_dest, + skb->dev->dev_addr)) + skb->pkt_type =PACKET_HOST ; +break ; + }; + }return (polling ?netif_receive_skb (skb) :netif_rx (skb)); }
2008/5/26 Patrick McHardy <[email protected]>:
Quoted 16 lines
Quoted 5 lines · Joonwoo Park
To be polite to the PACKET,
Don't kill the unclassified & hardware accelerated vlan packets if netdev is in promiscuous, set packet type withPACKET_OTHERHOST. Put the vlan tag intoskb->cbfor all hardware accelerated vlan packets.Conceptually I think this patch goes in the right direction, one question remaining is when to invalidate the VLAN tag again.
The only solution I could come up with is invalidating it in
netif_receive_skb()when the receiving device is not a VLAN device and additionally invalidating it in all callers ofdev_queue_xmitexcept VLAN itself, but I would really prefer something less error prone without touchingnetif_receive_skb().
Quoted 7 lines
BTW, I already have a patch queued to move the VLAN tag from
skb->cbto a seperate skb member to fix the the conflict with qdiscs (this should also allow to use vlan accel through virtual network devices later on). So please don't resend, I'll integrate the patch on top of this change once we find a good spot for invalidation.
Thanks Patrick for reviewing.
I'll be looking forward to seeing it on the list.
Thanks,
Joonwoo
Quoted 23 lines · Joonwoo Park
2008/5/26 Patrick McHardy <[email protected]>:
Quoted 14 lines
Quoted 4 lines · Joonwoo Park
To be polite to the PACKET,
Don't kill the unclassified & hardware accelerated vlan packets if netdev is in promiscuous, set packet type withPACKET_OTHERHOST. Put the vlan tag intoskb->cbfor all hardware accelerated vlan packets.Conceptually I think this patch goes in the right direction, one question remaining is when to invalidate the VLAN tag again.
The only solution I could come up with is invalidating it in
netif_receive_skb()when the receiving device is not a VLAN device and additionally invalidating it in all callers ofdev_queue_xmitexcept VLAN itself, but I would really prefer something less error prone without touchingnetif_receive_skb().Quoted 7 lines
BTW, I already have a patch queued to move the VLAN tag from
skb->cbto a seperate skb member to fix the the conflict with qdiscs (this should also allow to use vlan accel through virtual network devices later on). So please don't resend, I'll integrate the patch on top of this change once we find a good spot for invalidation.
Quoted 2 lines
Thanks Patrick for reviewing.
I'll be looking forward to seeing it on the list.
Well, we still need to find a good spot for invalidation. Suggestions welcome :)
2008/5/26 Patrick McHardy <[email protected]>:
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
BTW, I already have a patch queued to move the VLAN tag from
skb->cbto a seperate skb member to fix the the conflict with qdiscs (this should also allow to use vlan accel through virtual network devices later on). So please don't resend, I'll integrate the patch on top of this change once we find a good spot for invalidation.
Was that patch applied to certain tree?
If it was, may I know what tree is it?
Quoted 4 lines
Well, we still need to find a good spot for invalidation. Suggestions welcome :)
huh, also I'll try to find it! :-)
Thanks,
Joonwoo
Quoted 11 lines · Joonwoo Park
2008/5/26 Patrick McHardy <[email protected]>:
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
BTW, I already have a patch queued to move the VLAN tag from
skb->cbto a seperate skb member to fix the the conflict with qdiscs (this should also allow to use vlan accel through virtual network devices later on). So please don't resend, I'll integrate the patch on top of this change once we find a good spot for invalidation.Was that patch applied to certain tree?
If it was, may I know what tree is it?
I'm uploading it to kernel.org now, should appear within the next 30 minutes at:
git://git.kernel.org/pub/scm/linux/kernel/git/kaber/vlan-next-2.6.git
2008/5/27 Patrick McHardy <[email protected]>:
Quoted 12 lines
Quoted 11 lines · Joonwoo Park
2008/5/26 Patrick McHardy <[email protected]>:
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
BTW, I already have a patch queued to move the VLAN tag from
skb->cbto a seperate skb member to fix the the conflict with qdiscs (this should also allow to use vlan accel through virtual network devices later on). So please don't resend, I'll integrate the patch on top of this change once we find a good spot for invalidation.
Quoted 8 lines
Quoted 2 lines
Was that patch applied to certain tree?
If it was, may I know what tree is it?I'm uploading it to kernel.org now, should appear within the next 30 minutes at:
git://git.kernel.org/pub/scm/linux/kernel/git/kaber/vlan-next-2.6.git
Thanks a lot Patrick!
Joonwoo
2008/5/26 Patrick McHardy <[email protected]>:
Quoted 30 lines
Quoted 29 lines · Joonwoo Park
2008/5/26 Patrick McHardy <[email protected]>:
Quoted 26 lines
Quoted 7 lines · Joonwoo Park
To be polite to the PACKET,
Don't kill the unclassified & hardware accelerated vlan packets if netdev
is in promiscuous, set packet type withPACKET_OTHERHOST. Put the vlan tag
intoskb->cbfor all hardware accelerated vlan packets.Conceptually I think this patch goes in the right direction, one question remaining is when to invalidate the VLAN tag again.
The only solution I could come up with is invalidating it in
netif_receive_skb()when the receiving device is not a VLAN device and additionally invalidating it in all callers ofdev_queue_xmitexcept VLAN itself, but I would really prefer something less error prone without touchingnetif_receive_skb().BTW, I already have a patch queued to move the VLAN tag from
skb->cbto a seperate skb member to fix the the conflict with qdiscs (this should also allow to use vlan accel through virtual network devices later on). So please don't resend, I'll integrate the patch on top of this change once we find a good spot for invalidation.
Quoted 7 lines
Quoted 2 lines
Thanks Patrick for reviewing.
I'll be looking forward to seeing it on the list.Well, we still need to find a good spot for invalidation. Suggestions welcome :)
Patrick,
Do you mind inserting a new flag to indicate vlan_tag is just for af_packet or not into sk_buff?
I think if flag exists, we can pass vlan_tag to af_packet with just modifications of vlan_get_tag(), vlan_put_tag() and vlan_tag_present(),
without patching for invalidation.
Thanks,
Joonwoo
Quoted 7 lines · Joonwoo Park
Patrick,
Do you mind inserting a new flag to indicatevlan_tagis just foraf_packetor not intosk_buff?
I think if flag exists, we can passvlan_tagtoaf_packetwith just modifications ofvlan_get_tag(),vlan_put_tag()andvlan_tag_present(),
without patching for invalidation.
I've considered this, but that would mean that we report the vlan tag for all devices layered on top, which seems wrong.
For now I think I'm going to temporarily do something like that so I can continue integrating the remaining patches and fix it properly later.
Quoted 9 lines · Patrick McHardy
Quoted 7 lines · Joonwoo Park
Patrick,
Do you mind inserting a new flag to indicatevlan_tagis just foraf_packetor not intosk_buff?
I think if flag exists, we can passvlan_tagtoaf_packetwith just modifications ofvlan_get_tag(),vlan_put_tag()andvlan_tag_present(),
without patching for invalidation.
Quoted 7 lines
I've considered this, but that would mean that we report the vlan tag for all devices layered on top, which seems wrong.
For now I think I'm going to temporarily do something like that so I can continue integrating the remaining patches and fix it properly later.
The entire idea didn't work out too well. Your original patch passed packets for unknown VLANs to netif_receive_skb again, but since they don't have a VLAN header, they are treated like a normal non-VLAN packet and might end up in a protocol handler instead of getting dropped. It would also have left one inconsistency, for locally configured VLANs the packet is still not visible on the lower device.
This patch (on top of my local tree with other changes, I'll post the entire series soon) takes a different approach. __vlan_hwaccel_rx() is supposed to behave similar to netif_receive_skb(), so the easiest fix is to make it behave more like netif_receive_skb() and deliver packets to ptype_all handlers manually.
commit 5110748e684267ca3d381f47ab972d47ddf178c6
Author: Patrick McHardy <[email protected]> Date: Sat Jul 5 21:32:29 2008 +0200
vlan: deliver packets received with VLAN acceleration to network taps
When VLAN header stripping is used, packets currently bypass packet sockets (and other network taps) completely. For locally existing VLANs, they appear directly on the VLAN device, for unknown VLANs they are silently dropped.
Add a new function netif_nit_deliver() to deliver incoming packets to all network interface taps and use it in __vlan_hwaccel_rx() to make VLAN packets visible on the underlying device.
include/linux/netdevice.h +1 -0
@@ -1144,6 +1144,7 @@ extern int netif_rx(struct sk_buff *skb);extern int netif_rx_ni (struct sk_buff *skb);#define HAVE_NETIF_RECEIVE_SKB 1extern int netif_receive_skb (struct sk_buff *skb); +extern void netif_nit_deliver (struct sk_buff *skb);extern int dev_valid_name (const char *name);extern int dev_ioctl (struct net *net,unsigned int cmd,void __user *);extern int dev_ethtool (struct net *net,struct ifreq *);
net/8021q/vlan_core.c +4 -0
@@ -14,6 +14,9 @@ int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,return NET_RX_DROP ; } + skb->vlan_tag = vlan_tag; +netif_nit_deliver (skb); + skb->dev =vlan_group_get_device (grp, vlan_tag &VLAN_VID_MASK );if (skb->dev ==NULL ) {dev_kfree_skb_any (skb);
@@ -23,6 +26,7 @@ int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,*/ return 0 ; } + skb->vlan_tag =0 ; skb->dev->last_rx = jiffies;
net/core/dev.c +32 -0
@@ -2030,6 +2030,38 @@ out: }#endif +/* +* netif_tap_deliver - deliver received packets to network taps +* @skb: buffer +* +* This function is used to deliver incoming packets to network +* taps. It should be used when the normal netif_receive_skb path +* is bypassed, for example because of VLAN acceleration. +*/ +void netif_nit_deliver (struct sk_buff *skb) +{ +struct packet_type *ptype, *pt_prev =NULL ; + +if (list_empty (&ptype_all)) +return ; + +skb_reset_network_header (skb); +skb_reset_transport_header (skb); + skb->mac_len = skb->network_header - skb->mac_header; + +rcu_read_lock (); +list_for_each_entry_rcu (ptype, &ptype_all, list) { +if (!ptype->dev || ptype->dev == skb->dev) { +if (pt_prev) +deliver_skb (skb, pt_prev, skb->dev); + pt_prev = ptype; + } + } +if (pt_prev) + pt_prev->func (skb, skb->dev, pt_prev, skb->dev); +rcu_read_unlock (); +} +/** * netif_receive_skb - process receive buffer from network * @skb: buffer to process