aboutgitcodebugslistschat
Commit message (Collapse)AuthorAgeFilesLines
* contrib/selinux: Allow binding and connecting to all UDP and TCP portsStefano Brivio2023-03-091-12/+15
| | | | | | | | | | | | | | | | | | | | | Laine reports that with a simple: <portForward proto='tcp'> <range start='2022' to='22'/> </portForward> in libvirt's domain XML, passt won't start as it fails to bind arbitrary ports. That was actually the intention behind passt_port_t: the user or system administrator should have explicitly configured allowed ports on a given machine. But it's probably not realistic, so just allow any port to be bound and forwarded. Also fix up some missing operations on sockets. Reported-by: Laine Stump <laine@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Tested-by: Laine Stump <laine@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
* contrib/selinux: Let passt write to stdout and stderr when it startsStefano Brivio2023-03-091-0/+1
| | | | | | | | | | Otherwise, it's unusable as stand-alone tool, or in foreground mode, and it's also impossible to get output from --help or --version, because for SELinux it's just a daemon. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Tested-by: Laine Stump <laine@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
* contrib/selinux: Drop duplicate init_daemon_domain() ruleStefano Brivio2023-03-091-1/+0
| | | | | | Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Tested-by: Laine Stump <laine@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
* 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>
* Makefile: Fix SuperH 4 builds: it's AUDIT_ARCH_SH, not AUDIT_ARCH_SH4Stefano Brivio2023-03-091-0/+1
| | | | | | | sh4 builds currently fail because of this: https://buildd.debian.org/status/fetch.php?pkg=passt&arch=sh4&ver=0.0%7Egit20230216.4663ccc-1&stamp=1677091350&raw=0 Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Makefile, seccomp.sh: Fix cross-builds, adjust syscalls list to compilerStefano Brivio2023-03-092-3/+6
| | | | | | | | | | | | | Debian cross-building automatic checks: http://crossqa.debian.net/src/passt currently fail because we don't use the right target architecture and compiler while building the system call lists and resolving their numbers in seccomp.sh. Pass ARCH and CC to seccomp.sh and use them. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* util: Add own prototype for __clone2() on ia642023_02_27.c538ee8Stefano Brivio2023-02-271-0/+9
| | | | | | | | | | | | | ia64 needs to use __clone2() as clone() is not available, but glibc doesn't export the prototype. Take it from clone(2) to avoid an implicit declaration: util.c: In function ‘do_clone’: util.c:512:16: warning: implicit declaration of function ‘__clone2’ [-Wimplicit-function-declaration] 512 | return __clone2(fn, stack_area + stack_size / 2, stack_size / 2, | ^~~~~~~~ Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* contrib/apparmor: Split profile into abstractions, use themStefano Brivio2023-02-273-60/+89
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | One day, libvirt might actually support running passt to provide guest connectivity. Should libvirtd (or virtqemud) start passt, it will need to access socket and PID files in specific locations, and passt needs to accept SIGTERM in case QEMU fails to start after passt is already started. To make this more convenient, split the current profile into two abstractions, for passt and for pasta, so that external programmes can include the bits they need (and especially not include the pasta abstraction if they only need to start passt), plus whatever specific adaptation is needed. For stand-alone usage of passt and pasta, the 'passt' profile simply includes both abstractions, plus rules to create and access PID and capture files in default or reasonable ($HOME) locations. Tested on Debian with libvirt 9.0.0 together with a local fix to start passt as intended, namely libvirt commit c0efdbdb9f66 ("qemu_passt: Avoid double daemonizing passt"). This is an example of how the libvirtd profile (or virtqemud abstraction, or virtqemud profile) can use this: # support for passt network back-end /usr/bin/passt Cx -> passt, profile passt { /usr/bin/passt r, owner @{run}/user/[0-9]*/libvirt/qemu/run/passt/* rw, signal (receive) set=("term") peer=/usr/sbin/libvirtd, signal (receive) set=("term") peer=libvirtd, include if exists <abstractions/passt> } translated: - when executing /usr/bin/passt, switch to the subprofile "passt" (not the "discrete", i.e. stand-alone profile), described below. Scrub the environment (e.g. LD_PRELOAD is dropped) - in the "passt" subprofile: - allow reading the binary - allow read and write access to PID and socket files - make passt accept SIGTERM from /usr/sbin/libvirtd, and libvirtd peer names - include anything else that's needed by passt itself Suggested-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* qrap: Generate -netdev as JSONAndrea Bolognani2023-02-271-1/+5
| | | | | | | | | | While generating -device as JSON when JSON is in use is mandatory, because not doing so can often prevent the VM from starting up, using JSON for -netdev simply makes things a bit nicer. No reason not to do it, though. Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* qrap: Introduce machine-specific PCI address baseAndrea Bolognani2023-02-271-10/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | For pc machines, devices are placed directly on pci.0 with addresses like bus=pci.0,addr=0xa and in this case the existing code works correctly. For q35 machines, however, a separate PCI bus is created for each devices using a pcie-root-port, and the resulting addresses look like bus=pci.9,addr=0x0 In this case, we need to treat PCI addresses as decimal, not hexadecimal, both when parsing and generating them. This issue has gone unnoticed for a long time because it only shows up when enough PCI devices are present: for small numbers, decimal and hexadecimal overlap, masking the issue. Reported-by: Alona Paz <alkaplan@redhat.com> Fixes: 5307faa05997 ("qrap: Strip network devices from command line, set them up according to machine") Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* qrap: Drop args in JSON formatAndrea Bolognani2023-02-271-0/+5
| | | | | | | | | When JSON support was introduced, the drop_args array has not been updated accordingly. Fixes: b944ca185587 ("qrap: Support JSON syntax for -device") Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* qrap: Fix support for pc machinesAndrea Bolognani2023-02-271-1/+1
| | | | | | | | | | The JSON version of the template is incorrect, making qrap completely unusable when JSON arguments are used together with the pc machine type. Fixes: b944ca185587 ("qrap: Support JSON syntax for -device") Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* qrap: Fix limits for PCI addressesAndrea Bolognani2023-02-271-2/+2
| | | | | | | | | | | | The pci.0 bus on a pc machine has 32 slots. For q35 machines, we don't expect devices to be plugged into pcie.0 directly, so technically we could have a very large number of slots by adding many pcie-root-ports, but even in this scenario 32 is a reasonable number. Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* log, conf, tap: Define die() as err() plus exit(), drop cppcheck workaroundsStefano Brivio2023-02-274-18/+12
| | | | | | | | | | | If we define die() as a variadic macro, passing __VA_ARGS__ to err(), and calling exit() outside err() itself, we can drop the workarounds introduced in commit 36f0199f6ef4 ("conf, tap: Silence two false positive invalidFunctionArg from cppcheck"). 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>
* doc/demo: Fix and suppress ShellCheck warningsStefano Brivio2023-02-271-1/+3
| | | | | | | | | | | | ShellCheck reports (SC2034) that __qemu_arch is not used. Use it, and silence the resulting SC2086 warning as we want word splitting on options we pass with it. While at it, silence SC2317 warnings for commands in cleanup() that appear to be unreachable: cleanup() is only called as trap. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* Fix definitions of SOCKET_MAX, TCP_MAX_CONNSStefano Brivio2023-02-274-5/+7
| | | | | | | | ...and, given that I keep getting this wrong, add a convenience macro, MAX_FROM_BITS(). Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* tcp: Avoid (theoretical) resource leak (CWE-772) Coverity warningStefano Brivio2023-02-271-0/+3
| | | | | | | | | | If tcp_timer_ctl() gets a socket number greater than SOCKET_MAX (2 ^ 24), we return error but we don't close the socket. This is a rather formal issue given that, at least on Linux, socket numbers are monotonic and we're in general not allowed to open so many sockets. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* tcp: Avoid false (but convoluted) positive Coverity CWE-476 warningStefano Brivio2023-02-271-1/+1
| | | | | | | | | | | | | | | If there are no TCP options in the header, tcp_tap_handler() will pass the corresponding pointer, fetched via packet_get(), as NULL to tcp_conn_from_sock_finish(), which in turn indirectly calls tcp_opt_get(). If there are no options, tcp_opt_get() will stop right away because the option length is indicated as zero. However, if the logic is complicated enough to follow for static checkers, adding an explicit check against NULL in tcp_opt_get() is probably a good idea. 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-272-12/+24
| | | | | | | | | | | | | | 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>
* treewide: Disable gcc strict aliasing rules as needed, drop workaroundsStefano Brivio2023-02-274-33/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Recently, commit 4ddbcb9c0c55 ("tcp: Disable optimisations for tcp_hash()") worked around yet another issue we hit with gcc 12 and '-flto -O2': some stores affecting the input data to siphash_20b() were omitted altogether, and tcp_hash() wouldn't get the correct hash for incoming connections. Digging further into this revealed that, at least according to gcc's interpretation of C99 aliasing rules, passing pointers to functions with different types compared to the effective type of the object (for example, a uint8_t pointer to an anonymous struct, as it happens in tcp_hash()), doesn't guarantee that stores are not reordered across the function call. This means that, in general, our checksum and hash functions might not see parts of input data that was intended to be provided by callers. Not even switching from uint8_t to character types, which should be appropriate here, according to C99 (ISO/IEC 9899, TC3, draft N1256), section 6.5, "Expressions", paragraph 7: An object shall have its stored value accessed only by an lvalue expression that has one of the following types: [...] — a character type. does the trick. I guess this is also subject to interpretation: casting passed pointers to character types, and then using those as different types, might still violate (dubious) aliasing rules. Disable gcc strict aliasing rules for potentially affected functions, which, in turn, disables gcc's Type-Based Alias Analysis (TBAA) optimisations based on those function arguments. Drop the existing workarounds. Also the (seemingly?) bogus 'maybe-uninitialized' warning on the tcp_tap_handler() > tcp_hash() > siphash_20b() path goes away with -fno-strict-aliasing on siphash_20b(). Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* tcp: Suppress knownConditionTrueFalse cppcheck false positiveStefano Brivio2023-02-271-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | cppcheck 2.10 reports: tcp.c:1815:12: style: Condition 'wnd>prev_scaled' is always false [knownConditionTrueFalse] if ((wnd > prev_scaled && wnd * 99 / 100 < prev_scaled) || ^ tcp.c:1808:8: note: Assignment 'wnd=((1<<(16+8))<(wnd))?(1<<(16+8)):(wnd)', assigned value is less than 1 wnd = MIN(MAX_WINDOW, wnd); ^ tcp.c:1811:19: note: Assuming condition is false if (prev_scaled == wnd) ^ tcp.c:1815:12: note: Condition 'wnd>prev_scaled' is always false if ((wnd > prev_scaled && wnd * 99 / 100 < prev_scaled) || ^ but this is not actually the case: wnd is typically greater than 1, and might very well be greater than prev_scaled as well. I bisected this down to cppcheck commit b4d455df487c ("Fix 11349: FP negativeIndex for clamped array index (#4627)") and reported findings at https://github.com/danmar/cppcheck/pull/4627. Suppress the warning for the moment being. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* log: Send identifier string in log messages, openlog() won't work for usStefano Brivio2023-02-271-7/+5
| | | | | | | | | | | | | | | | | openlog() can be used to set "ident" and have all the log messages prefixed by it, but only if we call syslog() -- this is implemented by C libraries. We don't log messages with syslog(), though, as we have a custom implementation to ensure we don't need dynamic memory allocation. This means that it's perfectly useless to call openlog(), and that we have to prefix every message we log by the identifier on our own. Reported-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Tested-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* conf, udp: Allow any loopback address to be used as resolverStefano Brivio2023-02-272-10/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* conf: Split add_dns{4,6}() out of get_dns()Stefano Brivio2023-02-271-35/+51
| | | | | | | | | | | | The logic handling which resolvers we add, and whether to add them, is getting rather cramped in get_dns(): split it into separate functions. No functional changes intended. 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>
* tcp: Disable optimisations for tcp_hash()2023_02_22.4ddbcb9Stefano Brivio2023-02-221-0/+3
| | | | | | | | | | I'm not sure if we're breaking some aliasing rule here, but with gcc 12.2.1 on x86_64 and -flto, the siphash_20b() call in tcp_hash() doesn't see the connection address -- it gets all zeroes instead. Fix this temporarily by disabling optimisations for this tcp_hash(). Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* selinux/passt.te: Allow setting socket option on routing netlink socketStefano Brivio2023-02-211-1/+1
| | | | Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* selinux/passt.te: Allow /etc/resolv.conf symlinks to be followedStefano Brivio2023-02-211-0/+1
| | | | Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* selinux/passt.te: Allow setcap on the process itselfStefano Brivio2023-02-211-0/+1
| | | | | | | This is needed by the new functions in isolate.c, add the corresponding rule. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* selinux: Switch to a more reasonable model for PID and socket filesStefano Brivio2023-02-212-5/+7
| | | | | | | | Instead of restricting PID files to /var/run/passt.pid, which is a single file and unlikely to be used, use the user_tmp_t type which should cover any reasonable need. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* selinux: Define interfaces for libvirt and similar frameworksStefano Brivio2023-02-212-0/+27
| | | | | | | | | | Services running passt will commonly need to transition to its domain, terminate it, connect and write to its socket. The init_daemon_domain() macro now defines the default transition to the passt_t domain, using the passt_exec_t type. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* selinux/passt.if: Fix typo in passt_read_data interface definitionStefano Brivio2023-02-211-1/+1
| | | | | | | | | This is an example interface, currently unused, so it went undetected: m4 macros need a backtick at the beginning of a block instead of a single quote. Fixes: 1f4b7fa0d75d ("passt, pasta: Add examples of SELinux policy modules") Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* conf: Fix typo and logic in conf_ports() check for port binding2023_02_16.4663cccStefano Brivio2023-02-161-1/+7
| | | | | | | | | | | | | | Ouch, I accidentally pushed the previous change without running the tests: - we need to check, in conf_ports(), that udp_sock_init() managed to bind at least a port, not the opposite - for -T and -U, we have no way to know if we'll manage to bind the port later, so never report an error for those Fixes: 3d0de2c1d727 ("conf, tcp, udp: Exit if we fail to bind sockets for all given ports") Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* conf, tap: Silence two false positive invalidFunctionArg from cppcheckStefano Brivio2023-02-162-1/+8
| | | | | | | | The newly introduced die() calls exit(), but cppcheck doesn't see it and warns about possibly invalid arguments used after the check which triggers die(). Add return statements to silence the warnings. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Remove 'zero_len' goto from tcp_data_from_sockDavid Gibson2023-02-161-14/+12
| | | | | | | | | | This goto exists purely to move this exception case out of line. Although that does make the "normal" path a little clearer, it comes at the cost of not knowing how where control will flow after jumping to the zero_len label. The exceptional case isn't that long, so just put it inline. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Remove 'recvmsg' goto from tcp_data_from_sockDavid Gibson2023-02-161-6/+5
| | | | | | | This goto can be handled just as simply and more clearly with a do while. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tap: Eliminate goto from tap_handler()David Gibson2023-02-161-12/+7
| | | | | | | | The goto here really doesn't improve clarity or brevity at all. Use a clearer construct. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tap: Don't pcap frames that didn't get sentDavid Gibson2023-02-161-10/+20
| | | | | | | | | | | | | | In tap_send_frames() we send a number of frames to the tap device, then also write them to the pcap capture file (if configured). However the tap send can partially fail (short write()s or similar), meaning that some of the requested frames weren't actually sent, but we still write those frames to the capture file. We do give a debug message in this case, but it's misleading to add frames that we know weren't sent to the capture file. Rework to avoid this. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* passt.1: Fix typo, improve wording in examples of port forwarding specifiersStefano Brivio2023-02-161-12/+17
| | | | | | | | | | | | Based on a patch from Laine, and reports from Laine and Yalan: fix the "22-80:32-90" example, and improve wording for the other ones: instead of using "to" to denote the end of a range, use "between ... and", so that it's clear we're *not* referring to target ports. Reported-by: Laine Stump <laine@redhat.com> Reported-by: Yalan Zhang <yalzhang@redhat.com> Fixes: da20f57f19dc ("passt, qrap: Add man pages") Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* dhcp: Fix netmask calculation for option 1 from prefix lengthStefano Brivio2023-02-161-1/+1
| | | | | | | | | | | | | Similar to the conf_print() fix from commit 4129764ecaeb ("conf: Fix mask calculation from prefix_len in conf_print()"): to calculate an IPv4 netmask from the prefix length, we need to left shift 32 all-one bits by 32 minus the prefix length -- not by the prefix length itself. Reported-by: Yalan Zhang <yalzhang@redhat.com> Fixes: dd09cceaee21 ("Minor improvements to IPv4 netmask handling") Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* tap: Use single counter for iov elements in tap_send_frames_pasta()Stefano Brivio2023-02-161-3/+3
| | | | | | | | | | | | | David points out that using multiple counters to go over the iov array, namely 'i' and 'iov', makes mistakes easier. We can't just use 'iov', unless we reserve an element with zero iov_len at the end, which isn't really justified. Simply use 'i' to iterate over the array. Link: https://archives.passt.top/passt-dev/Y+mfenvLn3VJ7Dg5@yekko/ Signed-off-by: Stefano Brivio <sbrivio@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-165-27/+69
| | | | | | | | | | | | | | 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>
* log: Don't duplicate messages on stderr before daemonisingStefano Brivio2023-02-161-3/+4
| | | | | | | | | | | | | | | | | | | Now that logging functions force printing messages to stderr before passt forks to background, we'll have duplicate messages when running from an interactive terminal, or if --stderr is passed, because at some point we set LOG_PERROR in our __openlog() wrapper. We could defer setting LOG_PERROR, but that would change option semantics in other, unexpected ways. We could force calling passt_vsyslog() as long as the mask is set to LOG_EMERG, but that complicates the logic in logging functions even further. Go the easy way for now: don't force printing to stderr with LOG_EMERG if LOG_PERROR is already set. We should seriously consider a rework of those logging functions at this point. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* convert all remaining err() followed by exit() to die()Laine Stump2023-02-166-91/+47
| | | | | | | | This actually leaves us with 0 uses of err(), but someone could want to use it in the future, so we may as well leave it around. Signed-off-by: Laine Stump <laine@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* log a detailed error (not usage()) when there are extra non-option argumentsLaine Stump2023-02-161-1/+1
| | | | | Signed-off-by: Laine Stump <laine@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* make conf_netns_opt() exit immediately after logging errorLaine Stump2023-02-161-12/+4
| | | | | | | ...and return void to simplify the caller. Signed-off-by: Laine Stump <laine@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* make conf_ugid() exit immediately after logging errorLaine Stump2023-02-161-18/+9
| | | | | | | Again, it can then be made to return void, simplifying the caller. Signed-off-by: Laine Stump <laine@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* make conf_pasta_ns() exit immediately after logging errorLaine Stump2023-02-161-24/+11
| | | | | | | As with conf_ports, this allows us to make the function return void. Signed-off-by: Laine Stump <laine@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* make conf_ports() exit immediately after logging errorLaine Stump2023-02-161-24/+28
| | | | | | | | | | | | | | | | | | | | | Rather than having conf_ports() (possibly) log an error, and then letting the caller log the entire usage() message and exit, just log the errors and exit immediately (using die()). For some errors, conf_ports would previously not log any specific message, leaving it up to the user to determine the problem by guessing. We replace all of those silent returns with die() (logging a specific error), thus permitting us to make conf_ports() return void, which simplifies the caller. While modifying the two callers to conf_ports() to not check for a return value, we can further simplify the code by removing the check for a non-null optarg, as that is guaranteed to never happen (due to prior calls to getopt_long() with "argument required" for all relevant options - getopt_long() would have already caught this error). Signed-off-by: Laine Stump <laine@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* eliminate most calls to usage() in conf()Laine Stump2023-02-161-214/+122
| | | | | | | | | | | | | | | | | | | Nearly all of the calls to usage() in conf() occur immediately after logging a more detailed error message, and the fact that these errors are occuring indicates that the user has already seen the passt usage message (or read the manpage). Spamming the logfile with the complete contents of the usage message serves only to obscure the more detailed error message. The only time when the full usage message should be output is if the user explicitly asks for it with -h (or its synonyms) As a start to eliminating the excessive calls to usage(), this patch replaces most calls to err() followed by usage() with a call to die() instead. A few other usage() calls remain, but their removal involves bit more nuance that should be properly explained in separate commit messages. Signed-off-by: Laine Stump <laine@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-05 08:39:33 +0000