netdev - Re: [RFC PATCH 0/2] udp: avoid false sharing on sk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iJA1FRWAcwxwDCYxOV-AnCrdT5UEvdAhRnuCKafnwT7ug@mail.gmail.com>
Date: Mon, 10 Feb 2025 22:26:48 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: Willem de Bruijn <willemdebruijn.kernel@...il.com>, netdev@...r.kernel.org, 
	Kuniyuki Iwashima <kuniyu@...zon.com>, "David S. Miller" <davem@...emloft.net>, 
	Jakub Kicinski <kuba@...nel.org>, Simon Horman <horms@...nel.org>, Neal Cardwell <ncardwell@...gle.com>, 
	David Ahern <dsahern@...nel.org>
Subject: Re: [RFC PATCH 0/2] udp: avoid false sharing on sk_tsflags

On Mon, Feb 10, 2025 at 10:24 PM Paolo Abeni <pabeni@...hat.com> wrote:
>
> On 2/10/25 5:16 PM, Paolo Abeni wrote:
> > I expect the change you propose would perform alike the RFC patches, but
> > I'll try to do an explicit test later (and report here the results).
>
> I ran my test on the sock layout change, and it gave the same (good)
> results as the RFC. Note that such test uses a single socket receiver,
> so it's not affected in any way by the eventual increase of touched
> 'struct sock' cachelines.
>
> BTW it just occurred to me that if we could use another bit from
> sk_flags, something alike the following (completely untested!!!) would
> do, without changing the struct sock layout and without adding other
> sock proto ops:
>
> ---
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 8036b3b79cd8..a526db7f5c60 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -954,6 +954,7 @@ enum sock_flags {
>         SOCK_TSTAMP_NEW, /* Indicates 64 bit timestamps always */
>         SOCK_RCVMARK, /* Receive SO_MARK  ancillary data with packet */
>         SOCK_RCVPRIORITY, /* Receive SO_PRIORITY ancillary data with packet */
> +       SOCK_TIMESTAMPING_ANY, /* sk_tsflags & TSFLAGS_ANY */
>  };
>
>  #define SK_FLAGS_TIMESTAMP ((1UL << SOCK_TIMESTAMP) | (1UL <<
> SOCK_TIMESTAMPING_RX_SOFTWARE))
> @@ -2665,12 +2666,12 @@ static inline void sock_recv_cmsgs(struct msghdr
> *msg, struct sock *sk,
>  #define FLAGS_RECV_CMSGS ((1UL << SOCK_RXQ_OVFL)                       | \
>                            (1UL << SOCK_RCVTSTAMP)                      | \
>                            (1UL << SOCK_RCVMARK)                        |\
> -                          (1UL << SOCK_RCVPRIORITY))
> +                          (1UL << SOCK_RCVPRIORITY)                    |\
> +                          (1UL << SOCK_TIMESTAMPING_ANY))
>  #define TSFLAGS_ANY      (SOF_TIMESTAMPING_SOFTWARE                    | \
>                            SOF_TIMESTAMPING_RAW_HARDWARE)
>
> -       if (sk->sk_flags & FLAGS_RECV_CMSGS ||
> -           READ_ONCE(sk->sk_tsflags) & TSFLAGS_ANY)
> +       if (sk->sk_flags & FLAGS_RECV_CMSGS)
>                 __sock_recv_cmsgs(msg, sk, skb);
>         else if (unlikely(sock_flag(sk, SOCK_TIMESTAMP)))
>                 sock_write_timestamp(sk, skb->tstamp);
> diff --git a/net/core/sock.c b/net/core/sock.c
> index eae2ae70a2e0..a197f0a0b878 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -938,6 +938,7 @@ int sock_set_timestamping(struct sock *sk, int optname,
>
>         WRITE_ONCE(sk->sk_tsflags, val);
>         sock_valbool_flag(sk, SOCK_TSTAMP_NEW, optname == SO_TIMESTAMPING_NEW);
> +       sock_valbool_flag(sk, SOCK_TIMESTAMPING_ANY, !!(val & TSFLAGS_ANY));
>
>         if (val & SOF_TIMESTAMPING_RX_SOFTWARE)
>                 sock_enable_timestamp(sk,

This looks nice indeed.