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

On 10/22, Anjali Kulkarni wrote:

quoted

On Oct 17, 2024, at 5:55 PM, Stanislav Fomichev [off-list ref] wrote:

On 10/18, Anjali Kulkarni wrote:

quoted

quoted

On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev [off-list ref] wrote:

On 10/17, Anjali Kulkarni wrote:

quoted

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 <redacted>
---
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.

[..]

quoted

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?