[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e34bc47d-aada-4bf8-ac29-d3462f351f20@kernel.org>
Date: Tue, 10 Sep 2024 10:48:03 +0200
From: Jesper Dangaard Brouer <hawk@...nel.org>
To: Daniel Xu <dxu@...uu.xyz>, davem@...emloft.net, ast@...nel.org,
kuba@...nel.org, daniel@...earbox.net, john.fastabend@...il.com,
andrii@...nel.org
Cc: martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org,
yonghong.song@...ux.dev, kpsingh@...nel.org, sdf@...ichev.me,
haoluo@...gle.com, jolsa@...nel.org, netdev@...r.kernel.org,
bpf@...r.kernel.org, linux-kernel@...r.kernel.org, lorenzo@...nel.org,
aleksander.lobakin@...el.com, kernel-team@...a.com
Subject: Re: [PATCH bpf-next] bpf: cpumap: Move xdp:xdp_cpumap_kthread
tracepoint before rcv
On 06/09/2024 03.22, Daniel Xu wrote:
> cpumap takes RX processing out of softirq and onto a separate kthread.
> Since the kthread needs to be scheduled in order to run (versus softirq
> which does not), we can theoretically experience extra latency if the
> system is under load and the scheduler is being unfair to us.
>
> Moving the tracepoint to before passing the skb list up the stack allows
> users to more accurately measure enqueue/dequeue latency introduced by
> cpumap via xdp:xdp_cpumap_enqueue and xdp:xdp_cpumap_kthread tracepoints.
>
It makes sense for me to move this :-)
It actually fits my use-case even better.
> f9419f7bd7a5 ("bpf: cpumap add tracepoints") which added the tracepoints
> states that the intent behind them was for general observability and for
> a feedback loop to see if the queues are being overwhelmed. This change
> does not mess with either of those use cases but rather adds a third
> one.
Yes, my use-case is to this as a feedback loop, to see when queue is
overwhelmed as you say. I will soon be playing with this feature in
production environments, so I'm excited that it looks like you have
similar use-cases for this :-)
>
> Signed-off-by: Daniel Xu <dxu@...uu.xyz>
Acked-by: Jesper Dangaard Brouer <hawk@...nel.org>
> ---
> kernel/bpf/cpumap.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
> index fbdf5a1aabfe..a2f46785ac3b 100644
> --- a/kernel/bpf/cpumap.c
> +++ b/kernel/bpf/cpumap.c
> @@ -354,12 +354,14 @@ static int cpu_map_kthread_run(void *data)
>
> list_add_tail(&skb->list, &list);
> }
> - netif_receive_skb_list(&list);
>
> - /* Feedback loop via tracepoint */
> + /* Feedback loop via tracepoint.
> + * NB: keep before recv to allow measuring enqueue/dequeue latency.
> + */
> trace_xdp_cpumap_kthread(rcpu->map_id, n, kmem_alloc_drops,
> sched, &stats);
>
> + netif_receive_skb_list(&list);
> local_bh_enable(); /* resched point, may call do_softirq() */
> }
> __set_current_state(TASK_RUNNING);
Powered by blists - more mailing lists