This new test makes sure that ftrace can trace a
function that was introduced by a livepatch.

Signed-off-by: Filipe Xavier <felipeaggger@gmail.com>
---
 tools/testing/selftests/livepatch/test-ftrace.sh | 37 ++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tools/testing/selftests/livepatch/test-ftrace.sh b/tools/testing/selftests/livepatch/test-ftrace.sh
index fe14f248913acbec46fb6c0fec38a2fc84209d39..5f0d5308c88669e84210393ce7b8aa138b694ebd 100755
--- a/tools/testing/selftests/livepatch/test-ftrace.sh
+++ b/tools/testing/selftests/livepatch/test-ftrace.sh
@@ -61,4 +61,41 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete
 % rmmod $MOD_LIVEPATCH"
 
 
+# - verify livepatch can load
+# - check traces if have a patched function
+# - unload livepatch and reset trace
+
+start_test "livepatch trace patched function and check that the live patch remains in effect"
+
+TRACE_FILE="$SYSFS_DEBUG_DIR/tracing/trace"
+FUNCTION_NAME="livepatch_cmdline_proc_show"
+
+load_lp $MOD_LIVEPATCH
+
+echo $FUNCTION_NAME > $SYSFS_DEBUG_DIR/tracing/set_ftrace_filter
+echo "function" > $SYSFS_DEBUG_DIR/tracing/current_tracer
+echo "" > $TRACE_FILE
+
+if [[ "$(cat /proc/cmdline)" != "$MOD_LIVEPATCH: this has been live patched" ]] ; then
+	echo -e "FAIL\n\n"
+	die "livepatch kselftest(s) failed"
+fi
+
+grep -q $FUNCTION_NAME $TRACE_FILE
+FOUND=$?
+
+disable_lp $MOD_LIVEPATCH
+unload_lp $MOD_LIVEPATCH
+
+# Reset tracing
+echo "nop" > $SYSFS_DEBUG_DIR/tracing/current_tracer
+echo "" > $SYSFS_DEBUG_DIR/tracing/set_ftrace_filter
+echo "" > $TRACE_FILE
+
+if [ "$FOUND" -eq 1 ]; then
+	echo -e "FAIL\n\n"
+	die "livepatch kselftest(s) failed"
+fi
+
+
 exit 0

---
base-commit: fc033cf25e612e840e545f8d5ad2edd6ba613ed5
change-id: 20250101-ftrace-selftest-livepatch-161fb77dbed8

Best regards,
-- 
Filipe Xavier <felipeaggger@gmail.com>

On Thu, Jan 02, 2025 at 03:42:10PM -0300, Filipe Xavier wrote:
> This new test makes sure that ftrace can trace a
> function that was introduced by a livepatch.
>

Hi Filipe,

Thanks for adding a test!

Aside: another similar test could verify that the original function, in
this case cmdline_proc_show(), can still be traced despite it being
livepatched.  That may be non-intuitive but it demonstrates how the
ftrace handler works.
 
> Signed-off-by: Filipe Xavier <felipeaggger@gmail.com>
> ---
>  tools/testing/selftests/livepatch/test-ftrace.sh | 37 ++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/tools/testing/selftests/livepatch/test-ftrace.sh b/tools/testing/selftests/livepatch/test-ftrace.sh
> index fe14f248913acbec46fb6c0fec38a2fc84209d39..5f0d5308c88669e84210393ce7b8aa138b694ebd 100755
> --- a/tools/testing/selftests/livepatch/test-ftrace.sh
> +++ b/tools/testing/selftests/livepatch/test-ftrace.sh
> @@ -61,4 +61,41 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete
>  % rmmod $MOD_LIVEPATCH"
>  
>  
> +# - verify livepatch can load
> +# - check traces if have a patched function

nit: wording?  "check if traces have a patched function" ?

> +# - unload livepatch and reset trace
> +
> +start_test "livepatch trace patched function and check that the live patch remains in effect"

nit: wording?  "trace livepatched function and check ..." ?

> +
> +TRACE_FILE="$SYSFS_DEBUG_DIR/tracing/trace"
> +FUNCTION_NAME="livepatch_cmdline_proc_show"
> +
> +load_lp $MOD_LIVEPATCH
> +
> +echo $FUNCTION_NAME > $SYSFS_DEBUG_DIR/tracing/set_ftrace_filter
> +echo "function" > $SYSFS_DEBUG_DIR/tracing/current_tracer
> +echo "" > $TRACE_FILE

A few suggestions:

- The tracing is also dependent on the 'tracing_on' file, so if it
  happens to be turned off, the test will fail.

- See functions.sh :: push_config() and pop_config() for an example of
  saving the existing values rather than turning them all off at the end
  of the test.

- Nitpick: shellcheck suggests wrapping filenames in double quotations,
  applicable in several places.

> +
> +if [[ "$(cat /proc/cmdline)" != "$MOD_LIVEPATCH: this has been live patched" ]] ; then
> +	echo -e "FAIL\n\n"
> +	die "livepatch kselftest(s) failed"
> +fi
> +
> +grep -q $FUNCTION_NAME $TRACE_FILE
> +FOUND=$?
> +
> +disable_lp $MOD_LIVEPATCH
> +unload_lp $MOD_LIVEPATCH
> +
> +# Reset tracing
> +echo "nop" > $SYSFS_DEBUG_DIR/tracing/current_tracer
> +echo "" > $SYSFS_DEBUG_DIR/tracing/set_ftrace_filter
> +echo "" > $TRACE_FILE
> +
> +if [ "$FOUND" -eq 1 ]; then
> +	echo -e "FAIL\n\n"
> +	die "livepatch kselftest(s) failed"
> +fi
> +
> +
>  exit 0
> 
> ---
> base-commit: fc033cf25e612e840e545f8d5ad2edd6ba613ed5
> change-id: 20250101-ftrace-selftest-livepatch-161fb77dbed8
> 
> Best regards,
> -- 
> Filipe Xavier <felipeaggger@gmail.com>
> 

