| Message ID | 20220318021314.3225240-1-davidgow@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | kunit: Support redirecting function calls | expand |
On Fri, 18 Mar 2022 10:13:12 +0800 David Gow <davidgow@google.com> wrote: > Does either (or both) of these features sound useful, and is this > sort-of API the right model? (Personally, I think there's a reasonable > scope for both.) Is anything obviously missing or wrong? Do the names, > descriptions etc. make any sense? Obviously I'm biased toward the ftrace solution ;-) -- Steve
On Fri, Mar 18, 2022 at 9:22 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Fri, 18 Mar 2022 10:13:12 +0800 > David Gow <davidgow@google.com> wrote: > > > Does either (or both) of these features sound useful, and is this > > sort-of API the right model? (Personally, I think there's a reasonable > > scope for both.) Is anything obviously missing or wrong? Do the names, > > descriptions etc. make any sense? > > Obviously I'm biased toward the ftrace solution ;-) Personally, I like providing both - as long as we can keep the interface the same. Ftrace is less visually invasive, but it is also less flexible in capabilities, and requires substantial work to support on new architectures.
+Steve Muckle - since I think this might affect things he is working on. On Thu, Mar 17, 2022 at 10:13 PM David Gow <davidgow@google.com> wrote: > > When writing tests, it'd often be very useful to be able to intercept > calls to a function in the code being tested and replace it with a > test-specific stub. This has always been an obviously missing piece of > KUnit, and the solutions always involve some tradeoffs with cleanliness, > performance, or impact on non-test code. See the folowing document for > some of the challenges: > https://kunit.dev/mocking.html > > This series consists of two prototype patches which add support for this > sort of redirection to KUnit tests: > > 1: static_stub: Any function which might want to be intercepted adds a > call to a macro which checks if a test has redirected calls to it, and > calls the corresponding replacement. > > 2: ftrace_stub: Functions are intercepted using ftrace and livepatch. > This doesn't require adding a new prologue to each function being > replaced, but does have more dependencies (which restricts it to a small > number of architectures, not including UML), and doesn't work well with > inline functions. > > The API for both implementations is very similar, so it should be easy > to migrate from one to the other if necessary. Both of these > implementations restrict the redirection to the test context: it is > automatically undone after the KUnit test completes, and does not affect > calls in other threads. If CONFIG_KUNIT is not enabled, there should be > no overhead in either implementation. > > Does either (or both) of these features sound useful, and is this > sort-of API the right model? (Personally, I think there's a reasonable > scope for both.) Is anything obviously missing or wrong? Do the names, > descriptions etc. make any sense? > > Note that these patches are definitely still at the "prototype" level, > and things like error-handling, documentation, and testing are still > pretty sparse. There is also quite a bit of room for optimisation. > These'll all be improved for v1 if the concept seems good. > > Cheers, > -- David > > Daniel Latypov (1): > kunit: expose ftrace-based API for stubbing out functions during tests > > David Gow (1): > kunit: Expose 'static stub' API to redirect functions > > include/kunit/ftrace_stub.h | 84 +++++++++++++++++ > include/kunit/static_stub.h | 106 +++++++++++++++++++++ > lib/kunit/Kconfig | 11 +++ > lib/kunit/Makefile | 5 + > lib/kunit/ftrace_stub.c | 138 ++++++++++++++++++++++++++++ > lib/kunit/kunit-example-test.c | 64 +++++++++++++ > lib/kunit/static_stub.c | 125 +++++++++++++++++++++++++ > lib/kunit/stubs_example.kunitconfig | 11 +++ > 8 files changed, 544 insertions(+) > create mode 100644 include/kunit/ftrace_stub.h > create mode 100644 include/kunit/static_stub.h > create mode 100644 lib/kunit/ftrace_stub.c > create mode 100644 lib/kunit/static_stub.c > create mode 100644 lib/kunit/stubs_example.kunitconfig > > -- > 2.35.1.894.gb6a874cedc-goog >
On 4/4/22 13:13, Brendan Higgins wrote: > On Fri, Mar 18, 2022 at 9:22 AM Steven Rostedt <rostedt@goodmis.org> wrote: >> >> On Fri, 18 Mar 2022 10:13:12 +0800 >> David Gow <davidgow@google.com> wrote: >> >>> Does either (or both) of these features sound useful, and is this >>> sort-of API the right model? (Personally, I think there's a reasonable >>> scope for both.) Is anything obviously missing or wrong? Do the names, >>> descriptions etc. make any sense? >> >> Obviously I'm biased toward the ftrace solution ;-) > > Personally, I like providing both - as long as we can keep the > interface the same. > > Ftrace is less visually invasive, but it is also less flexible in > capabilities, and requires substantial work to support on new > architectures. The general feature looks useful to me. I'm not sure the ftrace based API is worth it given it is only offering a visual improvement and has some drawbacks compared to the other implementation (won't work with inline functions, dependencies on other features). Livepatch is absent on arm64 which mostly rules it out for my purposes (Android Generic Kernel Image testing). cheers, Steve
+Joe Fradley who is also looking at KUnit with Android. On 4/15/22 14:43, Steve Muckle wrote: > On 4/4/22 13:13, Brendan Higgins wrote: >> On Fri, Mar 18, 2022 at 9:22 AM Steven Rostedt <rostedt@goodmis.org> >> wrote: >>> >>> On Fri, 18 Mar 2022 10:13:12 +0800 >>> David Gow <davidgow@google.com> wrote: >>> >>>> Does either (or both) of these features sound useful, and is this >>>> sort-of API the right model? (Personally, I think there's a reasonable >>>> scope for both.) Is anything obviously missing or wrong? Do the names, >>>> descriptions etc. make any sense? >>> >>> Obviously I'm biased toward the ftrace solution ;-) >> >> Personally, I like providing both - as long as we can keep the >> interface the same. >> >> Ftrace is less visually invasive, but it is also less flexible in >> capabilities, and requires substantial work to support on new >> architectures. > > The general feature looks useful to me. I'm not sure the ftrace based > API is worth it given it is only offering a visual improvement and has > some drawbacks compared to the other implementation (won't work with > inline functions, dependencies on other features). Livepatch is absent > on arm64 which mostly rules it out for my purposes (Android Generic > Kernel Image testing). > > cheers, > Steve