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

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

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@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.

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.

> 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.