Re: [PATCH v8 3/6] tcp: Add parameter struct ctx *c to tcp_timer_ctl()

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:

...

On Mon, Nov 10, 2025 at 05:31:34PM +0800, Yumei Huang wrote:

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.

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.

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.

Still, yes, mentioning that, say:

--- 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. ---

would be nice.

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.

...

...

Signed-off-by: Yumei Huang

For the code itself,

Reviewed-by: David Gibson

...

--- tcp.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)

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, struct tcp_tap_conn *conn)

/** * tcp_timer_ctl() - Set timerfd based on flags/events, create timerfd 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 = { { 0 }, { 0 } };

@@ -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 == ACK_FROM_TAP_DUE) - tcp_timer_ctl(conn); + tcp_timer_ctl(c, conn);

return; } @@ -647,7 +648,7 @@ void conn_flag_do(const struct ctx *c, struct tcp_tap_conn *conn, if (flag == ACK_FROM_TAP_DUE || flag == ACK_TO_TAP_DUE || (flag == ~ACK_FROM_TAP_DUE && (conn->flags & ACK_TO_TAP_DUE)) || (flag == ~ACK_TO_TAP_DUE && (conn->flags & ACK_FROM_TAP_DUE))) - tcp_timer_ctl(conn); + tcp_timer_ctl(c, conn); }

/** @@ -702,7 +703,7 @@ void conn_event_do(const struct ctx *c, struct tcp_tap_conn *conn, tcp_epoll_ctl(c, conn);

if (CONN_HAS(conn, SOCK_FIN_SENT | TAP_FIN_ACKED)) - tcp_timer_ctl(conn); + tcp_timer_ctl(c, conn); }

/** @@ -1770,7 +1771,7 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn, seq, conn->seq_from_tap);

tcp_send_flag(c, conn, ACK); - tcp_timer_ctl(conn); + tcp_timer_ctl(c, conn);

if (p->count == 1) { tcp_tap_window_update(c, conn, @@ -2421,7 +2422,7 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref)

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, union epoll_ref ref) return;

tcp_data_from_sock(c, conn); - tcp_timer_ctl(conn); + tcp_timer_ctl(c, conn); } } else { struct itimerspec new = { { 0 }, { ACT_TIMEOUT, 0 } }; -- 2.51.0

-- Stefano

-- 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