Re: [PATCH net-next v5 3/3] connector/cn_proc: Selftest for threads

[Anjali Kulkarni <anjali.k.kulkarni@xxxxxxxxxx>]

> On Oct 18, 2024, at 3:04 AM, Simon Horman <horms@xxxxxxxxxx> wrote:
> 
> On Thu, Oct 17, 2024 at 11:14:36AM -0700, Anjali Kulkarni wrote:
>> Test to check if setting PROC_CN_MCAST_NOTIFY in proc connector API, allows
>> a thread's non-zero exit status to be returned to proc_filter.
>> 
>> The threads.c program creates 2 child threads. 1st thread handles signal
>> SIGSEGV, and 2nd thread needs to indicate some error condition (value 1)
>> to the kernel, instead of using pthread_exit() with 1.
>> 
>> In both cases, child sends notify_netlink_thread_exit(exit_code) to kernel,
>> to let kernel know it has exited abnormally with exit_code.
>> 
>> Compile:
>>    make thread
>>    make proc_filter
>> Run:
>>    ./threads
>> 
>> Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@xxxxxxxxxx>
>> ---
>> tools/testing/selftests/connector/Makefile    |  23 +-
>> .../testing/selftests/connector/proc_filter.c |  34 ++-
>> tools/testing/selftests/connector/thread.c    | 232 ++++++++++++++++++
>> .../selftests/connector/thread_filter.c       |  96 ++++++++
>> 4 files changed, 378 insertions(+), 7 deletions(-)
>> create mode 100644 tools/testing/selftests/connector/thread.c
>> create mode 100644 tools/testing/selftests/connector/thread_filter.c
>> 
>> diff --git a/tools/testing/selftests/connector/Makefile b/tools/testing/selftests/connector/Makefile
>> index 92188b9bac5c..bf335826bc3b 100644
>> --- a/tools/testing/selftests/connector/Makefile
>> +++ b/tools/testing/selftests/connector/Makefile
>> @@ -1,5 +1,26 @@
>> # SPDX-License-Identifier: GPL-2.0
>> -CFLAGS += -Wall $(KHDR_INCLUDES)
>> +KERNEL="../../../.."
>> +
>> +CFLAGS += -Wall $(KHDR_INCLUDES) -I $(KERNEL)/include/uapi -I $(KERNEL)/include
>> +
>> +proc_filter: proc_filter.o
>> + cc proc_filter.o -o proc_filter
>> +
>> +proc_filter.o: proc_filter.c
>> + cc -c proc_filter.c -o proc_filter.o $(CFLAGS)
>> +
>> +thread: thread.o thread_filter.o
>> + cc thread.o thread_filter.o -o thread
>> +
>> +thread.o: thread.c $(DEPS)
>> + cc -c thread.c -o thread.o $(CFLAGS)
>> +
>> +thread_filter.o: thread_filter.c
>> + cc -c thread_filter.c -o thread_filter.o $(CFLAGS)
>> +
>> +define EXTRA_CLEAN
>> + rm *.o thread
>> +endef
>> 
>> TEST_GEN_PROGS = proc_filter
>> 
> 
> I am a little confused by this, as it seems to result in user-space
> code using kernel headers. Is that expected?

If I do not do this, then it’s not possible to run the selftests on a
system which does not have the built image installed. This allows me to
to test the selftest code in the build tree without having to install the build
kernel on the system, as it uses the header files in the build itself instead
of the ones installed on the system.
I can remove it if required.

Anjali

> 
> $ make -C tools/testing/selftests/connector
> ...
> cc -c proc_filter.c -o proc_filter.o -Wall -isystem /home/horms/projects/linux/linux/tools/testing/selftests/../../../usr/include -I "../../../.."/include/uapi -I "../../../.."/include -D_GNU_SOURCE=
> In file included from ../../../../include/uapi/linux/netlink.h:7,
>                 from proc_filter.c:11:
> ../../../../include/uapi/linux/types.h:10:2: warning: #warning "Attempt to use kernel headers from user space, see https://urldefense.com/v3/__https://kernelnewbies.org/KernelHeaders__;!!ACWV5N9M2RV99hQ!P3LdH9x7VmRu9salLGieUHe27YSyiYXyUoUdHV9Narf8UAdmXLVPyHFi41zJVKFAKVgo9eMevokFdRLwTH8p$ " [-Wcpp]
>   10 | #warning "Attempt to use kernel headers from user space, see https://urldefense.com/v3/__https://kernelnewbies.org/KernelHeaders__;!!ACWV5N9M2RV99hQ!P3LdH9x7VmRu9salLGieUHe27YSyiYXyUoUdHV9Narf8UAdmXLVPyHFi41zJVKFAKVgo9eMevokFdRLwTH8p$ "
>      |  ^~~~~~~
> ...
> 
>> diff --git a/tools/testing/selftests/connector/thread.c b/tools/testing/selftests/connector/thread.c
> 
> ...
> 
>> +static inline void init_threads(pthread_attr_t *attr)
> 
> Please don't use inline in .c files unless there is a demonstrable,
> usually performance, reason to do so.
> 
> Likewise twice more in this patch and once in patch 1/3.
> 
>> +{
>> + int ret;
>> +
>> + ret = pthread_attr_init(attr);
>> + if (ret != 0) {
>> + perror("pthread_attr_init failed");
>> + exit(ret);
>> + }
>> +
>> + ret = pthread_attr_setdetachstate(attr, PTHREAD_CREATE_DETACHED);
>> + if (ret != 0) {
>> + perror("pthread_attr_setdetachstate failed");
>> + exit(ret);
>> + }
>> +}
> 
> ...