On Fri, Jun 26, 2020 at 3:13 PM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Fri, Jun 26, 2020 at 2:45 AM Jakub Sitnicki <jakub@xxxxxxxxxxxxxx> wrote: > > > > On Thu, Jun 25, 2020 at 10:50 PM CEST, Andrii Nakryiko wrote: > > > On Thu, Jun 25, 2020 at 7:17 AM Jakub Sitnicki <jakub@xxxxxxxxxxxxxx> 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. > > >> > > >> After this change bpf(PROG_QUERY) may block to allocate memory in > > >> bpf_prog_array_copy_to_user() for collected program IDs. This forces a > > >> change in how we protect access to the attached program in the query > > >> callback. Because bpf_prog_array_copy_to_user() can sleep, we switch from > > >> an RCU read lock to holding a mutex that serializes updaters. > > >> > > >> Because we allow only one BPF flow_dissector program to be attached to > > >> netns at all times, the bpf_prog_array pointed by net->bpf.run_array is > > >> always either detached (null) or one element long. > > >> > > >> No functional changes intended. > > >> > > >> Signed-off-by: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx> > > >> --- > > > > > > I wonder if instead of NULL prog_array, it's better to just use a > > > dummy empty (but allocated) array. Might help eliminate some of the > > > IFs, maybe even in the hot path. > > > > That was my initial approach, which I abandoned seeing that it leads to > > replacing NULL prog_array checks in flow_dissector with > > bpf_prog_array_is_empty() checks to determine which netns has a BPF > > program attached. So no IFs gone there. > > > > While on the hot path, where we run the program, we probably would still > > be left with an IF checking for empty prog_array to avoid building the > > context if no progs will RUN. > > > > The checks I'm referring to are on attach path, in > > flow_dissector_bpf_prog_attach_check(), and hot-path, > > __skb_flow_dissect(). > > > > Fair enough. > Acked-by: Andrii Nakryiko <andriin@xxxxxx>
