* [PATCH] bpf: Fix test_lru_sanity5() in test_lru_map.c
@ 2017-01-17 6:17 Martin KaFai Lau
2017-01-17 9:07 ` Daniel Borkmann
2017-01-17 20:40 ` David Miller
0 siblings, 2 replies; 4+ messages in thread
From: Martin KaFai Lau @ 2017-01-17 6:17 UTC (permalink / raw)
To: netdev; +Cc: Daniel Borkmann, Alexei Starovoitov, Kernel Team
test_lru_sanity5() fails when the number of online cpus
is fewer than the number of possible cpus. It can be
reproduced with qemu by using cmd args "--smp cpus=2,maxcpus=8".
The problem is the loop in test_lru_sanity5() is testing
'i' which is incorrect.
This patch:
1. Make sched_next_online() always return -1 if it cannot
find a next cpu to schedule the process.
2. In test_lru_sanity5(), the parent process does
sched_setaffinity() first (through sched_next_online())
and the forked process will inherit it according to
the 'man sched_setaffinity'.
Fixes: 5db58faf989f ("bpf: Add tests for the LRU bpf_htab")
Reported-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
tools/testing/selftests/bpf/test_lru_map.c | 53 +++++++++++++++---------------
1 file changed, 27 insertions(+), 26 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_lru_map.c b/tools/testing/selftests/bpf/test_lru_map.c
index b13fed534d76..9f7bd1915c21 100644
--- a/tools/testing/selftests/bpf/test_lru_map.c
+++ b/tools/testing/selftests/bpf/test_lru_map.c
@@ -67,21 +67,23 @@ static int map_equal(int lru_map, int expected)
return map_subset(lru_map, expected) && map_subset(expected, lru_map);
}
-static int sched_next_online(int pid, int next_to_try)
+static int sched_next_online(int pid, int *next_to_try)
{
cpu_set_t cpuset;
+ int next = *next_to_try;
+ int ret = -1;
- if (next_to_try == nr_cpus)
- return -1;
-
- while (next_to_try < nr_cpus) {
+ while (next < nr_cpus) {
CPU_ZERO(&cpuset);
- CPU_SET(next_to_try++, &cpuset);
- if (!sched_setaffinity(pid, sizeof(cpuset), &cpuset))
+ CPU_SET(next++, &cpuset);
+ if (!sched_setaffinity(pid, sizeof(cpuset), &cpuset)) {
+ ret = 0;
break;
+ }
}
- return next_to_try;
+ *next_to_try = next;
+ return ret;
}
/* Size of the LRU amp is 2
@@ -96,11 +98,12 @@ static void test_lru_sanity0(int map_type, int map_flags)
{
unsigned long long key, value[nr_cpus];
int lru_map_fd, expected_map_fd;
+ int next_cpu = 0;
printf("%s (map_type:%d map_flags:0x%X): ", __func__, map_type,
map_flags);
- assert(sched_next_online(0, 0) != -1);
+ assert(sched_next_online(0, &next_cpu) != -1);
if (map_flags & BPF_F_NO_COMMON_LRU)
lru_map_fd = create_map(map_type, map_flags, 2 * nr_cpus);
@@ -183,6 +186,7 @@ static void test_lru_sanity1(int map_type, int map_flags, unsigned int tgt_free)
int lru_map_fd, expected_map_fd;
unsigned int batch_size;
unsigned int map_size;
+ int next_cpu = 0;
if (map_flags & BPF_F_NO_COMMON_LRU)
/* Ther percpu lru list (i.e each cpu has its own LRU
@@ -196,7 +200,7 @@ static void test_lru_sanity1(int map_type, int map_flags, unsigned int tgt_free)
printf("%s (map_type:%d map_flags:0x%X): ", __func__, map_type,
map_flags);
- assert(sched_next_online(0, 0) != -1);
+ assert(sched_next_online(0, &next_cpu) != -1);
batch_size = tgt_free / 2;
assert(batch_size * 2 == tgt_free);
@@ -262,6 +266,7 @@ static void test_lru_sanity2(int map_type, int map_flags, unsigned int tgt_free)
int lru_map_fd, expected_map_fd;
unsigned int batch_size;
unsigned int map_size;
+ int next_cpu = 0;
if (map_flags & BPF_F_NO_COMMON_LRU)
/* Ther percpu lru list (i.e each cpu has its own LRU
@@ -275,7 +280,7 @@ static void test_lru_sanity2(int map_type, int map_flags, unsigned int tgt_free)
printf("%s (map_type:%d map_flags:0x%X): ", __func__, map_type,
map_flags);
- assert(sched_next_online(0, 0) != -1);
+ assert(sched_next_online(0, &next_cpu) != -1);
batch_size = tgt_free / 2;
assert(batch_size * 2 == tgt_free);
@@ -370,11 +375,12 @@ static void test_lru_sanity3(int map_type, int map_flags, unsigned int tgt_free)
int lru_map_fd, expected_map_fd;
unsigned int batch_size;
unsigned int map_size;
+ int next_cpu = 0;
printf("%s (map_type:%d map_flags:0x%X): ", __func__, map_type,
map_flags);
- assert(sched_next_online(0, 0) != -1);
+ assert(sched_next_online(0, &next_cpu) != -1);
batch_size = tgt_free / 2;
assert(batch_size * 2 == tgt_free);
@@ -430,11 +436,12 @@ static void test_lru_sanity4(int map_type, int map_flags, unsigned int tgt_free)
int lru_map_fd, expected_map_fd;
unsigned long long key, value[nr_cpus];
unsigned long long end_key;
+ int next_cpu = 0;
printf("%s (map_type:%d map_flags:0x%X): ", __func__, map_type,
map_flags);
- assert(sched_next_online(0, 0) != -1);
+ assert(sched_next_online(0, &next_cpu) != -1);
if (map_flags & BPF_F_NO_COMMON_LRU)
lru_map_fd = create_map(map_type, map_flags,
@@ -502,9 +509,8 @@ static void do_test_lru_sanity5(unsigned long long last_key, int map_fd)
static void test_lru_sanity5(int map_type, int map_flags)
{
unsigned long long key, value[nr_cpus];
- int next_sched_cpu = 0;
+ int next_cpu = 0;
int map_fd;
- int i;
if (map_flags & BPF_F_NO_COMMON_LRU)
return;
@@ -519,27 +525,20 @@ static void test_lru_sanity5(int map_type, int map_flags)
key = 0;
assert(!bpf_map_update(map_fd, &key, value, BPF_NOEXIST));
- for (i = 0; i < nr_cpus; i++) {
+ while (sched_next_online(0, &next_cpu) != -1) {
pid_t pid;
pid = fork();
if (pid == 0) {
- next_sched_cpu = sched_next_online(0, next_sched_cpu);
- if (next_sched_cpu != -1)
- do_test_lru_sanity5(key, map_fd);
+ do_test_lru_sanity5(key, map_fd);
exit(0);
} else if (pid == -1) {
- printf("couldn't spawn #%d process\n", i);
+ printf("couldn't spawn process to test key:%llu\n",
+ key);
exit(1);
} else {
int status;
- /* It is mostly redundant and just allow the parent
- * process to update next_shced_cpu for the next child
- * process
- */
- next_sched_cpu = sched_next_online(pid, next_sched_cpu);
-
assert(waitpid(pid, &status, 0) == pid);
assert(status == 0);
key++;
@@ -547,6 +546,8 @@ static void test_lru_sanity5(int map_type, int map_flags)
}
close(map_fd);
+ /* At least one key should be tested */
+ assert(key > 0);
printf("Pass\n");
}
--
2.5.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] bpf: Fix test_lru_sanity5() in test_lru_map.c
2017-01-17 6:17 [PATCH] bpf: Fix test_lru_sanity5() in test_lru_map.c Martin KaFai Lau
@ 2017-01-17 9:07 ` Daniel Borkmann
2017-01-17 18:16 ` Alexei Starovoitov
2017-01-17 20:40 ` David Miller
1 sibling, 1 reply; 4+ messages in thread
From: Daniel Borkmann @ 2017-01-17 9:07 UTC (permalink / raw)
To: Martin KaFai Lau, netdev; +Cc: Alexei Starovoitov, Kernel Team
On 01/17/2017 07:17 AM, Martin KaFai Lau wrote:
> test_lru_sanity5() fails when the number of online cpus
> is fewer than the number of possible cpus. It can be
> reproduced with qemu by using cmd args "--smp cpus=2,maxcpus=8".
>
> The problem is the loop in test_lru_sanity5() is testing
> 'i' which is incorrect.
>
> This patch:
> 1. Make sched_next_online() always return -1 if it cannot
> find a next cpu to schedule the process.
> 2. In test_lru_sanity5(), the parent process does
> sched_setaffinity() first (through sched_next_online())
> and the forked process will inherit it according to
> the 'man sched_setaffinity'.
>
> Fixes: 5db58faf989f ("bpf: Add tests for the LRU bpf_htab")
> Reported-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Looks good, thanks for fixing!
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
(Patch is against -net tree.)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bpf: Fix test_lru_sanity5() in test_lru_map.c
2017-01-17 9:07 ` Daniel Borkmann
@ 2017-01-17 18:16 ` Alexei Starovoitov
0 siblings, 0 replies; 4+ messages in thread
From: Alexei Starovoitov @ 2017-01-17 18:16 UTC (permalink / raw)
To: Daniel Borkmann, Martin KaFai Lau, netdev; +Cc: Alexei Starovoitov, Kernel Team
On 1/17/17 1:07 AM, Daniel Borkmann wrote:
> On 01/17/2017 07:17 AM, Martin KaFai Lau wrote:
>> test_lru_sanity5() fails when the number of online cpus
>> is fewer than the number of possible cpus. It can be
>> reproduced with qemu by using cmd args "--smp cpus=2,maxcpus=8".
>>
>> The problem is the loop in test_lru_sanity5() is testing
>> 'i' which is incorrect.
>>
>> This patch:
>> 1. Make sched_next_online() always return -1 if it cannot
>> find a next cpu to schedule the process.
>> 2. In test_lru_sanity5(), the parent process does
>> sched_setaffinity() first (through sched_next_online())
>> and the forked process will inherit it according to
>> the 'man sched_setaffinity'.
>>
>> Fixes: 5db58faf989f ("bpf: Add tests for the LRU bpf_htab")
>> Reported-by: Daniel Borkmann <daniel@iogearbox.net>
>> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
>
> Looks good, thanks for fixing!
>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
>
> (Patch is against -net tree.)
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bpf: Fix test_lru_sanity5() in test_lru_map.c
2017-01-17 6:17 [PATCH] bpf: Fix test_lru_sanity5() in test_lru_map.c Martin KaFai Lau
2017-01-17 9:07 ` Daniel Borkmann
@ 2017-01-17 20:40 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2017-01-17 20:40 UTC (permalink / raw)
To: kafai; +Cc: netdev, daniel, alexei.starovoitov, kernel-team
From: Martin KaFai Lau <kafai@fb.com>
Date: Mon, 16 Jan 2017 22:17:29 -0800
> test_lru_sanity5() fails when the number of online cpus
> is fewer than the number of possible cpus. It can be
> reproduced with qemu by using cmd args "--smp cpus=2,maxcpus=8".
>
> The problem is the loop in test_lru_sanity5() is testing
> 'i' which is incorrect.
>
> This patch:
> 1. Make sched_next_online() always return -1 if it cannot
> find a next cpu to schedule the process.
> 2. In test_lru_sanity5(), the parent process does
> sched_setaffinity() first (through sched_next_online())
> and the forked process will inherit it according to
> the 'man sched_setaffinity'.
>
> Fixes: 5db58faf989f ("bpf: Add tests for the LRU bpf_htab")
> Reported-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-01-17 20:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-17 6:17 [PATCH] bpf: Fix test_lru_sanity5() in test_lru_map.c Martin KaFai Lau
2017-01-17 9:07 ` Daniel Borkmann
2017-01-17 18:16 ` Alexei Starovoitov
2017-01-17 20:40 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.