Re: [PATCH net-next] net: remove useless check in napi_gro_frags()

[Aaron Conole <aconole@redhat.com>]

Eric Dumazet <eric.dumazet@gmail.com> writes:

> On Thu, 2015-11-19 at 21:20 +0100, Bjørn Mork wrote:
>> Eric Dumazet <eric.dumazet@gmail.com> 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?

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;