aboutgitcodebugslistschat
path: root/udp.c
Commit message (Collapse)AuthorAgeFilesLines
* Work around weird false positives with cppcheck-2.9.1David Gibson2023-03-211-1/+1
| | | | | | | | | | | | | | | | | | Commit 89e38f55 "treewide: Fix header includes to build with musl" added extra #includes to work with musl. Unfortunately with the cppcheck version I'm using (cppcheck-2.9-1.fc37.x86_64 in Fedora 37) this causes weird false positives: specifically cppcheck seems to hit a #error in <bits/unistd.h> complaining about including it directly instead of via <unistd.h> (which is not something we're doing). I have no idea why that would be happening; but I'm guessing it has to be a bug in the cpp implementation in that cppcheck version. In any case, it's possible to work around this by moving the include of <unistd.h> before the include of <signal.h>. So, do that. Fixes: 89e38f55405d ("treewide: Fix header includes to build with musl") Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Actually bind detected namespace ports in init namespaceStefano Brivio2023-03-211-1/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When I reworked udp_init() to move most of the port binding logic to conf_ports, I accidentally dropped this bit of automatic port detection (and binding) at start-up. On -U auto, in pasta mode, udp_sock_init_ns() binds ports in the namespace that correspond to ports bound in the init namespace, but on -u auto, nothing actually happens after port detection. Add udp_sock_init_init() to deal with this, and while at it fix the comment to udp_sock_init_ns(): the latter takes care of outbound "connections". This is currently not covered by tests, and the UDP port needs to be already bound in the namespace when pasta starts (periodic detection for UDP is a missing feature at the moment). It can be checked like this: $ unshare -rUn # echo $$ 590092 # socat -u UDP-LISTEN:5555 STDOUT $ pasta -q -u auto 590092 $ echo "test" | socat -u STDIN UDP:localhost:5555 Reported-by: Paul Holzinger <pholzing@redhat.com> Fixes: 3c6ae625101a ("conf, tcp, udp: Allow address specification for forwarded ports") Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp, udp: Fix partial success return codes in {tcp,udp}_sock_init()Stefano Brivio2023-03-091-16/+14
| | | | | | | | | | | | The comments say we should return 0 on partial success, and an error code on complete failure. Rationale: if the user configures a port forwarding, and we succeed to bind that port for IPv4 or IPv6 only, that might actually be what the user intended. Adjust the two functions to reflect the comments. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* tcp, udp, util: Pass socket creation errors all the way upStefano Brivio2023-03-091-9/+9
| | | | | | | | | ...starting from sock_l4(), pass negative error (errno) codes instead of -1. They will only be used in two commits from now, no functional changes intended here. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* treewide: Fix header includes to build with muslChris Kuhn2023-03-091-0/+1
| | | | | | | | | | | | | | | Roughly inspired from a patch by Chris Kuhn: fix up includes so that we can build against musl: glibc is more lenient as headers generally include a larger amount of other headers. Compared to the original patch, I only included what was needed directly in C files, instead of adding blanket includes in local header files. It's a bit more involved, but more consistent with the current (not ideal) situation. Reported-by: Chris Kuhn <kuhnchris+github@kuhnchris.eu> Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* conf, icmp, tcp, udp: Add options to bind to outbound address and interfaceStefano Brivio2023-03-091-17/+37
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I didn't notice earlier: libslirp (and slirp4netns) supports binding outbound sockets to specific IPv4 and IPv6 addresses, to force the source addresse selection. If we want to claim feature parity, we should implement that as well. Further, Podman supports specifying outbound interfaces as well, but this is simply done by resolving the primary address for an interface when the network back-end is started. However, since kernel version 5.7, commit c427bfec18f2 ("net: core: enable SO_BINDTODEVICE for non-root users"), we can actually bind to a specific interface name, which doesn't need to be validated in advance. Implement -o / --outbound ADDR to bind to IPv4 and IPv6 addresses, and --outbound-if4 and --outbound-if6 to bind IPv4 and IPv6 sockets to given interfaces. Given that it probably makes little sense to select addresses and routes from interfaces different than the ones given for outbound sockets, also assign those as "template" interfaces, by default, unless explicitly overridden by '-i'. For ICMP and UDP, we call sock_l4() to open outbound sockets, as we already needed to bind to given ports or echo identifiers, and we can bind() a socket only once: there, pass address (if any) and interface (if any) for the existing bind() and setsockopt() calls. For TCP, in general, we wouldn't otherwise bind sockets. Add a specific helper to do that. For UDP outbound sockets, we need to know if the final destination of the socket is a loopback address, before we decide whether it makes sense to bind the socket at all: move the block mangling the address destination before the creation of the socket in the IPv4 path. This was already the case for the IPv6 path. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* udp: Fix signedness warning on 32-bits architecturesStefano Brivio2023-03-091-1/+1
| | | | | | | | | | | | | | | | | | When a ssize_t is an int: udp.c: In function ‘udp_sock_handler’: udp.c:774:23: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘ssize_t’ {aka ‘int’} [-Wsign-compare] 774 | for (i = 0; i < n; i += m) { | ^ udp.c:781:43: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘ssize_t’ {aka ‘int’} [-Wsign-compare] 781 | for (m = 1; i + m < n; m++) { | Change 'i' and 'm' counters in udp_sock_handler() to signed versions, to match ssize_t n. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* conf, udp: Allow any loopback address to be used as resolverStefano Brivio2023-02-271-10/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Andrea reports that with a Fedora 37 guest running on a Fedora 37 host, both using systemd-resolved, with passt connecting them, running with default options, DNS queries don't work. systemd-resolved on the host is reachable only at the loopback address 127.0.0.53. We advertise the default gateway address to the guest as resolver, because our local address is of course unreachable from there, which means we see DNS queries directed to the default gateway, and we redirect them to 127.0.0.1. However, systemd-resolved doesn't answer on 127.0.0.1. To fix this, set @dns_match to the address of the default gateway, unless a different resolver address is explicitly configured, so that we know we explicitly have to map DNS queries, in this case, to the address of the local resolver. This means that in udp_tap_handler() we need to check, first, if the destination address of packets matches @dns_match: even if it's the address of the local gateway, we want to map that to a specific address, which isn't necessarily 127.0.0.1. Do the same for IPv6 for consistency, even though IPv6 defines a single loopback address. Reported-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Tested-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* udp: Actually use host resolver to forward DNS queriesStefano Brivio2023-02-271-2/+2
| | | | | | | | | | | | | | | Instead of the address of the first resolver we advertise to the guest or namespace. This was one of the intentions behind commit 3a2afde87dd1 ("conf, udp: Drop mostly duplicated dns_send arrays, rename related fields"), but I forgot to implement this part. In practice, they are usually the same thing, unless /etc/resolv.conf points to a loopback address. Fixes: 3a2afde87dd1 ("conf, udp: Drop mostly duplicated dns_send arrays, rename related fields") Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Tested-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* conf, tcp, udp: Exit if we fail to bind sockets for all given portsStefano Brivio2023-02-161-3/+13
| | | | | | | | | | | | | | passt supports ranges of forwarded ports as well as 'all' for TCP and UDP, so it might be convenient to proceed if we fail to bind only some of the desired ports. But if we fail to bind even a single port for a given specification, we're clearly, unexpectedly, conflicting with another network service. In that case, report failure and exit. Reported-by: Yalan Zhang <yalzhang@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* Make assertions actually usefulDavid Gibson2023-02-121-2/+1
| | | | | | | | | | | | | | | | | | There are some places in passt/pasta which #include <assert.h> and make various assertions. If we hit these something has already gone wrong, but they're there so that we a useful message instead of cryptic misbehaviour if assumptions we thought were correct turn out not to be. Except.. the glibc implementation of assert() uses syscalls that aren't in our seccomp filter, so we'll get a SIGSYS before it actually prints the message. Work around this by adding our own ASSERT() implementation using our existing err() function to log the message, and an abort(). The abort() probably also won't work exactly right with seccomp, but once we've printed the message, dying with a SIGSYS works just as well as dying with a SIGABRT. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Use tap_send_frames()David Gibson2023-01-231-134/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To send frames on the tap interface, the UDP uses a fairly complicated two level batching. First multiple frames are gathered into a single "message" for the qemu stream socket, then multiple messages are send with sendmmsg(). We now have tap_send_frames() which already deals with sending a number of frames, including batching and handling partial sends. Use that to considerably simplify things. This does make a couple of behavioural changes: * We used to split messages to keep them under 32kiB (except when a single frame was longer than that). The comments claim this is needed to stop qemu from closing the connection, but we don't have any equivalent logic for TCP. I wasn't able to reproduce the problem with this series, although it was apparently easy to reproduce earlier. My suspicion is that there was never an inherent need to keep messages small, however with larger messages (and default kernel buffer sizes) the chances of needing more than one resend for partial send()s is greatly increased. We used not to correctly handle that case of multiple resends, but now we do. * Previously when we got a partial send on UDP, we would resend the remainder of the entire "message", including multiple frames. The common code now only resends the remainder of a single frame, simply dropping any frames which weren't even partially sent. This is what TCP always did and is probably a better idea for UDP too. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Use abstracted tap headerDavid Gibson2023-01-231-55/+24
| | | | | | | | | Update the UDP code to use the tap layer abstractions for initializing and updating the L2 and lower headers. This will make adding other tap backends in future easier. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: Parameterize ethernet header initializer macroDavid Gibson2023-01-231-2/+2
| | | | | | | | | We have separate IPv4 and IPv6 versions of a macro to construct an initializer for ethernet headers. However, now that we have htons_constant it's easy to simply paramterize this with the ethernet protocol number. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp, udp: Use named field initializers in iov_init functionsDavid Gibson2023-01-231-9/+4
| | | | | | | | | | Both the TCP and UDP iov_init functions have some large structure literals defined in "field order" style. These are pretty hard to read since it's not obvious what value corresponds to what field. Use named field style initializers instead to make this clearer. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Don't use separate sockets to listen for spliced packetsDavid Gibson2023-01-131-40/+13
| | | | | | | | | | | | | | | Currently, when ports are forwarded inbound in pasta mode, we open two sockets for incoming traffic: one listens on the public IP address and will forward packets to the tuntap interface. The other listens on localhost and forwards via "splicing" (resending directly via sockets in the ns). Now that we've improved the logic about whether we "splice" any individual packet, we don't need this. Instead we can have a single socket bound to 0.0.0.0 or ::, marked as able to splice and udp_sock_handler() will deal with each packet as appropriate. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Decide whether to "splice" per datagram rather than per socketDavid Gibson2023-01-131-19/+33
| | | | | | | | | | | | | | | | | | | | | | | | 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> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Unify udp_sock_handler_splice() with udp_sock_handler()David Gibson2023-01-131-60/+34
| | | | | | | | | | | | | | These two functions now have a very similar structure, and their first part (calling recvmmsg()) is functionally identical. So, merge the two functions into one. This does have the side effect of meaning we no longer receive multiple packets at once for splice (we already didn't for tap). This does hurt throughput for small spliced packets, but improves it for large spliced packets and tap packets. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Pre-populate msg_names with local addressDavid Gibson2023-01-131-22/+18
| | | | | | | | | | | udp_splice_namebuf is now used only for spliced sending, and so it is only ever populated with the localhost address, either IPv4 or IPv6. So, replace the awkward initialization in udp_sock_handler_splice() with statically initialized versions for IPv4 and IPv6. We then just need to update the port number in udp_sock_handler_splice(). Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Don't handle tap receive batch size calculation within a #defineDavid Gibson2023-01-131-3/+6
| | | | | | | | | | | | | | | UDP_MAX_FRAMES gives the maximum number of datagrams we'll ever handle as a batch for sizing our buffers and control structures. The subtly different UDP_TAP_FRAMES gives the maximum number of datagrams we'll actually try to receive at once for tap packets in the current configuration. This depends on the mode, meaning that the macro has a non-obvious dependency on the usual 'c' context variable being available. We only use it in one place, so it makes more sense to open code this. Add an explanatory comment while we're there. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Split receive from preparation and send in udp_sock_handler()David Gibson2023-01-131-27/+52
| | | | | | | | | | | The receive part of udp_sock_handler() and udp_sock_handler_splice() is now almost identical. In preparation for merging that, split the receive part of udp_sock_handler() from the part preparing and sending the frames for sending on the tap interface. The latter goes into a new udp_tap_send() function. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Split sending to passt tap interface into separate functionDavid Gibson2023-01-131-58/+72
| | | | | | | | | | | | | | The last part of udp_sock_handler() does the actual sending of frames to the tap interface. For pasta that's just a call to udp_tap_send_pasta() but for passt, it's moderately complex and open coded. For symmetry, move the passt send path into its own function, udp_tap_send_passt(). This will make it easier to abstract the tap interface in future (e.g. when we want to add vhost-user). Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Move sending pasta tap frames to the end of udp_sock_handler()David Gibson2023-01-131-19/+42
| | | | | | | | | | | | | | udp_sock_handler() has a surprising difference in flow between pasta and passt mode: For pasta we send each frame to the tap interface as we prepare it. For passt, though, we prepare all the frames, then send them with a single sendmmsg(). Alter the pasta path to also prepare all the frames, then send them at the end. We already have a suitable data structure for the passt case. This will make it easier to abstract out the tap backend difference in future. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Factor out control structure management from udp_sock_fill_data_v[46]David Gibson2022-12-061-68/+50
| | | | | | | | | | | | | | | | | | | | The main purpose of udp_sock_fill_data_v[46]() is to construct the IP, UDP and other headers we'll need to forward data onto the tap interface. In addition they update the control structures (iovec and mmsghdr) we'll need to send the messages, and in the case of pasta actually sends it. This leads the control structure management and the send itself awkwardly split between udp_sock_fill_data_v[46]() and their caller udp_sock_handler(). In addition, this tail part of udp_sock_fill_datav[46] is essentially common between the IPv4 and IPv6 versions, apart from which control array we're working on. Clean this up by reducing these functions to just construct the headers and renaming them to udp_update_hdr[46]() accordingly. The control structure updates are now all in the caller, and common for IPv4 and IPv6. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Preadjust udp[46]_l2_iov_tap[].iov_base for pasta modeDavid Gibson2022-12-061-18/+18
| | | | | | | | | | | | | | | | | Currently, we always populate udp[46]_l2_iov_tap[].iov_base with the very start of the header buffers, including space for the qemu vnet_len tag suitable for passt mode. That's ok because we don't actually use these iovecs for pasta mode. However, we do know the mode in udp_sock[46]_iov_init() so adjust these to the beginning of the headers we'll actually need for the mode: including the vnet_len tag for passt, but excluding it for pasta. This allows a slightly nicer way to locate the right buffer to send in the pasta case, and will allow some additional cleanups later. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Better factor IPv4 and IPv6 paths in udp_sock_handler()David Gibson2022-12-061-22/+18
| | | | | | | | | | | | | | | Apart from which mh array they're operating on the recvmmsg() calls in udp_sock_handler() are identical between the IPv4 and IPv6 paths, as are some of the control structure updates. By using some local variables to refer to the IP version specific control arrays, make some more logic common between the IPv4 and IPv6 paths. As well as slightly reducing the code size, this makes it less likely that we'll accidentally use the IPv4 arrays in the IPv6 path or vice versa as we did in a recently fixed bug. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Fix incorrect use of IPv6 mh buffers in IPv4 pathDavid Gibson2022-12-061-4/+4
| | | | | | | | | | | | | | | | | | | | | | udp_sock_handler() incorrectly uses udp6_l2_mh_tap[] on the IPv4 path. In fact this is harmless because this assignment is redundant (the 0th entry msg_hdr will always point to the 0th iov entry for both IPv4 and IPv6 and won't change). There is also an incorrect usage of udp6_l2_mh_tap[] in udp_sock_fill_data_v4. This one can cause real problems, because we'll use stale iov_len values if we send multiple messages to the qemu socket. Most of the time that will be relatively harmless - we're likely to either drop UDP packets, or send duplicates. However, if the stale iov_len we use ends up referencing an uninitialized buffer we could desynchronize the qemu stream socket. Correct both these bugs. The UDP6 path appears to be correct, but it does have some comments that incorrectly reference the IPv4 versions, so fix those as well. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Correct splice forwarding when receiving from multiple sourcesDavid Gibson2022-12-061-5/+14
| | | | | | | | | | | | | | | | | | | | | | | | | udp_sock_handler_splice() reads a whole batch of datagrams at once with recvmmsg(). It then forwards them all via a single socket on the other side, based on the source port. However, it's entirely possible that the datagrams in the set have different source ports, and thus ought to be forwarded via different sockets on the destination side. In fact this situation arises with the iperf -P4 throughput tests in our own test suite. AFAICT we only get away with this because iperf3 is strictly one way and doesn't send reply packets which would be misdirected because of the incorrect source ports. Alter udp_sock_handler_splice() to split the packets it receives into batches with the same source address and send each batch with a separate sendmmsg(). For now we only look for already contiguous batches, which means that if there are multiple active flows interleaved this is likely to degenerate to batches of size 1. For now this is the simplest way to correct the behaviour and we can try to optimize later. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Split send half of udp_sock_handler_splice() from the receive halfDavid Gibson2022-12-061-23/+53
| | | | | | | | | Move the part of udp_sock_handler_splice() concerned with sending out the datagrams into a new udp_splice_sendfrom() helper. This will make later cleanups easier. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Unify buffers for tap and splice pathsDavid Gibson2022-12-061-40/+31
| | | | | | | | | | | | | | | | | | | | | | We maintain a set of buffers for UDP packets to be forwarded via the tap interface in udp[46]_l2_buf. We then have a separate set of buffers for packets to be "spliced" in udp_splice_buf[]. However, we only use one of these at a time, so we can share the buffer space. For the receiving splice packets we can not only re-use the data buffers but also the udp[46]_l2_iov_sock and udp[46]_l2_mh_sock control structures. For sending the splice packets we keep the same data buffers, but we need specific control structures. We create udp[46]_iov_splice - we can't reuse udp_l2_iov_sock[] because we need to write iov_len as we're writing spliced packets, but the tap path expects iov_len to remain the same (it only uses it for receive). Likewise we create udp[46]_mh_splice with the mmsghdr structures for sending spliced packets. As well as needing to reference different iovs, these need to all reference udp_splice_namebuf instead of individual msg_name fields for each slot. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Add helper to extract port from a sockaddr_in or sockaddr_in6David Gibson2022-12-061-12/+14
| | | | | | | | | | udp_sock_handler_splice() has a somewhat clunky if to extract the port from a socket address which could be either IPv4 or IPv6. Future changes are going to make this even more clunky, so introduce a helper function to do this extraction. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Make UDP_SPLICE_FRAMES and UDP_TAP_FRAMES_MEM the same thingDavid Gibson2022-12-061-28/+27
| | | | | | | | | | These two constants have the same value, and there's not a lot of reason they'd ever need to be different. Future changes will further integrate the spliced and "tap" paths so that these need to be the same. So, merge them into UDP_MAX_FRAMES. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Simplify udp_sock_handler_spliceDavid Gibson2022-12-061-32/+15
| | | | | | | | Previous cleanups mean that we can now rework some complex ifs in udp_sock_handler_splice() into a simpler set. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Update UDP "connection" timestamps in both directionsDavid Gibson2022-12-061-2/+17
| | | | | | | | | | | | | | | | | | | | | A UDP pseudo-connection between port A in the init namespace and port B in the pasta guest namespace involves two sockets: udp_splice_init[v6][B] and udp_splice_ns[v6][A]. The socket which originated this "connection" will be permanent but the other one will be closed on a timeout. When we get a packet from the originating socket, we update the timeout on the other socket, but we don't do the same when we get a reply packet from the other socket. However any activity on the "connection" probably indicates that it's still in use. Without this we could incorrectly time out a "connection" if it's using a protocol which involves a single initiating packet, but which then gets continuing replies from the target. Correct this by updating the timeout on both sockets for a packet in either direction. This also updates the timestamps for the permanent originating sockets which is unnecessary, but harmless. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Don't explicitly track originating socket for spliced "connections"David Gibson2022-12-061-61/+52
| | | | | | | | | | | | | | | | | | | | | | | | | | When we look up udp_splice_to_ns[][].orig_sock in udp_sock_handler_splice() we're finding the socket on which the originating packet for the "connection" was received on. However, we don't specifically need this socket to be the originating one - we just need one that's bound to the the source port of this reply packet in the init namespace. We can look this up in udp_splice_to_init[v6][src].target_sock, whose defining characteristic is exactly that. The same applies with init and ns swapped. In practice, of course, the port we locate this way will always be the originating port, since we couldn't have started this "connection" if it wasn't. Change this, and we no longer need the @orig_sock field at all. That leaves just @target_sock which we rename to simply @sock. The whole udp_splice_flow structure now more represents a single bound port than a "flow" per se, so rename and recomment it accordingly. Likewise the udp_splice_to_{ns,init} names are now misleading, since the ports in those maps are used in both directions. Rename them to udp_splice_{ns,init} indicating the location where the described socket is bound. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Re-use fixed bound sockets for packet forwarding when possibleDavid Gibson2022-12-061-9/+13
| | | | | | | | | | | | | | | | | | | | | | When we look up udp_splice_to_ns[v6][src].target_sock in udp_sock_handler_splice, all we really require of the socket is that it be bound to port src in the pasta guest namespace. Similarly for udp_splice_to_init but bound in the init namespace. Usually these sockets are created temporarily by udp_splice_connect() and cleaned up by udp_timer(). However, depending on the -u and -U options its possible we have a permanent socket bound to the relevant port created by udp_sock_init(). If such a socket exists, we could use it instead of creating a temporary one. In fact we *must* use it, because we'll fail trying to bind() a temporary one to the same port. So allow this, store permanently bound sockets into udp_splice_to_{ns,init} in udp_sock_init(). These won't get incorrectly removed by the timer because we don't put a corresponding entry in the udp_act[] structure which directs the timer what to clean up. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Don't create double sockets for -U portDavid Gibson2022-12-061-18/+14
| | | | | | | | | | | | | | | | | | For each IP version udp_socket() has 3 possible calls to sock_l4(). One is for the "non-spliced" bound socket in the init namespace, one for the "spliced" bound socket in the init namespace and one for the "spliced" bound socket in the pasta namespace. However when this is called to create a socket in the pasta namspeace there is a logic error which causes it to take the path for the init side spliced socket as well as the ns socket. This essentially tries to create two identical sockets on the ns side. Unsurprisingly the second bind() call fails according to strace. Correct this to only attempt to open one socket within the ns. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Split splice field in udp_epoll_ref into (mostly) independent bitsDavid Gibson2022-12-061-27/+26
| | | | | | | | | | | | | | | | | | | The @splice field in union udp_epoll_ref can have a number of values for different types of "spliced" packet flows. Split it into several single bit fields with more or less independent meanings. The new @splice field is just a boolean indicating whether the socket is associated with a spliced flow, making it identical to the @splice fiend in tcp_epoll_ref. The new bit @orig, indicates whether this is a socket which can originate new udp packet flows (created with -u or -U) or a socket created on the fly to handle reply socket. @ns indicates whether the socket lives in the init namespace or the pasta namespace. Making these bits more orthogonal to each other will simplify some future cleanups. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Remove the @bound field from union udp_epoll_refDavid Gibson2022-12-061-5/+3
| | | | | | | We set this field, but nothing ever checked it. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Don't connect "forward" sockets for spliced flowsDavid Gibson2022-12-061-50/+35
| | | | | | | | | | | | Currently we connect() the socket we use to forward spliced UDP flows. However, we now only ever use sendto() rather than send() on this socket so there's not actually any need to connect it. Don't do so. Rename a number of things that referred to "connect" or "conn" since that would now be misleading. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Always use sendto() rather than send() for forwarding spliced packetsDavid Gibson2022-12-061-33/+7
| | | | | | | | | | | | | | | udp_sock_handler_splice() has two different ways of sending out packets once it has determined the correct destination socket. For the originating sockets (which are not connected) it uses sendto() to specify a specific address. For the forward socket (which is connected) we use send(). However we know the correct destination address even for the forward socket we do also know the correct destination address. We can use this to use sendto() instead of send(), removing the need for two different paths and some staging data structures. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Separate tracking of inbound and outbound packet flowsDavid Gibson2022-12-061-57/+57
| | | | | | | | | | | | | | | | | | Each entry udp_splice_map[v6][N] keeps information about two essentially unrelated packet flows. @ns_conn_sock, @ns_conn_ts and @init_bound_sock track a packet flow from port N in the host init namespace to some other port in the pasta namespace (the one @ns_conn_sock is connected to). @init_conn_sock, @init_conn_ts and @ns_bound_sock track packet flow from port N in the pasta namespace to some other port in the host init namespace (the one @init_conn_sock is connected to). Split udp_splice_map[][] into two separate tables for the two directions. Each entry in each table is a 'struct udp_splice_flow' with @orig_sock (previously the bound socket), @target_sock (previously the connected socket) and @ts (the timeout for the target socket). Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Also bind() connected ports for "splice" forwardingDavid Gibson2022-12-061-52/+32
| | | | | | | | | | | | | | | | | | | | | | pasta handles "spliced" port forwarding by resending datagrams received on a bound socket in the init namespace to a connected socket in the guest namespace. This means there are actually three ports associated with each "connection". First there's the source and destination ports of the originating datagram. That's also the destination port of the forwarded datagram, but the source port of the forwarded datagram is the kernel allocated bound address of the connected socket. However, by bind()ing as well as connect()ing the forwarding socket we can choose the source port of the forwarded datagrams. By choosing it to match the original source port we remove that surprising third port number and no longer need to store port numbers in struct udp_splice_port. As a bonus this means that the recipient of the packets will see the original source port if they call getpeername(). This rarely matters, but it can't hurt. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* conf, udp: Drop mostly duplicated dns_send arrays, rename related fieldsStefano Brivio2022-11-161-9/+12
| | | | | | | | | | | | | | | | | | | | | Given that we use just the first valid DNS resolver address configured, or read from resolv.conf(5) on the host, to forward DNS queries to, in case --dns-forward is used, we don't need to duplicate dns[] to dns_send[]: - rename dns_send[] back to dns[]: those are the resolvers we advertise to the guest/container - for forwarding purposes, instead of dns[], use a single field (for each protocol version): dns_host - and rename dns_fwd to dns_match, so that it's clear this is the address we are matching DNS queries against, to decide if they need to be forwarded Suggested-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* tcp, udp: Don't initialise IPv6/IPv4 sockets if IPv4/IPv6 are not enabledStefano Brivio2022-11-101-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | If we disable a given IP version automatically (no corresponding default route on host) or administratively (--ipv4-only or --ipv6-only options), we don't initialise related buffers and services (DHCP for IPv4, NDP and DHCPv6 for IPv6). The "tap" handlers will also ignore packets with a disabled IP version. However, in commit 3c6ae625101a ("conf, tcp, udp: Allow address specification for forwarded ports") I happily changed socket initialisation functions to take AF_UNSPEC meaning "any enabled IP version", but I forgot to add checks back for the "enabled" part. Reported by Paul: on a host without default IPv6 route, but IPv6 enabled, connect, using IPv6, to a port handled by pasta, which tries to send data to a tap device without initialised buffers for that IP version and exits because the resulting write() fails. Simpler way to reproduce: pasta -6 and inbound IPv4 connection, or pasta -4 and inbound IPv6 connection. Reported-by: Paul Holzinger <pholzing@redhat.com> Fixes: 3c6ae625101a ("conf, tcp, udp: Allow address specification for forwarded ports") Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* udp: Check for answers to forwarded DNS queries before handling local redirectsStefano Brivio2022-11-041-11/+11
| | | | | | | | | | | | Now that we allow loopback DNS addresses to be used as targets for forwarding, we need to check if DNS answers come from those targets, before deciding to eventually remap traffic for local redirects. Otherwise, the source address won't match the one configured as forwarder, which means that the guest or the container will refuse those responses. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Use typing to reduce chances of IPv4 endianness errorsDavid Gibson2022-11-041-15/+15
| | | | | | | | | | | | | | | | | | | We recently corrected some errors handling the endianness of IPv4 addresses. These are very easy errors to make since although we mostly store them in network endianness, we sometimes need to manipulate them in host endianness. To reduce the chances of making such mistakes again, change to always using a (struct in_addr) instead of a bare in_addr_t or uint32_t to store network endian addresses. This makes it harder to accidentally do arithmetic or comparisons on such addresses as if they were host endian. We introduce a number of IN4_IS_ADDR_*() helpers to make it easier to directly work with struct in_addr values. This has the additional benefit of making the IPv4 and IPv6 paths more visually similar. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Use IPV4_IS_LOOPBACK more widelyDavid Gibson2022-11-041-1/+1
| | | | | | | | | | | | | This macro checks if an IPv4 address is in the loopback network (127.0.0.0/8). There are two places where we open code an identical check, use the macro instead. There are also a number of places we specifically exclude the loopback address (127.0.0.1), but we should actually be excluding anything in the loopback network. Change those sites to use the macro as well. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Fix port and address checks for DNS forwarderStefano Brivio2022-10-151-3/+3
| | | | | | | | | | | | | | | | | | | First off, as we swap endianness for source ports in udp_fill_data_v{4,6}(), we want host endianness, not network endianness. It doesn't actually matter if we use htons() or ntohs() here, but the current version is confusing. In the IPv4 path, when we remap DNS answers, we already swapped the endianness as needed for the source port: don't swap it again, otherwise we'll not map DNS answers for IPv4. In the IPv6 path, when we remap DNS answers, we want to check that they came from our upstream DNS server, not the one configured via --dns-forward (which doesn't even need to exist for this functionality to work). Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* conf, tcp, udp: Allow specification of interface to bind toStefano Brivio2022-10-151-17/+18
| | | | | | | | | | | | | | | | Since kernel version 5.7, commit c427bfec18f2 ("net: core: enable SO_BINDTODEVICE for non-root users"), we can bind sockets to interfaces, if they haven't been bound yet (as in bind()). Introduce an optional interface specification for forwarded ports, prefixed by %, that can be passed together with an address. Reported use case: running local services that use ports we want to have externally forwarded: https://github.com/containers/podman/issues/14425 Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>