netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: Quentin Monnet <quentin@isovalent.com>
Cc: Florian Westphal <fw@strlen.de>,
	netdev@vger.kernel.org, netfilter-devel@vger.kernel.org,
	bpf@vger.kernel.org, dxu@dxuuu.xyz, qde@naccy.de
Subject: Re: [PATCH bpf-next v2 5/6] tools: bpftool: print netfilter link info
Date: Fri, 14 Apr 2023 16:49:27 +0200	[thread overview]
Message-ID: <20230414144927.GA5927@breakpoint.cc> (raw)
In-Reply-To: <eeeaac99-9053-90c2-aa33-cc1ecb1ae9ca@isovalent.com>

Quentin Monnet <quentin@isovalent.com> wrote:
> 2023-04-14 12:41 UTC+0200 ~ Florian Westphal <fw@strlen.de>
> > Quentin Monnet <quentin@isovalent.com> wrote:
> >> On Thu, 13 Apr 2023 at 14:36, Florian Westphal <fw@strlen.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-)

  reply	other threads:[~2023-04-14 14:51 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-13 13:32 [PATCH bpf-next v2 0/6] bpf: add netfilter program type Florian Westphal
2023-04-13 13:32 ` [PATCH bpf-next v2 1/6] bpf: add bpf_link support for BPF_NETFILTER programs Florian Westphal
2023-04-13 13:32 ` [PATCH bpf-next v2 2/6] bpf: minimal support for programs hooked into netfilter framework Florian Westphal
2023-04-13 13:32 ` [PATCH bpf-next v2 3/6] netfilter: nfnetlink hook: dump bpf prog id Florian Westphal
2023-04-13 13:32 ` [PATCH bpf-next v2 4/6] netfilter: disallow bpf hook attachment at same priority Florian Westphal
2023-04-13 13:32 ` [PATCH bpf-next v2 5/6] tools: bpftool: print netfilter link info Florian Westphal
2023-04-13 21:14   ` Quentin Monnet
2023-04-14 10:41     ` Florian Westphal
2023-04-14 13:20       ` Quentin Monnet
2023-04-14 14:49         ` Florian Westphal [this message]
2023-04-14 14:54           ` Quentin Monnet
2023-04-13 13:32 ` [PATCH bpf-next v2 6/6] bpf: add test_run support for netfilter program type Florian Westphal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230414144927.GA5927@breakpoint.cc \
    --to=fw@strlen.de \
    --cc=bpf@vger.kernel.org \
    --cc=dxu@dxuuu.xyz \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=qde@naccy.de \
    --cc=quentin@isovalent.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).