| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CABVgOSksOO6EYJO7vwwZ0d9F7eOMJNpm6mw1Jbs1tWKybC71fQ@mail.gmail.com>
Date: Fri, 11 Oct 2024 15:22:16 +0800
From: David Gow <davidgow@...gle.com>
To: Diego Vieira <diego.daniel.professional@...il.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-kernel@...r.kernel.org,
Brendan Higgins <brendan.higgins@...ux.dev>, Rae Moar <rmoar@...gle.com>,
linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com, n@...aprado.net,
andrealmeid@...eup.net, vinicius@...elet.com
Subject: Re: [PATCH v2 1/1] lib/tests/kfifo_kunit.c: add tests for the kfifo structure
On Wed, 4 Sept 2024 at 05:38, Diego Vieira
<diego.daniel.professional@...il.com> wrote:
>
> Add KUnit tests for the kfifo data structure.
> They test the vast majority of macros defined in the kfifo
> header (include/linux/kfifo.h).
>
> These are inspired by the existing tests for the doubly
> linked list in lib/tests/list-test.c (previously at lib/list-test.c) [1].
>
> Note that this patch depends on the patch that moves the KUnit tests on
> lib/ into lib/tests/ [2].
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/list-test.c?h=v6.11-rc6
> [2] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/
>
> Signed-off-by: Diego Vieira <diego.daniel.professional@...il.com>
> ---
Hi Diego,
Sorry for the delay: as you surmised, things have been very busy.
I think this patch is pretty good as-is, though (as Rae notes) there
are some places it could be improved and/or extended. It's
nevertheless worth having even in its current state, IMO, so this is:
Reviewed-by: David Gow <davidgow@...gle.com>
I'd like to accept it as-is for now, though, as I'm collating and
rebasing patches for lib/ tests which depend on the renaming to get
added to mm-nonmm-unstable (so we can avoid merge conflicts). If you
want to add extra test cases (or rearrange them within the file),
those may be best suited as a follow-up patch.
Thanks,
-- David
> lib/Kconfig.debug | 14 +++
> lib/tests/Makefile | 1 +
> lib/tests/kfifo_kunit.c | 224 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 239 insertions(+)
> create mode 100644 lib/tests/kfifo_kunit.c
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 59b6765d86b8..d7a4b996d731 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2646,6 +2646,20 @@ config SYSCTL_KUNIT_TEST
>
> If unsure, say N.
>
> +config KFIFO_KUNIT_TEST
> + tristate "KUnit Test for the generic kernel FIFO implementation" if !KUNIT_ALL_TESTS
> + depends on KUNIT
> + default KUNIT_ALL_TESTS
> + help
> + This builds the generic FIFO implementation KUnit test suite.
> + It tests that the API and basic functionality of the kfifo type
> + and associated macros.
> +
> + For more information on KUnit and unit tests in general please refer
> + to the KUnit documentation in Documentation/dev-tools/kunit/.
> +
> + If unsure, say N.
> +
> config LIST_KUNIT_TEST
> tristate "KUnit Test for Kernel Linked-list structures" if !KUNIT_ALL_TESTS
> depends on KUNIT
> diff --git a/lib/tests/Makefile b/lib/tests/Makefile
> index c6a14cc8663e..42699c7ee638 100644
> --- a/lib/tests/Makefile
> +++ b/lib/tests/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_TEST_IOV_ITER) += kunit_iov_iter.o
> obj-$(CONFIG_IS_SIGNED_TYPE_KUNIT_TEST) += is_signed_type_kunit.o
> obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
> obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
> +obj-$(CONFIG_KFIFO_KUNIT_TEST) += kfifo_kunit.o
> obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
> obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
> obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o
> diff --git a/lib/tests/kfifo_kunit.c b/lib/tests/kfifo_kunit.c
> new file mode 100644
> index 000000000000..a85eedc3195a
> --- /dev/null
> +++ b/lib/tests/kfifo_kunit.c
> @@ -0,0 +1,224 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for the generic kernel FIFO implementation.
> + *
> + * Copyright (C) 2024 Diego Vieira <diego.daniel.professional@...il.com>
> + */
> +#include <kunit/test.h>
> +
> +#include <linux/kfifo.h>
> +
> +#define KFIFO_SIZE 32
> +#define N_ELEMENTS 5
> +
> +static void kfifo_test_reset_should_clear_the_fifo(struct kunit *test)
> +{
> + DEFINE_KFIFO(my_fifo, u8, KFIFO_SIZE);
> +
> + kfifo_put(&my_fifo, 1);
> + kfifo_put(&my_fifo, 2);
> + kfifo_put(&my_fifo, 3);
> + KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), 3);
> +
> + kfifo_reset(&my_fifo);
> +
> + KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), 0);
> + KUNIT_EXPECT_TRUE(test, kfifo_is_empty(&my_fifo));
> +}
> +
> +static void kfifo_test_define_should_define_an_empty_fifo(struct kunit *test)
> +{
> + DEFINE_KFIFO(my_fifo, u8, KFIFO_SIZE);
> +
> + KUNIT_EXPECT_TRUE(test, kfifo_initialized(&my_fifo));
> + KUNIT_EXPECT_TRUE(test, kfifo_is_empty(&my_fifo));
> + KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), 0);
> +}
> +
> +static void kfifo_test_len_should_ret_n_of_stored_elements(struct kunit *test)
> +{
> + u8 buffer1[N_ELEMENTS];
> +
> + for (int i = 0; i < N_ELEMENTS; i++)
> + buffer1[i] = i + 1;
> +
> + DEFINE_KFIFO(my_fifo, u8, KFIFO_SIZE);
> +
> + KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), 0);
> +
> + kfifo_in(&my_fifo, buffer1, N_ELEMENTS);
> + KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), N_ELEMENTS);
> +
> + kfifo_in(&my_fifo, buffer1, N_ELEMENTS);
> + KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), N_ELEMENTS * 2);
> +
> + kfifo_reset(&my_fifo);
> + KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), 0);
> +}
> +
> +static void kfifo_test_put_should_insert_and_get_should_pop(struct kunit *test)
> +{
> + u8 out_data = 0;
> + int processed_elements;
> + u8 elements[] = { 3, 5, 11 };
> +
> + DEFINE_KFIFO(my_fifo, u8, KFIFO_SIZE);
> +
> + // If the fifo is empty, get returns 0
> + processed_elements = kfifo_get(&my_fifo, &out_data);
> + KUNIT_EXPECT_EQ(test, processed_elements, 0);
> + KUNIT_EXPECT_EQ(test, out_data, 0);
> +
> + for (int i = 0; i < 3; i++)
> + kfifo_put(&my_fifo, elements[i]);
> +
> + for (int i = 0; i < 3; i++) {
> + processed_elements = kfifo_get(&my_fifo, &out_data);
> + KUNIT_EXPECT_EQ(test, processed_elements, 1);
> + KUNIT_EXPECT_EQ(test, out_data, elements[i]);
> + }
> +}
> +
> +static void kfifo_test_in_should_insert_multiple_elements(struct kunit *test)
> +{
> + u8 in_buffer[] = { 11, 25, 65 };
> + u8 out_data;
> + int processed_elements;
> +
> + DEFINE_KFIFO(my_fifo, u8, KFIFO_SIZE);
> +
> + kfifo_in(&my_fifo, in_buffer, 3);
> +
> + for (int i = 0; i < 3; i++) {
> + processed_elements = kfifo_get(&my_fifo, &out_data);
> + KUNIT_EXPECT_EQ(test, processed_elements, 1);
> + KUNIT_EXPECT_EQ(test, out_data, in_buffer[i]);
> + }
> +}
> +
> +static void kfifo_test_out_should_pop_multiple_elements(struct kunit *test)
> +{
> + u8 in_buffer[] = { 11, 25, 65 };
> + u8 out_buffer[3];
> + int copied_elements;
> +
> + DEFINE_KFIFO(my_fifo, u8, KFIFO_SIZE);
> +
> + for (int i = 0; i < 3; i++)
> + kfifo_put(&my_fifo, in_buffer[i]);
> +
> + copied_elements = kfifo_out(&my_fifo, out_buffer, 3);
> + KUNIT_EXPECT_EQ(test, copied_elements, 3);
> +
> + for (int i = 0; i < 3; i++)
> + KUNIT_EXPECT_EQ(test, out_buffer[i], in_buffer[i]);
> + KUNIT_EXPECT_TRUE(test, kfifo_is_empty(&my_fifo));
> +}
> +
> +static void kfifo_test_dec_init_should_define_an_empty_fifo(struct kunit *test)
> +{
> + DECLARE_KFIFO(my_fifo, u8, KFIFO_SIZE);
> +
> + INIT_KFIFO(my_fifo);
> +
> + // my_fifo is a struct with an inplace buffer
> + KUNIT_EXPECT_FALSE(test, __is_kfifo_ptr(&my_fifo));
> +
> + KUNIT_EXPECT_TRUE(test, kfifo_initialized(&my_fifo));
> +}
> +
> +static void kfifo_test_define_should_equal_declare_init(struct kunit *test)
> +{
> + // declare a variable my_fifo of type struct kfifo of u8
> + DECLARE_KFIFO(my_fifo1, u8, KFIFO_SIZE);
> + // initialize the my_fifo variable
> + INIT_KFIFO(my_fifo1);
> +
> + // DEFINE_KFIFO declares the variable with the initial value
> + // essentially the same as calling DECLARE_KFIFO and INIT_KFIFO
> + DEFINE_KFIFO(my_fifo2, u8, KFIFO_SIZE);
> +
> + // my_fifo1 and my_fifo2 have the same size
> + KUNIT_EXPECT_EQ(test, sizeof(my_fifo1), sizeof(my_fifo2));
> + KUNIT_EXPECT_EQ(test, kfifo_initialized(&my_fifo1),
> + kfifo_initialized(&my_fifo2));
> + KUNIT_EXPECT_EQ(test, kfifo_is_empty(&my_fifo1),
> + kfifo_is_empty(&my_fifo2));
> +}
> +
> +static void kfifo_test_alloc_should_initiliaze_a_ptr_fifo(struct kunit *test)
> +{
> + int ret;
> + DECLARE_KFIFO_PTR(my_fifo, u8);
> +
> + INIT_KFIFO(my_fifo);
> +
> + // kfifo_initialized returns false signaling the buffer pointer is NULL
> + KUNIT_EXPECT_FALSE(test, kfifo_initialized(&my_fifo));
> +
> + // kfifo_alloc allocates the buffer
> + ret = kfifo_alloc(&my_fifo, KFIFO_SIZE, GFP_KERNEL);
> + KUNIT_EXPECT_EQ_MSG(test, ret, 0, "Memory allocation should succeed");
> + KUNIT_EXPECT_TRUE(test, kfifo_initialized(&my_fifo));
> +
> + // kfifo_free frees the buffer
> + kfifo_free(&my_fifo);
> +}
> +
> +static void kfifo_test_peek_should_not_remove_elements(struct kunit *test)
> +{
> + u8 out_data;
> + int processed_elements;
> +
> + DEFINE_KFIFO(my_fifo, u8, KFIFO_SIZE);
> +
> + // If the fifo is empty, peek returns 0
> + processed_elements = kfifo_peek(&my_fifo, &out_data);
> + KUNIT_EXPECT_EQ(test, processed_elements, 0);
> +
> + kfifo_put(&my_fifo, 3);
> + kfifo_put(&my_fifo, 5);
> + kfifo_put(&my_fifo, 11);
> +
> + KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), 3);
> +
> + processed_elements = kfifo_peek(&my_fifo, &out_data);
> + KUNIT_EXPECT_EQ(test, processed_elements, 1);
> + KUNIT_EXPECT_EQ(test, out_data, 3);
> +
> + KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), 3);
> +
> + // Using peek doesn't remove the element
> + // so the read element and the fifo length
> + // remains the same
> + processed_elements = kfifo_peek(&my_fifo, &out_data);
> + KUNIT_EXPECT_EQ(test, processed_elements, 1);
> + KUNIT_EXPECT_EQ(test, out_data, 3);
> +
> + KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), 3);
> +}
> +
> +static struct kunit_case kfifo_test_cases[] = {
> + KUNIT_CASE(kfifo_test_reset_should_clear_the_fifo),
> + KUNIT_CASE(kfifo_test_define_should_define_an_empty_fifo),
> + KUNIT_CASE(kfifo_test_len_should_ret_n_of_stored_elements),
> + KUNIT_CASE(kfifo_test_put_should_insert_and_get_should_pop),
> + KUNIT_CASE(kfifo_test_in_should_insert_multiple_elements),
> + KUNIT_CASE(kfifo_test_out_should_pop_multiple_elements),
> + KUNIT_CASE(kfifo_test_dec_init_should_define_an_empty_fifo),
> + KUNIT_CASE(kfifo_test_define_should_equal_declare_init),
> + KUNIT_CASE(kfifo_test_alloc_should_initiliaze_a_ptr_fifo),
> + KUNIT_CASE(kfifo_test_peek_should_not_remove_elements),
> + {},
> +};
> +
> +static struct kunit_suite kfifo_test_module = {
> + .name = "kfifo",
> + .test_cases = kfifo_test_cases,
> +};
> +
> +kunit_test_suites(&kfifo_test_module);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Diego Vieira <diego.daniel.professional@...il.com>");
> +MODULE_DESCRIPTION("KUnit test for the kernel FIFO");
> --
> 2.34.1
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@...glegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20240903213649.21467-2-diego.daniel.professional%40gmail.com.
Download attachment "smime.p7s" of type "application/pkcs7-signature" (5294 bytes)
Powered by blists - more mailing lists