Re: [PATCH bpf-next v5 1/7] bpf: add bpf_link support for BPF_NETFILTER programs

Florian Westphal
2023-04-27T09:10:39Z

Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > @@ -1560,6 +1562,12 @@ union bpf_attr {
> > */
> > __u64 cookie;
> > } tracing;
> > + struct {
> > + __u32 pf;
> > + __u32 hooknum;
>
> catching up on stuff a bit...
>
> enum nf_inet_hooks {
> NF_INET_PRE_ROUTING,
> NF_INET_LOCAL_IN,
> NF_INET_FORWARD,
> NF_INET_LOCAL_OUT,
> NF_INET_POST_ROUTING,
> NF_INET_NUMHOOKS,
> NF_INET_INGRESS = NF_INET_NUMHOOKS,
> };
>
> So it seems like this "hook number" is more like "hook type", is my
> understanding correct?

What is 'hook type'?

> If so, wouldn't it be cleaner and more uniform
> with, say, cgroup network hooks to provide hook type as
> expected_attach_type? It would also allow to have a nicer interface in
> libbpf, by specifying that as part of SEC():
>
> SEC("netfilter/pre_routing"), SEC("netfilter/local_in"), etc...

I don't understand how that would help.
Attachment needs a priority and a family (ipv4, arp, etc.).

If we allow netdev type we'll also need an ifindex.
Daniel Xu work will need to pass extra arguments ("please enable ip
defrag").

> Also, it seems like you actually didn't wire NETFILTER link support in
> libbpf completely. See bpf_link_create under tools/lib/bpf/bpf.c, it
> has to handle this new type of link as well. Existing tests seem a bit
> bare-bones for SEC("netfilter"), would it be possible to add something
> that will demonstrate it a bit better and will be actually executed at
> runtime and validated?

I can have a look.