Re: [PATCH] net: drop skbs in napi->rx_list when removing the napi context.

On August 13, 2021 6:16:34 PM GMT+08:00, Eric Dumazet [off-list ref] wrote:

On 8/12/21 9:17 PM, Phi Nguyen wrote:

quoted

On 8/12/2021 3:07 PM, Eric Dumazet wrote:

quoted

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.

quoted

Actually, this was the first solution that I tried with syzbot (It

passed the test too).

quoted

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.