[DTrace-devel] [PATCH 1/3] Clear agg keys

[Eugene Loh]

On 8/8/23 17:24, eugene.loh at oracle.com wrote:

> From: Eugene Loh <eugene.loh at oracle.com>
>
> Aggregations use a "map of maps" to access per-CPU BPF maps whose
> keys are of size dt_maxtuplesize to accommodate keys of various
> sizes.  Care was exercised on the cg side to ensure that a key
> would be zeroed out before filling so that garbage bytes would not
> pollute the key.
>
> Exercise similar caution on the consumer side.
>
> THIS PATCH NEEDS TESTS.

Reminder:  if this gets worked into the trunc patch, then the trunc 
tests could be said to be the tests for this "Clear agg keys" stuff as 
well.  Also...

> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>   libdtrace/dt_aggregate.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/libdtrace/dt_aggregate.c b/libdtrace/dt_aggregate.c
> index 24faf1c1..1b8167d6 100644
> --- a/libdtrace/dt_aggregate.c
> +++ b/libdtrace/dt_aggregate.c
> @@ -570,11 +570,16 @@ hashnext:
>   
>   	/* Copy the key. */
>   	size = agg->dtagd_ksize;
> -	ptr = dt_alloc(dtp, size);
> +	ptr = dt_alloc(dtp, dtp->dt_maxtuplesize);
>   	if (ptr == NULL)
>   		return dt_set_errno(dtp, EDT_NOMEM);
>   
> -	memcpy(ptr, key, size);
> +	memset(ptr, 0, dtp->dt_maxtuplesize);

The dt_alloc() and memset() are dt_maxtuplesize because that's how big 
the keys for the aggs maps are.  But I don't think we ever use the last 
8 bytes of dt_maxtuplesize for aggs, do we?  If not, then the key size 
for the aggs map can be reduced by 8 bytes.  Again, though, where that 
goes depends on whether this patch is integrated into the trunc() patch.

> +	for (i = 0; i < agg->dtagd_nkrecs; i++) {
> +		rec = &agg->dtagd_krecs[i];
> +		off = rec->dtrd_offset;
> +		memcpy(&ptr[off], &key[off], rec->dtrd_size);
> +	}
>   	agd->dtada_key = ptr;
>   
>   	/* Copy the data. */