On 5/29/15 2:10 AM, Daniel Borkmann wrote:

+static void __prog_put_rcu(struct rcu_head *rcu)
+{
+    struct bpf_prog_aux *aux = container_of(rcu, struct bpf_prog_aux,
rcu);
+
+    free_used_maps(aux);
+    bpf_prog_free(aux->prog);

Not sure if it's worth it to move these two into a common helper shared
with bpf_prog_put()? Probably only in case that code should get further
extended.

I though about it too, but my recent re-reading of net/core/filter.c
taught me otherwise. We have too many tiny helper functions that
are hiding meaning instead of helping.
Like instead of having two pieces of the code:
do1(); do2(); do3(); and do1(); do2();
if we introduce a helper foo() { do1(); do2(); } and the code will do:
foo(), do3() and foo()
when the helper is close enough to invocation it's still easy to read,
but overtime the whole thing, imo, will become a mess. For example,
we have prog_release, prog_free, filter_release and all combinations
with and without __ prefix and _rcu suffix.
I think some of this stuff should be 'unhelpered'.
Like __sk_filter_release() and __bpf_prog_release() should be removed.
Of course, it's a grey line when to introduce a helper and when not to,
but just because two lines are close enough between two functions it
doesn't mean that helper is warranted. In this bpf_prog_put() case
I think helper is not needed _today_. If it grows, we'll reconsider.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to