On Sun, Nov 25, 2018 at 12:06 AM Deepa Dinamani <deepa.ker...@gmail.com> wrote: > > On Sat, Nov 24, 2018 at 7:59 PM Willem de Bruijn > <willemdebruijn.ker...@gmail.com> wrote: > > > > On Sat, Nov 24, 2018 at 3:59 AM Deepa Dinamani <deepa.ker...@gmail.com> > > wrote: > > > > > > SOCK_RCVTSTAMPNS is never set alone. SOCK_RCVTSTAMP > > > is always set along with SOCK_RCVTSTAMPNS. This leads to > > > checking for two flag states whenever we need to check for > > > SOCK_RCVTSTAMPS. > > > > > > Also SOCK_RCVTSTAMPS was the only flag that needed to be > > > checked in order to verify if either of the two flags are > > > set. But, the two features are not actually dependent on > > > each other. This artificial dependency creates more > > > confusion. > > > > This is done so that the hot path only has to check one flag > > in the common case where no timestamp is requested. > > In that case we could just check it this way: > > if (newsk->sk_flags & SK_FLAGS_TIMESTAMP) > > We are already doing this in many places. > > I do not see any other reason for the two timestamps to be intertwined. > > Do you have any objections to using this patch and replacing the > checks as above?
The existing logic is as is for a reason. There is no need to change it to satisfy the main purpose of your patchset? It is structured as one bit to test whether a timestamp is requested and another to select among two variants usec/nsec. Just add another layer of branching between new/old in cases where this distinction is needed. Please avoid code churn unless needed.