From: Daniel Xu <dxu@dxuuu.xyz>
To: bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
Subject: [RFC PATCH bpf-next 0/2] Improve prog array uref semantics
Date: Tue, 22 Aug 2023 18:08:29 -0600 [thread overview]
Message-ID: <cover.1692748902.git.dxu@dxuuu.xyz> (raw)
This patchset changes the behavior of TC and XDP hooks during attachment
such that any BPF_MAP_TYPE_PROG_ARRAY that the prog uses has an extra
uref taken.
The goal behind this change is to try and prevent confusion for the
majority of use cases. The current behavior where when the last uref is
dropped the prog array map is emptied is quite confusing. Confusing
enough for there to be multiple references to it in ebpf-go [0][1].
Completely solving the problem is difficult. As stated in c9da161c6517
("bpf: fix clearing on persistent program array maps"), it is
difficult-to-impossible to walk the full dependency graph b/c it is too
dynamic.
However in practice, I've found that all progs in a tailcall chain
share the same prog array map. Knowing that, if we take a uref on any
used prog array map when the program is attached, we can simplify the
majority use case and make it more ergonomic.
I'll be the first to admit this is not a very clean solution. It does
not fully solve the problem. Nor does it make overall logic any simpler.
But I do think it makes a pretty big usability hole slightly smaller.
I've done some basic testing using a repro program [3] I wrote to debug
the original issue that eventually led me to this patchset. If we wanna
move forward with this approach, I'll resend with selftests.
[0]: https://github.com/cilium/ebpf/blob/01ebd4c1e2b9f8b3dd4fd2382aa1092c3c9bfc9d/doc.go#L22-L24
[1]: https://github.com/cilium/ebpf/blob/d1a52333f2c0fed085f8d742a5a3c164795d8492/collection.go#L320-L321
[2]: https://github.com/danobi/tc_tailcall_repro
Daniel Xu (2):
net: bpf: Make xdp and cls_bpf use bpf_prog_put_dev()
bpf: Take a uref on BPF_MAP_TYPE_PROG_ARRAY maps during dev attachment
include/linux/bpf.h | 1 +
kernel/bpf/devmap.c | 8 ++++----
kernel/bpf/syscall.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
net/core/dev.c | 16 +++++++--------
net/sched/cls_bpf.c | 4 ++--
5 files changed, 60 insertions(+), 15 deletions(-)
--
2.41.0
next reply other threads:[~2023-08-23 0:08 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-23 0:08 Daniel Xu [this message]
2023-08-23 0:08 ` [RFC PATCH bpf-next 1/2] net: bpf: Make xdp and cls_bpf use bpf_prog_put_dev() Daniel Xu
2024-10-30 6:36 ` [RFC PATCH bpf-next 0/2] Improve prog array uref semantics Daniel Xu
2024-11-16 22:17 ` Alexei Starovoitov
2024-11-20 15:55 ` Daniel Xu
2024-11-20 16:07 ` Alexei Starovoitov
2024-11-20 21:54 ` Daniel Xu
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=cover.1692748902.git.dxu@dxuuu.xyz \
--to=dxu@dxuuu.xyz \
--cc=bpf@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).