[PATCH 4/4] bpf: support new commands BPF_MAP_*_BATCH

[Dmitry V. Levin]

On Sun, Feb 23, 2020 at 11:13:36AM +0100, Paul Chaignon wrote:
> * xlat/bpf_commands.in (BPF_MAP_LOOKUP_BATCH): New constant introduced
> by Linux commit v5.6-rc1~151^2~46^2~23^2~7.
> (BPF_MAP_LOOKUP_AND_DELETE_BATCH): New constant introduced by Linux
> commit v5.6-rc1~151^2~46^2~23^2~4.
> (BPF_MAP_UPDATE_BATCH, BPF_MAP_DELETE_BATCH): New constants introduced
> by Linux commit v5.6-rc1~151^2~46^2~23^2~6.
> * bpf.c (BEGIN_BPF_CMD_DECODER(BPF_MAP_LOOKUP_BATCH),
> BEGIN_BPF_CMD_DECODER(BPF_MAP_LOOKUP_AND_DELETE_BATCH),
> BEGIN_BPF_CMD_DECODER(BPF_MAP_UPDATE_BATCH),
> BEGIN_BPF_CMD_DECODER(BPF_MAP_DELETE_BATCH)): Decode the new commands.
> * bpf_attr.h (BPF_MAP_LOOKUP_BATCH_struct,
> BPF_MAP_LOOKUP_AND_DELETE_BATCH_struct, BPF_MAP_UPDATE_BATCH_struct,
> BPF_MAP_DELETE_BATCH_struct): New structs introduced by Linux commit
> v5.6-rc1~151^2~46^2~23^2~7.
> * NEWS: Mention this.
> * tests/bpf.c (BPF_MAP_LOOKUP_BATCH_checks): Tests for the new commands.
> 
> Signed-off-by: Paul Chaignon <paul.chaignon at gmail.com>
> ---
>  NEWS                 |  3 +++
>  bpf.c                | 23 +++++++++++++++++++++++
>  bpf_attr.h           | 24 ++++++++++++++++++++++++
>  tests/bpf.c          | 27 +++++++++++++++++++++++++++
>  xlat/bpf_commands.in |  4 ++++
>  5 files changed, 81 insertions(+)
> 
> diff --git a/NEWS b/NEWS
> index 4c7ebe1c..e68f2ada 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -2,6 +2,9 @@ Noteworthy changes in release ?.? (????-??-??)
>  ==============================================
>  
>  * Improvements
> +  * Implemented decoding of BPF_MAP_LOOKUP_BATCH,
> +    BPF_MAP_LOOKUP_AND_DELETE_BATCH, BPF_MAP_UPDATE_BATCH, and
> +    BPF_MAP_DELETE_BATCH bpf syscall commands.
>    * Enhanced decoding of BPF_MAP_CREATE and BPF_PROG_ATTACH bpf syscall
>      commands.
>    * Updated lists of BPF_* constants.
> diff --git a/bpf.c b/bpf.c
> index b2335d69..bcb70d8a 100644
> --- a/bpf.c
> +++ b/bpf.c
> @@ -949,6 +949,25 @@ BEGIN_BPF_CMD_DECODER(BPF_TASK_FD_QUERY)
>  }
>  END_BPF_CMD_DECODER(RVAL_DECODED)
>  
> +BEGIN_BPF_CMD_DECODER(BPF_MAP_LOOKUP_BATCH)
> +{
> +	PRINT_FIELD_ADDR64("{", attr, in_batch);
> +	PRINT_FIELD_ADDR64(", ", attr, out_batch);

'out_batch' is a field that's written by the kernel on exiting syscall,
printing it on entering would probably be confusing.

> +	PRINT_FIELD_ADDR64(", ", attr, keys);
> +	PRINT_FIELD_ADDR64(", ", attr, values);
> +	PRINT_FIELD_U(", ", attr, count);

'count' is an input/output field, maybe it would make sense to print
the value written by kernel as well.

The corresponding Linux kernel commit v5.6-rc1~151^2~46^2~23^2~7 claims:

"'count' will contain the
number of keys/values successfully retrieved. Note that 'count' is an
input/output variable and it can contain a lower value after a call.

If there's no more entries to retrieve, ENOENT will be returned. If error
is ENOENT, count might be > 0 in case it copied some values but there were
no more entries to retrieve.

Note that if the return code is an error and not -EFAULT,
count indicates the number of elements successfully processed."

> +	PRINT_FIELD_FD(", ", attr, map_fd, tcp);
> +	PRINT_FIELD_FLAGS(", ", attr, elem_flags, bpf_map_lookup_elem_flags,
> +			  "BPF_???");
> +	PRINT_FIELD_U64(", ", attr, flags);

I'm not sure what's the semantics of this "flags" field, but
I don't think PRINT_FIELD_U64 is the most appropriate way to print it,
probably PRINT_FIELD_X would do better.

> +}
> +END_BPF_CMD_DECODER(RVAL_DECODED)
> +
> +#define decode_BPF_MAP_LOOKUP_AND_DELETE_BATCH decode_BPF_MAP_LOOKUP_BATCH
> +#define decode_BPF_MAP_UPDATE_BATCH decode_BPF_MAP_LOOKUP_BATCH
> +#define decode_BPF_MAP_DELETE_BATCH decode_BPF_MAP_LOOKUP_BATCH
> +
>  SYS_FUNC(bpf)
>  {
>  	static const bpf_cmd_decoder_t bpf_cmd_decoders[] = {
> @@ -976,6 +995,10 @@ SYS_FUNC(bpf)
>  		BPF_CMD_ENTRY(BPF_MAP_LOOKUP_AND_DELETE_ELEM),
>  		BPF_CMD_ENTRY(BPF_MAP_FREEZE),
>  		BPF_CMD_ENTRY(BPF_BTF_GET_NEXT_ID),
> +		BPF_CMD_ENTRY(BPF_MAP_LOOKUP_BATCH),
> +		BPF_CMD_ENTRY(BPF_MAP_LOOKUP_AND_DELETE_BATCH),
> +		BPF_CMD_ENTRY(BPF_MAP_UPDATE_BATCH),
> +		BPF_CMD_ENTRY(BPF_MAP_DELETE_BATCH),
>  	};
>  
>  	const unsigned int cmd = tcp->u_arg[0];
> diff --git a/bpf_attr.h b/bpf_attr.h
> index e4826ec0..db5cb6c9 100644
> --- a/bpf_attr.h
> +++ b/bpf_attr.h
> @@ -360,4 +360,28 @@ struct bpf_prog_info_struct {
>  	sizeof(struct bpf_prog_info_struct)
>  # define expected_bpf_prog_info_struct_size 208
>  
> +struct BPF_MAP_LOOKUP_BATCH_struct /* batch */ {
> +	uint64_t ATTRIBUTE_ALIGNED(8) in_batch;
> +	uint64_t ATTRIBUTE_ALIGNED(8) out_batch;
> +	uint64_t ATTRIBUTE_ALIGNED(8) keys;
> +	uint64_t ATTRIBUTE_ALIGNED(8) values;
> +	uint32_t count;
> +	uint32_t map_fd;
> +	uint64_t elem_flags;
> +	uint64_t flags;
> +};

There is a comment in the beginning of bpf_attr.h file:

/*
 * The policy is that all fields of type uint64_t in this header file
 * must have ATTRIBUTE_ALIGNED(8).
 *
 * This should not cause any contradictions with <linux/bpf.h>
 * unless the latter is buggy.
 *
 * By word "buggy" I mean containing such changes as Linux kernel commit
 * v4.16-rc1~123^2~109^2~5^2~4.
 */


-- 
ldv