Re: [PATCH 7/8] udp: Decide whether to "splice" per datagram rather than per socket

[Stefano Brivio <sbrivio@redhat.com>]

On Mon,  5 Dec 2022 19:14:24 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Currently we have special sockets for receiving datagrams from locahost
> which can use the optimized "splice" path rather than going across the tap
> interface.
> 
> We want to loosen this so that sockets can receive sockets that will be
> forwarded by both the spliced and non-spliced paths.  To do this, we alter
> the meaning of the @splice bit in the reference to mean that packets
> receieved on this socket *can* be spliced, not that they *will* be spliced.
> They'll only actually be spliced if they come from 127.0.0.1 or ::1.
> 
> We can't (for now) remove the splice bit entirely, unlike with TCP.  Our
> gateway mapping means that if the ns initiates communication to the gw
> address, we'll translate that to target 127.0.0.1 on the host side.  Reply
> packets will therefore have source address 127.0.0.1 when received on the
> host, but these need to go via the tap path where that will be translated
> back to the gateway address.  We need the @splice bit to distinguish that
> case from packets going from localhost to a port mapped explicitly with
> -u which should be spliced.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  udp.c | 54 +++++++++++++++++++++++++++++++++++-------------------
>  udp.h |  2 +-
>  2 files changed, 36 insertions(+), 20 deletions(-)
> 
> diff --git a/udp.c b/udp.c
> index 6ccfe8c..011a157 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -513,16 +513,27 @@ static int udp_splice_new_ns(void *arg)
>  }
>  
>  /**
> - * sa_port() - Determine port from a sockaddr_in or sockaddr_in6
> + * udp_mmh_splice_port() - Is source address of message suitable for splicing?
>   * @v6:		Is @sa a sockaddr_in6 (otherwise sockaddr_in)?
> - * @sa:		Pointer to either sockaddr_in or sockaddr_in6
> + * @mmh:	mmsghdr of incoming message
> + *
> + * Return: if @sa refers to localhost (127.0.0.1 or ::1) the port from
> + *         @sa, otherwise 0.
> + *
> + * NOTE: this relies on the fact that it's not valid to use UDP port 0

The port is reserved by IANA indeed, but... it can actually be used. On
Linux, you can bind() it and you can connect() to it. As far as I can
tell from the new version of udp_sock_handler() we would actually
misdirect packets in that case.

How bad would it be to use an int here?

By the way, I think the comment should also mention that the port is
returned in host order.

-- 
Stefano