Skip to content

Commit 79c7da9

Browse files
committed
receive: use ring buffer for incoming handshakes
Apparently the spinlock on incoming_handshake's skb_queue is highly contended, and a torrent of handshake or cookie packets can bring the data plane to its knees, simply by virtue of enqueueing the handshake packets to be processed asynchronously. So, we try switching this to a ring buffer to hopefully have less lock contention. This alleviates the problem somewhat, though it still isn't perfect, so future patches will have to improve this further. However, it at least doesn't completely diminish the data plane. Reported-by: Streun Fabio <fstreun@student.ethz.ch> Reported-by: Joel Wanner <joel.wanner@inf.ethz.ch> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
1 parent 889f958 commit 79c7da9

File tree

5 files changed

+37
-43
lines changed

5 files changed

+37
-43
lines changed

src/device.c

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ static int wg_stop(struct net_device *dev)
106106
{
107107
struct wg_device *wg = netdev_priv(dev);
108108
struct wg_peer *peer;
109+
struct sk_buff *skb;
109110

110111
mutex_lock(&wg->device_update_lock);
111112
list_for_each_entry(peer, &wg->peer_list, peer_list) {
@@ -116,7 +117,9 @@ static int wg_stop(struct net_device *dev)
116117
wg_noise_reset_last_sent_handshake(&peer->last_sent_handshake);
117118
}
118119
mutex_unlock(&wg->device_update_lock);
119-
skb_queue_purge(&wg->incoming_handshakes);
120+
while ((skb = ptr_ring_consume(&wg->handshake_queue.ring)) != NULL)
121+
kfree_skb(skb);
122+
atomic_set(&wg->handshake_queue_len, 0);
120123
wg_socket_reinit(wg, NULL, NULL);
121124
return 0;
122125
}
@@ -243,14 +246,13 @@ static void wg_destruct(struct net_device *dev)
243246
destroy_workqueue(wg->handshake_receive_wq);
244247
destroy_workqueue(wg->handshake_send_wq);
245248
destroy_workqueue(wg->packet_crypt_wq);
246-
wg_packet_queue_free(&wg->decrypt_queue);
247-
wg_packet_queue_free(&wg->encrypt_queue);
249+
wg_packet_queue_free(&wg->handshake_queue, true);
250+
wg_packet_queue_free(&wg->decrypt_queue, false);
251+
wg_packet_queue_free(&wg->encrypt_queue, false);
248252
rcu_barrier(); /* Wait for all the peers to be actually freed. */
249253
wg_ratelimiter_uninit();
250254
memzero_explicit(&wg->static_identity, sizeof(wg->static_identity));
251-
skb_queue_purge(&wg->incoming_handshakes);
252255
free_percpu(dev->tstats);
253-
free_percpu(wg->incoming_handshakes_worker);
254256
kvfree(wg->index_hashtable);
255257
kvfree(wg->peer_hashtable);
256258
mutex_unlock(&wg->device_update_lock);
@@ -312,7 +314,6 @@ static int wg_newlink(struct net *src_net, struct net_device *dev,
312314
init_rwsem(&wg->static_identity.lock);
313315
mutex_init(&wg->socket_update_lock);
314316
mutex_init(&wg->device_update_lock);
315-
skb_queue_head_init(&wg->incoming_handshakes);
316317
wg_allowedips_init(&wg->peer_allowedips);
317318
wg_cookie_checker_init(&wg->cookie_checker, wg);
318319
INIT_LIST_HEAD(&wg->peer_list);
@@ -330,16 +331,10 @@ static int wg_newlink(struct net *src_net, struct net_device *dev,
330331
if (!dev->tstats)
331332
goto err_free_index_hashtable;
332333

333-
wg->incoming_handshakes_worker =
334-
wg_packet_percpu_multicore_worker_alloc(
335-
wg_packet_handshake_receive_worker, wg);
336-
if (!wg->incoming_handshakes_worker)
337-
goto err_free_tstats;
338-
339334
wg->handshake_receive_wq = alloc_workqueue("wg-kex-%s",
340335
WQ_CPU_INTENSIVE | WQ_FREEZABLE, 0, dev->name);
341336
if (!wg->handshake_receive_wq)
342-
goto err_free_incoming_handshakes;
337+
goto err_free_tstats;
343338

344339
wg->handshake_send_wq = alloc_workqueue("wg-kex-%s",
345340
WQ_UNBOUND | WQ_FREEZABLE, 0, dev->name);
@@ -361,10 +356,15 @@ static int wg_newlink(struct net *src_net, struct net_device *dev,
361356
if (ret < 0)
362357
goto err_free_encrypt_queue;
363358

364-
ret = wg_ratelimiter_init();
359+
ret = wg_packet_queue_init(&wg->handshake_queue, wg_packet_handshake_receive_worker,
360+
MAX_QUEUED_INCOMING_HANDSHAKES);
365361
if (ret < 0)
366362
goto err_free_decrypt_queue;
367363

364+
ret = wg_ratelimiter_init();
365+
if (ret < 0)
366+
goto err_free_handshake_queue;
367+
368368
ret = register_netdevice(dev);
369369
if (ret < 0)
370370
goto err_uninit_ratelimiter;
@@ -381,18 +381,18 @@ static int wg_newlink(struct net *src_net, struct net_device *dev,
381381

382382
err_uninit_ratelimiter:
383383
wg_ratelimiter_uninit();
384+
err_free_handshake_queue:
385+
wg_packet_queue_free(&wg->handshake_queue, false);
384386
err_free_decrypt_queue:
385-
wg_packet_queue_free(&wg->decrypt_queue);
387+
wg_packet_queue_free(&wg->decrypt_queue, false);
386388
err_free_encrypt_queue:
387-
wg_packet_queue_free(&wg->encrypt_queue);
389+
wg_packet_queue_free(&wg->encrypt_queue, false);
388390
err_destroy_packet_crypt:
389391
destroy_workqueue(wg->packet_crypt_wq);
390392
err_destroy_handshake_send:
391393
destroy_workqueue(wg->handshake_send_wq);
392394
err_destroy_handshake_receive:
393395
destroy_workqueue(wg->handshake_receive_wq);
394-
err_free_incoming_handshakes:
395-
free_percpu(wg->incoming_handshakes_worker);
396396
err_free_tstats:
397397
free_percpu(dev->tstats);
398398
err_free_index_hashtable:

src/device.h

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,18 @@ struct prev_queue {
3939

4040
struct wg_device {
4141
struct net_device *dev;
42-
struct crypt_queue encrypt_queue, decrypt_queue;
42+
struct crypt_queue encrypt_queue, decrypt_queue, handshake_queue;
4343
struct sock __rcu *sock4, *sock6;
4444
struct net __rcu *creating_net;
4545
struct noise_static_identity static_identity;
46-
struct workqueue_struct *handshake_receive_wq, *handshake_send_wq;
47-
struct workqueue_struct *packet_crypt_wq;
48-
struct sk_buff_head incoming_handshakes;
49-
int incoming_handshake_cpu;
50-
struct multicore_worker __percpu *incoming_handshakes_worker;
46+
struct workqueue_struct *packet_crypt_wq,*handshake_receive_wq, *handshake_send_wq;
5147
struct cookie_checker cookie_checker;
5248
struct pubkey_hashtable *peer_hashtable;
5349
struct index_hashtable *index_hashtable;
5450
struct allowedips peer_allowedips;
5551
struct mutex device_update_lock, socket_update_lock;
5652
struct list_head device_list, peer_list;
53+
atomic_t handshake_queue_len;
5754
unsigned int num_peers, device_update_gen;
5855
u32 fwmark;
5956
u16 incoming_port;

src/queueing.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ int wg_packet_queue_init(struct crypt_queue *queue, work_func_t function,
3838
return 0;
3939
}
4040

41-
void wg_packet_queue_free(struct crypt_queue *queue)
41+
void wg_packet_queue_free(struct crypt_queue *queue, bool purge)
4242
{
4343
free_percpu(queue->worker);
44-
WARN_ON(!__ptr_ring_empty(&queue->ring));
45-
ptr_ring_cleanup(&queue->ring, NULL);
44+
WARN_ON(!purge && !__ptr_ring_empty(&queue->ring));
45+
ptr_ring_cleanup(&queue->ring, purge ? (void(*)(void*))kfree_skb : NULL);
4646
}
4747

4848
#define NEXT(skb) ((skb)->prev)

src/queueing.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ struct sk_buff;
2323
/* queueing.c APIs: */
2424
int wg_packet_queue_init(struct crypt_queue *queue, work_func_t function,
2525
unsigned int len);
26-
void wg_packet_queue_free(struct crypt_queue *queue);
26+
void wg_packet_queue_free(struct crypt_queue *queue, bool purge);
2727
struct multicore_worker __percpu *
2828
wg_packet_percpu_multicore_worker_alloc(work_func_t function, void *ptr);
2929

src/receive.c

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ static void wg_receive_handshake_packet(struct wg_device *wg,
117117
return;
118118
}
119119

120-
under_load = skb_queue_len(&wg->incoming_handshakes) >=
121-
MAX_QUEUED_INCOMING_HANDSHAKES / 8;
120+
under_load = atomic_read(&wg->handshake_queue_len) >=
121+
MAX_QUEUED_INCOMING_HANDSHAKES / 8;
122122
if (under_load) {
123123
last_under_load = ktime_get_coarse_boottime_ns();
124124
} else if (last_under_load) {
@@ -213,13 +213,14 @@ static void wg_receive_handshake_packet(struct wg_device *wg,
213213

214214
void wg_packet_handshake_receive_worker(struct work_struct *work)
215215
{
216-
struct wg_device *wg = container_of(work, struct multicore_worker,
217-
work)->ptr;
216+
struct crypt_queue *queue = container_of(work, struct multicore_worker, work)->ptr;
217+
struct wg_device *wg = container_of(queue, struct wg_device, handshake_queue);
218218
struct sk_buff *skb;
219219

220-
while ((skb = skb_dequeue(&wg->incoming_handshakes)) != NULL) {
220+
while ((skb = ptr_ring_consume_bh(&queue->ring)) != NULL) {
221221
wg_receive_handshake_packet(wg, skb);
222222
dev_kfree_skb(skb);
223+
atomic_dec(&wg->handshake_queue_len);
223224
cond_resched();
224225
}
225226
}
@@ -563,21 +564,17 @@ void wg_packet_receive(struct wg_device *wg, struct sk_buff *skb)
563564
case cpu_to_le32(MESSAGE_HANDSHAKE_RESPONSE):
564565
case cpu_to_le32(MESSAGE_HANDSHAKE_COOKIE): {
565566
int cpu;
566-
567-
if (skb_queue_len(&wg->incoming_handshakes) >
568-
MAX_QUEUED_INCOMING_HANDSHAKES ||
569-
unlikely(!rng_is_initialized())) {
567+
if (unlikely(!rng_is_initialized() ||
568+
ptr_ring_produce_bh(&wg->handshake_queue.ring, skb))) {
570569
net_dbg_skb_ratelimited("%s: Dropping handshake packet from %pISpfsc\n",
571570
wg->dev->name, skb);
572571
goto err;
573572
}
574-
skb_queue_tail(&wg->incoming_handshakes, skb);
575-
/* Queues up a call to packet_process_queued_handshake_
576-
* packets(skb):
577-
*/
578-
cpu = wg_cpumask_next_online(&wg->incoming_handshake_cpu);
573+
atomic_inc(&wg->handshake_queue_len);
574+
cpu = wg_cpumask_next_online(&wg->handshake_queue.last_cpu);
575+
/* Queues up a call to packet_process_queued_handshake_packets(skb): */
579576
queue_work_on(cpu, wg->handshake_receive_wq,
580-
&per_cpu_ptr(wg->incoming_handshakes_worker, cpu)->work);
577+
&per_cpu_ptr(wg->handshake_queue.worker, cpu)->work);
581578
break;
582579
}
583580
case cpu_to_le32(MESSAGE_DATA):

0 commit comments

Comments
 (0)