Commit graph

200 commits

Author SHA1 Message Date
Stefano Brivio
da152331cf Move logging functions to a new file, log.c
Logging to file is going to add some further complexity that we don't
want to squeeze into util.c.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
2022-10-14 17:38:25 +02:00
David Gibson
1ae95f33cc cppcheck: Suppress NULL pointer warning in tcp_sock_consume()
Recent versions of cppcheck give a warning due to the NULL buffer passed
to recv() in tcp_sock_consume().  Since this apparently works, I assume
it's actually valid, but cppcheck doesn't know that recv() can take a NULL
buffer.  So, use a suppression to get rid of the error.

Also add an unmatchedSuppression suppression since only some cppcheck
versions complain about this.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:22:32 +02:00
David Gibson
e2b7d370d0 cppcheck: Work around false positive NULL pointer dereference error
Some versions of cppcheck could errneously report a NULL pointer deference
inside a sizeof().  This is now fixed in cppcheck upstream[0].  For systems
using an affected version, add a suppression to work around the bug.  Also
add an unmatchedSuppression suppression so the suppression itself doesn't
cause a warning if you *do* have a fixed cppcheck.

[0] https://github.com/danmar/cppcheck/pull/4471

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-09-29 12:22:13 +02:00
David Gibson
d5b80ccc72 Fix widespread off-by-one error dealing with port numbers
Port numbers (for both TCP and UDP) are 16-bit, and so fit exactly into a
'short'.  USHRT_MAX is therefore the maximum port number and this is widely
used in the code.  Unfortunately, a lot of those places don't actually
want the maximum port number (USHRT_MAX == 65535), they want the total
number of ports (65536).  This leads to a number of potentially nasty
consequences:

 * We have buffer overruns on the port_fwd::delta array if we try to use
   port 65535
 * We have similar potential overruns for the tcp_sock_* arrays
 * Interestingly udp_act had the correct size, but we can calculate it in
   a more direct manner
 * We have a logical overrun of the ports bitmap as well, although it will
   just use an unused bit in the last byte so isnt harmful
 * Many loops don't consider port 65535 (which does mitigate some but not
   all of the buffer overruns above)
 * In udp_invert_portmap() we incorrectly compute the reverse port
   translation for return packets

Correct all these by using a new NUM_PORTS defined explicitly for this
purpose.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-09-24 14:48:35 +02:00
David Gibson
3ede07aac9 Treat port numbers as unsigned
Port numbers are unsigned values, but we're storing them in (signed) int
variables in some places.  This isn't actually harmful, because int is
large enough to hold the entire range of ports.  However in places we don't
want to use an in_port_t (usually to avoid overflow on the last iteration
of a loop) it makes more conceptual sense to use an unsigned int. This will
also avoid some problems with later cleanups.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-09-24 14:48:35 +02:00
David Gibson
f5a31ee94c Don't use indirect remap functions for conf_ports()
Now that we've delayed initialization of the UDP specific "reverse" map
until udp_init(), the only difference between the various 'remap' functions
used in conf_ports() is which array they target.  So, simplify by open
coding the logic into conf_ports() with a pointer to the correct mapping
array.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-09-24 14:48:35 +02:00
David Gibson
163dc5f188 Consolidate port forwarding configuration into a common structure
The configuration for how to forward ports in and out of the guest/ns is
divided between several different variables.  For each connect direction
and protocol we have a mode in the udp/tcp context structure, a bitmap
of which ports to forward also in the context structure and an array of
deltas to apply if the outward facing and inward facing port numbers are
different.  This last is a separate global variable, rather than being in
the context structure, for no particular reason.  UDP also requires an
additional array which has the reverse mapping used for return packets.

Consolidate these into a re-used substructure in the context structure.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-09-24 14:48:35 +02:00
David Gibson
1128fa03fe Improve types and names for port forwarding configuration
enum conf_port_type is local to conf.c and is used to track the port
forwarding mode during configuration.  We don't keep it around in the
context structure, however the 'init_detect_ports' and 'ns_detect_ports'
fields in the context are based solely on this.  Rather than changing
encoding, just include the forwarding mode into the context structure.
Move the type definition to a new port_fwd.h, which is kind of trivial at
the moment but will have more stuff later.

While we're there, "conf_port_type" doesn't really convey that this enum is
describing how port forwarding is configured.  Rename it to port_fwd_mode.
The variables (now fields) of this type also have mildly confusing names
since it's not immediately obvious whether 'ns' and 'init' refer to the
source or destination of the packets.  Use "in" (host to guest / init to
ns) and "out" (guest to host / ns to init) instead.

This has the added bonus that we no longer have locals 'udp_init' and
'tcp_init' which shadow global functions.

In addition, add a typedef 'port_fwd_map' for a bitmap of each port number,
which is used in several places.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-09-24 14:48:35 +02:00
David Gibson
16f5586bb8 Make substructures for IPv4 and IPv6 specific context information
The context structure contains a batch of fields specific to IPv4 and to
IPv6 connectivity.  Split those out into a sub-structure.

