2018-08-23 16:01:19

by Pavel Tikhomirov

[permalink] [raw]
Subject: [PATCH] tracing: Fix event filters and triggers to handle negative numbers

Then tracing syscall exit event it is extremely useful to filter exit
codes equal to some negative value, to react only to required errors.
But negative numbers does not work:

[root@snorch sys_exit_read]# echo "ret == -1" > filter
bash: echo: write error: Invalid argument
[root@snorch sys_exit_read]# cat filter
ret == -1
^
parse_error: Invalid value (did you forget quotes)?

Similar thing happens when setting triggers.

These is a regression in v4.17 introduced by the commit mentioned below,
testing without these commit shows no problem with negative numbers.

Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
Signed-off-by: Pavel Tikhomirov <[email protected]>
---
kernel/trace/trace_events_filter.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 84a65173b1e9..2ba449292561 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1299,7 +1299,7 @@ static int parse_pred(const char *str, void *data,
/* go past the last quote */
i++;

- } else if (isdigit(str[i])) {
+ } else if (isdigit(str[i]) || str[i] == '-') {

/* Make sure the field is not a string */
if (is_string_field(field)) {
@@ -1312,6 +1312,9 @@ static int parse_pred(const char *str, void *data,
goto err_free;
}

+ if (str[i] == '-')
+ i++;
+
/* We allow 0xDEADBEEF */
while (isalnum(str[i]))
i++;
--
2.17.1



2018-08-24 00:51:10

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] tracing: Fix event filters and triggers to handle negative numbers

On Thu, 23 Aug 2018 13:25:34 +0300
Pavel Tikhomirov <[email protected]> wrote:

> Then tracing syscall exit event it is extremely useful to filter exit
> codes equal to some negative value, to react only to required errors.
> But negative numbers does not work:
>
> [root@snorch sys_exit_read]# echo "ret == -1" > filter
> bash: echo: write error: Invalid argument
> [root@snorch sys_exit_read]# cat filter
> ret == -1
> ^
> parse_error: Invalid value (did you forget quotes)?

Thanks for the patch. I'll apply it and then start testing it!

-- Steve

>
> Similar thing happens when setting triggers.
>
> These is a regression in v4.17 introduced by the commit mentioned below,
> testing without these commit shows no problem with negative numbers.
>
> Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
> Signed-off-by: Pavel Tikhomirov <[email protected]>
> ---
> kernel/trace/trace_events_filter.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 84a65173b1e9..2ba449292561 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -1299,7 +1299,7 @@ static int parse_pred(const char *str, void *data,
> /* go past the last quote */
> i++;
>
> - } else if (isdigit(str[i])) {
> + } else if (isdigit(str[i]) || str[i] == '-') {
>
> /* Make sure the field is not a string */
> if (is_string_field(field)) {
> @@ -1312,6 +1312,9 @@ static int parse_pred(const char *str, void *data,
> goto err_free;
> }
>
> + if (str[i] == '-')
> + i++;
> +
> /* We allow 0xDEADBEEF */
> while (isalnum(str[i]))
> i++;


2019-02-28 11:43:19

by Pavel Tikhomirov

[permalink] [raw]
Subject: Re: [PATCH] tracing: Fix event filters and triggers to handle negative numbers

ping, looks like the patch was lost

On 8/24/18 3:48 AM, Steven Rostedt wrote:
> On Thu, 23 Aug 2018 13:25:34 +0300
> Pavel Tikhomirov <[email protected]> wrote:
>
>> Then tracing syscall exit event it is extremely useful to filter exit
>> codes equal to some negative value, to react only to required errors.
>> But negative numbers does not work:
>>
>> [root@snorch sys_exit_read]# echo "ret == -1" > filter
>> bash: echo: write error: Invalid argument
>> [root@snorch sys_exit_read]# cat filter
>> ret == -1
>> ^
>> parse_error: Invalid value (did you forget quotes)?
>
> Thanks for the patch. I'll apply it and then start testing it!
>
> -- Steve
>
>>
>> Similar thing happens when setting triggers.
>>
>> These is a regression in v4.17 introduced by the commit mentioned below,
>> testing without these commit shows no problem with negative numbers.
>>
>> Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
>> Signed-off-by: Pavel Tikhomirov <[email protected]>
>> ---
>> kernel/trace/trace_events_filter.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
>> index 84a65173b1e9..2ba449292561 100644
>> --- a/kernel/trace/trace_events_filter.c
>> +++ b/kernel/trace/trace_events_filter.c
>> @@ -1299,7 +1299,7 @@ static int parse_pred(const char *str, void *data,
>> /* go past the last quote */
>> i++;
>>
>> - } else if (isdigit(str[i])) {
>> + } else if (isdigit(str[i]) || str[i] == '-') {
>>
>> /* Make sure the field is not a string */
>> if (is_string_field(field)) {
>> @@ -1312,6 +1312,9 @@ static int parse_pred(const char *str, void *data,
>> goto err_free;
>> }
>>
>> + if (str[i] == '-')
>> + i++;
>> +
>> /* We allow 0xDEADBEEF */
>> while (isalnum(str[i]))
>> i++;
>

--
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.

2019-02-28 19:36:00

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] tracing: Fix event filters and triggers to handle negative numbers

On Thu, 28 Feb 2019 09:11:37 +0000
Pavel Tikhomirov <[email protected]> wrote:

> ping, looks like the patch was lost
>
> On 8/24/18 3:48 AM, Steven Rostedt wrote:
> > On Thu, 23 Aug 2018 13:25:34 +0300
> > Pavel Tikhomirov <[email protected]> wrote:
> >
> >> Then tracing syscall exit event it is extremely useful to filter exit
> >> codes equal to some negative value, to react only to required errors.
> >> But negative numbers does not work:
> >>
> >> [root@snorch sys_exit_read]# echo "ret == -1" > filter
> >> bash: echo: write error: Invalid argument
> >> [root@snorch sys_exit_read]# cat filter
> >> ret == -1
> >> ^
> >> parse_error: Invalid value (did you forget quotes)?
> >
> > Thanks for the patch. I'll apply it and then start testing it!

?!?

I remember applying this to my git queue (I don't reply like this
before doing so). Not sure how it got lost. All I can think of is that
it might have been added with commits that ended up failing the tests,
and got reverted with that code :-/

I'm currently traveling. Hopefully I remember tomorrow to add this.
Thanks for pointing this out, and I need to be a bit more careful when
reverting failed commits :-p

-- Steve