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

Toke Høiland-Jørgensen
2023-03-02T20:04:45Z

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