[PATCH net] gve: Fixes for napi_poll when budget is 0

[PATCH net] gve: Fixes for napi_poll when budget is 0

[Ziwei Xiao]

Netpoll will explicilty pass the polling call with a budget of 0 to
indicate it's clearing the Tx path only. For the gve_rx_poll and
gve_xdp_poll, they were mistakenly taking the 0 budget as the indication
to do all the work. Add check to avoid the rx path and xdp path being
called when budget is 0. And also add check to avoid napi_complete_done
being triggered when budget is 0 for netpoll.

Fixes: f5cedc84a30d ("gve: Add transmit and receive support")
Signed-off-by: Ziwei Xiao <ziweixiao@google.com>
---
 drivers/net/ethernet/google/gve/gve_main.c | 10 +++++-----
 drivers/net/ethernet/google/gve/gve_rx.c   |  4 ----
 drivers/net/ethernet/google/gve/gve_tx.c   |  4 ----
 3 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
index 276f996f95dc..5a84ccfd3423 100644
--- a/drivers/net/ethernet/google/gve/gve_main.c
+++ b/drivers/net/ethernet/google/gve/gve_main.c
@@ -254,16 +254,16 @@ static int gve_napi_poll(struct napi_struct *napi, int budget)
 	if (block->tx) {
 		if (block->tx->q_num < priv->tx_cfg.num_queues)
 			reschedule |= gve_tx_poll(block, budget);
-		else
+		else if (budget)
 			reschedule |= gve_xdp_poll(block, budget);
 	}
 
-	if (block->rx) {
+	if (block->rx && budget > 0) {
 		work_done = gve_rx_poll(block, budget);
 		reschedule |= work_done == budget;
 	}
 
-	if (reschedule)
+	if (reschedule || budget == 0)
 		return budget;
 
        /* Complete processing - don't unmask irq if busy polling is enabled */
@@ -298,12 +298,12 @@ static int gve_napi_poll_dqo(struct napi_struct *napi, int budget)
 	if (block->tx)
 		reschedule |= gve_tx_poll_dqo(block, /*do_clean=*/true);
 
-	if (block->rx) {
+	if (block->rx && budget > 0) {
 		work_done = gve_rx_poll_dqo(block, budget);
 		reschedule |= work_done == budget;
 	}
 
-	if (reschedule)
+	if (reschedule || budget == 0)
 		return budget;
 
 	if (likely(napi_complete_done(napi, work_done))) {
diff --git a/drivers/net/ethernet/google/gve/gve_rx.c b/drivers/net/ethernet/google/gve/gve_rx.c
index e84a066aa1a4..73655347902d 100644
--- a/drivers/net/ethernet/google/gve/gve_rx.c
+++ b/drivers/net/ethernet/google/gve/gve_rx.c
@@ -1007,10 +1007,6 @@ int gve_rx_poll(struct gve_notify_block *block, int budget)
 
 	feat = block->napi.dev->features;
 
-	/* If budget is 0, do all the work */
-	if (budget == 0)
-		budget = INT_MAX;
-
 	if (budget > 0)
 		work_done = gve_clean_rx_done(rx, budget, feat);
 
diff --git a/drivers/net/ethernet/google/gve/gve_tx.c b/drivers/net/ethernet/google/gve/gve_tx.c
index 6957a865cff3..9f6ffc4a54f0 100644
--- a/drivers/net/ethernet/google/gve/gve_tx.c
+++ b/drivers/net/ethernet/google/gve/gve_tx.c
@@ -925,10 +925,6 @@ bool gve_xdp_poll(struct gve_notify_block *block, int budget)
 	bool repoll;
 	u32 to_do;
 
-	/* If budget is 0, do all the work */
-	if (budget == 0)
-		budget = INT_MAX;
-
 	/* Find out how much work there is to be done */
 	nic_done = gve_tx_load_event_counter(priv, tx);
 	to_do = min_t(u32, (nic_done - tx->done), budget);
-- 
2.43.0.rc0.421.g78406f8d94-goog

Re: [PATCH net] gve: Fixes for napi_poll when budget is 0

[Jakub Kicinski]

On Thu,  9 Nov 2023 15:59:16 -0800 Ziwei Xiao wrote:
> Fixes: f5cedc84a30d ("gve: Add transmit and receive support")

You need to CC everyone who put their tag on that patch. Use:

./scripts/get_maintainer.pl --git-min-percent 25 0001-your.patch

> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> index 276f996f95dc..5a84ccfd3423 100644
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
> @@ -254,16 +254,16 @@ static int gve_napi_poll(struct napi_struct *napi, int budget)
>  	if (block->tx) {
>  		if (block->tx->q_num < priv->tx_cfg.num_queues)
>  			reschedule |= gve_tx_poll(block, budget);
> -		else
> +		else if (budget)

So here you use it as a bool

>  			reschedule |= gve_xdp_poll(block, budget);
>  	}
>  
> -	if (block->rx) {
> +	if (block->rx && budget > 0) {

here as a signed int

>  		work_done = gve_rx_poll(block, budget);
>  		reschedule |= work_done == budget;
>  	}
>  
> -	if (reschedule)
> +	if (reschedule || budget == 0)

and here you compare to 0

Why is every single condition different :S

Just add a new if, before the block->rx handling which does:

	if (!budget)
		return 0;
-- 
pw-bot: cr
pv-bot: cc