wireguard: add barrier to sync data
The current implmentation of the hash table is not thread-safe.
This design leads to a segfault when VPP is handling a lot of tunnels
for Wireguard, where one thread modifies the hash table and other
threads start the lookup at the same time.
This fix adds a barrier sync to the hash table access when Wireguard
adds or deletes an element.
Type: fix
Signed-off-by: Gabriel Oginski <gabrielx.oginski@intel.com>
Change-Id: Id460dfcd46ace17c7bdcd23bd9687d26cecf0a39
diff --git a/src/plugins/wireguard/wireguard_if.c b/src/plugins/wireguard/wireguard_if.c
index a869df0..ee744e7 100644
--- a/src/plugins/wireguard/wireguard_if.c
+++ b/src/plugins/wireguard/wireguard_if.c
@@ -116,20 +116,20 @@
}
static uint32_t
-wg_index_set (noise_remote_t * remote)
+wg_index_set (vlib_main_t *vm, noise_remote_t *remote)
{
wg_main_t *wmp = &wg_main;
u32 rnd_seed = (u32) (vlib_time_now (wmp->vlib_main) * 1e6);
u32 ret =
- wg_index_table_add (&wmp->index_table, remote->r_peer_idx, rnd_seed);
+ wg_index_table_add (vm, &wmp->index_table, remote->r_peer_idx, rnd_seed);
return ret;
}
static void
-wg_index_drop (uint32_t key)
+wg_index_drop (vlib_main_t *vm, uint32_t key)
{
wg_main_t *wmp = &wg_main;
- wg_index_table_del (&wmp->index_table, key);
+ wg_index_table_del (vm, &wmp->index_table, key);
}
static clib_error_t *
diff --git a/src/plugins/wireguard/wireguard_index_table.c b/src/plugins/wireguard/wireguard_index_table.c
index 5f81204..da53bfd 100644
--- a/src/plugins/wireguard/wireguard_index_table.c
+++ b/src/plugins/wireguard/wireguard_index_table.c
@@ -13,13 +13,15 @@
* limitations under the License.
*/
+#include <vlib/vlib.h>
#include <vppinfra/hash.h>
#include <vppinfra/pool.h>
#include <vppinfra/random.h>
#include <wireguard/wireguard_index_table.h>
u32
-wg_index_table_add (wg_index_table_t * table, u32 peer_pool_idx, u32 rnd_seed)
+wg_index_table_add (vlib_main_t *vm, wg_index_table_t *table,
+ u32 peer_pool_idx, u32 rnd_seed)
{
u32 key;
@@ -29,19 +31,25 @@
if (hash_get (table->hash, key))
continue;
+ vlib_worker_thread_barrier_sync (vm);
hash_set (table->hash, key, peer_pool_idx);
+ vlib_worker_thread_barrier_release (vm);
break;
}
return key;
}
void
-wg_index_table_del (wg_index_table_t * table, u32 key)
+wg_index_table_del (vlib_main_t *vm, wg_index_table_t *table, u32 key)
{
uword *p;
p = hash_get (table->hash, key);
if (p)
- hash_unset (table->hash, key);
+ {
+ vlib_worker_thread_barrier_sync (vm);
+ hash_unset (table->hash, key);
+ vlib_worker_thread_barrier_release (vm);
+ }
}
u32 *
diff --git a/src/plugins/wireguard/wireguard_index_table.h b/src/plugins/wireguard/wireguard_index_table.h
index 67cae1f..e9aa374 100644
--- a/src/plugins/wireguard/wireguard_index_table.h
+++ b/src/plugins/wireguard/wireguard_index_table.h
@@ -16,6 +16,7 @@
#ifndef __included_wg_index_table_h__
#define __included_wg_index_table_h__
+#include <vlib/vlib.h>
#include <vppinfra/types.h>
typedef struct
@@ -23,9 +24,9 @@
uword *hash;
} wg_index_table_t;
-u32 wg_index_table_add (wg_index_table_t * table, u32 peer_pool_idx,
- u32 rnd_seed);
-void wg_index_table_del (wg_index_table_t * table, u32 key);
+u32 wg_index_table_add (vlib_main_t *vm, wg_index_table_t *table,
+ u32 peer_pool_idx, u32 rnd_seed);
+void wg_index_table_del (vlib_main_t *vm, wg_index_table_t *table, u32 key);
u32 *wg_index_table_lookup (const wg_index_table_t * table, u32 key);
#endif //__included_wg_index_table_h__
diff --git a/src/plugins/wireguard/wireguard_noise.c b/src/plugins/wireguard/wireguard_noise.c
index c9d8e31..19b0ce5 100644
--- a/src/plugins/wireguard/wireguard_noise.c
+++ b/src/plugins/wireguard/wireguard_noise.c
@@ -33,8 +33,10 @@
static noise_keypair_t *noise_remote_keypair_allocate (noise_remote_t *);
static void noise_remote_keypair_free (vlib_main_t * vm, noise_remote_t *,
noise_keypair_t **);
-static uint32_t noise_remote_handshake_index_get (noise_remote_t *);
-static void noise_remote_handshake_index_drop (noise_remote_t *);
+static uint32_t noise_remote_handshake_index_get (vlib_main_t *vm,
+ noise_remote_t *);
+static void noise_remote_handshake_index_drop (vlib_main_t *vm,
+ noise_remote_t *);
static uint64_t noise_counter_send (noise_counter_t *);
bool noise_counter_recv (noise_counter_t *, uint64_t);
@@ -86,7 +88,7 @@
}
void
-noise_remote_init (noise_remote_t * r, uint32_t peer_pool_idx,
+noise_remote_init (vlib_main_t *vm, noise_remote_t *r, uint32_t peer_pool_idx,
const uint8_t public[NOISE_PUBLIC_KEY_LEN],
u32 noise_local_idx)
{
@@ -97,18 +99,18 @@
r->r_local_idx = noise_local_idx;
r->r_handshake.hs_state = HS_ZEROED;
- noise_remote_precompute (r);
+ noise_remote_precompute (vm, r);
}
void
-noise_remote_precompute (noise_remote_t * r)
+noise_remote_precompute (vlib_main_t *vm, noise_remote_t *r)
{
noise_local_t *l = noise_local_get (r->r_local_idx);
if (!curve25519_gen_shared (r->r_ss, l->l_private, r->r_public))
clib_memset (r->r_ss, 0, NOISE_PUBLIC_KEY_LEN);
- noise_remote_handshake_index_drop (r);
+ noise_remote_handshake_index_drop (vm, r);
wg_secure_zero_memory (&r->r_handshake, sizeof (r->r_handshake));
}
@@ -154,9 +156,9 @@
/* {t} */
noise_tai64n_now (ets);
noise_msg_encrypt (vm, ets, ets, NOISE_TIMESTAMP_LEN, key_idx, hs->hs_hash);
- noise_remote_handshake_index_drop (r);
+ noise_remote_handshake_index_drop (vm, r);
hs->hs_state = CREATED_INITIATION;
- hs->hs_local_index = noise_remote_handshake_index_get (r);
+ hs->hs_local_index = noise_remote_handshake_index_get (vm, r);
*s_idx = hs->hs_local_index;
ret = true;
error:
@@ -237,7 +239,7 @@
goto error;
/* Ok, we're happy to accept this initiation now */
- noise_remote_handshake_index_drop (r);
+ noise_remote_handshake_index_drop (vm, r);
r->r_handshake = hs;
*rp = r;
ret = true;
@@ -291,7 +293,7 @@
hs->hs_state = CREATED_RESPONSE;
- hs->hs_local_index = noise_remote_handshake_index_get (r);
+ hs->hs_local_index = noise_remote_handshake_index_get (vm, r);
*r_idx = hs->hs_remote_index;
*s_idx = hs->hs_local_index;
ret = true;
@@ -451,7 +453,7 @@
void
noise_remote_clear (vlib_main_t * vm, noise_remote_t * r)
{
- noise_remote_handshake_index_drop (r);
+ noise_remote_handshake_index_drop (vm, r);
wg_secure_zero_memory (&r->r_handshake, sizeof (r->r_handshake));
clib_rwlock_writer_lock (&r->r_keypair_lock);
@@ -555,21 +557,21 @@
}
static uint32_t
-noise_remote_handshake_index_get (noise_remote_t * r)
+noise_remote_handshake_index_get (vlib_main_t *vm, noise_remote_t *r)
{
noise_local_t *local = noise_local_get (r->r_local_idx);
struct noise_upcall *u = &local->l_upcall;
- return u->u_index_set (r);
+ return u->u_index_set (vm, r);
}
static void
-noise_remote_handshake_index_drop (noise_remote_t * r)
+noise_remote_handshake_index_drop (vlib_main_t *vm, noise_remote_t *r)
{
noise_handshake_t *hs = &r->r_handshake;
noise_local_t *local = noise_local_get (r->r_local_idx);
struct noise_upcall *u = &local->l_upcall;
if (hs->hs_state != HS_ZEROED)
- u->u_index_drop (hs->hs_local_index);
+ u->u_index_drop (vm, hs->hs_local_index);
}
static void
diff --git a/src/plugins/wireguard/wireguard_noise.h b/src/plugins/wireguard/wireguard_noise.h
index e95211b..fd2c09e 100644
--- a/src/plugins/wireguard/wireguard_noise.h
+++ b/src/plugins/wireguard/wireguard_noise.h
@@ -121,8 +121,8 @@
{
void *u_arg;
noise_remote_t *(*u_remote_get) (const uint8_t[NOISE_PUBLIC_KEY_LEN]);
- uint32_t (*u_index_set) (noise_remote_t *);
- void (*u_index_drop) (uint32_t);
+ uint32_t (*u_index_set) (vlib_main_t *, noise_remote_t *);
+ void (*u_index_drop) (vlib_main_t *, uint32_t);
} l_upcall;
} noise_local_t;
@@ -148,11 +148,11 @@
bool noise_local_set_private (noise_local_t *,
const uint8_t[NOISE_PUBLIC_KEY_LEN]);
-void noise_remote_init (noise_remote_t *, uint32_t,
+void noise_remote_init (vlib_main_t *, noise_remote_t *, uint32_t,
const uint8_t[NOISE_PUBLIC_KEY_LEN], uint32_t);
/* Should be called anytime noise_local_set_private is called */
-void noise_remote_precompute (noise_remote_t *);
+void noise_remote_precompute (vlib_main_t *, noise_remote_t *);
/* Cryptographic functions */
bool noise_create_initiation (vlib_main_t * vm, noise_remote_t *,
@@ -266,7 +266,7 @@
struct noise_upcall *u = &local->l_upcall;
if (*kp)
{
- u->u_index_drop ((*kp)->kp_local_index);
+ u->u_index_drop (vm, (*kp)->kp_local_index);
vnet_crypto_key_del (vm, (*kp)->kp_send_index);
vnet_crypto_key_del (vm, (*kp)->kp_recv_index);
clib_mem_free (*kp);
diff --git a/src/plugins/wireguard/wireguard_peer.c b/src/plugins/wireguard/wireguard_peer.c
index f7bf235..32a92da 100644
--- a/src/plugins/wireguard/wireguard_peer.c
+++ b/src/plugins/wireguard/wireguard_peer.c
@@ -242,7 +242,7 @@
wg_if = wg_if_get (wg_if_find_by_sw_if_index (peer->wg_sw_if_index));
clib_memcpy (public_key, peer->remote.r_public, NOISE_PUBLIC_KEY_LEN);
- noise_remote_init (&peer->remote, peeri, public_key, wg_if->local_idx);
+ noise_remote_init (vm, &peer->remote, peeri, public_key, wg_if->local_idx);
wg_timers_send_first_handshake (peer);
}
@@ -484,7 +484,7 @@
return (rv);
}
- noise_remote_init (&peer->remote, peer - wg_peer_pool, public_key,
+ noise_remote_init (vm, &peer->remote, peer - wg_peer_pool, public_key,
wg_if->local_idx);
cookie_maker_init (&peer->cookie_maker, public_key);