Kunit tests to test hash table add, delete, duplicate add and delete.
Add following configs and compile kernel code:
CONFIG_CONNECTOR=y
CONFIG_PROC_EVENTS=y
CONFIG_NET=y
CONFIG_KUNIT=m
CONFIG_CN_HASH_KUNIT_TEST=m
To run kunit tests:
sudo modprobe cn_hash_test
Output of kunit tests and hash table contents are displayed in
/var/log/messages (at KERN_DEBUG level).
Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
---
drivers/connector/cn_hash.c | 49 +++++++++-
drivers/connector/connector.c | 15 ++-
include/linux/connector.h | 8 +-
lib/Kconfig.debug | 17 ++++
lib/Makefile | 1 +
lib/cn_hash_test.c | 167 ++++++++++++++++++++++++++++++++++
lib/cn_hash_test.h | 12 +++
7 files changed, 264 insertions(+), 5 deletions(-)
create mode 100644 lib/cn_hash_test.c
create mode 100644 lib/cn_hash_test.h
diff --git a/drivers/connector/cn_hash.c b/drivers/connector/cn_hash.c
index a0211cd99132..8f0eb6acb158 100644
--- a/drivers/connector/cn_hash.c
+++ b/drivers/connector/cn_hash.c
@@ -166,7 +166,7 @@ __u32 cn_hash_del_get_exval(struct cn_hash_dev *hdev, pid_t pid)
return 0;
}
-__u32 cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid)
+int cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid)
{
struct uexit_pid_hnode *hnode;
__u32 excde;
@@ -189,7 +189,52 @@ __u32 cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid)
return -EINVAL;
}
+int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len,
+ int *hkey, int *key_display)
+{
+ struct uexit_pid_hnode *hnode;
+ int key, count = 0;
+
+ mutex_lock(&hdev->uexit_hash_lock);
+ key = hash_min(pid, HASH_BITS(hdev->uexit_pid_htable));
+ pr_debug("Bucket: %d\n", key);
+
+ hlist_for_each_entry(hnode,
+ &hdev->uexit_pid_htable[key],
+ uexit_pid_hlist) {
+ if (key_display[key] != 1) {
+ if (hnode->uexit_pid_hlist.next == NULL)
+ pr_debug("pid %d ", hnode->pid);
+ else
+ pr_debug("pid %d --> ", hnode->pid);
+ }
+ count++;
+ }
+
+ mutex_unlock(&hdev->uexit_hash_lock);
+
+ if ((key_display[key] != 1) && !count)
+ pr_debug("(empty)\n");
+
+ pr_debug("\n");
+
+ *hkey = key;
+
+ if (count > max_len) {
+ pr_err("%d entries in hlist for key %d, expected %d\n",
+ count, key, max_len);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
bool cn_hash_table_empty(struct cn_hash_dev *hdev)
{
- return hash_empty(hdev->uexit_pid_htable);
+ bool is_empty;
+
+ is_empty = hash_empty(hdev->uexit_pid_htable);
+ pr_debug("Hash table is %s\n", (is_empty ? "empty" : "not empty"));
+
+ return is_empty;
}
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 506e3cbedf85..28e60c8b0fdf 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -308,7 +308,7 @@ int cn_add_elem(__u32 uexit_code, pid_t pid)
}
EXPORT_SYMBOL_GPL(cn_add_elem);
-__u32 cn_get_exval(pid_t pid)
+int cn_get_exval(pid_t pid)
{
struct cn_dev *dev = &cdev;
__u32 exval;
@@ -321,6 +321,19 @@ __u32 cn_get_exval(pid_t pid)
}
EXPORT_SYMBOL_GPL(cn_get_exval);
+int cn_display_hlist(pid_t pid, int max_len, int *hkey,
+ int *key_display)
+{
+ struct cn_dev *dev = &cdev;
+
+ if (!cn_already_initialized)
+ return 0;
+
+ return cn_hash_display_hlist(dev->hdev, pid, max_len,
+ hkey, key_display);
+}
+EXPORT_SYMBOL_GPL(cn_display_hlist);
+
bool cn_table_empty(void)
{
struct cn_dev *dev = &cdev;
diff --git a/include/linux/connector.h b/include/linux/connector.h
index 094e1730a4f6..af801c5005e8 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -165,7 +165,7 @@ __u32 cn_hash_del_get_exval(struct cn_hash_dev *hdev, pid_t pid);
int cn_add_elem(__u32 uexit_code, pid_t pid);
int cn_del_elem(pid_t pid);
__u32 cn_del_get_exval(pid_t pid);
-__u32 cn_get_exval(pid_t pid);
+int cn_get_exval(pid_t pid);
struct cn_hash_dev *cn_hash_alloc_dev(const char *name);
void cn_hash_free_dev(struct cn_hash_dev *hdev);
@@ -175,9 +175,13 @@ void cn_hash_free_elem(struct uexit_pid_hnode *elem);
int cn_hash_add_elem(struct cn_hash_dev *hdev, __u32 uexit_code, pid_t pid);
int cn_hash_del_elem(struct cn_hash_dev *hdev, pid_t pid);
__u32 cn_hash_del_get_exval(struct cn_hash_dev *hdev, pid_t pid);
-__u32 cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid);
+int cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid);
bool cn_table_empty(void);
bool cn_hash_table_empty(struct cn_hash_dev *hdev);
+int cn_display_hlist(pid_t pid, int max_len, int *hkey, int *key_display);
+int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len,
+ int *hkey, int *key_display);
+
#endif /* __CONNECTOR_H */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 7315f643817a..23599beb9bec 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST
If unsure, say N.
+config CONFIG_CN_HASH_KUNIT_TEST
+ tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ This builds the hashtable KUnit test suite.
+ It tests the basic functionality of the API defined in
+ drivers/connector/cn_hash.c.
+ CONFIG_CONNECTOR=y, CONFIG_PROC_EVENTS=y as well as CONFIG_NET=y
+ needs to be enabled along with CONFIG_CN_HASH_KUNIT_TEST=m and
+ CONFIG_KUNIT=m in .config file to compile and then test as a kernel
+ module with "modprobe cn_hash_test".
+ 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 LINEAR_RANGES_TEST
tristate "KUnit test for linear_ranges"
depends on KUNIT
diff --git a/lib/Makefile b/lib/Makefile
index 811ba12c8cd0..2c59c82b0b18 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -379,6 +379,7 @@ obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o
obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o
obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o
obj-$(CONFIG_IS_SIGNED_TYPE_KUNIT_TEST) += is_signed_type_kunit.o
+obj-$(CONFIG_CN_HASH_KUNIT_TEST) += cn_hash_test.o
CFLAGS_overflow_kunit.o = $(call cc-disable-warning, tautological-constant-out-of-range-compare)
obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o
CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable)
diff --git a/lib/cn_hash_test.c b/lib/cn_hash_test.c
new file mode 100644
index 000000000000..78c39c0a0997
--- /dev/null
+++ b/lib/cn_hash_test.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for the connector threads hashtable code.
+ *
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ * Author: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
+ */
+#include <kunit/test.h>
+
+#include "cn_hash_test.h"
+
+#define ARR_SIZE 4
+#define HASH_TABLE_LEN 1024
+
+struct add_data {
+ pid_t pid;
+ int exit_val;
+ int key;
+};
+
+static struct add_data adata[ARR_SIZE];
+static int key_display[HASH_TABLE_LEN];
+
+static int cn_hash_init(struct kunit *test)
+{
+ for (int i = 0; i < HASH_TABLE_LEN; i++)
+ key_display[i] = 0;
+
+ return 0;
+}
+
+static void cn_display_htable(struct kunit *test, int len)
+{
+ int i, err;
+
+ cn_hash_init(test);
+
+ pr_debug("\n");
+ pr_debug("Displaying hash table:\n");
+
+ for (i = 0; i < len; i++) {
+ err = cn_display_hlist(adata[i].pid, len, &adata[i].key,
+ key_display);
+ key_display[adata[i].key] = 1;
+ KUNIT_EXPECT_EQ(test, err, 0);
+ }
+}
+
+static void cn_hash_test_add(struct kunit *test)
+{
+ int err, i;
+ int exit_val;
+
+ adata[0].pid = 1;
+ adata[0].exit_val = 45;
+
+ adata[1].pid = 2;
+ adata[1].exit_val = 13;
+
+ adata[2].pid = 1024;
+ adata[2].exit_val = 16;
+
+ adata[3].pid = 1023;
+ adata[3].exit_val = 71;
+
+ for (i = 0; i < ARRAY_SIZE(adata); i++) {
+ err = cn_add_elem(adata[i].exit_val, adata[i].pid);
+ KUNIT_EXPECT_EQ_MSG(test, 0, err,
+ "Adding pid %d returned err %d",
+ adata[i].pid, err);
+
+ exit_val = cn_get_exval(adata[i].pid);
+ KUNIT_EXPECT_EQ(test, adata[i].exit_val, exit_val);
+ }
+
+ cn_display_htable(test, ARRAY_SIZE(adata));
+}
+
+static void cn_hash_test_del(struct kunit *test)
+{
+ int i, err;
+ int exit_val;
+
+ for (i = 0; i < ARRAY_SIZE(adata); i++) {
+ err = cn_del_elem(adata[i].pid);
+ KUNIT_EXPECT_EQ_MSG(test, 0, err,
+ "Deleting pid %d returned err %d",
+ adata[i].pid, err);
+
+ exit_val = cn_get_exval(adata[i].pid);
+ KUNIT_EXPECT_EQ(test, -EINVAL, exit_val);
+ }
+
+ cn_display_htable(test, ARRAY_SIZE(adata));
+ KUNIT_EXPECT_TRUE(test, cn_table_empty());
+}
+
+static void cn_hash_test_del_get_exval(struct kunit *test)
+{
+ int i, exval;
+
+ for (i = 0; i < ARRAY_SIZE(adata); i++) {
+ exval = cn_del_get_exval(adata[i].pid);
+ KUNIT_EXPECT_EQ(test, adata[i].exit_val, exval);
+
+ cn_display_htable(test, ARRAY_SIZE(adata));
+ }
+
+ KUNIT_EXPECT_TRUE(test, cn_table_empty());
+}
+static void cn_hash_test_dup_add(struct kunit *test)
+{
+ int err, exit_val;
+
+ adata[0].pid = 10;
+ adata[0].exit_val = 21;
+
+ err = cn_add_elem(adata[0].exit_val, adata[0].pid);
+ KUNIT_EXPECT_EQ(test, 0, err);
+
+ exit_val = cn_get_exval(adata[0].pid);
+ KUNIT_EXPECT_EQ(test, 21, exit_val);
+
+ adata[1].pid = 10;
+ adata[1].exit_val = 12;
+
+ err = cn_add_elem(adata[1].exit_val, adata[1].pid);
+ KUNIT_EXPECT_EQ(test, -EEXIST, err);
+
+ exit_val = cn_get_exval(adata[1].pid);
+ KUNIT_EXPECT_EQ(test, 21, exit_val);
+
+ cn_display_htable(test, 1);
+}
+
+static void cn_hash_test_dup_del(struct kunit *test)
+{
+ int err;
+
+ err = cn_del_elem(adata[0].pid);
+ KUNIT_EXPECT_EQ(test, 0, err);
+
+ err = cn_del_elem(adata[0].pid);
+ KUNIT_EXPECT_EQ(test, -EINVAL, err);
+
+ KUNIT_EXPECT_TRUE(test, cn_table_empty());
+}
+
+static struct kunit_case cn_hashtable_test_cases[] = {
+ KUNIT_CASE(cn_hash_test_add),
+ KUNIT_CASE(cn_hash_test_del),
+ KUNIT_CASE(cn_hash_test_dup_add),
+ KUNIT_CASE(cn_hash_test_dup_del),
+ KUNIT_CASE(cn_hash_test_add),
+ KUNIT_CASE(cn_hash_test_del_get_exval),
+ {},
+};
+
+static struct kunit_suite cn_hashtable_test_module = {
+ .name = "cn_hashtable",
+ .init = cn_hash_init,
+ .test_cases = cn_hashtable_test_cases,
+};
+kunit_test_suite(cn_hashtable_test_module);
+
+MODULE_DESCRIPTION("KUnit test for the connector threads hashtable code");
+MODULE_LICENSE("GPL");
diff --git a/lib/cn_hash_test.h b/lib/cn_hash_test.h
new file mode 100644
index 000000000000..46fcda31b25c
--- /dev/null
+++ b/lib/cn_hash_test.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ * Author: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
+ */
+extern int cn_display_hlist(pid_t pid, int max_len, int *hkey,
+ int *key_display);
+extern int cn_del_elem(pid_t pid);
+extern int cn_add_elem(__u32 uexit_code, pid_t pid);
+extern __u32 cn_del_get_exval(pid_t pid);
+extern int cn_get_exval(pid_t pid);
+extern bool cn_table_empty(void);
--
2.46.0
* Anjali Kulkarni <anjali.k.kulkarni@oracle.com> [241015 13:30]: > Kunit tests to test hash table add, delete, duplicate add and delete. > Add following configs and compile kernel code: > > CONFIG_CONNECTOR=y > CONFIG_PROC_EVENTS=y > CONFIG_NET=y > CONFIG_KUNIT=m > CONFIG_CN_HASH_KUNIT_TEST=m > > To run kunit tests: > sudo modprobe cn_hash_test > > Output of kunit tests and hash table contents are displayed in > /var/log/messages (at KERN_DEBUG level). > > Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com> > --- > drivers/connector/cn_hash.c | 49 +++++++++- > drivers/connector/connector.c | 15 ++- > include/linux/connector.h | 8 +- > lib/Kconfig.debug | 17 ++++ > lib/Makefile | 1 + > lib/cn_hash_test.c | 167 ++++++++++++++++++++++++++++++++++ > lib/cn_hash_test.h | 12 +++ > 7 files changed, 264 insertions(+), 5 deletions(-) > create mode 100644 lib/cn_hash_test.c > create mode 100644 lib/cn_hash_test.h > > diff --git a/drivers/connector/cn_hash.c b/drivers/connector/cn_hash.c > index a0211cd99132..8f0eb6acb158 100644 > --- a/drivers/connector/cn_hash.c > +++ b/drivers/connector/cn_hash.c > @@ -166,7 +166,7 @@ __u32 cn_hash_del_get_exval(struct cn_hash_dev *hdev, pid_t pid) > return 0; > } > > -__u32 cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid) > +int cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid) Is this return type change supposed to be here? > { > struct uexit_pid_hnode *hnode; > __u32 excde; > @@ -189,7 +189,52 @@ __u32 cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid) > return -EINVAL; > } > > +int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len, > + int *hkey, int *key_display) > +{ > + struct uexit_pid_hnode *hnode; > + int key, count = 0; > + > + mutex_lock(&hdev->uexit_hash_lock); > + key = hash_min(pid, HASH_BITS(hdev->uexit_pid_htable)); > + pr_debug("Bucket: %d\n", key); > + > + hlist_for_each_entry(hnode, > + &hdev->uexit_pid_htable[key], > + uexit_pid_hlist) { > + if (key_display[key] != 1) { > + if (hnode->uexit_pid_hlist.next == NULL) > + pr_debug("pid %d ", hnode->pid); > + else > + pr_debug("pid %d --> ", hnode->pid); > + } > + count++; > + } > + > + mutex_unlock(&hdev->uexit_hash_lock); > + > + if ((key_display[key] != 1) && !count) > + pr_debug("(empty)\n"); > + > + pr_debug("\n"); It seems like you maybe added an extra \n if you print the (empty) line above? > + > + *hkey = key; > + > + if (count > max_len) { > + pr_err("%d entries in hlist for key %d, expected %d\n", > + count, key, max_len); > + return -EINVAL; > + } > + > + return 0; > +} > + > bool cn_hash_table_empty(struct cn_hash_dev *hdev) > { > - return hash_empty(hdev->uexit_pid_htable); > + bool is_empty; > + > + is_empty = hash_empty(hdev->uexit_pid_htable); > + pr_debug("Hash table is %s\n", (is_empty ? "empty" : "not empty")); > + > + return is_empty; It seems odd to have this change in the kunit change. > } > diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c > index 506e3cbedf85..28e60c8b0fdf 100644 > --- a/drivers/connector/connector.c > +++ b/drivers/connector/connector.c > @@ -308,7 +308,7 @@ int cn_add_elem(__u32 uexit_code, pid_t pid) > } > EXPORT_SYMBOL_GPL(cn_add_elem); > > -__u32 cn_get_exval(pid_t pid) > +int cn_get_exval(pid_t pid) > { > struct cn_dev *dev = &cdev; > __u32 exval; > @@ -321,6 +321,19 @@ __u32 cn_get_exval(pid_t pid) > } > EXPORT_SYMBOL_GPL(cn_get_exval); > > +int cn_display_hlist(pid_t pid, int max_len, int *hkey, > + int *key_display) This can probably be one line here. > +{ > + struct cn_dev *dev = &cdev; > + > + if (!cn_already_initialized) > + return 0; > + > + return cn_hash_display_hlist(dev->hdev, pid, max_len, > + hkey, key_display); > +} > +EXPORT_SYMBOL_GPL(cn_display_hlist); > + > bool cn_table_empty(void) > { > struct cn_dev *dev = &cdev; > diff --git a/include/linux/connector.h b/include/linux/connector.h > index 094e1730a4f6..af801c5005e8 100644 > --- a/include/linux/connector.h > +++ b/include/linux/connector.h > @@ -165,7 +165,7 @@ __u32 cn_hash_del_get_exval(struct cn_hash_dev *hdev, pid_t pid); > int cn_add_elem(__u32 uexit_code, pid_t pid); > int cn_del_elem(pid_t pid); > __u32 cn_del_get_exval(pid_t pid); > -__u32 cn_get_exval(pid_t pid); > +int cn_get_exval(pid_t pid); > > struct cn_hash_dev *cn_hash_alloc_dev(const char *name); > void cn_hash_free_dev(struct cn_hash_dev *hdev); > @@ -175,9 +175,13 @@ void cn_hash_free_elem(struct uexit_pid_hnode *elem); > int cn_hash_add_elem(struct cn_hash_dev *hdev, __u32 uexit_code, pid_t pid); > int cn_hash_del_elem(struct cn_hash_dev *hdev, pid_t pid); > __u32 cn_hash_del_get_exval(struct cn_hash_dev *hdev, pid_t pid); > -__u32 cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid); > +int cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid); It seems like the change in type should be with another change or on its own? > > bool cn_table_empty(void); > bool cn_hash_table_empty(struct cn_hash_dev *hdev); > > +int cn_display_hlist(pid_t pid, int max_len, int *hkey, int *key_display); > +int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len, > + int *hkey, int *key_display); > + > #endif /* __CONNECTOR_H */ > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 7315f643817a..23599beb9bec 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST > > If unsure, say N. > > +config CONFIG_CN_HASH_KUNIT_TEST > + tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS > + depends on KUNIT > + default KUNIT_ALL_TESTS > + help > + This builds the hashtable KUnit test suite. > + It tests the basic functionality of the API defined in > + drivers/connector/cn_hash.c. > + CONFIG_CONNECTOR=y, CONFIG_PROC_EVENTS=y as well as CONFIG_NET=y > + needs to be enabled along with CONFIG_CN_HASH_KUNIT_TEST=m and > + CONFIG_KUNIT=m in .config file to compile and then test as a kernel > + module with "modprobe cn_hash_test". > + 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 LINEAR_RANGES_TEST > tristate "KUnit test for linear_ranges" > depends on KUNIT > diff --git a/lib/Makefile b/lib/Makefile > index 811ba12c8cd0..2c59c82b0b18 100644 > --- a/lib/Makefile > +++ b/lib/Makefile > @@ -379,6 +379,7 @@ obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o > obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o > obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o > obj-$(CONFIG_IS_SIGNED_TYPE_KUNIT_TEST) += is_signed_type_kunit.o > +obj-$(CONFIG_CN_HASH_KUNIT_TEST) += cn_hash_test.o > CFLAGS_overflow_kunit.o = $(call cc-disable-warning, tautological-constant-out-of-range-compare) > obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o > CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable) > diff --git a/lib/cn_hash_test.c b/lib/cn_hash_test.c > new file mode 100644 > index 000000000000..78c39c0a0997 > --- /dev/null > +++ b/lib/cn_hash_test.c > @@ -0,0 +1,167 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * KUnit test for the connector threads hashtable code. > + * > + * Copyright (c) 2024 Oracle and/or its affiliates. > + * Author: Anjali Kulkarni <anjali.k.kulkarni@oracle.com> > + */ > +#include <kunit/test.h> > + > +#include "cn_hash_test.h" > + > +#define ARR_SIZE 4 > +#define HASH_TABLE_LEN 1024 > + > +struct add_data { > + pid_t pid; > + int exit_val; > + int key; > +}; > + > +static struct add_data adata[ARR_SIZE]; > +static int key_display[HASH_TABLE_LEN]; > + > +static int cn_hash_init(struct kunit *test) > +{ > + for (int i = 0; i < HASH_TABLE_LEN; i++) > + key_display[i] = 0; > + > + return 0; > +} > + > +static void cn_display_htable(struct kunit *test, int len) > +{ > + int i, err; > + > + cn_hash_init(test); > + > + pr_debug("\n"); > + pr_debug("Displaying hash table:\n"); > + > + for (i = 0; i < len; i++) { > + err = cn_display_hlist(adata[i].pid, len, &adata[i].key, > + key_display); > + key_display[adata[i].key] = 1; > + KUNIT_EXPECT_EQ(test, err, 0); > + } > +} > + > +static void cn_hash_test_add(struct kunit *test) > +{ > + int err, i; > + int exit_val; > + > + adata[0].pid = 1; > + adata[0].exit_val = 45; > + > + adata[1].pid = 2; > + adata[1].exit_val = 13; > + > + adata[2].pid = 1024; > + adata[2].exit_val = 16; > + > + adata[3].pid = 1023; > + adata[3].exit_val = 71; > + > + for (i = 0; i < ARRAY_SIZE(adata); i++) { > + err = cn_add_elem(adata[i].exit_val, adata[i].pid); > + KUNIT_EXPECT_EQ_MSG(test, 0, err, > + "Adding pid %d returned err %d", > + adata[i].pid, err); > + > + exit_val = cn_get_exval(adata[i].pid); > + KUNIT_EXPECT_EQ(test, adata[i].exit_val, exit_val); > + } > + > + cn_display_htable(test, ARRAY_SIZE(adata)); > +} > + > +static void cn_hash_test_del(struct kunit *test) > +{ > + int i, err; > + int exit_val; > + > + for (i = 0; i < ARRAY_SIZE(adata); i++) { > + err = cn_del_elem(adata[i].pid); > + KUNIT_EXPECT_EQ_MSG(test, 0, err, > + "Deleting pid %d returned err %d", > + adata[i].pid, err); > + > + exit_val = cn_get_exval(adata[i].pid); > + KUNIT_EXPECT_EQ(test, -EINVAL, exit_val); > + } > + > + cn_display_htable(test, ARRAY_SIZE(adata)); > + KUNIT_EXPECT_TRUE(test, cn_table_empty()); > +} > + > +static void cn_hash_test_del_get_exval(struct kunit *test) > +{ > + int i, exval; > + > + for (i = 0; i < ARRAY_SIZE(adata); i++) { > + exval = cn_del_get_exval(adata[i].pid); > + KUNIT_EXPECT_EQ(test, adata[i].exit_val, exval); > + > + cn_display_htable(test, ARRAY_SIZE(adata)); > + } > + > + KUNIT_EXPECT_TRUE(test, cn_table_empty()); > +} > +static void cn_hash_test_dup_add(struct kunit *test) > +{ > + int err, exit_val; > + > + adata[0].pid = 10; > + adata[0].exit_val = 21; > + > + err = cn_add_elem(adata[0].exit_val, adata[0].pid); > + KUNIT_EXPECT_EQ(test, 0, err); > + > + exit_val = cn_get_exval(adata[0].pid); > + KUNIT_EXPECT_EQ(test, 21, exit_val); > + > + adata[1].pid = 10; > + adata[1].exit_val = 12; > + > + err = cn_add_elem(adata[1].exit_val, adata[1].pid); > + KUNIT_EXPECT_EQ(test, -EEXIST, err); > + > + exit_val = cn_get_exval(adata[1].pid); > + KUNIT_EXPECT_EQ(test, 21, exit_val); > + > + cn_display_htable(test, 1); > +} > + > +static void cn_hash_test_dup_del(struct kunit *test) > +{ > + int err; > + > + err = cn_del_elem(adata[0].pid); > + KUNIT_EXPECT_EQ(test, 0, err); > + > + err = cn_del_elem(adata[0].pid); > + KUNIT_EXPECT_EQ(test, -EINVAL, err); > + > + KUNIT_EXPECT_TRUE(test, cn_table_empty()); > +} > + > +static struct kunit_case cn_hashtable_test_cases[] = { > + KUNIT_CASE(cn_hash_test_add), > + KUNIT_CASE(cn_hash_test_del), > + KUNIT_CASE(cn_hash_test_dup_add), > + KUNIT_CASE(cn_hash_test_dup_del), > + KUNIT_CASE(cn_hash_test_add), > + KUNIT_CASE(cn_hash_test_del_get_exval), > + {}, > +}; > + > +static struct kunit_suite cn_hashtable_test_module = { > + .name = "cn_hashtable", > + .init = cn_hash_init, > + .test_cases = cn_hashtable_test_cases, > +}; > +kunit_test_suite(cn_hashtable_test_module); > + > +MODULE_DESCRIPTION("KUnit test for the connector threads hashtable code"); > +MODULE_LICENSE("GPL"); > diff --git a/lib/cn_hash_test.h b/lib/cn_hash_test.h > new file mode 100644 > index 000000000000..46fcda31b25c > --- /dev/null > +++ b/lib/cn_hash_test.h > @@ -0,0 +1,12 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * Copyright (c) 2024 Oracle and/or its affiliates. > + * Author: Anjali Kulkarni <anjali.k.kulkarni@oracle.com> > + */ > +extern int cn_display_hlist(pid_t pid, int max_len, int *hkey, > + int *key_display); > +extern int cn_del_elem(pid_t pid); > +extern int cn_add_elem(__u32 uexit_code, pid_t pid); > +extern __u32 cn_del_get_exval(pid_t pid); > +extern int cn_get_exval(pid_t pid); > +extern bool cn_table_empty(void); I think we are not adding externs to headers anymore. > -- > 2.46.0 >
> On Oct 15, 2024, at 12:16 PM, Liam Howlett <liam.howlett@oracle.com> wrote: > > * Anjali Kulkarni <anjali.k.kulkarni@oracle.com> [241015 13:30]: >> Kunit tests to test hash table add, delete, duplicate add and delete. >> Add following configs and compile kernel code: >> >> CONFIG_CONNECTOR=y >> CONFIG_PROC_EVENTS=y >> CONFIG_NET=y >> CONFIG_KUNIT=m >> CONFIG_CN_HASH_KUNIT_TEST=m >> >> To run kunit tests: >> sudo modprobe cn_hash_test >> >> Output of kunit tests and hash table contents are displayed in >> /var/log/messages (at KERN_DEBUG level). >> >> Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com> >> --- >> drivers/connector/cn_hash.c | 49 +++++++++- >> drivers/connector/connector.c | 15 ++- >> include/linux/connector.h | 8 +- >> lib/Kconfig.debug | 17 ++++ >> lib/Makefile | 1 + >> lib/cn_hash_test.c | 167 ++++++++++++++++++++++++++++++++++ >> lib/cn_hash_test.h | 12 +++ >> 7 files changed, 264 insertions(+), 5 deletions(-) >> create mode 100644 lib/cn_hash_test.c >> create mode 100644 lib/cn_hash_test.h >> >> diff --git a/drivers/connector/cn_hash.c b/drivers/connector/cn_hash.c >> index a0211cd99132..8f0eb6acb158 100644 >> --- a/drivers/connector/cn_hash.c >> +++ b/drivers/connector/cn_hash.c >> @@ -166,7 +166,7 @@ __u32 cn_hash_del_get_exval(struct cn_hash_dev *hdev, pid_t pid) >> return 0; >> } >> >> -__u32 cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid) >> +int cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid) > > Is this return type change supposed to be here? Yes, actually, I changed this function to return an error code instead of just exit code or 0, but rebased and put that change in the first patch. I will move this to the first patch. > >> { >> struct uexit_pid_hnode *hnode; >> __u32 excde; >> @@ -189,7 +189,52 @@ __u32 cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid) >> return -EINVAL; >> } >> >> +int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len, >> + int *hkey, int *key_display) >> +{ >> + struct uexit_pid_hnode *hnode; >> + int key, count = 0; >> + >> + mutex_lock(&hdev->uexit_hash_lock); >> + key = hash_min(pid, HASH_BITS(hdev->uexit_pid_htable)); >> + pr_debug("Bucket: %d\n", key); >> + >> + hlist_for_each_entry(hnode, >> + &hdev->uexit_pid_htable[key], >> + uexit_pid_hlist) { >> + if (key_display[key] != 1) { >> + if (hnode->uexit_pid_hlist.next == NULL) >> + pr_debug("pid %d ", hnode->pid); >> + else >> + pr_debug("pid %d --> ", hnode->pid); >> + } >> + count++; >> + } >> + >> + mutex_unlock(&hdev->uexit_hash_lock); >> + >> + if ((key_display[key] != 1) && !count) >> + pr_debug("(empty)\n"); >> + >> + pr_debug("\n"); > > It seems like you maybe added an extra \n if you print the (empty) line > above? Yes, so in the display you can see the Buckets more clearly delineated. > >> + >> + *hkey = key; >> + >> + if (count > max_len) { >> + pr_err("%d entries in hlist for key %d, expected %d\n", >> + count, key, max_len); >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> bool cn_hash_table_empty(struct cn_hash_dev *hdev) >> { >> - return hash_empty(hdev->uexit_pid_htable); >> + bool is_empty; >> + >> + is_empty = hash_empty(hdev->uexit_pid_htable); >> + pr_debug("Hash table is %s\n", (is_empty ? "empty" : "not empty")); >> + >> + return is_empty; > > It seems odd to have this change in the kunit change. Ok, will move to the first patch. > >> } >> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c >> index 506e3cbedf85..28e60c8b0fdf 100644 >> --- a/drivers/connector/connector.c >> +++ b/drivers/connector/connector.c >> @@ -308,7 +308,7 @@ int cn_add_elem(__u32 uexit_code, pid_t pid) >> } >> EXPORT_SYMBOL_GPL(cn_add_elem); >> >> -__u32 cn_get_exval(pid_t pid) >> +int cn_get_exval(pid_t pid) >> { >> struct cn_dev *dev = &cdev; >> __u32 exval; >> @@ -321,6 +321,19 @@ __u32 cn_get_exval(pid_t pid) >> } >> EXPORT_SYMBOL_GPL(cn_get_exval); >> >> +int cn_display_hlist(pid_t pid, int max_len, int *hkey, >> + int *key_display) > > This can probably be one line here. Yes, will do. > >> +{ >> + struct cn_dev *dev = &cdev; >> + >> + if (!cn_already_initialized) >> + return 0; >> + >> + return cn_hash_display_hlist(dev->hdev, pid, max_len, >> + hkey, key_display); >> +} >> +EXPORT_SYMBOL_GPL(cn_display_hlist); >> + >> bool cn_table_empty(void) >> { >> struct cn_dev *dev = &cdev; >> diff --git a/include/linux/connector.h b/include/linux/connector.h >> index 094e1730a4f6..af801c5005e8 100644 >> --- a/include/linux/connector.h >> +++ b/include/linux/connector.h >> @@ -165,7 +165,7 @@ __u32 cn_hash_del_get_exval(struct cn_hash_dev *hdev, pid_t pid); >> int cn_add_elem(__u32 uexit_code, pid_t pid); >> int cn_del_elem(pid_t pid); >> __u32 cn_del_get_exval(pid_t pid); >> -__u32 cn_get_exval(pid_t pid); >> +int cn_get_exval(pid_t pid); >> >> struct cn_hash_dev *cn_hash_alloc_dev(const char *name); >> void cn_hash_free_dev(struct cn_hash_dev *hdev); >> @@ -175,9 +175,13 @@ void cn_hash_free_elem(struct uexit_pid_hnode *elem); >> int cn_hash_add_elem(struct cn_hash_dev *hdev, __u32 uexit_code, pid_t pid); >> int cn_hash_del_elem(struct cn_hash_dev *hdev, pid_t pid); >> __u32 cn_hash_del_get_exval(struct cn_hash_dev *hdev, pid_t pid); >> -__u32 cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid); >> +int cn_hash_get_exval(struct cn_hash_dev *hdev, pid_t pid); > > It seems like the change in type should be with another change or on its > own? Yes, same reason I gave above - changed this function to return an error code instead of just exit code or 0, but rebased and put that change in the first patch. I will move this to the first patch. > >> >> bool cn_table_empty(void); >> bool cn_hash_table_empty(struct cn_hash_dev *hdev); >> >> +int cn_display_hlist(pid_t pid, int max_len, int *hkey, int *key_display); >> +int cn_hash_display_hlist(struct cn_hash_dev *hdev, pid_t pid, int max_len, >> + int *hkey, int *key_display); >> + >> #endif /* __CONNECTOR_H */ >> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug >> index 7315f643817a..23599beb9bec 100644 >> --- a/lib/Kconfig.debug >> +++ b/lib/Kconfig.debug >> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST >> >> If unsure, say N. >> >> +config CONFIG_CN_HASH_KUNIT_TEST >> + tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS >> + depends on KUNIT >> + default KUNIT_ALL_TESTS >> + help >> + This builds the hashtable KUnit test suite. >> + It tests the basic functionality of the API defined in >> + drivers/connector/cn_hash.c. >> + CONFIG_CONNECTOR=y, CONFIG_PROC_EVENTS=y as well as CONFIG_NET=y >> + needs to be enabled along with CONFIG_CN_HASH_KUNIT_TEST=m and >> + CONFIG_KUNIT=m in .config file to compile and then test as a kernel >> + module with "modprobe cn_hash_test". >> + 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 LINEAR_RANGES_TEST >> tristate "KUnit test for linear_ranges" >> depends on KUNIT >> diff --git a/lib/Makefile b/lib/Makefile >> index 811ba12c8cd0..2c59c82b0b18 100644 >> --- a/lib/Makefile >> +++ b/lib/Makefile >> @@ -379,6 +379,7 @@ obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o >> obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o >> obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o >> obj-$(CONFIG_IS_SIGNED_TYPE_KUNIT_TEST) += is_signed_type_kunit.o >> +obj-$(CONFIG_CN_HASH_KUNIT_TEST) += cn_hash_test.o >> CFLAGS_overflow_kunit.o = $(call cc-disable-warning, tautological-constant-out-of-range-compare) >> obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o >> CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable) >> diff --git a/lib/cn_hash_test.c b/lib/cn_hash_test.c >> new file mode 100644 >> index 000000000000..78c39c0a0997 >> --- /dev/null >> +++ b/lib/cn_hash_test.c >> @@ -0,0 +1,167 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * KUnit test for the connector threads hashtable code. >> + * >> + * Copyright (c) 2024 Oracle and/or its affiliates. >> + * Author: Anjali Kulkarni <anjali.k.kulkarni@oracle.com> >> + */ >> +#include <kunit/test.h> >> + >> +#include "cn_hash_test.h" >> + >> +#define ARR_SIZE 4 >> +#define HASH_TABLE_LEN 1024 >> + >> +struct add_data { >> + pid_t pid; >> + int exit_val; >> + int key; >> +}; >> + >> +static struct add_data adata[ARR_SIZE]; >> +static int key_display[HASH_TABLE_LEN]; >> + >> +static int cn_hash_init(struct kunit *test) >> +{ >> + for (int i = 0; i < HASH_TABLE_LEN; i++) >> + key_display[i] = 0; >> + >> + return 0; >> +} >> + >> +static void cn_display_htable(struct kunit *test, int len) >> +{ >> + int i, err; >> + >> + cn_hash_init(test); >> + >> + pr_debug("\n"); >> + pr_debug("Displaying hash table:\n"); >> + >> + for (i = 0; i < len; i++) { >> + err = cn_display_hlist(adata[i].pid, len, &adata[i].key, >> + key_display); >> + key_display[adata[i].key] = 1; >> + KUNIT_EXPECT_EQ(test, err, 0); >> + } >> +} >> + >> +static void cn_hash_test_add(struct kunit *test) >> +{ >> + int err, i; >> + int exit_val; >> + >> + adata[0].pid = 1; >> + adata[0].exit_val = 45; >> + >> + adata[1].pid = 2; >> + adata[1].exit_val = 13; >> + >> + adata[2].pid = 1024; >> + adata[2].exit_val = 16; >> + >> + adata[3].pid = 1023; >> + adata[3].exit_val = 71; >> + >> + for (i = 0; i < ARRAY_SIZE(adata); i++) { >> + err = cn_add_elem(adata[i].exit_val, adata[i].pid); >> + KUNIT_EXPECT_EQ_MSG(test, 0, err, >> + "Adding pid %d returned err %d", >> + adata[i].pid, err); >> + >> + exit_val = cn_get_exval(adata[i].pid); >> + KUNIT_EXPECT_EQ(test, adata[i].exit_val, exit_val); >> + } >> + >> + cn_display_htable(test, ARRAY_SIZE(adata)); >> +} >> + >> +static void cn_hash_test_del(struct kunit *test) >> +{ >> + int i, err; >> + int exit_val; >> + >> + for (i = 0; i < ARRAY_SIZE(adata); i++) { >> + err = cn_del_elem(adata[i].pid); >> + KUNIT_EXPECT_EQ_MSG(test, 0, err, >> + "Deleting pid %d returned err %d", >> + adata[i].pid, err); >> + >> + exit_val = cn_get_exval(adata[i].pid); >> + KUNIT_EXPECT_EQ(test, -EINVAL, exit_val); >> + } >> + >> + cn_display_htable(test, ARRAY_SIZE(adata)); >> + KUNIT_EXPECT_TRUE(test, cn_table_empty()); >> +} >> + >> +static void cn_hash_test_del_get_exval(struct kunit *test) >> +{ >> + int i, exval; >> + >> + for (i = 0; i < ARRAY_SIZE(adata); i++) { >> + exval = cn_del_get_exval(adata[i].pid); >> + KUNIT_EXPECT_EQ(test, adata[i].exit_val, exval); >> + >> + cn_display_htable(test, ARRAY_SIZE(adata)); >> + } >> + >> + KUNIT_EXPECT_TRUE(test, cn_table_empty()); >> +} >> +static void cn_hash_test_dup_add(struct kunit *test) >> +{ >> + int err, exit_val; >> + >> + adata[0].pid = 10; >> + adata[0].exit_val = 21; >> + >> + err = cn_add_elem(adata[0].exit_val, adata[0].pid); >> + KUNIT_EXPECT_EQ(test, 0, err); >> + >> + exit_val = cn_get_exval(adata[0].pid); >> + KUNIT_EXPECT_EQ(test, 21, exit_val); >> + >> + adata[1].pid = 10; >> + adata[1].exit_val = 12; >> + >> + err = cn_add_elem(adata[1].exit_val, adata[1].pid); >> + KUNIT_EXPECT_EQ(test, -EEXIST, err); >> + >> + exit_val = cn_get_exval(adata[1].pid); >> + KUNIT_EXPECT_EQ(test, 21, exit_val); >> + >> + cn_display_htable(test, 1); >> +} >> + >> +static void cn_hash_test_dup_del(struct kunit *test) >> +{ >> + int err; >> + >> + err = cn_del_elem(adata[0].pid); >> + KUNIT_EXPECT_EQ(test, 0, err); >> + >> + err = cn_del_elem(adata[0].pid); >> + KUNIT_EXPECT_EQ(test, -EINVAL, err); >> + >> + KUNIT_EXPECT_TRUE(test, cn_table_empty()); >> +} >> + >> +static struct kunit_case cn_hashtable_test_cases[] = { >> + KUNIT_CASE(cn_hash_test_add), >> + KUNIT_CASE(cn_hash_test_del), >> + KUNIT_CASE(cn_hash_test_dup_add), >> + KUNIT_CASE(cn_hash_test_dup_del), >> + KUNIT_CASE(cn_hash_test_add), >> + KUNIT_CASE(cn_hash_test_del_get_exval), >> + {}, >> +}; >> + >> +static struct kunit_suite cn_hashtable_test_module = { >> + .name = "cn_hashtable", >> + .init = cn_hash_init, >> + .test_cases = cn_hashtable_test_cases, >> +}; >> +kunit_test_suite(cn_hashtable_test_module); >> + >> +MODULE_DESCRIPTION("KUnit test for the connector threads hashtable code"); >> +MODULE_LICENSE("GPL"); >> diff --git a/lib/cn_hash_test.h b/lib/cn_hash_test.h >> new file mode 100644 >> index 000000000000..46fcda31b25c >> --- /dev/null >> +++ b/lib/cn_hash_test.h >> @@ -0,0 +1,12 @@ >> +/* SPDX-License-Identifier: GPL-2.0+ */ >> +/* >> + * Copyright (c) 2024 Oracle and/or its affiliates. >> + * Author: Anjali Kulkarni <anjali.k.kulkarni@oracle.com> >> + */ >> +extern int cn_display_hlist(pid_t pid, int max_len, int *hkey, >> + int *key_display); >> +extern int cn_del_elem(pid_t pid); >> +extern int cn_add_elem(__u32 uexit_code, pid_t pid); >> +extern __u32 cn_del_get_exval(pid_t pid); >> +extern int cn_get_exval(pid_t pid); >> +extern bool cn_table_empty(void); > > I think we are not adding externs to headers anymore. Ok, should I move it to the cn_hash_test.c file then? That is what I had initially but the checkpatch.pl gave a warning to move to .h file. > >> -- >> 2.46.0
* Anjali Kulkarni <anjali.k.kulkarni@oracle.com> [241015 17:11]: ... > >> +MODULE_DESCRIPTION("KUnit test for the connector threads hashtable code"); > >> +MODULE_LICENSE("GPL"); > >> diff --git a/lib/cn_hash_test.h b/lib/cn_hash_test.h > >> new file mode 100644 > >> index 000000000000..46fcda31b25c > >> --- /dev/null > >> +++ b/lib/cn_hash_test.h > >> @@ -0,0 +1,12 @@ > >> +/* SPDX-License-Identifier: GPL-2.0+ */ > >> +/* > >> + * Copyright (c) 2024 Oracle and/or its affiliates. > >> + * Author: Anjali Kulkarni <anjali.k.kulkarni@oracle.com> > >> + */ > >> +extern int cn_display_hlist(pid_t pid, int max_len, int *hkey, > >> + int *key_display); > >> +extern int cn_del_elem(pid_t pid); > >> +extern int cn_add_elem(__u32 uexit_code, pid_t pid); > >> +extern __u32 cn_del_get_exval(pid_t pid); > >> +extern int cn_get_exval(pid_t pid); > >> +extern bool cn_table_empty(void); > > > > I think we are not adding externs to headers anymore. > > Ok, should I move it to the cn_hash_test.c file then? That is what I had > initially but the checkpatch.pl gave a warning to move to .h file. No, if you can drop the extern and still have it build then do that, otherwise keep them in the header as they are.
© 2016 - 2024 Red Hat, Inc.