[PATCH v2] mvneta: fix "napi poll" infinite loop

[PATCH v2] mvneta: fix "napi poll" infinite loop

[Julien Blais]

In percpu mode, when there's a network load, one of the cpus can be
solicited without having anything to process.
If 0 is returned to napi poll, napi will ignore the next requests,
causing an infinite loop with ISR handling.

Without this change, patches hang around fixing the queue at 0 and
the interrupt remains stuck on the 1st CPU.
The percpu conf is useless in this case, so we might as well remove it.

Signed-off-by: Julien Blais <webmaster@jbsky.fr>
---
 drivers/net/ethernet/marvell/mvneta.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 3f124268b..b6e89b888 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -3186,7 +3186,10 @@ static int mvneta_poll(struct napi_struct *napi, int budget)
 
 	if (rx_done < budget) {
 		cause_rx_tx = 0;
-		napi_complete_done(napi, rx_done);
+		if (rx_done)
+			napi_complete_done(napi, rx_done);
+		else
+			napi_complete(napi);
 
 		if (pp->neta_armada3700) {
 			unsigned long flags;
-- 
2.39.2



--
This e-mail and any attached files are confidential and may be legally privileged. If you are not the addressee, any disclosure, reproduction, copying, distribution, or other dissemination or use of this communication is strictly prohibited. If you have received this transmission in error please notify the sender immediately and then delete this mail.
E-mail transmission cannot be guaranteed to be secure or error free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender therefore does not accept liability for any errors or omissions in the contents of this message which arise as a result of e-mail transmission or changes to transmitted date not specifically approved by the sender.
If this e-mail or attached files contain information which do not relate to our professional activity we do not accept liability for such information.

Re: [PATCH v2] mvneta: fix "napi poll" infinite loop

[Maxime Chevallier]

Hello Julien,

On Wed, 11 Sep 2024 13:28:46 +0200
Julien Blais <webmaster@jbsky.fr> wrote:

> In percpu mode, when there's a network load, one of the cpus can be
> solicited without having anything to process.
> If 0 is returned to napi poll, napi will ignore the next requests,
> causing an infinite loop with ISR handling.
> 
> Without this change, patches hang around fixing the queue at 0 and
> the interrupt remains stuck on the 1st CPU.
> The percpu conf is useless in this case, so we might as well remove it.
> 
> Signed-off-by: Julien Blais <webmaster@jbsky.fr>

If this patch is a fix, you must also include a "Fixes" tag stating
which commit you are fixing. You must also include in the subject
whether you are targetting the "net-next" tree or "net". In your case,
this would be the "net" tree, that gathers the bugfixes.

> ---
>  drivers/net/ethernet/marvell/mvneta.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 3f124268b..b6e89b888 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -3186,7 +3186,10 @@ static int mvneta_poll(struct napi_struct *napi, int budget)
>  
>  	if (rx_done < budget) {
>  		cause_rx_tx = 0;
> -		napi_complete_done(napi, rx_done);
> +		if (rx_done)
> +			napi_complete_done(napi, rx_done);
> +		else
> +			napi_complete(napi);

I don't quite get this patch. as napi_complete is just calling
napi_complete_done(napi, 0), so this is basically

if (rx_done != 0)
	napi_complete_done(napi, rx_done);
else
	napi_complete_done(napi, 0);

So, nothing actually changes.

Can you elaborate more on the issue you are facing ?

Thanks,

Maxime

Re: [PATCH v2] mvneta: fix "napi poll" infinite loop

[Jakub Kicinski]

On Wed, 11 Sep 2024 13:28:46 +0200 Julien Blais wrote:
> This e-mail and any attached files are confidential and may be legally privileged. If you are not the addressee, any disclosure, reproduction, copying, distribution, or other dissemination or use of this communication is strictly prohibited. If you have received this transmission in error please notify the sender immediately and then delete this mail.
> E-mail transmission cannot be guaranteed to be secure or error free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender therefore does not accept liability for any errors or omissions in the contents of this message which arise as a result of e-mail transmission or changes to transmitted date not specifically approved by the sender.
> If this e-mail or attached files contain information which do not relate to our professional activity we do not accept liability for such information.

In addition to answering Maxime's question / reworking the patch
you'll need to figure out a way to post the code without this
disclaimer.

Re: [PATCH v2] mvneta: fix "napi poll" infinite loop

[Julien Blais]

Hello,

Maxime, you've enlightened me, yes, what I've committed is useless as
it's dealt with further on.

Anyway, I can't reproduce it, I was on the wrong track.

Yes, for my next commit I'll test the mail so as not to have this
“disclamer” anymore.

Thanks for your feedback!

All the best,

Julien Blais



--
This e-mail and any attached files are confidential and may be legally privileged. If you are not the addressee, any disclosure, reproduction, copying, distribution, or other dissemination or use of this communication is strictly prohibited. If you have received this transmission in error please notify the sender immediately and then delete this mail.
E-mail transmission cannot be guaranteed to be secure or error free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender therefore does not accept liability for any errors or omissions in the contents of this message which arise as a result of e-mail transmission or changes to transmitted date not specifically approved by the sender.
If this e-mail or attached files contain information which do not relate to our professional activity we do not accept liability for such information.