netdev - Re: [PATCH net-next] wireguard: queuing: preserve napi

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: <CAHmME9rG1r5fJfubpcyK99g7G9YvnELq5+iW-+ms-Jb9dwPk+g@mail.gmail.com>
Date: Thu, 30 Oct 2025 18:57:34 +0100
From: "Jason A. Donenfeld" <Jason@...c4.com>
To: Miroslav Lichvar <mlichvar@...hat.com>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net-next] wireguard: queuing: preserve napi_id on decapsulation

Hi Miroslav,

On Thu, Oct 30, 2025 at 11:48 AM Miroslav Lichvar <mlichvar@...hat.com> wrote:
>
> The socket timestamping option SOF_TIMESTAMPING_OPT_PKTINFO needs the
> skb napi_id in order to provide the index of the device that captured
> the receive hardware timestamp. However, wireguard resets most of the
> skb headers, including the napi_id, which prevents the timestamping
> option from working as expected and applications that rely on it (e.g.
> chrony) from using the captured timestamps.
>
> Preserve the napi_id in wg_reset_packet() on decapsulation in order to
> make the timestamping option useful with wireguard tunnels and enable
> highly-accurate synchronization.

Thanks for the patch. Note below:

> +#if defined(CONFIG_NET_RX_BUSY_POLL) || defined(CONFIG_XPS)
> +               skb->napi_id = napi_id;
> +#endif

Seems incorrect. Although the union where napi_id is defined has that
define here:

#if defined(CONFIG_NET_RX_BUSY_POLL) || defined(CONFIG_XPS)
       union {
               unsigned int    napi_id;
               unsigned int    sender_cpu;
       };
#endif

The skb_napi_id() has the narrower condition here:

static inline unsigned int skb_napi_id(const struct sk_buff *skb)
{
#ifdef CONFIG_NET_RX_BUSY_POLL
       return skb->napi_id;
#else
       return 0;
#endif
}

So I think all we care about is CONFIG_NET_RX_BUSY_POLL.

Also,

> +       } else {

Why only do this in the !encapsulating case? Are get_timestamp() and
put_ts_pktinfo() only called when !encapsulating? What about
skb_get_tx_timestamp()? I've never looked closely at these APIs, so I
don't know. Seems like it'd be good to check into it for real.

Jason