From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202512 header.b=EPsm8OL/; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 909E45A0624 for ; Mon, 19 Jan 2026 09:36:38 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202512; t=1768811796; bh=ykgovETxaiRLDbnvBdZeBynfC3kvaxi7JjGZ97CIRTM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=EPsm8OL/cYqnh9oHkoyFzlKwSCdlyrnAY5sthRfwC74n9tko62xqKmk06gUoR2nRK oIaVUyEET9XbjoiX51n2HtWKuOEH8EQp9lD+WCpHdKvr2met8mUzpiH38d13ZFB2l7 IY+JoDxb61jJxx+XS7af5F5lUU3+q5cUFkBBF5P5oCfHDxTnaZaLs6amaCgJpiucHc 5AvT3npvyR5YTWE2ELnzKXhSx6TnhLY5jQ/+Zi5F72WcfL6B0WifZjywiK7HVKbsUK u1dpeOIf9o8C0bV2jrZdCeOjF7D2RV39trtIYdaCmK/duSCVBQjNnOUuUVldy6Be2E eAAvQtCEcnOGA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4dvkMS30NYz4wHX; Mon, 19 Jan 2026 19:36:36 +1100 (AEDT) Date: Mon, 19 Jan 2026 19:36:29 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH 1/3] tcp_splice: Register fds with epoll at flow creation Message-ID: References: <20260116155223.2717168-1-lvivier@redhat.com> <20260116155223.2717168-2-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="bK5bq1T2AsReazAM" Content-Disposition: inline In-Reply-To: Message-ID-Hash: AH4E7NURECELF3BJJQIVSIPAPNDA2Z43 X-Message-ID-Hash: AH4E7NURECELF3BJJQIVSIPAPNDA2Z43 X-MailFrom: dgibson@gandalf.ozlabs.org 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: --bK5bq1T2AsReazAM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jan 19, 2026 at 09:10:42AM +0100, Laurent Vivier wrote: > On 1/19/26 05:45, David Gibson wrote: > > On Fri, Jan 16, 2026 at 04:52:21PM +0100, Laurent Vivier wrote: > > > Register both splice connection sockets with epoll using empty events > > > (events=3D0) in tcp_splice_connect(), before initiating the connectio= n. > > >=20 > > > This allows tcp_splice_epoll_ctl() to always use EPOLL_CTL_MOD, remov= ing > > > the need to check whether fds are already registered. As a result, the > > > conditional ADD/MOD logic is no longer needed, simplifying the functi= on. > > >=20 > > > Signed-off-by: Laurent Vivier > >=20 > > Nice! One query below. > >=20 > > > --- > > > tcp_splice.c | 20 ++++++++++---------- > > > 1 file changed, 10 insertions(+), 10 deletions(-) > > >=20 > > > diff --git a/tcp_splice.c b/tcp_splice.c > > > index a7c04ca8652a..cb81e012ee4b 100644 > > > --- a/tcp_splice.c > > > +++ b/tcp_splice.c > > > @@ -142,20 +142,12 @@ static uint32_t tcp_splice_conn_epoll_events(ui= nt16_t events, unsigned sidei) > > > static int tcp_splice_epoll_ctl(struct tcp_splice_conn *conn) > > > { > > > uint32_t events[2]; > > > - int m; > > > - > > > - if (flow_in_epoll(&conn->f)) { > > > - m =3D EPOLL_CTL_MOD; > > > - } else { > > > - flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); > > > - m =3D EPOLL_CTL_ADD; > > > - } > > > events[0] =3D tcp_splice_conn_epoll_events(conn->events, 0); > > > events[1] =3D tcp_splice_conn_epoll_events(conn->events, 1); > > > - if (flow_epoll_set(&conn->f, m, events[0], conn->s[0], 0) || > > > - flow_epoll_set(&conn->f, m, events[1], conn->s[1], 1)) { > > > + if (flow_epoll_set(&conn->f, EPOLL_CTL_MOD, events[0], conn->s[0], = 0) || > > > + flow_epoll_set(&conn->f, EPOLL_CTL_MOD, events[1], conn->s[1], = 1)) { > > > int ret =3D -errno; > > > flow_perror(conn, "ERROR on epoll_ctl()"); > > > return ret; > > > @@ -368,6 +360,14 @@ static int tcp_splice_connect(const struct ctx *= c, struct tcp_splice_conn *conn) > > > pif_sockaddr(c, &sa, tgtpif, &tgt->eaddr, tgt->eport); > > > + flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); > > > + if (flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, conn->s[0], 0) || > > > + flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, conn->s[1], 1)) { > > > + int ret =3D -errno; > > > + flow_perror(conn, "Cannot register to epollfd"); > > > + return ret; > >=20 > > Do we need to worry about rollback here, if the first one succeeds, > > but the second one fails? >=20 > If we return an error here, tcp_splice_conn_from_sock() sets the CLOSING > flag on the connection and conn_flag() handles the closing flag by calling > epoll_del() for both sockets. So the cleanup path handles this case alrea= dy. >=20 > None of the error cases in tcp_splice_connect() worries about rollback, so > it's simpler to do the same. Ok, that makes sense. Maybe add that explanation to the commit message? --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --bK5bq1T2AsReazAM Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmlt7QwACgkQzQJF27ox 2GdTYRAAkf09vMcuz8Os3YOC1rjz8yJigcpq+y6lOYAHuanDNojhC/f8dtV0MmwQ tXaaaYOOyYRW6nhZQqTdiyjuWm/uwGK3W2f/8WgHAOLDkbOE33urEkIKgia5AWPl mo1yZQJDdF0pL4KpOsBMnWedcA6U3rBjtMtlFBYcU9kkPb3uxBQGe0Zo9cubTiC2 pOJscXGDbhRoo/tbsI4h0C/UGMTBK3VZjfA27IdkqHBxfuMXc6brwwtkJrIasIvi I+jSiCh8Y6vyLz/oF9YKZ9SwNixB1RpljkfZJUZ2k83JS3X7owhIphPsPO9vLb0a V+FKEUDE29fgNgbMiZb9AjqixrdwLC/ITxJkXyaL33MYpdYPppOabo/Vuyq9TONM CXuZ5QTFwZ4u/oOSuHF2iYK8T2AWnT4Y3iBP61yoPOQGkRreRMIgQgMEsF+oIF0Z +1itRHepGbXskgNLilobYvNjGEYETdF+YW6OTKO0knZaHs1ccxtvH0WHUiQHkd4e 96BqcJkDlN0vVsKHu9QyDIIJ1abwpAEVKf3Qw1JyveuA5Yj1VNuB2h5nFyzxc+ng m7Xqbrf/ZVJO/ER/0A9N1WPxSJsYsb5aJMBrAVvf0SmG7gerOQk0uNL1qAHU9hBL A9xkOF1yDmaZ94onPFgWQUaSkz6rRBUgrCECPOVEfJBye4hPJ5Q= =iwjp -----END PGP SIGNATURE----- --bK5bq1T2AsReazAM--