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

[Laurent Vivier <lvivier@redhat.com>]

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

Yes, qpairs are binded to an epoll fd. In the 3rd series (I didn't send it, 2nd is about 
to handle concurrency), each qpair is binded to an epollfd and each epollfd is handled by 
a thread. The socket-side qpair is the queue associated with the epoll instance that 
delivere the event.


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

Yes, you're right, I missed that. I added lock to the flow table but only to access the 
table, not the individual entry.

Perhaps we can do the migration at the end of the processing of the entry, perhaps in the 
post processing function?

Thanks,
Laurent