[DTrace-devel] [PATCH 1/3] Implement a 'state' BPF map to communicate tracing session state
Eugene Loh
eugene.loh at oracle.com
Fri Sep 4 11:41:54 PDT 2020
Looks good with two minor comments:
*) As we apparently agree, the dt_bpf_map_[lookup|update] code movement
does not belong in this patch.
*) The naming of the values DT_STATE_BEGANON and DT_STATE_ENDEDON
strike me as funny. A matter of opinion, to be sure. I guess "anon"
and "ANON" make me think of anonymous tracing; yes, I realize that's
not a thing in this version of DTrace, but so be it. Also, the strings
BEGIN and END are common, while the past tenses BEGAN and ENDED are
not. Also, "on" is less descriptive than, e.g., "CPU". So, my $0.02
would be that names like DT_STATE_BEGIN_CPU and DT_STATE_END_CPU would
be clearer, but I do not intend to wage a war of opinions. So... for
your consideration.
On 08/27/2020 11:54 AM, Kris Van Hees wrote:
> The is a need to communicate information about the tracing session
> between the BPF programs we generate and the userspace consumer. The
> current need covers:
>
> - DT_STATE_ACTIVITY Records the activity state of the session
> - DT_STATE_BEGANON Records the CPU the BEGIN probe ran on
> - DT_STATE_ENDEDON Records the CPU the END probe ran on
>
> Patches will be forthcoming that provide the implementation of these
> features.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
> libdtrace/dt_bpf.c | 73 +++++++++++++++++++++++++-------------------
> libdtrace/dt_dlibs.c | 1 +
> libdtrace/dt_impl.h | 1 +
> libdtrace/dt_state.h | 27 ++++++++++++++++
> 4 files changed, 71 insertions(+), 31 deletions(-)
> create mode 100644 libdtrace/dt_state.h
>
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> index 0531a37e..a381a86e 100644
> --- a/libdtrace/dt_bpf.c
> +++ b/libdtrace/dt_bpf.c
> @@ -14,6 +14,7 @@
> #include <dtrace.h>
> #include <dt_impl.h>
> #include <dt_probe.h>
> +#include <dt_state.h>
> #include <dt_bpf.h>
> #include <port.h>
>
> @@ -51,6 +52,37 @@ dt_bpf_error(dtrace_hdl_t *dtp, const char *fmt, ...)
> return dt_set_errno(dtp, EDT_BPF);
> }
>
> +/*
> + * Load the value for the given key in the map referenced by the given fd.
> + */
> +int dt_bpf_map_lookup(int fd, const void *key, void *val)
> +{
> + union bpf_attr attr;
> +
> + memset(&attr, 0, sizeof(attr));
> + attr.map_fd = fd;
> + attr.key = (uint64_t)(unsigned long)key;
> + attr.value = (uint64_t)(unsigned long)val;
> +
> + return bpf(BPF_MAP_LOOKUP_ELEM, &attr);
> +}
> +
> +/*
> + * Store the (key, value) pair in the map referenced by the given fd.
> + */
> +int dt_bpf_map_update(int fd, const void *key, const void *val)
> +{
> + union bpf_attr attr;
> +
> + memset(&attr, 0, sizeof(attr));
> + attr.map_fd = fd;
> + attr.key = (uint64_t)(unsigned long)key;
> + attr.value = (uint64_t)(unsigned long)val;
> + attr.flags = 0;
> +
> + return bpf(BPF_MAP_UPDATE_ELEM, &attr);
> +}
> +
> static int
> create_gmap(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
> int ksz, int vsz, int size)
> @@ -86,6 +118,9 @@ create_gmap(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
> * Create the global BPF maps that are shared between all BPF programs in a
> * single tracing session:
> *
> + * - state: DTrace session state, used to communicate state between BPF
> + * programs and userspace. The content of the map is defined in
> + * dt_state.h.
> * - buffers: Perf event output buffer map, associating a perf event output
> * buffer with each CPU. The map is indexed by CPU id.
> * - cpuinfo: CPU information map, associating a cpuinfo_t structure with
> @@ -149,6 +184,13 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
> tvarc = dt_idhash_peekid(dtp->dt_tls) - DIF_VAR_OTHER_UBASE;
>
> /* Create global maps as long as there are no errors. */
> + dtp->dt_stmap_fd = create_gmap(dtp, "state", BPF_MAP_TYPE_ARRAY,
> + sizeof(DT_STATE_KEY_TYPE),
> + sizeof(DT_STATE_VAL_TYPE),
> + DT_STATE_NUM_ELEMS);
> + if (dtp->dt_stmap_fd == -1)
> + return -1; /* dt_errno is set for us */
> +
> if (create_gmap(dtp, "buffers", BPF_MAP_TYPE_PERF_EVENT_ARRAY,
> sizeof(uint32_t), sizeof(uint32_t),
> dtp->dt_conf.num_online_cpus) == -1)
> @@ -185,37 +227,6 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
> return 0;
> }
>
> -/*
> - * Load the value for the given key in the map referenced by the given fd.
> - */
> -int dt_bpf_map_lookup(int fd, const void *key, void *val)
> -{
> - union bpf_attr attr;
> -
> - memset(&attr, 0, sizeof(attr));
> - attr.map_fd = fd;
> - attr.key = (uint64_t)(unsigned long)key;
> - attr.value = (uint64_t)(unsigned long)val;
> -
> - return bpf(BPF_MAP_LOOKUP_ELEM, &attr);
> -}
> -
> -/*
> - * Store the (key, value) pair in the map referenced by the given fd.
> - */
> -int dt_bpf_map_update(int fd, const void *key, const void *val)
> -{
> - union bpf_attr attr;
> -
> - memset(&attr, 0, sizeof(attr));
> - attr.map_fd = fd;
> - attr.key = (uint64_t)(unsigned long)key;
> - attr.value = (uint64_t)(unsigned long)val;
> - attr.flags = 0;
> -
> - return bpf(BPF_MAP_UPDATE_ELEM, &attr);
> -}
> -
> /*
> * Perform relocation processing on a program.
> */
> diff --git a/libdtrace/dt_dlibs.c b/libdtrace/dt_dlibs.c
> index 83eb4a10..15b8525a 100644
> --- a/libdtrace/dt_dlibs.c
> +++ b/libdtrace/dt_dlibs.c
> @@ -65,6 +65,7 @@ static const dt_ident_t dt_bpf_symbols[] = {
> DT_BPF_SYMBOL(cpuinfo, DT_IDENT_PTR),
> DT_BPF_SYMBOL(gvars, DT_IDENT_PTR),
> DT_BPF_SYMBOL(mem, DT_IDENT_PTR),
> + DT_BPF_SYMBOL(state, DT_IDENT_PTR),
> DT_BPF_SYMBOL(strtab, DT_IDENT_PTR),
> DT_BPF_SYMBOL(tvars, DT_IDENT_PTR),
> /* BPF internal identifiers */
> diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> index 0f79b45f..60095a80 100644
> --- a/libdtrace/dt_impl.h
> +++ b/libdtrace/dt_impl.h
> @@ -339,6 +339,7 @@ struct dtrace_hdl {
> int dt_ddefs_fd; /* file descriptor for D CTF debugging cache */
> int dt_stdout_fd; /* file descriptor for saved stdout */
> int dt_poll_fd; /* file descriptor for event polling */
> + int dt_stmap_fd; /* file descriptor for the 'state' BPF map */
> dtrace_handle_err_f *dt_errhdlr; /* error handler, if any */
> void *dt_errarg; /* error handler argument */
> dtrace_prog_t *dt_errprog; /* error handler program, if any */
> diff --git a/libdtrace/dt_state.h b/libdtrace/dt_state.h
> new file mode 100644
> index 00000000..9acb66b2
> --- /dev/null
> +++ b/libdtrace/dt_state.h
> @@ -0,0 +1,27 @@
> +/*
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + *
> + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
> + */
> +
> +#ifndef _DT_STATE_H
> +#define _DT_STATE_H
> +
> +/*
> + * DTrace 'state' BPF map.
> + *
> + * The keys are uint32_t values corresponding to dt_state_elem_t values.
> + * The values are uint32_t values.
> + */
> +#define DT_STATE_KEY_TYPE uint32_t
> +#define DT_STATE_VAL_TYPE uint32_t
> +
> +typedef enum dt_state_elem {
> + DT_STATE_ACTIVITY = 0, /* activity state of the seesion */
> + DT_STATE_BEGANON, /* cpu BEGIN probe executed on */
> + DT_STATE_ENDEDON, /* cpu END probe executed on */
> + DT_STATE_NUM_ELEMS
> +} dt_state_elem_t;
> +
> +#endif /* _DT_STATE_H */
More information about the DTrace-devel
mailing list