From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Alexander Lobakin <alexandr.lobakin@intel.com>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>
Cc: Alexander Lobakin <alexandr.lobakin@intel.com>,
Martin KaFai Lau <martin.lau@linux.dev>,
Song Liu <song@kernel.org>,
Jesper Dangaard Brouer <hawk@kernel.org>,
Jakub Kicinski <kuba@kernel.org>,
bpf@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 bpf] bpf, test_run: fix &xdp_frame misplacement for LIVE_FRAMES
Date: Mon, 13 Feb 2023 16:33:56 +0100 [thread overview]
Message-ID: <87ilg5vau3.fsf@toke.dk> (raw)
In-Reply-To: <20230213142747.3225479-1-alexandr.lobakin@intel.com>
Alexander Lobakin <alexandr.lobakin@intel.com> writes:
> &xdp_buff and &xdp_frame are bound in a way that
>
> xdp_buff->data_hard_start == xdp_frame
>
> It's always the case and e.g. xdp_convert_buff_to_frame() relies on
> this.
> IOW, the following:
>
> for (u32 i = 0; i < 0xdead; i++) {
> xdpf = xdp_convert_buff_to_frame(&xdp);
> xdp_convert_frame_to_buff(xdpf, &xdp);
> }
>
> shouldn't ever modify @xdpf's contents or the pointer itself.
> However, "live packet" code wrongly treats &xdp_frame as part of its
> context placed *before* the data_hard_start. With such flow,
> data_hard_start is sizeof(*xdpf) off to the right and no longer points
> to the XDP frame.
>
> Instead of replacing `sizeof(ctx)` with `offsetof(ctx, xdpf)` in several
> places and praying that there are no more miscalcs left somewhere in the
> code, unionize ::frm with ::data in a flex array, so that both starts
> pointing to the actual data_hard_start and the XDP frame actually starts
> being a part of it, i.e. a part of the headroom, not the context.
> A nice side effect is that the maximum frame size for this mode gets
> increased by 40 bytes, as xdp_buff::frame_sz includes everything from
> data_hard_start (-> includes xdpf already) to the end of XDP/skb shared
> info.
>
> Minor: align `&head->data` with how `head->frm` is assigned for
> consistency.
> Minor #2: rename 'frm' to 'frame' in &xdp_page_head while at it for
> clarity.
>
> (was found while testing XDP traffic generator on ice, which calls
> xdp_convert_frame_to_buff() for each XDP frame)
>
> Fixes: b530e9e1063e ("bpf: Add "live packet" mode for XDP in BPF_PROG_RUN")
> Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
> ---
> From v1[0]:
> - align `&head->data` with how `head->frm` is assigned for consistency
> (Toke);
> - rename 'frm' to 'frame' in &xdp_page_head (Jakub);
> - no functional changes.
>
> [0]
> https://lore.kernel.org/bpf/20230209172827.874728-1-alexandr.lobakin@intel.com
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
next prev parent reply other threads:[~2023-02-13 15:51 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-13 14:27 [PATCH v2 bpf] bpf, test_run: fix &xdp_frame misplacement for LIVE_FRAMES Alexander Lobakin
2023-02-13 15:33 ` Toke Høiland-Jørgensen [this message]
2023-02-14 15:24 ` Daniel Borkmann
2023-02-14 15:39 ` Alexander Lobakin
2023-02-14 16:04 ` Alexander Lobakin
2023-02-14 21:05 ` Toke Høiland-Jørgensen
2023-02-15 15:01 ` Alexander Lobakin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ilg5vau3.fsf@toke.dk \
--to=toke@redhat.com \
--cc=alexandr.lobakin@intel.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=hawk@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=netdev@vger.kernel.org \
--cc=song@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.