3 reasons for not doing this:

1. We do not want to break userspace. If we move this to
linux/socket.h all the userspace programs now have to include
linux/socket.h or get this definition through a new libc.
2. All the socket options are together in the file asm/socket.h. It
doesn't seem good for maintainability to move just a few bits
elsewhere.
3. There are only 4 arches (after the series is applied) that have
their own asm/socket.h. And, this is because there seems to be
significant differences to asm-generic/socket.h that don't seem
logically obvious to group and eliminate some of the defines.

Agreed. All good reasons to leave as is.

Also for the other comment. The reason the conditionals were not
consistent is because they were not consistent to begin with.

The only difference I see is an inversion of the test. Nesting order
is the same:

        int need_software_tstamp = sock_flag(sk, SOCK_RCVTSTAMP);
        ...
        if (need_software_tstamp) {
                if (!sock_flag(sk, SOCK_RCVTSTAMPNS)) {
                } else {
                }
        }

vs

                if (sock_flag(sk, SOCK_RCVTSTAMP)) {
                        if (sock_flag(sk, SOCK_RCVTSTAMPNS)) {
                        } else {
                        }
                }

I suggest just adding something like

        if (need_software_tstamp) {
+              if (sock_uses_new_tstamp(sk) {
+                   __sock_recv_timestamp_new(msg, sk,
ktime_to_timespec64(skb->tstamp));
+              } else if (!sock_flag(sk, SOCK_RCVTSTAMPNS)) {
-               if (!sock_flag(sk, SOCK_RCVTSTAMPNS)) {
                } else {
                }

and

                if (sock_flag(sk, SOCK_RCVTSTAMP)) {
+                      if (sock_uses_new_tstamp(sk) {
+                           __sock_recv_timestamp_new(msg, sk, ts);
+                      else if (sock_flag(sk, SOCK_RCVTSTAMPNS)) {
-                       if (sock_flag(sk, SOCK_RCVTSTAMPNS)) {
                        } else {
                        }

I think we can use the same helper for both the sock and tcp variant.
The only intended difference between the two functions, as described
in the tcp_recv_timestamp function comment, is the absence of an skb
in the tcp case. That is immaterial at this level.

Note also (2) tentative helper function sock_uses_new_tstamp(const
struct sock *sk) instead of testing sock_flag(sk, SOCK_TSTAMP_NEW)
directly. Since the .._NEW variants are equivalent to .._OLD on 64-bit,
I wonder if we can just compile out the branch. Something like

    static inline bool sock_uses_new_tstamp(const struct sock *sk) {
            return (sizeof(time_t) != sizeof(__kernel_long_t)) &&
                       sock_flag(sk, SOCK_TSTAMP_NEW);
    }

I'm trying to follow your request to keep code churn to minimal.
It's just that I moved to a different function as that seemed logical
to me. Do you prefer me to remove that refactoring?

Yes, please avoid rearranging existing code as much as possible.

If there is any refactoring to be done, I think it would be to
deduplicate the shared logic between __sock_recv_timestamp and
tcp_recv_timestamp. I think the first can be rewritten to reuse the
second, if the only difference really is that the first takes an skb with
embedded timestamps, while the second directly takes a pointer to
struct scm_timestamping.

Either way, that's out of scope for this patchset.