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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/18, Anjali Kulkarni wrote:
> 
> 
> > On Oct 17, 2024, at 5:13 PM, Stanislav Fomichev <stfomichev@xxxxxxxxx> 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@xxxxxxxxxx>
> >> ---
> >> 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.




[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux