[DTrace-devel] [PATCH 2/6] bpf: add agggen handling to get_agg()

[Eugene Loh]

Reviewed-by: Eugene Loh <eugene.loh at oracle.com>

How about calling the function dt_get_agg() instead of just get_agg(), 
both in the commit message and in the subject line. "get_agg" is the 
name of the file.

On 2/22/23 10:17, Kris Van Hees via DTrace-devel wrote:
> The get_agg() function will now consult the generation value for the
> aggregation id.  If existing data is of an earlier generation, the
> data is reset.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
>   bpf/get_agg.c | 18 +++++++++++++-----
>   1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/bpf/get_agg.c b/bpf/get_agg.c
> index aba8a229..37ac420e 100644
> --- a/bpf/get_agg.c
> +++ b/bpf/get_agg.c
> @@ -11,6 +11,8 @@
>   # define noinline	__attribute__((noinline))
>   #endif
>   
> +extern struct bpf_map_def agggen;
> +
>   /*
>    * Get a pointer to the data storage for an aggregation.  Regular aggregations
>    * are stored as an indexed aggregation with an empty key.  The 'id' parameter

The very next line (existing line, not seen in the patch) has the typo 
"aggregtion".  Fix the typo as part of this patch?  (not a big deal 
either way).

> @@ -21,16 +23,22 @@
>   noinline uint64_t *dt_get_agg(const dt_dctx_t *dctx, uint32_t id,
>   			      const char *key, uint64_t ival, const char *dflt)
>   {
> +	uint64_t	*genp;
>   	uint64_t	*valp;
>   
> +	/* get the gen value */
> +	genp = bpf_map_lookup_elem(&agggen, &id);
> +	if (genp == 0)
> +		return 0;
> +
>   	/* place the variable ID at the beginning of the key */
>   	*(uint32_t *)key = id;
>   
>   	/* try to look up the key */
>   	valp = bpf_map_lookup_elem(dctx->agg, key);
>   
> -	/* if not found, create it */
> -	if (valp == 0) {
> +	/* if not found, or older gen, set initial values */
> +	if (valp == 0 || valp[0] < *genp) {
>   		/* start with all zeroes */
>   		if (bpf_map_update_elem(dctx->agg, key, dflt, BPF_ANY) < 0)
>   			return 0;
> @@ -42,10 +50,10 @@ noinline uint64_t *dt_get_agg(const dt_dctx_t *dctx, uint32_t id,
>   		/* ival is nonzero only for min() and max() */
>   		if (ival)
>   			valp[1] = ival;
> -	}
>   
> -	/* increment the data counter */
> -	valp[0] += 1;
> +		/* set the gen value */
> +		valp[0] = *genp;
> +	}
>   
>   	/* advance past the data counter */
>   	valp += 1;