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=202608 header.b=rttXUS1Z; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 3B2335A0269 for ; Mon, 03 Aug 2026 08:11:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202608; t=1785737502; bh=wfo315lBVEfo0qNHs9H2uwgFIP82wxM2PwdkD1MuhLM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=rttXUS1ZYks3VdII4w3NdMQCv2OpDa+uamc1VIe9rex2GKFpcn/EJ4wXXvc3KC8q4 8vQyleGK5+cbvtIMp3DCO1EdLtM4F80csE0qrspFJQmkUQzv4KPnjvjYs4zgCdAj1u 7tYZL78SP87SNtmGDXmfyF6RsrbShCaMBd1iPKNmKwE1iij0r+Kr3lOfn8SPAcARoa 8BenQqPqrijT/+lQC55idvwIZ3shb1WoBZzHa0UNbKQC1uRpv5fkaicmR8T8dYAuEk dU2plW2JCLnhHJWZ0f2t+cHedkbguYE4en/Kpm8HA6KM/MlZG2nnCmLJD/6PvTv+Ot jA3GLWPqMTFYw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4hD5sp5K4nz4wCG; Mon, 03 Aug 2026 16:11:42 +1000 (AEST) Date: Mon, 3 Aug 2026 16:11:24 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v2 05/10] flow: Make flow timer per-caller for thread safety Message-ID: References: <20260731162329.3552800-1-lvivier@redhat.com> <20260731162329.3552800-6-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="4fNKeAGej86UXQsM" Content-Disposition: inline In-Reply-To: <20260731162329.3552800-6-lvivier@redhat.com> Message-ID-Hash: VIUZK5MAWOITBWUTHIAPAYDAC6JWT7HV X-Message-ID-Hash: VIUZK5MAWOITBWUTHIAPAYDAC6JWT7HV 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: --4fNKeAGej86UXQsM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jul 31, 2026 at 06:23:24PM +0200, Laurent Vivier wrote: > Move the static flow_timer_run variable out of flow.c and pass it as a > parameter to flow_defer_handler(). This allows each caller to maintain > its own timer state: each vhost-user queue pair worker uses the per-qpair > context. >=20 > Signed-off-by: Laurent Vivier One small misgiving about this, although I doubt it can cause a real problem. Becayse this timer processing now doesn't happen all at once, at each queue is on a separate timer cycle, it's theoretically possible for a flow to change qpair every FLOW_TIMER_INTERVAL and thereby indefinitely avoid having the timer running on it. It's pretty tricky to do (depending on how close in real time each queue's timers end up running), and I don't *think* that can do anything terribly bad (maybe delay cleanup). But since the guest could in principle control the qpair placements to specifically trigger this, it just makes me think it warrants a good close look to make sure that something bad can't be triggered that way. > --- > flow.c | 10 ++++------ > flow.h | 2 +- > passt.c | 8 +++++--- > 3 files changed, 10 insertions(+), 10 deletions(-) >=20 > diff --git a/flow.c b/flow.c > index 1efa9b966991..59963ea5b1c2 100644 > --- a/flow.c > +++ b/flow.c > @@ -142,9 +142,6 @@ static flow_sidx_t flow_hashtab[FLOW_HASH_SIZE]; > static_assert(ARRAY_SIZE(flow_hashtab) >=3D 2 * FLOW_MAX, > "Safe linear probing requires hash table with more entries than the numb= er of sides in the flow table"); > =20 > -/* Last time the flow timers ran */ > -static struct timespec flow_timer_run; > - > /** flowside_from_af() - Initialise flowside from addresses > * @side: flowside to initialise > * @af: Address family (AF_INET or AF_INET6) > @@ -875,10 +872,11 @@ flow_sidx_t flow_lookup_sa(const struct ctx *c, uin= t8_t proto, uint8_t pif, > * flow_defer_handler() - Handler for per-flow deferred and timed tasks > * @c: Execution context > * @now: Current timestamp > + * @timer_run: Last time the flow timers ran > * @qpair: Queue pair to process > */ > void flow_defer_handler(const struct ctx *c, const struct timespec *now, > - unsigned int qpair) > + struct timespec *timer_run, unsigned int qpair) > { > struct flow_free_cluster *free_head =3D NULL; > unsigned *last_next =3D &flow_first_free; > @@ -886,9 +884,9 @@ void flow_defer_handler(const struct ctx *c, const st= ruct timespec *now, > bool timer =3D false; > union flow *flow; > =20 > - if (timespec_diff_ms(now, &flow_timer_run) >=3D FLOW_TIMER_INTERVAL) { > + if (timespec_diff_ms(now, timer_run) >=3D FLOW_TIMER_INTERVAL) { > timer =3D true; > - flow_timer_run =3D *now; > + *timer_run =3D *now; > } > =20 > assert(!flow_new_entry); /* Incomplete flow at end of cycle */ > diff --git a/flow.h b/flow.h > index 028d42bccc0c..ac1d3897ca98 100644 > --- a/flow.h > +++ b/flow.h > @@ -272,7 +272,7 @@ int flow_epoll_set(const struct flow_common *f, int c= ommand, uint32_t events, > int fd, unsigned int sidei); > =20 > void flow_defer_handler(const struct ctx *c, const struct timespec *now, > - unsigned int qpair); > + struct timespec *timer_run, unsigned int qpair); > int flow_migrate_source_early(struct ctx *c, const struct migrate_stage = *stage, > int fd); > int flow_migrate_source_pre(struct ctx *c, const struct migrate_stage *s= tage, > diff --git a/passt.c b/passt.c > index bc9f1575ef8c..69fd7b12450a 100644 > --- a/passt.c > +++ b/passt.c > @@ -108,15 +108,16 @@ struct passt_stats { > * post_handler() - Run periodic and deferred tasks for L4 protocol hand= lers > * @c: Execution context > * @now: Current timestamp > + * @timer_run: Last time the flow timers ran > * @qpair: Queue pair to process > */ > static void post_handler(struct ctx *c, const struct timespec *now, > - unsigned int qpair) > + struct timespec *timer_run, unsigned int qpair) > { > if (!c->no_tcp) > tcp_defer_handler(c, now, qpair); > =20 > - flow_defer_handler(c, now, qpair); > + flow_defer_handler(c, now, timer_run, qpair); > fwd_scan_ports_timer(c, now); > =20 > if (!c->no_ndp) > @@ -231,6 +232,7 @@ static void print_stats(const struct ctx *c, const st= ruct passt_stats *stats, > static void passt_worker(void *opaque, int nfds, struct epoll_event *eve= nts) > { > static struct passt_stats stats =3D { 0 }; > + static struct timespec flow_timer_run; > struct ctx *c =3D opaque; > struct timespec now; > int i; > @@ -314,7 +316,7 @@ static void passt_worker(void *opaque, int nfds, stru= ct epoll_event *events) > print_stats(c, &stats, &now); > } > =20 > - post_handler(c, &now, QPAIR_DEFAULT); > + post_handler(c, &now, &flow_timer_run, QPAIR_DEFAULT); > =20 > migrate_handler(c, &now); > } > --=20 > 2.54.0 >=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 --4fNKeAGej86UXQsM Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmpwMP4ACgkQzQJF27ox 2Gc0lg/9GbwbLTiZMTJLJ/pusIxiz10LrYh0Z8O1gIJKaGyVUGlyGFWPMsQlzJ8V JxIOrJS9GshGPseIhANQ/7fIiIGcK7xmNLD8G3W1O2MxwVMwFNIostkQD2VOPZT8 ZW5RkmtDI8m9YQlMB1TyUXnb1CN+Clbu5xZ9YkHxOmASCQFt7WztDPqnUsFYvSDs nTSzP/YTN5DhIY/Afl4MPBrbzOBf/mFXEwab/gjFKsyMiMXB/hMLWMyNQH1iMR5I cfVIpeTe+GIddb30ji1ujxY16cEzNt20mTiV4Dw8Nw0pNagirTLsHlso5B6lVyxN tRw68XNeavn4aQAaaleTvqeNsip16Ggs+/L4Otjs5DypfT4yGL4Ab3eD4FbAqrUN sC8jjXyqfpM+/0Rck/NEgIaMep79KJQez2WYWGiFv1krRzCr/rfkE/YJcLyx6zyv lCpFRw1xVpmFtoLhtwzHl9L8MWSjwSrMFRc4a5GYf0s+YF5BpHj0TzMZ9tTGV1F5 Ctqpre6rMIvb4Z6uuWPjvpjuKUQHAv8E636tyeBU0YKvAjQ89H/yNjhgB9qJMWYZ iUZaTE8DE8eSAYGoFIc4j8IiMfA+EyUi5nO5HM5i/QKXdC9+6EQ+Y3uyjRTeHWQR n3ILQ0yvbheOOV8z5Ja6HOzqfS5MhG39g9stpkUUUjvQJifd1KE= =3AVN -----END PGP SIGNATURE----- --4fNKeAGej86UXQsM--