From patchwork Fri Jan 8 12:11:31 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thadeu Lima de Souza Cascardo X-Patchwork-Id: 564749 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (li376-54.members.linode.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id D0F471401DA for ; Fri, 8 Jan 2016 23:11:42 +1100 (AEDT) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 2361610B55; Fri, 8 Jan 2016 04:11:42 -0800 (PST) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx1e3.cudamail.com (mx1.cudamail.com [69.90.118.67]) by archives.nicira.com (Postfix) with ESMTPS id E070A10B53 for ; Fri, 8 Jan 2016 04:11:40 -0800 (PST) Received: from bar2.cudamail.com (localhost [127.0.0.1]) by mx1e3.cudamail.com (Postfix) with ESMTPS id 536B64201FC for ; Fri, 8 Jan 2016 05:11:40 -0700 (MST) X-ASG-Debug-ID: 1452255099-03dc5366eb15350001-byXFYA Received: from mx1-pf2.cudamail.com ([192.168.24.2]) by bar2.cudamail.com with ESMTP id jatCXkID4nK4ibxY (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 08 Jan 2016 05:11:39 -0700 (MST) X-Barracuda-Envelope-From: cascardo@redhat.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.2 Received: from unknown (HELO mx1.redhat.com) (209.132.183.28) by mx1-pf2.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 8 Jan 2016 12:11:38 -0000 Received-SPF: pass (mx1-pf2.cudamail.com: SPF record at _spf1.redhat.com designates 209.132.183.28 as permitted sender) X-Barracuda-Apparent-Source-IP: 209.132.183.28 X-Barracuda-RBL-IP: 209.132.183.28 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 55A2DFB75B; Fri, 8 Jan 2016 12:11:37 +0000 (UTC) Received: from indiana.gru.redhat.com (ovpn-113-48.phx2.redhat.com [10.3.113.48]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u08CBVA2017745 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 8 Jan 2016 07:11:34 -0500 Date: Fri, 8 Jan 2016 10:11:31 -0200 X-CudaMail-Envelope-Sender: cascardo@redhat.com From: Thadeu Lima de Souza Cascardo To: Konstantin Khlebnikov X-CudaMail-MID: CM-E2-107008195 X-CudaMail-DTE: 010816 X-CudaMail-Originating-IP: 209.132.183.28 Message-ID: <20160108121130.GL27290@indiana.gru.redhat.com> X-ASG-Orig-Subj: [##CM-E2-107008195##]Re: [PATCH] net: preserve IP control block during GSO segmentation References: <145225444176.22215.2003378381077166898.stgit@zurg> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <145225444176.22215.2003378381077166898.stgit@zurg> User-Agent: Mutt/1.5.24 (2015-08-30) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-GBUdb-Analysis: 0, 209.132.183.28, Ugly c=0.303425 p=-0.157895 Source Normal X-MessageSniffer-Rules: 0-0-0-10486-c X-Barracuda-Connect: UNKNOWN[192.168.24.2] X-Barracuda-Start-Time: 1452255099 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 2.60 X-Barracuda-Spam-Status: No, SCORE=2.60 using per-user scores of TAG_LEVEL=3.5 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=4.0 tests=BSF_SC0_MISMATCH_TO, BSF_SC7_SA_HREF_FROM_MISMATCH_TEXT_URIx1_HL, RDNS_NONE X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.25949 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.00 BSF_SC0_MISMATCH_TO Envelope rcpt doesn't match header 0.10 RDNS_NONE Delivered to trusted network by a host with no rDNS 2.50 BSF_SC7_SA_HREF_FROM_MISMATCH_TEXT_URIx1_HL Custom Rule HREF_FROM_MISMATCH_TEXT_URIx1_HL Cc: dev@openvswitch.org, netdev@vger.kernel.org, Florian Westphal , linux-kernel@vger.kernel.org, Eric Dumazet , Cong Wang , "David S. Miller" Subject: Re: [ovs-dev] [PATCH] net: preserve IP control block during GSO segmentation X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@openvswitch.org Sender: "dev" On Fri, Jan 08, 2016 at 03:00:41PM +0300, Konstantin Khlebnikov wrote: > Skb_gso_segment() uses skb control block during segmentation. > This patch adds 32-bytes room for previous control block which > will be copied into all resulting segments. > > This patch fixes kernel crash during fragmenting forwarded packets. > Fragmentation requires valid IP CB in skb for clearing ip options. > Also patch removes custom save/restore in ovs code, now it's redundant. > > Signed-off-by: Konstantin Khlebnikov > Link: http://lkml.kernel.org/r/CALYGNiP-0MZ-FExV2HutTvE9U-QQtkKSoE--KN=JQE5STYsjAA@mail.gmail.com > --- > include/linux/skbuff.h | 3 ++- > net/core/dev.c | 5 +++++ > net/ipv4/ip_output.c | 1 + > net/openvswitch/datapath.c | 4 +--- > net/xfrm/xfrm_output.c | 2 ++ > 5 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index 4355129fff91..9147f9f34cbe 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -3446,7 +3446,8 @@ struct skb_gso_cb { > int encap_level; > __u16 csum_start; > }; > -#define SKB_GSO_CB(skb) ((struct skb_gso_cb *)(skb)->cb) > +#define SKB_SGO_CB_OFFSET 32 > +#define SKB_GSO_CB(skb) ((struct skb_gso_cb *)((skb)->cb + SKB_SGO_CB_OFFSET)) > > static inline int skb_tnl_header_len(const struct sk_buff *inner_skb) > { > diff --git a/net/core/dev.c b/net/core/dev.c > index ae00b894e675..7f00f2439770 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -2542,6 +2542,8 @@ static inline bool skb_needs_check(struct sk_buff *skb, bool tx_path) > * > * It may return NULL if the skb requires no segmentation. This is > * only possible when GSO is used for verifying header integrity. > + * > + * Segmentation preserves SKB_SGO_CB_OFFSET bytes of previous skb cb. > */ > struct sk_buff *__skb_gso_segment(struct sk_buff *skb, > netdev_features_t features, bool tx_path) > @@ -2556,6 +2558,9 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb, > return ERR_PTR(err); > } > > + BUILD_BUG_ON(SKB_SGO_CB_OFFSET + > + sizeof(*SKB_GSO_CB(skb)) > sizeof(skb->cb)); > + > SKB_GSO_CB(skb)->mac_offset = skb_headroom(skb); > SKB_GSO_CB(skb)->encap_level = 0; > > diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c > index 4233cbe47052..59ed4b89b67a 100644 > --- a/net/ipv4/ip_output.c > +++ b/net/ipv4/ip_output.c > @@ -240,6 +240,7 @@ static int ip_finish_output_gso(struct net *net, struct sock *sk, > * from host network stack. > */ > features = netif_skb_features(skb); > + BUILD_BUG_ON(sizeof(*IPCB(skb)) > SKB_SGO_CB_OFFSET); > segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK); > if (IS_ERR_OR_NULL(segs)) { > kfree_skb(skb); > diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c > index 91a8b004dc51..b1b380ee667d 100644 > --- a/net/openvswitch/datapath.c > +++ b/net/openvswitch/datapath.c > @@ -336,12 +336,10 @@ static int queue_gso_packets(struct datapath *dp, struct sk_buff *skb, > unsigned short gso_type = skb_shinfo(skb)->gso_type; > struct sw_flow_key later_key; > struct sk_buff *segs, *nskb; > - struct ovs_skb_cb ovs_cb; > int err; > > - ovs_cb = *OVS_CB(skb); > + BUILD_BUG_ON(sizeof(*OVS_CB(skb)) > SKB_SGO_CB_OFFSET); > segs = __skb_gso_segment(skb, NETIF_F_SG, false); > - *OVS_CB(skb) = ovs_cb; > if (IS_ERR(segs)) > return PTR_ERR(segs); > if (segs == NULL) You are missing this hunk. > diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c > index cc3676eb6239..ff4a91fcab9f 100644 > --- a/net/xfrm/xfrm_output.c > +++ b/net/xfrm/xfrm_output.c > @@ -167,6 +167,8 @@ static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb > { > struct sk_buff *segs; > > + BUILD_BUG_ON(sizeof(*IPCB(skb)) > SKB_SGO_CB_OFFSET); > + BUILD_BUG_ON(sizeof(*IP6CB(skb)) > SKB_SGO_CB_OFFSET); > segs = skb_gso_segment(skb, 0); > kfree_skb(skb); > if (IS_ERR(segs)) > --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -359,7 +359,6 @@ static int queue_gso_packets(struct datapath *dp, struct sk_buff *skb, /* Queue all of the segments. */ skb = segs; do { - *OVS_CB(skb) = ovs_cb; if (gso_type & SKB_GSO_UDP && skb != segs) key = &later_key;