On Sun, Nov 25, 2018 at 12:06 AM Deepa Dinamani deepa.kernel@gmail.com wrote:
On Sat, Nov 24, 2018 at 7:59 PM Willem de Bruijn willemdebruijn.kernel@gmail.com wrote:
On Sat, Nov 24, 2018 at 3:59 AM Deepa Dinamani deepa.kernel@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.