[v5] Threads support in proc connector

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

Posted by Anjali Kulkarni 1 year, 3 months 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   |  40 ++++++++
 drivers/connector/connector.c |  12 +++
 include/linux/connector.h     |   4 +
 lib/Kconfig.debug             |  17 ++++
 lib/Makefile                  |   1 +
 lib/cn_hash_test.c            | 167 ++++++++++++++++++++++++++++++++++
 lib/cn_hash_test.h            |  10 ++
 7 files changed, 251 insertions(+)
 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 a079e9bcea6d..40099b5908ac 100644
--- a/drivers/connector/cn_hash.c
+++ b/drivers/connector/cn_hash.c
@@ -170,6 +170,46 @@ int 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)
 {
 	bool is_empty;
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index c1c0dcec53c0..2be2fe1adc12 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -168,4 +168,8 @@ int cn_get_exval(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..290cf0a6befa 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST
 
 	  If unsure, say N.
 
+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 and 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..f90989343468
--- /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_get_exval(adata[i].pid);
+		KUNIT_EXPECT_GT_MSG(test, err, 0,
+				"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_get_exval(adata[0].pid);
+	KUNIT_EXPECT_EQ(test, adata[0].exit_val, err);
+
+	err = cn_del_get_exval(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..b25033feab09
--- /dev/null
+++ b/lib/cn_hash_test.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ * Author: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
+ */
+int cn_display_hlist(pid_t pid, int max_len, int *hkey, int *key_display);
+int cn_add_elem(__u32 uexit_code, pid_t pid);
+int cn_del_get_exval(pid_t pid);
+int cn_get_exval(pid_t pid);
+bool cn_table_empty(void);
-- 
2.46.0

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

Posted by kernel test robot 1 year, 3 months ago

Hi Anjali,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Anjali-Kulkarni/connector-cn_proc-Add-hash-table-for-threads/20241018-021755
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241017181436.2047508-3-anjali.k.kulkarni%40oracle.com
patch subject: [PATCH net-next v5 2/3] connector/cn_proc: Kunit tests for threads hash table
config: x86_64-randconfig-104-20241019 (https://download.01.org/0day-ci/archive/20241019/202410191051.p3iFT9om-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241019/202410191051.p3iFT9om-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410191051.p3iFT9om-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

WARNING: modpost: missing MODULE_DESCRIPTION() in mm/kasan/kasan_test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in lib/slub_kunit.o
>> ERROR: modpost: "cn_del_get_exval" [lib/cn_hash_test.ko] undefined!
>> ERROR: modpost: "cn_table_empty" [lib/cn_hash_test.ko] undefined!
>> ERROR: modpost: "cn_display_hlist" [lib/cn_hash_test.ko] undefined!
>> ERROR: modpost: "cn_add_elem" [lib/cn_hash_test.ko] undefined!
>> ERROR: modpost: "cn_get_exval" [lib/cn_hash_test.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

Posted by kernel test robot 1 year, 3 months ago

Hi Anjali,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Anjali-Kulkarni/connector-cn_proc-Add-hash-table-for-threads/20241018-021755
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241017181436.2047508-3-anjali.k.kulkarni%40oracle.com
patch subject: [PATCH net-next v5 2/3] connector/cn_proc: Kunit tests for threads hash table
config: sparc-randconfig-001-20241019 (https://download.01.org/0day-ci/archive/20241019/202410190945.sGeQPUMr-lkp@intel.com/config)
compiler: sparc-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241019/202410190945.sGeQPUMr-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410190945.sGeQPUMr-lkp@intel.com/

All errors (new ones prefixed by >>):

   sparc-linux-ld: lib/cn_hash_test.o: in function `cn_hash_test_dup_del':
>> lib/cn_hash_test.c:140:(.text+0x44): undefined reference to `cn_del_get_exval'
>> sparc-linux-ld: lib/cn_hash_test.c:143:(.text+0xb4): undefined reference to `cn_del_get_exval'
>> sparc-linux-ld: lib/cn_hash_test.c:146:(.text+0x128): undefined reference to `cn_table_empty'
   sparc-linux-ld: lib/cn_hash_test.o: in function `cn_display_htable':
>> lib/cn_hash_test.c:42:(.text+0x1f8): undefined reference to `cn_display_hlist'
   sparc-linux-ld: lib/cn_hash_test.o: in function `cn_hash_test_del_get_exval':
   lib/cn_hash_test.c:103:(.text+0x2bc): undefined reference to `cn_del_get_exval'
   sparc-linux-ld: lib/cn_hash_test.c:109:(.text+0x350): undefined reference to `cn_table_empty'
   sparc-linux-ld: lib/cn_hash_test.o: in function `cn_hash_test_dup_add':
>> lib/cn_hash_test.c:118:(.text+0x3f0): undefined reference to `cn_add_elem'
>> sparc-linux-ld: lib/cn_hash_test.c:121:(.text+0x458): undefined reference to `cn_get_exval'
>> sparc-linux-ld: lib/cn_hash_test.c:127:(.text+0x4d8): undefined reference to `cn_add_elem'
   sparc-linux-ld: lib/cn_hash_test.c:130:(.text+0x540): undefined reference to `cn_get_exval'
   sparc-linux-ld: lib/cn_hash_test.o: in function `cn_hash_test_del':
   lib/cn_hash_test.c:85:(.text+0x5d0): undefined reference to `cn_del_get_exval'
   sparc-linux-ld: lib/cn_hash_test.c:90:(.text+0x640): undefined reference to `cn_get_exval'
   sparc-linux-ld: lib/cn_hash_test.c:95:(.text+0x6cc): undefined reference to `cn_table_empty'
   sparc-linux-ld: lib/cn_hash_test.o: in function `cn_hash_test_add':
   lib/cn_hash_test.c:67:(.text+0x7b4): undefined reference to `cn_add_elem'
   sparc-linux-ld: lib/cn_hash_test.c:72:(.text+0x824): undefined reference to `cn_get_exval'

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for GET_FREE_REGION
   Depends on [n]: SPARSEMEM [=n]
   Selected by [y]:
   - RESOURCE_KUNIT_TEST [=y] && RUNTIME_TESTING_MENU [=y] && KUNIT [=y]


vim +140 lib/cn_hash_test.c

    31	
    32	static void cn_display_htable(struct kunit *test, int len)
    33	{
    34		int i, err;
    35	
    36		cn_hash_init(test);
    37	
    38		pr_debug("\n");
    39		pr_debug("Displaying hash table:\n");
    40	
    41		for (i = 0; i < len; i++) {
  > 42			err = cn_display_hlist(adata[i].pid, len, &adata[i].key,
    43						key_display);
    44			key_display[adata[i].key] = 1;
    45			KUNIT_EXPECT_EQ(test, err, 0);
    46		}
    47	}
    48	
    49	static void cn_hash_test_add(struct kunit *test)
    50	{
    51		int err, i;
    52		int exit_val;
    53	
    54		adata[0].pid = 1;
    55		adata[0].exit_val = 45;
    56	
    57		adata[1].pid = 2;
    58		adata[1].exit_val = 13;
    59	
    60		adata[2].pid = 1024;
    61		adata[2].exit_val = 16;
    62	
    63		adata[3].pid = 1023;
    64		adata[3].exit_val = 71;
    65	
    66		for (i = 0; i < ARRAY_SIZE(adata); i++) {
    67			err = cn_add_elem(adata[i].exit_val, adata[i].pid);
    68			KUNIT_EXPECT_EQ_MSG(test, 0, err,
    69					"Adding pid %d returned err %d",
    70					adata[i].pid, err);
    71	
    72			exit_val = cn_get_exval(adata[i].pid);
    73			KUNIT_EXPECT_EQ(test, adata[i].exit_val, exit_val);
    74		}
    75	
    76		cn_display_htable(test, ARRAY_SIZE(adata));
    77	}
    78	
    79	static void cn_hash_test_del(struct kunit *test)
    80	{
    81		int i, err;
    82		int exit_val;
    83	
    84		for (i = 0; i < ARRAY_SIZE(adata); i++) {
    85			err = cn_del_get_exval(adata[i].pid);
    86			KUNIT_EXPECT_GT_MSG(test, err, 0,
    87					"Deleting pid %d returned err %d",
    88					adata[i].pid, err);
    89	
    90			exit_val = cn_get_exval(adata[i].pid);
    91			KUNIT_EXPECT_EQ(test, -EINVAL, exit_val);
    92		}
    93	
    94		cn_display_htable(test, ARRAY_SIZE(adata));
    95		KUNIT_EXPECT_TRUE(test, cn_table_empty());
    96	}
    97	
    98	static void cn_hash_test_del_get_exval(struct kunit *test)
    99	{
   100		int i, exval;
   101	
   102		for (i = 0; i < ARRAY_SIZE(adata); i++) {
   103			exval = cn_del_get_exval(adata[i].pid);
   104			KUNIT_EXPECT_EQ(test, adata[i].exit_val, exval);
   105	
   106			cn_display_htable(test, ARRAY_SIZE(adata));
   107		}
   108	
   109		KUNIT_EXPECT_TRUE(test, cn_table_empty());
   110	}
   111	static void cn_hash_test_dup_add(struct kunit *test)
   112	{
   113		int err, exit_val;
   114	
   115		adata[0].pid = 10;
   116		adata[0].exit_val = 21;
   117	
 > 118		err = cn_add_elem(adata[0].exit_val, adata[0].pid);
   119		KUNIT_EXPECT_EQ(test, 0, err);
   120	
 > 121		exit_val = cn_get_exval(adata[0].pid);
   122		KUNIT_EXPECT_EQ(test, 21, exit_val);
   123	
   124		adata[1].pid = 10;
   125		adata[1].exit_val = 12;
   126	
 > 127		err = cn_add_elem(adata[1].exit_val, adata[1].pid);
   128		KUNIT_EXPECT_EQ(test, -EEXIST, err);
   129	
   130		exit_val = cn_get_exval(adata[1].pid);
   131		KUNIT_EXPECT_EQ(test, 21, exit_val);
   132	
   133		cn_display_htable(test, 1);
   134	}
   135	
   136	static void cn_hash_test_dup_del(struct kunit *test)
   137	{
   138		int err;
   139	
 > 140		err = cn_del_get_exval(adata[0].pid);
   141		KUNIT_EXPECT_EQ(test, adata[0].exit_val, err);
   142	
 > 143		err = cn_del_get_exval(adata[0].pid);
   144		KUNIT_EXPECT_EQ(test, -EINVAL, err);
   145	
 > 146		KUNIT_EXPECT_TRUE(test, cn_table_empty());
   147	}
   148	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

Posted by Stanislav Fomichev 1 year, 3 months ago

On 10/17, Anjali Kulkarni wrote:
> 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   |  40 ++++++++
>  drivers/connector/connector.c |  12 +++
>  include/linux/connector.h     |   4 +
>  lib/Kconfig.debug             |  17 ++++
>  lib/Makefile                  |   1 +
>  lib/cn_hash_test.c            | 167 ++++++++++++++++++++++++++++++++++
>  lib/cn_hash_test.h            |  10 ++
>  7 files changed, 251 insertions(+)
>  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 a079e9bcea6d..40099b5908ac 100644
> --- a/drivers/connector/cn_hash.c
> +++ b/drivers/connector/cn_hash.c
> @@ -170,6 +170,46 @@ int 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)
>  {
>  	bool is_empty;
> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
> index c1c0dcec53c0..2be2fe1adc12 100644
> --- a/drivers/connector/connector.c
> +++ b/drivers/connector/connector.c
> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644
> --- a/include/linux/connector.h
> +++ b/include/linux/connector.h
> @@ -168,4 +168,8 @@ int cn_get_exval(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..290cf0a6befa 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST
>  
>  	  If unsure, say N.
>  
> +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 and 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.
> +

Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the
existing kunit tester complains about the missing symbols (see below).
Please also hold off reposting for a couple of days to give people some
time to review.

ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del':
cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval'
ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval'
ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty'
ld: vmlinux.o: in function `cn_display_htable':
cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist'
ld: vmlinux.o: in function `cn_hash_test_del_get_exval':
cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval'
ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty'
ld: vmlinux.o: in function `cn_hash_test_dup_add':
cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem'
ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval'
ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem'
ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval'
ld: vmlinux.o: in function `cn_hash_test_del':
cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval'
ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval'
ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty'
ld: vmlinux.o: in function `cn_hash_test_add':
cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem'
ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval'
make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1
make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2
make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2
make: *** [Makefile:224: __sub-make] Error 2

---
pw-bot: cr

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

Posted by Anjali Kulkarni 1 year, 3 months ago


> On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote:
> 
> On 10/17, Anjali Kulkarni wrote:
>> 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   |  40 ++++++++
>> drivers/connector/connector.c |  12 +++
>> include/linux/connector.h     |   4 +
>> lib/Kconfig.debug             |  17 ++++
>> lib/Makefile                  |   1 +
>> lib/cn_hash_test.c            | 167 ++++++++++++++++++++++++++++++++++
>> lib/cn_hash_test.h            |  10 ++
>> 7 files changed, 251 insertions(+)
>> 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 a079e9bcea6d..40099b5908ac 100644
>> --- a/drivers/connector/cn_hash.c
>> +++ b/drivers/connector/cn_hash.c
>> @@ -170,6 +170,46 @@ int 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)
>> {
>> bool is_empty;
>> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
>> index c1c0dcec53c0..2be2fe1adc12 100644
>> --- a/drivers/connector/connector.c
>> +++ b/drivers/connector/connector.c
>> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644
>> --- a/include/linux/connector.h
>> +++ b/include/linux/connector.h
>> @@ -168,4 +168,8 @@ int cn_get_exval(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..290cf0a6befa 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST
>> 
>>  If unsure, say N.
>> 
>> +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 and 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.
>> +
> 
> Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the
> existing kunit tester complains about the missing symbols (see below).
> Please also hold off reposting for a couple of days to give people some
> time to review.
> 
> ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del':
> cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval'
> ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval'
> ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty'
> ld: vmlinux.o: in function `cn_display_htable':
> cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist'
> ld: vmlinux.o: in function `cn_hash_test_del_get_exval':
> cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval'
> ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty'
> ld: vmlinux.o: in function `cn_hash_test_dup_add':
> cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem'
> ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval'
> ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem'
> ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval'
> ld: vmlinux.o: in function `cn_hash_test_del':
> cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval'
> ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval'
> ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty'
> ld: vmlinux.o: in function `cn_hash_test_add':
> cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem'
> ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval'
> make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1
> make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2
> make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2
> make: *** [Makefile:224: __sub-make] Error 2

Yes, I have added in the comments for CN_HASH_KUNIT_TEST, it depends on:
CONFIG_CONNECTOR, CONFIG_PROC_EVENTS, CONFIG_NET. I didn’t realize
I could add these to the “depends” field.
So something like this: (let me know if you see any issues)

        tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS
        depends on KUNIT
+       depends on CONNECTOR && PROC_EVENTS
+       depends on NET
        default KUNIT_ALL_TESTS

These are the configs I add to my .config file and compile it as a module and then
do modprobe to test.
Are you running the kunit tester with kunit.py?

Thanks
Anjali

> 
> ---
> pw-bot: cr

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

Posted by Stanislav Fomichev 1 year, 3 months ago

On 10/18, Anjali Kulkarni wrote:
> 
> 
> > On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote:
> > 
> > On 10/17, Anjali Kulkarni wrote:
> >> 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   |  40 ++++++++
> >> drivers/connector/connector.c |  12 +++
> >> include/linux/connector.h     |   4 +
> >> lib/Kconfig.debug             |  17 ++++
> >> lib/Makefile                  |   1 +
> >> lib/cn_hash_test.c            | 167 ++++++++++++++++++++++++++++++++++
> >> lib/cn_hash_test.h            |  10 ++
> >> 7 files changed, 251 insertions(+)
> >> 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 a079e9bcea6d..40099b5908ac 100644
> >> --- a/drivers/connector/cn_hash.c
> >> +++ b/drivers/connector/cn_hash.c
> >> @@ -170,6 +170,46 @@ int 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)
> >> {
> >> bool is_empty;
> >> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
> >> index c1c0dcec53c0..2be2fe1adc12 100644
> >> --- a/drivers/connector/connector.c
> >> +++ b/drivers/connector/connector.c
> >> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644
> >> --- a/include/linux/connector.h
> >> +++ b/include/linux/connector.h
> >> @@ -168,4 +168,8 @@ int cn_get_exval(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..290cf0a6befa 100644
> >> --- a/lib/Kconfig.debug
> >> +++ b/lib/Kconfig.debug
> >> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST
> >> 
> >>  If unsure, say N.
> >> 
> >> +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 and 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.
> >> +
> > 
> > Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the
> > existing kunit tester complains about the missing symbols (see below).
> > Please also hold off reposting for a couple of days to give people some
> > time to review.
> > 
> > ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del':
> > cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval'
> > ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval'
> > ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty'
> > ld: vmlinux.o: in function `cn_display_htable':
> > cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist'
> > ld: vmlinux.o: in function `cn_hash_test_del_get_exval':
> > cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval'
> > ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty'
> > ld: vmlinux.o: in function `cn_hash_test_dup_add':
> > cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem'
> > ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval'
> > ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem'
> > ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval'
> > ld: vmlinux.o: in function `cn_hash_test_del':
> > cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval'
> > ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval'
> > ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty'
> > ld: vmlinux.o: in function `cn_hash_test_add':
> > cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem'
> > ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval'
> > make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1
> > make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2
> > make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2
> > make: *** [Makefile:224: __sub-make] Error 2
> 
> Yes, I have added in the comments for CN_HASH_KUNIT_TEST, it depends on:
> CONFIG_CONNECTOR, CONFIG_PROC_EVENTS, CONFIG_NET. I didn’t realize
> I could add these to the “depends” field.
> So something like this: (let me know if you see any issues)
> 
>         tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS
>         depends on KUNIT
> +       depends on CONNECTOR && PROC_EVENTS
> +       depends on NET
>         default KUNIT_ALL_TESTS
> 
> These are the configs I add to my .config file and compile it as a module and then
> do modprobe to test.

[..]

> Are you running the kunit tester with kunit.py?

Yes, make sure all required options are picked up by
"./tools/testing/kunit/kunit.py run" instead of manually adding options
and doing modprobe.

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

Posted by Anjali Kulkarni 1 year, 3 months ago


> On Oct 17, 2024, at 5:55 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote:
> 
> On 10/18, Anjali Kulkarni wrote:
>> 
>> 
>>> On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote:
>>> 
>>> On 10/17, Anjali Kulkarni wrote:
>>>> 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   |  40 ++++++++
>>>> drivers/connector/connector.c |  12 +++
>>>> include/linux/connector.h     |   4 +
>>>> lib/Kconfig.debug             |  17 ++++
>>>> lib/Makefile                  |   1 +
>>>> lib/cn_hash_test.c            | 167 ++++++++++++++++++++++++++++++++++
>>>> lib/cn_hash_test.h            |  10 ++
>>>> 7 files changed, 251 insertions(+)
>>>> 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 a079e9bcea6d..40099b5908ac 100644
>>>> --- a/drivers/connector/cn_hash.c
>>>> +++ b/drivers/connector/cn_hash.c
>>>> @@ -170,6 +170,46 @@ int 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)
>>>> {
>>>> bool is_empty;
>>>> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
>>>> index c1c0dcec53c0..2be2fe1adc12 100644
>>>> --- a/drivers/connector/connector.c
>>>> +++ b/drivers/connector/connector.c
>>>> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644
>>>> --- a/include/linux/connector.h
>>>> +++ b/include/linux/connector.h
>>>> @@ -168,4 +168,8 @@ int cn_get_exval(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..290cf0a6befa 100644
>>>> --- a/lib/Kconfig.debug
>>>> +++ b/lib/Kconfig.debug
>>>> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST
>>>> 
>>>> If unsure, say N.
>>>> 
>>>> +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 and 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.
>>>> +
>>> 
>>> Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the
>>> existing kunit tester complains about the missing symbols (see below).
>>> Please also hold off reposting for a couple of days to give people some
>>> time to review.
>>> 
>>> ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del':
>>> cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval'
>>> ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval'
>>> ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty'
>>> ld: vmlinux.o: in function `cn_display_htable':
>>> cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist'
>>> ld: vmlinux.o: in function `cn_hash_test_del_get_exval':
>>> cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval'
>>> ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty'
>>> ld: vmlinux.o: in function `cn_hash_test_dup_add':
>>> cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem'
>>> ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval'
>>> ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem'
>>> ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval'
>>> ld: vmlinux.o: in function `cn_hash_test_del':
>>> cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval'
>>> ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval'
>>> ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty'
>>> ld: vmlinux.o: in function `cn_hash_test_add':
>>> cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem'
>>> ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval'
>>> make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1
>>> make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2
>>> make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2
>>> make: *** [Makefile:224: __sub-make] Error 2
>> 
>> Yes, I have added in the comments for CN_HASH_KUNIT_TEST, it depends on:
>> CONFIG_CONNECTOR, CONFIG_PROC_EVENTS, CONFIG_NET. I didn’t realize
>> I could add these to the “depends” field.
>> So something like this: (let me know if you see any issues)
>> 
>>        tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS
>>        depends on KUNIT
>> +       depends on CONNECTOR && PROC_EVENTS
>> +       depends on NET
>>        default KUNIT_ALL_TESTS
>> 
>> These are the configs I add to my .config file and compile it as a module and then
>> do modprobe to test.
> 
> [..]
> 
>> Are you running the kunit tester with kunit.py?
> 
> Yes, make sure all required options are picked up by
> "./tools/testing/kunit/kunit.py run" instead of manually adding options
> and doing modprobe.

The environment issues are resolved and I am able to run kunit.py, but my tests
are not invoked without giving options via —kconfig-add. Other tests are also not
invoked. Running with the manual options runs 413 tests, and with just kunit.py
runs 389 tests. (I have added 6). Any idea how I can make it run my tests?

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

Posted by Stanislav Fomichev 1 year, 3 months ago

On 10/22, Anjali Kulkarni wrote:
> 
> 
> > On Oct 17, 2024, at 5:55 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote:
> > 
> > On 10/18, Anjali Kulkarni wrote:
> >> 
> >> 
> >>> On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote:
> >>> 
> >>> On 10/17, Anjali Kulkarni wrote:
> >>>> 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   |  40 ++++++++
> >>>> drivers/connector/connector.c |  12 +++
> >>>> include/linux/connector.h     |   4 +
> >>>> lib/Kconfig.debug             |  17 ++++
> >>>> lib/Makefile                  |   1 +
> >>>> lib/cn_hash_test.c            | 167 ++++++++++++++++++++++++++++++++++
> >>>> lib/cn_hash_test.h            |  10 ++
> >>>> 7 files changed, 251 insertions(+)
> >>>> 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 a079e9bcea6d..40099b5908ac 100644
> >>>> --- a/drivers/connector/cn_hash.c
> >>>> +++ b/drivers/connector/cn_hash.c
> >>>> @@ -170,6 +170,46 @@ int 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)
> >>>> {
> >>>> bool is_empty;
> >>>> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
> >>>> index c1c0dcec53c0..2be2fe1adc12 100644
> >>>> --- a/drivers/connector/connector.c
> >>>> +++ b/drivers/connector/connector.c
> >>>> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644
> >>>> --- a/include/linux/connector.h
> >>>> +++ b/include/linux/connector.h
> >>>> @@ -168,4 +168,8 @@ int cn_get_exval(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..290cf0a6befa 100644
> >>>> --- a/lib/Kconfig.debug
> >>>> +++ b/lib/Kconfig.debug
> >>>> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST
> >>>> 
> >>>> If unsure, say N.
> >>>> 
> >>>> +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 and 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.
> >>>> +
> >>> 
> >>> Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the
> >>> existing kunit tester complains about the missing symbols (see below).
> >>> Please also hold off reposting for a couple of days to give people some
> >>> time to review.
> >>> 
> >>> ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del':
> >>> cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval'
> >>> ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval'
> >>> ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty'
> >>> ld: vmlinux.o: in function `cn_display_htable':
> >>> cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist'
> >>> ld: vmlinux.o: in function `cn_hash_test_del_get_exval':
> >>> cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval'
> >>> ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty'
> >>> ld: vmlinux.o: in function `cn_hash_test_dup_add':
> >>> cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem'
> >>> ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval'
> >>> ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem'
> >>> ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval'
> >>> ld: vmlinux.o: in function `cn_hash_test_del':
> >>> cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval'
> >>> ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval'
> >>> ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty'
> >>> ld: vmlinux.o: in function `cn_hash_test_add':
> >>> cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem'
> >>> ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval'
> >>> make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1
> >>> make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2
> >>> make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2
> >>> make: *** [Makefile:224: __sub-make] Error 2
> >> 
> >> Yes, I have added in the comments for CN_HASH_KUNIT_TEST, it depends on:
> >> CONFIG_CONNECTOR, CONFIG_PROC_EVENTS, CONFIG_NET. I didn’t realize
> >> I could add these to the “depends” field.
> >> So something like this: (let me know if you see any issues)
> >> 
> >>        tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS
> >>        depends on KUNIT
> >> +       depends on CONNECTOR && PROC_EVENTS
> >> +       depends on NET
> >>        default KUNIT_ALL_TESTS
> >> 
> >> These are the configs I add to my .config file and compile it as a module and then
> >> do modprobe to test.
> > 
> > [..]
> > 
> >> Are you running the kunit tester with kunit.py?
> > 
> > Yes, make sure all required options are picked up by
> > "./tools/testing/kunit/kunit.py run" instead of manually adding options
> > and doing modprobe.
> 
> The environment issues are resolved and I am able to run kunit.py, but my tests
> are not invoked without giving options via —kconfig-add. Other tests are also not
> invoked. Running with the manual options runs 413 tests, and with just kunit.py
> runs 389 tests. (I have added 6). Any idea how I can make it run my tests?

The runner does: ./tools/testing/kunit/kunit.py run --alltests
Is it not enough in your case? What options do you pass via
--kconfig-add? Is it because CONNECTOR stuff is disabled by default?

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

Posted by Anjali Kulkarni 1 year, 3 months ago


> On Oct 22, 2024, at 4:50 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote:
> 
> On 10/22, Anjali Kulkarni wrote:
>> 
>> 
>>> On Oct 17, 2024, at 5:55 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote:
>>> 
>>> On 10/18, Anjali Kulkarni wrote:
>>>> 
>>>> 
>>>>> On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote:
>>>>> 
>>>>> On 10/17, Anjali Kulkarni wrote:
>>>>>> 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   |  40 ++++++++
>>>>>> drivers/connector/connector.c |  12 +++
>>>>>> include/linux/connector.h     |   4 +
>>>>>> lib/Kconfig.debug             |  17 ++++
>>>>>> lib/Makefile                  |   1 +
>>>>>> lib/cn_hash_test.c            | 167 ++++++++++++++++++++++++++++++++++
>>>>>> lib/cn_hash_test.h            |  10 ++
>>>>>> 7 files changed, 251 insertions(+)
>>>>>> 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 a079e9bcea6d..40099b5908ac 100644
>>>>>> --- a/drivers/connector/cn_hash.c
>>>>>> +++ b/drivers/connector/cn_hash.c
>>>>>> @@ -170,6 +170,46 @@ int 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)
>>>>>> {
>>>>>> bool is_empty;
>>>>>> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
>>>>>> index c1c0dcec53c0..2be2fe1adc12 100644
>>>>>> --- a/drivers/connector/connector.c
>>>>>> +++ b/drivers/connector/connector.c
>>>>>> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644
>>>>>> --- a/include/linux/connector.h
>>>>>> +++ b/include/linux/connector.h
>>>>>> @@ -168,4 +168,8 @@ int cn_get_exval(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..290cf0a6befa 100644
>>>>>> --- a/lib/Kconfig.debug
>>>>>> +++ b/lib/Kconfig.debug
>>>>>> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST
>>>>>> 
>>>>>> If unsure, say N.
>>>>>> 
>>>>>> +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 and 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.
>>>>>> +
>>>>> 
>>>>> Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the
>>>>> existing kunit tester complains about the missing symbols (see below).
>>>>> Please also hold off reposting for a couple of days to give people some
>>>>> time to review.
>>>>> 
>>>>> ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del':
>>>>> cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval'
>>>>> ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval'
>>>>> ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty'
>>>>> ld: vmlinux.o: in function `cn_display_htable':
>>>>> cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist'
>>>>> ld: vmlinux.o: in function `cn_hash_test_del_get_exval':
>>>>> cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval'
>>>>> ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty'
>>>>> ld: vmlinux.o: in function `cn_hash_test_dup_add':
>>>>> cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem'
>>>>> ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval'
>>>>> ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem'
>>>>> ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval'
>>>>> ld: vmlinux.o: in function `cn_hash_test_del':
>>>>> cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval'
>>>>> ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval'
>>>>> ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty'
>>>>> ld: vmlinux.o: in function `cn_hash_test_add':
>>>>> cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem'
>>>>> ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval'
>>>>> make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1
>>>>> make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2
>>>>> make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2
>>>>> make: *** [Makefile:224: __sub-make] Error 2
>>>> 
>>>> Yes, I have added in the comments for CN_HASH_KUNIT_TEST, it depends on:
>>>> CONFIG_CONNECTOR, CONFIG_PROC_EVENTS, CONFIG_NET. I didn’t realize
>>>> I could add these to the “depends” field.
>>>> So something like this: (let me know if you see any issues)
>>>> 
>>>>       tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS
>>>>       depends on KUNIT
>>>> +       depends on CONNECTOR && PROC_EVENTS
>>>> +       depends on NET
>>>>       default KUNIT_ALL_TESTS
>>>> 
>>>> These are the configs I add to my .config file and compile it as a module and then
>>>> do modprobe to test.
>>> 
>>> [..]
>>> 
>>>> Are you running the kunit tester with kunit.py?
>>> 
>>> Yes, make sure all required options are picked up by
>>> "./tools/testing/kunit/kunit.py run" instead of manually adding options
>>> and doing modprobe.
>> 
>> The environment issues are resolved and I am able to run kunit.py, but my tests
>> are not invoked without giving options via —kconfig-add. Other tests are also not
>> invoked. Running with the manual options runs 413 tests, and with just kunit.py
>> runs 389 tests. (I have added 6). Any idea how I can make it run my tests?
> 
> The runner does: ./tools/testing/kunit/kunit.py run --alltests
> Is it not enough in your case? What options do you pass via
> --kconfig-add? Is it because CONNECTOR stuff is disabled by default?

No, it still does not run.
However, I added to tools/testing/kunit/configs/all_tests.config:

CONFIG_CONNECTOR=y
CONFIG_PROC_EVENTS=y
CONFIG_NET=y
CONFIG_CN_HASH_KUNIT_TEST=y

And now it does run.
Should I make the change above? I will also check with the kunit guys.
But I do not understand how it ran for you(and run into the error), or did
it just try to compile?

Anjali

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

Posted by Anjali Kulkarni 1 year, 3 months ago

[…snip…]

>>>> 
>>>> Yes, make sure all required options are picked up by
>>>> "./tools/testing/kunit/kunit.py run" instead of manually adding options
>>>> and doing modprobe.
>>> 
>>> The environment issues are resolved and I am able to run kunit.py, but my tests
>>> are not invoked without giving options via —kconfig-add. Other tests are also not
>>> invoked. Running with the manual options runs 413 tests, and with just kunit.py
>>> runs 389 tests. (I have added 6). Any idea how I can make it run my tests?
>> 
>> The runner does: ./tools/testing/kunit/kunit.py run --alltests
>> Is it not enough in your case? What options do you pass via
>> --kconfig-add? Is it because CONNECTOR stuff is disabled by default?
> 
> No, it still does not run.
> However, I added to tools/testing/kunit/configs/all_tests.config:
> 
> CONFIG_CONNECTOR=y
> CONFIG_PROC_EVENTS=y
> CONFIG_NET=y
> CONFIG_CN_HASH_KUNIT_TEST=y
> 
> And now it does run.
> Should I make the change above? I will also check with the kunit guys.
> But I do not understand how it ran for you(and run into the error), or did
> it just try to compile?

I see this in comments on top of all_tests.config.

# The config is manually maintained, though it uses KUNIT_ALL_TESTS=y to enable
# any tests whose dependencies are already satisfied. Please feel free to add
# more options if they any new tests.

So I suppose if a test needs more dependencies, it needs to be added here.

Anjali

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

Posted by Stanislav Fomichev 1 year, 3 months ago

On 10/23, Anjali Kulkarni wrote:
> […snip…]
> 
> >>>> 
> >>>> Yes, make sure all required options are picked up by
> >>>> "./tools/testing/kunit/kunit.py run" instead of manually adding options
> >>>> and doing modprobe.
> >>> 
> >>> The environment issues are resolved and I am able to run kunit.py, but my tests
> >>> are not invoked without giving options via —kconfig-add. Other tests are also not
> >>> invoked. Running with the manual options runs 413 tests, and with just kunit.py
> >>> runs 389 tests. (I have added 6). Any idea how I can make it run my tests?
> >> 
> >> The runner does: ./tools/testing/kunit/kunit.py run --alltests
> >> Is it not enough in your case? What options do you pass via
> >> --kconfig-add? Is it because CONNECTOR stuff is disabled by default?
> > 
> > No, it still does not run.
> > However, I added to tools/testing/kunit/configs/all_tests.config:
> > 
> > CONFIG_CONNECTOR=y
> > CONFIG_PROC_EVENTS=y
> > CONFIG_NET=y
> > CONFIG_CN_HASH_KUNIT_TEST=y
> > 
> > And now it does run.
> > Should I make the change above? I will also check with the kunit guys.
> > But I do not understand how it ran for you(and run into the error), or did
> > it just try to compile?
> 
> I see this in comments on top of all_tests.config.
> 
> # The config is manually maintained, though it uses KUNIT_ALL_TESTS=y to enable
> # any tests whose dependencies are already satisfied. Please feel free to add
> # more options if they any new tests.
> 
> So I suppose if a test needs more dependencies, it needs to be added here.

Let's try and CC a bunch of kunit people to confirm :-)

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

Posted by Anjali Kulkarni 1 year, 3 months ago


> On Oct 23, 2024, at 8:05 AM, Stanislav Fomichev <stfomichev@gmail.com> wrote:
> 
> On 10/23, Anjali Kulkarni wrote:
>> […snip…]
>> 
>>>>>> 
>>>>>> Yes, make sure all required options are picked up by
>>>>>> "./tools/testing/kunit/kunit.py run" instead of manually adding options
>>>>>> and doing modprobe.
>>>>> 
>>>>> The environment issues are resolved and I am able to run kunit.py, but my tests
>>>>> are not invoked without giving options via —kconfig-add. Other tests are also not
>>>>> invoked. Running with the manual options runs 413 tests, and with just kunit.py
>>>>> runs 389 tests. (I have added 6). Any idea how I can make it run my tests?
>>>> 
>>>> The runner does: ./tools/testing/kunit/kunit.py run --alltests
>>>> Is it not enough in your case? What options do you pass via
>>>> --kconfig-add? Is it because CONNECTOR stuff is disabled by default?
>>> 
>>> No, it still does not run.
>>> However, I added to tools/testing/kunit/configs/all_tests.config:
>>> 
>>> CONFIG_CONNECTOR=y
>>> CONFIG_PROC_EVENTS=y
>>> CONFIG_NET=y
>>> CONFIG_CN_HASH_KUNIT_TEST=y
>>> 
>>> And now it does run.
>>> Should I make the change above? I will also check with the kunit guys.
>>> But I do not understand how it ran for you(and run into the error), or did
>>> it just try to compile?
>> 
>> I see this in comments on top of all_tests.config.
>> 
>> # The config is manually maintained, though it uses KUNIT_ALL_TESTS=y to enable
>> # any tests whose dependencies are already satisfied. Please feel free to add
>> # more options if they any new tests.
>> 
>> So I suppose if a test needs more dependencies, it needs to be added here.
> 
> Let's try and CC a bunch of kunit people to confirm :-)

Ok! Will send out a new patch and cc the kunit folks on it. Hopefully it works:)

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

Posted by Anjali Kulkarni 1 year, 3 months ago


> On Oct 17, 2024, at 5:55 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote:
> 
> On 10/18, Anjali Kulkarni wrote:
>> 
>> 
>>> On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote:
>>> 
>>> On 10/17, Anjali Kulkarni wrote:
>>>> 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   |  40 ++++++++
>>>> drivers/connector/connector.c |  12 +++
>>>> include/linux/connector.h     |   4 +
>>>> lib/Kconfig.debug             |  17 ++++
>>>> lib/Makefile                  |   1 +
>>>> lib/cn_hash_test.c            | 167 ++++++++++++++++++++++++++++++++++
>>>> lib/cn_hash_test.h            |  10 ++
>>>> 7 files changed, 251 insertions(+)
>>>> 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 a079e9bcea6d..40099b5908ac 100644
>>>> --- a/drivers/connector/cn_hash.c
>>>> +++ b/drivers/connector/cn_hash.c
>>>> @@ -170,6 +170,46 @@ int 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)
>>>> {
>>>> bool is_empty;
>>>> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
>>>> index c1c0dcec53c0..2be2fe1adc12 100644
>>>> --- a/drivers/connector/connector.c
>>>> +++ b/drivers/connector/connector.c
>>>> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644
>>>> --- a/include/linux/connector.h
>>>> +++ b/include/linux/connector.h
>>>> @@ -168,4 +168,8 @@ int cn_get_exval(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..290cf0a6befa 100644
>>>> --- a/lib/Kconfig.debug
>>>> +++ b/lib/Kconfig.debug
>>>> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST
>>>> 
>>>> If unsure, say N.
>>>> 
>>>> +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 and 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.
>>>> +
>>> 
>>> Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the
>>> existing kunit tester complains about the missing symbols (see below).
>>> Please also hold off reposting for a couple of days to give people some
>>> time to review.
>>> 
>>> ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del':
>>> cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval'
>>> ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval'
>>> ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty'
>>> ld: vmlinux.o: in function `cn_display_htable':
>>> cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist'
>>> ld: vmlinux.o: in function `cn_hash_test_del_get_exval':
>>> cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval'
>>> ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty'
>>> ld: vmlinux.o: in function `cn_hash_test_dup_add':
>>> cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem'
>>> ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval'
>>> ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem'
>>> ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval'
>>> ld: vmlinux.o: in function `cn_hash_test_del':
>>> cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval'
>>> ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval'
>>> ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty'
>>> ld: vmlinux.o: in function `cn_hash_test_add':
>>> cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem'
>>> ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval'
>>> make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1
>>> make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2
>>> make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2
>>> make: *** [Makefile:224: __sub-make] Error 2
>> 
>> Yes, I have added in the comments for CN_HASH_KUNIT_TEST, it depends on:
>> CONFIG_CONNECTOR, CONFIG_PROC_EVENTS, CONFIG_NET. I didn’t realize
>> I could add these to the “depends” field.
>> So something like this: (let me know if you see any issues)
>> 
>>        tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS
>>        depends on KUNIT
>> +       depends on CONNECTOR && PROC_EVENTS
>> +       depends on NET
>>        default KUNIT_ALL_TESTS
>> 
>> These are the configs I add to my .config file and compile it as a module and then
>> do modprobe to test.
> 
> [..]
> 
>> Are you running the kunit tester with kunit.py?
> 
> Yes, make sure all required options are picked up by
> "./tools/testing/kunit/kunit.py run" instead of manually adding options
> and doing modprobe.

I’m unable to run kunit.py, it runs into various issues like UML, permissions, other 
errors. I talked to the kunit guys about this and we have been debugging it for a
while but unable to fix the environment issue. But the tests work fine.

What kind of VM is this being run on? Like ubuntu etc.? I will try on a different
OS and check if kunit.py works. 

Anjali

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

Posted by Stanislav Fomichev 1 year, 3 months ago

On 10/18, Anjali Kulkarni wrote:
> 
> 
> > On Oct 17, 2024, at 5:55 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote:
> > 
> > On 10/18, Anjali Kulkarni wrote:
> >> 
> >> 
> >>> On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@gmail.com> wrote:
> >>> 
> >>> On 10/17, Anjali Kulkarni wrote:
> >>>> 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   |  40 ++++++++
> >>>> drivers/connector/connector.c |  12 +++
> >>>> include/linux/connector.h     |   4 +
> >>>> lib/Kconfig.debug             |  17 ++++
> >>>> lib/Makefile                  |   1 +
> >>>> lib/cn_hash_test.c            | 167 ++++++++++++++++++++++++++++++++++
> >>>> lib/cn_hash_test.h            |  10 ++
> >>>> 7 files changed, 251 insertions(+)
> >>>> 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 a079e9bcea6d..40099b5908ac 100644
> >>>> --- a/drivers/connector/cn_hash.c
> >>>> +++ b/drivers/connector/cn_hash.c
> >>>> @@ -170,6 +170,46 @@ int 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)
> >>>> {
> >>>> bool is_empty;
> >>>> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
> >>>> index c1c0dcec53c0..2be2fe1adc12 100644
> >>>> --- a/drivers/connector/connector.c
> >>>> +++ b/drivers/connector/connector.c
> >>>> @@ -304,6 +304,18 @@ int 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 5384e4bb98e8..a75c3fcf182a 100644
> >>>> --- a/include/linux/connector.h
> >>>> +++ b/include/linux/connector.h
> >>>> @@ -168,4 +168,8 @@ int cn_get_exval(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..290cf0a6befa 100644
> >>>> --- a/lib/Kconfig.debug
> >>>> +++ b/lib/Kconfig.debug
> >>>> @@ -2705,6 +2705,23 @@ config HASHTABLE_KUNIT_TEST
> >>>> 
> >>>> If unsure, say N.
> >>>> 
> >>>> +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 and 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.
> >>>> +
> >>> 
> >>> Looks like this needs to depend on CONFIG_CONNECTOR? Otherwise, the
> >>> existing kunit tester complains about the missing symbols (see below).
> >>> Please also hold off reposting for a couple of days to give people some
> >>> time to review.
> >>> 
> >>> ERROR:root:ld: vmlinux.o: in function `cn_hash_test_dup_del':
> >>> cn_hash_test.c:(.text+0x3e9dc3): undefined reference to `cn_del_get_exval'
> >>> ld: cn_hash_test.c:(.text+0x3e9dee): undefined reference to `cn_del_get_exval'
> >>> ld: cn_hash_test.c:(.text+0x3e9e22): undefined reference to `cn_table_empty'
> >>> ld: vmlinux.o: in function `cn_display_htable':
> >>> cn_hash_test.c:(.text+0x3e9f67): undefined reference to `cn_display_hlist'
> >>> ld: vmlinux.o: in function `cn_hash_test_del_get_exval':
> >>> cn_hash_test.c:(.text+0x3ea037): undefined reference to `cn_del_get_exval'
> >>> ld: cn_hash_test.c:(.text+0x3ea088): undefined reference to `cn_table_empty'
> >>> ld: vmlinux.o: in function `cn_hash_test_dup_add':
> >>> cn_hash_test.c:(.text+0x3ea176): undefined reference to `cn_add_elem'
> >>> ld: cn_hash_test.c:(.text+0x3ea19e): undefined reference to `cn_get_exval'
> >>> ld: cn_hash_test.c:(.text+0x3ea1dc): undefined reference to `cn_add_elem'
> >>> ld: cn_hash_test.c:(.text+0x3ea205): undefined reference to `cn_get_exval'
> >>> ld: vmlinux.o: in function `cn_hash_test_del':
> >>> cn_hash_test.c:(.text+0x3ea387): undefined reference to `cn_del_get_exval'
> >>> ld: cn_hash_test.c:(.text+0x3ea3ab): undefined reference to `cn_get_exval'
> >>> ld: cn_hash_test.c:(.text+0x3ea3fd): undefined reference to `cn_table_empty'
> >>> ld: vmlinux.o: in function `cn_hash_test_add':
> >>> cn_hash_test.c:(.text+0x3ea571): undefined reference to `cn_add_elem'
> >>> ld: cn_hash_test.c:(.text+0x3ea591): undefined reference to `cn_get_exval'
> >>> make[3]: *** [../scripts/Makefile.vmlinux:34: vmlinux] Error 1
> >>> make[2]: *** [/home/kunit/testing/Makefile:1166: vmlinux] Error 2
> >>> make[1]: *** [/home/kunit/testing/Makefile:224: __sub-make] Error 2
> >>> make: *** [Makefile:224: __sub-make] Error 2
> >> 
> >> Yes, I have added in the comments for CN_HASH_KUNIT_TEST, it depends on:
> >> CONFIG_CONNECTOR, CONFIG_PROC_EVENTS, CONFIG_NET. I didn’t realize
> >> I could add these to the “depends” field.
> >> So something like this: (let me know if you see any issues)
> >> 
> >>        tristate "KUnit Test for connector hashtable code" if !KUNIT_ALL_TESTS
> >>        depends on KUNIT
> >> +       depends on CONNECTOR && PROC_EVENTS
> >> +       depends on NET
> >>        default KUNIT_ALL_TESTS
> >> 
> >> These are the configs I add to my .config file and compile it as a module and then
> >> do modprobe to test.
> > 
> > [..]
> > 
> >> Are you running the kunit tester with kunit.py?
> > 
> > Yes, make sure all required options are picked up by
> > "./tools/testing/kunit/kunit.py run" instead of manually adding options
> > and doing modprobe.
> 
> I’m unable to run kunit.py, it runs into various issues like UML, permissions, other 
> errors. I talked to the kunit guys about this and we have been debugging it for a
> while but unable to fix the environment issue. But the tests work fine.
> 
> What kind of VM is this being run on? Like ubuntu etc.? I will try on a different
> OS and check if kunit.py works. 

It's running on fedora.

[PATCH net-next v5 1/3] connector/cn_proc: Add hash table for threads
[PATCH net-next v5 2/3] connector/cn_proc: Kunit tests for threads hash table
[PATCH net-next v5 3/3] connector/cn_proc: Selftest for threads