netdev - Re: [PATCH net v2] net: Fix packet reordering caused by GRO and listified RX cooperation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 22 Jan 2020 20:38:04 +0100 (CET)
From:   David Miller <davem@...emloft.net>
To:     maximmi@...lanox.com
Cc:     alobakin@...nk.ru, ecree@...arflare.com, saeedm@...lanox.com,
        netdev@...r.kernel.org, johannes.berg@...el.com,
        emmanuel.grumbach@...el.com, luciano.coelho@...el.com,
        tariqt@...lanox.com, jiri@...lanox.com, edumazet@...gle.com
Subject: Re: [PATCH net v2] net: Fix packet reordering caused by GRO and
 listified RX cooperation

From: Maxim Mikityanskiy <maximmi@...lanox.com>
Date: Tue, 21 Jan 2020 15:09:40 +0000

> Commit 323ebb61e32b ("net: use listified RX for handling GRO_NORMAL
> skbs") introduces batching of GRO_NORMAL packets in napi_frags_finish,
> and commit 6570bc79c0df ("net: core: use listified Rx for GRO_NORMAL in
> napi_gro_receive()") adds the same to napi_skb_finish. However,
> dev_gro_receive (that is called just before napi_{frags,skb}_finish) can
> also pass skbs to the networking stack: e.g., when the GRO session is
> flushed, napi_gro_complete is called, which passes pp directly to
> netif_receive_skb_internal, skipping napi->rx_list. It means that the
> packet stored in pp will be handled by the stack earlier than the
> packets that arrived before, but are still waiting in napi->rx_list. It
> leads to TCP reorderings that can be observed in the TCPOFOQueue counter
> in netstat.
> 
> This commit fixes the reordering issue by making napi_gro_complete also
> use napi->rx_list, so that all packets going through GRO will keep their
> order. In order to keep napi_gro_flush working properly, gro_normal_list
> calls are moved after the flush to clear napi->rx_list.
> 
> iwlwifi calls napi_gro_flush directly and does the same thing that is
> done by gro_normal_list, so the same change is applied there:
> napi_gro_flush is moved to be before the flush of napi->rx_list.
> 
> A few other drivers also use napi_gro_flush (brocade/bna/bnad.c,
> cortina/gemini.c, hisilicon/hns3/hns3_enet.c). The first two also use
> napi_complete_done afterwards, which performs the gro_normal_list flush,
> so they are fine. The latter calls napi_gro_receive right after
> napi_gro_flush, so it can end up with non-empty napi->rx_list anyway.
> 
> Fixes: 323ebb61e32b ("net: use listified RX for handling GRO_NORMAL skbs")
> Signed-off-by: Maxim Mikityanskiy <maximmi@...lanox.com>

Applied and queued up for v5.4 -stable, thank you.