This allows the conf_ip4() and conf_ip6() functions, which take the
entire context but touch very little of it, to be given more specific
parameters, making it clearer what it affects without stepping through the
code.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-07-30 22:14:07 +02:00
David Gibson
5e12d23acb Separate IPv4 and IPv6 configuration
After recent changes, conf_ip() now has essentially entirely disjoint paths
for IPv4 and IPv6 configuration.  So, it's cleaner to split them out into
different functions conf_ip4() and conf_ip6().

Splitting these out also lets us make the interface a bit nicer, having
them return success or failure directly, rather than manipulating c->v4
and c->v6 to indicate success/failure of the two versions.

Since these functions may also initialize the interface index for each
protocol, it turns out we can then drop c->v4 and c->v6 entirely, replacing
tests on those with tests on whether c->ifi4 or c->ifi6 is non-zero (since
a 0 interface index is never valid).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
[sbrivio: Whitespace fixes]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-07-30 22:12:50 +02:00
David Gibson
4b2e018d70 Allow different external interfaces for IPv4 and IPv6 connectivity
It's quite plausible for a host to have both IPv4 and IPv6 connectivity,
but only via different interfaces.  For example, this will happen in the
case that IPv6 connectivity is via a tunnel (e.g. 6in4 or 6rd).  It would
also happen in the case that IPv4 access is via a tunnel on an otherwise
IPv6 only local network, which is a setup that might become more common in
the post IPv4 address exhaustion world.

In turns out there's no real need for passt/pasta to get its IPv4 and IPv6
connectivity via the same interface, so we can handle this situation fairly
easily.  Change the core to allow eparate external interfaces for IPv4 and
IPv6.  We don't actually set these separately for now.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2022-07-30 21:50:41 +02:00
Stefano Brivio
deda03bfc2 tcp: Silence warning from gcc 11.3 with -Ofast
If the first packet_get() call doesn't assign len, the second one
will also return NULL, but gcc doesn't see this.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-06-08 11:08:29 +02:00
Stefano Brivio
d27cc3e435 tcp: Work around gcc 12 bogus warning in tcp_rtt_dst_check()
gcc 12.1.x (e.g. current OpenSUSE Tumbleweed, x86_64 only,
gcc-12-1.4.x86_64) reports:

tcp.c: In function ‘tcp_send_flag’:
tcp.c:1014:9: warning: writing 16 bytes into a region of size 0 [-Wstringop-overflow=]
 1014 |         memcpy(low_rtt_dst + hole++, &conn->a.a6, sizeof(conn->a.a6));
      |         ^
tcp.c:559:24: note: at offset -16 into destination object ‘low_rtt_dst’ of size 128
  559 | static struct in6_addr low_rtt_dst[LOW_RTT_TABLE_SIZE];
      |

but 'hole' can't be -1, because the low_rtt_dst table is guaranteed
to have a hole: if we happened to write to the last entry, we'll go
back to index 0 and clear that one.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-20 10:36:11 +02:00
Stefano Brivio
3c6ae62510 conf, tcp, udp: Allow address specification for forwarded ports
This feature is available in slirp4netns but was missing in passt and
pasta.

Given that we don't do dynamic memory allocation, we need to bind
sockets while parsing port configuration. This means we need to
process all other options first, as they might affect addressing and
IP version support. It also implies a minor rework of how TCP and UDP
implementations bind sockets.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-05-01 07:19:05 +02:00
Stefano Brivio
5ab2e12f98 tcp: False "Out-of-bounds read" positive, CWE-125
Reported by Coverity: it doesn't see that tcp{4,6}_l2_buf_used are
set to zero by tcp_l2_data_buf_flush(), repeat that explicitly here.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
2a3b8dad33 tcp, tcp_splice: False "Negative array index read" positives, CWE-129
A flag or event bit is always set by callers. Reported by Coverity.

Signed-by-off: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
71a00f1449 tcp: Dereference null return value, CWE-476
Not an issue with a sane kernel behaviour. Reported by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
22ed4467a4 treewide: Unchecked return value from library, CWE-252
All instances were harmless, but it might be useful to have some
debug messages here and there. Reported by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
6a3f6df865 tcp: False "Untrusted loop bound" positive, CWE-606
Field doff in struct tcp_hdr is 4 bits wide, so optlen in
tcp_tap_handler() is already bound, but make that explicit.
Reported by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-05 18:47:07 +02:00
Stefano Brivio
dbd0a7035c treewide: Invalid type in argument to printf format specifier, CWE-686
Harmless except for two bad debugging prints.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-05 18:47:04 +02:00
Stefano Brivio
37c228ada8 tap, tcp, udp, icmp: Cut down on some oversized buffers
The existing sizes provide no measurable differences in throughput
and packet rates at this point. They were probably needed as batched
implementations were not complete, but they can be decreased quite a
bit now.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
f643c69806 tcp: Fix warning by gcc 5.4 on ppc64le about comparison in CONN_OR_NULL()
...we don't really need two extra bits, but it's easier to organise
things differently than to silence this.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
48582bf47f treewide: Mark constant references as const
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
bb70811183 treewide: Packet abstraction with mandatory boundary checks
Implement a packet abstraction providing boundary and size checks
based on packet descriptors: packets stored in a buffer can be queued
into a pool (without storage of its own), and data can be retrieved
referring to an index in the pool, specifying offset and length.

