aboutgitcodebugslistschat
path: root/tcp_splice.c
Commit message (Collapse)AuthorAgeFilesLines
* flow,tcp: Use epoll_ref type including flow and sideDavid Gibson2023-12-041-25/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | Currently TCP uses the 'flow' epoll_ref field for both connected sockets and timers, which consists of just the index of the relevant flow (connection). This is just fine for timers, for while it obviously works, it's subtly incomplete for sockets on spliced connections. In that case we want to know which side of the connection the event is occurring on as well as which connection. At present, we deduce that information by looking at the actual fd, and comparing it to the fds of the sockets on each side. When we use the flow table for more things, we expect more cases where something will need to know a specific side of a specific flow for an event, but nothing more. Therefore add a new 'flowside' epoll_ref field, with exactly that information. We use it for TCP connected sockets. This allows us to directly know the side for spliced connections. For "tap" connections, it's pretty meaningless, since the side is always the socket side. It still makes logical sense though, and it may become important for future flow table work. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp_splice: Use unsigned to represent sideDavid Gibson2023-12-041-3/+4
| | | | | | | | | | | | | | | Currently, we use 'int' values to represent the "side" of a connection, which must always be 0 or 1. This turns out to be dangerous. In some cases we're going to want to put the side into a 1-bit bitfield. However, if that bitfield has type 'int', when we copy it out to a regular 'int' variable, it will be sign-extended and so have values 0 and -1, instead of 0 and 1. To avoid this, always use unsigned variables for the side. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* flow,tcp: Generalise TCP epoll_ref to generic flowsDavid Gibson2023-12-041-2/+2
| | | | | | | | | | | | | | TCP uses three different epoll object types: one for connected sockets, one for timers and one for listening sockets. Listening sockets really need information that's specific to TCP, so need their own epoll_ref field. Timers and connected sockets, however, only need the connection (flow) they're associated with. As we expand the use of the flow table, we expect that to be true for more epoll fds. So, rename the "TCP" epoll_ref field to be a "flow" epoll_ref field that can be used both for TCP and for other future cases. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* flow, tcp: Add logging helpers for connection related messagesDavid Gibson2023-12-041-35/+26
| | | | | | | | | | | | | | | | | | | Most of the messages logged by the TCP code (be they errors, debug or trace messages) are related to a specific connection / flow. We're fairly consistent about prefixing these with the type of connection and the connection / flow index. However there are a few places where we put the index later in the message or omit it entirely. The template with the prefix is also a little bulky to carry around for every message, particularly for spliced connections. To help keep this consistent, introduce some helpers to log messages linked to a specific flow. It takes the flow as a parameter and adds a uniform prefix to each message. This makes things slightly neater now, but more importantly will help keep formatting consistent as we add more things to the flow table. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* flow: Make unified version of flow table compactionDavid Gibson2023-12-041-1/+1
| | | | | | | | | | | tcp_table_compact() will move entries in the connection/flow table to keep it compact when other entries are removed. The moved entries need not have the same type as the flow removed, so it needs to be able to handle moving any type of flow. Therefore, move it to flow.c rather than being purportedly TCP specific. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* flow, tcp: Consolidate flow pointer<->index helpersDavid Gibson2023-12-041-11/+10
| | | | | | | | | | | | | | | | | | | Both tcp.c and tcp_splice.c define CONN_IDX() variants to find the index of their connection structures in the connection table, now become the unified flow table. We can easily combine these into a common helper. While we're there, add some trickery for some additional type safety. They also define their own CONN() versions, which aren't so easily combined since they need to return different types, but we can have them use a common helper. In the process, we standardise on always using an unsigned type to store the connection / flow index, which makes more sense. tcp.c's conn_at_idx() remains for now, but we change its parameter to unsigned to match. That in turn means we can remove a check for negative values from it. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* flow, tcp: Move TCP connection table to unified flow tableDavid Gibson2023-12-041-9/+10
| | | | | | | | | | | | | We want to generalise "connection" tracking to things other than true TCP connections. Continue implenenting this by renaming the TCP connection table to the "flow table" and moving it to flow.c. The definitions are split between flow.h and flow_table.h - we need this separation to avoid circular dependencies: the definitions in flow.h will be needed by many headers using the flow mechanism, but flow_table.h needs all those protocol specific headers in order to define the full flow table entry. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* flow, tcp: Generalise connection typesDavid Gibson2023-12-041-1/+2
| | | | | | | | | | | | Currently TCP connections use a 1-bit selector, 'spliced', to determine the rest of the contents of the structure. We want to generalise the TCP connection table to other types of flows in other protocols. Make a start on this by replacing the tcp_conn_common structure with a new flow_common structure with an enum rather than a simple boolean indicating the type of flow. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* treewide: Use 'z' length modifier for size_t/ssize_t conversionsStefano Brivio2023-12-021-4/+4
| | | | | | | | Types size_t and ssize_t are not necessarily long, it depends on the architecture. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* tcp_splice: Simplify selection of socket and pipe sides in socket handlerDavid Gibson2023-11-071-59/+22
| | | | | | | | | | | | | tcp_splice_sock_handler() uses the tcp_splice_dir() helper to select which of the socket, pipe and counter fields to use depending on which side of the connection the socket event is coming from. Now that we are using arrays for the two sides, rather than separate named fields, we can instead just use a variable indicating the side and use that to index the arrays whever we need a particular side's field. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp_splice: Exploit side symmetry in tcp_splice_destroy()David Gibson2023-11-071-18/+14
| | | | | | | | | tcp_splice_destroy() has some close-to-duplicated logic handling closing of the socket and pipes for each side of the connection. We can use a loop across the sides to reduce the duplication. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp_splice: Exploit side symmetry in tcp_splice_connect_finish()David Gibson2023-11-071-40/+25
| | | | | | | | | tcp_splice_connect_finish() has two very similar blocks opening the two pipes for each direction of the connection. We can deduplicate this with a loop across the two sides. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp_splice: Exploit side symmetry in tcp_splice_timer()David Gibson2023-11-071-16/+11
| | | | | | | | | tcp_splice_timer() has two very similar blocks one after another that handle the SO_RCVLOWAT flags for the two sides of the connection. We can deduplicate this with a loop across the two sides. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp_splice: Rename sides of connection from a/b to 0/1David Gibson2023-11-071-113/+111
| | | | | | | | | | | | | Each spliced connection has two mostly, although not entirely, symmetric sides. We currently call those "a" and "b" and have different fields in the connection structure for each one. We can better exploit that symmetry if we use two element arrays rather thatn separately named fields. Do that in the places we can, and for the others change the "a"/"b" terminology to 0/1 to match. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp_splice: Don't pool pipes in pairsDavid Gibson2023-11-071-29/+31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To reduce latencies, the tcp splice code maintains a pool of pre-opened pipes to use for new connections. This is structured as an array of pairs of pipes, with each pipe, of course, being a pair of fds. Thus when we use the pool, a single pool "slot" provides both the a->b and b->a pipes. There's no strong reason to store the pool in pairs, though - we can with not much difficulty instead take the a->b and b->a pipes for a new connection independently from separate slots in the pool, or even take one from the the pool and create the other as we need it, if there's only one pipe left in the pool. This marginally increases the length of code, but simplifies the structure of the pipe pool. We should be able to re-shrink the code with later changes, too. In the process we also fix some minor bugs: - If we both failed to find a pipe in the pool and to create a new one, we didn't log an error and would silently drop the connection. That could make debugging such a situation difficult. Add in an error message for that case - When refilling the pool, if we were only able to open a single pipe in the pair, we attempted to rollback, but instead of closing the opened pipe, we instead closed the pipe we failed to open (probably leading to some ignored EBADFD errors). Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp_splice: Avoid awkward temporaries in tcp_splice_epoll_ctl()David Gibson2023-11-071-13/+11
| | | | | | | | | | | | | | | We initialise the events_a and events_b variables with tcp_splice_conn_epoll_events() function, then immediately copy the values into ev_a.events and ev_b.events. We can't simply pass &ev_[ab].events to tcp_splice_conn_epoll_events(), because struct epoll_event is packed, leading to 'pointer may be unaligned' warnings if we attempt that. We can, however, make tcp_splice_conn_epoll_events() take struct epoll_event pointers rather than raw u32 pointers, avoiding the awkward temporaries. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp_splice: Remove unnecessary forward declarationDavid Gibson2023-11-071-37/+34
| | | | | | | | | In tcp_splice.c we forward declare tcp_splice_epoll_ctl() then define it later on. However, there are no circular dependencies which prevent us from simply having the full definition in place of the forward declaration. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp_splice: Don't handle EPOLL_CTL_DEL as part of tcp_splice_epoll_ctl()David Gibson2023-11-071-8/+4
| | | | | | | | | | | | | | | | | | | | | | | | tcp_splice_epoll_ctl() removes both sockets from the epoll set if called when conn->flags & CLOSING. This will always happen immediately after setting that flag, since conn_flag_do() makes the call itself. That's also the _only_ time it can happen: we perform the EPOLL_CTL_DEL without clearing the conn->in_epoll flag, meaning that any further calls to tcp_splice_epoll_ctl() would attempt EPOLL_CTL_MOD, which would necessarily fail since the fds are no longer in the epoll. The EPOLL_CTL_DEL path in tcp_splice_epoll_ctl() has essentially zero overlap with anything else the function does, so just move them to be open coded in conn_flag_do(). This does require kernel 2.6.9 or later, in order to pass NULL as the event structure for epoll_ctl(). However, we already require at least 3.13 to allow unprivileged user namespaces. Given that, simply directly perform the EPOLL_CTL_DEL operations from conn_flag_do() rather than unnecessarily multiplexini Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp_splice: Correct error handling in tcp_splice_epoll_ctl()David Gibson2023-11-071-9/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we get an error from epoll_ctl() in tcp_splice_epoll_ctl() we goto the 'delete' path where we remove both sockets from the epoll set and return an error. There are several problems with this: - We 'return -errno' after the EPOLL_CTL_DEL operations, which means the deleting epoll_ctl() calls may have overwritten the errno values which actually triggered the failures. - The call from conn_flag_do() occurs when the CLOSING flag is set, in which case we go do the delete path regardless of error. In that case the 'return errno' is meaningless since we don't expect the EPOLL_CTL_DEL operations to fail and we ignore the return code anyway. - All other calls to tcp_splice_epoll_ctl() check the return code and if non-zero immediately call conn_flag(..., CLOSING) which will call tcp_splice_epoll_ctl() again explicitly to remove the sockets from epoll. That means removing them when the error first occurs is redundant. - We never specifically report an error on the epoll_ctl() operations. We just set the connection to CLOSING, more or less silently killing it. This could make debugging difficult in the unlikely even that we get a failure here. Re-organise tcp_splice_epoll_ctl() to just log a message then return in the error case, and only EPOLL_CTL_DEL when explicitly asked to with the CLOSING flag. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp_splice: Remove redundant tcp_splice_epoll_ctl()David Gibson2023-11-071-1/+0
| | | | | | | | | | | | | | | | tcp_splice_conn_update() calls tcp_splice_epoll_ctl() twice: first ignoring the return value, then checking it. This serves no purpose. If the first call succeeds, the second call will do exactly the same thing again, since nothing has changed in conn. If the first call fails, then tcp_splice_epoll_ctl() itself will EPOLL_CTL_DEL both fds, meaning when the second call tries to EPOLL_CTL_MOD them it will necessarily fail. It appears that this duplication was introduced by accident in an otherwise unrelated patch. Fixes: bb708111 ("treewide: Packet abstraction with mandatory boundary checks") Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* pif: Record originating pif in listening socket refsDavid Gibson2023-11-071-4/+6
| | | | | | | | | | For certain socket types, we record in the epoll ref whether they're sockets in the namespace, or on the host. We now have the notion of "pif" to indicate what "place" a socket is associated with, so generalise the simple one-bit 'ns' to a pif id. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* cppcheck: Make many pointers constDavid Gibson2023-10-041-2/+3
| | | | | | | | | Newer versions of cppcheck (as of 2.12.0, at least) added a warning for pointers which could be declared to point at const data, but aren't. Based on that, make many pointers throughout the codebase const. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* siphash: Use incremental rather than all-at-once siphash functionsDavid Gibson2023-09-301-0/+1
| | | | | | | | | | | | | | | | | | We have a bunch of variants of the siphash functions for different data sizes. The callers, in tcp.c, need to pack the various values they want to hash into a temporary structure, then call the appropriate version. We can avoid the copy into the temporary by directly using the incremental siphash functions. The length specific hash functions also have an undocumented constraint that the data pointer they take must, in fact, be aligned to avoid unaligned accesses, which may cause crashes on some architectures. So, prefer the incremental approach and remove the length-specific functions. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Avoid shadowing index(3)David Gibson2023-09-271-1/+1
| | | | | | | | | | | | | | | | | A classic gotcha of the standard C library is that its unwise to call any variable 'index' because it will shadow the standard string library function index(3). This can cause warnings from cppcheck amongst others, and it also means that if the variable is removed you tend to get confusing type errors (or sometimes nothing at all) instead of a nice simple "name is not defined" error. Strictly speaking this only occurs if <string.h> is included, but that is so common that as a rule it's best to just avoid it always. We have a number of places which hit this trap, so rename variables and parameters to avoid it. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Remove broken pressure calculations for tcp_defer_handler()David Gibson2023-08-221-2/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | tcp_defer_handler() performs a potentially expensive linear scan of the connection table. So, to mitigate the cost of that we skip if if we're not under at least moderate pressure: either 30% of available connections or 30% (estimated) of available fds used. But, the calculation for this has been broken since it was introduced: we calculate "max_conns" based on c->tcp.conn_count, not TCP_MAX_CONNS, meaning we only exit early if conn_count is less than 30% of itself, i.e. never. If that calculation is "corrected" to be based on TCP_MAX_CONNS, it completely tanks the TCP CRR times for passt - from ~60ms to >1000ms on my laptop. My guess is that this is because in the case of many short lived connections, we're letting the table become much fuller before compacting it. That means that other places which perform a table scan now have to do much, much more. For the time being, simply remove the tests, since they're not doing anything useful. We can reintroduce them more carefully if we see a need for them. This also removes the only user of c->tcp.splice_conn_count, so that can be removed as well. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Move in_epoll flag out of common connection structureDavid Gibson2023-08-221-2/+2
| | | | | | | | | | | | | | | | | | | The in_epoll boolean is one of only two fields (currently) in the common structure shared between tap and spliced connections. It seems like it belongs there, because both tap and spliced connections use it, and it has roughly the same meaning. Roughly, however, isn't exactly: which fds this flag says are in the epoll varies between the two connection types, and are in type specific fields. So, it's only possible to meaningfully use this value locally in type specific code anyway. This common field is going to get in the way of more widespread generalisation of connection / flow tracking, so move it to separate fields in the tap and splice specific structures. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* epoll: Split handling of listening TCP sockets into their own handlerDavid Gibson2023-08-131-2/+2
| | | | | | | | | | | | | | | | | tcp_sock_handler() handles both listening TCP sockets, and connected TCP sockets, but what it needs to do in those cases has essentially nothing in common. Therefore, give listening sockets their own epoll_type value and dispatch directly to their own handler from the top level. Furthermore, the two handlers need essentially entirely different information from the reference: we re-(ab)used the index field in the tcp_epoll_ref to indicate the port for the listening socket, but that's not the same meaning. So, switch listening sockets to their own reference type which we can lay out as we please. That lets us remove the listen and outbound fields from the normal (connected) tcp_epoll_ref, reducing it to just the connection table index. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* epoll: Generalize epoll_ref to cover things other than socketsDavid Gibson2023-08-131-2/+2
| | | | | | | | | | | | | | The epoll_ref type includes fields for the IP protocol of a socket, and the socket fd. However, we already have a few things in the epoll which aren't protocol sockets, and we may have more in future. Rename these fields to an abstract "fd type" and file descriptor for more generality. Similarly, rather than using existing IP protocol numbers for the type, introduce our own number space. For now these just correspond to the supported protocols, but we'll expand on that in future. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Use C11 anonymous members to make poll refs less verbose to useDavid Gibson2023-08-041-6/+5
| | | | | | | | | | | | union epoll_ref has a deeply nested set of structs and unions to let us subdivide it into the various different fields we want. This means that referencing elements can involve an awkward long string of intermediate fields. Using C11 anonymous structs and unions lets us do this less clumsily. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* passt: Relicense to GPL 2.0, or any later versionStefano Brivio2023-04-061-1/+1
| | | | | | | | | | | | | | | | | | | In practical terms, passt doesn't benefit from the additional protection offered by the AGPL over the GPL, because it's not suitable to be executed over a computer network. Further, restricting the distribution under the version 3 of the GPL wouldn't provide any practical advantage either, as long as the passt codebase is concerned, and might cause unnecessary compatibility dilemmas. Change licensing terms to the GNU General Public License Version 2, or any later version, with written permission from all current and past contributors, namely: myself, David Gibson, Laine Stump, Andrea Bolognani, Paul Holzinger, Richard W.M. Jones, Chris Kuhn, Florian Weimer, Giuseppe Scrivano, Stefan Hajnoczi, and Vasiliy Ulyanov. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* 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>
* 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>
* tcp, tcp_splice: Get rid of false positive CWE-394 Coverity warning from fls()Stefano Brivio2023-02-271-8/+16
| | | | | | | | | | | | | | We use the return value of fls() as array index for debug strings. While fls() can return -1 (if no bit is set), Coverity Scan doesn't see that we're first checking the return value of another fls() call with the same bitmask, before using it. Call fls() once, store its return value, check it, and use the stored value as array index. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* tcp: Improve handling of fallback if socket pool is empty on new spliceDavid Gibson2023-02-141-59/+30
| | | | | | | | | | | | | | | | | | | | | | | | When creating a new spliced connection, we need to get a socket in the other ns from the originating one. To avoid excessive ns switches we usually get these from a pool refilled on a timer. However, if the pool runs out we need a fallback. Currently that's done by passing -1 as the socket to tcp_splice_connnect() and running it in the target ns. This means that tcp_splice_connect() itself needs to have different cases depending on whether it's given an existing socket or not, which is a separate concern from what it's mostly doing. We change it to require a suitable open socket to be passed in, and ensuring in the caller that we have one. This requires adding the fallback paths to the caller, tcp_splice_new(). We use slightly different approaches for a socket in the init ns versus the guest ns. This also means that we no longer need to run tcp_splice_connect() itself in the guest ns, which allows us to remove a bunch of boilerplate code. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Split pool lookup from creating new sockets in tcp_conn_new_sock()David Gibson2023-02-141-6/+2
| | | | | | | | | | | | | | tcp_conn_new_sock() first looks for a socket in a pre-opened pool, then if that's empty creates a new socket in the init namespace. Both parts of this are duplicated in other places: the pool lookup logic is duplicated in tcp_splice_new(), and the socket opening logic is duplicated in tcp_sock_refill_pool(). Split the function into separate parts so we can remove both these duplications. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Move socket pool declarations aroundDavid Gibson2023-02-141-7/+43
| | | | | | | | | | | | | | | | | tcp_splice.c has some explicit extern declarations to access the socket pools. This is pretty dangerous - if we changed the type of these variables in tcp.c, we'd have tcp.c and tcp_splice.c using the same memory in different ways with no compiler error. So, move the extern declarations to tcp_conn.h so they're visible to both tcp.c and tcp_splice.c, but not the rest of pasta. In fact the pools for the guest namespace are necessarily only used by tcp_splice.c - we have no sockets on the guest side if we're not splicing. So move those declarations and the functions that deal exclusively with them to tcp_splice.c Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* 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>
* tcp: Pass union tcp_conn pointer to destroy and splice timer functionsStefano Brivio2022-11-251-6/+10
| | | | | | | | | | | | The pointers are actually the same, but we later pass the container union to tcp_table_compact(), which might zero the size of the whole union, and this confuses Coverity Scan. Given that we have pointers to the container union to start with, just pass those instead, all the way down to tcp_table_compact(). Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* tcp_splice: Allow splicing of connections from IPv4-mapped loopbackDavid Gibson2022-11-251-8/+11
| | | | | | | | | | | For non-spliced connections we now treat IPv4-mapped IPv6 addresses the same as the corresponding IPv4 addresses. However currently we won't splice a connection from ::ffff:127.0.0.1 the way we would one from 127.0.0.1. Correct this so that we can splice connections from IPv4 localhost that have been received on an IPv6 dual stack socket. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Remove v6 flag from tcp_epoll_refDavid Gibson2022-11-251-5/+3
| | | | | | | | | | | | | | | | | | | This bit in the TCP specific epoll reference indicates whether the connection is IPv6 or IPv4. However the sites which refer to it are already calling accept() which (optionally) returns an address for the remote end of the connection. We can use the sa_family field in that address to determine the connection type independent of the epoll reference. This does have a cost: for the spliced case, it means we now need to get that address from accept() which introduces an extran copy_to_user(). However, in future we want to allow handling IPv4 connectons through IPv6 sockets, which means we won't be able to determine the IP version at the time we create the listening socket and epoll reference. So, at some point we'll have to pay this cost anyway. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* inany: Helper functions for handling addresses which could be IPv4 or IPv6David Gibson2022-11-251-0/+1
| | | | | | | | | | | | | | struct tcp_conn stores an address which could be IPv6 or IPv4 using a union. We can do this without an additional tag by encoding IPv4 addresses as IPv4-mapped IPv6 addresses. This approach is useful wider than the specific place in tcp_conn, so expose a new 'union inany_addr' like this from a new inany.h. Along with that create a number of helper functions to make working with these "inany" addresses easier. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Remove splice from tcp_epoll_refDavid Gibson2022-11-251-16/+10
| | | | | | | | | | | | Currently the epoll reference for tcp sockets includes a bit indicating whether the socket maps to a spliced connection. However, the reference also has the index of the connection structure which also indicates whether it is spliced. We can therefore avoid the splice bit in the epoll_ref by unifying the first part of the non-spliced and spliced handlers where we look up the connection state. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Use the same sockets to listen for spliced and non-spliced connectionsDavid Gibson2022-11-251-4/+22
| | | | | | | | | | | | | | | | | | | | In pasta mode, tcp_sock_init[46]() create separate sockets to listen for spliced connections (these are bound to localhost) and non-spliced connections (these are bound to the host address). This introduces a subtle behavioural difference between pasta and passt: by default, pasta will listen only on a single host address, whereas passt will listen on all addresses (0.0.0.0 or ::). This also prevents us using some additional optimizations that only work with the unspecified (0.0.0.0 or ::) address. However, it turns out we don't need to do this. We can splice a connection if and only if it originates from the loopback address. Currently we ensure this by having the "spliced" listening sockets listening only on loopback. Instead, defer the decision about whether to splice a connection until after accept(), by checking if the connection was made from the loopback address. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Unify part of spliced and non-spliced conn_from_sock pathDavid Gibson2022-11-251-27/+28
| | | | | | | | | | | | | | In tcp_sock_handler() we split off to handle spliced sockets before checking anything else. However the first steps of the "new connection" path for each case are the same: allocate a connection entry and accept() the connection. Remove this duplication by making tcp_conn_from_sock() handle both spliced and non-spliced cases, with help from more specific tcp_tap_conn_from_sock and tcp_splice_conn_from_sock functions for the later stages which differ. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Unify the IN_EPOLL flagDavid Gibson2022-11-251-4/+4
| | | | | | | | | | | | There is very little common between the tcp_tap_conn and tcp_splice_conn structures. However, both do have an IN_EPOLL flag which has the same meaning in each case, though it's stored in a different location. Simplify things slightly by moving this bit into the common header of the two structures. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Partially unify tcp_timer() and tcp_splice_timer()David Gibson2022-11-251-32/+25
| | | | | | | | | | These two functions scan all the non-splced and spliced connections respectively and perform timed updates on them. Avoid scanning the now unified table twice, by having tcp_timer scan it once calling the relevant per-connection function for each one. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Unify tcp_defer_handler and tcp_splice_defer_handler()David Gibson2022-11-251-23/+1
| | | | | | | | | | | These two functions each step through non-spliced and spliced connections respectively and clean up entries for closed connections. To avoid scanning the connection table twice, we merge these into a single function which scans the unified table and performs the appropriate sort of cleanup action on each one. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Unify spliced and non-spliced connection tablesDavid Gibson2022-11-251-48/+18
| | | | | | | | | | | | | | | | | | | | | | | Currently spliced and non-spliced connections are stored in completely separate tables, so there are completely independent limits on the number of spliced and non-spliced connections. This is a bit counter-intuitive. More importantly, the fact that the tables are separate prevents us from unifying some other logic between the two cases. So, merge these two tables into one, using the 'c.spliced' common field to distinguish between them when necessary. For now we keep a common limit of 128k connections, whether they're spliced or non-spliced, which means we save memory overall. If necessary we could increase this to a 256k or higher total, which would cost memory but give some more flexibility. For now, the code paths which need to step through all extant connections are still separate for the two cases, just skipping over entries which aren't for them. We'll improve that in later patches. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Improved helpers to update connections after movingDavid Gibson2022-11-251-3/+14
| | | | | | | | | | When we compact the connection tables (both spliced and non-spliced) we need to move entries from one slot to another. That requires some updates in the entries themselves. Add helpers to make all the necessary updates for the spliced and non-spliced cases. This will simplify later cleanups. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Add connection union typeDavid Gibson2022-11-251-0/+2
| | | | | | | | | | | | | | | | Currently, the tables for spliced and non-spliced connections are entirely separate, with different types in different arrays. We want to unify them. As a first step, create a union type which can represent either a spliced or non-spliced connection. For them to be distinguishable, the individual types need to have a common header added, with a bit indicating which type this structure is. This comes at the cost of increasing the size of tcp_tap_conn to over one (64 byte) cacheline. This isn't ideal, but it makes things simpler for now and we'll re-optimize this later. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-05 14:05:32 +0000