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=202606 header.b=OLCfLgSD; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id B87BD5A0262 for ; Mon, 22 Jun 2026 12:03:24 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1782122600; bh=fOpObSdaqhxUgbj9TMjel7f8EyoKM9YOOqq2fbLoF3w=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=OLCfLgSDUy1xgzMTrYy4WzqkpF+UYIhEBmuGk2/Ch9ingfasrubsKA0oMzb6lOEz4 lpPyjh8/Bz+FT2/OJzMRAS3xI/Z3jhAPLap5cdeJMqaDbhyGJECLzoYBQGXgOzirlJ KC6jXxXYbk+4IABM+AfgF4eKzNcKA2DpiwtybS2vOEd6plhgGr9VClHOiBtqP9yh4L Ijwo7j38lMRIDnBl+eX2giXayHcM2g/an17fazV6IMH9ozcLjxTTOZr//7G8+6Vl07 oiD6+qNtb9VepIUS2zr2lHJc4dGFOE7zc4XblpHGjJnpNY38tL9K1BMyrMklQCVtER dznEUcZM5VCFA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4gkP0S1bJPz4wSw; Mon, 22 Jun 2026 20:03:20 +1000 (AEST) Date: Mon, 22 Jun 2026 20:01:50 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v5 06/12] tcp: Pass queue pair explicitly through TCP send path Message-ID: References: <20260616125130.1324274-1-lvivier@redhat.com> <20260616125130.1324274-7-lvivier@redhat.com> <499724cc-6262-402d-9e63-b38ec171a7b2@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="1fsGC1uh2yrkUEcH" Content-Disposition: inline In-Reply-To: Message-ID-Hash: WRHYPFZQ7V3JDEWGT2FCH4JNNYEGMIHG X-Message-ID-Hash: WRHYPFZQ7V3JDEWGT2FCH4JNNYEGMIHG 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: --1fsGC1uh2yrkUEcH Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jun 22, 2026 at 09:44:45AM +0200, Laurent Vivier wrote: > On 6/22/26 04:36, David Gibson wrote: > > On Fri, Jun 19, 2026 at 07:07:56PM +0200, Laurent Vivier wrote: > > > On 6/19/26 08:00, David Gibson wrote: > > > > On Tue, Jun 16, 2026 at 02:51:24PM +0200, Laurent Vivier wrote: > > > > > Thread a qpair parameter from the entry points (tcp_sock_handler, > > > > > tcp_timer_handler, tcp_tap_handler, tcp_defer_handler) through ev= ery > > > > > intermediate function down to the vhost-user send functions, so c= allers > > > > > explicitly select the target RX virtqueue instead of hardcoding > > > > > QPAIR_DEFAULT. > > > > >=20 > > > > > Add a qpair parameter to tcp_send_flag(), tcp_data_from_sock(), > > > > > tcp_rst_do() and its tcp_rst() macro, tcp_rewind_seq(), > > > > > tcp_data_from_tap(), tcp_conn_from_sock_finish(), tcp_connect_fin= ish(), > > > > > tcp_tap_window_update(), tcp_conn_from_tap(), tcp_rst_no_conn(), > > > > > tcp_keepalive(), and tcp_inactivity(). > > > >=20 > > > > For the to-guest functions which take a connection parameter, this > > > > seems odd to me. Can't they deduce the right queue from the > > > > connection? > > >=20 > > > The connection's qpair (conn->f.qpair) can change at any time when > > > another thread processes a tap packet for the same flow and calls > > > FLOW_MIGRATE(). > >=20 > > Ahh. Hmm. Right. I guess I don't really know the concurrency model > > you're going for. I had assumed that each flow was pinned to a > > thread, and migrating it was an operation requiring a maybe complex > > and heavyweight synchronization step. > >=20 > > > If a socket event fires on qpair 0's epoll instance and we read > > > conn->f.qpair during processing, another thread might change it > > > concurrently via FLOW_MIGRATE(). The qpair from the epoll event is > > > the stable, race-free reference for "which queue am I operating on." > >=20 > > Ok. For call chains initiated on the tap side, that's clear enough - > > it's the queue we got the initiating event on. For things initiaed on > > the socket side it's less clear what "queue I am operating on" means. > > I guess it means the queue associated with the epoll set tne > > initiating event occurred on? >=20 > Yes, qpairs are binded to an epoll fd. In the 3rd series (I didn't send i= t, > 2nd is about to handle concurrency), each qpair is binded to an epollfd a= nd > each epollfd is handled by a thread. The socket-side qpair is the queue > associated with the epoll instance that delivere the event. Ok. > > > So the explicit parameter is intentional: it carries the qpair from > > > the epoll event through the call chain, independent of the mutable > > > flow state. > >=20 > > I guess I haven't looked at the multithread series yet, but it feels > > like the picture has a gap. In general we freely access fields in the > > connection. I had assumed that meant it was "owned" by the current > > thread. If the owning thread can change midstream, don't we need some > > other sort of synchronization accessing *anything* in the flow entry? >=20 > Yes, you're right, I missed that. I added lock to the flow table but only= to > access the table, not the individual entry. Right, we certainly have to consider synchronization both each entry and the table as a whole. > Perhaps we can do the migration at the end of the processing of the entry, > perhaps in the post processing function? That might be helpful, but it's not a solution on its own: each thread will presumably be doing post epoll() processing independently, so they'd still need to be synchronized. With or without that deferring, could we do this by making the table lock an rwlock instead of a mutex? Each thread would take a read lock while they process an event. Adding, removing or migrating flows would require a write lock. --=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 --1fsGC1uh2yrkUEcH Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmo5B/8ACgkQzQJF27ox 2GeShhAAkEV9fGiOTw2/4zGrORsXcSKrznYydkndiyqFcjYCp4Cys2rgW4+f/iyY UJLhMbsnoKoQb9skfRCKN8XOBRcKwk3J0p4Bw6XCpsDjUSu0gPPxU7I9phzxza0G k6NZKuh64jqdmtq4+0BhklKMphafDkr9EYVBBqCY/84IXXVsmFbiBgdMd5VzXvgC 5O7wH/B1hA3nLAS6LyPRoO+aAc6jyNiV47rHV9FnCLg1ynaOdOJKVDk6ei1WxQrG N0QpY+L+VtrfH3SHZRF5Rj0bsQaotXsOnIw1KQDrc4ybVwkXnXDAMgvX1aqenwyV AhFWhMW80uON4WuySwV7SfGJi/FPrW1eo/3jZEtYp3nHAaffk0GMk5tVD/MN0vvU 7fWF9G7fsX03hvZfQ8rpc3+1oJMI1Mfrh3Czu+eS/vvA7LXODZqo3v9Fhx6zSsNb pRNy5jm+bG2qbaiE28Ej7gDSlShBMD5g6YBsvnlkt+fTBfC0GCh2OK0Av3V3XrBV QEyIzlbkzA0TvYF0/40ySLRgzUF02Yvc3QcdA/aIdaYqUxzpagDZ9wHz5F8xseju UZrOYa7iY7zx8gi2jqRVPQB2yJEZlJrNW20SHpq9S2Vs2LqyZUgsokuwLJ+hOOPs 9NyCHEfa+XWJbWgVl1HTPdMrULW//ZHEhD0YBh5SqHhzKFQb9OQ= =sxcG -----END PGP SIGNATURE----- --1fsGC1uh2yrkUEcH--