Checks ensure data is not read outside the boundaries of buffer and
descriptors, and that packets added to a pool are within the buffer
range with valid offset and indices.

This implies a wider rework: usage of the "queueing" part of the
abstraction mostly affects tap_handler_{passt,pasta}() functions and
their callees, while the "fetching" part affects all the guest or tap
facing implementations: TCP, UDP, ICMP, ARP, NDP, DHCP and DHCPv6
handlers.

Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
415ccf6116 tcp, tcp_splice: Use less awkward syntax to swap in/out sockets from pools
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
54d9df3903 tcp: Fit struct tcp_conn into a single 64-byte cacheline
...by:

- storing the chained-hash next connection pointer as numeric
  reference rather than as pointer

- storing the MSS as 14-bit value, and rounding it

- using only the effective amount of bits needed to store the hash
  bucket number

- explicitly limiting window scaling factors to 4-bit values
  (maximum factor is 14, from RFC 7323)

- scaling SO_SNDBUF values, and using a 8-bit representation for
  the duplicate ACK sequence

- keeping window values unscaled, as received and sent

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
92074c16a8 tcp_splice: Close sockets right away on high number of open files
We can't take for granted that the hard limit for open files is
big enough as to allow to delay closing sockets to a timer.

Store the value of RTLIMIT_NOFILE we set at start, and use it to
understand if we're approaching the limit with pending, spliced
TCP connections. If that's the case, close sockets right away as
soon as they're not needed, instead of deferring this task to a
timer.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
be5bbb9b06 tcp: Rework timers to use timerfd instead of periodic bitmap scan
With a lot of concurrent connections, the bitmap scan approach is
not really sustainable.

Switch to per-connection timerfd timers, set based on events and on
two new flags, ACK_FROM_TAP_DUE and ACK_TO_TAP_DUE. Timers are added
to the common epoll list, and implement the existing timeouts.

While at it, drop the CONN_ prefix from flag names, otherwise they
get quite long, and fix the logic to decide if a connection has a
local, possibly unreachable endpoint: we shouldn't go through the
rest of tcp_conn_from_tap() if we reset the connection due to a
successful bind(2), and we'll get EACCES if the port number is low.

Suggested by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
3eb19cfd8a tcp, udp, util: Enforce 24-bit limit on socket numbers
This should never happen, but there are no formal guarantees: ensure
socket numbers are below SOCKET_MAX.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
5ca555cf78 dhcpv6, tap, tcp: Use IN6_ARE_ADDR_EQUAL instead of open-coded memcmp()
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-28 17:11:40 +02:00
Stefano Brivio
e5eefe7743 tcp: Refactor to use events instead of states, split out spliced implementation
Using events and flags instead of states makes the implementation
much more straightforward: actions are mostly centered on events
that occurred on the connection rather than states.

An example is given by the ESTABLISHED_SOCK_FIN_SENT and
FIN_WAIT_1_SOCK_FIN abominations: we don't actually care about
which side started closing the connection to handle closing of
connection halves.

Split out the spliced implementation, as it has very little in
common with the "regular" TCP path.

Refactor things here and there to improve clarity. Add helpers
to trace where resets and flag settings come from.

No functional changes intended.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-28 17:11:40 +02:00
Stefano Brivio
04fd94ab07 seccomp, tcp: Add fcntl64 to pasta syscalls for armv6l, armv7l
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-02-28 04:23:31 +01:00
Stefano Brivio
6c93111864 tcp, udp: Receive batching doesn't pay off when writing single frames to tap
In pasta mode, when we get data from sockets and write it as single
frames to the tap device, we batch receive operations considerably,
and then (conceptually) split the data in many smaller writes.

It looked like an obvious choice, but performance is actually better
if we receive data in many small frame-sized recvmsg()/recvmmsg().

The syscall overhead with the previous behaviour, observed by perf,
comes predominantly from write operations, but receiving data in
shorter chunks probably improves cache locality by a considerable
amount.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-02-21 13:41:13 +01:00
Stefano Brivio
0515adceaa passt, pasta: Namespace-based sandboxing, defer seccomp policy application
To reach (at least) a conceptually equivalent security level as
implemented by --enable-sandbox in slirp4netns, we need to create a
new mount namespace and pivot_root() into a new (empty) mountpoint, so
that passt and pasta can't access any filesystem resource after
initialisation.

