[v4,bpf-next,3/3] selftests/bpf: add selftest for get_netns_id helper
diff mbox series

Message ID 20200225044538.61889-4-forrest0579@gmail.com
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series
  • bpf: Add get_netns_id helper for sock_ops
Related show

Commit Message

Forrest Chen Feb. 25, 2020, 4:45 a.m. UTC
adding selftest for new bpf helper function get_netns_id

Signed-off-by: Lingpeng Chen <forrest0579@gmail.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
 .../selftests/bpf/progs/test_tcpbpf_kern.c    | 11 +++++
 .../testing/selftests/bpf/test_tcpbpf_user.c  | 46 ++++++++++++++++++-
 2 files changed, 56 insertions(+), 1 deletion(-)

Comments

Andrii Nakryiko Feb. 25, 2020, 6:13 a.m. UTC | #1
On Mon, Feb 24, 2020 at 8:47 PM Lingpeng Chen <forrest0579@gmail.com> wrote:
>
> adding selftest for new bpf helper function get_netns_id
>
> Signed-off-by: Lingpeng Chen <forrest0579@gmail.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---

It would be nice if this selftests becomes part of test_progs. That
way it would be exercised regularly, both by committers, as well as by
automated CI in libbpf's Github repo. Using global variables and BPF
skeleton would also clean up both BPF and user-space code.

It seems like this test runs Python script for server, but doesn't
seem like that server is doing anything complicated, so writing that
in C shouldn't be a problem as well. Thoughts?

