netdev - Re: [patch net-next-2.6 v2] net: vlan: make non-hw-accel rx path similar to hw-accel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<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