On Thu, Feb 27, 2020 at 06:06:57PM +0100, Toke Høiland-Jørgensen wrote:

Alexei Starovoitov [off-list ref] writes:

quoted

On Thu, Feb 27, 2020 at 3:55 AM Toke Høiland-Jørgensen [off-list ref] wrote:

quoted

However, my issue with this encoding is that it is write-only: You can't
inspect a BPF program already loaded into the kernel and tell which type
it is. So my proposal would be to make it explicit: Expose the
expected_attach_type as a new field in bpf_prog_info so userspace can
query it, and clearly document it as, essentially, a program subtype
that can significantly affect how a program is treated by the kernel.

You had the same request for "freplace" target prog.

Yes, and for the same reason; I'm being consistent here :)

quoted

My answer to both is still the same:
Please take a look at drgn and the script that Andrey posted.
All this information is trivial to extract from the kernel
without introducing new uapi.

I'm sorry, but having this kind of write-only data structure is a
horrible API design; and saying "you can just parse the internal kernel
data structures to see what is going on" is a cop-out. The whole point
of having a stable UAPI is to make it possible for people to write
applications that make use of kernel features with an expectation that
those applications will keep working. XDP is a networking feature;
people building networking applications shouldn't have to chase internal
kernel APIs just to keep their packet processing programs working.

You're mistaking human needs for tooling needs.
Humans want to see all sorts of internal things and they can tweak drgn
script to look at them on the live kernel and examine kdump's vmcore
with the same drgn script.
Tooling needs uapi to work on a live system.
There are 26 attach_types that are used by various cgroup based progs,
sockmap, lirc, flow_dissector and tracing. Many of these attach types
are used in production and not a single time _tools_ had a need to
retrieve that field from the kernel.
Show me the tool that needs to read expected_attach_type back and
then we can discuss how to expose it in uapi.

There is another side of this. struct bpf_prog->pages field currently
is not exposed in uapi and I've seen tools doing
cat /proc/pid/fdinfo/fd|grep "memlock:"
just to retrive 'pages' field. That's the case where 'struct bpf_prog_info'
should be extended.