[PATCH] ethernet-bridge: update skb->priority in case forwarded frame has VLAN-header

[PATCH] ethernet-bridge: update skb->priority in case forwarded frame has VLAN-header

[Leo Yuriev]

Kernel 2.6 (2.6.11)

When ethernet-bridge forward a packet and such ethernet-frame has
VLAN-tag, bridge should update skb->prioriry for properly QoS
handling.

This small patch does this. Currently vlan_TCI-priority directly
mapped to skb->priority, but this looks enough.

Patch-by: Leo Yuriev <leo@yuriev.ru>


-- net/bridge/br_input.c.orig   2005-03-02 10:37:50.000000000 +0300
+++ net/bridge/br_input.c       2005-03-05 16:11:00.000000000 +0300
@@ -5,6 +5,10 @@
  *     Authors:
  *     Lennert Buytenhek               <buytenh@gnu.org>
  *
+ *     Changes:
+ *             03/Mar/2005: Leo Yuriev <leo@yuriev.ru>
+ *             Update skb->priority for packets with VLAN-tag.
+ *
  *     $Id: br_input.c,v 1.10 2001/12/24 04:50:20 davem Exp $
  *
  *     This program is free software; you can redistribute it and/or
@@ -17,6 +21,9 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/netfilter_bridge.h>
+#ifdef CONFIG_NET_SCHED
+#      include <linux/if_vlan.h>
+#endif /* CONFIG_NET_SCHED*/
 #include "br_private.h"
 
 const unsigned char bridge_ula[6] = { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x00 };
@@ -45,6 +52,40 @@ static void br_pass_frame_up(struct net_
                        br_pass_frame_up_finish);
 }
 