While at it, also detach IPC, PID (only for passt, to prevent
vulnerabilities based on the knowledge of a target PID), and UTS
namespaces.

With this approach, if we apply the seccomp filters right after the
configuration step, the number of allowed syscalls grows further. To
prevent this, defer the application of seccomp policies after the
initialisation phase, before the main loop, that's where we expect bad
things to happen, potentially. This way, we get back to 22 allowed
syscalls for passt and 34 for pasta, on x86_64.

While at it, move #syscalls notes to specific code paths wherever it
conceptually makes sense.

We have to open all the file handles we'll ever need before
sandboxing:

- the packet capture file can only be opened once, drop instance
  numbers from the default path and use the (pre-sandbox) PID instead

- /proc/net/tcp{,v6} and /proc/net/udp{,v6}, for automatic detection
  of bound ports in pasta mode, are now opened only once, before
  sandboxing, and their handles are stored in the execution context

- the UNIX domain socket for passt is also bound only once, before
  sandboxing: to reject clients after the first one, instead of
  closing the listening socket, keep it open, accept and immediately
  discard new connection if we already have a valid one

Clarify the (unchanged) behaviour for --netns-only in the man page.

To actually make passt and pasta processes run in a separate PID
namespace, we need to unshare(CLONE_NEWPID) before forking to
background (if configured to do so). Introduce a small daemon()
implementation, __daemon(), that additionally saves the PID file
before forking. While running in foreground, the process itself can't
move to a new PID namespace (a process can't change the notion of its
own PID): mention that in the man page.

For some reason, fork() in a detached PID namespace causes SIGTERM
and SIGQUIT to be ignored, even if the handler is still reported as
SIG_DFL: add a signal handler that just exits.

We can now drop most of the pasta_child_handler() implementation,
that took care of terminating all processes running in the same
namespace, if pasta started a shell: the shell itself is now the
init process in that namespace, and all children will terminate
once the init process exits.

Issuing 'echo $$' in a detached PID namespace won't return the
actual namespace PID as seen from the init namespace: adapt
demo and test setup scripts to reflect that.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-02-21 13:41:13 +01:00
Stefano Brivio
292c185553 passt: Address new clang-tidy warnings from LLVM 13.0.1
clang-tidy from LLVM 13.0.1 reports some new warnings from these
checkers:

- altera-unroll-loops, altera-id-dependent-backward-branch: ignore
  for the moment being, add a TODO item

- bugprone-easily-swappable-parameters: ignore, nothing to do about
  those

- readability-function-cognitive-complexity: ignore for the moment
  being, add a TODO item

- altera-struct-pack-align: ignore, alignment is forced in protocol
  headers

- concurrency-mt-unsafe: ignore for the moment being, add a TODO
  item

Fix bugprone-implicit-widening-of-multiplication-result warnings,
though, that's doable and they seem to make sense.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-30 02:59:12 +01:00
Stefano Brivio
34e6429235 passt, tap: Daemonise once socket is ready without waiting for connection
The existing behaviour is not really practical: an automated agent in
charge of starting both qemu and passt would need to fork itself to
start passt, because passt won't fork to background until qemu
connects, and the agent needs to unblock to start qemu.

Instead of waiting for a connection to daemonise, do it right away as
soon as a socket is available: that can be considered an initialised
state already.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-28 18:51:50 +01:00
Stefano Brivio
33b1bdd079 seccomp: Add a number of alternate and per-arch syscalls
Depending on the C library, but not necessarily in all the
functions we use, statx() might be used instead of stat(),
getdents() instead of getdents64(), readlinkat() instead of
readlink(), openat() instead of open().

On aarch64, it's clone() and not fork(), and dup3() instead of
dup2() -- just allow the existing alternative instead of dealing
with per-arch selections.

