diff mbox

[dpdk-dev,v3] rte_vhost: added user callbacks for socket open/close

Message ID 1504090258-164350-1-git-send-email-dariuszx.stojaczyk@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Yuanhan Liu
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Stojaczyk, DariuszX Aug. 30, 2017, 10:50 a.m. UTC 
  Added new callbacks to notify about socket connection status.
As destroy_device is used for virtqueue processing *pause* as well as
connection close, the user has no distinction between those.

Consider the following scenario:
rte_vhost: received SET_VRING_BASE message,
           calling destroy_device() as usual

user:  end-user asks to remove the device (together with socket file),
       OK, device is not *in use* - that's NOT the behavior we want
       calling rte_vhost_driver_unregister() etc.

Instead of changing new_device/destroy_device callbacks and breaking
the ABI, a set of new functions new_connection/destroy_connection
has been added.

Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
---
v3: improved err-handling path and updated commit msg
v2: also updated vhost_lib.rst
 lib/librte_vhost/rte_vhost.h |  5 ++++-
 lib/librte_vhost/socket.c    | 31 ++++++++++++++++++++++++-------
 2 files changed, 28 insertions(+), 8 deletions(-)

Comments

Yuanhan Liu Oct. 10, 2017, 3:14 a.m. UTC | #1 
On Wed, Aug 30, 2017 at 12:50:58PM +0200, Dariusz Stojaczyk wrote:
> Added new callbacks to notify about socket connection status.
> As destroy_device is used for virtqueue processing *pause* as well as
> connection close, the user has no distinction between those.
> 
> Consider the following scenario:
> rte_vhost: received SET_VRING_BASE message,
>            calling destroy_device() as usual
> 
> user:  end-user asks to remove the device (together with socket file),
>        OK, device is not *in use* - that's NOT the behavior we want
>        calling rte_vhost_driver_unregister() etc.
> 
> Instead of changing new_device/destroy_device callbacks and breaking
> the ABI, a set of new functions new_connection/destroy_connection
> has been added.
> 
> Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
> ---
> v3: improved err-handling path and updated commit msg
> v2: also updated vhost_lib.rst

The doc update is missing. I have cherry-picked it from v2.

Applied to dpdk-next-virtio.

Thanks.

	--yliu
diff mbox

Patch

diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
index 8c974eb..fe5c94c 100644
--- a/lib/librte_vhost/rte_vhost.h
+++ b/lib/librte_vhost/rte_vhost.h
@@ -107,7 +107,10 @@  struct vhost_device_ops {
 	 */
 	int (*features_changed)(int vid, uint64_t features);
 
-	void *reserved[4]; /**< Reserved for future extension */
+	int (*new_connection)(int vid);
+	void (*destroy_connection)(int vid);
+
+	void *reserved[2]; /**< Reserved for future extension */
 };
 
 /**
diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 41aa3f9..7018150 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -217,9 +217,7 @@  vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket)
 
 	vid = vhost_new_device();
 	if (vid == -1) {
-		close(fd);
-		free(conn);
-		return;
+		goto err;
 	}
 
 	size = strnlen(vsocket->path, PATH_MAX);
@@ -230,24 +228,40 @@  vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket)
 
 	RTE_LOG(INFO, VHOST_CONFIG, "new device, handle is %d\n", vid);
 
+	if (vsocket->notify_ops->new_connection) {
+		ret = vsocket->notify_ops->new_connection(vid);
+		if (ret < 0) {
+			RTE_LOG(ERR, VHOST_CONFIG,
+				"failed to add vhost user connection with fd %d\n",
+				fd);
+			goto err;
+		}
+	}
+
 	conn->connfd = fd;
 	conn->vsocket = vsocket;
 	conn->vid = vid;
 	ret = fdset_add(&vhost_user.fdset, fd, vhost_user_read_cb,
 			NULL, conn);
 	if (ret < 0) {
-		conn->connfd = -1;
-		free(conn);
-		close(fd);
 		RTE_LOG(ERR, VHOST_CONFIG,
 			"failed to add fd %d into vhost server fdset\n",
 			fd);
-		return;
+
+		if (vsocket->notify_ops->destroy_connection)
+			vsocket->notify_ops->destroy_connection(conn->vid);
+
+		goto err;
 	}
 
 	pthread_mutex_lock(&vsocket->conn_mutex);
 	TAILQ_INSERT_TAIL(&vsocket->conn_list, conn, next);
 	pthread_mutex_unlock(&vsocket->conn_mutex);
+	return;
+
+err:
+	free(conn);
+	close(fd);
 }
 
 /* call back when there is new vhost-user connection from client  */
@@ -277,6 +291,9 @@  vhost_user_read_cb(int connfd, void *dat, int *remove)
 		*remove = 1;
 		vhost_destroy_device(conn->vid);
 
+		if (vsocket->notify_ops->destroy_connection)
+			vsocket->notify_ops->destroy_connection(conn->vid);
+
 		pthread_mutex_lock(&vsocket->conn_mutex);
 		TAILQ_REMOVE(&vsocket->conn_list, conn, next);
 		pthread_mutex_unlock(&vsocket->conn_mutex);