netdev - Re: [PATCH bpf-next v2 5/6] tools: bpftool: print netfilter link info

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230414144927.GA5927@breakpoint.cc>
Date:   Fri, 14 Apr 2023 16:49:27 +0200
From:   Florian Westphal <fw@...len.de>
To:     Quentin Monnet <quentin@...valent.com>
Cc:     Florian Westphal <fw@...len.de>, netdev@...r.kernel.org,
        netfilter-devel@...r.kernel.org, bpf@...r.kernel.org,
        dxu@...uu.xyz, qde@...cy.de
Subject: Re: [PATCH bpf-next v2 5/6] tools: bpftool: print netfilter link info

Quentin Monnet <quentin@...valent.com> wrote:
> 2023-04-14 12:41 UTC+0200 ~ Florian Westphal <fw@...len.de>
> > Quentin Monnet <quentin@...valent.com> wrote:
> >> On Thu, 13 Apr 2023 at 14:36, Florian Westphal <fw@...len.de> wrote:
> >>>
> >>> Dump protocol family, hook and priority value:
> >>> $ bpftool link
> >>> 2: type 10  prog 20
> >>
> >> Could you please update link_type_name in libbpf (libbpf.c) so that we
> >> display "netfilter" here instead of "type 10"?
> > 
> > Done.
> 
> Thanks!
> 
> I'm just thinking we could also maybe print something nicer for the pf
> and the hook, "NF_INET_LOCAL_IN" would be more user-friendly than "hook 1"?

Done.  I've also made the first patch more restrictive wrt. allowed
attachment points and priorities.

Better safe than sorry, we can be more liberal later if there are
use-cases.

v3 will be coming next week.

> > I don't know how to make it work to actually attach it, because
> > the hook is unregistered when the link fd is closed.
> > 
> > So either bpftool would have to fork and auto-daemon (maybe
> > unexpected...) or wait/block until CTRL-C.
> > 
> > This also needs new libbpf api AFAICS because existing bpf_link
> > are specific to the program type, so I'd have to add something like:
> > 
> > struct bpf_link *
> > bpf_program__attach_netfilter(const struct bpf_program *prog,
> > 			      const struct bpf_netfilter_opts *opts)
> > 
> > Advice welcome.
> 
> OK, yes we'd need something like this if we wanted to load and attach
> from bpftool. If you already have the tooling elsewhere, it's maybe not
> necessary to add it here. Depends if you want users to be able to attach
> netfilter programs with bpftool or even libbpf.

[..]

> I'd say let's keep this out of the current patchset anyway. If we have a
> use case for attaching via libbpf/bpftool we can do this as a follow-up.

Sounds good to me.

Quentin Deslandes or Daniel Xu might want/need libbpf support for their
projects.

> The way I see it, "bpftool net" should provide a more structured
> overview of the different programs affecting networking, in particular
> for JSON. The idea would be to display all BPF programs that can affect
> packet processing. See what we have for XDP for example:
> 
> 
>     # bpftool net -p
>     [{
>             "xdp": [{
>                     "devname": "eni88np1",
>                     "ifindex": 12,
>                     "multi_attachments": [{
>                             "mode": "driver",
>                             "id": 1238
>                         },{
>                             "mode": "offload",
>                             "id": 1239
>                         }
>                     ]
>                 }
>             ],
>             "tc": [{
>                     "devname": "eni88np1",
>                     "ifindex": 12,
>                     "kind": "clsact/ingress",
>                     "name": "sample_ret0.o:[.text]",
>                     "id": 1241
>                 },{
>                     "devname": "eni88np1",
>                     "ifindex": 12,
>                     "kind": "clsact/ingress",
>                     "name": "sample_ret0.o:[.text]",
>                     "id": 1240
>                 }
>             ],
>             "flow_dissector": [
>                 "id": 1434
>             ]
>         }
>     ]
> 
> This gives us all the info about XDP programs at once, grouped by device
> when relevant. By contrast, listing them in "bpftool link" would likely
> only show one at a time, in an uncorrelated manner. Similarly, we could
> have netfilter sorted by pf then hook in "bpftool net". If there's more
> relevant info that we get from program info and not from the netfilter
> link, this would also be a good place to have it (but not sure there's
> any info we're missing from "bpftool link"?).

Currently 'bpftool link' shows everything wrt. netfilter bpf programs.

> But given that the info will be close, or identical, if not for the JSON
> structure, I don't mean to impose this to you - it's also OK to just
> skip "bpftool net" for now if you prefer.

I'll probably make 'bpftool net' and 'bpftool link' print identical
netfilter output, I'll check this on Monday (to make sure the formatting
doesn't seem out of place).

Its kinda silly to not have anything netfilter related in 'bpftool
net', this thing isn't named 'linkfilter' after all 8-)