From patchwork Thu Nov 19 21:06:40 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Aaron Conole X-Patchwork-Id: 546696 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 2862D14140E for ; Fri, 20 Nov 2015 08:06:47 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161407AbbKSVGm (ORCPT ); Thu, 19 Nov 2015 16:06:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48020 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030227AbbKSVGl convert rfc822-to-8bit (ORCPT ); Thu, 19 Nov 2015 16:06:41 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 8F141C0A515C; Thu, 19 Nov 2015 21:06:41 +0000 (UTC) Received: from aconole.bos.csb (dhcp-25-177.bos.redhat.com [10.18.25.177]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tAJL6e4f017760 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 19 Nov 2015 16:06:40 -0500 From: Aaron Conole To: Eric Dumazet Cc: =?utf-8?Q?Bj=C3=B8rn?= Mork , Alexei Starovoitov , David Miller , netdev Subject: Re: [PATCH net-next] net: remove useless check in napi_gro_frags() References: <1447962170.22599.240.camel@edumazet-glaptop2.roam.corp.google.com> <20151119195419.GA6728@ast-mbp.thefacebook.com> <1447964139.22599.246.camel@edumazet-glaptop2.roam.corp.google.com> <87oaepzqq2.fsf@nemi.mork.no> <1447965199.22599.252.camel@edumazet-glaptop2.roam.corp.google.com> Date: Thu, 19 Nov 2015 16:06:40 -0500 In-Reply-To: <1447965199.22599.252.camel@edumazet-glaptop2.roam.corp.google.com> (Eric Dumazet's message of "Thu, 19 Nov 2015 12:33:19 -0800") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5.50 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Eric Dumazet writes: > On Thu, 2015-11-19 at 21:20 +0100, Bjørn Mork wrote: >> Eric Dumazet writes: >> >> > How many times should we crash before napi_frags_skb() returns NULL ? >> .. >> > return NULL; >> >> Huh? Now I'm lost here, too. > > > Well, Ethernet drivers should not feed GRO with frames with less than 14 bytes. > > ( eth_type_trans() would crash the same ) > > Lets fix buggy drivers instead of adding defensive code all over the stack. > > napi_gro_frags() is used by exactly 10 drivers, and I am pretty sure > they are OK. > Would the following be an appropriate change in addition to the one you've posted, then? If so I can repost as a formal patch, if you'd like. At present, there's only one user of napi_frags_skb(), and your patch removes the NULL check. If this can really only be the result of buggy driver, then perhaps we should just call out the bug? --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/core/dev.c b/net/core/dev.c index 41cef3e..b71c8b4 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -4440,10 +4440,7 @@ static struct sk_buff *napi_frags_skb(struct napi_struct *napi) eth = skb_gro_header_fast(skb, 0); if (unlikely(skb_gro_header_hard(skb, hlen))) { eth = skb_gro_header_slow(skb, hlen, 0); - if (unlikely(!eth)) { - napi_reuse_skb(napi, skb); - return NULL; - } + BUG_ON(!eth); } else { gro_pull_from_frag0(skb, hlen); NAPI_GRO_CB(skb)->frag0 += hlen;