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

[Kris Van Hees]

On Thu, Feb 23, 2023 at 07:46:09PM -0500, Eugene Loh via DTrace-devel wrote:
> 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.

Good point.

> 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).

Thanks.

> > @@ -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;
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel