diff mbox

[1/1] net: nps_enet: Disable interrupts before napi reschedule

Message ID 1464264006-26627-1-git-send-email-eladkan@mellanox.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Elad Kanfi May 26, 2016, noon UTC
From: Elad Kanfi <eladkan@mellanox.com>

Since NAPI works by shutting down event interrupts when theres
work and turning them on when theres none, the net driver must
make sure that interrupts are disabled when it reschedules polling.
By calling napi_reschedule, the driver switches to polling mode,
therefor there should be no interrupt interference.
Any received packets will be handled in nps_enet_poll by polling the HW
indication of received packet until all packets are handled.

Signed-off-by: Elad Kanfi <eladkan@mellanox.com>
Acked-by: Noam Camus <noamca@mellanox.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

Comments

Alexey Brodkin May 26, 2016, 5:53 p.m. UTC | #1
Hi Elad,

On Thu, 2016-05-26 at 15:00 +0300, Elad Kanfi wrote:
> From: Elad Kanfi <eladkan@mellanox.com>
> 
> Since NAPI works by shutting down event interrupts when theres
> work and turning them on when theres none, the net driver must
> make sure that interrupts are disabled when it reschedules polling.
> By calling napi_reschedule, the driver switches to polling mode,
> therefor there should be no interrupt interference.
> Any received packets will be handled in nps_enet_poll by polling the HW
> indication of received packet until all packets are handled.
> 
> Signed-off-by: Elad Kanfi <eladkan@mellanox.com>
> Acked-by: Noam Camus <noamca@mellanox.com>
> ---
>  drivers/net/ethernet/ezchip/nps_enet.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
> index 085f912..06f0317 100644
> --- a/drivers/net/ethernet/ezchip/nps_enet.c
> +++ b/drivers/net/ethernet/ezchip/nps_enet.c
> @@ -205,8 +205,10 @@ static int nps_enet_poll(struct napi_struct *napi, int budget)
>  		 * re-adding ourselves to the poll list.
>  		 */
>  
> -		if (priv->tx_skb && !tx_ctrl_ct)
> +		if (priv->tx_skb && !tx_ctrl_ct) {
> +			nps_enet_reg_set(priv, NPS_ENET_REG_BUF_INT_ENABLE, 0);
>  			napi_reschedule(napi);
> +		}
>  	}
>  
>  	return work_done;

We just bumped into the same problem (data exchange hangs on the very first "ping")
with released Linux v4.6 and linux-next on our nSIM OSCI virtual platform.

I believe it was commit 05c00d82f4d1 ("net: nps_enet: bug fix - handle lost tx interrupts")
that introduced the problem. At least reverting it I got networking working.

And indeed that patch fixes mentioned issue.
In other words...

Tested-by: Alexey Brodkin <abrodkin@synopsys.com>

P.S. Given my observation is correct please add following to your commit
message if you ever do a respin:
------------------>8---------------
Fixes: 05c00d82f4d1 ("net: nps_enet: bug fix - handle lost tx interrupts")

Cc: <stable@vger.kernel.org> # 4.6.x
------------------>8---------------
Vineet Gupta May 27, 2016, 4:07 a.m. UTC | #2
Hi Elad, Noam,

On Thursday 26 May 2016 11:23 PM, Alexey Brodkin wrote:

> 
> We just bumped into the same problem (data exchange hangs on the very first "ping")
> with released Linux v4.6 and linux-next on our nSIM OSCI virtual platform.
> 
> I believe it was commit 05c00d82f4d1 ("net: nps_enet: bug fix - handle lost tx interrupts")
> that introduced the problem. At least reverting it I got networking working.
> 
> And indeed that patch fixes mentioned issue.
> In other words...
> 
> Tested-by: Alexey Brodkin <abrodkin@synopsys.com>

FWIW, we now actively use the same driver (and same systemc model) in one of our
our simulation platforms used for testing regressions. So please try to keep arc
mailing list on CC for any nps_enet driver patches so we are in loop and know what
is going on !

Thx,
-Vineet
David Miller May 30, 2016, 5:35 a.m. UTC | #3
From: Elad Kanfi <eladkan@mellanox.com>
Date: Thu, 26 May 2016 15:00:06 +0300

> From: Elad Kanfi <eladkan@mellanox.com>
> 
> Since NAPI works by shutting down event interrupts when theres
> work and turning them on when theres none, the net driver must
> make sure that interrupts are disabled when it reschedules polling.
> By calling napi_reschedule, the driver switches to polling mode,
> therefor there should be no interrupt interference.
> Any received packets will be handled in nps_enet_poll by polling the HW
> indication of received packet until all packets are handled.
> 
> Signed-off-by: Elad Kanfi <eladkan@mellanox.com>
> Acked-by: Noam Camus <noamca@mellanox.com>

Applied.
Vineet Gupta June 2, 2016, 10:13 a.m. UTC | #4
On Monday 30 May 2016 11:05 AM, David Miller wrote:
> From: Elad Kanfi <eladkan@mellanox.com>
> Date: Thu, 26 May 2016 15:00:06 +0300
> 
>> From: Elad Kanfi <eladkan@mellanox.com>
>>
>> Since NAPI works by shutting down event interrupts when theres
>> work and turning them on when theres none, the net driver must
>> make sure that interrupts are disabled when it reschedules polling.
>> By calling napi_reschedule, the driver switches to polling mode,
>> therefor there should be no interrupt interference.
>> Any received packets will be handled in nps_enet_poll by polling the HW
>> indication of received packet until all packets are handled.
>>
>> Signed-off-by: Elad Kanfi <eladkan@mellanox.com>
>> Acked-by: Noam Camus <noamca@mellanox.com>
> 
> Applied.
> 

ARC osci platform in 4.6 currently have broken networking. This needs a stable
backport to 4.6

Upstream 86651650d16a359e4 ("net: nps_enet: Disable interrupts before napi
reschedule")

Thx,
-Vineet
diff mbox

Patch

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index 085f912..06f0317 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -205,8 +205,10 @@  static int nps_enet_poll(struct napi_struct *napi, int budget)
 		 * re-adding ourselves to the poll list.
 		 */
 
-		if (priv->tx_skb && !tx_ctrl_ct)
+		if (priv->tx_skb && !tx_ctrl_ct) {
+			nps_enet_reg_set(priv, NPS_ENET_REG_BUF_INT_ENABLE, 0);
 			napi_reschedule(napi);
+		}
 	}
 
 	return work_done;