diff mbox series

[PATCHv2,bpf-next,2/5] selftests/bpf: Add re-attach test to fentry_test

Message ID 20210406212913.970917-3-jolsa@kernel.org
State New
Headers show
Series bpf: Tracing and lsm programs re-attach | expand

Commit Message

Jiri Olsa April 6, 2021, 9:29 p.m. UTC 
Adding the test to re-attach (detach/attach again) tracing
fentry programs, plus check that already linked program can't
be attached again.

Fixing the number of check-ed results, which should be 8.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../selftests/bpf/prog_tests/fentry_test.c    | 48 +++++++++++++++----
 1 file changed, 38 insertions(+), 10 deletions(-)

Comments

Andrii Nakryiko April 7, 2021, 10:47 p.m. UTC | #1 
On Wed, Apr 7, 2021 at 4:21 AM Jiri Olsa <jolsa@kernel.org> wrote:
>

> Adding the test to re-attach (detach/attach again) tracing

> fentry programs, plus check that already linked program can't

> be attached again.

>

> Fixing the number of check-ed results, which should be 8.

>

> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

> ---

>  .../selftests/bpf/prog_tests/fentry_test.c    | 48 +++++++++++++++----

>  1 file changed, 38 insertions(+), 10 deletions(-)

>

> diff --git a/tools/testing/selftests/bpf/prog_tests/fentry_test.c b/tools/testing/selftests/bpf/prog_tests/fentry_test.c

> index 04ebbf1cb390..1f7566e772e9 100644

> --- a/tools/testing/selftests/bpf/prog_tests/fentry_test.c

> +++ b/tools/testing/selftests/bpf/prog_tests/fentry_test.c

> @@ -3,20 +3,24 @@

>  #include <test_progs.h>

>  #include "fentry_test.skel.h"

>

> -void test_fentry_test(void)

> +static __u32 duration;

> +

> +static int fentry_test(struct fentry_test *fentry_skel)

