2019-01-18 17:50 UTC+0100 ~ Paolo Abeni <pab...@redhat.com>
> When updating a percpu map, bpftool currently copies the provided
> value only into the first per CPU copy of the specified value,
> all others instances are left zeroed.
>
> This change explicitly copies the user-provided bytes to all the
> per CPU instances, keeping the sub-command syntax unchanged.
>
> Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
> Signed-off-by: Paolo Abeni <pab...@redhat.com>
> ---
> tools/bpf/bpftool/map.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> index 2037e3dc864b..a73b5bb33ddf 100644
> --- a/tools/bpf/bpftool/map.c
> +++ b/tools/bpf/bpftool/map.c
> @@ -347,6 +347,20 @@ static char **parse_bytes(char **argv, const char *name,
> unsigned char *val,
> return argv + i;
> }
>
> +/* on per cpu maps we must copy the provided value on all value instances */
> +static void fill_value(struct bpf_map_info *info, void *value, __u32
> value_size)
> +{
> + unsigned int i, n, step;
> +
> + if (!map_is_per_cpu(info->type))
> + return;
> +
> + n = get_possible_cpus();
> + step = round_up(info->value_size, 8);
> + for (i = 1; i < n; i++)
> + memcpy(value + i * step, value, info->value_size);
> +}
> +
> static int parse_elem(char **argv, struct bpf_map_info *info,
> void *key, void *value, __u32 key_size, __u32 value_size,
> __u32 *flags, __u32 **value_fd)
> @@ -426,6 +440,8 @@ static int parse_elem(char **argv, struct bpf_map_info
> *info,
> argv = parse_bytes(argv, "value", value, value_size);
> if (!argv)
> return -1;
> +
> + fill_value(info, value, value_size);
Hi Paolo,
The patch looks good to me, thanks for it! One minor nit though, could
we make it more obvious in parse_elem() that fill_value() is only for
per-CPU maps? Reading parse_elem()'s code it seems that we need to fill
a value for all map types, this is a bit confusing. I was thinking about
either renaming the function ("fill_per_cpu_value()" or something like
this?), or moving the check on map_is_per_cpu() before the function is
called. What do you think?
> }
>
> return parse_elem(argv, info, key, NULL, key_size, value_size,
>
