netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V1 net] net: ena: fix napi handler misbehavior when the napi budget is zero
@ 2019-12-08 17:30 Netanel Belgazal
  2019-12-09 18:34 ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Netanel Belgazal @ 2019-12-08 17:30 UTC (permalink / raw)
  To: davem, netdev
  Cc: Netanel Belgazal, dwmw, zorik, matua, saeedb, msw, aliguori,
	nafea, gtzalik, alisaidi, benh, akiyano

In netpoll the napi handler could be called with budget equal to zero.
Current ENA napi handler doesn't take that into consideration.

The napi handler handles Rx packets in a do-while loop.
Currently, the budget check happens only after decrementing the
budget, therefore the napi handler, in rare cases, could run over
MAX_INT packets.

In addition to that, this moves all budget related variables to int
calculation and stop mixing u32 to avoid ambiguity

Signed-off-by: Netanel Belgazal <netanel@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index c487d2a7d6dd..b4a145220aba 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -1238,8 +1238,8 @@ static int ena_io_poll(struct napi_struct *napi, int budget)
 	struct ena_napi *ena_napi = container_of(napi, struct ena_napi, napi);
 	struct ena_ring *tx_ring, *rx_ring;
 
-	u32 tx_work_done;
-	u32 rx_work_done;
+	int tx_work_done;
+	int rx_work_done = 0;
 	int tx_budget;
 	int napi_comp_call = 0;
 	int ret;
@@ -1256,7 +1256,11 @@ static int ena_io_poll(struct napi_struct *napi, int budget)
 	}
 
 	tx_work_done = ena_clean_tx_irq(tx_ring, tx_budget);
-	rx_work_done = ena_clean_rx_irq(rx_ring, napi, budget);
+	/* On netpoll the budget is zero and the handler should only clean the
+	 * tx completions.
+	 */
+	if (likely(budget))
+		rx_work_done = ena_clean_rx_irq(rx_ring, napi, budget);
 
 	/* If the device is about to reset or down, avoid unmask
 	 * the interrupt and return 0 so NAPI won't reschedule
-- 
2.17.2


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

* Re: [PATCH V1 net] net: ena: fix napi handler misbehavior when the napi budget is zero
  2019-12-08 17:30 [PATCH V1 net] net: ena: fix napi handler misbehavior when the napi budget is zero Netanel Belgazal
@ 2019-12-09 18:34 ` David Miller
  2019-12-10 11:28   ` Belgazal, Netanel
  0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2019-12-09 18:34 UTC (permalink / raw)
  To: netanel
  Cc: netdev, dwmw, zorik, matua, saeedb, msw, aliguori, nafea,
	gtzalik, alisaidi, benh, akiyano

From: Netanel Belgazal <netanel@amazon.com>
Date: Sun, 8 Dec 2019 17:30:26 +0000

> In netpoll the napi handler could be called with budget equal to zero.
> Current ENA napi handler doesn't take that into consideration.
> 
> The napi handler handles Rx packets in a do-while loop.
> Currently, the budget check happens only after decrementing the
> budget, therefore the napi handler, in rare cases, could run over
> MAX_INT packets.
> 
> In addition to that, this moves all budget related variables to int
> calculation and stop mixing u32 to avoid ambiguity
> 
> Signed-off-by: Netanel Belgazal <netanel@amazon.com>

Bug fixes need to have an appropriate Fixes: tag.

Please repost with that added, thank you.

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

* Re: [PATCH V1 net] net: ena: fix napi handler misbehavior when the napi budget is zero
  2019-12-09 18:34 ` David Miller
@ 2019-12-10 11:28   ` Belgazal, Netanel
  0 siblings, 0 replies; 3+ messages in thread
From: Belgazal, Netanel @ 2019-12-10 11:28 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Woodhouse, David, Machulsky, Zorik, Matushevsky,
	Alexander, Bshara, Saeed, Wilson, Matt, Liguori, Anthony, Bshara,
	Nafea, Tzalik, Guy, Saidi, Ali, Herrenschmidt, Benjamin,
	Kiyanovski, Arthur

I just did it.

Regards,
Netanel

On 12/9/19, 8:35 PM, "David Miller" <davem@davemloft.net> wrote:

    From: Netanel Belgazal <netanel@amazon.com>
    Date: Sun, 8 Dec 2019 17:30:26 +0000
    
    > In netpoll the napi handler could be called with budget equal to zero.
    > Current ENA napi handler doesn't take that into consideration.
    > 
    > The napi handler handles Rx packets in a do-while loop.
    > Currently, the budget check happens only after decrementing the
    > budget, therefore the napi handler, in rare cases, could run over
    > MAX_INT packets.
    > 
    > In addition to that, this moves all budget related variables to int
    > calculation and stop mixing u32 to avoid ambiguity
    > 
    > Signed-off-by: Netanel Belgazal <netanel@amazon.com>
    
    Bug fixes need to have an appropriate Fixes: tag.
    
    Please repost with that added, thank you.
    


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

end of thread, other threads:[~2019-12-10 11:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-08 17:30 [PATCH V1 net] net: ena: fix napi handler misbehavior when the napi budget is zero Netanel Belgazal
2019-12-09 18:34 ` David Miller
2019-12-10 11:28   ` Belgazal, Netanel

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).