Since glibc commit 9a7565403758 ("posix: Consolidate fork
implementation"), we need to allow set_robust_list() for
fork()/clone(), even in a single-threaded context.

On some architectures, epoll_pwait() is provided instead of
epoll_wait(), but never both. Same with newfstat() and
fstat(), sigreturn() and rt_sigreturn(), getdents64() and
getdents(), readlink() and readlinkat(), unlink() and
unlinkat(), whereas pipe() might not be available, but
pipe2() always is, exclusively or not.

Seen on Fedora 34: newfstatat() is used on top of fstat().

syslog() is an actual system call on some glibc/arch combinations,
instead of a connect()/send() implementation.

On ppc64 and ppc64le, _llseek(), recv(), send() and getuid()
are used. For ppc64 only: ugetrlimit() for the getrlimit()
implementation, plus sigreturn() and fcntl64().

On s390x, additionally, we need to allow socketcall() (on top
of socket()), and sigreturn() also for passt (not just for
pasta).

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-26 16:30:59 +01:00
Stefano Brivio
be265eef06 tcp: Don't round down MSS to >= 64KiB page size, but clamp it in any case
On some architectures, the page size is bigger than the maximum size
of an Ethernet frame.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-26 16:30:59 +01:00
Stefano Brivio
caa22aa644 tcp, udp, util: Fixes for bitmap handling on big-endian, casts
Bitmap manipulating functions would otherwise refer to inconsistent
sets of bits on big-endian architectures. While at it, fix up a
couple of casts.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-26 16:30:59 +01:00
Stefano Brivio
1776de0140 tcp, netlink, HAS{BYTES_ACKED,MIN_RTT,GETRANDOM} and NETLINK_GET_STRICT_CHK
tcpi_bytes_acked and tcpi_min_rtt are only available on recent
kernel versions: provide fall-back paths (incurring some grade of
performance penalty).

Support for getrandom() was introduced in Linux 3.17 and glibc 2.25:
provide an alternate mechanism for that as well, reading from
/dev/random.

Also check if NETLINK_GET_STRICT_CHK is defined before using it:
it's not strictly needed, we'll filter out irrelevant results from
netlink anyway.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-26 16:30:59 +01:00
Stefano Brivio
b93c2c1713 passt: Drop <linux/ipv6.h> include, carry own ipv6hdr and opt_hdr definitions
This is the only remaining Linux-specific include -- drop it to avoid
clang-tidy warnings and to make code more portable.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-26 07:57:09 +01:00
Stefano Brivio
f6d9787d30 tap, tcp: Fix two comparisons with different signedness reported by gcc 7
For some reason, those are not reported by recent versions of gcc.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-26 07:57:09 +01:00
Stefano Brivio
6040f16239 tcp: Cover all usages of tcpi_snd_wnd with HAS_SND_WND
...I forgot two of them.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-26 07:57:09 +01:00
Stefano Brivio
a2b86c5f90 tcp: Restore source address to network endianness before using it for hash table
This was actually fine "on the wire", but it's inconsistent with the
way we hash other addresses/protocols and also ends up with a wrong
endianness in captures in case we replace the address with our
default gateway.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-21 17:54:03 +02:00
Stefano Brivio
627e18fa8a passt: Add cppcheck target, test, and address resulting warnings
...mostly false positives, but a number of very relevant ones too,
in tcp_get_sndbuf(), tcp_conn_from_tap(), and siphash PREAMBLE().

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-21 09:41:13 +02:00
Stefano Brivio
dd942eaa48 passt: Fix build with gcc 7, use std=c99, enable some more Clang checkers
Unions and structs, you all have names now.

Take the chance to enable bugprone-reserved-identifier,
cert-dcl37-c, and cert-dcl51-cpp checkers in clang-tidy.

Provide a ffsl() weak declaration using gcc built-in.

Start reordering includes, but that's not enough for the
llvm-include-order checker yet.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-21 04:26:08 +02:00
Stefano Brivio
849308d207 Makefile, tcp: Don't try to use tcpi_snd_wnd from tcp_info on pre-5.3 kernels
Detect missing tcpi_snd_wnd in struct tcp_info at build time,
otherwise build fails with a pre-5.3 linux/tcp.h header.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-21 01:19:27 +02:00
Stefano Brivio
9618d24700 ndp, dhcpv6, tcp, udp: Always use link-local as source if gateway isn't
This shouldn't happen on any sane configuration, but I just met an
example of that: the default IPv6 gateway on the host is configured
with a global unicast address, we use that as source for RA, DHCPv6
replies, and the guest ignores it. Same later on if we talk TCP or
UDP and the guest has no idea where that address comes from.

Use our link-local address in case the gateway address is global.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-20 11:10:23 +02:00
Stefano Brivio
12cfa6444c passt: Add clang-tidy Makefile target and test, take care of warnings
Most are just about style and form, but a few were actually
serious mistakes (NDP-related).

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-20 08:34:22 +02:00
Stefano Brivio
b0b77118fe passt: Address warnings from Clang's scan-build
All false positives so far.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-20 08:29:30 +02:00
Stefano Brivio
1a563a0cbd passt: Address gcc 11 warnings
A mix of unchecked return values, a missing permission mask for
open(2) with O_CREAT, and some false positives from
-Wstringop-overflow and -Wmaybe-uninitialized.

Reported-by: Martin Hauke <mardnh@gmx.de>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-20 08:29:30 +02:00
Stefano Brivio
1bddcf3dd7 tcp: Fix for non-blocking splice() on older kernels
For some reason, on 4.19, splice() doesn't honour SOCK_NONBLOCK from
accept4() while reading from a TCP socket. Pass SPLICE_F_NONBLOCK
explicitly in all cases.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-19 09:19:50 +02:00
Stefano Brivio
9e065b1448 tcp: Fix ACK reporting on older kernels (no tcp.kernel_snd_wnd case)
If the window isn't updated on !c->tcp.kernel_snd_wnd, we still
have to send ACKs if the ACK sequence was updated, or if an error
occurred while querying TCP_INFO.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-19 09:13:53 +02:00
Stefano Brivio
1ac0d52820 tcp: Arm tcp_data_noack on insufficient window too, don't reset if ACK doesn't match
...and while at it, reverse the operands in the window equality
comparison to detect the need for fast re-transmit: it's easier
to read this way.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-16 16:58:16 +02:00
Stefano Brivio
6943d41d6c tcp: ...and so I got a socket called zero
I thought I'd get away with it, but no, after some clean-ups, I
finally got a socket with number 0. Fix up all the convenient,
yet botched assumptions.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-15 20:46:17 +02:00
Stefano Brivio
2f4f29c5a7 tcp: Bump TCP_TAP_FRAMES back to 256
With a batched sendmsg(), this is now beneficial.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-15 17:17:57 +02:00
Stefano Brivio
38fbfdbcb9 tcp: Get rid of iov with cached MSS, drop sendmmsg(), add deferred flush
Caching iov_len for messages from socket doesn't actually decrease
overhead by the tiniest bit, and added a lot of complexity. Drop
that.

Also drop the sendmmsg(), we don't need to send multiple messages
with TCP, as long as we make sure no messages with a length
descriptor are sent partially, qemu is fine with it.

Just like it's done for segments without data (flags), also defer
the sendmsg() for sending data segments, to improve batching.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-15 17:13:23 +02:00
Stefano Brivio
54e6513d17 tcp: Clamp MSS depending on IP version, properly derive buffer sizes
It makes no sense to include an IPv6 header in the calculation for
clamping MSS on IPv4.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-15 17:09:37 +02:00
Stefano Brivio
c61944a1f8 tcp: Explicitly align IP headers in tcp4_l2_{,flags}buf_t also in non-AVX2 build
Otherwise, tcp4_l2_flags_buf_t is not consistent with tcp4_l2_buf_t and
header fields get all mixed up in tcp_l2_buf_fill_headers().

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-14 13:20:34 +02:00
Stefano Brivio
f45891cf26 conf, tcp, udp: Add --no-map-gw to disable mapping gateway address to host
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-14 13:19:52 +02:00
Stefano Brivio
66d5930ec7 passt, pasta: Add seccomp support
List of allowed syscalls comes from comments in the form:
	#syscalls <list>

for syscalls needed both in passt and pasta mode, and:
	#syscalls:pasta <list>
	#syscalls:passt <list>

for syscalls specifically needed in pasta or passt mode only.

seccomp.sh builds a list of BPF statements from those comments,
prefixed by a binary search tree to keep lookup fast.

While at it, clean up a bit the Makefile using wildcards.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-14 13:15:46 +02:00
Stefano Brivio
c9d57fee7c tcp: Decrease pool size for pipes to 16
This should be a reasonable balance between quick connection
establishment and a fast start-up.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-14 13:15:12 +02:00
Stefano Brivio
675174d4ba conf, tap: Split netlink and pasta functions, allow interface configuration
Move netlink routines to their own file, and use netlink to configure
or fetch all the information we need, except for the TUNSETIFF ioctl.

Move pasta-specific functions to their own file as well, add
parameters and calls to configure the tap interface in the namespace.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-14 13:15:12 +02:00
Giuseppe Scrivano
9a175cc2ce pasta: Allow specifying paths and names of namespaces
Based on a patch from Giuseppe Scrivano, this adds the ability to:

- specify paths and names of target namespaces to join, instead of
  a PID, also for user namespaces, with --userns

- request to join or create a network namespace only, without
  entering or creating a user namespace, with --netns-only

- specify the base directory for netns mountpoints, with --nsrun-dir

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
[sbrivio: reworked logic to actually join the given namespaces when
 they're not created, implemented --netns-only and --nsrun-dir,
 updated pasta demo script and man page]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-07 04:05:15 +02:00
Stefano Brivio
8131fc9175 tcp: Check if timestamp is passed also while sending FIN to tap/guest
...it's probably possible that we might need to reset a connection
together with a FIN segment.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-05 23:21:40 +02:00
Stefano Brivio
ccbf13ed1b tcp: Drop EPOLLOUT for connections being established earlier
That's the first thing we have to do, before sending SYN, ACK:
if tcp_send_to_tap() fails, we'll get a lot of useless events
otherwise.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-05 21:22:59 +02:00
Stefano Brivio
16f4b983de passt: Shrink binary size by dropping static initialisers
...from 11MiB to 155KiB for 'make avx2', 95KiB with -Os and stripped.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-05 21:22:59 +02:00
Stefano Brivio
d565082f84 tcp: Simplify ACK-sending conditions in tcp_data_from_tap()
Now that we have a proper function checking when and how to send
ACKs and window updates, we don't need to duplicate this logic in
tcp_data_from_tap().

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-05 20:02:03 +02:00
Stefano Brivio
eda446ba54 tcp: Always probe SO_SNDBUF, second attempt
I fell for this already: the sending buffer might shrink later!

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-05 20:02:03 +02:00
Stefano Brivio
a4826ee04b tcp: Defer and coalesce all segments with no data (flags) to handler
...using pre-cooked buffers, just like we do with other segments.

While at it, remove some code duplication by having separate
functions for updating ACK sequence and window, and for filling in
buffer headers.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-05 20:02:03 +02:00
Stefano Brivio
371667fcfb tcp: Increase LOW_RTT_THRESHOLD to 10us
Sometimes we can get up to 6-7us minimum RTT for local connections too.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-05 20:02:03 +02:00
Stefano Brivio
78631ceb99 tcp: Reduce size of socket pools
A large pool helps marginally with CRR latency, but has detrimental
effects on TCP memory pressure.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-05 20:02:03 +02:00
Stefano Brivio
cf9976beac tcp: Increase TCP_TAP_FRAMES once more
With an increased sending buffer size for the AF_UNIX socket, we
can get slightly lower overhead.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-05 20:02:03 +02:00
Stefano Brivio
d4d61480b6 tcp, tap: Turn tcp_probe_mem() into sock_probe_mem(), use for AF_UNIX socket too
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-05 20:02:03 +02:00
Stefano Brivio
52054d8b37 tcp: Fix botched timeout comparison
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-04 22:21:21 +02:00
Stefano Brivio
98dfe1cdf4 tcp: Check pending ACK every two thirds of window, not every half
...to spare some syscalls. If it's not enough, the timer will take
care of it.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-04 22:21:21 +02:00
Stefano Brivio
ffaf1d09f2 tcp: Don't set ACK flag while merely updating window value
The receiver might take this as a duplicate ACK othewise.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-04 22:21:21 +02:00
Stefano Brivio
81128241d6 tcp: Set TCP_TAP_FRAMES back to 32
Now that we fixed the issue with small receiving buffers, we can
safely increase this back and get slightly lower syscall overhead.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-04 22:21:21 +02:00
Stefano Brivio
683043e200 tcp: Probe net.core.{r,w}mem_max, don't set SO_{RCV,SND}BUF if low
If net.core.rmem_max and net.core.wmem_max sysctls have low values,
we can get bigger buffers by not trying to set them high -- the
kernel would lock their values to what we get.

Try, instead, to get bigger buffers by queueing as much as possible,
and if maximum values in tcp_wmem and tcp_rmem are bigger than this,
that will work.

While at it, drop QUICKACK option for non-spliced sockets, I set
that earlier by mistake.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-04 22:20:43 +02:00
Stefano Brivio
e1a2e2780c tcp: Check if connection is local or low RTT was seen before using large MSS
If the connection is local or the RTT was comparable to the time it
takes to queue a batch of messages, we can safely use a large MSS
regardless of the sending buffer, but otherwise not.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-04 22:20:43 +02:00
Stefano Brivio
f6bff339a9 tcp: Adjust usage of sending buffer depending on its size
If we start with a very small sending buffer, we can make the kernel
expand it if we cause the congestion window to get bigger, but this
won't reliably happen if we use just half (other half is accounted
as overhead).

