Message ID | 20201019202431.3472335-1-dlatypov@google.com (mailing list archive) |
---|---|
State | Not Applicable |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | wireguard: convert selftest/{counter,ratelimiter}.c to KUnit | expand |
Hi Daniel, Thanks for this patch. KUnit looks interesting. But I'm not totally sure what this would gain here, except more complicated infrastructure to handle. We're already running these tests in our CI on every single commit made to a variety of trees on a variety of architectures -- see https://www.wireguard.com/build-status/ -- runnable via `make -C tools/testing/selftests/wireguard/qemu -j$(nproc)`. It looks like this commit breaks that while making everything slightly more complex. Is there a good reason to switch over to this other than fad? From a development perspective, I don't see this as really helping with much. Jason On Mon, Oct 19, 2020 at 10:24 PM Daniel Latypov <dlatypov@google.com> wrote: > > These tests already focus on testing individual functions that can run > in a more minimal environment like KUnit. > > The primary motivation for this change it to make it faster and easier > to run these tests, and thus encourage the addition of more test cases. > > E.g. > Test timing after make mrproper: 47.418s building, 0.000s running > With an incremental build: 3.891s building, 0.000s running > > KUnit also provides a bit more structure, like tracking overall > pass/fail status and printing failure messages like > > # wg_packet_counter_test: EXPECTATION FAILED at drivers/net/wireguard/counter_test.c:32 > > Expected counter_validate(counter, (COUNTER_WINDOW_SIZE + 1)) == false, but > > Note: so we no longer need to track test_num in counter_test.c. > But deleting the /*1*/ test_num comments means git (with the default > threshold) no longer recognizes that the file was moved. > > Signed-off-by: Daniel Latypov <dlatypov@google.com> > Cc: Jason A. Donenfeld <Jason@zx2c4.com> > Cc: David Miller <davem@davemloft.net> > Cc: Brendan Higgins <brendanhiggins@google.com> > --- > drivers/net/Kconfig | 12 ++++ > .../{selftest/counter.c => counter_test.c} | 45 ++++++------ > drivers/net/wireguard/main.c | 3 +- > drivers/net/wireguard/queueing.h | 4 -- > drivers/net/wireguard/ratelimiter.c | 4 +- > .../ratelimiter.c => ratelimiter_test.c} | 68 +++++++++++-------- > drivers/net/wireguard/receive.c | 6 +- > 7 files changed, 80 insertions(+), 62 deletions(-) > rename drivers/net/wireguard/{selftest/counter.c => counter_test.c} (73%) > rename drivers/net/wireguard/{selftest/ratelimiter.c => ratelimiter_test.c} (85%) > > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig > index c3dbe64e628e..208ed162bcc0 100644 > --- a/drivers/net/Kconfig > +++ b/drivers/net/Kconfig > @@ -114,6 +114,18 @@ config WIREGUARD_DEBUG > > Say N here unless you know what you're doing. > > +config WIREGUARD_KUNIT_TEST > + tristate "KUnit tests for WireGuard" > + default KUNIT_ALL_TESTS > + depends on KUNIT && WIREGUARD > + help > + This enables KUnit tests for Wireguard. > + > + For more information on KUnit and unit tests in general please refer > + to the KUnit documentation in Documentation/dev-tools/kunit/. > + > + Say N here unless you know what you're doing. > + > config EQUALIZER > tristate "EQL (serial line load balancing) support" > help > diff --git a/drivers/net/wireguard/selftest/counter.c b/drivers/net/wireguard/counter_test.c > similarity index 73% > rename from drivers/net/wireguard/selftest/counter.c > rename to drivers/net/wireguard/counter_test.c > index ec3c156bf91b..167153fc249f 100644 > --- a/drivers/net/wireguard/selftest/counter.c > +++ b/drivers/net/wireguard/counter_test.c > @@ -3,32 +3,23 @@ > * Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved. > */ > > -#ifdef DEBUG > -bool __init wg_packet_counter_selftest(void) > +#include <kunit/test.h> > + > +static void wg_packet_counter_test(struct kunit *test) > { > struct noise_replay_counter *counter; > - unsigned int test_num = 0, i; > - bool success = true; > + unsigned int i; > > - counter = kmalloc(sizeof(*counter), GFP_KERNEL); > - if (unlikely(!counter)) { > - pr_err("nonce counter self-test malloc: FAIL\n"); > - return false; > - } > + counter = kunit_kmalloc(test, sizeof(*counter), GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, counter); > > #define T_INIT do { \ > memset(counter, 0, sizeof(*counter)); \ > spin_lock_init(&counter->lock); \ > } while (0) > #define T_LIM (COUNTER_WINDOW_SIZE + 1) > -#define T(n, v) do { \ > - ++test_num; \ > - if (counter_validate(counter, n) != (v)) { \ > - pr_err("nonce counter self-test %u: FAIL\n", \ > - test_num); \ > - success = false; \ > - } \ > - } while (0) > +#define T(n, v) \ > + KUNIT_EXPECT_EQ(test, counter_validate(counter, n), v) > > T_INIT; > /* 1 */ T(0, true); > @@ -102,10 +93,18 @@ bool __init wg_packet_counter_selftest(void) > #undef T > #undef T_LIM > #undef T_INIT > - > - if (success) > - pr_info("nonce counter self-tests: pass\n"); > - kfree(counter); > - return success; > } > -#endif > + > +static struct kunit_case wg_packet_counter_test_cases[] = { > + KUNIT_CASE(wg_packet_counter_test), > + {} > +}; > + > +static struct kunit_suite wg_packet_counter_test_suite = { > + .name = "wg_packet_counter", > + .test_cases = wg_packet_counter_test_cases, > +}; > + > +kunit_test_suites(&wg_packet_counter_test_suite); > + > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/net/wireguard/main.c b/drivers/net/wireguard/main.c > index 7a7d5f1a80fc..bfd3312d5133 100644 > --- a/drivers/net/wireguard/main.c > +++ b/drivers/net/wireguard/main.c > @@ -22,8 +22,7 @@ static int __init mod_init(void) > int ret; > > #ifdef DEBUG > - if (!wg_allowedips_selftest() || !wg_packet_counter_selftest() || > - !wg_ratelimiter_selftest()) > + if (!wg_allowedips_selftest()) > return -ENOTRECOVERABLE; > #endif > wg_noise_init(); > diff --git a/drivers/net/wireguard/queueing.h b/drivers/net/wireguard/queueing.h > index dfb674e03076..5d428ddf176f 100644 > --- a/drivers/net/wireguard/queueing.h > +++ b/drivers/net/wireguard/queueing.h > @@ -186,8 +186,4 @@ static inline void wg_queue_enqueue_per_peer_napi(struct sk_buff *skb, > wg_peer_put(peer); > } > > -#ifdef DEBUG > -bool wg_packet_counter_selftest(void); > -#endif > - > #endif /* _WG_QUEUEING_H */ > diff --git a/drivers/net/wireguard/ratelimiter.c b/drivers/net/wireguard/ratelimiter.c > index 3fedd1d21f5e..f7a7c48aee40 100644 > --- a/drivers/net/wireguard/ratelimiter.c > +++ b/drivers/net/wireguard/ratelimiter.c > @@ -220,4 +220,6 @@ void wg_ratelimiter_uninit(void) > mutex_unlock(&init_lock); > } > > -#include "selftest/ratelimiter.c" > +#if IS_ENABLED(CONFIG_WIREGUARD_KUNIT_TEST) > +#include "ratelimiter_test.c" > +#endif > diff --git a/drivers/net/wireguard/selftest/ratelimiter.c b/drivers/net/wireguard/ratelimiter_test.c > similarity index 85% > rename from drivers/net/wireguard/selftest/ratelimiter.c > rename to drivers/net/wireguard/ratelimiter_test.c > index 007cd4457c5f..a49f508cccb2 100644 > --- a/drivers/net/wireguard/selftest/ratelimiter.c > +++ b/drivers/net/wireguard/ratelimiter_test.c > @@ -3,8 +3,7 @@ > * Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved. > */ > > -#ifdef DEBUG > - > +#include <kunit/test.h> > #include <linux/jiffies.h> > > static const struct { > @@ -32,7 +31,7 @@ static __init unsigned int maximum_jiffies_at_index(int index) > > static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, > struct sk_buff *skb6, struct ipv6hdr *hdr6, > - int *test) > + int *test_num) > { > unsigned long loop_start_time; > int i; > @@ -51,7 +50,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, > if (wg_ratelimiter_allow(skb4, &init_net) != > expected_results[i].result) > return -EXFULL; > - ++(*test); > + ++(*test_num); > > hdr4->saddr = htonl(ntohl(hdr4->saddr) + i + 1); > if (time_is_before_jiffies(loop_start_time + > @@ -59,7 +58,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, > return -ETIMEDOUT; > if (!wg_ratelimiter_allow(skb4, &init_net)) > return -EXFULL; > - ++(*test); > + ++(*test_num); > > hdr4->saddr = htonl(ntohl(hdr4->saddr) - i - 1); > > @@ -72,7 +71,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, > if (wg_ratelimiter_allow(skb6, &init_net) != > expected_results[i].result) > return -EXFULL; > - ++(*test); > + ++(*test_num); > > hdr6->saddr.in6_u.u6_addr32[0] = > htonl(ntohl(hdr6->saddr.in6_u.u6_addr32[0]) + i + 1); > @@ -81,7 +80,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, > return -ETIMEDOUT; > if (!wg_ratelimiter_allow(skb6, &init_net)) > return -EXFULL; > - ++(*test); > + ++(*test_num); > > hdr6->saddr.in6_u.u6_addr32[0] = > htonl(ntohl(hdr6->saddr.in6_u.u6_addr32[0]) - i - 1); > @@ -95,7 +94,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, > } > > static __init int capacity_test(struct sk_buff *skb4, struct iphdr *hdr4, > - int *test) > + int *test_num) > { > int i; > > @@ -104,45 +103,45 @@ static __init int capacity_test(struct sk_buff *skb4, struct iphdr *hdr4, > > if (atomic_read(&total_entries)) > return -EXFULL; > - ++(*test); > + ++(*test_num); > > for (i = 0; i <= max_entries; ++i) { > hdr4->saddr = htonl(i); > if (wg_ratelimiter_allow(skb4, &init_net) != (i != max_entries)) > return -EXFULL; > - ++(*test); > + ++(*test_num); > } > return 0; > } > > -bool __init wg_ratelimiter_selftest(void) > +static void wg_ratelimiter_test(struct kunit *test) > { > enum { TRIALS_BEFORE_GIVING_UP = 5000 }; > bool success = false; > - int test = 0, trials; > + int test_num = 0, trials; > struct sk_buff *skb4, *skb6 = NULL; > struct iphdr *hdr4; > struct ipv6hdr *hdr6 = NULL; > > if (IS_ENABLED(CONFIG_KASAN) || IS_ENABLED(CONFIG_UBSAN)) > - return true; > + return; > > BUILD_BUG_ON(MSEC_PER_SEC % PACKETS_PER_SECOND != 0); > > if (wg_ratelimiter_init()) > goto out; > - ++test; > + ++test_num; > if (wg_ratelimiter_init()) { > wg_ratelimiter_uninit(); > goto out; > } > - ++test; > + ++test_num; > if (wg_ratelimiter_init()) { > wg_ratelimiter_uninit(); > wg_ratelimiter_uninit(); > goto out; > } > - ++test; > + ++test_num; > > skb4 = alloc_skb(sizeof(struct iphdr), GFP_KERNEL); > if (unlikely(!skb4)) > @@ -151,7 +150,7 @@ bool __init wg_ratelimiter_selftest(void) > hdr4 = (struct iphdr *)skb_put(skb4, sizeof(*hdr4)); > hdr4->saddr = htonl(8182); > skb_reset_network_header(skb4); > - ++test; > + ++test_num; > > #if IS_ENABLED(CONFIG_IPV6) > skb6 = alloc_skb(sizeof(struct ipv6hdr), GFP_KERNEL); > @@ -164,7 +163,7 @@ bool __init wg_ratelimiter_selftest(void) > hdr6->saddr.in6_u.u6_addr32[0] = htonl(1212); > hdr6->saddr.in6_u.u6_addr32[1] = htonl(289188); > skb_reset_network_header(skb6); > - ++test; > + ++test_num; > #endif > > for (trials = TRIALS_BEFORE_GIVING_UP;;) { > @@ -173,16 +172,16 @@ bool __init wg_ratelimiter_selftest(void) > ret = timings_test(skb4, hdr4, skb6, hdr6, &test_count); > if (ret == -ETIMEDOUT) { > if (!trials--) { > - test += test_count; > + test_num += test_count; > goto err; > } > msleep(500); > continue; > } else if (ret < 0) { > - test += test_count; > + test_num += test_count; > goto err; > } else { > - test += test_count; > + test_num += test_count; > break; > } > } > @@ -192,13 +191,13 @@ bool __init wg_ratelimiter_selftest(void) > > if (capacity_test(skb4, hdr4, &test_count) < 0) { > if (!trials--) { > - test += test_count; > + test_num += test_count; > goto err; > } > msleep(50); > continue; > } > - test += test_count; > + test_num += test_count; > break; > } > > @@ -216,11 +215,20 @@ bool __init wg_ratelimiter_selftest(void) > /* Uninit one extra time to check underflow detection. */ > wg_ratelimiter_uninit(); > out: > - if (success) > - pr_info("ratelimiter self-tests: pass\n"); > - else > - pr_err("ratelimiter self-test %d: FAIL\n", test); > - > - return success; > + if (!success) > + KUNIT_FAIL(test, "test #%d failed", test_num); > } > -#endif > + > +static struct kunit_case wg_ratelimiter_test_cases[] = { > + KUNIT_CASE(wg_ratelimiter_test), > + {} > +}; > + > +static struct kunit_suite wg_ratelimiter_test_suite = { > + .name = "wg_ratelimiter", > + .test_cases = wg_ratelimiter_test_cases, > +}; > + > +kunit_test_suites(&wg_ratelimiter_test_suite); > + > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c > index 2c9551ea6dc7..30d3d9685e8d 100644 > --- a/drivers/net/wireguard/receive.c > +++ b/drivers/net/wireguard/receive.c > @@ -336,8 +336,6 @@ static bool counter_validate(struct noise_replay_counter *counter, u64 their_cou > return ret; > } > > -#include "selftest/counter.c" > - > static void wg_packet_consume_data_done(struct wg_peer *peer, > struct sk_buff *skb, > struct endpoint *endpoint) > @@ -588,3 +586,7 @@ void wg_packet_receive(struct wg_device *wg, struct sk_buff *skb) > err: > dev_kfree_skb(skb); > } > + > +#if IS_ENABLED(CONFIG_WIREGUARD_KUNIT_TEST) > +#include "counter_test.c" > +#endif > > base-commit: 7cf726a59435301046250c42131554d9ccc566b8 > -- > 2.29.0.rc1.297.gfa9743e501-goog
On Mon, Oct 19, 2020 at 3:36 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > Hi Daniel, > > Thanks for this patch. KUnit looks interesting. But I'm not totally > sure what this would gain here, except more complicated infrastructure > to handle. We're already running these tests in our CI on every single > commit made to a variety of trees on a variety of architectures -- see > https://www.wireguard.com/build-status/ -- runnable via `make -C > tools/testing/selftests/wireguard/qemu -j$(nproc)`. It looks like this > commit breaks that while making everything slightly more complex. Is Thanks for the informative response and the pointer to the build-status/ page, I was unaware of its existence. Digging into that a bit deeper, I'd agree with you that this patch isn't worth it. Yes, this would stop these two tests from running under selftests, and thus in your CI. A not-too-long glance at the code made it seem like the specific code-under-test here was reasonably arch-independent, but yes, this would make it more annoying to test different arches. > there a good reason to switch over to this other than fad? From a > development perspective, I don't see this as really helping with much. In my mind, the breakdown is Pros: * more minimal environment * config file is 6 lines, instead of 87 * doesn't rely on a userspace or a custom init, etc. * slightly faster build times on my machine (with -j8) * the option to provide a bit more structure via its MACROS * but that's optional, can fall back to `if (success) KUNIT_FAIL("my message")` Cons: * separate set of tooling needed to run tests * needs to be then integrated into WireGuard's CI * not as mature, so it lacks integration via KernelCI, etc. * Brendan (CC'd) is working on this KernelCI integration in particular. * qemu/init.c has more features that KUnit currently lacks, like kmemleak checks * and others I'm not able to think of. WG's tooling is really nice, so these cons are much more apparent. I think a feature that might make this worth looking at again later on is if selftest modules could be written using KUnit. Commit c475c77d5b56 ("kunit: allow kunit tests to be loaded as a module") was necessary but not sufficient here. If that becomes possible, KUnit would mainly provide the boilerplate for tracking pass-fail, generating error messages, and the *option* of running the tests via either KUnit or Kselftest. Until that time, I'd agree that WG is better off as-is. Cheers, Daniel > > Jason > > On Mon, Oct 19, 2020 at 10:24 PM Daniel Latypov <dlatypov@google.com> wrote: > > > > These tests already focus on testing individual functions that can run > > in a more minimal environment like KUnit. > > > > The primary motivation for this change it to make it faster and easier > > to run these tests, and thus encourage the addition of more test cases. > > > > E.g. > > Test timing after make mrproper: 47.418s building, 0.000s running > > With an incremental build: 3.891s building, 0.000s running > > > > KUnit also provides a bit more structure, like tracking overall > > pass/fail status and printing failure messages like > > > # wg_packet_counter_test: EXPECTATION FAILED at drivers/net/wireguard/counter_test.c:32 > > > Expected counter_validate(counter, (COUNTER_WINDOW_SIZE + 1)) == false, but > > > > Note: so we no longer need to track test_num in counter_test.c. > > But deleting the /*1*/ test_num comments means git (with the default > > threshold) no longer recognizes that the file was moved. > > > > Signed-off-by: Daniel Latypov <dlatypov@google.com> > > Cc: Jason A. Donenfeld <Jason@zx2c4.com> > > Cc: David Miller <davem@davemloft.net> > > Cc: Brendan Higgins <brendanhiggins@google.com> > > --- > > drivers/net/Kconfig | 12 ++++ > > .../{selftest/counter.c => counter_test.c} | 45 ++++++------ > > drivers/net/wireguard/main.c | 3 +- > > drivers/net/wireguard/queueing.h | 4 -- > > drivers/net/wireguard/ratelimiter.c | 4 +- > > .../ratelimiter.c => ratelimiter_test.c} | 68 +++++++++++-------- > > drivers/net/wireguard/receive.c | 6 +- > > 7 files changed, 80 insertions(+), 62 deletions(-) > > rename drivers/net/wireguard/{selftest/counter.c => counter_test.c} (73%) > > rename drivers/net/wireguard/{selftest/ratelimiter.c => ratelimiter_test.c} (85%) > > > > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig > > index c3dbe64e628e..208ed162bcc0 100644 > > --- a/drivers/net/Kconfig > > +++ b/drivers/net/Kconfig > > @@ -114,6 +114,18 @@ config WIREGUARD_DEBUG > > > > Say N here unless you know what you're doing. > > > > +config WIREGUARD_KUNIT_TEST > > + tristate "KUnit tests for WireGuard" > > + default KUNIT_ALL_TESTS > > + depends on KUNIT && WIREGUARD > > + help > > + This enables KUnit tests for Wireguard. > > + > > + For more information on KUnit and unit tests in general please refer > > + to the KUnit documentation in Documentation/dev-tools/kunit/. > > + > > + Say N here unless you know what you're doing. > > + > > config EQUALIZER > > tristate "EQL (serial line load balancing) support" > > help > > diff --git a/drivers/net/wireguard/selftest/counter.c b/drivers/net/wireguard/counter_test.c > > similarity index 73% > > rename from drivers/net/wireguard/selftest/counter.c > > rename to drivers/net/wireguard/counter_test.c > > index ec3c156bf91b..167153fc249f 100644 > > --- a/drivers/net/wireguard/selftest/counter.c > > +++ b/drivers/net/wireguard/counter_test.c > > @@ -3,32 +3,23 @@ > > * Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved. > > */ > > > > -#ifdef DEBUG > > -bool __init wg_packet_counter_selftest(void) > > +#include <kunit/test.h> > > + > > +static void wg_packet_counter_test(struct kunit *test) > > { > > struct noise_replay_counter *counter; > > - unsigned int test_num = 0, i; > > - bool success = true; > > + unsigned int i; > > > > - counter = kmalloc(sizeof(*counter), GFP_KERNEL); > > - if (unlikely(!counter)) { > > - pr_err("nonce counter self-test malloc: FAIL\n"); > > - return false; > > - } > > + counter = kunit_kmalloc(test, sizeof(*counter), GFP_KERNEL); > > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, counter); > > > > #define T_INIT do { \ > > memset(counter, 0, sizeof(*counter)); \ > > spin_lock_init(&counter->lock); \ > > } while (0) > > #define T_LIM (COUNTER_WINDOW_SIZE + 1) > > -#define T(n, v) do { \ > > - ++test_num; \ > > - if (counter_validate(counter, n) != (v)) { \ > > - pr_err("nonce counter self-test %u: FAIL\n", \ > > - test_num); \ > > - success = false; \ > > - } \ > > - } while (0) > > +#define T(n, v) \ > > + KUNIT_EXPECT_EQ(test, counter_validate(counter, n), v) > > > > T_INIT; > > /* 1 */ T(0, true); > > @@ -102,10 +93,18 @@ bool __init wg_packet_counter_selftest(void) > > #undef T > > #undef T_LIM > > #undef T_INIT > > - > > - if (success) > > - pr_info("nonce counter self-tests: pass\n"); > > - kfree(counter); > > - return success; > > } > > -#endif > > + > > +static struct kunit_case wg_packet_counter_test_cases[] = { > > + KUNIT_CASE(wg_packet_counter_test), > > + {} > > +}; > > + > > +static struct kunit_suite wg_packet_counter_test_suite = { > > + .name = "wg_packet_counter", > > + .test_cases = wg_packet_counter_test_cases, > > +}; > > + > > +kunit_test_suites(&wg_packet_counter_test_suite); > > + > > +MODULE_LICENSE("GPL v2"); > > diff --git a/drivers/net/wireguard/main.c b/drivers/net/wireguard/main.c > > index 7a7d5f1a80fc..bfd3312d5133 100644 > > --- a/drivers/net/wireguard/main.c > > +++ b/drivers/net/wireguard/main.c > > @@ -22,8 +22,7 @@ static int __init mod_init(void) > > int ret; > > > > #ifdef DEBUG > > - if (!wg_allowedips_selftest() || !wg_packet_counter_selftest() || > > - !wg_ratelimiter_selftest()) > > + if (!wg_allowedips_selftest()) > > return -ENOTRECOVERABLE; > > #endif > > wg_noise_init(); > > diff --git a/drivers/net/wireguard/queueing.h b/drivers/net/wireguard/queueing.h > > index dfb674e03076..5d428ddf176f 100644 > > --- a/drivers/net/wireguard/queueing.h > > +++ b/drivers/net/wireguard/queueing.h > > @@ -186,8 +186,4 @@ static inline void wg_queue_enqueue_per_peer_napi(struct sk_buff *skb, > > wg_peer_put(peer); > > } > > > > -#ifdef DEBUG > > -bool wg_packet_counter_selftest(void); > > -#endif > > - > > #endif /* _WG_QUEUEING_H */ > > diff --git a/drivers/net/wireguard/ratelimiter.c b/drivers/net/wireguard/ratelimiter.c > > index 3fedd1d21f5e..f7a7c48aee40 100644 > > --- a/drivers/net/wireguard/ratelimiter.c > > +++ b/drivers/net/wireguard/ratelimiter.c > > @@ -220,4 +220,6 @@ void wg_ratelimiter_uninit(void) > > mutex_unlock(&init_lock); > > } > > > > -#include "selftest/ratelimiter.c" > > +#if IS_ENABLED(CONFIG_WIREGUARD_KUNIT_TEST) > > +#include "ratelimiter_test.c" > > +#endif > > diff --git a/drivers/net/wireguard/selftest/ratelimiter.c b/drivers/net/wireguard/ratelimiter_test.c > > similarity index 85% > > rename from drivers/net/wireguard/selftest/ratelimiter.c > > rename to drivers/net/wireguard/ratelimiter_test.c > > index 007cd4457c5f..a49f508cccb2 100644 > > --- a/drivers/net/wireguard/selftest/ratelimiter.c > > +++ b/drivers/net/wireguard/ratelimiter_test.c > > @@ -3,8 +3,7 @@ > > * Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved. > > */ > > > > -#ifdef DEBUG > > - > > +#include <kunit/test.h> > > #include <linux/jiffies.h> > > > > static const struct { > > @@ -32,7 +31,7 @@ static __init unsigned int maximum_jiffies_at_index(int index) > > > > static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, > > struct sk_buff *skb6, struct ipv6hdr *hdr6, > > - int *test) > > + int *test_num) > > { > > unsigned long loop_start_time; > > int i; > > @@ -51,7 +50,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, > > if (wg_ratelimiter_allow(skb4, &init_net) != > > expected_results[i].result) > > return -EXFULL; > > - ++(*test); > > + ++(*test_num); > > > > hdr4->saddr = htonl(ntohl(hdr4->saddr) + i + 1); > > if (time_is_before_jiffies(loop_start_time + > > @@ -59,7 +58,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, > > return -ETIMEDOUT; > > if (!wg_ratelimiter_allow(skb4, &init_net)) > > return -EXFULL; > > - ++(*test); > > + ++(*test_num); > > > > hdr4->saddr = htonl(ntohl(hdr4->saddr) - i - 1); > > > > @@ -72,7 +71,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, > > if (wg_ratelimiter_allow(skb6, &init_net) != > > expected_results[i].result) > > return -EXFULL; > > - ++(*test); > > + ++(*test_num); > > > > hdr6->saddr.in6_u.u6_addr32[0] = > > htonl(ntohl(hdr6->saddr.in6_u.u6_addr32[0]) + i + 1); > > @@ -81,7 +80,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, > > return -ETIMEDOUT; > > if (!wg_ratelimiter_allow(skb6, &init_net)) > > return -EXFULL; > > - ++(*test); > > + ++(*test_num); > > > > hdr6->saddr.in6_u.u6_addr32[0] = > > htonl(ntohl(hdr6->saddr.in6_u.u6_addr32[0]) - i - 1); > > @@ -95,7 +94,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, > > } > > > > static __init int capacity_test(struct sk_buff *skb4, struct iphdr *hdr4, > > - int *test) > > + int *test_num) > > { > > int i; > > > > @@ -104,45 +103,45 @@ static __init int capacity_test(struct sk_buff *skb4, struct iphdr *hdr4, > > > > if (atomic_read(&total_entries)) > > return -EXFULL; > > - ++(*test); > > + ++(*test_num); > > > > for (i = 0; i <= max_entries; ++i) { > > hdr4->saddr = htonl(i); > > if (wg_ratelimiter_allow(skb4, &init_net) != (i != max_entries)) > > return -EXFULL; > > - ++(*test); > > + ++(*test_num); > > } > > return 0; > > } > > > > -bool __init wg_ratelimiter_selftest(void) > > +static void wg_ratelimiter_test(struct kunit *test) > > { > > enum { TRIALS_BEFORE_GIVING_UP = 5000 }; > > bool success = false; > > - int test = 0, trials; > > + int test_num = 0, trials; > > struct sk_buff *skb4, *skb6 = NULL; > > struct iphdr *hdr4; > > struct ipv6hdr *hdr6 = NULL; > > > > if (IS_ENABLED(CONFIG_KASAN) || IS_ENABLED(CONFIG_UBSAN)) > > - return true; > > + return; > > > > BUILD_BUG_ON(MSEC_PER_SEC % PACKETS_PER_SECOND != 0); > > > > if (wg_ratelimiter_init()) > > goto out; > > - ++test; > > + ++test_num; > > if (wg_ratelimiter_init()) { > > wg_ratelimiter_uninit(); > > goto out; > > } > > - ++test; > > + ++test_num; > > if (wg_ratelimiter_init()) { > > wg_ratelimiter_uninit(); > > wg_ratelimiter_uninit(); > > goto out; > > } > > - ++test; > > + ++test_num; > > > > skb4 = alloc_skb(sizeof(struct iphdr), GFP_KERNEL); > > if (unlikely(!skb4)) > > @@ -151,7 +150,7 @@ bool __init wg_ratelimiter_selftest(void) > > hdr4 = (struct iphdr *)skb_put(skb4, sizeof(*hdr4)); > > hdr4->saddr = htonl(8182); > > skb_reset_network_header(skb4); > > - ++test; > > + ++test_num; > > > > #if IS_ENABLED(CONFIG_IPV6) > > skb6 = alloc_skb(sizeof(struct ipv6hdr), GFP_KERNEL); > > @@ -164,7 +163,7 @@ bool __init wg_ratelimiter_selftest(void) > > hdr6->saddr.in6_u.u6_addr32[0] = htonl(1212); > > hdr6->saddr.in6_u.u6_addr32[1] = htonl(289188); > > skb_reset_network_header(skb6); > > - ++test; > > + ++test_num; > > #endif > > > > for (trials = TRIALS_BEFORE_GIVING_UP;;) { > > @@ -173,16 +172,16 @@ bool __init wg_ratelimiter_selftest(void) > > ret = timings_test(skb4, hdr4, skb6, hdr6, &test_count); > > if (ret == -ETIMEDOUT) { > > if (!trials--) { > > - test += test_count; > > + test_num += test_count; > > goto err; > > } > > msleep(500); > > continue; > > } else if (ret < 0) { > > - test += test_count; > > + test_num += test_count; > > goto err; > > } else { > > - test += test_count; > > + test_num += test_count; > > break; > > } > > } > > @@ -192,13 +191,13 @@ bool __init wg_ratelimiter_selftest(void) > > > > if (capacity_test(skb4, hdr4, &test_count) < 0) { > > if (!trials--) { > > - test += test_count; > > + test_num += test_count; > > goto err; > > } > > msleep(50); > > continue; > > } > > - test += test_count; > > + test_num += test_count; > > break; > > } > > > > @@ -216,11 +215,20 @@ bool __init wg_ratelimiter_selftest(void) > > /* Uninit one extra time to check underflow detection. */ > > wg_ratelimiter_uninit(); > > out: > > - if (success) > > - pr_info("ratelimiter self-tests: pass\n"); > > - else > > - pr_err("ratelimiter self-test %d: FAIL\n", test); > > - > > - return success; > > + if (!success) > > + KUNIT_FAIL(test, "test #%d failed", test_num); > > } > > -#endif > > + > > +static struct kunit_case wg_ratelimiter_test_cases[] = { > > + KUNIT_CASE(wg_ratelimiter_test), > > + {} > > +}; > > + > > +static struct kunit_suite wg_ratelimiter_test_suite = { > > + .name = "wg_ratelimiter", > > + .test_cases = wg_ratelimiter_test_cases, > > +}; > > + > > +kunit_test_suites(&wg_ratelimiter_test_suite); > > + > > +MODULE_LICENSE("GPL v2"); > > diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c > > index 2c9551ea6dc7..30d3d9685e8d 100644 > > --- a/drivers/net/wireguard/receive.c > > +++ b/drivers/net/wireguard/receive.c > > @@ -336,8 +336,6 @@ static bool counter_validate(struct noise_replay_counter *counter, u64 their_cou > > return ret; > > } > > > > -#include "selftest/counter.c" > > - > > static void wg_packet_consume_data_done(struct wg_peer *peer, > > struct sk_buff *skb, > > struct endpoint *endpoint) > > @@ -588,3 +586,7 @@ void wg_packet_receive(struct wg_device *wg, struct sk_buff *skb) > > err: > > dev_kfree_skb(skb); > > } > > + > > +#if IS_ENABLED(CONFIG_WIREGUARD_KUNIT_TEST) > > +#include "counter_test.c" > > +#endif > > > > base-commit: 7cf726a59435301046250c42131554d9ccc566b8 > > -- > > 2.29.0.rc1.297.gfa9743e501-goog
Hi Daniel, Thank you for the patch! Yet something to improve: [auto build test ERROR on 7cf726a59435301046250c42131554d9ccc566b8] url: https://github.com/0day-ci/linux/commits/Daniel-Latypov/wireguard-convert-selftest-counter-ratelimiter-c-to-KUnit/20201020-042650 base: 7cf726a59435301046250c42131554d9ccc566b8 config: x86_64-allmodconfig (attached as .config) compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/7a0f82af0af9735a7f20ef9e291e704aff218e8f git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Daniel-Latypov/wireguard-convert-selftest-counter-ratelimiter-c-to-KUnit/20201020-042650 git checkout 7a0f82af0af9735a7f20ef9e291e704aff218e8f # save the attached .config to linux build tree make W=1 ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): ld: drivers/net/wireguard/receive.o: in function `kunit_test_suites_init': >> receive.c:(.init.text+0x0): multiple definition of `init_module'; drivers/net/wireguard/main.o:main.c:(.init.text+0x0): first defined here ld: drivers/net/wireguard/receive.o: in function `kunit_test_suites_exit': >> receive.c:(.exit.text+0x0): multiple definition of `cleanup_module'; drivers/net/wireguard/main.o:main.c:(.exit.text+0x0): first defined here ld: drivers/net/wireguard/ratelimiter.o: in function `kunit_test_suites_init': ratelimiter.c:(.init.text+0x0): multiple definition of `init_module'; drivers/net/wireguard/main.o:main.c:(.init.text+0x0): first defined here ld: drivers/net/wireguard/ratelimiter.o: in function `kunit_test_suites_exit': ratelimiter.c:(.exit.text+0x0): multiple definition of `cleanup_module'; drivers/net/wireguard/main.o:main.c:(.exit.text+0x0): first defined here --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Daniel,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on 7cf726a59435301046250c42131554d9ccc566b8]
url: https://github.com/0day-ci/linux/commits/Daniel-Latypov/wireguard-convert-selftest-counter-ratelimiter-c-to-KUnit/20201020-042650
base: 7cf726a59435301046250c42131554d9ccc566b8
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/7a0f82af0af9735a7f20ef9e291e704aff218e8f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Daniel-Latypov/wireguard-convert-selftest-counter-ratelimiter-c-to-KUnit/20201020-042650
git checkout 7a0f82af0af9735a7f20ef9e291e704aff218e8f
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/net/wireguard/counter_test.c:84:2: note: in expansion of macro 'T'
84 | T(COUNTER_WINDOW_SIZE + 1, true);
| ^
include/linux/minmax.h:18:28: warning: comparison of distinct pointer types lacks a cast
18 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
| ^~
include/kunit/test.h:748:9: note: in expansion of macro '__typecheck'
748 | ((void)__typecheck(__left, __right)); \
| ^~~~~~~~~~~
include/kunit/test.h:772:2: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
772 | KUNIT_BASE_BINARY_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:861:2: note: in expansion of macro 'KUNIT_BASE_EQ_MSG_ASSERTION'
861 | KUNIT_BASE_EQ_MSG_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:871:2: note: in expansion of macro 'KUNIT_BINARY_EQ_MSG_ASSERTION'
871 | KUNIT_BINARY_EQ_MSG_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:1234:2: note: in expansion of macro 'KUNIT_BINARY_EQ_ASSERTION'
1234 | KUNIT_BINARY_EQ_ASSERTION(test, KUNIT_EXPECTATION, left, right)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wireguard/counter_test.c:22:3: note: in expansion of macro 'KUNIT_EXPECT_EQ'
22 | KUNIT_EXPECT_EQ(test, counter_validate(counter, n), v)
| ^~~~~~~~~~~~~~~
drivers/net/wireguard/counter_test.c:85:2: note: in expansion of macro 'T'
85 | T(0, false);
| ^
include/linux/minmax.h:18:28: warning: comparison of distinct pointer types lacks a cast
18 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
| ^~
include/kunit/test.h:748:9: note: in expansion of macro '__typecheck'
748 | ((void)__typecheck(__left, __right)); \
| ^~~~~~~~~~~
include/kunit/test.h:772:2: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
772 | KUNIT_BASE_BINARY_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:861:2: note: in expansion of macro 'KUNIT_BASE_EQ_MSG_ASSERTION'
861 | KUNIT_BASE_EQ_MSG_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:871:2: note: in expansion of macro 'KUNIT_BINARY_EQ_MSG_ASSERTION'
871 | KUNIT_BINARY_EQ_MSG_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:1234:2: note: in expansion of macro 'KUNIT_BINARY_EQ_ASSERTION'
1234 | KUNIT_BINARY_EQ_ASSERTION(test, KUNIT_EXPECTATION, left, right)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wireguard/counter_test.c:22:3: note: in expansion of macro 'KUNIT_EXPECT_EQ'
22 | KUNIT_EXPECT_EQ(test, counter_validate(counter, n), v)
| ^~~~~~~~~~~~~~~
drivers/net/wireguard/counter_test.c:89:3: note: in expansion of macro 'T'
89 | T(i, true);
| ^
include/linux/minmax.h:18:28: warning: comparison of distinct pointer types lacks a cast
18 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
| ^~
include/kunit/test.h:748:9: note: in expansion of macro '__typecheck'
748 | ((void)__typecheck(__left, __right)); \
| ^~~~~~~~~~~
include/kunit/test.h:772:2: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
772 | KUNIT_BASE_BINARY_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:861:2: note: in expansion of macro 'KUNIT_BASE_EQ_MSG_ASSERTION'
861 | KUNIT_BASE_EQ_MSG_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:871:2: note: in expansion of macro 'KUNIT_BINARY_EQ_MSG_ASSERTION'
871 | KUNIT_BINARY_EQ_MSG_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:1234:2: note: in expansion of macro 'KUNIT_BINARY_EQ_ASSERTION'
1234 | KUNIT_BINARY_EQ_ASSERTION(test, KUNIT_EXPECTATION, left, right)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wireguard/counter_test.c:22:3: note: in expansion of macro 'KUNIT_EXPECT_EQ'
22 | KUNIT_EXPECT_EQ(test, counter_validate(counter, n), v)
| ^~~~~~~~~~~~~~~
drivers/net/wireguard/counter_test.c:90:2: note: in expansion of macro 'T'
90 | T(0, true);
| ^
include/linux/minmax.h:18:28: warning: comparison of distinct pointer types lacks a cast
18 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
| ^~
include/kunit/test.h:748:9: note: in expansion of macro '__typecheck'
748 | ((void)__typecheck(__left, __right)); \
| ^~~~~~~~~~~
include/kunit/test.h:772:2: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
772 | KUNIT_BASE_BINARY_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:861:2: note: in expansion of macro 'KUNIT_BASE_EQ_MSG_ASSERTION'
861 | KUNIT_BASE_EQ_MSG_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:871:2: note: in expansion of macro 'KUNIT_BINARY_EQ_MSG_ASSERTION'
871 | KUNIT_BINARY_EQ_MSG_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:1234:2: note: in expansion of macro 'KUNIT_BINARY_EQ_ASSERTION'
1234 | KUNIT_BINARY_EQ_ASSERTION(test, KUNIT_EXPECTATION, left, right)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wireguard/counter_test.c:22:3: note: in expansion of macro 'KUNIT_EXPECT_EQ'
22 | KUNIT_EXPECT_EQ(test, counter_validate(counter, n), v)
| ^~~~~~~~~~~~~~~
drivers/net/wireguard/counter_test.c:91:2: note: in expansion of macro 'T'
91 | T(COUNTER_WINDOW_SIZE + 1, true);
| ^
In file included from drivers/net/wireguard/receive.c:591:
>> drivers/net/wireguard/counter_test.c:96:1: warning: the frame size of 3224 bytes is larger than 2048 bytes [-Wframe-larger-than=]
96 | }
| ^
vim +96 drivers/net/wireguard/counter_test.c
7a0f82af0af973 drivers/net/wireguard/counter_test.c Daniel Latypov 2020-10-19 7
7a0f82af0af973 drivers/net/wireguard/counter_test.c Daniel Latypov 2020-10-19 8 static void wg_packet_counter_test(struct kunit *test)
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 9 {
a9e90d9931f3a4 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2020-05-19 10 struct noise_replay_counter *counter;
7a0f82af0af973 drivers/net/wireguard/counter_test.c Daniel Latypov 2020-10-19 11 unsigned int i;
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 12
7a0f82af0af973 drivers/net/wireguard/counter_test.c Daniel Latypov 2020-10-19 13 counter = kunit_kmalloc(test, sizeof(*counter), GFP_KERNEL);
7a0f82af0af973 drivers/net/wireguard/counter_test.c Daniel Latypov 2020-10-19 14 KUNIT_ASSERT_NOT_ERR_OR_NULL(test, counter);
a9e90d9931f3a4 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2020-05-19 15
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 16 #define T_INIT do { \
a9e90d9931f3a4 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2020-05-19 17 memset(counter, 0, sizeof(*counter)); \
a9e90d9931f3a4 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2020-05-19 18 spin_lock_init(&counter->lock); \
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 19 } while (0)
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 20 #define T_LIM (COUNTER_WINDOW_SIZE + 1)
7a0f82af0af973 drivers/net/wireguard/counter_test.c Daniel Latypov 2020-10-19 21 #define T(n, v) \
7a0f82af0af973 drivers/net/wireguard/counter_test.c Daniel Latypov 2020-10-19 @22 KUNIT_EXPECT_EQ(test, counter_validate(counter, n), v)
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 23
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 24 T_INIT;
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 25 /* 1 */ T(0, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 26 /* 2 */ T(1, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 27 /* 3 */ T(1, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 28 /* 4 */ T(9, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 29 /* 5 */ T(8, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 30 /* 6 */ T(7, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 31 /* 7 */ T(7, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 32 /* 8 */ T(T_LIM, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 33 /* 9 */ T(T_LIM - 1, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 34 /* 10 */ T(T_LIM - 1, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 35 /* 11 */ T(T_LIM - 2, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 36 /* 12 */ T(2, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 37 /* 13 */ T(2, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 38 /* 14 */ T(T_LIM + 16, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 39 /* 15 */ T(3, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 40 /* 16 */ T(T_LIM + 16, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 41 /* 17 */ T(T_LIM * 4, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 42 /* 18 */ T(T_LIM * 4 - (T_LIM - 1), true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 43 /* 19 */ T(10, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 44 /* 20 */ T(T_LIM * 4 - T_LIM, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 45 /* 21 */ T(T_LIM * 4 - (T_LIM + 1), false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 46 /* 22 */ T(T_LIM * 4 - (T_LIM - 2), true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 47 /* 23 */ T(T_LIM * 4 + 1 - T_LIM, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 48 /* 24 */ T(0, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 49 /* 25 */ T(REJECT_AFTER_MESSAGES, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 50 /* 26 */ T(REJECT_AFTER_MESSAGES - 1, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 51 /* 27 */ T(REJECT_AFTER_MESSAGES, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 52 /* 28 */ T(REJECT_AFTER_MESSAGES - 1, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 53 /* 29 */ T(REJECT_AFTER_MESSAGES - 2, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 54 /* 30 */ T(REJECT_AFTER_MESSAGES + 1, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 55 /* 31 */ T(REJECT_AFTER_MESSAGES + 2, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 56 /* 32 */ T(REJECT_AFTER_MESSAGES - 2, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 57 /* 33 */ T(REJECT_AFTER_MESSAGES - 3, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 58 /* 34 */ T(0, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 59
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 60 T_INIT;
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 61 for (i = 1; i <= COUNTER_WINDOW_SIZE; ++i)
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 62 T(i, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 63 T(0, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 64 T(0, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 65
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 66 T_INIT;
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 67 for (i = 2; i <= COUNTER_WINDOW_SIZE + 1; ++i)
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 68 T(i, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 69 T(1, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 70 T(0, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 71
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 72 T_INIT;
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 73 for (i = COUNTER_WINDOW_SIZE + 1; i-- > 0;)
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 74 T(i, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 75
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 76 T_INIT;
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 77 for (i = COUNTER_WINDOW_SIZE + 2; i-- > 1;)
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 78 T(i, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 79 T(0, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 80
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 81 T_INIT;
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 82 for (i = COUNTER_WINDOW_SIZE + 1; i-- > 1;)
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 83 T(i, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 84 T(COUNTER_WINDOW_SIZE + 1, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 85 T(0, false);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 86
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 87 T_INIT;
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 88 for (i = COUNTER_WINDOW_SIZE + 1; i-- > 1;)
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 89 T(i, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 90 T(0, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 91 T(COUNTER_WINDOW_SIZE + 1, true);
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 92
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 93 #undef T
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 94 #undef T_LIM
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 95 #undef T_INIT
e7096c131e5161 drivers/net/wireguard/selftest/counter.c Jason A. Donenfeld 2019-12-09 @96 }
7a0f82af0af973 drivers/net/wireguard/counter_test.c Daniel Latypov 2020-10-19 97
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index c3dbe64e628e..208ed162bcc0 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -114,6 +114,18 @@ config WIREGUARD_DEBUG Say N here unless you know what you're doing. +config WIREGUARD_KUNIT_TEST + tristate "KUnit tests for WireGuard" + default KUNIT_ALL_TESTS + depends on KUNIT && WIREGUARD + help + This enables KUnit tests for Wireguard. + + For more information on KUnit and unit tests in general please refer + to the KUnit documentation in Documentation/dev-tools/kunit/. + + Say N here unless you know what you're doing. + config EQUALIZER tristate "EQL (serial line load balancing) support" help diff --git a/drivers/net/wireguard/selftest/counter.c b/drivers/net/wireguard/counter_test.c similarity index 73% rename from drivers/net/wireguard/selftest/counter.c rename to drivers/net/wireguard/counter_test.c index ec3c156bf91b..167153fc249f 100644 --- a/drivers/net/wireguard/selftest/counter.c +++ b/drivers/net/wireguard/counter_test.c @@ -3,32 +3,23 @@ * Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved. */ -#ifdef DEBUG -bool __init wg_packet_counter_selftest(void) +#include <kunit/test.h> + +static void wg_packet_counter_test(struct kunit *test) { struct noise_replay_counter *counter; - unsigned int test_num = 0, i; - bool success = true; + unsigned int i; - counter = kmalloc(sizeof(*counter), GFP_KERNEL); - if (unlikely(!counter)) { - pr_err("nonce counter self-test malloc: FAIL\n"); - return false; - } + counter = kunit_kmalloc(test, sizeof(*counter), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, counter); #define T_INIT do { \ memset(counter, 0, sizeof(*counter)); \ spin_lock_init(&counter->lock); \ } while (0) #define T_LIM (COUNTER_WINDOW_SIZE + 1) -#define T(n, v) do { \ - ++test_num; \ - if (counter_validate(counter, n) != (v)) { \ - pr_err("nonce counter self-test %u: FAIL\n", \ - test_num); \ - success = false; \ - } \ - } while (0) +#define T(n, v) \ + KUNIT_EXPECT_EQ(test, counter_validate(counter, n), v) T_INIT; /* 1 */ T(0, true); @@ -102,10 +93,18 @@ bool __init wg_packet_counter_selftest(void) #undef T #undef T_LIM #undef T_INIT - - if (success) - pr_info("nonce counter self-tests: pass\n"); - kfree(counter); - return success; } -#endif + +static struct kunit_case wg_packet_counter_test_cases[] = { + KUNIT_CASE(wg_packet_counter_test), + {} +}; + +static struct kunit_suite wg_packet_counter_test_suite = { + .name = "wg_packet_counter", + .test_cases = wg_packet_counter_test_cases, +}; + +kunit_test_suites(&wg_packet_counter_test_suite); + +MODULE_LICENSE("GPL v2"); diff --git a/drivers/net/wireguard/main.c b/drivers/net/wireguard/main.c index 7a7d5f1a80fc..bfd3312d5133 100644 --- a/drivers/net/wireguard/main.c +++ b/drivers/net/wireguard/main.c @@ -22,8 +22,7 @@ static int __init mod_init(void) int ret; #ifdef DEBUG - if (!wg_allowedips_selftest() || !wg_packet_counter_selftest() || - !wg_ratelimiter_selftest()) + if (!wg_allowedips_selftest()) return -ENOTRECOVERABLE; #endif wg_noise_init(); diff --git a/drivers/net/wireguard/queueing.h b/drivers/net/wireguard/queueing.h index dfb674e03076..5d428ddf176f 100644 --- a/drivers/net/wireguard/queueing.h +++ b/drivers/net/wireguard/queueing.h @@ -186,8 +186,4 @@ static inline void wg_queue_enqueue_per_peer_napi(struct sk_buff *skb, wg_peer_put(peer); } -#ifdef DEBUG -bool wg_packet_counter_selftest(void); -#endif - #endif /* _WG_QUEUEING_H */ diff --git a/drivers/net/wireguard/ratelimiter.c b/drivers/net/wireguard/ratelimiter.c index 3fedd1d21f5e..f7a7c48aee40 100644 --- a/drivers/net/wireguard/ratelimiter.c +++ b/drivers/net/wireguard/ratelimiter.c @@ -220,4 +220,6 @@ void wg_ratelimiter_uninit(void) mutex_unlock(&init_lock); } -#include "selftest/ratelimiter.c" +#if IS_ENABLED(CONFIG_WIREGUARD_KUNIT_TEST) +#include "ratelimiter_test.c" +#endif diff --git a/drivers/net/wireguard/selftest/ratelimiter.c b/drivers/net/wireguard/ratelimiter_test.c similarity index 85% rename from drivers/net/wireguard/selftest/ratelimiter.c rename to drivers/net/wireguard/ratelimiter_test.c index 007cd4457c5f..a49f508cccb2 100644 --- a/drivers/net/wireguard/selftest/ratelimiter.c +++ b/drivers/net/wireguard/ratelimiter_test.c @@ -3,8 +3,7 @@ * Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved. */ -#ifdef DEBUG - +#include <kunit/test.h> #include <linux/jiffies.h> static const struct { @@ -32,7 +31,7 @@ static __init unsigned int maximum_jiffies_at_index(int index) static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, struct sk_buff *skb6, struct ipv6hdr *hdr6, - int *test) + int *test_num) { unsigned long loop_start_time; int i; @@ -51,7 +50,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, if (wg_ratelimiter_allow(skb4, &init_net) != expected_results[i].result) return -EXFULL; - ++(*test); + ++(*test_num); hdr4->saddr = htonl(ntohl(hdr4->saddr) + i + 1); if (time_is_before_jiffies(loop_start_time + @@ -59,7 +58,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, return -ETIMEDOUT; if (!wg_ratelimiter_allow(skb4, &init_net)) return -EXFULL; - ++(*test); + ++(*test_num); hdr4->saddr = htonl(ntohl(hdr4->saddr) - i - 1); @@ -72,7 +71,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, if (wg_ratelimiter_allow(skb6, &init_net) != expected_results[i].result) return -EXFULL; - ++(*test); + ++(*test_num); hdr6->saddr.in6_u.u6_addr32[0] = htonl(ntohl(hdr6->saddr.in6_u.u6_addr32[0]) + i + 1); @@ -81,7 +80,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, return -ETIMEDOUT; if (!wg_ratelimiter_allow(skb6, &init_net)) return -EXFULL; - ++(*test); + ++(*test_num); hdr6->saddr.in6_u.u6_addr32[0] = htonl(ntohl(hdr6->saddr.in6_u.u6_addr32[0]) - i - 1); @@ -95,7 +94,7 @@ static __init int timings_test(struct sk_buff *skb4, struct iphdr *hdr4, } static __init int capacity_test(struct sk_buff *skb4, struct iphdr *hdr4, - int *test) + int *test_num) { int i; @@ -104,45 +103,45 @@ static __init int capacity_test(struct sk_buff *skb4, struct iphdr *hdr4, if (atomic_read(&total_entries)) return -EXFULL; - ++(*test); + ++(*test_num); for (i = 0; i <= max_entries; ++i) { hdr4->saddr = htonl(i); if (wg_ratelimiter_allow(skb4, &init_net) != (i != max_entries)) return -EXFULL; - ++(*test); + ++(*test_num); } return 0; } -bool __init wg_ratelimiter_selftest(void) +static void wg_ratelimiter_test(struct kunit *test) { enum { TRIALS_BEFORE_GIVING_UP = 5000 }; bool success = false; - int test = 0, trials; + int test_num = 0, trials; struct sk_buff *skb4, *skb6 = NULL; struct iphdr *hdr4; struct ipv6hdr *hdr6 = NULL; if (IS_ENABLED(CONFIG_KASAN) || IS_ENABLED(CONFIG_UBSAN)) - return true; + return; BUILD_BUG_ON(MSEC_PER_SEC % PACKETS_PER_SECOND != 0); if (wg_ratelimiter_init()) goto out; - ++test; + ++test_num; if (wg_ratelimiter_init()) { wg_ratelimiter_uninit(); goto out; } - ++test; + ++test_num; if (wg_ratelimiter_init()) { wg_ratelimiter_uninit(); wg_ratelimiter_uninit(); goto out; } - ++test; + ++test_num; skb4 = alloc_skb(sizeof(struct iphdr), GFP_KERNEL); if (unlikely(!skb4)) @@ -151,7 +150,7 @@ bool __init wg_ratelimiter_selftest(void) hdr4 = (struct iphdr *)skb_put(skb4, sizeof(*hdr4)); hdr4->saddr = htonl(8182); skb_reset_network_header(skb4); - ++test; + ++test_num; #if IS_ENABLED(CONFIG_IPV6) skb6 = alloc_skb(sizeof(struct ipv6hdr), GFP_KERNEL); @@ -164,7 +163,7 @@ bool __init wg_ratelimiter_selftest(void) hdr6->saddr.in6_u.u6_addr32[0] = htonl(1212); hdr6->saddr.in6_u.u6_addr32[1] = htonl(289188); skb_reset_network_header(skb6); - ++test; + ++test_num; #endif for (trials = TRIALS_BEFORE_GIVING_UP;;) { @@ -173,16 +172,16 @@ bool __init wg_ratelimiter_selftest(void) ret = timings_test(skb4, hdr4, skb6, hdr6, &test_count); if (ret == -ETIMEDOUT) { if (!trials--) { - test += test_count; + test_num += test_count; goto err; } msleep(500); continue; } else if (ret < 0) { - test += test_count; + test_num += test_count; goto err; } else { - test += test_count; + test_num += test_count; break; } } @@ -192,13 +191,13 @@ bool __init wg_ratelimiter_selftest(void) if (capacity_test(skb4, hdr4, &test_count) < 0) { if (!trials--) { - test += test_count; + test_num += test_count; goto err; } msleep(50); continue; } - test += test_count; + test_num += test_count; break; } @@ -216,11 +215,20 @@ bool __init wg_ratelimiter_selftest(void) /* Uninit one extra time to check underflow detection. */ wg_ratelimiter_uninit(); out: - if (success) - pr_info("ratelimiter self-tests: pass\n"); - else - pr_err("ratelimiter self-test %d: FAIL\n", test); - - return success; + if (!success) + KUNIT_FAIL(test, "test #%d failed", test_num); } -#endif + +static struct kunit_case wg_ratelimiter_test_cases[] = { + KUNIT_CASE(wg_ratelimiter_test), + {} +}; + +static struct kunit_suite wg_ratelimiter_test_suite = { + .name = "wg_ratelimiter", + .test_cases = wg_ratelimiter_test_cases, +}; + +kunit_test_suites(&wg_ratelimiter_test_suite); + +MODULE_LICENSE("GPL v2"); diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c index 2c9551ea6dc7..30d3d9685e8d 100644 --- a/drivers/net/wireguard/receive.c +++ b/drivers/net/wireguard/receive.c @@ -336,8 +336,6 @@ static bool counter_validate(struct noise_replay_counter *counter, u64 their_cou return ret; } -#include "selftest/counter.c" - static void wg_packet_consume_data_done(struct wg_peer *peer, struct sk_buff *skb, struct endpoint *endpoint) @@ -588,3 +586,7 @@ void wg_packet_receive(struct wg_device *wg, struct sk_buff *skb) err: dev_kfree_skb(skb); } + +#if IS_ENABLED(CONFIG_WIREGUARD_KUNIT_TEST) +#include "counter_test.c" +#endif