[Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables "see" bridged VLAN tagged{I,AR}P packets

[Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables "see" bridged VLAN tagged{I,AR}P packets

[Christian Darnell]

>-----Original Message-----
>From: Ben Greear [mailto:greearb@candelatech.com]
>Sent: Monday, October 06, 2003 10:24 PM
>To: Bart De Schuymer
>Cc: netdev@oss.sgi.com; vlan@wanfear.com; bridge
>Subject: [VLAN] Re: [PATCH/RFC] Let {ip,arp}tables "see" bridged VLAN
>tagged{I,AR}P packets
>
>
>Bart De Schuymer wrote:
>> - add some code in vlan_dev.c::vlan_dev_hard_start_xmit():
>> skb->protocol = __constant_htons(ETH_P_8021Q);
>> skb->mac.raw -= VLAN_HLEN;
>> skb->nh.raw -= VLAN_HLEN;
>
>I wonder if this is what was messing up the tcpdump packet capture
>as well?

Hi Ben and all others, 

Just to clarify for other who hasn't been a part of this discussion before. 

---- 8< ----
When trying to grab a packet with pcap when using VLAN the beginning of the
packet is corrupt an the VLAN TCI bits are missing. This is only a problem
when sniffing on incoming traffic not outgoing.

00 60 08 50 00 60 08 50 26 2a 00 60 08 6a b4 53 xx xx xx xx 08 00 45 00
^^^^^^^^^^^                                     ^^^^^^^^^^^^
Where does these bytes come from?               Bytes missing (VLAN header)?

The correct MAC addresses here are:
00 60 08 50 26 2a and 00 60 08 6a b4 53
---- 8< ----


I added those lines (see above) to vlan_dev.c but it didn't solve the
problem with tcpdump (pcap) incoming packets. vlan_dev_hard_start_xmit()
tells me that this is only outgoing packets right? For outgoing packets
tcpdump works (and Ethereal for Windows sees the packet correctly on the
wire) so the problem doesn't occur when sending packets. I added following
line to vlan_dev.c:

vlan_dev.c:: int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
struct packet_type* ptype):

vlan_TCI = ntohs(vhdr->h_vlan_TCI);

+ /* Print the 802.1p priority */
+printk("VLAN Prio: %hx\n", vlan_TCI >> 13);

vid = (vlan_TCI & VLAN_VID_MASK);

With the code above I get the 802.1p correctly. Does anyone know how and
when pcap grabs the packet (does pcap grab the packet before or after
vlan_skb_recv is called)?

When I'm sniffing with pcap I sniff the Ethernet interface, so the VLAN
stuff isn't really involved when pcap grab the packet or have I
misunderstand how it works?


Best Regards, 

Christian Darnell

Re: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables "see" bridged VLAN tagged{I,AR}P packets

[Tommy Christensen]

On Tue, 2003-10-07 at 11:06, Christian Darnell wrote:

> Hi Ben and all others, 
> 
> Just to clarify for other who hasn't been a part of this discussion before. 
> 
> ---- 8< ----
> When trying to grab a packet with pcap when using VLAN the beginning of the
> packet is corrupt an the VLAN TCI bits are missing. This is only a problem
> when sniffing on incoming traffic not outgoing.
> 
> 00 60 08 50 00 60 08 50 26 2a 00 60 08 6a b4 53 xx xx xx xx 08 00 45 00
> ^^^^^^^^^^^                                     ^^^^^^^^^^^^
> Where does these bytes come from?               Bytes missing (VLAN header)?
> 
> The correct MAC addresses here are:
> 00 60 08 50 26 2a and 00 60 08 6a b4 53
> ---- 8< ----

This is because the VLAN code is mangling shared data.
You need to do something like this:


--- linux-2.4/net/8021q/vlan_dev.c.org	2003-02-25 15:23:09.000000000
+0100
+++ linux-2.4/net/8021q/vlan_dev.c	2003-10-07 16:01:29.000000000 +0200
@@ -75,7 +75,12 @@
 static inline struct sk_buff *vlan_check_reorder_header(struct sk_buff
*skb)
 {
 	if (VLAN_DEV_INFO(skb->dev)->flags & 1) {
-		skb = skb_share_check(skb, GFP_ATOMIC);
+		if (skb_shared(skb) || skb_cloned(skb)) {
+			struct sk_buff *nskb;
+			nskb = skb_copy(skb, GFP_ATOMIC);
+			kfree_skb(skb);
+			skb = nskb;
+		}
 		if (skb) {
 			/* Lifted from Gleb's VLAN code... */
 			memmove(skb->data - ETH_HLEN,


Christian, could you try this out?


Regarding sharing, the following should be applied as well.
The VLAN code is handed shared sk_buff's, but doesn't handle them
as such.


--- linux-2.4/net/8021q/vlan.c.org	2003-02-25 15:23:09.000000000 +0100
+++ linux-2.4/net/8021q/vlan.c	2003-10-07 16:02:52.000000000 +0200
@@ -67,7 +67,7 @@
 	type: __constant_htons(ETH_P_8021Q),
 	dev:  NULL,
 	func: vlan_skb_recv, /* VLAN receive method */
-	data: (void *)(-1),  /* Set here '(void *)1' when this code can SHARE
SKBs */
+	data: NULL,          /* Set here '(void *)1' when this code can SHARE
SKBs */
 	next: NULL
 };
 

I guess this is a special case of "off-by-one" ;-)

-Tommy

Re: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables "see" bridged VLAN tagged{I,AR}P packets

[Tommy Christensen]

On Tue, 2003-10-07 at 11:06, Christian Darnell wrote:

> Hi Ben and all others, 
> 
> Just to clarify for other who hasn't been a part of this discussion before. 
> 
> ---- 8< ----
> When trying to grab a packet with pcap when using VLAN the beginning of the
> packet is corrupt an the VLAN TCI bits are missing. This is only a problem
> when sniffing on incoming traffic not outgoing.
> 
> 00 60 08 50 00 60 08 50 26 2a 00 60 08 6a b4 53 xx xx xx xx 08 00 45 00
> ^^^^^^^^^^^                                     ^^^^^^^^^^^^
> Where does these bytes come from?               Bytes missing (VLAN header)?
> 
> The correct MAC addresses here are:
> 00 60 08 50 26 2a and 00 60 08 6a b4 53
> ---- 8< ----

This is because the VLAN code is mangling shared data.
You need to do something like this:


--- linux-2.4/net/8021q/vlan_dev.c.org	2003-02-25 15:23:09.000000000
+0100
+++ linux-2.4/net/8021q/vlan_dev.c	2003-10-07 16:01:29.000000000 +0200
@@ -75,7 +75,12 @@
 static inline struct sk_buff *vlan_check_reorder_header(struct sk_buff
*skb)
 {
 	if (VLAN_DEV_INFO(skb->dev)->flags & 1) {
-		skb = skb_share_check(skb, GFP_ATOMIC);
+		if (skb_shared(skb) || skb_cloned(skb)) {
+			struct sk_buff *nskb;
+			nskb = skb_copy(skb, GFP_ATOMIC);
+			kfree_skb(skb);
+			skb = nskb;
+		}
 		if (skb) {
 			/* Lifted from Gleb's VLAN code... */
 			memmove(skb->data - ETH_HLEN,


Christian, could you try this out?


Regarding sharing, the following should be applied as well.
The VLAN code is handed shared sk_buff's, but doesn't handle them
as such.


--- linux-2.4/net/8021q/vlan.c.org	2003-02-25 15:23:09.000000000 +0100
+++ linux-2.4/net/8021q/vlan.c	2003-10-07 16:02:52.000000000 +0200
@@ -67,7 +67,7 @@
 	type: __constant_htons(ETH_P_8021Q),
 	dev:  NULL,
 	func: vlan_skb_recv, /* VLAN receive method */
-	data: (void *)(-1),  /* Set here '(void *)1' when this code can SHARE
SKBs */
+	data: NULL,          /* Set here '(void *)1' when this code can SHARE
SKBs */
 	next: NULL
 };
 

I guess this is a special case of "off-by-one" ;-)

-Tommy

RE: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables "see" bridged VLAN tagged{I,AR}P packets

[Christian Darnell]

-----Original Message-----
>From: Tommy Christensen [mailto:tommy.christensen@tpack.net]
>Sent: Wednesday, October 08, 2003 10:09 AM
>To: Christian Darnell
>Cc: 'Linux 802.1Q VLAN'; Bart De Schuymer; netdev@oss.sgi.com; bridge
>Subject: Re: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables
>"see" bridged VLAN tagged{I,AR}P packets
>
>
>
>This is because the VLAN code is mangling shared data.
>You need to do something like this:
>
>
>--- linux-2.4/net/8021q/vlan_dev.c.org	2003-02-25 15:23:09.000000000
>+0100
>+++ linux-2.4/net/8021q/vlan_dev.c	2003-10-07 16:01:29.000000000 +0200
>@@ -75,7 +75,12 @@
> static inline struct sk_buff *vlan_check_reorder_header(struct sk_buff
>*skb)
> {
> 	if (VLAN_DEV_INFO(skb->dev)->flags & 1) {
>-		skb = skb_share_check(skb, GFP_ATOMIC);
>+		if (skb_shared(skb) || skb_cloned(skb)) {
>+			struct sk_buff *nskb;
>+			nskb = skb_copy(skb, GFP_ATOMIC);
>+			kfree_skb(skb);
>+			skb = nskb;
>+		}
> 		if (skb) {
> 			/* Lifted from Gleb's VLAN code... */
> 			memmove(skb->data - ETH_HLEN,
>
>
>Christian, could you try this out?


Thanks Tommy! I tried this (on kernel 2.4.22) and it works great!


Best Regards, 

Christian Darnell

RE: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables "see" bridged VLAN tagged{I,AR}P packets

[Christian Darnell]

-----Original Message-----
>From: Tommy Christensen [mailto:tommy.christensen@tpack.net]
>Sent: Wednesday, October 08, 2003 10:09 AM
>To: Christian Darnell
>Cc: 'Linux 802.1Q VLAN'; Bart De Schuymer; netdev@oss.sgi.com; bridge
>Subject: Re: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables
>"see" bridged VLAN tagged{I,AR}P packets
>
>
>
>This is because the VLAN code is mangling shared data.
>You need to do something like this:
>
>
>--- linux-2.4/net/8021q/vlan_dev.c.org	2003-02-25 15:23:09.000000000
>+0100
>+++ linux-2.4/net/8021q/vlan_dev.c	2003-10-07 16:01:29.000000000 +0200
>@@ -75,7 +75,12 @@
> static inline struct sk_buff *vlan_check_reorder_header(struct sk_buff
>*skb)
> {
> 	if (VLAN_DEV_INFO(skb->dev)->flags & 1) {
>-		skb = skb_share_check(skb, GFP_ATOMIC);
>+		if (skb_shared(skb) || skb_cloned(skb)) {
>+			struct sk_buff *nskb;
>+			nskb = skb_copy(skb, GFP_ATOMIC);
>+			kfree_skb(skb);
>+			skb = nskb;
>+		}
> 		if (skb) {
> 			/* Lifted from Gleb's VLAN code... */
> 			memmove(skb->data - ETH_HLEN,
>
>
>Christian, could you try this out?


Thanks Tommy! I tried this (on kernel 2.4.22) and it works great!


Best Regards, 

Christian Darnell

RE: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables "see" bridged VLAN tagged{I,AR}P packets

[Christian Darnell]

>-----Original Message-----
>From: Christian Darnell [mailto:Christian.Darnell@se.flextronics.com]
>Sent: Wednesday, October 08, 2003 10:18 AM
>To: 'Tommy Christensen'
>Cc: netdev@oss.sgi.com; 'Linux 802.1Q VLAN'; bridge
>Subject: RE: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables
>"see" bridged VLAN tagged{I,AR}P packets
>
>
>
>-----Original Message-----
>>From: Tommy Christensen [mailto:tommy.christensen@tpack.net]
>>Sent: Wednesday, October 08, 2003 10:09 AM
>>To: Christian Darnell
>>Cc: 'Linux 802.1Q VLAN'; Bart De Schuymer; netdev@oss.sgi.com; bridge
>>Subject: Re: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables
>>"see" bridged VLAN tagged{I,AR}P packets
>>
>>
>>
>>This is because the VLAN code is mangling shared data.
>>You need to do something like this:
>>
>>
>>--- linux-2.4/net/8021q/vlan_dev.c.org	2003-02-25
15:23:09.000000000
>>+0100
>>+++ linux-2.4/net/8021q/vlan_dev.c	2003-10-07 16:01:29.000000000 +0200
>>@@ -75,7 +75,12 @@
>> static inline struct sk_buff *vlan_check_reorder_header(struct sk_buff
>>*skb)
>> {
>> 	if (VLAN_DEV_INFO(skb->dev)->flags & 1) {
>>-		skb = skb_share_check(skb, GFP_ATOMIC);
>>+		if (skb_shared(skb) || skb_cloned(skb)) {
>>+			struct sk_buff *nskb;
>>+			nskb = skb_copy(skb, GFP_ATOMIC);
>>+			kfree_skb(skb);
>>+			skb = nskb;
>>+		}
>> 		if (skb) {
>> 			/* Lifted from Gleb's VLAN code... */
>> 			memmove(skb->data - ETH_HLEN,
>>
>>
>>Christian, could you try this out?
>
>
>Thanks Tommy! I tried this (on kernel 2.4.22) and it works great!
>
>

Hi again, when the problem with VLAN and pcap was solved I found a bug in
pcap. The problem is that pcap_setfilter doesn't work correctly when using
vlan x. The problem is that pcap don't mask VID. This results in that for
packets with priority (802.1q) set to 0 the vlan x filter works fine, but
when the packet has a priority != 0 the vlan x fails. 

An example:
If one have a vlan with VID = 2 and sniffing using the filter "vlan 2"
packets with priority = 0 will be shown. Changing to priority = 1 the
packets wont appear, if one changes the filter to "vlan 8194" the packets
will appear.

Fixing so pcap will mask the TCI field with "& 0xfff " to get the VID should
probably fix this problem. I have try to locate where this check is
performed in the pcap code but with no success. I think
gencode.c::gen_vlan(vlan_num) has something to do with...

Is there someone who knows the pcap code and know where this check is done?
I have tried with tcpdump-workers@tcpdump.org before without any response,
is there anyone who knows how to contact the maintainers of the libpcap?


Best regards,

Christian Darnell

Re: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables "see" bridged VLAN tagged{I,AR}P packets

[David S. Miller]

On 08 Oct 2003 10:09:25 +0200
Tommy Christensen <tommy.christensen@tpack.net> wrote:

> This is because the VLAN code is mangling shared data.
> You need to do something like this:

This fix is definitely needed and correct, patch applied
thanks.

Re: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables "see" bridged VLAN tagged{I,AR}P packets

[David S. Miller]

On 08 Oct 2003 10:09:25 +0200
Tommy Christensen <tommy.christensen@tpack.net> wrote:

> This is because the VLAN code is mangling shared data.
> You need to do something like this:

This fix is definitely needed and correct, patch applied
thanks.

Re: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables "see" bridged VLAN tagged{I,AR}P packets

[David S. Miller]

On Wed, 08 Oct 2003 09:34:20 -0700
Ben Greear <greearb@candelatech.com> wrote:

> So, what good is skb_share_check then?
> Maybe we should have a skb_share_or_cloned_check() ?

What input handlers are supposed to do is first:

	skb = skb_share_check(...);


then look at packet contents etc., then if they need to write
to the header do something like skb_cow().

The best example, as usual, is ipv4 input.  Look at how ip_rcv()
makes sure it can safely get at the packet header parts it needs
to parse, then look at ip_forward and how it cows the IPV4 header
so it can modify the TTL field.

Re: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables "see" bridged VLAN tagged{I,AR}P packets

[David S. Miller]

On Wed, 08 Oct 2003 09:34:20 -0700
Ben Greear <greearb@candelatech.com> wrote:

> So, what good is skb_share_check then?
> Maybe we should have a skb_share_or_cloned_check() ?

What input handlers are supposed to do is first:

	skb = skb_share_check(...);


then look at packet contents etc., then if they need to write
to the header do something like skb_cow().

The best example, as usual, is ipv4 input.  Look at how ip_rcv()
makes sure it can safely get at the packet header parts it needs
to parse, then look at ip_forward and how it cows the IPV4 header
so it can modify the TTL field.

Re: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables "see" bridged VLAN tagged{I,AR}P packets

[Ben Greear]

Christian Darnell wrote:
> -----Original Message-----
> 
>>From: Tommy Christensen [mailto:tommy.christensen@tpack.net]
>>Sent: Wednesday, October 08, 2003 10:09 AM
>>To: Christian Darnell
>>Cc: 'Linux 802.1Q VLAN'; Bart De Schuymer; netdev@oss.sgi.com; bridge
>>Subject: Re: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables
>>"see" bridged VLAN tagged{I,AR}P packets
>>
>>
>>
>>This is because the VLAN code is mangling shared data.
>>You need to do something like this:
>>
>>
>>--- linux-2.4/net/8021q/vlan_dev.c.org	2003-02-25 15:23:09.000000000
>>+0100
>>+++ linux-2.4/net/8021q/vlan_dev.c	2003-10-07 16:01:29.000000000 +0200
>>@@ -75,7 +75,12 @@
>>static inline struct sk_buff *vlan_check_reorder_header(struct sk_buff
>>*skb)
>>{
>>	if (VLAN_DEV_INFO(skb->dev)->flags & 1) {
>>-		skb = skb_share_check(skb, GFP_ATOMIC);
>>+		if (skb_shared(skb) || skb_cloned(skb)) {
>>+			struct sk_buff *nskb;
>>+			nskb = skb_copy(skb, GFP_ATOMIC);
>>+			kfree_skb(skb);
>>+			skb = nskb;
>>+		}

Thanks for catching that!

So, what good is skb_share_check then?
Maybe we should have a skb_share_or_cloned_check() ?

Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables "see" bridged VLAN tagged{I,AR}P packets

[Ben Greear]

Christian Darnell wrote:
> -----Original Message-----
> 
>>From: Tommy Christensen [mailto:tommy.christensen@tpack.net]
>>Sent: Wednesday, October 08, 2003 10:09 AM
>>To: Christian Darnell
>>Cc: 'Linux 802.1Q VLAN'; Bart De Schuymer; netdev@oss.sgi.com; bridge
>>Subject: Re: [Bridge] RE: [VLAN] Re: [PATCH/RFC] Let {ip, arp}tables
>>"see" bridged VLAN tagged{I,AR}P packets
>>
>>
>>
>>This is because the VLAN code is mangling shared data.
>>You need to do something like this:
>>
>>
>>--- linux-2.4/net/8021q/vlan_dev.c.org	2003-02-25 15:23:09.000000000
>>+0100
>>+++ linux-2.4/net/8021q/vlan_dev.c	2003-10-07 16:01:29.000000000 +0200
>>@@ -75,7 +75,12 @@
>>static inline struct sk_buff *vlan_check_reorder_header(struct sk_buff
>>*skb)
>>{
>>	if (VLAN_DEV_INFO(skb->dev)->flags & 1) {
>>-		skb = skb_share_check(skb, GFP_ATOMIC);
>>+		if (skb_shared(skb) || skb_cloned(skb)) {
>>+			struct sk_buff *nskb;
>>+			nskb = skb_copy(skb, GFP_ATOMIC);
>>+			kfree_skb(skb);
>>+			skb = nskb;
>>+		}

Thanks for catching that!

So, what good is skb_share_check then?
Maybe we should have a skb_share_or_cloned_check() ?

Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com