[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110521072925.GA2588@jirka.orion>
Date: Sat, 21 May 2011 09:29:26 +0200
From: Jiri Pirko <jpirko@...hat.com>
To: Changli Gao <xiaosuo@...il.com>
Cc: David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
shemminger@...ux-foundation.org, kaber@...sh.net, fubar@...ibm.com,
eric.dumazet@...il.com, nicolas.2p.debian@...il.com,
andy@...yhouse.net, jesse@...ira.com, ebiederm@...ssion.com
Subject: Re: [patch net-next-2.6 v2] net: vlan: make non-hw-accel rx path
similar to hw-accel
Sat, May 21, 2011 at 03:11:05AM CEST, xiaosuo@...il.com wrote:
>On Wed, Apr 13, 2011 at 5:16 AM, David Miller <davem@...emloft.net> wrote:
>> From: Jiri Pirko <jpirko@...hat.com>
>> Date: Fri, 8 Apr 2011 07:48:33 +0200
>>
>>> Now there are 2 paths for rx vlan frames. When rx-vlan-hw-accel is
>>> enabled, skb is untagged by NIC, vlan_tci is set and the skb gets into
>>> vlan code in __netif_receive_skb - vlan_hwaccel_do_receive.
>>>
>>> For non-rx-vlan-hw-accel however, tagged skb goes thru whole
>>> __netif_receive_skb, it's untagged in ptype_base hander and reinjected
>>>
>>> This incosistency is fixed by this patch. Vlan untagging happens early in
>>> __netif_receive_skb so the rest of code (ptype_all handlers, rx_handlers)
>>> see the skb like it was untagged by hw.
>>>
>>> Signed-off-by: Jiri Pirko <jpirko@...hat.com>
>>>
>>> v1->v2:
>>> remove "inline" from vlan_core.c functions
>>
>> Ok, I've applied this, let's see what happens :-)
>>
>
>I think we should revert it.
>
>File: net/8021q/vlan_core.c:
>
>161 skb_pull_rcsum(skb, VLAN_HLEN);
>
>skb->data and skb->len are updated, but network_header and
>transport_header are left unchanged. This will break the assumption in
>net_sched.
>
> for example:
> file: cls_u32.c
> 104 unsigned int off = skb_network_offset(skb);
> After this patch, skb_network_offset may be negative.
>
>162 vlan_set_encap_proto(skb, vhdr);
>163
>164 skb = vlan_check_reorder_header(skb);
>vlan_check_reorder_header assume skb->dev is a vlan_dev. Even though
>the correct dev is assigned temporarily, we should not reorder the
>header here as HW accelerated vlan RX does, as this may breaks the
>bridging comes later.
>
>165 if (unlikely(!skb))
>166 goto err_free;
>
>The hardware accelerated vlan RX doesn't always do the "right" things
>as it strips the vlan header, so we should not emulate it in software
>all the time.
I do not see a reason why to not emulate that. To make paths as much
similar as they can be, that is the point of this patch.
I think it would be better to fix an issue you are pointing at
rather that revert this.
Jirka
>
>--
>Regards,
>Changli Gao(xiaosuo@...il.com)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists