Jason Xing wrote: > On Wed, Oct 16, 2024 at 5:56 AM Willem de Bruijn > <willemdebruijn.kernel@xxxxxxxxx> wrote: > > > > Martin KaFai Lau wrote: > > > On 10/11/24 9:06 PM, Jason Xing wrote: > > > > static int sol_socket_sockopt(struct sock *sk, int optname, > > > > char *optval, int *optlen, > > > > bool getopt) > > > > { > > > > + struct so_timestamping ts; > > > > + int ret = 0; > > > > + > > > > switch (optname) { > > > > case SO_REUSEADDR: > > > > case SO_SNDBUF: > > > > @@ -5225,6 +5245,13 @@ static int sol_socket_sockopt(struct sock *sk, int optname, > > > > break; > > > > case SO_BINDTODEVICE: > > > > break; > > > > + case SO_TIMESTAMPING_NEW: > > > > + case SO_TIMESTAMPING_OLD: > > > > > > How about remove the "_OLD" support ? > > > > +1 I forgot to mention that yesterday. > > Hello Willem, Martin, > > I did a test on this and found that if we only use > SO_TIMESTAMPING_NEW, we will never enter the real set sk_tsflags_bpf > logic, unless there is "case SO_TIMESTAMPING_OLD". > > And I checked SO_TIMESTAMPING in include/uapi/asm-generic/socket.h: > #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__)) > /* on 64-bit and x32, avoid the ?: operator */ > ... > #define SO_TIMESTAMPING SO_TIMESTAMPING_OLD > ... > #else > ... > #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? > SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW) > ... > #endif > > The SO_TIMESTAMPING is defined as SO_TIMESTAMPING_OLD. I wonder if I > missed something? Thanks in advance. The _NEW vs _OLD aim to deal with y2038 issues on 32-bit platforms. For new APIs, like BPF timestamping, we should always use the safe structs, such as timespec64. Then we can just use SO_TIMESTAMPING without the NEW or OLD suffix.