Re: [PATCH] bpf, btf: store BTF dict id for kernel modules

[Nick Alcock <nick.alcock@oracle.com>]

On 25 Apr 2024, Kris Van Hees said:

> BPF requires a file descriptor (provided by the kernel) to be associated
> with BTF type ids for loadble kernel modules.  This file descriptor can

s/loadble/loadable/

> only be obtained from the kernel based on a BTF dict id for the module,
> and these ids are assigned dynamically at runtime when modules are
> loaded.

... were they *trying* to make this unnecessarily painful to use?

> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>

Reviewed-by: Nick Alcock <nick.alcock@oracle.com>

with a few niggles

> diff --git a/libdtrace/dt_btf.h b/libdtrace/dt_btf.h
> index 0146386b..60256f12 100644
> --- a/libdtrace/dt_btf.h
> +++ b/libdtrace/dt_btf.h
> @@ -15,7 +15,8 @@
>  extern "C" {
>  #endif
>  
> -typedef struct dt_btf	dt_btf_t;
> +typedef struct dt_btf		dt_btf_t;
> +typedef struct bpf_btf_info	btf_info_t;

This sort of thing will start throwing compiler errors whenever any user
#includes the header that gives the concrete definition of any of those
types, because we're probably typedeffing them there too, and unlike
struct definitions you cannot repeat typedefs.

(Not a problem now, but just noting that this sort of pattern tends to
store up trouble for the future.)

> @@ -339,6 +340,8 @@ struct dtrace_hdl {
>  	char *dt_ctfa_path;	/* path to vmlinux.ctfa */
>  	ctf_archive_t *dt_ctfa; /* ctf archive for the entire kernel tree */
>  	struct dt_btf *dt_shared_btf; /* BTF data for the kernel (shared) */
> +	int dt_btf_cnt;		/* BTF id count */
> +	int *dt_btf_fds;	/* BTF id-to-fd mapping */
>  	ctf_file_t *dt_shared_ctf; /* Handle to the shared CTF */
>  	dt_htab_t *dt_kernpaths; /* hash table of dt_kern_path_t's */
>  	dt_module_t *dt_exec;	/* pointer to executable module */

These don't seem to be used anywhere in this patch: did you swap between
storing globally and storing per-module?

-- 
NULL && (void)