diff mbox series

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

Message ID 20231109235916.1105860-1-ziweixiao@google.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net] gve: Fixes for napi_poll when budget is 0 | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1273 this patch: 1273
netdev/cc_maintainers fail 2 blamed authors not CCed: willemb@google.com csully@google.com; 12 maintainers not CCed: shailend@google.com ast@kernel.org john.fastabend@gmail.com daniel@iogearbox.net edumazet@google.com pabeni@redhat.com pkaligineedi@google.com willemb@google.com csully@google.com jeroendb@google.com hawk@kernel.org bpf@vger.kernel.org
netdev/build_clang success Errors and warnings before: 1301 this patch: 1301
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1301 this patch: 1301
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 53 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Ziwei Xiao Nov. 9, 2023, 11:59 p.m. UTC
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(-)

Comments

Jakub Kicinski Nov. 10, 2023, 8:36 p.m. UTC | #1
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;
diff mbox series

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