>  .../selftests/bpf/progs/test_tcpbpf_kern.c    | 11 +++++
>  .../testing/selftests/bpf/test_tcpbpf_user.c  | 46 ++++++++++++++++++-
>  2 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c b/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
> index 1f1966e86e9f..d7d851ddd2cc 100644
> --- a/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
> +++ b/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
> @@ -28,6 +28,13 @@ struct {
>         __type(value, int);
>  } sockopt_results SEC(".maps");
>
> +struct {
> +       __uint(type, BPF_MAP_TYPE_ARRAY);
> +       __uint(max_entries, 1);
> +       __type(key, __u32);
> +       __type(value, __u64);
> +} netns_number SEC(".maps");
> +
>  static inline void update_event_map(int event)
>  {
>         __u32 key = 0;
> @@ -61,6 +68,7 @@ int bpf_testcb(struct bpf_sock_ops *skops)
>         int rv = -1;
>         int v = 0;
>         int op;
> +       __u64 netns_id;
>
>         op = (int) skops->op;
>
> @@ -144,6 +152,9 @@ int bpf_testcb(struct bpf_sock_ops *skops)
>                 __u32 key = 0;
>
>                 bpf_map_update_elem(&sockopt_results, &key, &v, BPF_ANY);
> +
> +               netns_id = bpf_get_netns_id(skops);
> +               bpf_map_update_elem(&netns_number, &key, &netns_id, BPF_ANY);
>                 break;
>         default:
>                 rv = -1;
> diff --git a/tools/testing/selftests/bpf/test_tcpbpf_user.c b/tools/testing/selftests/bpf/test_tcpbpf_user.c
> index 3ae127620463..fef2f4d77ecc 100644
> --- a/tools/testing/selftests/bpf/test_tcpbpf_user.c
> +++ b/tools/testing/selftests/bpf/test_tcpbpf_user.c
> @@ -76,6 +76,41 @@ int verify_sockopt_result(int sock_map_fd)
>         return ret;
>  }
>
> +int verify_netns(__u64 netns_id)
> +{
> +       char buf1[40];
> +       char buf2[40];
> +       int ret = 0;
> +       ssize_t len = 0;
> +
> +       len = readlink("/proc/self/ns/net", buf1, 39);
> +       sprintf(buf2, "net:[%llu]", netns_id);
> +
> +       if (len <= 0) {
> +               printf("FAILED: readlink /proc/self/ns/net");
> +               return ret;
> +       }
> +
> +       if (strncmp(buf1, buf2, len)) {
> +               printf("FAILED: netns don't match");
> +               ret = 1;
> +       }
> +       return ret;
> +}
> +
> +int verify_netns_result(int netns_map_fd)
> +{
> +       __u32 key = 0;
> +       __u64 res = 0;
> +       int ret = 0;
> +       int rv;
> +
> +       rv = bpf_map_lookup_elem(netns_map_fd, &key, &res);
> +       EXPECT_EQ(0, rv, "d");
> +
> +       return verify_netns(res);
> +}
> +
>  static int bpf_find_map(const char *test, struct bpf_object *obj,
>                         const char *name)
>  {
> @@ -92,7 +127,7 @@ static int bpf_find_map(const char *test, struct bpf_object *obj,
>  int main(int argc, char **argv)
>  {
>         const char *file = "test_tcpbpf_kern.o";
> -       int prog_fd, map_fd, sock_map_fd;
> +       int prog_fd, map_fd, sock_map_fd, netns_map_fd;
>         struct tcpbpf_globals g = {0};
>         const char *cg_path = "/foo";
>         int error = EXIT_FAILURE;
> @@ -137,6 +172,10 @@ int main(int argc, char **argv)
>         if (sock_map_fd < 0)
>                 goto err;
>
> +       netns_map_fd = bpf_find_map(__func__, obj, "netns_number");
> +       if (netns_map_fd < 0)
> +               goto err;
> +
>  retry_lookup:
>         rv = bpf_map_lookup_elem(map_fd, &key, &g);
>         if (rv != 0) {
> @@ -161,6 +200,11 @@ int main(int argc, char **argv)
>                 goto err;
>         }
>
> +       if (verify_netns_result(netns_map_fd)) {
> +               printf("FAILED: Wrong netns stats\n");
> +               goto err;
> +       }
> +
>         printf("PASSED!\n");
>         error = 0;
>  err:
> --
> 2.20.1
>
Andrii Nakryiko Feb. 25, 2020, 5:13 p.m. UTC | #2
On Mon, Feb 24, 2020 at 11:20 PM Forrest Chen <forrest0579@gmail.com> wrote:
>
> > It would be nice if this selftests becomes part of test_progs.
>
> You mean the whole tests of tcpbpf or only the changes I made in this test?
> If you mean the whole tests of tcpbpf, I think we could fire another thread
> to do this?

Yeah, I meant entire tcpbpf test.

>
> Andrii Nakryiko <andrii.nakryiko@gmail.com> 于2020年2月25日周二 下午2:13写道:
>>
>> On Mon, Feb 24, 2020 at 8:47 PM Lingpeng Chen <forrest0579@gmail.com> wrote:
>> >
>> > adding selftest for new bpf helper function get_netns_id
>> >
>> > Signed-off-by: Lingpeng Chen <forrest0579@gmail.com>
>> > Acked-by: Song Liu <songliubraving@fb.com>
>> > ---
>>
>> It would be nice if this selftests becomes part of test_progs. That
>> way it would be exercised regularly, both by committers, as well as by
>> automated CI in libbpf's Github repo. Using global variables and BPF
>> skeleton would also clean up both BPF and user-space code.
>>
>> It seems like this test runs Python script for server, but doesn't
>> seem like that server is doing anything complicated, so writing that
>> in C shouldn't be a problem as well. Thoughts?
>>
>> >  .../selftests/bpf/progs/test_tcpbpf_kern.c    | 11 +++++
>> >  .../testing/selftests/bpf/test_tcpbpf_user.c  | 46 ++++++++++++++++++-
>> >  2 files changed, 56 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c b/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
>> > index 1f1966e86e9f..d7d851ddd2cc 100644
>> > --- a/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
>> > +++ b/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
>> > @@ -28,6 +28,13 @@ struct {
>> >         __type(value, int);
>> >  } sockopt_results SEC(".maps");
>> >
>> > +struct {
>> > +       __uint(type, BPF_MAP_TYPE_ARRAY);
>> > +       __uint(max_entries, 1);
>> > +       __type(key, __u32);
>> > +       __type(value, __u64);
>> > +} netns_number SEC(".maps");
>> > +
>> >  static inline void update_event_map(int event)
>> >  {
>> >         __u32 key = 0;
>> > @@ -61,6 +68,7 @@ int bpf_testcb(struct bpf_sock_ops *skops)
>> >         int rv = -1;
>> >         int v = 0;
>> >         int op;
>> > +       __u64 netns_id;
>> >
>> >         op = (int) skops->op;
>> >
>> > @@ -144,6 +152,9 @@ int bpf_testcb(struct bpf_sock_ops *skops)
>> >                 __u32 key = 0;
>> >
>> >                 bpf_map_update_elem(&sockopt_results, &key, &v, BPF_ANY);
>> > +
>> > +               netns_id = bpf_get_netns_id(skops);
>> > +               bpf_map_update_elem(&netns_number, &key, &netns_id, BPF_ANY);
>> >                 break;
>> >         default:
>> >                 rv = -1;
>> > diff --git a/tools/testing/selftests/bpf/test_tcpbpf_user.c b/tools/testing/selftests/bpf/test_tcpbpf_user.c
>> > index 3ae127620463..fef2f4d77ecc 100644
>> > --- a/tools/testing/selftests/bpf/test_tcpbpf_user.c
>> > +++ b/tools/testing/selftests/bpf/test_tcpbpf_user.c
>> > @@ -76,6 +76,41 @@ int verify_sockopt_result(int sock_map_fd)
>> >         return ret;
>> >  }
>> >
>> > +int verify_netns(__u64 netns_id)
>> > +{
>> > +       char buf1[40];
>> > +       char buf2[40];
>> > +       int ret = 0;
>> > +       ssize_t len = 0;
>> > +
>> > +       len = readlink("/proc/self/ns/net", buf1, 39);
>> > +       sprintf(buf2, "net:[%llu]", netns_id);
>> > +
>> > +       if (len <= 0) {
>> > +               printf("FAILED: readlink /proc/self/ns/net");
>> > +               return ret;
>> > +       }
>> > +
>> > +       if (strncmp(buf1, buf2, len)) {
>> > +               printf("FAILED: netns don't match");
>> > +               ret = 1;
>> > +       }
>> > +       return ret;
>> > +}
>> > +
>> > +int verify_netns_result(int netns_map_fd)
>> > +{
>> > +       __u32 key = 0;
>> > +       __u64 res = 0;
>> > +       int ret = 0;
>> > +       int rv;
>> > +
>> > +       rv = bpf_map_lookup_elem(netns_map_fd, &key, &res);
>> > +       EXPECT_EQ(0, rv, "d");
>> > +
>> > +       return verify_netns(res);
>> > +}
>> > +
>> >  static int bpf_find_map(const char *test, struct bpf_object *obj,
>> >                         const char *name)
>> >  {
>> > @@ -92,7 +127,7 @@ static int bpf_find_map(const char *test, struct bpf_object *obj,
>> >  int main(int argc, char **argv)
>> >  {
>> >         const char *file = "test_tcpbpf_kern.o";
>> > -       int prog_fd, map_fd, sock_map_fd;
>> > +       int prog_fd, map_fd, sock_map_fd, netns_map_fd;
>> >         struct tcpbpf_globals g = {0};
>> >         const char *cg_path = "/foo";
>> >         int error = EXIT_FAILURE;
>> > @@ -137,6 +172,10 @@ int main(int argc, char **argv)
>> >         if (sock_map_fd < 0)
>> >                 goto err;
>> >
>> > +       netns_map_fd = bpf_find_map(__func__, obj, "netns_number");
>> > +       if (netns_map_fd < 0)
>> > +               goto err;
>> > +
>> >  retry_lookup:
>> >         rv = bpf_map_lookup_elem(map_fd, &key, &g);
>> >         if (rv != 0) {
>> > @@ -161,6 +200,11 @@ int main(int argc, char **argv)
>> >                 goto err;
>> >         }
>> >
>> > +       if (verify_netns_result(netns_map_fd)) {
>> > +               printf("FAILED: Wrong netns stats\n");
>> > +               goto err;
>> > +       }
>> > +
>> >         printf("PASSED!\n");
>> >         error = 0;
>> >  err:
>> > --
>> > 2.20.1
>> >
>
>
>
> --
> Beijing University of Posts and Telecommunications
> forrest0579@gmail.com
>
>
Andrii Nakryiko Feb. 26, 2020, 4:35 a.m. UTC | #3
On Tue, Feb 25, 2020 at 5:20 PM Forrest Chen <forrest0579@gmail.com> wrote:
>
> Got it. So I think we could first merge this and refactor the tcpbpf test(or maybe also some other tests) in another thread, is that ok with you?

Sure, as long as there is a follow up.

Also, please make sure to reply inline.

>
> Andrii Nakryiko <andrii.nakryiko@gmail.com> 于2020年2月26日周三 上午1:13写道:
>>
>> On Mon, Feb 24, 2020 at 11:20 PM Forrest Chen <forrest0579@gmail.com> wrote:
>> >
>> > > It would be nice if this selftests becomes part of test_progs.
>> >
>> > You mean the whole tests of tcpbpf or only the changes I made in this test?
>> > If you mean the whole tests of tcpbpf, I think we could fire another thread
>> > to do this?
>>
>> Yeah, I meant entire tcpbpf test.
>>
>> >
>> > Andrii Nakryiko <andrii.nakryiko@gmail.com> 于2020年2月25日周二 下午2:13写道:
>> >>
>> >> On Mon, Feb 24, 2020 at 8:47 PM Lingpeng Chen <forrest0579@gmail.com> wrote:
>> >> >
>> >> > adding selftest for new bpf helper function get_netns_id
>> >> >
>> >> > Signed-off-by: Lingpeng Chen <forrest0579@gmail.com>
>> >> > Acked-by: Song Liu <songliubraving@fb.com>
>> >> > ---
>> >>
>> >> It would be nice if this selftests becomes part of test_progs. That
>> >> way it would be exercised regularly, both by committers, as well as by
>> >> automated CI in libbpf's Github repo. Using global variables and BPF
>> >> skeleton would also clean up both BPF and user-space code.
>> >>
>> >> It seems like this test runs Python script for server, but doesn't
>> >> seem like that server is doing anything complicated, so writing that
>> >> in C shouldn't be a problem as well. Thoughts?
>> >>
>> >> >  .../selftests/bpf/progs/test_tcpbpf_kern.c    | 11 +++++
>> >> >  .../testing/selftests/bpf/test_tcpbpf_user.c  | 46 ++++++++++++++++++-
>> >> >  2 files changed, 56 insertions(+), 1 deletion(-)
>> >> >

[...]

Patch
diff mbox series

diff --git a/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c b/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
index 1f1966e86e9f..d7d851ddd2cc 100644
--- a/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
+++ b/tools/testing/selftests/bpf/progs/test_tcpbpf_kern.c
@@ -28,6 +28,13 @@  struct {
 	__type(value, int);
 } sockopt_results SEC(".maps");
 
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, __u32);
+	__type(value, __u64);
+} netns_number SEC(".maps");
+
 static inline void update_event_map(int event)
 {
 	__u32 key = 0;
@@ -61,6 +68,7 @@  int bpf_testcb(struct bpf_sock_ops *skops)
 	int rv = -1;
 	int v = 0;
 	int op;
+	__u64 netns_id;
 
 	op = (int) skops->op;
 
@@ -144,6 +152,9 @@  int bpf_testcb(struct bpf_sock_ops *skops)
 		__u32 key = 0;
 
 		bpf_map_update_elem(&sockopt_results, &key, &v, BPF_ANY);
+
+		netns_id = bpf_get_netns_id(skops);
+		bpf_map_update_elem(&netns_number, &key, &netns_id, BPF_ANY);
 		break;
 	default:
 		rv = -1;
diff --git a/tools/testing/selftests/bpf/test_tcpbpf_user.c b/tools/testing/selftests/bpf/test_tcpbpf_user.c
index 3ae127620463..fef2f4d77ecc 100644
--- a/tools/testing/selftests/bpf/test_tcpbpf_user.c
+++ b/tools/testing/selftests/bpf/test_tcpbpf_user.c
@@ -76,6 +76,41 @@  int verify_sockopt_result(int sock_map_fd)
 	return ret;
 }
 
+int verify_netns(__u64 netns_id)
+{
+	char buf1[40];
+	char buf2[40];
+	int ret = 0;
+	ssize_t len = 0;
+
+	len = readlink("/proc/self/ns/net", buf1, 39);
+	sprintf(buf2, "net:[%llu]", netns_id);
+
+	if (len <= 0) {
+		printf("FAILED: readlink /proc/self/ns/net");
+		return ret;
+	}
+
+	if (strncmp(buf1, buf2, len)) {
+		printf("FAILED: netns don't match");
+		ret = 1;
+	}
+	return ret;
+}
+
+int verify_netns_result(int netns_map_fd)
+{
+	__u32 key = 0;
+	__u64 res = 0;
+	int ret = 0;
+	int rv;
+
+	rv = bpf_map_lookup_elem(netns_map_fd, &key, &res);
+	EXPECT_EQ(0, rv, "d");
+
+	return verify_netns(res);
+}
+
 static int bpf_find_map(const char *test, struct bpf_object *obj,
 			const char *name)
 {
@@ -92,7 +127,7 @@  static int bpf_find_map(const char *test, struct bpf_object *obj,
 int main(int argc, char **argv)
 {
 	const char *file = "test_tcpbpf_kern.o";
-	int prog_fd, map_fd, sock_map_fd;
+	int prog_fd, map_fd, sock_map_fd, netns_map_fd;
 	struct tcpbpf_globals g = {0};
 	const char *cg_path = "/foo";
 	int error = EXIT_FAILURE;
@@ -137,6 +172,10 @@  int main(int argc, char **argv)
 	if (sock_map_fd < 0)
 		goto err;
 
+	netns_map_fd = bpf_find_map(__func__, obj, "netns_number");
+	if (netns_map_fd < 0)
+		goto err;
+
 retry_lookup:
 	rv = bpf_map_lookup_elem(map_fd, &key, &g);
 	if (rv != 0) {
@@ -161,6 +200,11 @@  int main(int argc, char **argv)
 		goto err;
 	}
 
+	if (verify_netns_result(netns_map_fd)) {
+		printf("FAILED: Wrong netns stats\n");
+		goto err;
+	}
+
 	printf("PASSED!\n");
 	error = 0;
 err: