From 3f340175aebaf84ed3994799e819e0801c7e3212 Mon Sep 17 00:00:00 2001 From: Mohsin Kazmi Date: Mon, 27 May 2019 15:53:25 +0200 Subject: [PATCH] tap: crash in multi-thread environment In tap tx routine, virtio_interface_tx_inline, there used to be an interface spinlock to ensure packets are processed in an orderly fashion clib_spinlock_lock_if_init (&vif->lockp); When virtio code was introduced in 19.04, that line is changed to clib_spinlock_lock_if_init (&vring->lockp); to accommodate multi-queues. Unfortunately, althrough the spinlock exists in the vring, it was never initialized for tap, only for virtio. As a result, many nasty things can happen when running tap interface in multi-thread environment. Crash is inevitable. The fix is to initialize vring->lockp for tap and remove vif->lockp as it is not used anymore. Change-Id: I82b15d3e9b0fb6add9b9ac49bf602a538946634a Signed-off-by: Mohsin Kazmi (cherry picked from commit c2c89782d34df0dc7197b18b042b4c2464a101ef) --- src/vnet/devices/tap/tap.c | 4 ---- src/vnet/devices/virtio/virtio.c | 4 ++++ src/vnet/devices/virtio/virtio.h | 1 - 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/vnet/devices/tap/tap.c b/src/vnet/devices/tap/tap.c index a1a9fee2d77..35f1f2a3451 100644 --- a/src/vnet/devices/tap/tap.c +++ b/src/vnet/devices/tap/tap.c @@ -113,7 +113,6 @@ void tap_create_if (vlib_main_t * vm, tap_create_if_args_t * args) { vnet_main_t *vnm = vnet_get_main (); - vlib_thread_main_t *thm = vlib_get_thread_main (); virtio_main_t *vim = &virtio_main; tap_main_t *tm = &tap_main; vnet_sw_interface_t *sw; @@ -448,8 +447,6 @@ tap_create_if (vlib_main_t * vm, tap_create_if_args_t * args) vif->sw_if_index, vif->tap_fd); vif->tap_file_index = clib_file_add (&file_main, &t); - if (thm->n_vlib_mains > 1) - clib_spinlock_init (&vif->lockp); goto done; error: @@ -526,7 +523,6 @@ tap_delete_if (vlib_main_t * vm, u32 sw_if_index) vec_free (vif->txq_vrings); tm->tap_ids = clib_bitmap_set (tm->tap_ids, vif->id, 0); - clib_spinlock_free (&vif->lockp); clib_memset (vif, 0, sizeof (*vif)); pool_put (mm->interfaces, vif); diff --git a/src/vnet/devices/virtio/virtio.c b/src/vnet/devices/virtio/virtio.c index 90acb75fa6d..a3fd05dbb55 100644 --- a/src/vnet/devices/virtio/virtio.c +++ b/src/vnet/devices/virtio/virtio.c @@ -83,9 +83,12 @@ virtio_vring_init (vlib_main_t * vm, virtio_if_t * vif, u16 idx, u16 sz) if (idx % 2) { + vlib_thread_main_t *thm = vlib_get_thread_main (); vec_validate_aligned (vif->txq_vrings, TX_QUEUE_ACCESS (idx), CLIB_CACHE_LINE_BYTES); vring = vec_elt_at_index (vif->txq_vrings, TX_QUEUE_ACCESS (idx)); + if (thm->n_vlib_mains > 1) + clib_spinlock_init (&vring->lockp); } else { @@ -230,6 +233,7 @@ virtio_vring_free_tx (vlib_main_t * vm, virtio_if_t * vif, u32 idx) if (vring->avail) clib_mem_free (vring->avail); vec_free (vring->buffers); + clib_spinlock_free (&vring->lockp); return 0; } diff --git a/src/vnet/devices/virtio/virtio.h b/src/vnet/devices/virtio/virtio.h index 57bb8d6c167..55b6271b337 100644 --- a/src/vnet/devices/virtio/virtio.h +++ b/src/vnet/devices/virtio/virtio.h @@ -135,7 +135,6 @@ typedef struct { CLIB_CACHE_LINE_ALIGN_MARK (cacheline0); u32 flags; - clib_spinlock_t lockp; u32 dev_instance; u32 hw_if_index; -- 2.16.6