On Fri, Aug 01, 2014 at 09:50:31AM -0700, Alexei Starovoitov wrote:

On Fri, Aug 1, 2014 at 9:06 AM, Pablo Neira Ayuso [off-list ref] wrote:

quoted

On Thu, Jul 31, 2014 at 02:02:19PM -0700, Alexei Starovoitov wrote:

quoted

On Thu, Jul 31, 2014 at 12:40 PM, Pablo Neira Ayuso [off-list ref] wrote:

quoted

On Wed, Jul 30, 2014 at 08:34:16PM -0700, Alexei Starovoitov wrote:

quoted

clean up names related to socket filtering and bpf in the following way:
- everything that deals with sockets keeps 'sk_*' prefix
- everything that is pure BPF is changed to 'bpf_*' prefix

split 'struct sk_filter' into
struct sk_filter {
      atomic_t        refcnt;
      struct rcu_head rcu;
      struct bpf_prog *prog;
};

I think you can use 'struct bpf_prog prog' instead so the entire
sk_filter remains in the same memory blob (as it is before this
patch).

You can add an inline function to retrieve the bpg prog from the
filter:

static inline struct bpf_prog *sk_filter_bpf(struct sk_filter *)

and use it whenever possible to fetch the bpf program. I'm suggesting
this because we can use the zero array size in the socket filtering
abstraction later on, if the function above is used, this just needs
one line in that function to be updated to fetch the program from the
placeholder.

correct. It would speed up SK_RUN_FILTER macro a little and I've
considered it, but decided to go with the pointer for two reasons:
1.In sk_attach_filter() the bpf_prog is allocated, then reallocated
as part of bpf_prepare_filter(). My patch #1 cleans up that part to
avoid 'struct sock *' dependency, so all bpf_* functions work
purely with bpf_prog... If bpf_prog is embedded inside sk_filter,
bpf_prepare_filter would need to have a callback to reallocate
the container struct and pass this callback through the chain
of calls, which is ugly

I think you can allocate the sk_filter once you get the final bpf
program, then you can memcpy() it. This adds some extra overhead in
the sk_attach_filter(), but that path is executed from user context
and it's also a rare operation (only once to attach the filter). It's
still not going to be a beauty, but IMO it's worth to focus on getting
that little speed up in the packet path at the cost of adding some
overhead on the socket attach path.

memcpy of 'bpf_prog' is not just 'not a beauty', it won't work, since
bpf_prog is freed via work_queue due to JIT. See bpf_jit_free()

[...]

I see, in this patch you renamed sk_filter to bpf_prog in
bpf_jit_free() so no access to sk_filter anymore and alignment needs a
closer look.

OK... let's stick to the struct bpf_prog pointer.