On Wed, 20 May 2020 10:40:00 -0700
sdf@xxxxxxxxxx wrote:
> > +static void __net_exit flow_dissector_pernet_pre_exit(struct net *net)
> > +{
> > + struct bpf_prog *attached;
> > +
> > + /* We don't lock the update-side because there are no
> > + * references left to this netns when we get called. Hence
> > + * there can be no attach/detach in progress.
> > + */
> > + rcu_read_lock();
> > + attached = rcu_dereference(net->flow_dissector_prog);
> > + if (attached) {
> > + RCU_INIT_POINTER(net->flow_dissector_prog, NULL);
> > + bpf_prog_put(attached);
> > + }
> > + rcu_read_unlock();
> > +}
> I wonder, should we instead refactor existing
> skb_flow_dissector_bpf_prog_detach to accept netns (instead of attr)
> can call that here? Instead of reimplementing it (I don't think we
> care about mutex lock/unlock efficiency here?). Thoughts?
I wanted to be nice to container-heavy workloads where network
namespaces get torn down frequently and in parallel and avoid
locking a global mutex. OTOH we already do it today, for instance in
devlink pre_exit callback.
In our case I think there is a way to have the cake and it eat too:
https://lore.kernel.org/bpf/20200521083435.560256-1-jakub@xxxxxxxxxxxxxx/
Thanks for reviewing it,
-jkbs
