Message ID | 20241017181436.2047508-3-anjali.k.kulkarni@oracle.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Threads support in proc connector | expand |
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
> 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
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.
> 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
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);
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