Scale usage depending on its own size, we might eventually get some
retransmissions because we can't queue messages the sender sends us
in-window, but it's better than keeping that small buffer forever.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-04 22:20:43 +02:00
Stefano Brivio
2408ddffa3 tcp: Derive MSS announced to guest/namespace from configured MTU if present
...and from the sending socket only if the MTU is not configured.

Otherwise, a connection to a host from a local guest, with a
non-loopback destination address, will get its MSS from the MTU of the
outbound interface with that address, which is unnecessary as we know
the guest can send us larger segments.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-29 16:46:58 +02:00
Stefano Brivio
9657b6ed05 conf, tcp: Periodic detection of bound ports for pasta port forwarding
Detecting bound ports at start-up time isn't terribly useful: do this
periodically instead, if configured.

This is only implemented for TCP at the moment, UDP is somewhat more
complicated: leave a TODO there.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-27 11:23:44 +02:00
Stefano Brivio
904b86ade7 tcp: Rework window handling, timers, add SO_RCVLOWAT and pools for sockets/pipes
This introduces a number of fundamental changes that would be quite
messy to split. Summary:

- advertised window scaling can be as big as we want, we just need
  to clamp window sizes to avoid exceeding the size of our "discard"
  buffer for unacknowledged data from socket

- add macros to compare sequence numbers

