Re: [PATCH] bpf: Fix test_lru_sanity5() in test_lru_map.c

2017-01-17 Thread David Miller
From: Martin KaFai Lau 
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 
> Signed-off-by: Martin KaFai Lau 

Applied, thanks.


Re: [PATCH] bpf: Fix test_lru_sanity5() in test_lru_map.c

2017-01-17 Thread Alexei Starovoitov

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 
Signed-off-by: Martin KaFai Lau 


Looks good, thanks for fixing!

Acked-by: Daniel Borkmann 

(Patch is against -net tree.)


Acked-by: Alexei Starovoitov 



Re: [PATCH] bpf: Fix test_lru_sanity5() in test_lru_map.c

2017-01-17 Thread Daniel Borkmann

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 
Signed-off-by: Martin KaFai Lau 


Looks good, thanks for fixing!

Acked-by: Daniel Borkmann 

(Patch is against -net tree.)


[PATCH] bpf: Fix test_lru_sanity5() in test_lru_map.c

2017-01-16 Thread Martin KaFai Lau
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 
Signed-off-by: Martin KaFai Lau 
---
 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) !