Linux-Wireless Archive on lore.kernel.org
 help / color / Atom feed
* [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	[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
       [not found]     ` <PSXP216MB0438B2F163C635F8B8B4AD8AA4440@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM>
@ 2019-11-27 10:29       ` Alexander Lobakin
  2019-11-27 11:16         ` Nicholas Johnson
  0 siblings, 1 reply; 11+ messages in thread
From: Alexander Lobakin @ 2019-11-27 10:29 UTC (permalink / raw)
  To: Nicholas Johnson
  Cc: Luciano Coelho, 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, Kenneth R. Crudup, netdev,
	linux-wireless, linux-kernel

Nicholas Johnson wrote 27.11.2019 13:23:
> Hi,

Hi Nicholas,

>  Sorry for top down reply, stuck with my phone. If it replies HTML
> then I am so done with Outlook client.
> 
>  Does my Reported-by tag apply here?
> 
>  As the reporter, should I check to see that it indeed solves the
> issue on the original hardware setup? I can do this within two hours
> and give Tested-by then.

Oops, I'm sorry I forgot to mention you in the commit message. Let's
see what Dave will say, I have no problems with waiting for your test
results and publishing v2.

>  Thanks
> 
>  Regards,
> 
>  Nicholas

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 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
  0 siblings, 2 replies; 11+ messages in thread
From: Nicholas Johnson @ 2019-11-27 11:16 UTC (permalink / raw)
  To: Alexander Lobakin
  Cc: Luciano Coelho, 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, Kenneth R. Crudup, netdev,
	linux-wireless, linux-kernel

On Wed, Nov 27, 2019 at 01:29:03PM +0300, Alexander Lobakin wrote:
> Nicholas Johnson wrote 27.11.2019 13:23:
> > Hi,
> 
> Hi Nicholas,
> 
> >  Sorry for top down reply, stuck with my phone. If it replies HTML
> > then I am so done with Outlook client.

I am very sorry to everybody for the improper reply. It looks like it 
was HTML as vger.kernel.org told me I was spam. If anybody knows a good 
email client for kernel development for Android then I am all ears.

I went home early and I have my computer(s) now.

> > 
> >  Does my Reported-by tag apply here?
> > 
> >  As the reporter, should I check to see that it indeed solves the
> > issue on the original hardware setup? I can do this within two hours
> > and give Tested-by then.
> 
> Oops, I'm sorry I forgot to mention you in the commit message. Let's
> see what Dave will say, I have no problems with waiting for your test
> results and publishing v2.

All good. :)

I tested the the patch and it works fine. Great work, the first 
hypothesis as to what the problem was is correct. It now connects to 
wireless networks without any hassles.

Reported-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
Tested-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>

I do not understand the networking subsystem well enough to give 
Reviewed-by, yet. Hopefully in time.

Thanks to everybody for handling my report.

> 
> >  Thanks
> > 
> >  Regards,
> > 
> >  Nicholas
> 
> Regards,
> ᚷ ᛖ ᚢ ᚦ ᚠ ᚱ

Regards,
Nicholas

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net] net: wireless: intel: iwlwifi: fix GRO_NORMAL packet stalling
  2019-11-27 11:16         ` Nicholas Johnson
@ 2019-11-27 11:44           ` Alexander Lobakin
  2019-11-27 13:38           ` Kalle Valo
  1 sibling, 0 replies; 11+ messages in thread
From: Alexander Lobakin @ 2019-11-27 11:44 UTC (permalink / raw)
  To: Nicholas Johnson
  Cc: Luciano Coelho, 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, Kenneth R. Crudup, netdev,
	linux-wireless, linux-kernel

Nicholas Johnson wrote 27.11.2019 14:16:
> On Wed, Nov 27, 2019 at 01:29:03PM +0300, Alexander Lobakin wrote:
>> Nicholas Johnson wrote 27.11.2019 13:23:
>> > Hi,
>> 
>> Hi Nicholas,
>> 
>> >  Sorry for top down reply, stuck with my phone. If it replies HTML
>> > then I am so done with Outlook client.
> 
> I am very sorry to everybody for the improper reply. It looks like it
> was HTML as vger.kernel.org told me I was spam. If anybody knows a good
> email client for kernel development for Android then I am all ears.
> 
> I went home early and I have my computer(s) now.
> 
>> >
>> >  Does my Reported-by tag apply here?
>> >
>> >  As the reporter, should I check to see that it indeed solves the
>> > issue on the original hardware setup? I can do this within two hours
>> > and give Tested-by then.
>> 
>> Oops, I'm sorry I forgot to mention you in the commit message. Let's
>> see what Dave will say, I have no problems with waiting for your test
>> results and publishing v2.
> 
> All good. :)
> 
> I tested the the patch and it works fine. Great work, the first
> hypothesis as to what the problem was is correct. It now connects to
> wireless networks without any hassles.
> 
> Reported-by: Nicholas Johnson 
> <nicholas.johnson-opensource@outlook.com.au>
> Tested-by: Nicholas Johnson 
> <nicholas.johnson-opensource@outlook.com.au>

Oh, much thanks for testing this out! I think this one will hit netdev
fixes tree soon.

> I do not understand the networking subsystem well enough to give
> Reviewed-by, yet. Hopefully in time.

I'm sure you will :)

> Thanks to everybody for handling my report.
> 
>> 
>> >  Thanks
>> >
>> >  Regards,
>> >
>> >  Nicholas
>> 
>> Regards,
>> ᚷ ᛖ ᚢ ᚦ ᚠ ᚱ
> 
> Regards,
> Nicholas

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 11:16         ` Nicholas Johnson
  2019-11-27 11:44           ` Alexander Lobakin
@ 2019-11-27 13:38           ` Kalle Valo
  1 sibling, 0 replies; 11+ messages in thread
From: Kalle Valo @ 2019-11-27 13:38 UTC (permalink / raw)
  To: Nicholas Johnson
  Cc: Alexander Lobakin, Luciano Coelho, 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,
	Kenneth R. Crudup, netdev\, linux-wireless\, linux-kernel\

Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au> writes:

> On Wed, Nov 27, 2019 at 01:29:03PM +0300, Alexander Lobakin wrote:
>> Nicholas Johnson wrote 27.11.2019 13:23:
>> > Hi,
>> 
>> Hi Nicholas,
>> 
>> >  Sorry for top down reply, stuck with my phone. If it replies HTML
>> > then I am so done with Outlook client.
>
> I am very sorry to everybody for the improper reply. It looks like it 
> was HTML as vger.kernel.org told me I was spam. If anybody knows a good 
> email client for kernel development for Android then I am all ears.

I use K-9 Mail because Greg K-H used it :) TBH I use it mostly only for
reading but seems to work quite well:

https://k9mail.github.io

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ 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, back to index

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

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org
	public-inbox-index linux-wireless

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git