- force sending ACK to guest/tap on PSH segments, always in pasta
  mode, whenever we see an overlapping segment, or when we reach a
  given threshold compared to our window

- we don't actually use recvmmsg() here, fix comments and label

- introduce pools for pre-opened sockets and pipes, to decrease
  latency on new connections

- set receiving and sending buffer sizes to the maximum allowed,
  kernel will clamp and round appropriately

- defer clean-up of spliced and non-spliced connection to timer

- in tcp_send_to_tap(), there's no need anymore to keep a large
  buffer, shrink it down to what we actually need

- introduce SO_RCVLOWAT setting and activity tracking for spliced
  connections, to coalesce data moved by splice() calls as much as
  possible

- as we now have a compacted connection table, there's no need to
  keep sparse bitmaps tracking connection activity -- simply go
  through active connections with a loop in the timer handler

- always clamp the advertised window to half our sending buffer,
  too, to minimise retransmissions from the guest/tap

- set TCP_QUICKACK for originating socket in spliced connections,
  there's no need to delay them

- fix up timeout for unacknowledged data from socket

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-27 01:28:02 +02:00
Stefano Brivio
3c839bfc46 tcp: Drop TODO about sequence collision attacks
A random initial sequence number based on a secret has already been
there for a while.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-27 01:28:02 +02:00
Stefano Brivio
dd581730e5 tap: Completely de-serialise input message batches
Until now, messages would be passed to protocol handlers in a single
batch only if they happened to be dequeued in a row. Packets
interleaved between different connections would result in multiple
calls to the same protocol handler for a single connection.

