Re: [PATCH RFC v2 bpf-next 0/3] bpf: add netfilter program type

["Toke Høiland-Jørgensen" <toke@kernel.org>]

Florian Westphal <fw@strlen.de> writes:

> Add minimal support to hook bpf programs to netfilter hooks,
> e.g. PREROUTING or FORWARD.
>
> For this the most relevant parts for registering a netfilter
> hook via the in-kernel api are exposed to userspace via bpf_link.
>
> The new program type is 'tracing style' and assumes skb dynptrs are used
> rather than 'direct packet access'.
>
> With this its possible to build a small test program such as:
>
> #include "vmlinux.h"
>
> extern int bpf_dynptr_from_skb(struct __sk_buff *skb, __u64 flags,
>                                struct bpf_dynptr *ptr__uninit) __ksym;
> extern void *bpf_dynptr_slice(const struct bpf_dynptr *ptr, uint32_t offset,
>                                    void *buffer, uint32_t buffer__sz) __ksym;
>
> SEC("netfilter")
> int nf_test(struct bpf_nf_ctx *ctx)
> {
> 	struct nf_hook_state *state = ctx->state;
> 	struct sk_buff *skb = ctx->skb;
> 	const struct iphdr *iph, _iph;
> 	const struct tcphdr *th, _th;
> 	struct bpf_dynptr ptr;
>
> 	if (bpf_dynptr_from_skb(skb, 0, &ptr))
> 		return NF_DROP;
>
> 	iph = bpf_dynptr_slice(&ptr, 0, &_iph, sizeof(_iph));
> 	if (!iph)
> 		return NF_DROP;
>
> 	th = bpf_dynptr_slice(&ptr, iph->ihl << 2, &_th, sizeof(_th));
> 	if (!th)
> 		return NF_DROP;
>
> 	bpf_printk("accept %x:%d->%x:%d, hook %d ifin %d\n", iph->saddr, bpf_ntohs(th->source), iph->daddr, bpf_ntohs(th->dest), state->hook, state->in->ifindex);
>         return NF_ACCEPT;
> }
>
> (output can be observed via /sys/kernel/tracing/trace_pipe).
>
> At this point I think its fairly complete.  Known problems are:
> - no test cases, I will look into this.  Might take some time
>   though because I might have to extend libbpf first.
> - nfnetlink_hook needs minor work so that it can dump the bpf
>   program id. As-is, userspace could see that a bpf program
>   is attached to e.g. forward and output, but it cannot tell
>   which program.  This is fairly simple and doesn't need changes
>   on bpf side.
>
> I will work on these address those two next unless anyone spots
> a fundamental issue with this rfc set.

I only spotted one small nit on the third patch, which I replied to
separately. Otherwise I think it looks pretty good, in fact I'm amazed
at how little code it takes to enable this; nice work! :)

-Toke