[DTrace-devel] [PATCH v2] Implement the ppid builtin D variable
Kris Van Hees
kris.van.hees at oracle.com
Thu Sep 17 11:58:03 PDT 2020
On Thu, Sep 17, 2020 at 01:31:20AM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
> bpf/get_bvar.c | 36 ++++++++++++++++++++++++
> bpf/map_state.c | 15 ++++++++++
> libdtrace/dt_bpf.c | 28 ++++++++++++++++++
> libdtrace/dt_state.h | 5 ++++
> test/unittest/variables/bvar/tst.ppid.d | 1 -
> test/unittest/variables/bvar/tst.ppid2.d | 20 +++++++++++++
> 6 files changed, 104 insertions(+), 1 deletion(-)
> create mode 100644 bpf/map_state.c
> create mode 100644 test/unittest/variables/bvar/tst.ppid2.d
>
> diff --git a/bpf/get_bvar.c b/bpf/get_bvar.c
> index 0cac46d5..8d595efb 100644
> --- a/bpf/get_bvar.c
> +++ b/bpf/get_bvar.c
> @@ -9,12 +9,14 @@
> #include <dtrace/conf.h>
> #include <dtrace/dif_defines.h>
> #include <dt_dctx.h>
> +#include <dt_state.h>
>
> #ifndef noinline
> # define noinline __attribute__((noinline))
> #endif
>
> extern struct bpf_map_def cpuinfo;
> +extern struct bpf_map_def state;
>
> noinline uint64_t dt_get_bvar(dt_mstate_t *mst, uint32_t id)
> {
> @@ -47,6 +49,40 @@ noinline uint64_t dt_get_bvar(dt_mstate_t *mst, uint32_t id)
>
> return val & 0x00000000ffffffffUL;
> }
> + case DIF_VAR_PPID: {
> + uint64_t ptr;
> + int32_t val = -1;
> + uint32_t key;
> + uint32_t *parent_off;
> + uint32_t *tgid_off;
> +
> + /*
> + * In the "state" map, look up the "struct task_struct" offsets
> + * of real_parent and tgid.
> + */
> + key = DT_STATE_TASK_PARENT_OFF;
> + parent_off = bpf_map_lookup_elem(&state, &key);
> + if (parent_off == NULL)
> + return -1;
> +
> + key = DT_STATE_TASK_TGID_OFF;
> + tgid_off = bpf_map_lookup_elem(&state, &key);
> + if (tgid_off == NULL)
> + return -1;
> +
> + /* Chase pointers val = current->real_parent->tgid. */
> + ptr = bpf_get_current_task();
> + if (ptr == 0)
> + return -1;
> + if (bpf_probe_read((void *) &ptr, 8,
> + (const void *) (ptr + *parent_off)))
> + return -1;
> + if (bpf_probe_read((void *) &val, 4,
> + (const void *) (ptr + *tgid_off)))
> + return -1;
> +
> + return (uint64_t) val;
> + }
> case DIF_VAR_UID: {
> uint64_t val = bpf_get_current_uid_gid();
>
> diff --git a/bpf/map_state.c b/bpf/map_state.c
> new file mode 100644
> index 00000000..cdd46f00
> --- /dev/null
> +++ b/bpf/map_state.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
> + */
> +#include <linux/bpf.h>
> +#include <stdint.h>
> +#include <bpf-helpers.h>
> +#include <dt_state.h>
> +
> +struct bpf_map_def SEC("maps") state = {
> + .type = BPF_MAP_TYPE_ARRAY,
> + .key_size = sizeof(DT_STATE_KEY_TYPE),
> + .value_size = sizeof(DT_STATE_VAL_TYPE),
> + .max_entries = DT_STATE_NUM_ELEMS,
> +};
This file is not necessary, which is also why the patch that introduces the map
does not add it. Incidentally, you didn't add it to the Build file so it is not
actually used either.
And yes, there are a few bpf/map_*.c files that are still in the tree even
though they do not get used - I need to put in a patch to remove those.
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> index 4e0cdc91..85657754 100644
> --- a/libdtrace/dt_bpf.c
> +++ b/libdtrace/dt_bpf.c
> @@ -115,6 +115,28 @@ create_gmap(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,
> return fd;
> }
>
> +static int
> +set_task_offsets(dtrace_hdl_t *dtp)
> +{
> + ctf_id_t type;
> + ctf_membinfo_t ctm;
> + ctf_file_t *cfp = dtp->dt_shared_ctf;
> +
> + type = ctf_lookup_by_name(cfp, "struct task_struct");
> + if (type == CTF_ERR)
> + return -1;
> +
> + if (ctf_member_info(cfp, type, "real_parent", &ctm) == CTF_ERR)
> + return -1;
> + dt_state_set_offparent(dtp, ctm.ctm_offset / NBBY);
> +
> + if (ctf_member_info(cfp, type, "tgid", &ctm) == CTF_ERR)
> + return -1;
> + dt_state_set_offtgid(dtp, ctm.ctm_offset / NBBY);
> +
> + return 0;
> +}
> +
> /*
> * Create the global BPF maps that are shared between all BPF programs in a
> * single tracing session:
> @@ -225,6 +247,12 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
> /* Populate the 'cpuinfo' map. */
> dt_bpf_map_update(ci_mapfd, &key, dtp->dt_conf.cpus);
>
> + /* Set some task_struct offsets in state. */
> + if (set_task_offsets(dtp)) {
> + dtp->dt_errno = EDT_CTF;
> + return -1;
> + }
Use:
if (set_task_offsets(dtp))
return dt_set_errno(dtp, EDT_CTF);
instead.
> +
> return 0;
> }
>
> diff --git a/libdtrace/dt_state.h b/libdtrace/dt_state.h
> index c6500dca..fe4f690d 100644
> --- a/libdtrace/dt_state.h
> +++ b/libdtrace/dt_state.h
> @@ -21,6 +21,8 @@ typedef enum dt_state_elem {
> DT_STATE_ACTIVITY = 0, /* activity state of the session */
> DT_STATE_BEGANON, /* cpu BEGIN probe executed on */
> DT_STATE_ENDEDON, /* cpu END probe executed on */
> + DT_STATE_TASK_PARENT_OFF, /* offsetof(struct task_struct, real_parent) */
> + DT_STATE_TASK_TGID_OFF, /* offsetof(struct task_struct, tgid) */
> DT_STATE_NUM_ELEMS
> } dt_state_elem_t;
>
> @@ -61,6 +63,9 @@ dt_state_set(dtrace_hdl_t *dtp, uint32_t key, uint32_t val)
>
> # define dt_state_get_beganon(dtp) dt_state_get(dtp, DT_STATE_BEGANON)
> # define dt_state_get_endedon(dtp) dt_state_get(dtp, DT_STATE_ENDEDON)
> +
> +# define dt_state_set_offparent(dtp, x) dt_state_set(dtp, DT_STATE_TASK_PARENT_OFF, x)
> +# define dt_state_set_offtgid(dtp, x) dt_state_set(dtp, DT_STATE_TASK_TGID_OFF, x)
For safety, change this to:
# define dt_state_set_offparent(dtp, x) dt_state_set(dtp, DT_STATE_TASK_PARENT_OFF, (x))
# define dt_state_set_offtgid(dtp, x) dt_state_set(dtp, DT_STATE_TASK_TGID_OFF, (x))
Technically we should do the same for dtp, but the 'dtp' argument is never
populated as an expression so it won't really matter. But x is much more
likely to be an expression.
> #endif
>
> #endif /* _DT_STATE_H */
> diff --git a/test/unittest/variables/bvar/tst.ppid.d b/test/unittest/variables/bvar/tst.ppid.d
> index e406b36f..87bc52da 100644
> --- a/test/unittest/variables/bvar/tst.ppid.d
> +++ b/test/unittest/variables/bvar/tst.ppid.d
> @@ -4,7 +4,6 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> -/* @@xfail: dtv2 */
>
> /*
> * ASSERTION: The 'ppid' variable can be accessed and is not -1.
> diff --git a/test/unittest/variables/bvar/tst.ppid2.d b/test/unittest/variables/bvar/tst.ppid2.d
> new file mode 100644
> index 00000000..7a50ac89
> --- /dev/null
> +++ b/test/unittest/variables/bvar/tst.ppid2.d
> @@ -0,0 +1,20 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +/*
> + * ASSERTION: The 'ppid' variable matches the scripting variable for BEGIN.
> + *
> + * SECTION: Variables/Built-in Variables/ppid
> + */
> +
> +#pragma D option quiet
> +
> +BEGIN {
> + trace(ppid);
> + trace($ppid);
> + exit(ppid == $ppid ? 0 : 1);
> +}
> --
> 2.18.4
>
>
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel
More information about the DTrace-devel
mailing list