2022-07-12 20:30:29

by Steven Rostedt

Subject: [PATCH] selftests/kprobe: Do not test for GRP/ without event failures

From: "Steven Rostedt (Google)" <[email protected]>

A new feature is added where kprobes (and other probes) do not need to
explicitly state the event name when creating a probe. The event name will
come from what is being attached.

That is:

# echo 'p:foo/ vfs_read' > kprobe_events

Will no longer error, but instead create an event:

# cat kprobe_events
p:foo/p_vfs_read_0 vfs_read

This should not be tested as an error case anymore. Remove it from the
selftest as now this feature "breaks" the selftest as it no longer fails
as expected.

Link: https://lore.kernel.org/all/[email protected]/

Signed-off-by: Steven Rostedt (Google) <[email protected]>
---
.../selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 1 -
1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
index fa928b431555..7c02509c71d0 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
@@ -21,7 +21,6 @@ check_error 'p:^/bar vfs_read' # NO_GROUP_NAME
check_error 'p:^12345678901234567890123456789012345678901234567890123456789012345/bar vfs_read' # GROUP_TOO_LONG

check_error 'p:^foo.1/bar vfs_read' # BAD_GROUP_NAME
-check_error 'p:foo/^ vfs_read' # NO_EVENT_NAME
check_error 'p:foo/^12345678901234567890123456789012345678901234567890123456789012345 vfs_read' # EVENT_TOO_LONG
check_error 'p:foo/^bar.1 vfs_read' # BAD_EVENT_NAME

--
2.35.1

2022-07-18 02:37:45

by Masami Hiramatsu

Subject: Re: [PATCH] selftests/kprobe: Do not test for GRP/ without event failures

On Tue, 12 Jul 2022 16:17:07 -0400
Steven Rostedt <[email protected]> wrote:

> From: "Steven Rostedt (Google)" <[email protected]>
>
> A new feature is added where kprobes (and other probes) do not need to
> explicitly state the event name when creating a probe. The event name will
> come from what is being attached.
>
> That is:
>
> # echo 'p:foo/ vfs_read' > kprobe_events
>
> Will no longer error, but instead create an event:
>
> # cat kprobe_events
> p:foo/p_vfs_read_0 vfs_read
>
> This should not be tested as an error case anymore. Remove it from the
> selftest as now this feature "breaks" the selftest as it no longer fails
> as expected.

Good catch!

Acked-by: Masami Hiramatsu (Google) <[email protected]>

BTW, in this case, NO_EVENT_NAME error should not happen anymore.
Let me cleanup the code.

Thank you,

>
> Link: https://lore.kernel.org/all/[email protected]/
>
> Signed-off-by: Steven Rostedt (Google) <[email protected]>
> ---
> .../selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
> index fa928b431555..7c02509c71d0 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
> @@ -21,7 +21,6 @@ check_error 'p:^/bar vfs_read' # NO_GROUP_NAME
> check_error 'p:^12345678901234567890123456789012345678901234567890123456789012345/bar vfs_read' # GROUP_TOO_LONG
>
> check_error 'p:^foo.1/bar vfs_read' # BAD_GROUP_NAME
> -check_error 'p:foo/^ vfs_read' # NO_EVENT_NAME
> check_error 'p:foo/^12345678901234567890123456789012345678901234567890123456789012345 vfs_read' # EVENT_TOO_LONG
> check_error 'p:foo/^bar.1 vfs_read' # BAD_EVENT_NAME
>
> --
> 2.35.1
>


--
Masami Hiramatsu (Google) <[email protected]>

2022-07-18 06:06:54

by Masami Hiramatsu

Subject: Re: [PATCH] selftests/kprobe: Do not test for GRP/ without event failures

On Mon, 18 Jul 2022 11:08:53 +0900
Masami Hiramatsu (Google) <[email protected]> wrote:

> On Tue, 12 Jul 2022 16:17:07 -0400
> Steven Rostedt <[email protected]> wrote:
>
> > From: "Steven Rostedt (Google)" <[email protected]>
> >
> > A new feature is added where kprobes (and other probes) do not need to
> > explicitly state the event name when creating a probe. The event name will
> > come from what is being attached.
> >
> > That is:
> >
> > # echo 'p:foo/ vfs_read' > kprobe_events
> >
> > Will no longer error, but instead create an event:
> >
> > # cat kprobe_events
> > p:foo/p_vfs_read_0 vfs_read
> >
> > This should not be tested as an error case anymore. Remove it from the
> > selftest as now this feature "breaks" the selftest as it no longer fails
> > as expected.
>
> Good catch!
>
> Acked-by: Masami Hiramatsu (Google) <[email protected]>
>
> BTW, in this case, NO_EVENT_NAME error should not happen anymore.
> Let me cleanup the code.

Oops, no. There is an error case of NO_EVENT_NAME. 'p: vfs_read' will cause
this error because it expects an event name after ':'. Thus, the correct
fix is just removing "foo/":

check_error 'p:^ vfs_read' # NO_EVENT_NAME

Thank you,

--
Masami Hiramatsu (Google) <[email protected]>

2022-07-18 07:18:35

by Masami Hiramatsu

Subject: [PATCH] selftests/kprobe: Update test for no event name syntax error

From: Masami Hiramatsu (Google) <[email protected]>

