From patchwork Tue May 24 00:11:51 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 97110 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id E64E2B6FBB for ; Tue, 24 May 2011 10:12:12 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755849Ab1EXAMG (ORCPT ); Mon, 23 May 2011 20:12:06 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:46413 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755063Ab1EXAME (ORCPT ); Mon, 23 May 2011 20:12:04 -0400 Received: from in01.mta.xmission.com ([166.70.13.51]) by out01.mta.xmission.com with esmtps (TLSv1:AES256-SHA:256) (Exim 4.69) (envelope-from ) id 1QOfE4-00033G-MO; Mon, 23 May 2011 18:11:56 -0600 Received: from c-98-207-153-68.hsd1.ca.comcast.net ([98.207.153.68] helo=fess.ebiederm.org) by in01.mta.xmission.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.69) (envelope-from ) id 1QOfE4-00023y-E2; Mon, 23 May 2011 18:11:56 -0600 Received: from fess.ebiederm.org (localhost [127.0.0.1]) by fess.ebiederm.org (8.14.3/8.14.3/Debian-9.1ubuntu1) with ESMTP id p4O0BrgL021654; Mon, 23 May 2011 17:11:53 -0700 Received: (from eric@localhost) by fess.ebiederm.org (8.14.3/8.14.3/Submit) id p4O0BpNJ021648; Mon, 23 May 2011 17:11:51 -0700 From: ebiederm@xmission.com (Eric W. Biederman) To: David Miller Cc: shemminger@linux-foundation.org, greearb@candelatech.com, nicolas.2p.debian@gmail.com, jpirko@redhat.com, xiaosuo@gmail.com, netdev@vger.kernel.org, kaber@trash.net, fubar@us.ibm.com, eric.dumazet@gmail.com, andy@greyhouse.net, jesse@nicira.com Subject: [PATCH] vlan: Fix the b0rked ingress VLAN_FLAG_REORDER_HDR check. References: <4DDAB72B.9050101@gmail.com> <4DDAC26C.2070601@candelatech.com> <20110523140048.777fb378@nehalam> <20110523.172047.1438754754048434316.davem@davemloft.net> Date: Mon, 23 May 2011 17:11:51 -0700 In-Reply-To: <20110523.172047.1438754754048434316.davem@davemloft.net> (David Miller's message of "Mon, 23 May 2011 17:20:47 -0400 (EDT)") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=; ; ; mid=; ; ; hst=in01.mta.xmission.com; ; ; ip=98.207.153.68; ; ; frm=ebiederm@xmission.com; ; ; spf=neutral X-XM-AID: U2FsdGVkX1/2C8R1DCpgNBFdREUK8VfnEspLS+7HIq0= X-SA-Exim-Connect-IP: 98.207.153.68 X-SA-Exim-Mail-From: ebiederm@xmission.com X-SA-Exim-Scanned: No (on in01.mta.xmission.com); SAEximRunCond expanded to false Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Testing of VLAN_FLAG_REORDER_HDR does not belong in vlan_untag but rather in vlan_do_receive. In attempting to test VLAN_FLAG_REORDER_HDR we are attempting to deference vlan private network device members a non vlan device, which is just wrong. Move the test into vlan_do_receive so that the vlan header will not be properly put on the packet in the case of vlan header acceleration. As we remove the check from vlan_check_reorder_header rename it vlan_reorder_header to keep the naming clean. Hopefully at somepoint we will just declare the case where VLAN_FLAG_REORDER_HDR is cleared as unsupported and remove the code. Until then this keeps it working correctly. Signed-off-by: Eric W. Biederman --- include/linux/if_vlan.h | 25 ++++++++++++++++++++++--- net/8021q/vlan_core.c | 25 ++++++++++++++----------- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h index 290bd8a..26f3c92 100644 --- a/include/linux/if_vlan.h +++ b/include/linux/if_vlan.h @@ -220,7 +220,7 @@ static inline int vlan_hwaccel_receive_skb(struct sk_buff *skb, } /** - * __vlan_put_tag - regular VLAN tag inserting + * vlan_insert_tag - regular VLAN tag inserting * @skb: skbuff to tag * @vlan_tci: VLAN TCI to insert * @@ -229,8 +229,10 @@ static inline int vlan_hwaccel_receive_skb(struct sk_buff *skb, * * Following the skb_unshare() example, in case of error, the calling function * doesn't have to worry about freeing the original skb. + * + * Does not change skb->protocol so this function can be used for receive. */ -static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci) +static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb, u16 vlan_tci) { struct vlan_ethhdr *veth; @@ -250,8 +252,25 @@ static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci) /* now, the TCI */ veth->h_vlan_TCI = htons(vlan_tci); - skb->protocol = htons(ETH_P_8021Q); + return skb; +} +/** + * __vlan_put_tag - regular VLAN tag inserting + * @skb: skbuff to tag + * @vlan_tci: VLAN TCI to insert + * + * Inserts the VLAN tag into @skb as part of the payload + * Returns a VLAN tagged skb. If a new skb is created, @skb is freed. + * + * Following the skb_unshare() example, in case of error, the calling function + * doesn't have to worry about freeing the original skb. + */ +static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci) +{ + skb = vlan_insert_tag(skb, vlan_tci); + if (skb) + skb->protocol = htons(ETH_P_8021Q); return skb; } diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c index 41495dc..c08dae7 100644 --- a/net/8021q/vlan_core.c +++ b/net/8021q/vlan_core.c @@ -23,6 +23,11 @@ bool vlan_do_receive(struct sk_buff **skbp) return false; skb->dev = vlan_dev; + if (unlikely(!(vlan_dev_info(vlan_dev)->flags & VLAN_FLAG_REORDER_HDR))) { + skb = *skbp = vlan_insert_tag(skb, skb->vlan_tci); + if (!skb) + return false; + } skb->priority = vlan_get_ingress_priority(vlan_dev, skb->vlan_tci); skb->vlan_tci = 0; @@ -89,17 +94,15 @@ gro_result_t vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp, } EXPORT_SYMBOL(vlan_gro_frags); -static struct sk_buff *vlan_check_reorder_header(struct sk_buff *skb) +static struct sk_buff *vlan_reorder_header(struct sk_buff *skb) { - if (vlan_dev_info(skb->dev)->flags & VLAN_FLAG_REORDER_HDR) { - if (skb_cow(skb, skb_headroom(skb)) < 0) - skb = NULL; - if (skb) { - /* Lifted from Gleb's VLAN code... */ - memmove(skb->data - ETH_HLEN, - skb->data - VLAN_ETH_HLEN, 12); - skb->mac_header += VLAN_HLEN; - } + if (skb_cow(skb, skb_headroom(skb)) < 0) + skb = NULL; + if (skb) { + /* Lifted from Gleb's VLAN code... */ + memmove(skb->data - ETH_HLEN, + skb->data - VLAN_ETH_HLEN, 12); + skb->mac_header += VLAN_HLEN; } return skb; } @@ -161,7 +164,7 @@ struct sk_buff *vlan_untag(struct sk_buff *skb) skb_pull_rcsum(skb, VLAN_HLEN); vlan_set_encap_proto(skb, vhdr); - skb = vlan_check_reorder_header(skb); + skb = vlan_reorder_header(skb); if (unlikely(!skb)) goto err_free;