Hi Willem,
On 06/28/2018 07:40 AM, Willem de Bruijn wrote: > On Thu, Jun 28, 2018 at 10:26 AM Willem de Bruijn > <willemdebruijn.ker...@gmail.com> wrote: >> >> On Wed, Jun 27, 2018 at 6:08 PM Jesus Sanchez-Palencia >> <jesus.sanchez-palen...@intel.com> wrote: >>> >>> From: Richard Cochran <rcoch...@linutronix.de> >>> >>> This patch introduces SO_TXTIME. User space enables this option in >>> order to pass a desired future transmit time in a CMSG when calling >>> sendmsg(2). The argument to this socket option is a 6-bytes long struct >>> defined as: >>> >>> struct sock_txtime { >>> clockid_t clockid; >>> u16 flags; >>> }; >> >> clockid_t is __kernel_clockid_t is int is a variable length field. >> Please use fixed length fields. > > Sorry, int is fine, of course, and clockid_t is used between userspace and > kernel already. Great. So, in addition to the other feedback in sock.c, what I'm thinking here for the v2 is: - move this struct to and the flags definition (as enums) to include/uapi/linux/net_tstamp.h; - keep clockid as a clockid_t and increase flags to u32 since this already takes 8 bytes in total anyway; - reduce sk_clockid and sk_txtime_flags from struct sock from a u16 to a u8 each. Thanks, Jesus > >> Also, as MAX_CLOCKS is 16, only 4 bits are needed. A single u16 >> is probably sufficient as cmsg argument. To future proof, a u32 will >> allow for more >> than 4 flags. But in struct sock, 16 bits should be sufficient to >> encode both clock id >> and flags.