Re: [PATCH v5 06/12] tcp: Pass queue pair explicitly through TCP send path

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2704 bytes --]

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 every
> > > intermediate function down to the vhost-user send functions, so callers
> > > explicitly select the target RX virtqueue instead of hardcoding
> > > QPAIR_DEFAULT.
> > > 
> > > 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_finish(),
> > > tcp_tap_window_update(), tcp_conn_from_tap(), tcp_rst_no_conn(),
> > > tcp_keepalive(), and tcp_inactivity().
> > 
> > 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?
> 
> 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().

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.

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

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?

> So the explicit parameter is intentional: it carries the qpair from
> the epoll event through the call chain, independent of the mutable
> flow state.

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?

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]