Re: [PATCH net-next v4 3/3] selftests: txtimestamp: add SCM_TS_OPT_ID test

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

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@gmail.com>
>> Reviewed-by: Willem de Bruijn <willemb@google.com>
>> Signed-off-by: Vadim Fedorenko <vadfed@meta.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.
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.