Florian Westphal [off-list ref] writes:

Toke Høiland-Jørgensen [off-list ref] wrote:

quoted

quoted

Tag and program id are dumped to userspace on 'list' to allow to see which
program is in use in case the filename isn't available/present.

It seems a bit odd to include the file path in the kernel as well.

Its needed to be able to re-load the ruleset.

How does that work, exactly? Is this so that the userspace binary can
query the current ruleset, and feed it back to the kernel expecting it
to stay the same? Because in that case, if the pinned object goes away
in the meantime (or changes to a different program), this could lead to
some really hard to debug errors, where a reload subtly changes the
behaviour because the BPF program is not in fact the same.

Using IDs would avoid this ambiguity at least, so I think that's a
better solution. We'd have to make sure the BPF program is not released
completely until after the reload has finished, so that it doesn't
suddenly disappear.

quoted

But doesn't NFT already have a per-rule comment feature,
so why add another specifically for BPF?

You can attach up to 256 bytes to a rule, yes.
Might not be enough for a longer path, and there could be multiple
expressions in the same rule.

This way was the most simple solution.

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).

quoted

Instead we could just teach the
userspace utility to extract metadata from the BPF program (based on the
ID) like bpftool does. This would include the program name, BTW, so it
does have a semantic identifier.

Sure, I could change the grammar so it expects a tag or ID, e.g.
'ebpf id 42'

If thats preferred, I can change this, it avoids the need for storing
the name.

I think for echoing back, just relying on the ID is better as that is at
least guaranteed to stay constant for the lifetime of the BPF program in
the kernel. We could still support the 'pinned <path>' syntax on the
command line so that the initial load could be done from a pinned file,
just as a user interface improvement...

quoted

quoted

cbpf bytecode isn't supported.
add rule ... ebpf pinned "/sys/fs/bpf/myprog"

Any plan to also teach the nft binary to load a BPF program from an ELF
file (instead of relying on pinning)?

I used pinning because that is what '-m bpf' uses.

I'm not against supporting pinning, per se (except for the issues noted
above), but we could do multiple things, including supporting loading
the program from an object file. This is similar to how TC operates, for
instance...

-Toke