[PATCH net-next] wireguard: queuing: preserve napi_id on decapsulation

[PATCH net-next] wireguard: queuing: preserve napi_id on decapsulation

[Miroslav Lichvar]

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.

Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
---

Notes:
    This is a minimal change that fixes the described problem for me, but
    I don't really understand the code well enough to see if there are any
    major side effects. If there is a better way to fix the option for wg or
    tunnels in general, please let me know.

 drivers/net/wireguard/queueing.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireguard/queueing.h b/drivers/net/wireguard/queueing.h
index 79b6d70de236..bcf03bc01992 100644
--- a/drivers/net/wireguard/queueing.h
+++ b/drivers/net/wireguard/queueing.h
@@ -75,6 +75,7 @@ static inline bool wg_check_packet_protocol(struct sk_buff *skb)
 
 static inline void wg_reset_packet(struct sk_buff *skb, bool encapsulating)
 {
+	unsigned int napi_id = skb_napi_id(skb);
 	u8 l4_hash = skb->l4_hash;
 	u8 sw_hash = skb->sw_hash;
 	u32 hash = skb->hash;
@@ -84,6 +85,10 @@ static inline void wg_reset_packet(struct sk_buff *skb, bool encapsulating)
 		skb->l4_hash = l4_hash;
 		skb->sw_hash = sw_hash;
 		skb->hash = hash;
+	} else {
+#if defined(CONFIG_NET_RX_BUSY_POLL) || defined(CONFIG_XPS)
+		skb->napi_id = napi_id;
+#endif
 	}
 	skb->queue_mapping = 0;
 	skb->nohdr = 0;
-- 
2.51.0

Re: [PATCH net-next] wireguard: queuing: preserve napi_id on decapsulation

[Jason A. Donenfeld]

Hi Miroslav,

On Thu, Oct 30, 2025 at 11:48 AM Miroslav Lichvar <mlichvar@redhat.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

Re: [PATCH net-next] wireguard: queuing: preserve napi_id on decapsulation

[Miroslav Lichvar]

On Thu, Oct 30, 2025 at 06:57:34PM +0100, Jason A. Donenfeld wrote:
> > +#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:

> So I think all we care about is CONFIG_NET_RX_BUSY_POLL.

Ok, I'll fix that.

> > +       } else {
> 
> Why only do this in the !encapsulating case? Are get_timestamp() and
> put_ts_pktinfo() only called when !encapsulating?

Yes, I think so. The cmsg enabled by the timestamping option is added
only for received packets. Transmit timestamps are provided separately
with the packet looped back to the error queue (the timestamp is known
only after the actual transmission) and that can already provide the
index of the physical device in the IP_PKTINFO cmsg, which seems to
work correctly even with wireguard interfaces.

I'm not sure if the napi_id is actually used when sending. In my tests
it looked like it's the other field of the union, the sender cpu
index.

Thanks,

-- 
Miroslav Lichvar