From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=HhNuAyUI; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id B5D445A0619 for ; Thu, 16 Oct 2025 23:31:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1760650295; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=c73EKtM1YytLDzcLBqJNB4SXdyJ54otWODvGfD01UpE=; b=HhNuAyUI+J1o10TfkLDqoX7wp3X53FwwFcu+2c/Ebvs+sQE4bQz0StNG1nXZJdXLRPACIB 8Gkbi70YA12FKC2a1RCYBnTVCaWRjOXiSDtrV2L/MFmt/Qohed8CCMoEQqVs+1dxseeL/b 91RP0mGowxKdAfy2EehNwGMcFf6PoPU= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-682-PfSkksWcOVShC-Rima2BHw-1; Thu, 16 Oct 2025 17:31:33 -0400 X-MC-Unique: PfSkksWcOVShC-Rima2BHw-1 X-Mimecast-MFC-AGG-ID: PfSkksWcOVShC-Rima2BHw_1760650292 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-3f384f10762so1217486f8f.3 for ; Thu, 16 Oct 2025 14:31:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1760650291; x=1761255091; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=c73EKtM1YytLDzcLBqJNB4SXdyJ54otWODvGfD01UpE=; b=aSnQrxzzT/ood9vXl+gNQt9FlT+qL/tv0cMur5omPEHlMrhFmqcJ7ZnfJZUW9a1eA6 xh2FMxM+SlYqIRmLlohVwD2Ryp0V6ApB45QfhrlCo0xwT5u+dVRlmvilNK9gCLZ7uvxY WMqxaIXc/UXu8JZlg2eAfO+Gs69YC/g9suHBMBpI/BDVC4lF6jLc8Z30ethDyi5PjVyv DLGu6cTFoApEybS4Qa/Dfuaagr1+XXXIn2b05YfyEHmZ+kw8Nzs+CAjy6EBJnnuRvoQl J79HBy4Pme42dWuKt1aTgeqbPF8SZkkPfjH5vGMecLDXEZ5bpq1uRyD3ScKf7nfPblX3 Op8w== X-Gm-Message-State: AOJu0YzVks77iw/VThfisq+/3VfxGb+WXMLz69cCB4Va1FlL4Ga92T7l GZT99RsOzvQVgJ7pTZGeVKBv7E6lsIrikOCGeNCZ26GnPV2N9Z20lgKDVnmo0oFFvPrHBEfjrZh x+F4+LrrBTbvlWM6ILzLKR8xA9xGXWBHrkWTlCQiL9S1gZKzNi/fE10feWnqEmp90v4WPQlSvG8 lypThFHpsheNSzlL9Pjag+42mm/NLzUnu68lD7 X-Gm-Gg: ASbGncvTdAeRUrn5x4+vUAR6JzxZ13HrpSu8B/jZ+epOeyjeTsJstEevX73pzBu6Mep KpjsBPWkT4vEAlyMEXCYDOyCoQqPl6DfPtY8PEltrb+t8Y7Tz9HjQe+qB2UF09YLLeGBNkJNaxL RuVcZjbxgzOYslBsRDH0l1COwFlYGqe+ga5LDdy6X6hBcnRTJ37eevfJ5tsYuh9GjtgKYb9XL8x 8ayw4x9BU6tcSE0+TJ5mmfDtEM6if0BknW6qoaz+pjQV6eVrv/fgwR+NjNnfjoS8uVPjlToof0j /BXAJQO26iXbT8IfiM0HdGQB6BaFQX+7pgwAM5i4r7IRR8pmcBbt+G7gxY1JerPEF4YhbtxxI75 G2d8Hnc8eyA== X-Received: by 2002:a05:6000:24c9:b0:426:d549:5861 with SMTP id ffacd0b85a97d-42704e029bcmr1097329f8f.42.1760650290878; Thu, 16 Oct 2025 14:31:30 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHrp9btEbSpJalMiD2g44mYU0giG16uP6GpgFBO9meIGwdegA1HD8hKDVH8syc19XY9mGri4A== X-Received: by 2002:a05:6000:24c9:b0:426:d549:5861 with SMTP id ffacd0b85a97d-42704e029bcmr1097312f8f.42.1760650290331; Thu, 16 Oct 2025 14:31:30 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4711441f66dsm46545925e9.2.2025.10.16.14.31.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Oct 2025 14:31:29 -0700 (PDT) Date: Thu, 16 Oct 2025 23:31:28 +0200 From: Stefano Brivio To: Laurent Vivier Subject: Re: [PATCH v3 2/6] epoll_ctl: Extract epoll operations Message-ID: <20251016233128.2ef96233@elisabeth> In-Reply-To: <20251009130409.3931795-3-lvivier@redhat.com> References: <20251009130409.3931795-1-lvivier@redhat.com> <20251009130409.3931795-3-lvivier@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: oEfvmcoY43hZMianZRaHXVOPcb7a1Jna1mCeu2r6qsk_1760650292 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: QOPXPQNVL45FC23IVTE54QSIR2Y2WRJL X-Message-ID-Hash: QOPXPQNVL45FC23IVTE54QSIR2Y2WRJL X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu, 9 Oct 2025 15:04:05 +0200 Laurent Vivier wrote: > Centralize epoll_add() and epoll_del() helper functions into new > epoll_ctl.c/h files. > > This also moves the union epoll_ref definition from passt.h to > epoll_ctl.h where it's more logically placed. > > The new epoll_add() helper simplifies adding file descriptors to epoll > by taking an epoll_ref and events, handling error reporting > consistently across all call sites. > > Signed-off-by: Laurent Vivier > --- > Makefile | 22 +++++++++++----------- > epoll_ctl.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ > epoll_ctl.h | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ > icmp.c | 4 +--- > passt.c | 2 +- > passt.h | 34 ---------------------------------- > pasta.c | 7 +++---- > repair.c | 14 +++++--------- > tap.c | 13 ++++--------- > tcp.c | 2 +- > tcp_splice.c | 2 +- > udp.c | 2 +- > udp_flow.c | 1 + > util.c | 22 +++------------------- > util.h | 4 +++- > vhost_user.c | 8 ++------ > vu_common.c | 2 +- > 17 files changed, 134 insertions(+), 101 deletions(-) > create mode 100644 epoll_ctl.c > create mode 100644 epoll_ctl.h > > diff --git a/Makefile b/Makefile > index 3328f8324140..91e037b8fd3c 100644 > --- a/Makefile > +++ b/Makefile > @@ -37,23 +37,23 @@ FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE) > FLAGS += -DVERSION=\"$(VERSION)\" > FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS) > > -PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c flow.c fwd.c \ > - icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c log.c mld.c \ > - ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c pif.c \ > - repair.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c udp_flow.c \ > - udp_vu.c util.c vhost_user.c virtio.c vu_common.c > +PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c epoll_ctl.c \ > + flow.c fwd.c icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c \ > + log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c \ > + pif.c repair.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c \ > + udp_flow.c udp_vu.c util.c vhost_user.c virtio.c vu_common.c > QRAP_SRCS = qrap.c > PASST_REPAIR_SRCS = passt-repair.c > SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) > > MANPAGES = passt.1 pasta.1 qrap.1 passt-repair.1 > > -PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h flow.h fwd.h \ > - flow_table.h icmp.h icmp_flow.h inany.h iov.h ip.h isolation.h \ > - lineread.h log.h migrate.h ndp.h netlink.h packet.h passt.h pasta.h \ > - pcap.h pif.h repair.h siphash.h tap.h tcp.h tcp_buf.h tcp_conn.h \ > - tcp_internal.h tcp_splice.h tcp_vu.h udp.h udp_flow.h udp_internal.h \ > - udp_vu.h util.h vhost_user.h virtio.h vu_common.h > +PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h epoll_ctl.h \ > + flow.h fwd.h flow_table.h icmp.h icmp_flow.h inany.h iov.h ip.h \ > + isolation.h lineread.h log.h migrate.h ndp.h netlink.h packet.h \ > + passt.h pasta.h pcap.h pif.h repair.h siphash.h tap.h tcp.h tcp_buf.h \ > + tcp_conn.h tcp_internal.h tcp_splice.h tcp_vu.h udp.h udp_flow.h \ > + udp_internal.h udp_vu.h util.h vhost_user.h virtio.h vu_common.h > HEADERS = $(PASST_HEADERS) seccomp.h > > C := \#include \nint main(){int a=getrandom(0, 0, 0);} > diff --git a/epoll_ctl.c b/epoll_ctl.c > new file mode 100644 > index 000000000000..0a06350f87a5 > --- /dev/null > +++ b/epoll_ctl.c > @@ -0,0 +1,45 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* epoll_ctl.c - epoll manipulation helpers > + * > + * Copyright Red Hat > + * Author: Laurent Vivier > + */ > + > +#include > + > +#include "epoll_ctl.h" > + > +/** > + * epoll_add() - Add a file descriptor to an epollfd > + * @epollfd: epoll file descriptor to add to > + * @events: epoll events > + * @ref: epoll reference for the file descriptor (includes fd and metadata) > + * > + * Return: 0 on success, negative errno on failure > + */ > +int epoll_add(int epollfd, uint32_t events, union epoll_ref *ref) > +{ > + struct epoll_event ev; > + int ret; > + > + ev.events = events; > + ev.data.u64 = ref->u64; > + > + ret = epoll_ctl(epollfd, EPOLL_CTL_ADD, ref->fd, &ev); > + if (ret == -1) { > + ret = -errno; > + warn("Failed to add fd to epoll: %s", strerror_(-ret)); > + } > + > + return ret; > +} > + > +/** > + * epoll_del() - Remove a file descriptor from an epollfd > + * @epollfd: epoll file descriptor to remove from > + * @fd: File descriptor to remove > + */ > +void epoll_del(int epollfd, int fd) > +{ > + epoll_ctl(epollfd, EPOLL_CTL_DEL, fd, NULL); > +} > diff --git a/epoll_ctl.h b/epoll_ctl.h > new file mode 100644 > index 000000000000..cf92b0f63f26 > --- /dev/null > +++ b/epoll_ctl.h > @@ -0,0 +1,51 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later > + * Copyright Red Hat > + * Author: Laurent Vivier > + */ > + > +#ifndef EPOLL_CTL_H > +#define EPOLL_CTL_H > + > +#include > + > +#include "util.h" > +#include "passt.h" > +#include "epoll_type.h" > +#include "flow.h" > +#include "tcp.h" > +#include "udp.h" > + > +/** > + * union epoll_ref - Breakdown of reference for epoll fd bookkeeping > + * @type: Type of fd (tells us what to do with events) > + * @fd: File descriptor number (implies < 2^24 total descriptors) > + * @flow: Index of the flow this fd is linked to > + * @tcp_listen: TCP-specific reference part for listening sockets > + * @udp: UDP-specific reference part > + * @data: Data handled by protocol handlers > + * @nsdir_fd: netns dirfd for fallback timer checking if namespace is gone > + * @queue: vhost-user queue index for this fd > + * @u64: Opaque reference for epoll_ctl() and epoll_wait() > + */ > +union epoll_ref { > + struct { > + enum epoll_type type:8; > + int32_t fd:FD_REF_BITS; Do the definitions of FD_REF_BITS and FD_REF_MAX really belong to util.h? It would be clearer to have them here. I didn't check all the possible places where you would need to include this header though. > + union { > + uint32_t flow; > + flow_sidx_t flowside; > + union tcp_listen_epoll_ref tcp_listen; > + union udp_listen_epoll_ref udp; > + uint32_t data; > + int nsdir_fd; > + int queue; > + }; > + }; > + uint64_t u64; > +}; > +static_assert(sizeof(union epoll_ref) <= sizeof(union epoll_data), > + "epoll_ref must have same size as epoll_data"); Either the comment is misleading, or it should be sizeof(...) != ... The rest of the patch looks good to me, I didn't finish reviewing the series though. -- Stefano