Instead, keep track of incoming packet descriptors, arrange them in
sequences, and call protocol handlers only as we completely sorted
input messages in batches.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-27 01:28:02 +02:00
Stefano Brivio
522878e6bb tcp: Decrease TCP_TAP_FRAMES to 8
This significantly improves fairness in serving concurrent connections.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-27 01:28:02 +02:00
Stefano Brivio
e9961cecfc pasta, tcp: Update comment about spliced connection states
...we now have SPLICE_FIN_{FROM,TO,BOTH} too.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-27 01:28:02 +02:00
Stefano Brivio
9b6769d53b tcp: Don't reset connection from ESTABLISHED state on EPOLLHUP
That might just mean we shut down the socket -- but we still have to
go through the other states to ensure a orderly shutdown guest-side.

While at it, drop the EPOLLHUP check for unhandled states: we should
never hit that, but if we do, resetting the connection at that point
is probably the wrong thing to do.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-16 08:50:02 +02:00
Stefano Brivio
62bace390b pasta, tcp: Mask EPOLLIN and EPOLLRDHUP after sending FIN
Now that we dropped EPOLLET, we'll keep getting EPOLLRDHUP, and
possibly EPOLLIN, even if there's nothing to read anymore.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-16 08:50:02 +02:00
Stefano Brivio
492b58d64b pasta, tcp: Break splice() loop once we've written everything that was read
That's a guarantee that we don't need to retry writing.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-16 08:50:02 +02:00
Stefano Brivio
34dd4b28b0 pasta, tcp: Don't set SPLICE_FIN_BOTH state on EPOLLHUP
EPOLLHUP just means we shut down one side of the connection on
*one* socket: remember, we have two sockets here.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-16 08:50:02 +02:00
Stefano Brivio
e8540b3f26 pasta, tcp: Don't reset 'never_read' flag on write retries
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-16 08:50:02 +02:00
Stefano Brivio
7ecf693297 pasta, tcp: Don't set TCP_CORK on spliced sockets
...throughput isn't everything: this leads (of course) to horrible
latency with small, sparse messages. As a consequence, there's no
need to set TCP_NODELAY either.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-16 08:49:58 +02:00
Stefano Brivio
a7eb8bb2f6 tcp: Fix setting window from maximum ACK sequence in batch
If we're at the first message in a batch, it's safe to get the
window value from it, and there's no need to subtract anything for
a comparison on that's not even done -- we'll override it later in
any case if we find messages with a higher ACK sequence number.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-16 08:20:50 +02:00
Stefano Brivio
3be131280d pasta, tcp: Set pipe descriptor numbers to -1 after closing
...so that we don't try to close them again, even if harmless.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-16 08:19:39 +02:00
Stefano Brivio
d481578882 pasta, tcp: Drop EPOLLET for spliced, established connections
...tcp_handler_splice() doesn't guarantee we read all the available
data, the sending buffer might be full.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-16 08:17:18 +02:00
Stefano Brivio
45d9b0000e tcp: Read SO_SNDBUF unconditionally
Checking it only when the cached value is smaller than the current
window of the receiver is not enough: it might shrink further while
the receiver window is growing.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-16 08:14:01 +02:00
Stefano Brivio
474b8e6fb7 pasta: Clean up FIN connection flags once a connection is deleted
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-15 10:43:59 +02:00
Stefano Brivio
57d17292f9 pasta: Set spliced connection flag in epoll reference on compaction
...otherwise, we'll mix indices with non-spliced connections.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-09-15 10:41:31 +02:00