Skip to contentSkip to conversation

VLAN header accounting for packets leaving bridge

Bridge Cameron Schaus 3 participants 4 messages Jul 30, 2007

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

I have been working on an ebtables module to insert an 802.1Q (VLAN) tags into network packets leaving the bridge, via the POSTROUTING NAT chain. To insert the VLAN tag the ethernet header size needs to be increased by 4 bytes. I noticed that after increasing the size of the ethernet header that the ethernet header in packets on the wire was incorrect.

I think it is because the br_dev_queue_push_xmit function does not take the VLAN header into account when it moves the data pointer in the skb to the start of the ethernet header. The following patch corrects this.

Does this make sense to do? I plan to do more extensive testing passing vlan and non-vlan traffic across a bridge, and will report back with the results.

net/bridge/br_forward.c +2 -0
index ada7f49..f30dc56 100644
@@ -43,6 +43,8 @@ int br_dev_queue_push_xmit(struct sk_buff *skb)
                      kfree_skb(skb);
              else {
                      skb_push(skb, ETH_HLEN);
+                       if(skb->protocol == htons(ETH_P_8021Q))
+                         skb_push(skb, VLAN_HLEN);
 
                      dev_queue_xmit(skb);
              }
#1 lore raw
Quoted 21 lines · On Mon, 30 Jul 2007 10:48:40 -0600 Cameron Schaus <[email protected]> wrote:

I have been working on an ebtables module to insert an 802.1Q (VLAN) tags into network packets leaving the bridge, via the POSTROUTING NAT chain. To insert the VLAN tag the ethernet header size needs to be increased by 4 bytes. I noticed that after increasing the size of the ethernet header that the ethernet header in packets on the wire was incorrect.

I think it is because the br_dev_queue_push_xmit function does not take the VLAN header into account when it moves the data pointer in the skb to the start of the ethernet header. The following patch corrects this.

Does this make sense to do? I plan to do more extensive testing passing vlan and non-vlan traffic across a bridge, and will report back with the results.

net/bridge/br_forward.c +0 -0
index ada7f49..f30dc56 100644

net/bridge/br_forward.c int br_dev_queue_push_xmit(struct sk_buff *skb) · line 50

Quoted 8 lines
net/bridge/br_forward.c +2 -0
@@ -43,6 +43,8 @@ int br_dev_queue_push_xmit(struct sk_buff *skb)
                       kfree_skb(skb);
               else {
                       skb_push(skb, ETH_HLEN);
+                       if(skb->protocol == htons(ETH_P_8021Q))
+                         skb_push(skb, VLAN_HLEN);
 
                       dev_queue_xmit(skb);
               }

This might break VLAN transparent bridging (ie. when eth0 is bridged to eth1 and the bridge doesn't know or care about vlan's). There is also the case where packet comes in over VLAN and goes out without tag.

#2 lore raw
Quoted 20 lines · Stephen Hemminger
Quoted 17 lines · On Mon, 30 Jul 2007 10:48:40 -0600 Cameron Schaus <[email protected]> wrote:

I have been working on an ebtables module to insert an 802.1Q (VLAN) tags into network packets leaving the bridge, via the POSTROUTING NAT chain. To insert the VLAN tag the ethernet header size needs to be increased by 4 bytes. I noticed that after increasing the size of the ethernet header that the ethernet header in packets on the wire was incorrect.

I think it is because the br_dev_queue_push_xmit function does not take the VLAN header into account when it moves the data pointer in the skb to the start of the ethernet header. The following patch corrects this.

Does this make sense to do? I plan to do more extensive testing passing vlan and non-vlan traffic across a bridge, and will report back with the results.

net/bridge/br_forward.c +2 -0
index ada7f49..f30dc56 100644
@@ -43,6 +43,8 @@ int br_dev_queue_push_xmit(struct sk_buff *skb)
                       kfree_skb(skb);
               else {
                       skb_push(skb, ETH_HLEN);
+                       if(skb->protocol == htons(ETH_P_8021Q))
+                         skb_push(skb, VLAN_HLEN);
 
Quoted 7 lines

net/bridge/br_forward.c int br_dev_queue_push_xmit(struct sk_buff *skb) · line 50

Quoted 3 lines
net/bridge/br_forward.c +0 -0
                       dev_queue_xmit(skb);
               }

This might break VLAN transparent bridging (ie. when eth0 is bridged to eth1 and the bridge doesn't know or care about vlan's). There is also the case where packet comes in over VLAN and goes out without tag.

There are general issues in this area for people wanting to support Q-in-Q as well.

Perhaps we should figure out how we would like things to work, as opposed to how they might work now. For instance, currently if we transmit an skb on a vlan device, it does not matter if the skb has a vlan tag or not, as the vlan device will force the VID to be there and to be the VID of that vlan device. This will not work for Q-in-Q.

One option is to have the bridge add or strip tags before sending to the VLAN, or maybe we could pass appropriate info to the VLAN code so that it can determine how to do the tag insertion. Passing info to the VLAN code, perhaps in a flag in the skb, might be best...as that will allow us to pass similar information from user-space for anyone bridging (or similar) with packet-sockets.

Thanks,
Ben

Quoted 4 lines

_______________________________________________
Bridge mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/bridge

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
#3 lore raw
In reply to Stephen Hemminger
Quoted 4 lines · Stephen Hemminger

This might break VLAN transparent bridging (ie. when eth0 is bridged to eth1 and the bridge doesn't know or care about vlan's). There is also the case where packet comes in over VLAN and goes out without tag.

I was definitely worried about the first case you mentioned. However, the nf_bridge_copy_header function does the same skb_push taking into account the 8021Q header size, if it's called.

In light of the cases you mentioned above, would it be best to just alter the data pointer in the ebtables module to take into account the new vlan header? I hesitate to do this because doing so could affect other rules run after the modification, if any.

Cam