On Sun, Nov 25, 2018 at 12:06 AM Deepa Dinamani [off-list ref] wrote:

On Sat, Nov 24, 2018 at 7:59 PM Willem de Bruijn
[off-list ref] wrote:

quoted

On Sat, Nov 24, 2018 at 3:59 AM Deepa Dinamani [off-list ref] wrote:

quoted

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.