vhost: fix FD entries cleanup

With the recent rework of the FD manager to use epoll, an error message is emitted with Vhost-user at FD entry removal: ERR|VHOST_FDMAN: could not remove 102 fd from 101 epfd: No such file or directory It occurs because the read callback closes the file descriptor before it is removed from the FD set. This patch defers the close() after the FD entry is removed from the set. Fixes: 0e38b42 ("vhost: manage FD with epoll") Cc: [email protected] Signed-off-by: David Marchand <[email protected]> Signed-off-by: Maxime Coquelin <[email protected]> Reviewed-by: Chenbo Xia <[email protected]>
DPDK · Feb 7, 2025 · 5096693 · 5096693
1 parent 36309ee
commit 5096693
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 17 deletions.
diff --git a/lib/vhost/fd_man.c b/lib/vhost/fd_man.c
@@ -333,7 +333,7 @@ fdset_event_dispatch(void *arg)
 	fd_cb rcb, wcb;
 	void *dat;
 	int fd, numfds;
-	int remove1, remove2;
+	int close1, close2;
 	struct fdset *pfdset = arg;
 
 	if (pfdset == NULL)
@@ -357,7 +357,7 @@ fdset_event_dispatch(void *arg)
 				continue;
 			}
 
-			remove1 = remove2 = 0;
+			close1 = close2 = 0;
 
 			rcb = pfdentry->rcb;
 			wcb = pfdentry->wcb;
@@ -367,9 +367,9 @@ fdset_event_dispatch(void *arg)
 			pthread_mutex_unlock(&pfdset->fd_mutex);
 
 			if (rcb && events[i].events & (EPOLLIN | EPOLLERR | EPOLLHUP))
-				rcb(fd, dat, &remove1);
+				rcb(fd, dat, &close1);
 			if (wcb && events[i].events & (EPOLLOUT | EPOLLERR | EPOLLHUP))
-				wcb(fd, dat, &remove2);
+				wcb(fd, dat, &close2);
 			pfdentry->busy = 0;
 			/*
 			 * fdset_del needs to check busy flag.
@@ -381,8 +381,10 @@ fdset_event_dispatch(void *arg)
 			 * fdentry not to be busy, so we can't call
 			 * fdset_del_locked().
 			 */
-			if (remove1 || remove2)
+			if (close1 || close2) {
 				fdset_del(pfdset, fd);
+				close(fd);
+			}
 		}
 
 		if (pfdset->destroy)

diff --git a/lib/vhost/fd_man.h b/lib/vhost/fd_man.h
@@ -12,7 +12,7 @@ struct fdset;
 
 #define MAX_FDS 1024
 
-typedef void (*fd_cb)(int fd, void *dat, int *remove);
+typedef void (*fd_cb)(int fd, void *dat, int *close);
 
 struct fdset *fdset_init(const char *name);
 

diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
@@ -84,8 +84,8 @@ struct vhost_user {
 
 #define MAX_VIRTIO_BACKLOG 128
 
-static void vhost_user_server_new_connection(int fd, void *data, int *remove);
-static void vhost_user_read_cb(int fd, void *dat, int *remove);
+static void vhost_user_server_new_connection(int fd, void *data, int *close);
+static void vhost_user_read_cb(int fd, void *dat, int *close);
 static int create_unix_socket(struct vhost_user_socket *vsocket);
 static int vhost_user_start_client(struct vhost_user_socket *vsocket);
 
@@ -290,7 +290,7 @@ vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket)
 
 /* call back when there is new vhost-user connection from client  */
 static void
-vhost_user_server_new_connection(int fd, void *dat, int *remove __rte_unused)
+vhost_user_server_new_connection(int fd, void *dat, int *close __rte_unused)
 {
 	struct vhost_user_socket *vsocket = dat;
 
@@ -303,7 +303,7 @@ vhost_user_server_new_connection(int fd, void *dat, int *remove __rte_unused)
 }
 
 static void
-vhost_user_read_cb(int connfd, void *dat, int *remove)
+vhost_user_read_cb(int connfd, void *dat, int *close)
 {
 	struct vhost_user_connection *conn = dat;
 	struct vhost_user_socket *vsocket = conn->vsocket;
@@ -313,8 +313,7 @@ vhost_user_read_cb(int connfd, void *dat, int *remove)
 	if (ret < 0) {
 		struct virtio_net *dev = get_device(conn->vid);
 
-		close(connfd);
-		*remove = 1;
+		*close = 1;
 
 		if (dev)
 			vhost_destroy_device_notify(dev);

diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
@@ -117,7 +117,7 @@ static struct vhost_backend_ops vduse_backend_ops = {
 };
 
 static void
-vduse_control_queue_event(int fd, void *arg, int *remove __rte_unused)
+vduse_control_queue_event(int fd, void *arg, int *close __rte_unused)
 {
 	struct virtio_net *dev = arg;
 	uint64_t buf;
@@ -351,7 +351,7 @@ vduse_device_stop(struct virtio_net *dev)
 }
 
 static void
-vduse_events_handler(int fd, void *arg, int *remove __rte_unused)
+vduse_events_handler(int fd, void *arg, int *close __rte_unused)
 {
 	struct virtio_net *dev = arg;
 	struct vduse_dev_request req;
@@ -564,14 +564,13 @@ vduse_reconnect_log_check(struct virtio_net *dev, uint64_t features, uint32_t to
 }
 
 static void
-vduse_reconnect_handler(int fd, void *arg, int *remove)
+vduse_reconnect_handler(int fd __rte_unused, void *arg, int *close)
 {
 	struct virtio_net *dev = arg;
 
 	vduse_device_start(dev, true);
 
-	close(fd);
-	*remove = 1;
+	*close = 1;
 }
 
 static int