"Arthur Fabre" [off-list ref] writes:

On Mon Sep 30, 2024 at 12:52 PM CEST, Toke Høiland-Jørgensen wrote:

quoted

quoted

Thinking about it more, my only relectance for a registration API is how
to communicate the ID back to other consumers (our discussion below).

quoted

quoted

Dynamically registering fields means you have to share the returned ID
with whoever is interested, which sounds tricky.
If an XDP program sets a field like packet_id, every tracing
program that looks at it, and userspace service, would need to know what
the ID of that field is.
Is there a way to easily share that ID with all of them?

Right, so I'll admit this was one of the handwavy bits of my original
proposal, but I don't think it's unsolvable. You could do something like
(once, on application initialisation):

__u64 my_key = bpf_register_metadata_field(my_size); /* maybe add a name for introspection? */
bpf_map_update(&shared_application_config, &my_key_index, &my_key);

and then just get the key out of that map from all programs that want to
use it?

Passing it out of band works (whether it's via a pinned map like you
described, or through other means like a unix socket or some other
API), it's just more complicated.

Every consumer also needs to know about that API. That won't work with
standard tools. For example if we set a PACKET_ID KV, maybe we could
give it to pwru so it could track packets using it?
Without registering keys, we could pass it as a cli flag. With
registration, we'd have to have some helper to get the KV ID.

It also introduces ordering dependencies between the services on
startup, eg packet tracing hooks could only be attached once our XDP
service has registered a PACKET_ID KV, and they could query it's API.

Yeah, we definitely need a way to make that accessible and not too
cumbersome.

I suppose what we really need is a way to map an application-specific
identifier to an ID. And, well, those identifiers could just be (string)
names? That's what we do for CO-RE, after all. So you'd get something
like:

id = bpf_register_metadata_field("packet_id", 8, BPF_CREATE); /* register */

id = bpf_register_metadata_field("packet_id", 8, BPF_NO_CREATE); /* resolve */

and we make that idempotent, so that two callers using the same name and
size will just get the same id back; and if called with BPF_NO_CREATE,
you'll get -ENOENT if it hasn't already been registered by someone else?

We could even make this a sub-command of the bpf() syscall if we want it
to be UAPI, but that's not strictly necessary, applications can also
just call the registration from a syscall program at startup...

That's a nice API, it makes sharing the IDs much easier.

We still have to worry about collisions (what if two different things
want to add their own "packet_id" field?). But at least:

* "Any string" has many more possibilities than 0-64 keys.

Yes, and it's easy to namespace (by prefixing, like
APPNAME_my_metaname). But sure, if everyone uses very generic names that
will be a problem.

* bpf_register_metadata() could return an error if a field is already
registered, instead of silently letting an application overwrite
metadata

I don't think we can fundamentally solve the collision problem if we
also need to allow sharing keys (on purpose). I.e., we can't distinguish
between "these two applications deliberately want to share the packet_id
field" and "these two applications accidentally both picked packet_id as
their internal key".

I suppose we could pre-define some extra reserved keys if there turns
out to be widely used identifiers that many applications want. But I'm
not sure if that should be there from the start, it quickly becomes very
speculative(packet_id comes to mind as one that might be generally
useful, but I'm really not sure, TBH).

(although arguably we could have add a BPF_NOEXIST style flag
to the KV set() to kind of do the same).

A global registry will need locking, so not sure it's a good idea to
support inserting into it in the fast path?

At least internally, it still feels like we'd maintain a registry of
these string fields and make them configurable for each service to avoid
collisions.

Yeah, see above. Some kind of coordination (like a registry) is
impossible to avoid if you *want* to share data, but not sure how
common that is?

-Toke