netdev - Re: [PATCH net-next v4 3/3] selftests: txtimestamp: add SCM_TS_OPT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <40eab0d1-f35d-4a96-91c5-a9d050bdf236@linux.dev>
Date: Mon, 9 Sep 2024 21:37:26 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>,
 Willem de Bruijn <willemb@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, David Ahern <dsahern@...nel.org>,
 Jason Xing <kerneljasonxing@...il.com>, Simon Horman <horms@...nel.org>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net-next v4 3/3] selftests: txtimestamp: add SCM_TS_OPT_ID
 test

On 09/09/2024 21:31, Willem de Bruijn wrote:
> Vadim Fedorenko wrote:
>> On 09/09/2024 18:50, Willem de Bruijn wrote:
>>> Vadim Fedorenko wrote:
>>>> Extend txtimestamp test to run with fixed tskey using
>>>> SCM_TS_OPT_ID control message for all types of sockets.
>>>>
>>>> Reviewed-by: Jason Xing <kerneljasonxing@...il.com>
>>>> Reviewed-by: Willem de Bruijn <willemb@...gle.com>
>>>> Signed-off-by: Vadim Fedorenko <vadfed@...a.com>
>>>> ---
>>>>    tools/include/uapi/asm-generic/socket.h    |  2 +
>>>>    tools/testing/selftests/net/txtimestamp.c  | 48 +++++++++++++++++-----
>>>>    tools/testing/selftests/net/txtimestamp.sh | 12 +++---
>>>>    3 files changed, 47 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/tools/include/uapi/asm-generic/socket.h b/tools/include/uapi/asm-generic/socket.h
>>>> index 54d9c8bf7c55..281df9139d2b 100644
>>>> --- a/tools/include/uapi/asm-generic/socket.h
>>>> +++ b/tools/include/uapi/asm-generic/socket.h
>>>> @@ -124,6 +124,8 @@
>>>>    #define SO_PASSPIDFD		76
>>>>    #define SO_PEERPIDFD		77
>>>>    
>>>> +#define SCM_TS_OPT_ID		78
>>>> +
>>>>    #if !defined(__KERNEL__)
>>>>    
>>>>    #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
>>>> diff --git a/tools/testing/selftests/net/txtimestamp.c b/tools/testing/selftests/net/txtimestamp.c
>>>> index ec60a16c9307..bdd0eb74326c 100644
>>>> --- a/tools/testing/selftests/net/txtimestamp.c
>>>> +++ b/tools/testing/selftests/net/txtimestamp.c
>>>> @@ -54,6 +54,10 @@
>>>>    #define USEC_PER_SEC	1000000L
>>>>    #define NSEC_PER_SEC	1000000000LL
>>>>    
>>>> +#ifndef SCM_TS_OPT_ID
>>>> +# define SCM_TS_OPT_ID 78
>>>> +#endif
>>>
>>> This should not be needed. And along with the uapi change above means
>>> the test will be broken on other platforms.
>>>
>>> (SO|SCM)_TXTIME ostensibly has the same issue and does not do this.
>>
>> I had the same feeling, but apparently I wasn't able to build tests
>> without this addition. Looks like selftests rely on system's uapi rather
>> the one provided in tool/include/uapi.
> 
> Right, as they should.
> 
> make headers_install will install headers by default under $KSRC/usr
> 
> tools/testing/selftests/net/Makefile has
> 
> CFLAGS += -I../../../../usr/include/ $(KHDR_INCLUDES)
> 
> Haven't tried, but I assume this will pick up the right header
> depending on the arch.

I see, thanks!
I'll respin (in usual 24 hours timeout) the series with nits fixes and 
will remove this ifdefs in selftests.

>> With SCM_TXTIME it worked because the option was added back in 2018 in
>> 80b14dee2bea ("net: Add a new socket option for a future transmit
>> time.") by Richard while tests were added in 2019 by yourself in
>> af5136f95045 ("selftests/net: SO_TXTIME with ETF and FQ").
>>
>> Though selftests do miss uapi for other architectures, it might be a
>> good reason to respin the series, but fixing selftests infra is a bit
>> different story, I believe... I may try to fix it and post another
>> series.
> 
>