Re: [PATCH] selftests: livepatch: test if ftrace can trace a livepatched function

[Joe Lawrence <joe.lawrence@redhat.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