On Thu, Jun 13, 2024 at 05:06:54PM +0200, Stefano Brivio wrote:On Wed, 5 Jun 2024 11:39:01 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote:Right. In fact this function will go away entirely with the flow table.udp_mmh_splice_port() is used to determine if a UDP datagram can be "spliced" (forwarded via a socket instead of tap). We only invoke it if the origin socket has the 'splice' flag set. Fold the checking of the flag into the helper itself, which makes the caller simpler. It does mean we have a loop looking for a batch of spliceable or non-spliceable packets even in the case where the flag is clear. This shouldn't be that expensive though, since each call to udp_mmh_splice_port() will return without accessing memory in that case. In any case we're going to need a similar loop in more cases with upcoming flow table work. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- udp.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/udp.c b/udp.c index 3abafc99..7487d2b2 100644 --- a/udp.c +++ b/udp.c @@ -467,21 +467,25 @@ static int udp_splice_new_ns(void *arg) /** * udp_mmh_splice_port() - Is source address of message suitable for splicing? - * @v6: Is @sa a sockaddr_in6 (otherwise sockaddr_in)? + * @uref: UDP epoll reference for incoming message's origin socket * @mmh: mmsghdr of incoming message * - * Return: if @sa refers to localhost (127.0.0.1 or ::1) the port from - * @sa in host order, otherwise -1. + * Return: if source address of message in @mmh refers to localhost (127.0.0.1Pre-existing, and I guess this might change again with the complete flow table implementation, so it probably doesn't make sense to fix this now: it's 127.0.0.0/8, not necessarily 127.0.0.1.As to whether we actually need to preserve a source address that's not 127.0.0.1, but in 127.0.0.0/8, as we "splice", I'm not quite sure. I think we could bind() the socket in the target namespace, but I haven't tried, and I don't know if it makes sense at all (I can't think of any use case).So, how to handle 127.0.0.0/8 is something I'm actively thinking about. It should be much easier to tweak this with the flow table in place.-- David Gibson | 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+ * or ::1) its source port (host order), otherwise -1. */ -static int udp_mmh_splice_port(bool v6, const struct mmsghdr *mmh) +static int udp_mmh_splice_port(union udp_epoll_ref uref, + const struct mmsghdr *mmh) { const struct sockaddr_in6 *sa6 = mmh->msg_hdr.msg_name; const struct sockaddr_in *sa4 = mmh->msg_hdr.msg_name; - if (v6 && IN6_IS_ADDR_LOOPBACK(&sa6->sin6_addr)) + if (!uref.splice) + return -1; + + if (uref.v6 && IN6_IS_ADDR_LOOPBACK(&sa6->sin6_addr)) return ntohs(sa6->sin6_port); - if (!v6 && IN4_IS_ADDR_LOOPBACK(&sa4->sin_addr)) + if (!uref.v6 && IN4_IS_ADDR_LOOPBACK(&sa4->sin_addr)) return ntohs(sa4->sin_port); return -1; @@ -768,18 +772,15 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events,(now renamed to udp_buf_sock_handler() if you're wondering)for (i = 0; i < n; i += m) { int splicefrom = -1; - m = n; - if (ref.udp.splice) { - splicefrom = udp_mmh_splice_port(v6, mmh_recv + i); + splicefrom = udp_mmh_splice_port(ref.udp, mmh_recv + i); - for (m = 1; i + m < n; m++) { - int p; + for (m = 1; i + m < n; m++) { + int p; - p = udp_mmh_splice_port(v6, mmh_recv + i + m); - if (p != splicefrom) - break; - } + p = udp_mmh_splice_port(ref.udp, mmh_recv + i + m); + if (p != splicefrom) + break; } if (splicefrom >= 0)