Re: [PATCH bpf-next 2/3] bpf, netns: Keep attached programs in bpf_prog_array

On Mon, Jun 22, 2020 at 9:04 AM Jakub Sitnicki [off-list ref] wrote:

Prepare for having multi-prog attachments for new netns attach types by
storing programs to run in a bpf_prog_array, which is well suited for
iterating over programs and running them in sequence.

Because bpf_prog_array is dynamically resized, after this change a
potentially blocking memory allocation in bpf(PROG_QUERY) callback can
happen, in order to collect program IDs before copying the values to
user-space supplied buffer. This forces us to adapt how we protect access
to the attached program in the callback. As bpf_prog_array_copy_to_user()
helper can sleep, we switch from an RCU read lock to holding a mutex that
serializes updaters.

To handle bpf(PROG_ATTACH) scenario when we are replacing an already
attached program, we introduce a new bpf_prog_array helper called
bpf_prog_array_replace_item that will exchange the old program with a new
one. bpf-cgroup does away with such helper by computing an index into the
array based on program position in an external list of attached
programs/links. Such approach seems fragile, however, when dummy progs can
be left in the array after a memory allocation failure on link release.

bpf-cgroup can have the same BPF program present multiple times in the
effective prog array due to inheritance. It also has strict
guarantee/requirement about relative order of programs in parent
cgroup vs child cgroups. For such cases, replacing a BPF program based
on its pointer is not going to work correctly.

We do need to make sure that cgroup detachment never fails by falling
back to replacing BPF prog with dummy prog, though. If you are
interested in a challenge, you are very welcome to do that! :)

No functional changes intended.

Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
---

LGTM.

Acked-by: Andrii Nakryiko <redacted>

 include/linux/bpf.h        |   3 +
 include/net/netns/bpf.h    |   5 +-
 kernel/bpf/core.c          |  19 +++--
 kernel/bpf/net_namespace.c | 137 +++++++++++++++++++++++++++----------
 net/core/flow_dissector.c  |  21 +++---
 5 files changed, 131 insertions(+), 54 deletions(-)

[...]

+
+void bpf_prog_array_delete_safe(struct bpf_prog_array *array,
+                               struct bpf_prog *old_prog)
+{
+       bpf_prog_array_replace_item(array, old_prog, &dummy_bpf_prog.prog);

nit: (void) cast to show we don't care about return code?

[...]