Re: [PATCH bpf-next v2] selftests/bpf: test access to ringbuf position with map pointer

[Leon Hwang]

On 23/3/26 10:18, Menglong Dong wrote:
> Add the testing to access the bpf_ringbuf with the map pointer.
> "consumer_pos" and "producer_pos" is accessed in this testing. We reserve
> 128 bytes in the ringbuf to test the producer_pos, which should be
> "128 + 8", and the "8" is BPF_RINGBUF_HDR_SZ.
> 
> It will be helpful if we want to evaluate the usage of the ringbuf in bpf
> prog with the consumer and producer position.
> 

128 is a plain test for ringbuf, like those in
ringbuf_overwrite_mode_subtest().

The reserved size can be 0; however, a test for it is missing. It would
be better to add a test against 0-size here or there.

> Signed-off-by: Menglong Dong <dongml2@xxxxxxxxxxxxxxx>
> Reviewed-by: Emil Tsalapatis <emil@xxxxxxxxxxxxxxx>

Acked-by: Leon Hwang <leon.hwang@xxxxxxxxx>

One nit below.

> ---
> v2:
> - don't set the max_entries for the ringbuf map
> - add comment for the producer_pos
> ---
>  .../testing/selftests/bpf/progs/map_ptr_kern.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/progs/map_ptr_kern.c b/tools/testing/selftests/bpf/progs/map_ptr_kern.c
> index efaf622c28dd..d7611e7018ca 100644
> --- a/tools/testing/selftests/bpf/progs/map_ptr_kern.c
> +++ b/tools/testing/selftests/bpf/progs/map_ptr_kern.c
> @@ -647,8 +647,14 @@ static inline int check_devmap_hash(void)
>  	return 1;
>  }
>  
> +struct bpf_ringbuf {
> +	unsigned long consumer_pos;
> +	unsigned long producer_pos;
> +} __attribute__((preserve_access_index));
> +
>  struct bpf_ringbuf_map {
>  	struct bpf_map map;
> +	struct bpf_ringbuf *rb;
>  } __attribute__((preserve_access_index));
>  
>  struct {
> @@ -659,9 +665,21 @@ static inline int check_ringbuf(void)
>  {
>  	struct bpf_ringbuf_map *ringbuf = (struct bpf_ringbuf_map *)&m_ringbuf;
>  	struct bpf_map *map = (struct bpf_map *)&m_ringbuf;
> +	struct bpf_ringbuf *rb;
> +	void *ptr;
>  
>  	VERIFY(check(&ringbuf->map, map, 0, 0, page_size));
>  
> +	ptr = bpf_ringbuf_reserve(&m_ringbuf, 128, 0);
> +	VERIFY(ptr);
> +
> +	bpf_ringbuf_discard(ptr, 0);
> +	rb = ringbuf->rb;
> +	VERIFY(rb);
> +	VERIFY(rb->consumer_pos == 0);
> +	/* The "8" here is BPF_RINGBUF_HDR_SZ */
> +	VERIFY(rb->producer_pos == 128 + 8);
> +

NIT: Use BPF_RINGBUF_HDR_SZ directly. Is it missing in vmlinux.h?

Thanks,
Leon

>  	return 1;
>  }
>