From f869197b76ff6981e901b6d9a05789e1b993494a Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Mon, 29 Aug 2022 19:59:31 +0000 Subject: [PATCH] virtio: assert and ERRLOG for virtio-user dynamic mem allocations We do not support dynamic memory allocation with the virtio-user library - it results in SET_MEM_TABLE vhost messages for every change which is not supported by the vhost target. Add '-s 256' to vhost fuzz script, to ensure it does not violate the new restriction. This is a follow-on patch for issue #2596. Signed-off-by: Jim Harris Change-Id: If851f53d7d670ac8443f0d9c8f4e3cbe82e0df7c Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/14249 Tested-by: SPDK CI Jenkins Reviewed-by: Dong Yi Reviewed-by: Changpeng Liu Reviewed-by: Shuhei Matsumoto Reviewed-by: Aleksey Marchuk --- CHANGELOG.md | 10 ++++++++++ lib/virtio/virtio_vhost_user.c | 14 ++++++++++++++ test/vhost/fuzz/fuzz.sh | 2 +- 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98b7ece56..b9e34d239 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,6 +85,16 @@ AEN. Added new functions: `spdk_hexlify` and `spdk_unhexlify`. +### virtio + +virtio-vhost-user no longer tries to support dynamic memory allocation. The vhost target does +not support the high rate of SET_MEM_TABLE vhost messages that result from dynamic memory +allocation, so a virtio-vhost-user device will now present an ERRLOG, assert, and skip the +SET_MEM_TABLE vhost message if a memory notification is received outside of the normal device +start/stop. Applications using the virtio library in vhost-user mode should now pre-allocate +the application's memory using the -s/--mem-size option and use single shared memory file +segments using the -g/--single-file-segments option. + ## v22.05 ### sock diff --git a/lib/virtio/virtio_vhost_user.c b/lib/virtio/virtio_vhost_user.c index b467cca3b..23149452f 100644 --- a/lib/virtio/virtio_vhost_user.c +++ b/lib/virtio/virtio_vhost_user.c @@ -29,6 +29,7 @@ struct virtio_user_dev { uint32_t queue_size; uint8_t status; + bool is_stopping; char path[PATH_MAX]; uint64_t protocol_features; struct vring vrings[SPDK_VIRTIO_MAX_VIRTQUEUES]; @@ -586,6 +587,18 @@ virtio_user_map_notify(void *cb_ctx, struct spdk_mem_map *map, uint64_t features; int ret; + /* We do not support dynamic memory allocation with virtio-user. If this is the + * initial notification when the device is started, dev->mem_map will be NULL. If + * this is the final notification when the device is stopped, dev->is_stopping will + * be true. All other cases are unsupported. + */ + if (dev->mem_map != NULL && !dev->is_stopping) { + assert(false); + SPDK_ERRLOG("Memory map change with active virtio_user_devs not allowed.\n"); + SPDK_ERRLOG("Pre-allocate memory for application using -s (mem_size) option.\n"); + return -1; + } + /* We have to resend all mappings anyway, so don't bother with any * page tracking. */ @@ -626,6 +639,7 @@ virtio_user_unregister_mem(struct virtio_dev *vdev) { struct virtio_user_dev *dev = vdev->ctx; + dev->is_stopping = true; spdk_mem_map_free(&dev->mem_map); } diff --git a/test/vhost/fuzz/fuzz.sh b/test/vhost/fuzz/fuzz.sh index 7502f1976..7b77c5493 100755 --- a/test/vhost/fuzz/fuzz.sh +++ b/test/vhost/fuzz/fuzz.sh @@ -7,7 +7,7 @@ source "$rootdir/scripts/common.sh" VHOST_APP+=(-p 0) FUZZ_RPC_SOCK="/var/tmp/spdk_fuzz.sock" -VHOST_FUZZ_APP+=(-r "$FUZZ_RPC_SOCK" -g --wait-for-rpc) +VHOST_FUZZ_APP+=(-r "$FUZZ_RPC_SOCK" -g -s 256 --wait-for-rpc) vhost_rpc_py="$rootdir/scripts/rpc.py" fuzz_generic_rpc_py="$rootdir/scripts/rpc.py -s $FUZZ_RPC_SOCK"