[PATCH net-next v2 2/3] connector/cn_proc: Kunit tests for threads hash table

Anjali Kulkarni posted 3 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH net-next v2 2/3] connector/cn_proc: Kunit tests for threads hash table
Posted by Anjali Kulkarni 1 month, 1 week ago
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
Re: [PATCH net-next v2 2/3] connector/cn_proc: Kunit tests for threads hash table
Posted by Liam R. Howlett 1 month, 1 week ago
* 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
>
Re: [PATCH net-next v2 2/3] connector/cn_proc: Kunit tests for threads hash table
Posted by Anjali Kulkarni 1 month, 1 week ago

> 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


Re: [PATCH net-next v2 2/3] connector/cn_proc: Kunit tests for threads hash table
Posted by Liam R. Howlett 1 month, 1 week ago
* 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.