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=UMIb5PEH; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id 316F25A0BC2 for ; Fri, 14 Nov 2025 01:01:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1763078481; 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=c+6EC3Jh6P4w9AYs7TgVOyOcYmT44P+mgcJdWDB5y0U=; b=UMIb5PEH8x6uEbgRxRaq54MXmGIZ3h/0lQvV5S09p7SdjCymC8mQXYY+RYV+0OAqXUC09t cNWD6+A1TDmQE4DueG8aCN519VFvP5j/V0WbBIwlI/Iz6xUm2Gc2TyvDNCnysVe4QU/ZV8 FhzV14CBJV+zFOxClr8GicvaxkXR0Hg= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-147-MpuOx5UlOvGGjlWwYM6yNQ-1; Thu, 13 Nov 2025 19:01:16 -0500 X-MC-Unique: MpuOx5UlOvGGjlWwYM6yNQ-1 X-Mimecast-MFC-AGG-ID: MpuOx5UlOvGGjlWwYM6yNQ_1763078475 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-471201dc0e9so9653015e9.2 for ; Thu, 13 Nov 2025 16:01:16 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763078475; x=1763683275; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=c+6EC3Jh6P4w9AYs7TgVOyOcYmT44P+mgcJdWDB5y0U=; b=v+l5bedxt+owg1bMBOQ80MtSzwlxqwCh4MdJvvqZ7b1fz18MSEenhFUMQ3G11B6/h6 +PhfZeIppjK2XWVmBVOy3MLrnQ4liIskp3jQRQr3fJe5OBT/XYL0ex7AZnnZMlN6/KXh mREvJj71s1FWD4/j0wh/gUNDZIMiEsDFaDUzpDcUCqWIQeH+BpFTVJZfTouh9hWIfiTi +RXoV9hDBAUxdCoOBX9NhoPDSN/ELaBNJ/t2O1MWuCIT22IbLRvj8JFUbsM5KGQL0X1Q BHke3GVEBY2m50jzT4W2ouoFOpc4aGyZ1fp+fyfj6wj0XLWSzqZ3q9HauQHkNiSn6lY0 +32g== X-Forwarded-Encrypted: i=1; AJvYcCVxlGFxRQG3nnKTv9nnJctmGEeb1zbk8OK8TjETr7Ti30Ni/gpijKR87/NAsgZGF6/VFrYCvoM+4mQ=@passt.top X-Gm-Message-State: AOJu0YyDcpcO/w0uP74tkO2Pb2uVkHc05okbQs4tIl2/cD98gz6WN7VP 1u/9TgIdNGH07dHngHVIWc6bwCdcwedxBS00Uh5kYK0ugKoDbvkeObeSPl4/v+nnrEWIKyofYJm 7xsZ1xNGDzqHhOQziga9VjV/ag/X33pT8SavzONZ17i21M+0q842gNQ== X-Gm-Gg: ASbGncuXBYj+jQy1oSuBw+sC9N4EXHRVmD9RTjEab8ZOyFpPxkK7bQY6eOIpJ0sY73x BEYd+1QnKFImMT2uYtR0bJ1Lu5BLUZtu/qJvqO5vf+ZX3zdBD3OCeBovmwDBhwn1KApZAXa49nc 43DiaI9wwbEV8vSiUz0pDRCYLTj+DRgqkWM0Sqz1pGFGhlY/lXovhyTzRpr7Fs6ynFkCJAddKzI 71mDuMZwSOVWPnj70z88cbBhBQIe3XBg9fTCoOTGvP2mSbwRIFEnFXJP+SVXNE2xBjNab7FqBTH wApB7jwp32iaPbGfPttKE5JYEgYQdReVYrSzbRn7PjZLRtSjMquW8LrD0hKL8awDzPEAT2p4Tr4 HkJHfPR0WzUyk37x4p+KLWiuxhdEwhlI9RZ5gKA== X-Received: by 2002:a05:600c:8b37:b0:46f:b42e:e39e with SMTP id 5b1f17b1804b1-4778feb156amr9111985e9.39.1763078475085; Thu, 13 Nov 2025 16:01:15 -0800 (PST) X-Google-Smtp-Source: AGHT+IHCQSdBGAh3ac+6cuu2vh49hwSJLxJo6jlWBbGG6ghW4P6J5bj4cymKvfPf3CddyA+WxGlObg== X-Received: by 2002:a05:600c:8b37:b0:46f:b42e:e39e with SMTP id 5b1f17b1804b1-4778feb156amr9111765e9.39.1763078474540; Thu, 13 Nov 2025 16:01:14 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-47787e35b7esm120642815e9.4.2025.11.13.16.01.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Nov 2025 16:01:14 -0800 (PST) Date: Fri, 14 Nov 2025 01:01:12 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v8 3/6] tcp: Add parameter struct ctx *c to tcp_timer_ctl() Message-ID: <20251114010112.20ab39e1@elisabeth> In-Reply-To: References: <20251110093137.87705-1-yuhuang@redhat.com> <20251110093137.87705-4-yuhuang@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: 95wwrh8cTu4g02fU-wJelPKCVPDNoOcvD9WRmKQflLA_1763078475 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: JRCL3N6LOPUCMPWEUVZQIL3OSVWVGZTO X-Message-ID-Hash: JRCL3N6LOPUCMPWEUVZQIL3OSVWVGZTO 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: 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: 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. 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