lkml.org 
[lkml]   [2020]   [Mar]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 5/5] bpf: sockmap, sockhash: test looking up fds
Date
On Tue, Mar 10, 2020 at 06:47 PM CET, Lorenz Bauer wrote:
> Make sure that looking up an element from the map succeeds,
> and that the fd is valid by using it an fcntl call.
>
> Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> ---
> .../selftests/bpf/prog_tests/sockmap_listen.c | 26 ++++++++++++++-----
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> index 52aa468bdccd..929e1e77ecc6 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> @@ -453,7 +453,7 @@ static void test_lookup_after_delete(int family, int sotype, int mapfd)
> xclose(s);
> }
>
> -static void test_lookup_32_bit_value(int family, int sotype, int mapfd)
> +static void test_lookup_fd(int family, int sotype, int mapfd)
> {
> u32 key, value32;
> int err, s;
> @@ -466,7 +466,7 @@ static void test_lookup_32_bit_value(int family, int sotype, int mapfd)
> sizeof(value32), 1, 0);
> if (mapfd < 0) {
> FAIL_ERRNO("map_create");
> - goto close;
> + goto close_sock;
> }
>
> key = 0;
> @@ -475,11 +475,25 @@ static void test_lookup_32_bit_value(int family, int sotype, int mapfd)
>
> errno = 0;
> err = bpf_map_lookup_elem(mapfd, &key, &value32);
> - if (!err || errno != ENOSPC)
> - FAIL_ERRNO("map_lookup: expected ENOSPC");
> + if (err) {
> + FAIL_ERRNO("map_lookup");
> + goto close_map;
> + }
>
> + if ((int)value32 == s) {
> + FAIL("return value is identical");
> + goto close;
> + }
> +
> + err = fcntl(value32, F_GETFD);
> + if (err == -1)
> + FAIL_ERRNO("fcntl");

I would call getsockopt()/getsockname() to assert that the FD lookup
succeeded. We want to know not only that it's an FD (-EBADFD case), but
also that it's associated with a socket (-ENOTSOCK).

We can go even further, and compare socket cookies to ensure we got an
FD for the expected socket.

Also, I'm wondering if we could keep the -ENOSPC case test-covered by
temporarily dropping NET_ADMIN capability.

> +
> +close:
> + xclose(value32);
> +close_map:
> xclose(mapfd);
> -close:
> +close_sock:
> xclose(s);
> }
>
> @@ -1456,7 +1470,7 @@ static void test_ops(struct test_sockmap_listen *skel, struct bpf_map *map,
> /* lookup */
> TEST(test_lookup_after_insert),
> TEST(test_lookup_after_delete),
> - TEST(test_lookup_32_bit_value),
> + TEST(test_lookup_fd),
> /* update */
> TEST(test_update_existing),
> /* races with insert/delete */

\
 
 \ /
  Last update: 2020-03-11 14:53    [W:0.049 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site