The commit 208003254c32 ("selftests/kprobe: Do not test for GRP/
without event failures") removed a syntax which is no more cause
a syntax error (NO_EVENT_NAME error with GRP/).
However, there are another case (NO_EVENT_NAME error without GRP/)
which causes a same error. This adds a test for that case.

Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
---
.../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 1 +
1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
index 7c02509c71d0..9e85d3019ff0 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
@@ -21,6 +21,7 @@ check_error 'p:^/bar vfs_read' # NO_GROUP_NAME
check_error 'p:^12345678901234567890123456789012345678901234567890123456789012345/bar vfs_read' # GROUP_TOO_LONG

check_error 'p:^foo.1/bar vfs_read' # BAD_GROUP_NAME
+check_error 'p:^ vfs_read' # NO_EVENT_NAME
check_error 'p:foo/^12345678901234567890123456789012345678901234567890123456789012345 vfs_read' # EVENT_TOO_LONG
check_error 'p:foo/^bar.1 vfs_read' # BAD_EVENT_NAME

2022-07-18 09:59:21

by Linyu Yuan

Subject: Re: [PATCH] selftests/kprobe: Update test for no event name syntax error

hi Masami,

On 7/18/2022 3:05 PM, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <[email protected]>
>
> The commit 208003254c32 ("selftests/kprobe: Do not test for GRP/
> without event failures") removed a syntax which is no more cause
> a syntax error (NO_EVENT_NAME error with GRP/).
> However, there are another case (NO_EVENT_NAME error without GRP/)
> which causes a same error. This adds a test for that case.
>
> Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
> ---
> .../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
> index 7c02509c71d0..9e85d3019ff0 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
> @@ -21,6 +21,7 @@ check_error 'p:^/bar vfs_read' # NO_GROUP_NAME
> check_error 'p:^12345678901234567890123456789012345678901234567890123456789012345/bar vfs_read' # GROUP_TOO_LONG
>
> check_error 'p:^foo.1/bar vfs_read' # BAD_GROUP_NAME
> +check_error 'p:^ vfs_read' # NO_EVENT_NAME

i think you fix the issue which exist from start, right ?

is there better comment than NO_EVENT_NAME  ?

> check_error 'p:foo/^12345678901234567890123456789012345678901234567890123456789012345 vfs_read' # EVENT_TOO_LONG
> check_error 'p:foo/^bar.1 vfs_read' # BAD_EVENT_NAME
>
>

2022-07-18 22:42:50

by Steven Rostedt

Subject: Re: [PATCH] selftests/kprobe: Update test for no event name syntax error

On Mon, 18 Jul 2022 16:05:10 +0900
"Masami Hiramatsu (Google)" <[email protected]> wrote:

> From: Masami Hiramatsu (Google) <[email protected]>
>
> The commit 208003254c32 ("selftests/kprobe: Do not test for GRP/
> without event failures") removed a syntax which is no more cause
> a syntax error (NO_EVENT_NAME error with GRP/).
> However, there are another case (NO_EVENT_NAME error without GRP/)
> which causes a same error. This adds a test for that case.
>
> Signed-off-by: Masami Hiramatsu (Google) <[email protected]>

Queued. Thanks Masami!

-- Steve

> ---
> .../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
> index 7c02509c71d0..9e85d3019ff0 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
> @@ -21,6 +21,7 @@ check_error 'p:^/bar vfs_read' # NO_GROUP_NAME
> check_error 'p:^12345678901234567890123456789012345678901234567890123456789012345/bar vfs_read' # GROUP_TOO_LONG
>
> check_error 'p:^foo.1/bar vfs_read' # BAD_GROUP_NAME
> +check_error 'p:^ vfs_read' # NO_EVENT_NAME
> check_error 'p:foo/^12345678901234567890123456789012345678901234567890123456789012345 vfs_read' # EVENT_TOO_LONG
> check_error 'p:foo/^bar.1 vfs_read' # BAD_EVENT_NAME
>

2022-07-21 14:59:09

by Masami Hiramatsu

Subject: Re: [PATCH] selftests/kprobe: Update test for no event name syntax error

On Mon, 18 Jul 2022 17:36:43 +0800
Linyu Yuan <[email protected]> wrote:

> hi Masami,
>
> On 7/18/2022 3:05 PM, Masami Hiramatsu (Google) wrote:
> > From: Masami Hiramatsu (Google) <[email protected]>
> >
> > The commit 208003254c32 ("selftests/kprobe: Do not test for GRP/
> > without event failures") removed a syntax which is no more cause
> > a syntax error (NO_EVENT_NAME error with GRP/).
> > However, there are another case (NO_EVENT_NAME error without GRP/)
> > which causes a same error. This adds a test for that case.
> >
> > Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
> > ---
> > .../ftrace/test.d/kprobe/kprobe_syntax_errors.tc | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
> > index 7c02509c71d0..9e85d3019ff0 100644
> > --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
> > @@ -21,6 +21,7 @@ check_error 'p:^/bar vfs_read' # NO_GROUP_NAME
> > check_error 'p:^12345678901234567890123456789012345678901234567890123456789012345/bar vfs_read' # GROUP_TOO_LONG
> >
> > check_error 'p:^foo.1/bar vfs_read' # BAD_GROUP_NAME
> > +check_error 'p:^ vfs_read' # NO_EVENT_NAME
>
> i think you fix the issue which exist from start, right ?

Yes, this is not a new bug but the error case which still
exists.

>
> is there better comment than NO_EVENT_NAME  ?

These comments are corresponding to the error name, so that we can
find the logging code easily. (Not for users)

Thank you,

>
> > check_error 'p:foo/^12345678901234567890123456789012345678901234567890123456789012345 vfs_read' # EVENT_TOO_LONG
> > check_error 'p:foo/^bar.1 vfs_read' # BAD_EVENT_NAME
> >
> >


--
Masami Hiramatsu (Google) <[email protected]>