Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Nguyen Dinh Phi <phind.uet@gmail.com>
To: Eric Dumazet <eric.dumazet@gmail.com>,
	Eric Dumazet <edumazet@google.com>
Cc: "David Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Jesper Dangaard Brouer" <hawk@kernel.org>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Martin KaFai Lau" <kafai@fb.com>,
	"Song Liu" <songliubraving@fb.com>, "Yonghong Song" <yhs@fb.com>,
	kpsingh@kernel.org, "Antoine Tenart" <atenart@kernel.org>,
	"Alexander Lobakin" <alobakin@pm.me>,
	"Wei Wang" <weiwan@google.com>, "Taehee Yoo" <ap420073@gmail.com>,
	"Björn Töpel" <bjorn@kernel.org>,
	memxor@gmail.com, netdev <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
	linux-kernel-mentees@lists.linuxfoundation.org,
	syzbot+989efe781c74de1ddb54@syzkaller.appspotmail.com,
	"Mahesh Bandewar" <maheshb@google.com>
Subject: Re: [PATCH] net: drop skbs in napi->rx_list when removing the napi context.
Date: Fri, 13 Aug 2021 18:51:35 +0800	[thread overview]
Message-ID: <804FB1AF-90C7-4430-9C5A-6837C32E431A@gmail.com> (raw)
In-Reply-To: <8f324e46-f05c-42d7-9599-a43de7be17dc@gmail.com>

On August 13, 2021 6:16:34 PM GMT+08:00, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>On 8/12/21 9:17 PM, Phi Nguyen wrote:
>> On 8/12/2021 3:07 PM, Eric Dumazet wrote:
>>> Also I object to this fix.
>>>
>>> If packets have been stored temporarily in GRO, they should be
>>> released at some point,
>>> normally at the end of a napi poll.
>>>
>>> By released, I mean that these packets should reach the upper stack,
>>> instead of being dropped without
>>> any notification.
>>>
>>> It seems a call to gro_normal_list() is missing somewhere.
>>>
>>> Can you find where ?
>>>
>>> Thanks !
>>> H Eric,
>> 
>> I think the location that should have a call to gro_normal_list() is
>__netif_napi_del(). Let say, if the driver call a function that lead to
>gro_normal_one(), and add a skb to the rx_list while the napi poll is
>not scheduled, and the driver remove the napi context before a napi
>poll could be triggered, then the added skb will be lost.
>> 
>> Actually, this was the first solution that I tried with syzbot (It
>passed the test too).
>> Best regards,
>> Phi
>
>I think the real bug is in drivers/net/tun.c
>
>It can call napi_gro_frags() and I do not see corresponding
>napi_complete()
>
>This seems completely bogus.
>
>Your patch only works around one the many bugs caused by 
>commit 90e33d45940793def6f773b2d528e9f3c84ffdc7 tun: enable
>napi_gro_frags() for TUN/TAP driver
>
>I suggest not adding your patch, because we should fix the root cause.

Yes, it truely comes from tun driver. But it was there before listified RX added (that time the skb will be put to stack immediately), that why I made the fix in dev.c. I thought I should change the later commit.

      reply	other threads:[~2021-08-13 10:51 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-11 23:59 Nguyen Dinh Phi
2021-08-12  7:07 ` Eric Dumazet
2021-08-12 19:17   ` Phi Nguyen
2021-08-13 10:16     ` Eric Dumazet
2021-08-13 10:51       ` Nguyen Dinh Phi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=804FB1AF-90C7-4430-9C5A-6837C32E431A@gmail.com \
    --to=phind.uet@gmail.com \
    --cc=alobakin@pm.me \
    --cc=andrii@kernel.org \
    --cc=ap420073@gmail.com \
    --cc=ast@kernel.org \
    --cc=atenart@kernel.org \
    --cc=bjorn@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=eric.dumazet@gmail.com \
    --cc=hawk@kernel.org \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kpsingh@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel-mentees@lists.linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maheshb@google.com \
    --cc=memxor@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=syzbot+989efe781c74de1ddb54@syzkaller.appspotmail.com \
    --cc=weiwan@google.com \
    --cc=yhs@fb.com \
    --subject='Re: [PATCH] net: drop skbs in napi->rx_list when removing the napi context.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).