diff mbox series

[bpf-next] bpf/devmap: remove drops variable from bq_xmit_all()

Message ID 20210528024356.24333-1-liuhangbin@gmail.com (mailing list archive)
State Accepted
Commit e8e0f0f484780d7b90a63ea50020ac4bb027178d
Delegated to: BPF
Headers show
Series [bpf-next] bpf/devmap: remove drops variable from bq_xmit_all() | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for bpf-next
netdev/subject_prefix success Link
netdev/cc_maintainers fail 1 blamed authors not CCed: hawk@kernel.org; 9 maintainers not CCed: yhs@fb.com kpsingh@kernel.org hawk@kernel.org andrii@kernel.org kafai@fb.com ast@kernel.org songliubraving@fb.com davem@davemloft.net kuba@kernel.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 1 this patch: 1
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 27 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 1 this patch: 1
netdev/header_inline success Link

Commit Message

Hangbin Liu May 28, 2021, 2:43 a.m. UTC
As Colin pointed out, the first drops assignment after declaration will
be overwritten by the second drops assignment before using, which makes
it useless.

Since the drops variable will be used only once. Just remove it and
use "cnt - sent" in trace_xdp_devmap_xmit()

Reported-by: Colin Ian King <colin.king@canonical.com>
Fixes: cb261b594b41 ("bpf: Run devmap xdp_prog on flush instead of bulk enqueue")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 kernel/bpf/devmap.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

John Fastabend May 28, 2021, 4:02 a.m. UTC | #1
Hangbin Liu wrote:
> As Colin pointed out, the first drops assignment after declaration will
> be overwritten by the second drops assignment before using, which makes
> it useless.
> 
> Since the drops variable will be used only once. Just remove it and
> use "cnt - sent" in trace_xdp_devmap_xmit()
> 
> Reported-by: Colin Ian King <colin.king@canonical.com>
> Fixes: cb261b594b41 ("bpf: Run devmap xdp_prog on flush instead of bulk enqueue")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  kernel/bpf/devmap.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 

Thanks

Acked-by: John Fastabend <john.fastabend@gmail.com>
patchwork-bot+netdevbpf@kernel.org May 28, 2021, 8:20 p.m. UTC | #2
Hello:

This patch was applied to bpf/bpf-next.git (refs/heads/master):

On Thu, 27 May 2021 22:43:56 -0400 you wrote:
> As Colin pointed out, the first drops assignment after declaration will
> be overwritten by the second drops assignment before using, which makes
> it useless.
> 
> Since the drops variable will be used only once. Just remove it and
> use "cnt - sent" in trace_xdp_devmap_xmit()
> 
> [...]

Here is the summary with links:
  - [bpf-next] bpf/devmap: remove drops variable from bq_xmit_all()
    https://git.kernel.org/bpf/bpf-next/c/e8e0f0f48478

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
diff mbox series

Patch

diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index f9148daab0e3..2a75e6c2d27d 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -370,8 +370,8 @@  static int dev_map_bpf_prog_run(struct bpf_prog *xdp_prog,
 static void bq_xmit_all(struct xdp_dev_bulk_queue *bq, u32 flags)
 {
 	struct net_device *dev = bq->dev;
-	int sent = 0, drops = 0, err = 0;
 	unsigned int cnt = bq->count;
+	int sent = 0, err = 0;
 	int to_send = cnt;
 	int i;
 
@@ -388,8 +388,6 @@  static void bq_xmit_all(struct xdp_dev_bulk_queue *bq, u32 flags)
 		to_send = dev_map_bpf_prog_run(bq->xdp_prog, bq->q, cnt, dev);
 		if (!to_send)
 			goto out;
-
-		drops = cnt - to_send;
 	}
 
 	sent = dev->netdev_ops->ndo_xdp_xmit(dev, to_send, bq->q, flags);
@@ -408,9 +406,8 @@  static void bq_xmit_all(struct xdp_dev_bulk_queue *bq, u32 flags)
 		xdp_return_frame_rx_napi(bq->q[i]);
 
 out:
-	drops = cnt - sent;
 	bq->count = 0;
-	trace_xdp_devmap_xmit(bq->dev_rx, dev, sent, drops, err);
+	trace_xdp_devmap_xmit(bq->dev_rx, dev, sent, cnt - sent, err);
 }
 
 /* __dev_flush is called from xdp_do_flush() which _must_ be signaled