lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Tue, 18 Sep 2018 23:36:04 -0500
From:   Mauricio Vasquez <mauricio.vasquez@...ito.it>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
        Yonghong Song <yhs@...com>
Subject: Re: [RFC PATCH bpf-next v3 7/7] selftests/bpf: add test cases for
 queue and stack maps



On 09/18/2018 06:32 PM, Alexei Starovoitov wrote:
> On Tue, Sep 18, 2018 at 06:53:07AM +0200, Mauricio Vasquez B wrote:
>> Two types of tests are done:
>> - test_maps: only userspace api.
>> - test_progs: userspace api and ebpf helpers.
>>
>> Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@...ito.it>
>> ---
>>   kernel/bpf/helpers.c                               |    2
>>   tools/lib/bpf/bpf.c                                |   12 ++
>>   tools/lib/bpf/bpf.h                                |    1
>>   tools/testing/selftests/bpf/Makefile               |    5 +
>>   tools/testing/selftests/bpf/bpf_helpers.h          |    7 +
>>   tools/testing/selftests/bpf/test_maps.c            |  130 ++++++++++++++++++++
>>   tools/testing/selftests/bpf/test_progs.c           |   99 +++++++++++++++
>>   tools/testing/selftests/bpf/test_queue_map.c       |    4 +
>>   tools/testing/selftests/bpf/test_queue_stack_map.h |   59 +++++++++
>>   tools/testing/selftests/bpf/test_stack_map.c       |    4 +
>>   10 files changed, 321 insertions(+), 2 deletions(-)
>>   create mode 100644 tools/testing/selftests/bpf/test_queue_map.c
>>   create mode 100644 tools/testing/selftests/bpf/test_queue_stack_map.h
>>   create mode 100644 tools/testing/selftests/bpf/test_stack_map.c
>>
>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>> index 5f364e6acaf1..1293cd5240e3 100644
>> --- a/kernel/bpf/helpers.c
>> +++ b/kernel/bpf/helpers.c
>> @@ -76,7 +76,7 @@ const struct bpf_func_proto bpf_map_delete_elem_proto = {
>>   	.arg2_type	= ARG_PTR_TO_MAP_KEY,
>>   };
>>   
>> -BPF_CALL_4(bpf_map_push_elem, struct bpf_map *, map, void *, value, u32 size,
>> +BPF_CALL_4(bpf_map_push_elem, struct bpf_map *, map, void *, value, u32, size,
>>   	   u64, flags)
> part of earlier patch?

Yes, this is wrong on patch 4 and somehow I managed to include the fix 
into this one. Will move to right patch in next version.

>
>>   {
>>   	if (map->value_size != size)
>> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
>> index 60aa4ca8b2c5..7056b2eb554d 100644
>> --- a/tools/lib/bpf/bpf.c
>> +++ b/tools/lib/bpf/bpf.c
>> @@ -286,6 +286,18 @@ int bpf_map_lookup_elem(int fd, const void *key, void *value)
>>   	return sys_bpf(BPF_MAP_LOOKUP_ELEM, &attr, sizeof(attr));
>>   }
>>   
>> +int bpf_map_lookup_and_delete_elem(int fd, const void *key, const void *value)
>> +{
>> +	union bpf_attr attr;
>> +
>> +	bzero(&attr, sizeof(attr));
>> +	attr.map_fd = fd;
>> +	attr.key = ptr_to_u64(key);
>> +	attr.value = ptr_to_u64(value);
>> +
>> +	return sys_bpf(BPF_MAP_LOOKUP_AND_DELETE_ELEM, &attr, sizeof(attr));
>> +}
>> +
>>   int bpf_map_delete_elem(int fd, const void *key)
>>   {
>>   	union bpf_attr attr;
>> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
>> index 6f38164b2618..6134ed9517d3 100644
>> --- a/tools/lib/bpf/bpf.h
>> +++ b/tools/lib/bpf/bpf.h
>> @@ -86,6 +86,7 @@ int bpf_map_update_elem(int fd, const void *key, const void *value,
>>   			__u64 flags);
>>   
>>   int bpf_map_lookup_elem(int fd, const void *key, void *value);
>> +int bpf_map_lookup_and_delete_elem(int fd, const void *key, const void *value);
>>   int bpf_map_delete_elem(int fd, const void *key);
>>   int bpf_map_get_next_key(int fd, const void *key, void *next_key);
>>   int bpf_obj_pin(int fd, const char *pathname);
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index fff7fb1285fc..ad8a2b8fb738 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -35,7 +35,7 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
>>   	test_get_stack_rawtp.o test_sockmap_kern.o test_sockhash_kern.o \
>>   	test_lwt_seg6local.o sendmsg4_prog.o sendmsg6_prog.o test_lirc_mode2_kern.o \
>>   	get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o \
>> -	test_skb_cgroup_id_kern.o
>> +	test_skb_cgroup_id_kern.o test_queue_map.o test_stack_map.o
>>   
>>   # Order correspond to 'make run_tests' order
>>   TEST_PROGS := test_kmod.sh \
>> @@ -110,6 +110,9 @@ CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
>>   $(OUTPUT)/test_l4lb_noinline.o: CLANG_FLAGS += -fno-inline
>>   $(OUTPUT)/test_xdp_noinline.o: CLANG_FLAGS += -fno-inline
>>   
>> +$(OUTPUT)/test_queue_map.o: test_queue_stack_map.h
>> +$(OUTPUT)/test_stack_map.o: test_queue_stack_map.h
>> +
>>   BTF_LLC_PROBE := $(shell $(LLC) -march=bpf -mattr=help 2>&1 | grep dwarfris)
>>   BTF_PAHOLE_PROBE := $(shell $(BTF_PAHOLE) --help 2>&1 | grep BTF)
>>   BTF_OBJCOPY_PROBE := $(shell $(LLVM_OBJCOPY) --help 2>&1 | grep -i 'usage.*llvm')
>> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
>> index e4be7730222d..bdbe8f84023e 100644
>> --- a/tools/testing/selftests/bpf/bpf_helpers.h
>> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
>> @@ -16,6 +16,13 @@ static int (*bpf_map_update_elem)(void *map, void *key, void *value,
>>   	(void *) BPF_FUNC_map_update_elem;
>>   static int (*bpf_map_delete_elem)(void *map, void *key) =
>>   	(void *) BPF_FUNC_map_delete_elem;
>> +static int (*bpf_map_push_elem)(void *map, const void *value, int len,
>> +				unsigned long long flags) =
>> +	(void *) BPF_FUNC_map_push_elem;
>> +static int (*bpf_map_pop_elem)(void *map, void *value, int len) =
>> +	(void *) BPF_FUNC_map_pop_elem;
>> +static int (*bpf_map_peek_elem)(void *map, void *value, int len) =
>> +	(void *) BPF_FUNC_map_peek_elem;
>>   static int (*bpf_probe_read)(void *dst, int size, void *unsafe_ptr) =
>>   	(void *) BPF_FUNC_probe_read;
>>   static unsigned long long (*bpf_ktime_get_ns)(void) =
>> diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
>> index 6f54f84144a0..abb21ea731fa 100644
>> --- a/tools/testing/selftests/bpf/test_maps.c
>> +++ b/tools/testing/selftests/bpf/test_maps.c
>> @@ -15,6 +15,7 @@
>>   #include <string.h>
>>   #include <assert.h>
>>   #include <stdlib.h>
>> +#include <time.h>
>>   
>>   #include <sys/wait.h>
>>   #include <sys/socket.h>
>> @@ -471,6 +472,130 @@ static void test_devmap(int task, void *data)
>>   	close(fd);
>>   }
>>   
>> +static void test_queuemap(int task, void *data)
>> +{
>> +	const int MAP_SIZE = 32;
>> +	__u32 vals[MAP_SIZE + MAP_SIZE/2], val;
>> +	int fd, i;
>> +
>> +	/* Fill test values to be used */
>> +	for (i = 0; i < MAP_SIZE + MAP_SIZE/2; i++)
>> +		vals[i] = rand();
>> +
>> +	/* Invalid key size */
>> +	fd = bpf_create_map(BPF_MAP_TYPE_QUEUE, 4, sizeof(val), MAP_SIZE,
>> +			    map_flags);
>> +	assert(fd < 0 && errno == EINVAL);
>> +
>> +	/* Invalid value size */
>> +	fd = bpf_create_map(BPF_MAP_TYPE_QUEUE, 0, 3, MAP_SIZE,map_flags);
>> +	assert(fd < 0 && errno == EINVAL);
>> +
>> +	fd = bpf_create_map(BPF_MAP_TYPE_QUEUE, 0, sizeof(val), MAP_SIZE,
>> +			    map_flags);
>> +	/* Queue map does not support BPF_F_NO_PREALLOC */
>> +	if (map_flags & BPF_F_NO_PREALLOC) {
>> +		assert(fd < 0 && errno == EINVAL);
>> +		return;
>> +	}
>> +	if (fd < 0) {
>> +		printf("Failed to create queuemap '%s'!\n", strerror(errno));
>> +		exit(1);
>> +	}
>> +
>> +	/* Push MAP_SIZE elements */
>> +	for (i = 0; i < MAP_SIZE; i++)
>> +		assert(bpf_map_update_elem(fd, NULL, &vals[i], 0) == 0);
>> +
>> +	/* Check that element cannot be pushed due to max_entries limit */
>> +	assert(bpf_map_update_elem(fd, NULL, &val, 0) == -1 &&
>> +	       errno == E2BIG);
>> +
>> +	/* Peek element */
>> +	assert(bpf_map_lookup_elem(fd, NULL, &val) == 0 && val == vals[0]);
>> +
>> +	/* Replace half elements */
>> +	for (i = MAP_SIZE; i < MAP_SIZE + MAP_SIZE/2; i++)
>> +		assert(bpf_map_update_elem(fd, NULL, &vals[i], BPF_EXIST) == 0);
>> +
>> +	/* Pop all elements */
>> +	for (i = MAP_SIZE/2; i < MAP_SIZE + MAP_SIZE/2; i++)
>> +		assert(bpf_map_lookup_and_delete_elem(fd, NULL, &val) == 0 &&
>> +		       val == vals[i]);
>> +
>> +	/* Check that there are not elements left */
>> +	assert(bpf_map_lookup_and_delete_elem(fd, NULL, &val) == -1 &&
>> +	       errno == ENOENT);
>> +
>> +	/* Check that non supported functions set errno to EINVAL */
>> +	assert(bpf_map_delete_elem(fd, NULL) == -1 && errno == EINVAL);
>> +	assert(bpf_map_get_next_key(fd, NULL, NULL) == -1 && errno == EINVAL);
>> +
>> +	close(fd);
>> +}
>> +
>> +static void test_stackmap(int task, void *data)
>> +{
>> +	const int MAP_SIZE = 32;
>> +	__u32 vals[MAP_SIZE + MAP_SIZE/2], val;
>> +	int fd, i;
>> +
>> +	/* Fill test values to be used */
>> +	for (i = 0; i < MAP_SIZE + MAP_SIZE/2; i++)
>> +		vals[i] = rand();
>> +
>> +	/* Invalid key size */
>> +	fd = bpf_create_map(BPF_MAP_TYPE_STACK, 4, sizeof(val), MAP_SIZE,
>> +			    map_flags);
>> +	assert(fd < 0 && errno == EINVAL);
>> +
>> +	/* Invalid value size */
>> +	fd = bpf_create_map(BPF_MAP_TYPE_STACK, 0, 3, MAP_SIZE,map_flags);
>> +	assert(fd < 0 && errno == EINVAL);
>> +
>> +	fd = bpf_create_map(BPF_MAP_TYPE_STACK, 0, sizeof(val), MAP_SIZE,
>> +			    map_flags);
>> +	/* Stack map does not support BPF_F_NO_PREALLOC */
>> +	if (map_flags & BPF_F_NO_PREALLOC) {
>> +		assert(fd < 0 && errno == EINVAL);
>> +		return;
>> +	}
>> +	if (fd < 0) {
>> +		printf("Failed to create stackmap '%s'!\n", strerror(errno));
>> +		exit(1);
>> +	}
>> +
>> +	/* Push MAP_SIZE elements */
>> +	for (i = 0; i < MAP_SIZE; i++)
>> +		assert(bpf_map_update_elem(fd, NULL, &vals[i], 0) == 0);
>> +
>> +	/* Check that element cannot be pushed due to max_entries limit */
>> +	assert(bpf_map_update_elem(fd, NULL, &val, 0) == -1 &&
>> +	       errno == E2BIG);
>> +
>> +	/* Peek element */
>> +	assert(bpf_map_lookup_elem(fd, NULL, &val) == 0 && val == vals[i - 1]);
>> +
>> +	/* Replace half elements */
>> +	for (i = MAP_SIZE; i < MAP_SIZE + MAP_SIZE/2; i++)
>> +		assert(bpf_map_update_elem(fd, NULL, &vals[i], BPF_EXIST) == 0);
>> +
>> +	/* Pop all elements */
>> +	for (i = MAP_SIZE + MAP_SIZE/2 - 1; i >= MAP_SIZE/2; i--)
>> +		assert(bpf_map_lookup_and_delete_elem(fd, NULL, &val) == 0 &&
>> +		       val == vals[i]);
>> +
>> +	/* Check that there are not elements left */
>> +	assert(bpf_map_lookup_and_delete_elem(fd, NULL, &val) == -1 &&
>> +	       errno == ENOENT);
>> +
>> +	/* Check that non supported functions set errno to EINVAL */
>> +	assert(bpf_map_delete_elem(fd, NULL) == -1 && errno == EINVAL);
>> +	assert(bpf_map_get_next_key(fd, NULL, NULL) == -1 && errno == EINVAL);
>> +
>> +	close(fd);
>> +}
>> +
>>   #include <sys/socket.h>
>>   #include <sys/ioctl.h>
>>   #include <arpa/inet.h>
>> @@ -1430,10 +1555,15 @@ static void run_all_tests(void)
>>   	test_map_wronly();
>>   
>>   	test_reuseport_array();
>> +
>> +	test_queuemap(0, NULL);
>> +	test_stackmap(0, NULL);
>>   }
>>   
>>   int main(void)
>>   {
>> +	srand(time(NULL));
>> +
>>   	map_flags = 0;
>>   	run_all_tests();
>>   
>> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
>> index 63a671803ed6..a69b9e1e668d 100644
>> --- a/tools/testing/selftests/bpf/test_progs.c
>> +++ b/tools/testing/selftests/bpf/test_progs.c
>> @@ -1698,8 +1698,105 @@ static void test_task_fd_query_tp(void)
>>   				   "sys_enter_read");
>>   }
>>   
>> +enum {
>> +	QUEUE,
>> +	STACK,
>> +};
>> +
>> +static void test_queue_stack_map(int type)
>> +{
>> +	const int MAP_SIZE = 32;
>> +	__u32 vals[MAP_SIZE], duration, retval, size, val;
>> +	int i, err, prog_fd, map_in_fd, map_out_fd;
>> +	char file[32], buf[128];
>> +	struct bpf_object *obj;
>> +	struct iphdr *iph = (void *)buf + sizeof(struct ethhdr);
>> +
>> +	/* Fill test values to be used */
>> +	for (i = 0; i < MAP_SIZE; i++)
>> +		vals[i] = rand();
>> +
>> +	if (type == QUEUE)
>> +		strncpy(file, "./test_queue_map.o", sizeof(file));
>> +	else if (type == STACK)
>> +		strncpy(file, "./test_stack_map.o", sizeof(file));
>> +	else
>> +		return;
>> +
>> +	err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
>> +	if (err) {
>> +		error_cnt++;
>> +		return;
>> +	}
>> +
>> +	map_in_fd = bpf_find_map(__func__, obj, "map_in");
>> +	if (map_in_fd < 0)
>> +		goto out;
>> +
>> +	map_out_fd = bpf_find_map(__func__, obj, "map_out");
>> +	if (map_out_fd < 0)
>> +		goto out;
>> +
>> +	/* Push 32 elements to the input map */
>> +	for (i = 0; i < MAP_SIZE; i++) {
>> +		err = bpf_map_update_elem(map_in_fd, NULL, &vals[i], 0);
>> +		if (err) {
>> +			error_cnt++;
>> +			goto out;
>> +		}
>> +	}
>> +
>> +	/* The eBPF program pushes iph.saddr in the output map,
>> +	 * pops the input map and saves this value in iph.daddr
>> +	 */
>> +	for (i = 0; i < MAP_SIZE; i++) {
>> +		if (type == QUEUE) {
>> +			val = vals[i];
>> +			pkt_v4.iph.saddr = vals[i] * 5;
>> +		} else if (type == STACK) {
>> +			val = vals[MAP_SIZE - 1 - i];
>> +			pkt_v4.iph.saddr = vals[MAP_SIZE - 1 - i] * 5;
>> +		}
>> +
>> +		err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
>> +					buf, &size, &retval, &duration);
>> +		if (err || retval || size != sizeof(pkt_v4) ||
>> +		    iph->daddr != val)
>> +			break;
>> +	}
>> +
>> +	CHECK(err || retval || size != sizeof(pkt_v4) || iph->daddr != val,
>> +	      "bpf_map_pop_elem",
>> +	      "err %d errno %d retval %d size %d iph->daddr %u\n",
>> +	      err, errno, retval, size, iph->daddr);
>> +
>> +	/* Queue is empty, program should return TC_ACT_SHOT */
>> +	err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
>> +				buf, &size, &retval, &duration);
>> +	CHECK(err || retval != 2 /* TC_ACT_SHOT */|| size != sizeof(pkt_v4),
>> +	      "check-queue-stack-map-empty",
>> +	      "err %d errno %d retval %d size %d\n",
>> +	      err, errno, retval, size);
>> +
>> +	/* Check that the program pushed elements correctly */
>> +	for (i = 0; i < MAP_SIZE; i++) {
>> +		err = bpf_map_lookup_and_delete_elem(map_out_fd, NULL, &val);
>> +		if (err || val != vals[i] * 5)
>> +			break;
>> +	}
>> +
>> +	CHECK(i != MAP_SIZE && (err || val != vals[i] * 5),
>> +	      "bpf_map_push_elem", "err %d value %u\n", err, val);
>> +
>> +out:
>> +	pkt_v4.iph.saddr = 0;
>> +	bpf_object__close(obj);
>> +}
>> +
>>   int main(void)
>>   {
>> +	srand(time(NULL));
>> +
>>   	jit_enabled = is_jit_enabled();
>>   
>>   	test_pkt_access();
>> @@ -1719,6 +1816,8 @@ int main(void)
>>   	test_get_stack_raw_tp();
>>   	test_task_fd_query_rawtp();
>>   	test_task_fd_query_tp();
>> +	test_queue_stack_map(QUEUE);
>> +	test_queue_stack_map(STACK);
>>   
>>   	printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
>>   	return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
>> diff --git a/tools/testing/selftests/bpf/test_queue_map.c b/tools/testing/selftests/bpf/test_queue_map.c
>> new file mode 100644
>> index 000000000000..3fdb3b9cb038
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/test_queue_map.c
>> @@ -0,0 +1,4 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// Copyright (c) 2017 Politecnico di Torino
>> +#define MAP_TYPE BPF_MAP_TYPE_QUEUE
>> +#include "test_queue_stack_map.h"
>> diff --git a/tools/testing/selftests/bpf/test_queue_stack_map.h b/tools/testing/selftests/bpf/test_queue_stack_map.h
>> new file mode 100644
>> index 000000000000..23a5b5d60069
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/test_queue_stack_map.h
>> @@ -0,0 +1,59 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +// Copyright (c) 2017 Politecnico di Torino
>> +#include <stddef.h>
>> +#include <string.h>
>> +#include <linux/bpf.h>
>> +#include <linux/if_ether.h>
>> +#include <linux/ip.h>
>> +#include <linux/pkt_cls.h>
>> +#include "bpf_helpers.h"
>> +
>> +int _version SEC("version") = 1;
>> +
>> +struct bpf_map_def __attribute__ ((section("maps"), used)) map_in = {
>> +	.type = MAP_TYPE,
>> +	.key_size = 0,
>> +	.value_size = sizeof(__u32),
>> +	.max_entries = 32,
>> +	.map_flags = 0,
>> +};
>> +
>> +struct bpf_map_def __attribute__ ((section("maps"), used)) map_out = {
>> +	.type = MAP_TYPE,
>> +	.key_size = 0,
>> +	.value_size = sizeof(__u32),
>> +	.max_entries = 32,
>> +	.map_flags = 0,
>> +};
>> +
>> +SEC("test")
>> +int _test(struct __sk_buff *skb)
>> +{
>> +	void *data_end = (void *)(long)skb->data_end;
>> +	void *data = (void *)(long)skb->data;
>> +	struct ethhdr *eth = (struct ethhdr *)(data);
>> +	__u32 value;
>> +	int err;
>> +
>> +	if (eth + 1 > data_end)
>> +		return TC_ACT_SHOT;
>> +
>> +	struct iphdr *iph = (struct iphdr *)(eth + 1);
>> +
>> +	if (iph + 1 > data_end)
>> +		return TC_ACT_SHOT;
>> +
>> +	err = bpf_map_pop_elem(&map_in, &value, sizeof(value));
>> +	if (err)
>> +		return TC_ACT_SHOT;
>> +
>> +	iph->daddr = value;
>> +
>> +	err = bpf_map_push_elem(&map_out, &iph->saddr, sizeof(iph->saddr), 0);
>> +	if (err)
>> +		return TC_ACT_SHOT;
> is it possible to add a test for 'peek' helper in reliable way?
> it seems it will alwasy be racy. may be we shouldn't implement peek at all?
>
With the per-value implementation it should not be racy.   Why do you 
say it is seems to be racy?
A very silly test could be to always do peek before pop and check both 
values are the same.

Powered by blists - more mailing lists