When we plan to use valgrind, we need to build passt a bit differently:
  * We need debug symbols so that valgrind can match problems it finds to
    meaningful locations
  * We need to allow additional syscalls in the seccomp filter, because
    valgrind's wrappers need them

Currently we also disable optimization (-O0).  This is unfortunate, because
it will make valgrind tests even slower than they'd otherwise be.  Worse,
it's possible that the asm behaviour without optimization might be
different enough that valgrind could miss a use of uninitialized variable
or other fault it would detect.

I suspect this was originally done because without it inlining could mean
that suppressions we use don't reliably match the places we want them to.
Alas, it turns out this is true even with -O0.  We've now implemented a
more robust workaround for this (explicit ((noinline)) attributes when
compiled with -DVALGRIND).  So, we can re-enable optimization for valgrind
builds, making them closer to regular builds.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

valgrind complains if we pass a NULL buffer to recv(), even if we use
MSG_TRUNC, in which case it's actually safe.  For a long time we've had
a valgrind suppression for this.  It singles out the recv() in
tcp_sock_consume(), the only place we use MSG_TRUNC.

However, tcp_sock_consume() only has a single caller, which makes it a
prime candidate for inlining.  If inlined, it won't appear on the stack and
valgrind won't match the correct suppression.

It appears that certain compiler versions (for example gcc-13.2.1 in
Fedora 39) will inline this function even with the -O0 we use for valgrind
builds.  This breaks the suppression leading to a spurious failure in the
tests.

There's not really any way to adjust the suppression itself without making
it overly broad (we don't want to match other recv() calls).  So, as a hack
explicitly prevent inlining of this function when we're making a valgrind
build.  To accomplish this add an explicit -DVALGRIND when making such a
build.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

clang-tidy from LLVM 17.0.3 (which is in Fedora 39) includes a new
"misc-include-cleaner" warning that tries to make sure that headers
*directly* provide the things that are used in the .c file.  That sounds
great in theory but is in practice unusable:

Quite a few common things in the standard library are ultimately provided
by OS-specific system headers, but for portability should be accessed via
closer-to-standardised library headers.  This will warn constantly about
such cases: e.g. it will want you to include <linux/limits.h> instead of
<limits.h> to get PATH_MAX.

So, suppress this warning globally in the Makefile.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

logmsg() takes printf like arguments, but because it's not a built in, the
compiler won't generate warnings if the format string and parameters don't
match.  Enable those by using the format attribute.

Strictly speaking this is a gcc extension, but I believe it is also
supported by some other common compilers.  We already use some other
attributes in various places.  For now, just use it and we can worry about
compilers that don't support it if it comes up.

This exposes some warnings from existing callers, both in gcc and in
clang-tidy:
 - Some are straight out bugs, which we correct
 - It's occasionally useful to invoke the logging functions with an empty
   string, which gcc objects to, so disable that specific warning in the
   Makefile
 - Strictly speaking the C standard requires that the parameter for a %p
   be a (void *), not some other pointer type.  That's only likely to cause
   problems in practice on weird architectures with different sized
   representations for pointers to different types.  Nonetheless add the
   casts to make it happy.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

We have several possible ways of communicating with other entities.  We use
sockets to communicate with the host and other network sites, but also in
a different context to communicate "spliced" channels to a namespace.  We
also use a tuntap device or a qemu socket to communicate with the namespace
or guest.

For the time being these are just defined implicitly by how we structure
things.  However, there are other communication channels we want to use in
future (e.g. virtio-user), and we want to allow more flexible forwarding
between those.  To accomplish that we're going to want a specific way of
referring to those channels.

Introduce the concept of a "passt/pasta interface" or "pif" representing a
specific channel to communicate network data.  Each pif is assumed to be
associated with a specific network namespace in the broad sense (that is
as a place where IP addresses have a consistent meaning - not the Linux
specific sense).  But there could be multiple pifs communicating with the
same namespace (e.g. the spliced and tap interfaces in pasta).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

The implementation of scanning /proc files to do automatic port forwarding
is a bit awkwardly split between procfs_scan_listen() in util.c,
get_bound_ports() and related functions in conf.c and the initial setup
code in conf().

Consolidate all of this into port_fwd.h, which already has some related
definitions, and a new port_fwd.c.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Recent enough cppcheck versions (at least as of cppcheck 2.12) give this
error processing conf.c:

conf.c:1179:1: information: ValueFlow analysis is limited in conf. Use --check-level=exhaustive if full analysis is wanted. [checkLevelNormal]

Adding --check-level=exhaustive doesn't seem to significantly increase the
cppcheck run time for us, so enable it when possible, suppressing that
warning.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

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>

We have several workarounds for a clang-tidy bug where the checker doesn't
recognize that a number of system calls write to - and therefore initialise
- a socket address.  We can't neatly use a suppression, because the bogus
warning shows up some time after the actual system call, when we access
a field of the socket address which clang-tidy erroneously thinks is
uninitialised.

Consolidate these workarounds into one place by using macros to implement
wrappers around affected system calls which add a memset() of the sockaddr
to silence clang-tidy.  This removes the need for the individual memset()
workarounds at the callers - and the somewhat longwinded explanatory
comments.

We can then use a #define to not include the hack in "real" builds, but
only consider it for clang-tidy.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

C11 has some features that will allow us to make some things a bit cleaner.
Alter the Makefile to tell the compiler to allow us to use C11 code.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>