diff mbox series

[bpf] bpf: selftest fix for sockmap

Message ID 20180611184735.31255.51105.stgit@john-Precision-Tower-5810
State Changes Requested, archived
Delegated to: BPF Maintainers
Headers show
Series [bpf] bpf: selftest fix for sockmap | expand

Commit Message

John Fastabend June 11, 2018, 6:47 p.m. UTC
In selftest test_maps the sockmap test case attempts to add a socket
in listening state to the sockmap. This is no longer a valid operation
so it fails as expected. However, the test wrongly reports this as an
error now. Fix the test to avoid adding sockets in listening state.

Fixes: 945ae430aa44 ("bpf: sockmap only allow ESTABLISHED sock state")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 tools/testing/selftests/bpf/test_maps.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Y Song June 11, 2018, 11:28 p.m. UTC | #1
On Mon, Jun 11, 2018 at 11:47 AM, John Fastabend
<john.fastabend@gmail.com> wrote:
> In selftest test_maps the sockmap test case attempts to add a socket
> in listening state to the sockmap. This is no longer a valid operation
> so it fails as expected. However, the test wrongly reports this as an
> error now. Fix the test to avoid adding sockets in listening state.
>
> Fixes: 945ae430aa44 ("bpf: sockmap only allow ESTABLISHED sock state")

I cannot reproduce the failure and I cannot find the above "Fixes" commit.
I checked latest bpf/bpf-next/net-next trees. Maybe I missed something?

> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
>  tools/testing/selftests/bpf/test_maps.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
> index 6c25334..9fed5f0 100644
> --- a/tools/testing/selftests/bpf/test_maps.c
> +++ b/tools/testing/selftests/bpf/test_maps.c
> @@ -564,7 +564,7 @@ static void test_sockmap(int tasks, void *data)
>         }
>
>         /* Test update without programs */
> -       for (i = 0; i < 6; i++) {
> +       for (i = 2; i < 6; i++) {
>                 err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_ANY);
>                 if (err) {
>                         printf("Failed noprog update sockmap '%i:%i'\n",
> @@ -727,7 +727,7 @@ static void test_sockmap(int tasks, void *data)
>         }
>
>         /* Test map update elem afterwards fd lives in fd and map_fd */
> -       for (i = 0; i < 6; i++) {
> +       for (i = 2; i < 6; i++) {
>                 err = bpf_map_update_elem(map_fd_rx, &i, &sfd[i], BPF_ANY);
>                 if (err) {
>                         printf("Failed map_fd_rx update sockmap %i '%i:%i'\n",
>
Daniel Borkmann June 13, 2018, 12:31 a.m. UTC | #2
On 06/11/2018 08:47 PM, John Fastabend wrote:
> In selftest test_maps the sockmap test case attempts to add a socket
> in listening state to the sockmap. This is no longer a valid operation
> so it fails as expected. However, the test wrongly reports this as an
> error now. Fix the test to avoid adding sockets in listening state.
> 
> Fixes: 945ae430aa44 ("bpf: sockmap only allow ESTABLISHED sock state")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>

(fyi, discussed with John that this will be enrolled into the set of
 fixes he has pending for bpf since the test is related to the one
 restricting to ESTABLISHED state.)
John Fastabend June 13, 2018, 5:52 p.m. UTC | #3
On 06/12/2018 05:31 PM, Daniel Borkmann wrote:
> On 06/11/2018 08:47 PM, John Fastabend wrote:
>> In selftest test_maps the sockmap test case attempts to add a socket
>> in listening state to the sockmap. This is no longer a valid operation
>> so it fails as expected. However, the test wrongly reports this as an
>> error now. Fix the test to avoid adding sockets in listening state.
>>
>> Fixes: 945ae430aa44 ("bpf: sockmap only allow ESTABLISHED sock state")
>> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> 
> (fyi, discussed with John that this will be enrolled into the set of
>  fixes he has pending for bpf since the test is related to the one
>  restricting to ESTABLISHED state.)
> 

Patch part of this series now,

https://patchwork.ozlabs.org/project/netdev/list/?series=49988
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 6c25334..9fed5f0 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -564,7 +564,7 @@  static void test_sockmap(int tasks, void *data)
 	}
 
 	/* Test update without programs */
-	for (i = 0; i < 6; i++) {
+	for (i = 2; i < 6; i++) {
 		err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_ANY);
 		if (err) {
 			printf("Failed noprog update sockmap '%i:%i'\n",
@@ -727,7 +727,7 @@  static void test_sockmap(int tasks, void *data)
 	}
 
 	/* Test map update elem afterwards fd lives in fd and map_fd */
-	for (i = 0; i < 6; i++) {
+	for (i = 2; i < 6; i++) {
 		err = bpf_map_update_elem(map_fd_rx, &i, &sfd[i], BPF_ANY);
 		if (err) {
 			printf("Failed map_fd_rx update sockmap %i '%i:%i'\n",