>  {

> -       struct fentry_test *fentry_skel = NULL;

> +       struct bpf_link *link;

>         int err, prog_fd, i;

> -       __u32 duration = 0, retval;

>         __u64 *result;

> -

> -       fentry_skel = fentry_test__open_and_load();

> -       if (CHECK(!fentry_skel, "fentry_skel_load", "fentry skeleton failed\n"))

> -               goto cleanup;

> +       __u32 retval;

>

>         err = fentry_test__attach(fentry_skel);

>         if (CHECK(err, "fentry_attach", "fentry attach failed: %d\n", err))

> -               goto cleanup;

> +               return err;

> +

> +       /* Check that already linked program can't be attached again. */

> +       link = bpf_program__attach(fentry_skel->progs.test1);

> +       if (CHECK(!IS_ERR(link), "fentry_attach_link",


if (!ASSERT_ERR_PTR(link, "fentry_attach_link")) ?

> +                 "re-attach without detach should not succeed"))

> +               return -1;

>

>         prog_fd = bpf_program__fd(fentry_skel->progs.test1);

>         err = bpf_prog_test_run(prog_fd, 1, NULL, 0,

> @@ -26,12 +30,36 @@ void test_fentry_test(void)

>               err, errno, retval, duration);

>

>         result = (__u64 *)fentry_skel->bss;

> -       for (i = 0; i < 6; i++) {

> +       for (i = 0; i < 8; i++) {


how about using sizeof(*fentry_skel->bss) / sizeof(__u64) ?

>                 if (CHECK(result[i] != 1, "result",

>                           "fentry_test%d failed err %lld\n", i + 1, result[i]))

> -                       goto cleanup;

> +                       return -1;

>         }

>

> +       fentry_test__detach(fentry_skel);

> +

> +       /* zero results for re-attach test */

> +       for (i = 0; i < 8; i++)

> +               result[i] = 0;

> +       return 0;

> +}

> +

> +void test_fentry_test(void)

> +{

> +       struct fentry_test *fentry_skel = NULL;

> +       int err;

> +

> +       fentry_skel = fentry_test__open_and_load();

> +       if (CHECK(!fentry_skel, "fentry_skel_load", "fentry skeleton failed\n"))

> +               goto cleanup;

> +

> +       err = fentry_test(fentry_skel);

> +       if (CHECK(err, "fentry_test", "first attach failed\n"))

> +               goto cleanup;

> +

> +       err = fentry_test(fentry_skel);

> +       CHECK(err, "fentry_test", "second attach failed\n");


overall: please try to use ASSERT_xxx macros, they are easier to
follow and require less typing
> +

>  cleanup:

>         fentry_test__destroy(fentry_skel);

>  }

> --

> 2.30.2

>
Jiri Olsa April 8, 2021, 11:52 a.m. UTC | #2 
On Wed, Apr 07, 2021 at 03:47:30PM -0700, Andrii Nakryiko wrote:
> On Wed, Apr 7, 2021 at 4:21 AM Jiri Olsa <jolsa@kernel.org> wrote:

> >

> > Adding the test to re-attach (detach/attach again) tracing

> > fentry programs, plus check that already linked program can't

> > be attached again.

> >

> > Fixing the number of check-ed results, which should be 8.

> >

> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>

> > ---

> >  .../selftests/bpf/prog_tests/fentry_test.c    | 48 +++++++++++++++----

> >  1 file changed, 38 insertions(+), 10 deletions(-)

> >

> > diff --git a/tools/testing/selftests/bpf/prog_tests/fentry_test.c b/tools/testing/selftests/bpf/prog_tests/fentry_test.c

> > index 04ebbf1cb390..1f7566e772e9 100644

> > --- a/tools/testing/selftests/bpf/prog_tests/fentry_test.c

> > +++ b/tools/testing/selftests/bpf/prog_tests/fentry_test.c

> > @@ -3,20 +3,24 @@

> >  #include <test_progs.h>

> >  #include "fentry_test.skel.h"

> >

> > -void test_fentry_test(void)

> > +static __u32 duration;

> > +

> > +static int fentry_test(struct fentry_test *fentry_skel)

> >  {

> > -       struct fentry_test *fentry_skel = NULL;

> > +       struct bpf_link *link;

> >         int err, prog_fd, i;

> > -       __u32 duration = 0, retval;

> >         __u64 *result;

> > -

> > -       fentry_skel = fentry_test__open_and_load();

> > -       if (CHECK(!fentry_skel, "fentry_skel_load", "fentry skeleton failed\n"))

> > -               goto cleanup;

> > +       __u32 retval;

> >

> >         err = fentry_test__attach(fentry_skel);

> >         if (CHECK(err, "fentry_attach", "fentry attach failed: %d\n", err))

> > -               goto cleanup;

> > +               return err;

> > +

> > +       /* Check that already linked program can't be attached again. */

> > +       link = bpf_program__attach(fentry_skel->progs.test1);

> > +       if (CHECK(!IS_ERR(link), "fentry_attach_link",

> 

> if (!ASSERT_ERR_PTR(link, "fentry_attach_link")) ?


ok

> 

> > +                 "re-attach without detach should not succeed"))

> > +               return -1;

> >

> >         prog_fd = bpf_program__fd(fentry_skel->progs.test1);

> >         err = bpf_prog_test_run(prog_fd, 1, NULL, 0,

> > @@ -26,12 +30,36 @@ void test_fentry_test(void)

> >               err, errno, retval, duration);

> >

> >         result = (__u64 *)fentry_skel->bss;

> > -       for (i = 0; i < 6; i++) {

> > +       for (i = 0; i < 8; i++) {

> 

> how about using sizeof(*fentry_skel->bss) / sizeof(__u64) ?


ok

> 

> >                 if (CHECK(result[i] != 1, "result",

> >                           "fentry_test%d failed err %lld\n", i + 1, result[i]))

> > -                       goto cleanup;

> > +                       return -1;

> >         }

> >

> > +       fentry_test__detach(fentry_skel);

> > +

> > +       /* zero results for re-attach test */

> > +       for (i = 0; i < 8; i++)

> > +               result[i] = 0;

> > +       return 0;

> > +}

> > +

> > +void test_fentry_test(void)

> > +{

> > +       struct fentry_test *fentry_skel = NULL;

> > +       int err;

> > +

> > +       fentry_skel = fentry_test__open_and_load();

> > +       if (CHECK(!fentry_skel, "fentry_skel_load", "fentry skeleton failed\n"))

> > +               goto cleanup;

> > +

> > +       err = fentry_test(fentry_skel);

> > +       if (CHECK(err, "fentry_test", "first attach failed\n"))

> > +               goto cleanup;

> > +

> > +       err = fentry_test(fentry_skel);

> > +       CHECK(err, "fentry_test", "second attach failed\n");

> 

> overall: please try to use ASSERT_xxx macros, they are easier to

> follow and require less typing


ok, will check

thanks,
jirka
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/fentry_test.c b/tools/testing/selftests/bpf/prog_tests/fentry_test.c
index 04ebbf1cb390..1f7566e772e9 100644
--- a/tools/testing/selftests/bpf/prog_tests/fentry_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/fentry_test.c
@@ -3,20 +3,24 @@ 
 #include <test_progs.h>
 #include "fentry_test.skel.h"
 
-void test_fentry_test(void)
+static __u32 duration;
+
+static int fentry_test(struct fentry_test *fentry_skel)
 {
-	struct fentry_test *fentry_skel = NULL;
+	struct bpf_link *link;
 	int err, prog_fd, i;
-	__u32 duration = 0, retval;
 	__u64 *result;
-
-	fentry_skel = fentry_test__open_and_load();
-	if (CHECK(!fentry_skel, "fentry_skel_load", "fentry skeleton failed\n"))
-		goto cleanup;
+	__u32 retval;
 
 	err = fentry_test__attach(fentry_skel);
 	if (CHECK(err, "fentry_attach", "fentry attach failed: %d\n", err))
-		goto cleanup;
+		return err;
+
+	/* Check that already linked program can't be attached again. */
+	link = bpf_program__attach(fentry_skel->progs.test1);
+	if (CHECK(!IS_ERR(link), "fentry_attach_link",
+		  "re-attach without detach should not succeed"))
+		return -1;
 
 	prog_fd = bpf_program__fd(fentry_skel->progs.test1);
 	err = bpf_prog_test_run(prog_fd, 1, NULL, 0,
@@ -26,12 +30,36 @@  void test_fentry_test(void)
 	      err, errno, retval, duration);
 
 	result = (__u64 *)fentry_skel->bss;
-	for (i = 0; i < 6; i++) {
+	for (i = 0; i < 8; i++) {
 		if (CHECK(result[i] != 1, "result",
 			  "fentry_test%d failed err %lld\n", i + 1, result[i]))
-			goto cleanup;
+			return -1;
 	}
 
+	fentry_test__detach(fentry_skel);
+
+	/* zero results for re-attach test */
+	for (i = 0; i < 8; i++)
+		result[i] = 0;
+	return 0;
+}
+
+void test_fentry_test(void)
+{
+	struct fentry_test *fentry_skel = NULL;
+	int err;
+
+	fentry_skel = fentry_test__open_and_load();
+	if (CHECK(!fentry_skel, "fentry_skel_load", "fentry skeleton failed\n"))
+		goto cleanup;
+
+	err = fentry_test(fentry_skel);
+	if (CHECK(err, "fentry_test", "first attach failed\n"))
+		goto cleanup;
+
+	err = fentry_test(fentry_skel);
+	CHECK(err, "fentry_test", "second attach failed\n");
+
 cleanup:
 	fentry_test__destroy(fentry_skel);
 }