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=202510 header.b=D11/53f5; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id F02975A0BC2 for ; Fri, 14 Nov 2025 01:47:03 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202510; t=1763081221; bh=VqqMveUtYGOZlMfzq97lDl7thg6T0ELpWBXGqhXRiPI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=D11/53f5S7UTtecrVT2ADQlN5bosAraqCEdYgvkNsKjkeoZbpOMegvoSXUXO29FZJ bkk+/7umu+JZiMuNA+DD2EXBfDExBuZTGngL0CSG3c8G+ZPewn2/l0h46tqbq3Yin2 uixl8jSPutrTj3WBVcoLL7HxjwLZ/wNXGyoAlFwoesyxcnYzF7gKLRRjUz4H9mGpDr WCLgyfkoYXhd99wpHvxNXIce3rF6H0YqNSSfhQ+K7HRmHqscG0UtZWUyOYGG49GFUh kdl3+xtj7Pl4ZGAVVz6yo1pikdljMiDzvVfYBlLYBX8Qs/jU9gm7vVpGn4Y4wFwa1M oREcaZ7/UQdbw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4d6z450KGcz4wM9; Fri, 14 Nov 2025 11:47:01 +1100 (AEDT) Date: Fri, 14 Nov 2025 11:36:35 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v8 3/6] tcp: Add parameter struct ctx *c to tcp_timer_ctl() Message-ID: References: <20251110093137.87705-1-yuhuang@redhat.com> <20251110093137.87705-4-yuhuang@redhat.com> <20251114010112.20ab39e1@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="46mb5RyjHNSRu7ma" Content-Disposition: inline In-Reply-To: <20251114010112.20ab39e1@elisabeth> Message-ID-Hash: PWM3XPVFT5FBGQVZA5THJYEYVE5BVBLC X-Message-ID-Hash: PWM3XPVFT5FBGQVZA5THJYEYVE5BVBLC 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: Yumei Huang , 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: --46mb5RyjHNSRu7ma Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 14, 2025 at 01:01:12AM +0100, Stefano Brivio wrote: > On Mon, 10 Nov 2025 21:35:24 +1100 > David Gibson wrote: >=20 > > On Mon, Nov 10, 2025 at 05:31:34PM +0800, Yumei Huang wrote: > >=20 > > Should have a commit message explaining that it was recently removed, > > and why you need it back. Stefano might be able to add that on merge, = though. >=20 > Even better: this should be part of the patch where tcp_timer_ctl() > needs 'c' again, so that if we are bisecting stuff and end up exactly > after this patch/commit, we don't get spurious static checkers warnings. >=20 > But we don't typically check those while bisecting (and I don't see any > good reason to do so), so I don't really care. >=20 > Still, yes, mentioning that, say: >=20 > --- > Commit x ("y") dropped the 'c' parameter to tcp_timer_ctl() but we'll > need it again in the next commit, so add it back. > --- >=20 > would be nice. >=20 > And yes, I can add it myself (even though it doesn't scale much, if I > need to edit a few commit messages and add references for every > series...), but looking at comments to 6/6 I think a respin might be > convenient anyway. Right. Adjusting on merge would only make sense if there was nothing else to polish in a respin. > If you merge this change with the next patch you don't really need to > explain it, by the way, as it's obvious from the rest of the commit. >=20 > > > Signed-off-by: Yumei Huang =20 > >=20 > > For the code itself, > >=20 > > Reviewed-by: David Gibson > >=20 > > > --- > > > tcp.c | 15 ++++++++------- > > > 1 file changed, 8 insertions(+), 7 deletions(-) > > >=20 > > > diff --git a/tcp.c b/tcp.c > > > index ca3d742..2f49327 100644 > > > --- a/tcp.c > > > +++ b/tcp.c > > > @@ -543,11 +543,12 @@ static int tcp_epoll_ctl(const struct ctx *c, s= truct tcp_tap_conn *conn) > > > =20 > > > /** > > > * tcp_timer_ctl() - Set timerfd based on flags/events, create timer= fd if needed > > > + * @c: Execution context > > > * @conn: Connection pointer > > > * > > > * #syscalls timerfd_create timerfd_settime > > > */ > > > -static void tcp_timer_ctl(struct tcp_tap_conn *conn) > > > +static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *= conn) > > > { > > > struct itimerspec it =3D { { 0 }, { 0 } }; > > > =20 > > > @@ -631,7 +632,7 @@ void conn_flag_do(const struct ctx *c, struct tcp= _tap_conn *conn, > > > * flags and factor this into the logic below. > > > */ > > > if (flag =3D=3D ACK_FROM_TAP_DUE) > > > - tcp_timer_ctl(conn); > > > + tcp_timer_ctl(c, conn); > > > =20 > > > return; > > > } > > > @@ -647,7 +648,7 @@ void conn_flag_do(const struct ctx *c, struct tcp= _tap_conn *conn, > > > if (flag =3D=3D ACK_FROM_TAP_DUE || flag =3D=3D ACK_TO_TAP_DUE || > > > (flag =3D=3D ~ACK_FROM_TAP_DUE && (conn->flags & ACK_TO_TAP_DUE= )) || > > > (flag =3D=3D ~ACK_TO_TAP_DUE && (conn->flags & ACK_FROM_TAP_D= UE))) > > > - tcp_timer_ctl(conn); > > > + tcp_timer_ctl(c, conn); > > > } > > > =20 > > > /** > > > @@ -702,7 +703,7 @@ void conn_event_do(const struct ctx *c, struct tc= p_tap_conn *conn, > > > tcp_epoll_ctl(c, conn); > > > =20 > > > if (CONN_HAS(conn, SOCK_FIN_SENT | TAP_FIN_ACKED)) > > > - tcp_timer_ctl(conn); > > > + tcp_timer_ctl(c, conn); > > > } > > > =20 > > > /** > > > @@ -1770,7 +1771,7 @@ static int tcp_data_from_tap(const struct ctx *= c, struct tcp_tap_conn *conn, > > > seq, conn->seq_from_tap); > > > =20 > > > tcp_send_flag(c, conn, ACK); > > > - tcp_timer_ctl(conn); > > > + tcp_timer_ctl(c, conn); > > > =20 > > > if (p->count =3D=3D 1) { > > > tcp_tap_window_update(c, conn, > > > @@ -2421,7 +2422,7 @@ void tcp_timer_handler(const struct ctx *c, uni= on epoll_ref ref) > > > =20 > > > if (conn->flags & ACK_TO_TAP_DUE) { > > > tcp_send_flag(c, conn, ACK_IF_NEEDED); > > > - tcp_timer_ctl(conn); > > > + tcp_timer_ctl(c, conn); > > > } else if (conn->flags & ACK_FROM_TAP_DUE) { > > > if (!(conn->events & ESTABLISHED)) { > > > flow_dbg(conn, "handshake timeout"); > > > @@ -2443,7 +2444,7 @@ void tcp_timer_handler(const struct ctx *c, uni= on epoll_ref ref) > > > return; > > > =20 > > > tcp_data_from_sock(c, conn); > > > - tcp_timer_ctl(conn); > > > + tcp_timer_ctl(c, conn); > > > } > > > } else { > > > struct itimerspec new =3D { { 0 }, { ACT_TIMEOUT, 0 } }; > > > --=20 > > > 2.51.0 >=20 > --=20 > Stefano >=20 --=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 --46mb5RyjHNSRu7ma Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmkWeZIACgkQzQJF27ox 2GfeVw//fQPDkDel5dptpziRzcKCjuFT700sRc7PEh7vbqHTUZbx5GBreya785ph pHDxUfP4eWL3tmh8rIBiOhQEzEjhKDnm4zcH0oMhmdRfGPuznkZWm3zyvKM1iRF5 H/DbEFqnfRqKLo5qMfiyh4hqwCI04F4xzJ/LrwmcGWqolc5sNpqsxEfIEBtzmGgX T3VpKvadw4d5QQVzZvf42mjZRIo6mpsrk7EDG7QvokP168FOzW3YWoRzvXR9FA0Z pjis2HLCEt3PfneR1ZfcF6T55pGrEr3qbQJJ4rV5kxzXmNHsEORfvJbo68NFcdqo WFfL7x9qUttz3+PhhHsIgRjziGXTWK+fGHynBHQpUtNIPek3WRyFjMUTlCio5BAw A81AAYc0ycj4ppR/8Hn595xn9KjYfdAS2J1S+eXOQIlppgdfDAXvKMZtnY61L9Nv vaAaWobHT/Ygdrt9JS/J246jyyFbKKHAaSZeJsni4gk+H42OP0XINSKmBX4pja1Z Kq5A+ywfKbBfUAhOf2VhG7U1oNJGhC/Ct/jlGzF9S1cvrEwA/CnJDfF3QgHyeBF/ vLD3mI4M/w01a8NhoHwKA91vzBLig5Q0L++BnoTk9QzKR8KyLZzWZ58xa4Q082RO l1UCU3psU20hVJ0ppe7kCIqHu7hiUS/NxFuhrFyduJSDRr4/yiA= =bmdr -----END PGP SIGNATURE----- --46mb5RyjHNSRu7ma--