Thanks,
--
Joe

Em 07/01/2025 13:23, Joe Lawrence escreveu:

> On Thu, Jan 02, 2025 at 03:42:10PM -0300, Filipe Xavier wrote:
>> This new test makes sure that ftrace can trace a
>> function that was introduced by a livepatch.
>>
> Hi Filipe,
>
> Thanks for adding a test!
>
> Aside: another similar test could verify that the original function, in
> this case cmdline_proc_show(), can still be traced despite it being
> livepatched.  That may be non-intuitive but it demonstrates how the
> ftrace handler works.

Thanks for the review Joe!

I have fixed all points mentioned below,

and have a patch ready to submit.

Do you believe that this other similar test could be sent later,

or is it required in this patch?

>   
>> Signed-off-by: Filipe Xavier <felipeaggger@gmail.com>
>> ---
>>   tools/testing/selftests/livepatch/test-ftrace.sh | 37 ++++++++++++++++++++++++
>>   1 file changed, 37 insertions(+)
>>
>> diff --git a/tools/testing/selftests/livepatch/test-ftrace.sh b/tools/testing/selftests/livepatch/test-ftrace.sh
>> index fe14f248913acbec46fb6c0fec38a2fc84209d39..5f0d5308c88669e84210393ce7b8aa138b694ebd 100755
>> --- a/tools/testing/selftests/livepatch/test-ftrace.sh
>> +++ b/tools/testing/selftests/livepatch/test-ftrace.sh
>> @@ -61,4 +61,41 @@ livepatch: '$MOD_LIVEPATCH': unpatching complete
>>   % rmmod $MOD_LIVEPATCH"
>>   
>>   
>> +# - verify livepatch can load
>> +# - check traces if have a patched function
> nit: wording?  "check if traces have a patched function" ?
>
>> +# - unload livepatch and reset trace
>> +
>> +start_test "livepatch trace patched function and check that the live patch remains in effect"
> nit: wording?  "trace livepatched function and check ..." ?
>
>> +
>> +TRACE_FILE="$SYSFS_DEBUG_DIR/tracing/trace"
>> +FUNCTION_NAME="livepatch_cmdline_proc_show"
>> +
>> +load_lp $MOD_LIVEPATCH
>> +
>> +echo $FUNCTION_NAME > $SYSFS_DEBUG_DIR/tracing/set_ftrace_filter
>> +echo "function" > $SYSFS_DEBUG_DIR/tracing/current_tracer
>> +echo "" > $TRACE_FILE
> A few suggestions:
>
> - The tracing is also dependent on the 'tracing_on' file, so if it
>    happens to be turned off, the test will fail.
>
> - See functions.sh :: push_config() and pop_config() for an example of
>    saving the existing values rather than turning them all off at the end
>    of the test.
>
> - Nitpick: shellcheck suggests wrapping filenames in double quotations,
>    applicable in several places.
>
>> +
>> +if [[ "$(cat /proc/cmdline)" != "$MOD_LIVEPATCH: this has been live patched" ]] ; then
>> +	echo -e "FAIL\n\n"
>> +	die "livepatch kselftest(s) failed"
>> +fi
>> +
>> +grep -q $FUNCTION_NAME $TRACE_FILE
>> +FOUND=$?
>> +
>> +disable_lp $MOD_LIVEPATCH
>> +unload_lp $MOD_LIVEPATCH
>> +
>> +# Reset tracing
>> +echo "nop" > $SYSFS_DEBUG_DIR/tracing/current_tracer
>> +echo "" > $SYSFS_DEBUG_DIR/tracing/set_ftrace_filter
>> +echo "" > $TRACE_FILE
>> +
>> +if [ "$FOUND" -eq 1 ]; then
>> +	echo -e "FAIL\n\n"
>> +	die "livepatch kselftest(s) failed"
>> +fi
>> +
>> +
>>   exit 0
>>
>> ---
>> base-commit: fc033cf25e612e840e545f8d5ad2edd6ba613ed5
>> change-id: 20250101-ftrace-selftest-livepatch-161fb77dbed8
>>
>> Best regards,
>> -- 
>> Filipe Xavier <felipeaggger@gmail.com>
>>
> Thanks,
> --
> Joe
>

On 1/10/25 12:13, Filipe Xavier wrote:
> Em 07/01/2025 13:23, Joe Lawrence escreveu:
> 
>> On Thu, Jan 02, 2025 at 03:42:10PM -0300, Filipe Xavier wrote:
>>> This new test makes sure that ftrace can trace a
>>> function that was introduced by a livepatch.
>>>
>> Hi Filipe,
>>
>> Thanks for adding a test!
>>
>> Aside: another similar test could verify that the original function, in
>> this case cmdline_proc_show(), can still be traced despite it being
>> livepatched.  That may be non-intuitive but it demonstrates how the
>> ftrace handler works.
> 
> Thanks for the review Joe!
> 
> I have fixed all points mentioned below,
> 
> and have a patch ready to submit.
> 
> Do you believe that this other similar test could be sent later,
> 
> or is it required in this patch?
> 

The second test could be added later if you like, either way is fine w/me.

Thanks,

-- 
Joe