netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: "Toke Høiland-Jørgensen" <toke@kernel.org>
Cc: Florian Westphal <fw@strlen.de>,
	netfilter-devel@vger.kernel.org, bpf@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: [PATCH nf-next] netfilter: nf_tables: add ebpf expression
Date: Wed, 31 Aug 2022 17:26:24 +0200	[thread overview]
Message-ID: <20220831152624.GA15107@breakpoint.cc> (raw)
In-Reply-To: <87ilm84goh.fsf@toke.dk>

Toke Høiland-Jørgensen <toke@kernel.org> wrote:
> > Same with a 'nft list ruleset > /etc/nft.txt', reboot,
> > 'nft -f /etc/nft.txt' fails because user forgot to load/pin the program
> > first.
> 
> Right, so under what conditions is the identifier expected to survive,
> exactly? It's okay if it fails after a reboot, but it should keep
> working while the system is up?

Right, thats the question.  I think it boils down to 'least surprise',
which to me would mean useable labels are:

1. pinned name
2. elf filename
3. filter name

3) has the advantage that afaiu I can extend nft to use the dumped
id + program tag to query the name from the kernel, whereas 1+2 would
need to store the label.

1 and 2 have the upside that its easy to handle a 'file not found'
error.

> >> My point here was more that if it's just a label for human consumption,
> >> the comment field should be fine, didn't realise it was needed for the
> >> tool operation (and see above re: that).
> >
> > Yes, this is unfortunate.  I would like to avoid introducing an
> > asymmetry between input and output (as in "... add rule ebpf pinned
> > bla', but 'nft list ruleset' showing 'ebpf id 42') or similar, UNLESS we
> > can somehow use that alternate output to reconstruct that was originally
> > intended.  And so far I can only see that happening with storing some
> > label in the kernel for userspace to consume (elf filename, pinned name,
> > program name ... ).
> >
> > To give an example:
> >
> > With 'ebpf id 42', we might be able to let this get echoed back as if
> > user would have said 'ebpf progname myfilter' (I am making this up!),
> > just to have a more 'stable' identifier.
> >
> > This would make it necessary to also support load-by-program-name, of
> > course.
> 
> Seems like this kind of mapping can be done in userspace without
> involving the kernel?
>
> For example, the 'progname' thing could be implemented by defining an
> nft-specific pinning location so that 'ebpf progname myfilter' is
> equivalent to 'ebpf pinned /sys/bpf/nft/myfilter' and when nft receives
> an ID from the kernel it goes looking in /sys/bpf/nft to see if it can
> find the program with that ID and echoes it with the appropriate
> progname if it does exist?

Thats an interesting proposal.

I had not considered an nft specific namespace, just the raw pinned
filename.

> This could also be extended, so that if a user does '... add rule ebpf
> file /usr/lib/bpf/myrule.o' the nft binary stashes the id -> .o file
> mapping somewhere (in /run for instance) so that it can echo back where
> it got it from later?

Oh, I would prefer to abuse the kernel as a "database" directly for
that rather than keeping text files that have to be managed externally.

But, all things considered, what about this:

I'll respin, with the FILENAME attribute removed, so user says
'ebpf pinned bla', and on listing nft walks /sys/bpf/nft/ to see if
it can find the name again.

If it can't find it, print the id instead.

This would mean nft would also have to understand
'ebpf id 12' on input, but I think thats fine. We can document that
this is not the preferred method due to the difficulty of determining
the correct id to use.

With this, no 'extra' userspace-sake info needs to be stored.
We can revisit what do with 'ebpf file /bla/foo.o' once/if that gets
added.

What do you think?
Will take a while because I'll need to extend the nft side first to cope
with lack of 'FILENAME' attribute.

  parent reply	other threads:[~2022-08-31 15:26 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-31 10:16 [PATCH nf-next] netfilter: nf_tables: add ebpf expression Florian Westphal
2022-08-31 12:13 ` Toke Høiland-Jørgensen
2022-08-31 12:56   ` Florian Westphal
2022-08-31 13:41     ` Toke Høiland-Jørgensen
2022-08-31 13:57       ` Florian Westphal
2022-08-31 14:43         ` Toke Høiland-Jørgensen
2022-08-31 15:09           ` Pablo Neira Ayuso
2022-08-31 15:35             ` Florian Westphal
2022-08-31 20:38               ` Pablo Neira Ayuso
2022-08-31 15:26           ` Florian Westphal [this message]
2022-08-31 15:39             ` Alexei Starovoitov
2022-08-31 15:53               ` Florian Westphal
2022-08-31 17:26                 ` Alexei Starovoitov
2022-08-31 21:49                   ` Daniel Borkmann
2022-09-01  5:18                     ` Eyal Birger
2022-09-02 16:53                       ` Alexei Starovoitov
2022-09-05 17:50                         ` Eyal Birger
2022-09-01 10:14                     ` Florian Westphal
2022-09-02 17:06                       ` Alexei Starovoitov
2022-09-02 17:52                         ` Florian Westphal
2022-08-31 21:57                   ` Florian Westphal
2022-09-06  6:57                     ` Nicolas Dichtel
2022-09-07  3:04                       ` Alexei Starovoitov
2022-09-07 15:52                         ` Nicolas Dichtel
2022-09-01  8:08                   ` Jan Engelhardt
2022-08-31 20:44             ` Toke Høiland-Jørgensen
2022-08-31 13:44     ` 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=20220831152624.GA15107@breakpoint.cc \
    --to=fw@strlen.de \
    --cc=bpf@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=toke@kernel.org \
    /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).