Re: Re: [PATCH bpf v2] xsk: Fix out of order segment free in __xsk_generic_xmit()

[Maciej Fijalkowski]

On Mon, Jun 02, 2025 at 08:28:51AM -0700, Stanislav Fomichev wrote:
> On 06/02, Eryk Kubanski wrote:
> > > I'm not sure I understand what's the issue here. If you're using the
> > > same XSK from different CPUs, you should take care of the ordering
> > > yourself on the userspace side?
> > 
> > It's not a problem with user-space Completion Queue READER side.
> > Im talking exclusively about kernel-space Completion Queue WRITE side.
> > 
> > This problem can occur when multiple sockets are bound to the same
> > umem, device, queue id. In this situation Completion Queue is shared.
> > This means it can be accessed by multiple threads on kernel-side.
> > Any use is indeed protected by spinlock, however any write sequence
> > (Acquire write slot as writer, write to slot, submit write slot to reader)
> > isn't atomic in any way and it's possible to submit not-yet-sent packet
> > descriptors back to user-space as TX completed.
> > 
> > Up untill now, all write-back operations had two phases, each phase
> > locks the spinlock and unlocks it:
> > 1) Acquire slot + Write descriptor (increase cached-writer by N + write values)
> > 2) Submit slot to the reader (increase writer by N)
> > 
> > Slot submission was solely based on the timing. Let's consider situation,
> > where two different threads issue a syscall for two different AF_XDP sockets
> > that are bound to the same umem, dev, queue-id.
> > 
> > AF_XDP setup:
> >                                                             
> >                              kernel-space                   
> >                                                             
> >            Write   Read                                     
> >             +--+   +--+                                     
> >             |  |   |  |                                     
> >             |  |   |  |                                     
> >             |  |   |  |                                     
> >  Completion |  |   |  | Fill                                
> >  Queue      |  |   |  | Queue                               
> >             |  |   |  |                                     
> >             |  |   |  |                                     
> >             |  |   |  |                                     
> >             |  |   |  |                                     
> >             +--+   +--+                                     
> >             Read   Write                                    
> >                              user-space                     
> >                                                             
> >                                                             
> >    +--------+         +--------+                            
> >    | AF_XDP |         | AF_XDP |                            
> >    +--------+         +--------+                            
> >                                                             
> >                                                             
> >                                                             
> >                                                             
> > 
> > Possible out-of-order scenario:
> >                                                                                                                                        
> >                                                                                                                                        
> >                               writer         cached_writer1                      cached_writer2                                        
> >                                  |                 |                                   |                                               
> >                                  |                 |                                   |                                               
> >                                  |                 |                                   |                                               
> >                                  |                 |                                   |                                               
> >                   +--------------|--------|--------|--------|--------|--------|--------|----------------------------------------------+
> >                   |              |        |        |        |        |        |        |                                              |
> >  Completion Queue |              |        |        |        |        |        |        |                                              |
> >                   |              |        |        |        |        |        |        |                                              |
> >                   +--------------|--------|--------|--------|--------|--------|--------|----------------------------------------------+
> >                                  |                 |                                   |                                               
> >                                  |                 |                                   |                                               
> >                                  |-----------------|                                   |                                               
> >                                   A) T1 syscall    |                                   |                                               
> >                                   writes 2         |                                   |                                               
> >                                   descriptors      |-----------------------------------|                                               
> >                                                     B) T2 syscall writes 4 descriptors                                                 
> >                                                                                                                                        
> >                                                                                                                                        
> >                                                                                                                                        
> >                                                                                                                                        
> >                  Notes:                                                                                                                
> >                  1) T1 and T2 AF_XDP sockets are two different sockets,                                                                
> >                     __xsk_generic_xmit will obtain two different mutexes.                                                              
> >                  2) T1 and T2 can be executed simultaneously, there is no                                                              
> >                     critical section whatsoever between them.                                                                          
> 
> XSK represents a single queue and each queue is single producer single
> consumer. The fact that you can dup a socket and call sendmsg from
> different threads/processes does not lift that restriction. I think
> if you add synchronization on the userspace (lock(); sendmsg();
> unlock();), that should help, right?

Eryk, can you tell us a bit more about HW you're using? The problem you
described simply can not happen for HW with in-order completions. You
can't complete descriptor from slot 5 without going through completion of
slot 3. So our assumption is you're using HW with out-of-order
completions, correct?

If that is the case then we have to think about possible solutions which
probably won't be straight-forward. As Stan said current fix is a no-go.