* [PATCH 1/2] tcp: Always pass NULL event with EPOLL_CTL_DEL
2025-01-30 6:52 [PATCH 0/2] Trivial cleanups to EPOLL_CTL_DEL handling David Gibson
@ 2025-01-30 6:52 ` David Gibson
2025-01-30 6:52 ` [PATCH 2/2] util: Rename and make global vu_remove_watch() David Gibson
1 sibling, 0 replies; 3+ messages in thread
From: David Gibson @ 2025-01-30 6:52 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
In tcp_epoll_ctl() we pass an event pointer with EPOLL_CTL_DEL, even though
it will be ignored. It's possible this was a workaround for pre-2.6.9
kernels which required a non-NULL pointer here, but we rely on the kernel
accepting NULL events for EPOLL_CTL_DEL in lots of other places. Use
NULL instead for simplicity and consistency.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
tcp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tcp.c b/tcp.c
index c89f3232..4eed82bf 100644
--- a/tcp.c
+++ b/tcp.c
@@ -468,9 +468,9 @@ static int tcp_epoll_ctl(const struct ctx *c, struct tcp_tap_conn *conn)
if (conn->events == CLOSED) {
if (conn->in_epoll)
- epoll_ctl(c->epollfd, EPOLL_CTL_DEL, conn->sock, &ev);
+ epoll_ctl(c->epollfd, EPOLL_CTL_DEL, conn->sock, NULL);
if (conn->timer != -1)
- epoll_ctl(c->epollfd, EPOLL_CTL_DEL, conn->timer, &ev);
+ epoll_ctl(c->epollfd, EPOLL_CTL_DEL, conn->timer, NULL);
return 0;
}
--
@@ -468,9 +468,9 @@ static int tcp_epoll_ctl(const struct ctx *c, struct tcp_tap_conn *conn)
if (conn->events == CLOSED) {
if (conn->in_epoll)
- epoll_ctl(c->epollfd, EPOLL_CTL_DEL, conn->sock, &ev);
+ epoll_ctl(c->epollfd, EPOLL_CTL_DEL, conn->sock, NULL);
if (conn->timer != -1)
- epoll_ctl(c->epollfd, EPOLL_CTL_DEL, conn->timer, &ev);
+ epoll_ctl(c->epollfd, EPOLL_CTL_DEL, conn->timer, NULL);
return 0;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 2/2] util: Rename and make global vu_remove_watch()
2025-01-30 6:52 [PATCH 0/2] Trivial cleanups to EPOLL_CTL_DEL handling David Gibson
2025-01-30 6:52 ` [PATCH 1/2] tcp: Always pass NULL event with EPOLL_CTL_DEL David Gibson
@ 2025-01-30 6:52 ` David Gibson
1 sibling, 0 replies; 3+ messages in thread
From: David Gibson @ 2025-01-30 6:52 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
vu_remove_watch() is used in vhost_user.c to remove an fd from the global
epoll set. There's nothing really vhost user specific about it though,
so rename, move to util.c and use it in a bunch of places outside
vhost_user.c where it makes things marginally more readable.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
icmp.c | 2 +-
tap.c | 2 +-
tcp.c | 4 ++--
tcp_splice.c | 4 ++--
udp_flow.c | 2 +-
util.c | 10 ++++++++++
util.h | 1 +
vhost_user.c | 21 +++++----------------
vu_common.c | 6 ++----
9 files changed, 25 insertions(+), 27 deletions(-)
diff --git a/icmp.c b/icmp.c
index 143e93be..bcf498d2 100644
--- a/icmp.c
+++ b/icmp.c
@@ -150,7 +150,7 @@ unexpected:
static void icmp_ping_close(const struct ctx *c,
const struct icmp_ping_flow *pingf)
{
- epoll_ctl(c->epollfd, EPOLL_CTL_DEL, pingf->sock, NULL);
+ epoll_del(c, pingf->sock);
close(pingf->sock);
flow_hash_remove(c, FLOW_SIDX(pingf, INISIDE));
}
diff --git a/tap.c b/tap.c
index cd32a901..772648f9 100644
--- a/tap.c
+++ b/tap.c
@@ -1005,7 +1005,7 @@ void tap_sock_reset(struct ctx *c)
exit(EXIT_SUCCESS);
/* Close the connected socket, wait for a new connection */
- epoll_ctl(c->epollfd, EPOLL_CTL_DEL, c->fd_tap, NULL);
+ epoll_del(c, c->fd_tap);
close(c->fd_tap);
c->fd_tap = -1;
if (c->mode == MODE_VU)
diff --git a/tcp.c b/tcp.c
index 4eed82bf..77873815 100644
--- a/tcp.c
+++ b/tcp.c
@@ -468,9 +468,9 @@ static int tcp_epoll_ctl(const struct ctx *c, struct tcp_tap_conn *conn)
if (conn->events == CLOSED) {
if (conn->in_epoll)
- epoll_ctl(c->epollfd, EPOLL_CTL_DEL, conn->sock, NULL);
+ epoll_del(c, conn->sock);
if (conn->timer != -1)
- epoll_ctl(c->epollfd, EPOLL_CTL_DEL, conn->timer, NULL);
+ epoll_del(c, conn->timer);
return 0;
}
diff --git a/tcp_splice.c b/tcp_splice.c
index 3a000ffa..5db1d62e 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -200,8 +200,8 @@ static void conn_flag_do(const struct ctx *c, struct tcp_splice_conn *conn,
}
if (flag == CLOSING) {
- epoll_ctl(c->epollfd, EPOLL_CTL_DEL, conn->s[0], NULL);
- epoll_ctl(c->epollfd, EPOLL_CTL_DEL, conn->s[1], NULL);
+ epoll_del(c, conn->s[0]);
+ epoll_del(c, conn->s[1]);
}
}
diff --git a/udp_flow.c b/udp_flow.c
index 9fd7d06a..7fae81de 100644
--- a/udp_flow.c
+++ b/udp_flow.c
@@ -52,7 +52,7 @@ void udp_flow_close(const struct ctx *c, struct udp_flow *uflow)
if (uflow->s[TGTSIDE] >= 0) {
/* But the flow specific one needs to be removed */
- epoll_ctl(c->epollfd, EPOLL_CTL_DEL, uflow->s[TGTSIDE], NULL);
+ epoll_del(c, uflow->s[TGTSIDE]);
close(uflow->s[TGTSIDE]);
uflow->s[TGTSIDE] = -1;
}
diff --git a/util.c b/util.c
index 11973c44..c7b09f08 100644
--- a/util.c
+++ b/util.c
@@ -837,3 +837,13 @@ void raw_random(void *buf, size_t buflen)
if (random_read < buflen)
die("Unexpected EOF on random data source");
}
+
+/**
+ * epoll_del() - Remove a file descriptor from our passt epoll
+ * @c: Execution context
+ * @fd: File descriptor to remove
+ */
+void epoll_del(const struct ctx *c, int fd)
+{
+ epoll_ctl(c->epollfd, EPOLL_CTL_DEL, fd, NULL);
+}
diff --git a/util.h b/util.h
index d02333d5..800a28be 100644
--- a/util.h
+++ b/util.h
@@ -276,6 +276,7 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m)
#define FPRINTF(f, ...) (void)fprintf(f, __VA_ARGS__)
void raw_random(void *buf, size_t buflen);
+void epoll_del(const struct ctx *c, int fd);
/*
* Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror,
diff --git a/vhost_user.c b/vhost_user.c
index 6bf0dda8..bbbf504f 100644
--- a/vhost_user.c
+++ b/vhost_user.c
@@ -162,17 +162,6 @@ static void vmsg_close_fds(const struct vhost_user_msg *vmsg)
close(vmsg->fds[i]);
}
-/**
- * vu_remove_watch() - Remove a file descriptor from our passt epoll
- * file descriptor
- * @vdev: vhost-user device
- * @fd: file descriptor to remove
- */
-static void vu_remove_watch(const struct vu_dev *vdev, int fd)
-{
- epoll_ctl(vdev->context->epollfd, EPOLL_CTL_DEL, fd, NULL);
-}
-
/**
* vmsg_set_reply_u64() - Set reply payload.u64 and clear request flags
* and fd_num
@@ -748,7 +737,7 @@ static bool vu_get_vring_base_exec(struct vu_dev *vdev,
vdev->vq[idx].call_fd = -1;
}
if (vdev->vq[idx].kick_fd != -1) {
- vu_remove_watch(vdev, vdev->vq[idx].kick_fd);
+ epoll_del(vdev->context, vdev->vq[idx].kick_fd);
close(vdev->vq[idx].kick_fd);
vdev->vq[idx].kick_fd = -1;
}
@@ -816,7 +805,7 @@ static bool vu_set_vring_kick_exec(struct vu_dev *vdev,
vu_check_queue_msg_file(msg);
if (vdev->vq[idx].kick_fd != -1) {
- vu_remove_watch(vdev, vdev->vq[idx].kick_fd);
+ epoll_del(vdev->context, vdev->vq[idx].kick_fd);
close(vdev->vq[idx].kick_fd);
vdev->vq[idx].kick_fd = -1;
}
@@ -1063,7 +1052,7 @@ static bool vu_set_device_state_fd_exec(struct vu_dev *vdev,
die("Invalide device_state_fd direction: %d", direction);
if (vdev->device_state_fd != -1) {
- vu_remove_watch(vdev, vdev->device_state_fd);
+ epoll_del(vdev->context, vdev->device_state_fd);
close(vdev->device_state_fd);
}
@@ -1145,7 +1134,7 @@ void vu_cleanup(struct vu_dev *vdev)
vq->err_fd = -1;
}
if (vq->kick_fd != -1) {
- vu_remove_watch(vdev, vq->kick_fd);
+ epoll_del(vdev->context, vq->kick_fd);
close(vq->kick_fd);
vq->kick_fd = -1;
}
@@ -1169,7 +1158,7 @@ void vu_cleanup(struct vu_dev *vdev)
vu_close_log(vdev);
if (vdev->device_state_fd != -1) {
- vu_remove_watch(vdev, vdev->device_state_fd);
+ epoll_del(vdev->context, vdev->device_state_fd);
close(vdev->device_state_fd);
vdev->device_state_fd = -1;
vdev->device_state_result = -1;
diff --git a/vu_common.c b/vu_common.c
index f43d8ac4..2c12dca3 100644
--- a/vu_common.c
+++ b/vu_common.c
@@ -325,8 +325,7 @@ void vu_migrate(struct vu_dev *vdev, uint32_t events)
/* value to be returned by VHOST_USER_CHECK_DEVICE_STATE */
vdev->device_state_result = ret == -1 ? -1 : 0;
/* Closing the file descriptor signals the end of transfer */
- epoll_ctl(vdev->context->epollfd, EPOLL_CTL_DEL,
- vdev->device_state_fd, NULL);
+ epoll_del(vdev->context, vdev->device_state_fd);
close(vdev->device_state_fd);
vdev->device_state_fd = -1;
} else if (events & EPOLLIN) {
@@ -346,8 +345,7 @@ void vu_migrate(struct vu_dev *vdev, uint32_t events)
debug("Closing migration channel");
/* The end of file signals the end of the transfer. */
- epoll_ctl(vdev->context->epollfd, EPOLL_CTL_DEL,
- vdev->device_state_fd, NULL);
+ epoll_del(vdev->context, vdev->device_state_fd);
close(vdev->device_state_fd);
vdev->device_state_fd = -1;
}
--
@@ -325,8 +325,7 @@ void vu_migrate(struct vu_dev *vdev, uint32_t events)
/* value to be returned by VHOST_USER_CHECK_DEVICE_STATE */
vdev->device_state_result = ret == -1 ? -1 : 0;
/* Closing the file descriptor signals the end of transfer */
- epoll_ctl(vdev->context->epollfd, EPOLL_CTL_DEL,
- vdev->device_state_fd, NULL);
+ epoll_del(vdev->context, vdev->device_state_fd);
close(vdev->device_state_fd);
vdev->device_state_fd = -1;
} else if (events & EPOLLIN) {
@@ -346,8 +345,7 @@ void vu_migrate(struct vu_dev *vdev, uint32_t events)
debug("Closing migration channel");
/* The end of file signals the end of the transfer. */
- epoll_ctl(vdev->context->epollfd, EPOLL_CTL_DEL,
- vdev->device_state_fd, NULL);
+ epoll_del(vdev->context, vdev->device_state_fd);
close(vdev->device_state_fd);
vdev->device_state_fd = -1;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 3+ messages in thread