Messages in this thread | | | Date | Fri, 10 Feb 2023 13:31:28 +0100 | Subject | Re: [PATCH bpf] bpf, test_run: fix &xdp_frame misplacement for LIVE_FRAMES | From | Alexander Lobakin <> |
| |
From: Toke Høiland-Jørgensen <toke@redhat.com> Date: Thu, 09 Feb 2023 21:58:07 +0100
> Alexander Lobakin <alexandr.lobakin@intel.com> writes: > >> From: Alexander Lobakin <alexandr.lobakin@intel.com> >> Date: Thu, 9 Feb 2023 18:28:27 +0100 >> >>> &xdp_buff and &xdp_frame are bound in a way that >>> >>> xdp_buff->data_hard_start == xdp_frame >> >> [...] >> >>> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c >>> index 2723623429ac..c3cce7a8d47d 100644 >>> --- a/net/bpf/test_run.c >>> +++ b/net/bpf/test_run.c >>> @@ -97,8 +97,11 @@ static bool bpf_test_timer_continue(struct bpf_test_timer *t, int iterations, >>> struct xdp_page_head { >>> struct xdp_buff orig_ctx; >>> struct xdp_buff ctx; >>> - struct xdp_frame frm; >>> - u8 data[]; >>> + union { >>> + /* ::data_hard_start starts here */ >>> + DECLARE_FLEX_ARRAY(struct xdp_frame, frm); >>> + DECLARE_FLEX_ARRAY(u8, data); >>> + }; >> >> BTW, xdp_frame here starts at 112 byte offset, i.e. in 16 bytes a >> cacheline boundary is hit, so xdp_frame gets sliced into halves: 16 >> bytes in CL1 + 24 bytes in CL2. Maybe we'd better align this union to >> %NET_SKB_PAD / %SMP_CACHE_BYTES / ... to avoid this? > > Hmm, IIRC my reasoning was that both those cache lines will be touched > by the code in xdp_test_run_batch(), so it wouldn't matter? But if > there's a performance benefit I don't mind adding an explicit alignment > annotation, certainly!
Let me retest both ways and will see. I saw some huge CPU loads on reading xdpf in ice_xdp_xmit(), so that was my first thought.
> >> (but in bpf-next probably) > > Yeah... > > -Toke >
Thanks, Olek
| |