[DTrace-devel] [PATCH 2/2] cc, bpf: separate final BPF program creation from linking and loading

Eugene Loh eugene.loh at oracle.com
Thu Aug 24 22:50:17 UTC 2023


Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
noting previous discussion.  The test problems I encountered, however, 
appear to be my problems.

On 8/24/23 11:13, Kris Van Hees via DTrace-devel wrote:
> Some providers may need to do more complex code generaetion for the
> probe BPF program trampoline.  The generation of trampoline code was
> being done after BPF map creation, making it impossible for trampoline
> code to use constructs that require data to be added to BPF maps.
>
> The final BPF program handling has been changed to first create the
> final programs, then create the BPF maps, and finally link and load the
> BPF programs (performing the final relocations) and attaching them to
> their probes.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
>   libdtrace/dt_bpf.c   | 48 ++++++++++++++++++++++++++++++--------------
>   libdtrace/dt_bpf.h   |  1 +
>   libdtrace/dt_cc.c    |  4 ++--
>   libdtrace/dt_impl.h  |  4 ++++
>   libdtrace/dt_probe.c |  3 +++
>   libdtrace/dt_probe.h |  1 +
>   libdtrace/dt_work.c  |  4 ++++
>   7 files changed, 48 insertions(+), 17 deletions(-)
>
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> index 9688c444..5ae5f55f 100644
> --- a/libdtrace/dt_bpf.c
> +++ b/libdtrace/dt_bpf.c
> @@ -1085,12 +1085,11 @@ dt_bpf_reloc_error_prog(dtrace_hdl_t *dtp, dtrace_difo_t *dp)
>   }
>   
>   int
> -dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
> +dt_bpf_make_progs(dtrace_hdl_t *dtp, uint_t cflags)
>   {
>   	dt_probe_t	*prp;
>   	dtrace_difo_t	*dp;
>   	dt_ident_t	*idp = dt_dlib_get_func(dtp, "dt_error");
> -	dtrace_optval_t	dest_ok = DTRACEOPT_UNSET;
>   
>   	assert(idp != NULL);
>   
> @@ -1108,19 +1107,11 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
>   	idp->di_flags |= DT_IDFLG_CGREG;	/* mark it as inline-ready */
>   	dt_bpf_reloc_error_prog(dtp, dp);
>   
> -	/*
> -	 * Determine whether we can allow destructive actions.
> -	 */
> -	dtrace_getopt(dtp, "destructive", &dest_ok);
> -
>   	/*
>   	 * Now construct all the other programs.
>   	 */
>   	for (prp = dt_list_next(&dtp->dt_enablings); prp != NULL;
>   	     prp = dt_list_next(prp)) {
> -		int	fd;
> -		int	rc = -1;
> -
>   		/* Already done. */
>   		if (prp == dtp->dt_error)
>   			continue;
> @@ -1132,19 +1123,46 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
>   		if (prp->prov->impl->prog_type == BPF_PROG_TYPE_UNSPEC)
>   			continue;
>   
> -		dp = dt_program_construct(dtp, prp, cflags, NULL);
> +		dp = dt_construct(dtp, prp, cflags, NULL);
>   		if (dp == NULL)
>   			return -1;
> -		if (dp->dtdo_flags & DIFOFLG_DESTRUCTIVE &&
> +
> +		prp->difo = dp;
> +	}
> +
> +	return 0;
> +}
> +
> +int
> +dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
> +{
> +	dt_probe_t	*prp;
> +	dtrace_optval_t	dest_ok = DTRACEOPT_UNSET;
> +
> +	/*
> +	 * Determine whether we can allow destructive actions.
> +	 */
> +	dtrace_getopt(dtp, "destructive", &dest_ok);
> +
> +	for (prp = dt_list_next(&dtp->dt_enablings); prp != NULL;
> +	     prp = dt_list_next(prp)) {
> +		int	fd;
> +		int	rc = -1;
> +
> +		if (prp->difo == NULL)
> +			continue;
> +
> +		if (dt_link(dtp, prp, prp->difo, NULL) == -1)
> +			return -1;
> +
> +		if (prp->difo->dtdo_flags & DIFOFLG_DESTRUCTIVE &&
>   		    dest_ok == DTRACEOPT_UNSET)
>   			return dt_set_errno(dtp, EDT_DESTRUCTIVE);
>   
> -		fd = dt_bpf_load_prog(dtp, prp, dp, cflags);
> +		fd = dt_bpf_load_prog(dtp, prp, prp->difo, cflags);
>   		if (fd == -1)
>   			return -1;
>   
> -		dt_difo_free(dtp, dp);
> -
>   		if (prp->prov->impl->attach)
>   		    rc = prp->prov->impl->attach(dtp, prp, fd);
>   
> diff --git a/libdtrace/dt_bpf.h b/libdtrace/dt_bpf.h
> index 578b7cf9..0fee533b 100644
> --- a/libdtrace/dt_bpf.h
> +++ b/libdtrace/dt_bpf.h
> @@ -63,6 +63,7 @@ extern int dt_bpf_prog_load(enum bpf_prog_type prog_type,
>   			    const dtrace_difo_t *dp, uint32_t log_level,
>   			    char *log_buf, size_t log_buf_sz);
>   extern int dt_bpf_raw_tracepoint_open(const void *tp, int fd);
> +extern int dt_bpf_make_progs(struct dtrace_hdl *, uint_t);
>   extern int dt_bpf_load_progs(struct dtrace_hdl *, uint_t);
>   extern void dt_bpf_init_helpers(struct dtrace_hdl *dtp);
>   
> diff --git a/libdtrace/dt_cc.c b/libdtrace/dt_cc.c
> index 86807caa..a42109f1 100644
> --- a/libdtrace/dt_cc.c
> +++ b/libdtrace/dt_cc.c
> @@ -775,7 +775,7 @@ out:
>   	return err ? NULL : rv;
>   }
>   
> -static dtrace_difo_t *
> +dtrace_difo_t *
>   dt_construct(dtrace_hdl_t *dtp, dt_probe_t *prp, uint_t cflags, dt_ident_t *idp)
>   {
>   	dt_pcb_t	pcb;
> @@ -1264,7 +1264,7 @@ dt_link_resolve(dtrace_hdl_t *dtp, dtrace_difo_t *dp)
>   	}
>   }
>   
> -static int
> +int
>   dt_link(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp,
>   	dt_ident_t *idp)
>   {
> diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> index de46d3db..674905b9 100644
> --- a/libdtrace/dt_impl.h
> +++ b/libdtrace/dt_impl.h
> @@ -803,6 +803,10 @@ extern int dt_load_libs(dtrace_hdl_t *dtp);
>   extern void *dt_compile(dtrace_hdl_t *dtp, int context,
>   			dtrace_probespec_t pspec, void *arg, uint_t cflags,
>   			int argc, char *const argv[], FILE *fp, const char *s);
> +extern dtrace_difo_t *dt_construct(dtrace_hdl_t *dtp, struct dt_probe *prp,
> +				   uint_t cflags, dt_ident_t *idp);
> +extern int dt_link(dtrace_hdl_t *dtp, const struct dt_probe *prp,
> +		   dtrace_difo_t *dp, dt_ident_t *idp);
>   extern dtrace_difo_t *dt_program_construct(dtrace_hdl_t *dtp,
>   					   struct dt_probe *prp, uint_t cflags,
>   					   dt_ident_t *idp);
> diff --git a/libdtrace/dt_probe.c b/libdtrace/dt_probe.c
> index 8f32adce..93266ff7 100644
> --- a/libdtrace/dt_probe.c
> +++ b/libdtrace/dt_probe.c
> @@ -477,6 +477,9 @@ dt_probe_destroy(dt_probe_t *prp)
>   	else
>   		dtp = yypcb->pcb_hdl;
>   
> +	if (prp->difo)
> +		dt_difo_free(dtp, prp->difo);
> +
>   	if (prp->desc) {
>   		dtp->dt_probes[prp->desc->id] = NULL;
>   
> diff --git a/libdtrace/dt_probe.h b/libdtrace/dt_probe.h
> index 4b9396a7..c9839124 100644
> --- a/libdtrace/dt_probe.h
> +++ b/libdtrace/dt_probe.h
> @@ -54,6 +54,7 @@ typedef struct dt_probe {
>   	dtrace_typeinfo_t *argv;	/* output argument types */
>   	int argc;			/* output argument count */
>   	dt_probe_instance_t *pr_inst;	/* list of functions and offsets */
> +	dtrace_difo_t *difo;		/* BPF probe program */
>   } dt_probe_t;
>   
>   extern dt_probe_t *dt_probe_lookup2(dt_provider_t *, const char *);
> diff --git a/libdtrace/dt_work.c b/libdtrace/dt_work.c
> index 4492f563..1bb6104c 100644
> --- a/libdtrace/dt_work.c
> +++ b/libdtrace/dt_work.c
> @@ -150,6 +150,10 @@ dtrace_go(dtrace_hdl_t *dtp, uint_t cflags)
>                   setrlimit(RLIMIT_MEMLOCK, &rl);
>           }
>   
> +	/* Create the BPF programs. */
> +	if (dt_bpf_make_progs(dtp, cflags) == -1)
> +		return -1;
> +
>   	/* Create the global BPF maps. */
>   	if (dt_bpf_gmap_create(dtp) == -1)
>   		return -1;



More information about the DTrace-devel mailing list