On Tue, Jun 23, 2020 at 10:59:37PM +0200, Jakub Sitnicki wrote: > On Tue, Jun 23, 2020 at 09:33 PM CEST, Martin KaFai Lau wrote: > > On Tue, Jun 23, 2020 at 12:34:58PM +0200, Jakub Sitnicki wrote: > > > > [ ... ] > > > >> @@ -93,8 +108,16 @@ static int bpf_netns_link_update_prog(struct bpf_link > >> *link, > >> goto out_unlock; > >> } > >> > >> + run_array = rcu_dereference_protected(net->bpf.run_array[type], > >> + > >> lockdep_is_held(&netns_bpf_mutex)); > >> + if (run_array) > >> + ret = bpf_prog_array_replace_item(run_array, link->prog, > >> new_prog); > >> + else > > When will this happen? > > This will never happen, unless there is a bug. As long as there is a > link attached, run_array should never be detached (null). Because it can > be handled gracefully, we fail the bpf(LINK_UPDATE) syscall. > > Your question makes me think that perhaps it should trigger a warning, > with WARN_ON_ONCE, to signal clearly to the reader that this is an > unexpected state. > > WDYT? Thanks for confirming and the explanation.
If it will never happen, I would skip the "if (run_array)". That will help the code reading in the future. I would not WARN also.
