* [PATCH net v2] wireguard: wait for per-peer crypto during removal
@ 2026-09-13 12:19 Chris J Arges
2026-09-16 10:06 ` Jason A. Donenfeld
0 siblings, 1 reply; 3+ messages in thread
From: Chris J Arges @ 2026-09-13 12:19 UTC (permalink / raw)
To: Jason A. Donenfeld, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: wireguard, netdev, linux-kernel, kernel-team, Chris J Arges
Calling peer_remove_after_dead() currently flushes device-wide packet
crypto and handshake workqueues while holding RTNL. This is problematic as
unrelated peers can continue adding work to those queues, blocking other
tasks that want to take the RTNL lock.
Instead, this patch tracks pending crypto handoffs for each peer using a
counter. After marking the peer dead, synchronize_net() prevents new
submissions. Next, wait for pending crypto workers to schedule TX work and
for RX NAPI to drain the peer's RX queue. Then flush only the peer's TX
packet and handshake work.
This scopes teardown synchronization to the removed peer and prevents
unrelated peers from extending the RTNL hold time.
Fixes: e7096c131e51 ("net: WireGuard secure network tunnel")
Signed-off-by: Chris J Arges <carges@cloudflare.com>
---
Changes in v2:
- wait for RX packets to drain from peer’s NAPI queue
- Link to v1: https://patch.msgid.link/20260902-fix-wg-peer-removal-v1-1-7a0190f5cdb1@cloudflare.com
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Andrew Lunn <andrew+netdev@lunn.ch>
To: "David S. Miller" <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
Cc: wireguard@lists.zx2c4.com
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/net/wireguard/peer.c | 30 ++++++++++++++++--------------
drivers/net/wireguard/peer.h | 1 +
drivers/net/wireguard/queueing.h | 4 ++++
drivers/net/wireguard/receive.c | 4 +++-
4 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wireguard/peer.c b/drivers/net/wireguard/peer.c
index 1cb502a932e0..34842b33b28a 100644
--- a/drivers/net/wireguard/peer.c
+++ b/drivers/net/wireguard/peer.c
@@ -14,6 +14,7 @@
#include <linux/lockdep.h>
#include <linux/rcupdate.h>
#include <linux/list.h>
+#include <linux/wait_bit.h>
static struct kmem_cache *peer_cache;
static atomic64_t peer_counter = ATOMIC64_INIT(0);
@@ -49,6 +50,8 @@ struct wg_peer *wg_peer_create(struct wg_device *wg,
INIT_WORK(&peer->transmit_packet_work, wg_packet_tx_worker);
wg_prev_queue_init(&peer->tx_queue);
wg_prev_queue_init(&peer->rx_queue);
+ /* Keep this above zero until teardown prevents new packet handoffs. */
+ atomic_set(&peer->packet_crypt_pending, 1);
rwlock_init(&peer->endpoint_lock);
kref_init(&peer->refcount);
skb_queue_head_init(&peer->staged_packet_queue);
@@ -105,28 +108,27 @@ static void peer_remove_after_dead(struct wg_peer *peer)
*/
wg_timers_stop(peer);
- /* The transition between packet encryption/decryption queues isn't
- * guarded by is_dead, but each reference's life is strictly bounded by
- * two generations: once for parallel crypto and once for serial
- * ingestion, so we can simply flush twice, and be sure that we no
- * longer have references inside these queues.
+ /* Lookup removal and is_dead prevent new packets from entering the
+ * parallel crypto queues after synchronize_net() waits for pre-existing
+ * submission paths. Drop the initial count and wait for existing TX
+ * packets to schedule their serial work and RX packets to leave rx_queue.
*/
+ atomic_dec(&peer->packet_crypt_pending);
+ wait_var_event(&peer->packet_crypt_pending,
+ !atomic_read_acquire(&peer->packet_crypt_pending));
+
+ flush_work(&peer->transmit_packet_work);
- /* a) For encrypt/decrypt. */
- flush_workqueue(peer->device->packet_crypt_wq);
- /* b.1) For send (but not receive, since that's napi). */
- flush_workqueue(peer->device->packet_crypt_wq);
- /* b.2.1) For receive (but not send, since that's wq). */
napi_disable(&peer->napi);
- /* b.2.1) It's now safe to remove the napi struct, which must be done
+ /* It's now safe to remove the napi struct, which must be done
* here from process context.
*/
netif_napi_del(&peer->napi);
- /* Ensure any workstructs we own (like transmit_handshake_work or
- * clear_peer_work) no longer are in use.
+ /* clear_peer_work was flushed by wg_timers_stop(). Ensure the remaining
+ * peer-owned handshake work is no longer in use.
*/
- flush_workqueue(peer->device->handshake_send_wq);
+ flush_work(&peer->transmit_handshake_work);
/* After the above flushes, a peer might still be active in a few
* different contexts: 1) from xmit(), before hitting is_dead and
diff --git a/drivers/net/wireguard/peer.h b/drivers/net/wireguard/peer.h
index 718fb42bdac7..64412c67f413 100644
--- a/drivers/net/wireguard/peer.h
+++ b/drivers/net/wireguard/peer.h
@@ -37,6 +37,7 @@ struct endpoint {
struct wg_peer {
struct wg_device *device;
struct prev_queue tx_queue, rx_queue;
+ atomic_t packet_crypt_pending;
struct sk_buff_head staged_packet_queue;
int serial_work_cpu;
bool is_dead;
diff --git a/drivers/net/wireguard/queueing.h b/drivers/net/wireguard/queueing.h
index 79b6d70de236..5ce57565c8ff 100644
--- a/drivers/net/wireguard/queueing.h
+++ b/drivers/net/wireguard/queueing.h
@@ -11,6 +11,7 @@
#include <linux/skbuff.h>
#include <linux/ip.h>
#include <linux/ipv6.h>
+#include <linux/wait_bit.h>
#include <net/ip_tunnels.h>
struct wg_device;
@@ -161,6 +162,7 @@ static inline int wg_queue_enqueue_per_device_and_peer(
*/
if (unlikely(!wg_prev_queue_enqueue(peer_queue, skb)))
return -ENOSPC;
+ atomic_inc(&PACKET_PEER(skb)->packet_crypt_pending);
/* Then we queue it up in the device queue, which consumes the
* packet as soon as it can.
@@ -182,6 +184,8 @@ static inline void wg_queue_enqueue_per_peer_tx(struct sk_buff *skb, enum packet
atomic_set_release(&PACKET_CB(skb)->state, state);
queue_work_on(wg_cpumask_choose_online(&peer->serial_work_cpu, peer->internal_id),
peer->device->packet_crypt_wq, &peer->transmit_packet_work);
+ if (atomic_dec_and_test(&peer->packet_crypt_pending))
+ wake_up_var(&peer->packet_crypt_pending);
wg_peer_put(peer);
}
diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c
index 824bbefce61c..bb35e3205491 100644
--- a/drivers/net/wireguard/receive.c
+++ b/drivers/net/wireguard/receive.c
@@ -476,9 +476,11 @@ int wg_packet_rx_poll(struct napi_struct *napi, int budget)
next:
wg_noise_keypair_put(keypair, false);
- wg_peer_put(peer);
if (unlikely(free))
dev_kfree_skb(skb);
+ if (atomic_dec_and_test(&peer->packet_crypt_pending))
+ wake_up_var(&peer->packet_crypt_pending);
+ wg_peer_put(peer);
if (++work_done >= budget)
break;
---
base-commit: 70f3995830d3f1e79faa14eb0605914f778feca9
change-id: 20260901-fix-wg-peer-removal-43fc390d0311
Best regards,
--
Chris J Arges <carges@cloudflare.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net v2] wireguard: wait for per-peer crypto during removal
2026-09-13 12:19 [PATCH net v2] wireguard: wait for per-peer crypto during removal Chris J Arges
@ 2026-09-16 10:06 ` Jason A. Donenfeld
2026-09-16 16:40 ` Chris Arges
0 siblings, 1 reply; 3+ messages in thread
From: Jason A. Donenfeld @ 2026-09-16 10:06 UTC (permalink / raw)
To: Chris J Arges
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, wireguard, netdev, linux-kernel, kernel-team
On Sun, Sep 13, 2026 at 07:19:08AM -0500, Chris J Arges wrote:
> Calling peer_remove_after_dead() currently flushes device-wide packet
> crypto and handshake workqueues while holding RTNL. This is problematic as
> unrelated peers can continue adding work to those queues, blocking other
> tasks that want to take the RTNL lock.
>
> Instead, this patch tracks pending crypto handoffs for each peer using a
> counter. After marking the peer dead, synchronize_net() prevents new
> submissions. Next, wait for pending crypto workers to schedule TX work and
> for RX NAPI to drain the peer's RX queue. Then flush only the peer's TX
> packet and handshake work.
>
> This scopes teardown synchronization to the removed peer and prevents
> unrelated peers from extending the RTNL hold time.
> struct wg_device;
> @@ -161,6 +162,7 @@ static inline int wg_queue_enqueue_per_device_and_peer(
> */
> if (unlikely(!wg_prev_queue_enqueue(peer_queue, skb)))
> return -ENOSPC;
> + atomic_inc(&PACKET_PEER(skb)->packet_crypt_pending);
>
> /* Then we queue it up in the device queue, which consumes the
> * packet as soon as it can.
> @@ -182,6 +184,8 @@ static inline void wg_queue_enqueue_per_peer_tx(struct sk_buff *skb, enum packet
> atomic_set_release(&PACKET_CB(skb)->state, state);
> queue_work_on(wg_cpumask_choose_online(&peer->serial_work_cpu, peer->internal_id),
> peer->device->packet_crypt_wq, &peer->transmit_packet_work);
> + if (atomic_dec_and_test(&peer->packet_crypt_pending))
> + wake_up_var(&peer->packet_crypt_pending);
> wg_peer_put(peer);
> }
>
> diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c
> index 824bbefce61c..bb35e3205491 100644
> --- a/drivers/net/wireguard/receive.c
> +++ b/drivers/net/wireguard/receive.c
> @@ -476,9 +476,11 @@ int wg_packet_rx_poll(struct napi_struct *napi, int budget)
>
> next:
> wg_noise_keypair_put(keypair, false);
> - wg_peer_put(peer);
> if (unlikely(free))
> dev_kfree_skb(skb);
> + if (atomic_dec_and_test(&peer->packet_crypt_pending))
> + wake_up_var(&peer->packet_crypt_pending);
> + wg_peer_put(peer);
This adds two atomic updates to a per-peer counter for every RX packet
and TX batch. That could introduce contention across crypto workers. I
suppose it'd be good to see some measurements in if this changes
anything. Certainly it should change _something_. Question is by how
much.
Jason
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net v2] wireguard: wait for per-peer crypto during removal
2026-09-16 10:06 ` Jason A. Donenfeld
@ 2026-09-16 16:40 ` Chris Arges
0 siblings, 0 replies; 3+ messages in thread
From: Chris Arges @ 2026-09-16 16:40 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, wireguard, netdev, linux-kernel, kernel-team
On 2026-09-16 12:06:08, Jason A. Donenfeld wrote:
> On Sun, Sep 13, 2026 at 07:19:08AM -0500, Chris J Arges wrote:
> > Calling peer_remove_after_dead() currently flushes device-wide packet
> > crypto and handshake workqueues while holding RTNL. This is problematic as
> > unrelated peers can continue adding work to those queues, blocking other
> > tasks that want to take the RTNL lock.
> >
> > Instead, this patch tracks pending crypto handoffs for each peer using a
> > counter. After marking the peer dead, synchronize_net() prevents new
> > submissions. Next, wait for pending crypto workers to schedule TX work and
> > for RX NAPI to drain the peer's RX queue. Then flush only the peer's TX
> > packet and handshake work.
> >
> > This scopes teardown synchronization to the removed peer and prevents
> > unrelated peers from extending the RTNL hold time.
> > struct wg_device;
> > @@ -161,6 +162,7 @@ static inline int wg_queue_enqueue_per_device_and_peer(
> > */
> > if (unlikely(!wg_prev_queue_enqueue(peer_queue, skb)))
> > return -ENOSPC;
> > + atomic_inc(&PACKET_PEER(skb)->packet_crypt_pending);
> >
> > /* Then we queue it up in the device queue, which consumes the
> > * packet as soon as it can.
> > @@ -182,6 +184,8 @@ static inline void wg_queue_enqueue_per_peer_tx(struct sk_buff *skb, enum packet
> > atomic_set_release(&PACKET_CB(skb)->state, state);
> > queue_work_on(wg_cpumask_choose_online(&peer->serial_work_cpu, peer->internal_id),
> > peer->device->packet_crypt_wq, &peer->transmit_packet_work);
> > + if (atomic_dec_and_test(&peer->packet_crypt_pending))
> > + wake_up_var(&peer->packet_crypt_pending);
> > wg_peer_put(peer);
> > }
> >
> > diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c
> > index 824bbefce61c..bb35e3205491 100644
> > --- a/drivers/net/wireguard/receive.c
> > +++ b/drivers/net/wireguard/receive.c
> > @@ -476,9 +476,11 @@ int wg_packet_rx_poll(struct napi_struct *napi, int budget)
> >
> > next:
> > wg_noise_keypair_put(keypair, false);
> > - wg_peer_put(peer);
> > if (unlikely(free))
> > dev_kfree_skb(skb);
> > + if (atomic_dec_and_test(&peer->packet_crypt_pending))
> > + wake_up_var(&peer->packet_crypt_pending);
> > + wg_peer_put(peer);
>
> This adds two atomic updates to a per-peer counter for every RX packet
> and TX batch. That could introduce contention across crypto workers. I
> suppose it'd be good to see some measurements in if this changes
> anything. Certainly it should change _something_. Question is by how
> much.
>
> Jason
Thanks, I'll get some numbers here and share. We're testing this on some
production machines now. In addition I'll get numbers from my synthetic testing
and report back.
--chris
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-16 16:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-13 12:19 [PATCH net v2] wireguard: wait for per-peer crypto during removal Chris J Arges
2026-09-16 10:06 ` Jason A. Donenfeld
2026-09-16 16:40 ` Chris Arges
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®