Re: [PATCH v7 21/27] udp: Handle "spliced" datagrams with per-flow sockets

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

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

On Fri, Jul 12, 2024 at 03:34:07PM +0200, Stefano Brivio wrote:
> On Fri,  5 Jul 2024 12:07:18 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > When forwarding a datagram to a socket, we need to find a socket with a
> > suitable local address to send it.  Currently we keep track of such sockets
> > in an array indexed by local port, but this can't properly handle cases
> > where we have multiple local addresses in active use.
> > 
> > For "spliced" (socket to socket) cases, improve this by instead opening
> > a socket specifically for the target side of the flow.  We connect() as
> > well as bind()ing that socket, so that it will only receive the flow's
> > reply packets, not anything else.  We direct datagrams sent via that socket
> > using the addresses from the flow table, effectively replacing bespoke
> > addressing logic with the unified logic in fwd.c
> > 
> > When we create the flow, we also take a duplicate of the originating
> > socket, and use that to deliver reply datagrams back to the origin, again
> > using addresses from the flow table entry.
> 
> For some reason, after this patch (I bisected), I'm getting an EPOLLERR
> loop:
> 
>   pasta: epoll event on UDP socket 6 (events: 0x00000001)
>   Flow 0 (NEW): FREE -> NEW
>   Flow 0 (INI): NEW -> INI
>   Flow 0 (INI): HOST [127.0.0.1]:47041 -> [0.0.0.0]:10001 => ?
>   Flow 0 (TGT): INI -> TGT
>   Flow 0 (TGT): HOST [127.0.0.1]:47041 -> [0.0.0.0]:10001 => SPLICE [127.0.0.1]:47041 -> [127.0.0.1]:10001
>   Flow 0 (UDP flow): TGT -> TYPED
>   Flow 0 (UDP flow): HOST [127.0.0.1]:47041 -> [0.0.0.0]:10001 => SPLICE [127.0.0.1]:47041 -> [127.0.0.1]:10001
>   Flow 0 (UDP flow): Side 0 hash table insert: bucket: 97474
>   Flow 0 (UDP flow): TYPED -> ACTIVE
>   Flow 0 (UDP flow): HOST [127.0.0.1]:47041 -> [0.0.0.0]:10001 => SPLICE [127.0.0.1]:47041 -> [127.0.0.1]:10001
>   pasta: epoll event on UDP reply socket 116 (events: 0x00000008)
>   pasta: epoll event on UDP reply socket 116 (events: 0x00000008)
>   pasta: epoll event on UDP reply socket 116 (events: 0x00000008)
>   [...repeated until I terminate the process]
> 
> by sending one UDP datagram from the parent namespace with no
> "listening" process in the namespace, using the "spliced" path,
> something like this:
> 
>   echo a | nc -q1 -u localhost 10001
> 
> after running pasta with:
> 
>   ./pasta -u 10001 --trace -l /tmp/pasta.trace --log-size $((1 << 30))
> 
> I tried bigger/multiple datagrams, same result.

Ouch.  I see it too.  I'll debug...

> 
> Before this patch, I get something like this instead:
> 
>   5.1018: pasta: epoll event on UDP socket 6 (events: 0x00000001)
>   5.1018: Flow 0 (NEW): FREE -> NEW
>   5.1018: Flow 0 (INI): NEW -> INI
>   5.1019: Flow 0 (INI): HOST [127.0.0.1]:41245 -> [0.0.0.0]:10001 => ?
>   5.1019: Flow 0 (TGT): INI -> TGT
>   5.1019: Flow 0 (TGT): HOST [127.0.0.1]:41245 -> [0.0.0.0]:10001 => SPLICE [127.0.0.1]:41245 -> [127.0.0.1]:10001
>   5.1019: Flow 0 (UDP flow): TGT -> TYPED
>   5.1019: Flow 0 (UDP flow): HOST [127.0.0.1]:41245 -> [0.0.0.0]:10001 => SPLICE [127.0.0.1]:41245 -> [127.0.0.1]:10001
>   5.1019: Flow 0 (UDP flow): Side 0 hash table insert: bucket: 111174
>   5.1019: Flow 0 (UDP flow): TYPED -> ACTIVE
>   5.1019: Flow 0 (UDP flow): HOST [127.0.0.1]:41245 -> [0.0.0.0]:10001 => SPLICE [127.0.0.1]:41245 -> [127.0.0.1]:10001
> 
> I didn't really investigate, though.
> 

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