vhost: allow multiple sessions per device

With all the patches in place, we can finally
enable having more than one simultaneous sessions
to a single vhost device.

This patch adds a unique id to the session structure,
similar to the one in a vhost device and also fills in
the implementation holes in foreach_session().

Vhost-NVMe can support only one session per device
and now has an additional check that prevents it from
starting more than one at a time.

Vhost-SCSI also has the same check now since it needs
additional work on the lcore assignment policy. The
check will be removed once the required work is done.

Change-Id: I13a32c7a0eae808e9bec63a7b8c15ec0bc2e36ed
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/439324
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>

lib/vhost/vhost.c

@@ -52,6 +52,9 @@ struct spdk_vhost_session_fn_ctx {
 	/** Device pointer obtained before enqueuing the event */
 	struct spdk_vhost_dev *vdev;
 
+	/** ID of the session to send event to. */
+	uint32_t vsession_id;
+
 	/** User callback function to be executed on given lcore. */
 	spdk_vhost_session_fn cb_fn;
 
@@ -505,14 +508,31 @@ spdk_vhost_vring_desc_to_iov(struct spdk_vhost_session *vsession, struct iovec *
 	return 0;
 }
 
+static struct spdk_vhost_session *
+spdk_vhost_session_find_by_id(struct spdk_vhost_dev *vdev, unsigned id)
+{
+	struct spdk_vhost_session *vsession;
+
+	TAILQ_FOREACH(vsession, &vdev->vsessions, tailq) {
+		if (vsession->id == id) {
+			return vsession;
+		}
+	}
+
+	return NULL;
+}
+
 static struct spdk_vhost_session *
 spdk_vhost_session_find_by_vid(int vid)
 {
 	struct spdk_vhost_dev *vdev;
+	struct spdk_vhost_session *vsession;
 
 	TAILQ_FOREACH(vdev, &g_spdk_vhost_devices, tailq) {
-		if (vdev->session && vdev->session->vid == vid) {
+		TAILQ_FOREACH(vsession, &vdev->vsessions, tailq) {
-			return vdev->session;
+			if (vsession->vid == vid) {
+				return vsession;
+			}
 		}
 	}
 
@@ -735,7 +755,7 @@ spdk_vhost_dev_register(struct spdk_vhost_dev *vdev, const char *name, const cha
 	vdev->cpumask = cpumask;
 	vdev->registered = true;
 	vdev->backend = backend;
-
+	TAILQ_INIT(&vdev->vsessions);
 	spdk_vhost_set_coalescing(vdev, SPDK_VHOST_COALESCING_DELAY_BASE_US,
 				  SPDK_VHOST_VQ_IOPS_COALESCING_THRESHOLD);
 
@@ -752,7 +772,7 @@ out:
 int
 spdk_vhost_dev_unregister(struct spdk_vhost_dev *vdev)
 {
-	if (vdev->session) {
+	if (!TAILQ_EMPTY(&vdev->vsessions)) {
 		SPDK_ERRLOG("Controller %s has still valid connection.\n", vdev->name);
 		return -EBUSY;
 	}
@@ -776,7 +796,14 @@ spdk_vhost_dev_unregister(struct spdk_vhost_dev *vdev)
 static struct spdk_vhost_session *
 spdk_vhost_session_next(struct spdk_vhost_dev *vdev, unsigned prev_id)
 {
-	/* so far there's only one session per device */
+	struct spdk_vhost_session *vsession;
+
+	TAILQ_FOREACH(vsession, &vdev->vsessions, tailq) {
+		if (vsession->id > prev_id) {
+			return vsession;
+		}
+	}
+
 	return NULL;
 }
 
@@ -831,8 +858,10 @@ static void
 spdk_vhost_event_cb(void *arg1, void *arg2)
 {
 	struct spdk_vhost_session_fn_ctx *ctx = arg1;
+	struct spdk_vhost_session *vsession;
 
-	ctx->cb_fn(ctx->vdev, ctx->vdev->session, ctx);
+	vsession = spdk_vhost_session_find_by_id(ctx->vdev, ctx->vsession_id);
+	ctx->cb_fn(ctx->vdev, vsession, ctx);
 }
 
 static void spdk_vhost_external_event_foreach_continue(struct spdk_vhost_dev *vdev,
@@ -855,8 +884,15 @@ spdk_vhost_event_async_foreach_fn(void *arg1, void *arg2)
 		return;
 	}
 
-	vsession = vdev->session;
+	vsession = spdk_vhost_session_find_by_id(vdev, ctx->vsession_id);
-	if (vsession != NULL && vsession->lcore >= 0 &&
+	if (vsession == NULL) {
+		/* The session must have been removed in the meantime, so we
+		 * just skip it in our foreach chain
+		 */
+		goto out_unlock_continue;
+	}
+
+	if (vsession->lcore >= 0 &&
 	    (uint32_t)vsession->lcore != spdk_env_get_current_core()) {
 		/* if session has been relocated to other core, it is no longer thread-safe
 		 * to access its contents here. Even though we're running under the global
@@ -875,8 +911,8 @@ spdk_vhost_event_async_foreach_fn(void *arg1, void *arg2)
 		goto out_unlock;
 	}
 
-	/* FIXME use a real session ID */
+out_unlock_continue:
-	vsession = spdk_vhost_session_next(vdev, -1);
+	vsession = spdk_vhost_session_next(vdev, ctx->vsession_id);
 	spdk_vhost_external_event_foreach_continue(vdev, vsession, ctx->cb_fn, arg2);
 out_unlock:
 	pthread_mutex_unlock(&g_spdk_vhost_mutex);
@@ -900,6 +936,7 @@ spdk_vhost_session_send_event(struct spdk_vhost_session *vsession,
 	}
 
 	ev_ctx.vdev = vsession->vdev;
+	ev_ctx.vsession_id = vsession->id;
 	ev_ctx.cb_fn = cb_fn;
 	ev = spdk_event_allocate(vsession->lcore, spdk_vhost_event_cb, &ev_ctx, NULL);
 	assert(ev);
@@ -936,6 +973,7 @@ spdk_vhost_event_async_send_foreach_continue(struct spdk_vhost_session *vsession
 	}
 
 	ev_ctx->vdev = vdev;
+	ev_ctx->vsession_id = vsession->id;
 	ev_ctx->cb_fn = cb_fn;
 
 	ev = spdk_event_allocate(vsession->lcore,
@@ -1193,6 +1231,7 @@ new_connection(int vid)
 	char ifname[PATH_MAX];
 
 	pthread_mutex_lock(&g_spdk_vhost_mutex);
+
 	if (rte_vhost_get_ifname(vid, ifname, PATH_MAX) < 0) {
 		SPDK_ERRLOG("Couldn't get a valid ifname for device with vid %d\n", vid);
 		pthread_mutex_unlock(&g_spdk_vhost_mutex);
@@ -1206,10 +1245,14 @@ new_connection(int vid)
 		return -1;
 	}
 
-	if (vdev->session != NULL) {
+	/* We expect sessions inside vdev->vsessions to be sorted in ascending
-		SPDK_ERRLOG("Device %s is already connected.\n", vdev->name);
+	 * order in regard of vsession->id. For now we always set id = vsessions_cnt++
-		pthread_mutex_unlock(&g_spdk_vhost_mutex);
+	 * and append each session to the very end of the vsessions list.
-		return -1;
+	 * This is required for spdk_vhost_dev_foreach_session() to work.
+	 */
+	if (vdev->vsessions_num == UINT_MAX) {
+		assert(false);
+		return -EINVAL;
 	}
 
 	vsession = spdk_dma_zmalloc(sizeof(struct spdk_vhost_session) +
@@ -1222,12 +1265,13 @@ new_connection(int vid)
 	}
 
 	vsession->vdev = vdev;
+	vsession->id = vdev->vsessions_num++;
 	vsession->vid = vid;
 	vsession->lcore = -1;
 	vsession->next_stats_check_time = 0;
 	vsession->stats_check_interval = SPDK_VHOST_STATS_CHECK_INTERVAL_MS *
 					 spdk_get_ticks_hz() / 1000UL;
-	vdev->session = vsession;
+	TAILQ_INSERT_TAIL(&vdev->vsessions, vsession, tailq);
 	pthread_mutex_unlock(&g_spdk_vhost_mutex);
 	return 0;
 }
@@ -1245,7 +1289,7 @@ destroy_connection(int vid)
 		return;
 	}
 
-	vsession->vdev->session = NULL;
+	TAILQ_REMOVE(&vsession->vdev->vsessions, vsession, tailq);
 	spdk_dma_free(vsession);
 	pthread_mutex_unlock(&g_spdk_vhost_mutex);
 }
@@ -1277,8 +1321,7 @@ spdk_vhost_external_event_foreach_continue(struct spdk_vhost_dev *vdev,
 		if (rc < 0) {
 			return;
 		}
-		/* FIXME use a real session ID */
+		vsession = spdk_vhost_session_next(vdev, vsession->id);
-		vsession = spdk_vhost_session_next(vdev, -1);
 		if (vsession == NULL) {
 			goto out_finish_foreach;
 		}
@@ -1300,9 +1343,11 @@ void
 spdk_vhost_dev_foreach_session(struct spdk_vhost_dev *vdev,
 			       spdk_vhost_session_fn fn, void *arg)
 {
+	struct spdk_vhost_session *vsession = TAILQ_FIRST(&vdev->vsessions);
+
 	assert(vdev->pending_async_op_num < UINT32_MAX);
 	vdev->pending_async_op_num++;
-	spdk_vhost_external_event_foreach_continue(vdev, vdev->session, fn, arg);
+	spdk_vhost_external_event_foreach_continue(vdev, vsession, fn, arg);
 }
 
 void

lib/vhost/vhost_blk.c

@@ -410,7 +410,7 @@ process_vq(struct spdk_vhost_blk_session *bvsession, struct spdk_vhost_virtqueue
 {
 	struct spdk_vhost_blk_dev *bvdev = bvsession->bvdev;
 	struct spdk_vhost_blk_task *task;
-	struct spdk_vhost_session *vsession = bvdev->vdev.session;
+	struct spdk_vhost_session *vsession = &bvsession->vsession;
 	int rc;
 	uint16_t reqs[32];
 	uint16_t reqs_cnt, i;

lib/vhost/vhost_internal.h

@@ -117,6 +117,9 @@ struct spdk_vhost_session {
 	/* rte_vhost connection ID. */
 	int vid;
 
+	/* Unique session ID. */
+	unsigned id;
+
 	int32_t lcore;
 
 	struct rte_vhost_memory *mem;
@@ -138,6 +141,8 @@ struct spdk_vhost_session {
 	uint64_t stats_check_interval;
 
 	struct spdk_vhost_virtqueue virtqueue[SPDK_VHOST_MAX_VQUEUES];
+
+	TAILQ_ENTRY(spdk_vhost_session) tailq;
 };
 
 struct spdk_vhost_dev {
@@ -155,8 +160,11 @@ struct spdk_vhost_dev {
 	uint32_t coalescing_delay_us;
 	uint32_t coalescing_iops_threshold;
 
-	/* Current connection to the device */
+	/* Current connections to the device */
-	struct spdk_vhost_session *session;
+	TAILQ_HEAD(, spdk_vhost_session) vsessions;
+
+	/* Increment-only session counter */
+	uint64_t vsessions_num;
 
 	/* Number of started and actively polled sessions */
 	uint32_t active_session_num;

lib/vhost/vhost_nvme.c

@@ -146,6 +146,9 @@ struct spdk_vhost_nvme_dev {
 	struct spdk_vhost_nvme_sq sq_queue[MAX_IO_QUEUES + 1];
 	struct spdk_vhost_nvme_cq cq_queue[MAX_IO_QUEUES + 1];
 
+	/* The one and only session associated with this device */
+	struct spdk_vhost_session *vsession;
+
 	TAILQ_ENTRY(spdk_vhost_nvme_dev) tailq;
 	STAILQ_HEAD(, spdk_vhost_nvme_task) free_tasks;
 	struct spdk_poller *requestq_poller;
@@ -248,7 +251,7 @@ static int
 spdk_nvme_map_prps(struct spdk_vhost_nvme_dev *nvme, struct spdk_nvme_cmd *cmd,
 		   struct spdk_vhost_nvme_task *task, uint32_t len)
 {
-	struct spdk_vhost_session *vsession = nvme->vdev.session;
+	struct spdk_vhost_session *vsession = nvme->vsession;
 	uint64_t prp1, prp2;
 	void *vva;
 	uint32_t i;
@@ -699,7 +702,7 @@ static int
 vhost_nvme_doorbell_buffer_config(struct spdk_vhost_nvme_dev *nvme,
 				  struct spdk_nvme_cmd *cmd, struct spdk_nvme_cpl *cpl)
 {
-	struct spdk_vhost_session *vsession = nvme->vdev.session;
+	struct spdk_vhost_session *vsession = nvme->vsession;
 	uint64_t dbs_dma_addr, eis_dma_addr;
 
 	dbs_dma_addr = cmd->dptr.prp.prp1;
@@ -765,7 +768,7 @@ vhost_nvme_create_io_sq(struct spdk_vhost_nvme_dev *nvme,
 	sq->size = qsize + 1;
 	sq->sq_head = sq->sq_tail = 0;
 	requested_len = sizeof(struct spdk_nvme_cmd) * sq->size;
-	sq->sq_cmd = spdk_vhost_gpa_to_vva(nvme->vdev.session, dma_addr, requested_len);
+	sq->sq_cmd = spdk_vhost_gpa_to_vva(nvme->vsession, dma_addr, requested_len);
 	if (!sq->sq_cmd) {
 		return -1;
 	}
@@ -848,7 +851,7 @@ vhost_nvme_create_io_cq(struct spdk_vhost_nvme_dev *nvme,
 	cq->guest_signaled_cq_head = 0;
 	cq->need_signaled_cnt = 0;
 	requested_len = sizeof(struct spdk_nvme_cpl) * cq->size;
-	cq->cq_cqe = spdk_vhost_gpa_to_vva(nvme->vdev.session, dma_addr, requested_len);
+	cq->cq_cqe = spdk_vhost_gpa_to_vva(nvme->vsession, dma_addr, requested_len);
 	if (!cq->cq_cqe) {
 		return -1;
 	}
@@ -893,7 +896,7 @@ spdk_vhost_nvme_get_by_name(int vid)
 	struct spdk_vhost_nvme_dev *nvme;
 
 	TAILQ_FOREACH(nvme, &g_nvme_ctrlrs, tailq) {
-		if (nvme->vdev.session != NULL && nvme->vdev.session->vid == vid) {
+		if (nvme->vsession != NULL && nvme->vsession->vid == vid) {
 			return nvme;
 		}
 	}
@@ -1103,6 +1106,12 @@ spdk_vhost_nvme_start_cb(struct spdk_vhost_dev *vdev,
 static int
 spdk_vhost_nvme_start(struct spdk_vhost_session *vsession)
 {
+	if (vsession->vdev->active_session_num > 0) {
+		/* We're trying to start a second session */
+		SPDK_ERRLOG("Vhost-NVMe devices can support only one simultaneous connection.\n");
+		return -1;
+	}
+
 	return spdk_vhost_session_send_event(vsession, spdk_vhost_nvme_start_cb,
 					     3, "start session");
 }

lib/vhost/vhost_scsi.c

@@ -661,7 +661,7 @@ static void
 process_controlq(struct spdk_vhost_scsi_session *svsession, struct spdk_vhost_virtqueue *vq)
 {
 	struct spdk_vhost_scsi_dev *svdev = svsession->svdev;
-	struct spdk_vhost_session *vsession = svdev->vdev.session;
+	struct spdk_vhost_session *vsession = &svsession->vsession;
 	struct spdk_vhost_scsi_task *task;
 	uint16_t reqs[32];
 	uint16_t reqs_cnt, i;
@@ -1266,6 +1266,12 @@ out:
 static int
 spdk_vhost_scsi_start(struct spdk_vhost_session *vsession)
 {
+	if (vsession->vdev->active_session_num > 0) {
+		/* We're trying to start a second session */
+		SPDK_ERRLOG("Vhost-SCSI devices can support only one simultaneous connection.\n");
+		return -1;
+	}
+
 	return spdk_vhost_session_send_event(vsession, spdk_vhost_scsi_start_cb,
 					     3, "start session");
 }

test/unit/lib/vhost/vhost.c/vhost_ut.c

@@ -139,6 +139,7 @@ static void
 start_vdev(struct spdk_vhost_dev *vdev)
 {
 	struct rte_vhost_memory *mem;
+	struct spdk_vhost_session *vsession = NULL;
 	int rc;
 
 	mem = calloc(1, sizeof(*mem) + 2 * sizeof(struct rte_vhost_mem_region));
@@ -151,28 +152,31 @@ start_vdev(struct spdk_vhost_dev *vdev)
 	mem->regions[1].size = 0x400000; /* 4 MB */
 	mem->regions[1].host_user_addr = 0x2000000;
 
-	assert(vdev->session == NULL);
+	assert(TAILQ_EMPTY(&vdev->vsessions));
 	/* spdk_vhost_dev must be allocated on a cache line boundary. */
-	rc = posix_memalign((void **)&vdev->session, 64, sizeof(*vdev->session));
+	rc = posix_memalign((void **)&vsession, 64, sizeof(*vsession));
 	CU_ASSERT(rc == 0);
-	SPDK_CU_ASSERT_FATAL(vdev->session != NULL);
+	SPDK_CU_ASSERT_FATAL(vsession != NULL);
-	vdev->session->lcore = 0;
+	vsession->lcore = 0;
-	vdev->session->vid = 0;
+	vsession->vid = 0;
-	vdev->session->mem = mem;
+	vsession->mem = mem;
+	TAILQ_INSERT_TAIL(&vdev->vsessions, vsession, tailq);
 }
 
 static void
 stop_vdev(struct spdk_vhost_dev *vdev)
 {
-	free(vdev->session->mem);
+	struct spdk_vhost_session *vsession = TAILQ_FIRST(&vdev->vsessions);
-	free(vdev->session);
+
-	vdev->session = NULL;
+	TAILQ_REMOVE(&vdev->vsessions, vsession, tailq);
+	free(vsession->mem);
+	free(vsession);
 }
 
 static void
 cleanup_vdev(struct spdk_vhost_dev *vdev)
 {
-	if (vdev->session) {
+	if (!TAILQ_EMPTY(&vdev->vsessions)) {
 		stop_vdev(vdev);
 	}
 	spdk_vhost_dev_unregister(vdev);
@@ -193,7 +197,7 @@ desc_to_iov_test(void)
 	SPDK_CU_ASSERT_FATAL(rc == 0 && vdev);
 	start_vdev(vdev);
 
-	vsession = vdev->session;
+	vsession = TAILQ_FIRST(&vdev->vsessions);
 
 	/* Test simple case where iov falls fully within a 2MB page. */
 	desc.addr = 0x110000;
@@ -300,6 +304,7 @@ static void
 session_find_by_vid_test(void)
 {
 	struct spdk_vhost_dev *vdev;
+	struct spdk_vhost_session *vsession;
 	struct spdk_vhost_session *tmp;
 	int rc;
 
@@ -307,11 +312,13 @@ session_find_by_vid_test(void)
 	SPDK_CU_ASSERT_FATAL(rc == 0 && vdev);
 	start_vdev(vdev);
 
-	tmp = spdk_vhost_session_find_by_vid(vdev->session->vid);
+	vsession = TAILQ_FIRST(&vdev->vsessions);
-	CU_ASSERT(tmp == vdev->session);
+
+	tmp = spdk_vhost_session_find_by_vid(vsession->vid);
+	CU_ASSERT(tmp == vsession);
 
 	/* Search for a device with incorrect vid */
-	tmp = spdk_vhost_session_find_by_vid(vdev->session->vid + 0xFF);
+	tmp = spdk_vhost_session_find_by_vid(vsession->vid + 0xFF);
 	CU_ASSERT(tmp == NULL);
 
 	cleanup_vdev(vdev);
@@ -328,6 +335,7 @@ remove_controller_test(void)
 
 	/* Remove device when controller is in use */
 	start_vdev(vdev);
+	SPDK_CU_ASSERT_FATAL(!TAILQ_EMPTY(&vdev->vsessions));
 	ret = spdk_vhost_dev_unregister(vdev);
 	CU_ASSERT(ret != 0);