+
+#ifdef CONFIG_NET_SCHED
+/*
+ *   Leo Yuriev: Just update skb->priority for properly QoS handling in case
+ *               frame in the skb is contain VLAN-header.
+ *
+ *  SANITY NOTE: We are referencing to the VLAN_HDR frields, which MAY be
+ *               stored UNALIGNED in the memory.
+ *               According to Dave Miller & Alexey, it will always be aligned,
+ *               so there doesn't need to be any of the unaligned stuff.
+ *
+ */
+static __inline__ void br_update_skb_priority_if_vlan(struct sk_buff *skb)
+{
+       unsigned short vlan_TCI;
+       struct vlan_hdr *vhdr;
+
+       if (skb->protocol == __constant_htons(ETH_P_8021Q)) {
+               vhdr = (struct vlan_hdr *)(skb->data);
+               /* vlan_TCI = ntohs(get_unaligned(&vhdr->h_vlan_TCI)); */
+               vlan_TCI = ntohs(vhdr->h_vlan_TCI);
+#ifdef VLAN_DEBUG
+               printk(VLAN_DBG "%s: skb: %p vlan_id: %hx\n",
+                       __FUNCTION__, skb, (vlan_TCI & VLAN_VID_MASK));
+#endif
+               /*
+                *   We map VLAN_TCI priority (0..7) to skb->priority (0..15) 
+                *   most similarly e.g. 0->0, 1->1, .., 7->7
+                */
+               skb->priority = (vlan_TCI >> 13) & 7;
+       }
+}
+#endif /* CONFIG_NET_SCHED */
+
 /* note: already called with rcu_read_lock (preempt_disabled) */
 int br_handle_frame_finish(struct sk_buff *skb)
 {
@@ -54,6 +95,10 @@ int br_handle_frame_finish(struct sk_buf
        struct net_bridge_fdb_entry *dst;
        int passedup = 0;
 
+#ifdef CONFIG_NET_SCHED
+       br_update_skb_priority_if_vlan(skb);
+#endif /* CONFIG_NET_SCHED*/
+
        if (br->dev->flags & IFF_PROMISC) {
                struct sk_buff *skb2;

Re: [PATCH] ethernet-bridge: update skb->priority in case forwarded frame has VLAN-header

[Ben Greear]

Leo Yuriev wrote:
> Kernel 2.6 (2.6.11)
> 
> When ethernet-bridge forward a packet and such ethernet-frame has
> VLAN-tag, bridge should update skb->prioriry for properly QoS
> handling.
> 
> This small patch does this. Currently vlan_TCI-priority directly
> mapped to skb->priority, but this looks enough.

If this packet came in from an 802.1Q VLAN device, the VLAN code already
has the logic necessary to map the .1q priority to an arbitrary skb->priority.

See the vconfig commands:
        set_egress_map  [vlan-name]      [skb_priority]   [vlan_qos]
        set_ingress_map [vlan-name]      [skb_priority]   [vlan_qos]

Thanks,
Ben

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

Re[2]: [PATCH] ethernet-bridge: update skb->priority in case forwarded frame has VLAN-header

[Leo Yuriev]

LY>> Kernel 2.6 (2.6.11)
LY>>
LY>> When ethernet-bridge forward a packet and such ethernet-frame has
LY>> VLAN-tag, bridge should update skb->prioriry for properly QoS
LY>> handling.
LY>>
LY>> This small patch does this. Currently vlan_TCI-priority directly
LY>> mapped to skb->priority, but this looks enough.

PM> It needs to verify the tag is present and accessible using
PM> pskb_may_pull(). But I think an ebtables target similar to the iptables
PM> CLASSIFY target is a better place for this. It could allow setting
PM> skb->priority to an arbitary value or derive it from vlan priority or IP
PM> tos.

BG> The VLAN code has it's own (user-configurable) mapping from skb->priority to .1q priority,
BG> and .1q priority to skb->priority.  You might want to clone or somehow
BG> use the .1q mapping logic to allow something other than just a
BG> straight .1q -> skb->priority mapping.

BG> If this packet came in from an 802.1Q VLAN device, the VLAN code already
BG> has the logic necessary to map the .1q priority to an arbitrary skb->priority.

- Yes, but linux-box can be a all-vlan-bridge without the 8021q
module. So, in a some cases of configuration (like currently is my
own) nobody in kernel can set skb->priority in the forwarded packets.
But I (as a user) can expect than the bridge with QoS will able to
plain and evident prioritization without my immersion into ebtables...

1) I will put the verification by pskb_may_pull() as pointed Patrick
McHardy. It is just my mistake/bug.

2) Despite of opportunities ebtables I think, that this addition is
necessary. As small forces, by default, provide more correct and
expected behaviour of system, without conflicts to other
opportunities.

3) But I think, it is not necessary to provide a customization of
mapping .1q priority to skb->priority (e.g. clone a code from
VLAN-module) for the following reasons:
    - my patch is intended only for the basic, obvious behaviour;
    - ebtables already provide a powerful abilities;
    - user can expect that the bridge will not require more
      configuration that currently is provided;

Re: [PATCH] ethernet-bridge: update skb->priority in case forwarded frame has VLAN-header

[Ben Greear]

Leo Yuriev wrote:

> 3) But I think, it is not necessary to provide a customization of
> mapping .1q priority to skb->priority (e.g. clone a code from
> VLAN-module) for the following reasons:
>     - my patch is intended only for the basic, obvious behaviour;
>     - ebtables already provide a powerful abilities;
>     - user can expect that the bridge will not require more
>       configuration that currently is provided;

Thats fine by me.

Thanks,
Ben

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

Re: [PATCH] ethernet-bridge: update skb->priority in case forwarded frame has VLAN-header

[Alexey Kuznetsov]

Hello!

> If this packet came in from an 802.1Q VLAN device, the VLAN code already
> has the logic necessary to map the .1q priority to an arbitrary 
> skb->priority.

Actually, the patch makes sense when it is straight ethernet bridge
not involving full parsing of VLAN. I guess the case when the frame
passed through VLAN device should be eliminated to avoid overwriting
priority set by VLAN.

Alexey