* [PATCH net] net: wireless: intel: iwlwifi: fix GRO_NORMAL packet stalling
@ 2019-11-27 9:41 Alexander Lobakin
2019-11-27 9:58 ` Luciano Coelho
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Alexander Lobakin @ 2019-11-27 9:41 UTC (permalink / raw)
To: David S. Miller
Cc: Edward Cree, Jiri Pirko, Eric Dumazet, Ido Schimmel, Paolo Abeni,
Petr Machata, Sabrina Dubroca, Florian Fainelli, Jassi Brar,
Manish Chopra, GR-Linux-NIC-Dev, Johannes Berg,
Emmanuel Grumbach, Luca Coelho, Intel Linux Wireless, Kalle Valo,
Nicholas Johnson, Kenneth R. Crudup, Alexander Lobakin, netdev,
linux-wireless, linux-kernel
Commit 6570bc79c0df ("net: core: use listified Rx for GRO_NORMAL in
napi_gro_receive()") has applied batched GRO_NORMAL packets processing
to all napi_gro_receive() users, including mac80211-based drivers.
However, this change has led to a regression in iwlwifi driver [1][2] as
it is required for NAPI users to call napi_complete_done() or
napi_complete() and the end of every polling iteration, whilst iwlwifi
doesn't use NAPI scheduling at all and just calls napi_gro_flush().
In that particular case, packets which have not been already flushed
from napi->rx_list stall in it until at least next Rx cycle.
Fix this by adding a manual flushing of the list to iwlwifi driver right
before napi_gro_flush() call to mimic napi_complete() logics.
I prefer to open-code gro_normal_list() rather than exporting it for 2
reasons:
* to prevent from using it and napi_gro_flush() in any new drivers,
as it is the *really* bad way to use NAPI that should be avoided;
* to keep gro_normal_list() static and don't lose any CC optimizations.
I also don't add the "Fixes:" tag as the mentioned commit was only a
trigger that only exposed an improper usage of NAPI in this particular
driver.
[1] https://lore.kernel.org/netdev/PSXP216MB04388962C411CD0B17A86F47804A0@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM
[2] https://bugzilla.kernel.org/show_bug.cgi?id=205647
Signed-off-by: Alexander Lobakin <alobakin@dlink.ru>
---
drivers/net/wireless/intel/iwlwifi/pcie/rx.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/rx.c b/drivers/net/wireless/intel/iwlwifi/pcie/rx.c
index a4d325fcf94a..452da44a21e0 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/rx.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/rx.c
@@ -1421,6 +1421,7 @@ static struct iwl_rx_mem_buffer *iwl_pcie_get_rxb(struct iwl_trans *trans,
static void iwl_pcie_rx_handle(struct iwl_trans *trans, int queue)
{
struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
+ struct napi_struct *napi;
struct iwl_rxq *rxq;
u32 r, i, count = 0;
bool emergency = false;
@@ -1526,8 +1527,16 @@ static void iwl_pcie_rx_handle(struct iwl_trans *trans, int queue)
if (unlikely(emergency && count))
iwl_pcie_rxq_alloc_rbs(trans, GFP_ATOMIC, rxq);
- if (rxq->napi.poll)
- napi_gro_flush(&rxq->napi, false);
+ napi = &rxq->napi;
+ if (napi->poll) {
+ if (napi->rx_count) {
+ netif_receive_skb_list(&napi->rx_list);
+ INIT_LIST_HEAD(&napi->rx_list);
+ napi->rx_count = 0;
+ }
+
+ napi_gro_flush(napi, false);
+ }
iwl_pcie_rxq_restock(trans, rxq);
}
--
2.24.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net] net: wireless: intel: iwlwifi: fix GRO_NORMAL packet stalling
2019-11-27 9:41 [PATCH net] net: wireless: intel: iwlwifi: fix GRO_NORMAL packet stalling Alexander Lobakin
@ 2019-11-27 9:58 ` Luciano Coelho
2019-11-27 10:12 ` Alexander Lobakin
2019-11-27 16:05 ` Edward Cree
2019-11-27 19:23 ` David Miller
2 siblings, 1 reply; 11+ messages in thread
From: Luciano Coelho @ 2019-11-27 9:58 UTC (permalink / raw)
To: Alexander Lobakin, David S. Miller
Cc: Edward Cree, Jiri Pirko, Eric Dumazet, Ido Schimmel, Paolo Abeni,
Petr Machata, Sabrina Dubroca, Florian Fainelli, Jassi Brar,
Manish Chopra, GR-Linux-NIC-Dev, Johannes Berg,
Emmanuel Grumbach, Intel Linux Wireless, Kalle Valo,
Nicholas Johnson, Kenneth R. Crudup, netdev, linux-wireless,
linux-kernel
On Wed, 2019-11-27 at 12:41 +0300, Alexander Lobakin wrote:
> Commit 6570bc79c0df ("net: core: use listified Rx for GRO_NORMAL in
> napi_gro_receive()") has applied batched GRO_NORMAL packets processing
> to all napi_gro_receive() users, including mac80211-based drivers.
>
> However, this change has led to a regression in iwlwifi driver [1][2] as
> it is required for NAPI users to call napi_complete_done() or
> napi_complete() and the end of every polling iteration, whilst iwlwifi
> doesn't use NAPI scheduling at all and just calls napi_gro_flush().
> In that particular case, packets which have not been already flushed
> from napi->rx_list stall in it until at least next Rx cycle.
>
> Fix this by adding a manual flushing of the list to iwlwifi driver right
> before napi_gro_flush() call to mimic napi_complete() logics.
>
> I prefer to open-code gro_normal_list() rather than exporting it for 2
> reasons:
> * to prevent from using it and napi_gro_flush() in any new drivers,
> as it is the *really* bad way to use NAPI that should be avoided;
> * to keep gro_normal_list() static and don't lose any CC optimizations.
>
> I also don't add the "Fixes:" tag as the mentioned commit was only a
> trigger that only exposed an improper usage of NAPI in this particular
> driver.
>
> [1] https://lore.kernel.org/netdev/PSXP216MB04388962C411CD0B17A86F47804A0@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM
> [2] https://bugzilla.kernel.org/show_bug.cgi?id=205647
>
> Signed-off-by: Alexander Lobakin <alobakin@dlink.ru>
> ---
We don't usually use "net: wireless: intel:" in the commit message, we
would use "iwlwifi: pcie:", but I don't care much.
Otherwise:
Acked-by: Luca Coelho <luciano.coelho@intel.com>
Thanks a lot for the fix!
Dave, I'm assuming you'll take this directly into your tree, right?
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] net: wireless: intel: iwlwifi: fix GRO_NORMAL packet stalling
2019-11-27 9:58 ` Luciano Coelho
@ 2019-11-27 10:12 ` Alexander Lobakin
[not found] ` <PSXP216MB0438B2F163C635F8B8B4AD8AA4440@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM>
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Lobakin @ 2019-11-27 10:12 UTC (permalink / raw)
To: Luciano Coelho
Cc: David S. Miller, Edward Cree, Jiri Pirko, Eric Dumazet,
Ido Schimmel, Paolo Abeni, Petr Machata, Sabrina Dubroca,
Florian Fainelli, Jassi Brar, Manish Chopra, GR-Linux-NIC-Dev,
Johannes Berg, Emmanuel Grumbach, Intel Linux Wireless,
Kalle Valo, Nicholas Johnson, Kenneth R. Crudup, netdev,
linux-wireless, linux-kernel
Luciano Coelho wrote 27.11.2019 12:58:
> On Wed, 2019-11-27 at 12:41 +0300, Alexander Lobakin wrote:
>> Commit 6570bc79c0df ("net: core: use listified Rx for GRO_NORMAL in
>> napi_gro_receive()") has applied batched GRO_NORMAL packets processing
>> to all napi_gro_receive() users, including mac80211-based drivers.
>>
>> However, this change has led to a regression in iwlwifi driver [1][2]
>> as
>> it is required for NAPI users to call napi_complete_done() or
>> napi_complete() and the end of every polling iteration, whilst iwlwifi
>> doesn't use NAPI scheduling at all and just calls napi_gro_flush().
>> In that particular case, packets which have not been already flushed
>> from napi->rx_list stall in it until at least next Rx cycle.
>>
>> Fix this by adding a manual flushing of the list to iwlwifi driver
>> right
>> before napi_gro_flush() call to mimic napi_complete() logics.
>>
>> I prefer to open-code gro_normal_list() rather than exporting it for 2
>> reasons:
>> * to prevent from using it and napi_gro_flush() in any new drivers,
>> as it is the *really* bad way to use NAPI that should be avoided;
>> * to keep gro_normal_list() static and don't lose any CC
>> optimizations.
>>
>> I also don't add the "Fixes:" tag as the mentioned commit was only a
>> trigger that only exposed an improper usage of NAPI in this particular
>> driver.
>>
>> [1]
>> https://lore.kernel.org/netdev/PSXP216MB04388962C411CD0B17A86F47804A0@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM
>> [2] https://bugzilla.kernel.org/show_bug.cgi?id=205647
>>
>> Signed-off-by: Alexander Lobakin <alobakin@dlink.ru>
>> ---
>
> We don't usually use "net: wireless: intel:" in the commit message, we
> would use "iwlwifi: pcie:", but I don't care much.
>
> Otherwise:
>
> Acked-by: Luca Coelho <luciano.coelho@intel.com>
Thank you!
> Thanks a lot for the fix!
>
> Dave, I'm assuming you'll take this directly into your tree, right?
Also please let me know if I should send v2 with Ack and fixed commit
subject!
> --
> Cheers,
> Luca.
Regards,
ᚷ ᛖ ᚢ ᚦ ᚠ ᚱ
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] net: wireless: intel: iwlwifi: fix GRO_NORMAL packet stalling
2019-11-27 9:41 [PATCH net] net: wireless: intel: iwlwifi: fix GRO_NORMAL packet stalling Alexander Lobakin
2019-11-27 9:58 ` Luciano Coelho
@ 2019-11-27 16:05 ` Edward Cree
2019-11-27 16:31 ` Alexander Lobakin
2019-11-27 19:23 ` David Miller
2 siblings, 1 reply; 11+ messages in thread
From: Edward Cree @ 2019-11-27 16:05 UTC (permalink / raw)
To: Alexander Lobakin, David S. Miller
Cc: Jiri Pirko, Eric Dumazet, Ido Schimmel, Paolo Abeni,
Petr Machata, Sabrina Dubroca, Florian Fainelli, Jassi Brar,
Manish Chopra, GR-Linux-NIC-Dev, Johannes Berg,
Emmanuel Grumbach, Luca Coelho, Intel Linux Wireless, Kalle Valo,
Nicholas Johnson, Kenneth R. Crudup, netdev, linux-wireless,
linux-kernel
On 27/11/2019 09:41, Alexander Lobakin wrote:
> Commit 6570bc79c0df ("net: core: use listified Rx for GRO_NORMAL in
> napi_gro_receive()") has applied batched GRO_NORMAL packets processing
> to all napi_gro_receive() users, including mac80211-based drivers.
>
> However, this change has led to a regression in iwlwifi driver [1][2] as
> it is required for NAPI users to call napi_complete_done() or
> napi_complete() and the end of every polling iteration, whilst iwlwifi
> doesn't use NAPI scheduling at all and just calls napi_gro_flush().
> In that particular case, packets which have not been already flushed
> from napi->rx_list stall in it until at least next Rx cycle.
>
> Fix this by adding a manual flushing of the list to iwlwifi driver right
> before napi_gro_flush() call to mimic napi_complete() logics.
>
> I prefer to open-code gro_normal_list() rather than exporting it for 2
> reasons:
> * to prevent from using it and napi_gro_flush() in any new drivers,
> as it is the *really* bad way to use NAPI that should be avoided;
> * to keep gro_normal_list() static and don't lose any CC optimizations.
>
> I also don't add the "Fixes:" tag as the mentioned commit was only a
> trigger that only exposed an improper usage of NAPI in this particular
> driver.
>
> [1] https://lore.kernel.org/netdev/PSXP216MB04388962C411CD0B17A86F47804A0@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM
> [2] https://bugzilla.kernel.org/show_bug.cgi?id=205647
>
> Signed-off-by: Alexander Lobakin <alobakin@dlink.ru>
Reviewed-by: Edward Cree <ecree@solarflare.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] net: wireless: intel: iwlwifi: fix GRO_NORMAL packet stalling
2019-11-27 16:05 ` Edward Cree
@ 2019-11-27 16:31 ` Alexander Lobakin
0 siblings, 0 replies; 11+ messages in thread
From: Alexander Lobakin @ 2019-11-27 16:31 UTC (permalink / raw)
To: Edward Cree
Cc: David S. Miller, Jiri Pirko, Eric Dumazet, Ido Schimmel,
Paolo Abeni, Petr Machata, Sabrina Dubroca, Florian Fainelli,
Jassi Brar, Manish Chopra, GR-Linux-NIC-Dev, Johannes Berg,
Emmanuel Grumbach, Luca Coelho, Intel Linux Wireless, Kalle Valo,
Nicholas Johnson, Kenneth R. Crudup, netdev, linux-wireless,
linux-kernel
Edward Cree wrote 27.11.2019 19:05:
> On 27/11/2019 09:41, Alexander Lobakin wrote:
>> Commit 6570bc79c0df ("net: core: use listified Rx for GRO_NORMAL in
>> napi_gro_receive()") has applied batched GRO_NORMAL packets processing
>> to all napi_gro_receive() users, including mac80211-based drivers.
>>
>> However, this change has led to a regression in iwlwifi driver [1][2]
>> as
>> it is required for NAPI users to call napi_complete_done() or
>> napi_complete() and the end of every polling iteration, whilst iwlwifi
>> doesn't use NAPI scheduling at all and just calls napi_gro_flush().
>> In that particular case, packets which have not been already flushed
>> from napi->rx_list stall in it until at least next Rx cycle.
>>
>> Fix this by adding a manual flushing of the list to iwlwifi driver
>> right
>> before napi_gro_flush() call to mimic napi_complete() logics.
>>
>> I prefer to open-code gro_normal_list() rather than exporting it for 2
>> reasons:
>> * to prevent from using it and napi_gro_flush() in any new drivers,
>> as it is the *really* bad way to use NAPI that should be avoided;
>> * to keep gro_normal_list() static and don't lose any CC
>> optimizations.
>>
>> I also don't add the "Fixes:" tag as the mentioned commit was only a
>> trigger that only exposed an improper usage of NAPI in this particular
>> driver.
>>
>> [1]
>> https://lore.kernel.org/netdev/PSXP216MB04388962C411CD0B17A86F47804A0@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM
>> [2] https://bugzilla.kernel.org/show_bug.cgi?id=205647
>>
>> Signed-off-by: Alexander Lobakin <alobakin@dlink.ru>
> Reviewed-by: Edward Cree <ecree@solarflare.com>
Thanks! And you were the first who's found the root of the issue.
Regards,
ᚷ ᛖ ᚢ ᚦ ᚠ ᚱ
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] net: wireless: intel: iwlwifi: fix GRO_NORMAL packet stalling
2019-11-27 9:41 [PATCH net] net: wireless: intel: iwlwifi: fix GRO_NORMAL packet stalling Alexander Lobakin
2019-11-27 9:58 ` Luciano Coelho
2019-11-27 16:05 ` Edward Cree
@ 2019-11-27 19:23 ` David Miller
2019-11-27 19:28 ` Alexander Lobakin
2 siblings, 1 reply; 11+ messages in thread
From: David Miller @ 2019-11-27 19:23 UTC (permalink / raw)
To: alobakin
Cc: ecree, jiri, edumazet, idosch, pabeni, petrm, sd, f.fainelli,
jaswinder.singh, manishc, GR-Linux-NIC-Dev, johannes.berg,
emmanuel.grumbach, luciano.coelho, linuxwifi, kvalo,
nicholas.johnson-opensource, kenny, netdev, linux-wireless,
linux-kernel
From: Alexander Lobakin <alobakin@dlink.ru>
Date: Wed, 27 Nov 2019 12:41:23 +0300
> Commit 6570bc79c0df ("net: core: use listified Rx for GRO_NORMAL in
> napi_gro_receive()") has applied batched GRO_NORMAL packets processing
> to all napi_gro_receive() users, including mac80211-based drivers.
>
> However, this change has led to a regression in iwlwifi driver [1][2] as
> it is required for NAPI users to call napi_complete_done() or
> napi_complete() and the end of every polling iteration, whilst iwlwifi
> doesn't use NAPI scheduling at all and just calls napi_gro_flush().
> In that particular case, packets which have not been already flushed
> from napi->rx_list stall in it until at least next Rx cycle.
>
> Fix this by adding a manual flushing of the list to iwlwifi driver right
> before napi_gro_flush() call to mimic napi_complete() logics.
>
> I prefer to open-code gro_normal_list() rather than exporting it for 2
> reasons:
> * to prevent from using it and napi_gro_flush() in any new drivers,
> as it is the *really* bad way to use NAPI that should be avoided;
> * to keep gro_normal_list() static and don't lose any CC optimizations.
>
> I also don't add the "Fixes:" tag as the mentioned commit was only a
> trigger that only exposed an improper usage of NAPI in this particular
> driver.
>
> [1] https://lore.kernel.org/netdev/PSXP216MB04388962C411CD0B17A86F47804A0@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM
> [2] https://bugzilla.kernel.org/show_bug.cgi?id=205647
>
> Signed-off-by: Alexander Lobakin <alobakin@dlink.ru>
Applied, thanks for the quick turnaround.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] net: wireless: intel: iwlwifi: fix GRO_NORMAL packet stalling
2019-11-27 19:23 ` David Miller
@ 2019-11-27 19:28 ` Alexander Lobakin
0 siblings, 0 replies; 11+ messages in thread
From: Alexander Lobakin @ 2019-11-27 19:28 UTC (permalink / raw)
To: David Miller
Cc: ecree, jiri, edumazet, idosch, pabeni, petrm, sd, f.fainelli,
jaswinder.singh, manishc, GR-Linux-NIC-Dev, johannes.berg,
emmanuel.grumbach, luciano.coelho, linuxwifi, kvalo,
nicholas.johnson-opensource, kenny, netdev, linux-wireless,
linux-kernel
David Miller wrote 27.11.2019 22:23:
> From: Alexander Lobakin <alobakin@dlink.ru>
> Date: Wed, 27 Nov 2019 12:41:23 +0300
>
>> Commit 6570bc79c0df ("net: core: use listified Rx for GRO_NORMAL in
>> napi_gro_receive()") has applied batched GRO_NORMAL packets processing
>> to all napi_gro_receive() users, including mac80211-based drivers.
>>
>> However, this change has led to a regression in iwlwifi driver [1][2]
>> as
>> it is required for NAPI users to call napi_complete_done() or
>> napi_complete() and the end of every polling iteration, whilst iwlwifi
>> doesn't use NAPI scheduling at all and just calls napi_gro_flush().
>> In that particular case, packets which have not been already flushed
>> from napi->rx_list stall in it until at least next Rx cycle.
>>
>> Fix this by adding a manual flushing of the list to iwlwifi driver
>> right
>> before napi_gro_flush() call to mimic napi_complete() logics.
>>
>> I prefer to open-code gro_normal_list() rather than exporting it for 2
>> reasons:
>> * to prevent from using it and napi_gro_flush() in any new drivers,
>> as it is the *really* bad way to use NAPI that should be avoided;
>> * to keep gro_normal_list() static and don't lose any CC
>> optimizations.
>>
>> I also don't add the "Fixes:" tag as the mentioned commit was only a
>> trigger that only exposed an improper usage of NAPI in this particular
>> driver.
>>
>> [1]
>> https://lore.kernel.org/netdev/PSXP216MB04388962C411CD0B17A86F47804A0@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM
>> [2] https://bugzilla.kernel.org/show_bug.cgi?id=205647
>>
>> Signed-off-by: Alexander Lobakin <alobakin@dlink.ru>
>
> Applied, thanks for the quick turnaround.
Thank you all folks!
Regards,
ᚷ ᛖ ᚢ ᚦ ᚠ ᚱ
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-11-27 19:29 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-27 9:41 [PATCH net] net: wireless: intel: iwlwifi: fix GRO_NORMAL packet stalling Alexander Lobakin
2019-11-27 9:58 ` Luciano Coelho
2019-11-27 10:12 ` Alexander Lobakin
[not found] ` <PSXP216MB0438B2F163C635F8B8B4AD8AA4440@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM>
2019-11-27 10:29 ` Alexander Lobakin
2019-11-27 11:16 ` Nicholas Johnson
2019-11-27 11:44 ` Alexander Lobakin
2019-11-27 13:38 ` Kalle Valo
2019-11-27 16:05 ` Edward Cree
2019-11-27 16:31 ` Alexander Lobakin
2019-11-27 19:23 ` David Miller
2019-11-27 19:28 ` Alexander Lobakin
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).