- dev - passt

[PATCH] conf: Report usage for --no-netns-quit
by Stefano Brivio 09 Oct '22

09 Oct '22

Fixes: 745a9ba4284c ("pasta: By default, quit if filesystem-bound net namespace goes away") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/conf.c b/conf.c index 5a7f9b9..4ec1590 100644 --- a/conf.c +++ b/conf.c @@ -787,6 +787,8 @@ pasta_opts: info( " --netns PATH|NAME Target network namespace to join"); info( " --netns-only Don't join existing user namespace"); info( " implied if PATH or NAME are given without --userns"); + info( " --no-netns-quit Don't quit if filesystem-bound target"); + info( " network namespace is deleted"); info( " --config-net Configure tap interface in namespace"); info( " --ns-mac-addr ADDR Set MAC address on tap interface"); -- 2.35.1

2 1

public-inbox list archives at https://archives.passt.top/
by Stefano Brivio 08 Oct '22

08 Oct '22

Hi, Archives powered by public-inbox for passt-dev and passt-user are now available at: https://archives.passt.top/ Patch hunk headers link to code blobs (coderepo). They are also linked from the Mailman interface. -- Stefano

1 0

[PATCH] conf, tap: Add option to quit once the client closes the connection
by Stefano Brivio 08 Oct '22

08 Oct '22

This is practical to avoid explicit lifecycle management in users, e.g. libvirtd, and is trivial to implement. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 17 ++++++++++++++++- passt.1 | 5 +++++ passt.h | 1 + tap.c | 5 +++++ 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/conf.c b/conf.c index 4ec3153..53a8dcf 100644 --- a/conf.c +++ b/conf.c @@ -712,6 +712,7 @@ static void usage(const char *name) if (strstr(name, "pasta")) goto pasta_opts; + info( " -1, --one-off Quit after handling one single client"); info( " -t, --tcp-ports SPEC TCP port forwarding to guest"); info( " can be specified multiple times"); info( " SPEC can be:"); @@ -1024,6 +1025,7 @@ void conf(struct ctx *c, int argc, char **argv) {"no-map-gw", no_argument, &c->no_map_gw, 1 }, {"ipv4-only", no_argument, NULL, '4' }, {"ipv6-only", no_argument, NULL, '6' }, + {"one-off", no_argument, NULL, '1' }, {"tcp-ports", required_argument, NULL, 't' }, {"udp-ports", required_argument, NULL, 'u' }, {"tcp-ns", required_argument, NULL, 'T' }, @@ -1062,7 +1064,7 @@ void conf(struct ctx *c, int argc, char **argv) c->no_dhcp_dns = c->no_dhcp_dns_search = 1; optstring = "vdqfel:hI:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:"; } else { - optstring = "vdqfel:hs:p:P:m:a:n:M:g:i:D:S:46t:u:"; + optstring = "vdqfel:hs:p:P:m:a:n:M:g:i:D:S:461t:u:"; } c->tcp.fwd_in.mode = c->tcp.fwd_out.mode = 0; @@ -1488,6 +1490,19 @@ void conf(struct ctx *c, int argc, char **argv) case '6': v6_only = true; break; + case '1': + if (c->mode != MODE_PASST) { + err("--one-off is for passt mode only"); + usage(argv[0]); + } + + if (c->one_off) { + err("Redundant --one-off option"); + usage(argv[0]); + } + + c->one_off = true; + break; case 't': case 'u': case 'T': diff --git a/passt.1 b/passt.1 index c63a439..41f8836 100644 --- a/passt.1 +++ b/passt.1 @@ -302,6 +302,11 @@ Path for UNIX domain socket used by \fBqemu\fR(1) or \fBqrap\fR(1) to connect to Default is to probe a free socket, not accepting connections, starting from \fI/tmp/passt_1.socket\fR to \fI/tmp/passt_64.socket\fR. +.TP +.BR \-1 ", " \-\-one-off +Quit after handling a single client connection, that is, once the client closes +the socket, or once we get a socket error. + .TP .BR \-t ", " \-\-tcp-ports " " \fIspec Configure TCP port forwarding to guest. \fIspec\fR can be one of: diff --git a/passt.h b/passt.h index de79e7b..67281db 100644 --- a/passt.h +++ b/passt.h @@ -196,6 +196,7 @@ struct ctx { char sock_path[UNIX_PATH_MAX]; char pcap[PATH_MAX]; char pid_file[PATH_MAX]; + int one_off; int pasta_netns_fd; diff --git a/tap.c b/tap.c index bdcc161..78de42c 100644 --- a/tap.c +++ b/tap.c @@ -1011,5 +1011,10 @@ void tap_handler(struct ctx *c, int fd, uint32_t events, return; reinit: + if (c->one_off) { + info("Client closed connection, exiting"); + exit(EXIT_SUCCESS); + } + tap_sock_init(c); } -- 2.35.1

2 2

[PATCH v2 0/7] Add support for log file and version display
by Stefano Brivio 07 Oct '22

07 Oct '22

Patches 1/7 to 4/7 add support for logging to a file, via -l/--log-file, with mandatory size limit and rotation. Patches 5/7 and 7/7 fix two minor details that came up while implementing the feature itself. Patch 6/7 adds a --version option with a version string generated by the Makefile using git, if available, and includes the version string in the log header. v2: - Drop patch adding new test from the series (8/8), subject to further discussion - Changes for 4/7 and 6/7 reported in change messages Stefano Brivio (7): Move logging functions to a new file, log.c conf: Drop duplicate, diverging optstring assignments passt.h: Include netinet/if_ether.h before struct ctx declaration log, conf: Add support for logging to file log: Add missing function comment for trace_init() conf, log, Makefile: Add versioning information util: Check return value of lseek() while reading bound ports from procfs Makefile | 9 +- README.md | 2 +- conf.c | 74 ++++++++--- dhcp.c | 1 + dhcpv6.c | 1 + icmp.c | 1 + isolation.c | 1 + log.c | 363 +++++++++++++++++++++++++++++++++++++++++++++++++++ log.h | 32 +++++ ndp.c | 1 + netlink.c | 1 + packet.c | 1 + passt.1 | 18 ++- passt.c | 2 + passt.h | 2 + pasta.c | 1 + pcap.c | 1 + tap.c | 1 + tcp.c | 1 + tcp_splice.c | 1 + udp.c | 1 + util.c | 131 ++----------------- util.h | 22 +--- 23 files changed, 509 insertions(+), 159 deletions(-) create mode 100644 log.c create mode 100644 log.h -- 2.35.1

1 7

[PATCH] test: Add rudimentary support to run selected tests only
by Stefano Brivio 07 Oct '22

07 Oct '22

To keep this simple, only support tests that have corresponding setup and teardown functions implied by their path. For example: ./run passt/ndp will trigger the 'passt' setup and teardown functions. This is not really elegant, but it looks robust, and while David is considering proper alternatives, it should be quite useful. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- test/README.md | 16 ++++++++++++++++ test/lib/term | 2 +- test/run | 37 ++++++++++++++++++++++++++++++++++++- 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/test/README.md b/test/README.md index 72a4986..09ad05d 100644 --- a/test/README.md +++ b/test/README.md @@ -85,6 +85,22 @@ variable settings: DEBUG=1 enables debugging messages, TRACE=1 enables tracing PCAP=1 TRACE=1 ./run +## Running selected tests + +Rudimentary support to run a list of selected tests, without support for +dependencies, is available. Tests need to have a setup function corresponding to +their path. For example: + + ./run passt/ndp passt/dhcp pasta/ndp + +will call the 'passt' setup function (from lib/setup), run the two corresponding +tests, call the 'passt' teardown function, the 'pasta' setup, run the pasta/ndp +test, and finally tear down the 'pasta' setup. + +Note that requirements on steps implemented by related tests are not handled. +For example, if the 'passt/tcp' needs guest connectivity set up by the +'passt/ndp' and 'passt/dhcp' tests, those need to be listed explicitly. + ## Continuous integration Issuing: diff --git a/test/lib/term b/test/lib/term index eade2cd..1b42df8 100755 --- a/test/lib/term +++ b/test/lib/term @@ -650,7 +650,7 @@ run_term() { asciinema rec --overwrite "${STATEBASE}/demo.uncut" -c "$TMUX /bin/sh -c './run_demo from_term'" video_postprocess "${STATEBASE}/demo.uncut" else - $TMUX /bin/sh -c './run from_term' + $TMUX /bin/sh -c "./run from_term ${*}" fi } diff --git a/test/run b/test/run index 4bb9cd8..1ae270e 100755 --- a/test/run +++ b/test/run @@ -127,6 +127,37 @@ run() { return 0 } +# run_selected() - Run list of tests, with setup/teardown based on test path +# $@: List of tests +run_selected() { + mkfifo $STATEBASE/log_pipe + + term + VALGRIND=1 + + __setup= + for __test; do + if [ "${__test%%/*}" != "${__setup}" ]; then + [ -n "${__setup}" ] && teardown "${__setup}" + __setup="${__test%%/*}" + setup "${__setup}" + fi + + test "${__test}" + done + teardown "${__setup}" + + log "PASS: ${STATUS_PASS}, FAIL: ${STATUS_FAIL}" + + pause_continue \ + "Press any key to keep test session open" \ + "Closing in " \ + "Interrupted, press any key to quit" \ + 9 + + return 0 +} + # demo() - Simpler path for demo purposes demo() { mkfifo $STATEBASE/log_pipe @@ -160,11 +191,15 @@ demo() { [ "$(basename "${0}")" = "run_demo" ] && DEMO=1 if [ "${1}" = "from_term" ]; then + shift + exec > ${LOGDIR}/script.log 2>&1 [ ${DEBUG} -eq 1 ] && set -x cd .. if [ ${DEMO} -eq 1 ]; then demo + elif [ -n "${1}" ]; then + run_selected ${*} else run fi @@ -176,7 +211,7 @@ else :> "${LOGFILE}" STATEBASE="$(mktemp -d --tmpdir passt-tests-XXXXXX)" trap "cleanup" EXIT - run_term + run_term ${*} fi [ ${DEMO} -eq 1 ] && exit 0 -- 2.35.1

2 2

[PATCH] conf, tcp, udp: Allow specification of interface to bind to
by Stefano Brivio 07 Oct '22

07 Oct '22

Since kernel version 5.7, commit c427bfec18f2 ("net: core: enable SO_BINDTODEVICE for non-root users"), we can bind sockets to interfaces, if they haven't been bound yet (as in bind()). Introduce an optional interface specification for forwarded ports, prefixed by %, that can be passed together with an address. Reported use case: running local services that use ports we want to have externally forwarded: https://github.com/containers/podman/issues/14425 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 31 +++++++++++++++++++++---------- icmp.c | 4 ++-- passt.1 | 12 ++++++++++-- tcp.c | 27 +++++++++++++++------------ tcp.h | 2 +- udp.c | 35 ++++++++++++++++++----------------- udp.h | 2 +- util.c | 19 ++++++++++++++++++- util.h | 3 ++- 9 files changed, 88 insertions(+), 47 deletions(-) diff --git a/conf.c b/conf.c index 53a8dcf..5a7f9b9 100644 --- a/conf.c +++ b/conf.c @@ -180,8 +180,8 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg, struct port_fwd *fwd) { char addr_buf[sizeof(struct in6_addr)] = { 0 }, *addr = addr_buf; + char buf[BUFSIZ], *spec, *ifname = NULL, *p; uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; - char buf[BUFSIZ], *spec, *p; sa_family_t af = AF_UNSPEC; bool exclude_only = true; @@ -209,9 +209,9 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg, for (i = 0; i < PORT_EPHEMERAL_MIN; i++) { if (optname == 't') - tcp_sock_init(c, 0, AF_UNSPEC, NULL, i); + tcp_sock_init(c, 0, AF_UNSPEC, NULL, NULL, i); else if (optname == 'u') - udp_sock_init(c, 0, AF_UNSPEC, NULL, i); + udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL, i); } return 0; @@ -231,6 +231,14 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg, if (optname != 't' && optname != 'u') goto bad; + if ((ifname = strchr(buf, '%'))) { + if (ifname - buf >= IFNAMSIZ - 1) + goto bad; + + *ifname = 0; + ifname++; + } + if (inet_pton(AF_INET, buf, addr)) af = AF_INET; else if (inet_pton(AF_INET6, buf, addr)) @@ -278,9 +286,9 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg, bitmap_set(fwd->map, i); if (optname == 't') - tcp_sock_init(c, 0, af, addr, i); + tcp_sock_init(c, 0, af, addr, NULL, i); else if (optname == 'u') - udp_sock_init(c, 0, af, addr, i); + udp_sock_init(c, 0, af, addr, NULL, i); } return 0; @@ -324,9 +332,9 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg, fwd->delta[i] = mapped_range.first - orig_range.first; if (optname == 't') - tcp_sock_init(c, 0, af, addr, i); + tcp_sock_init(c, 0, af, addr, ifname, i); else if (optname == 'u') - udp_sock_init(c, 0, af, addr, i); + udp_sock_init(c, 0, af, addr, ifname, i); } } while ((p = next_chunk(p, ','))); @@ -720,8 +728,9 @@ static void usage(const char *name) info( " 'all': forward all unbound, non-ephemeral ports"); info( " a comma-separated list, optionally ranged with '-'"); info( " and optional target ports after ':', with optional"); - info( " address specification suffixed by '/'. Ranges can be"); - info( " reduced by excluding ports or ranges prefixed by '~'"); + info( " address specification suffixed by '/' and optional"); + info( " interface prefixed by '%'. Ranges can be reduced by"); + info( " excluding ports or ranges prefixed by '~'"); info( " Examples:"); info( " -t 22 Forward local port 22 to 22 on guest"); info( " -t 22:23 Forward local port 22 to 23 on guest"); @@ -740,6 +749,7 @@ static void usage(const char *name) exit(EXIT_FAILURE); pasta_opts: + info( " -t, --tcp-ports SPEC TCP port forwarding to namespace"); info( " can be specified multiple times"); info( " SPEC can be:"); @@ -747,7 +757,8 @@ pasta_opts: info( " 'auto': forward all ports currently bound in namespace"); info( " a comma-separated list, optionally ranged with '-'"); info( " and optional target ports after ':', with optional"); - info( " address specification suffixed by '/'. Examples:"); + info( " address specification suffixed by '/' and optional"); + info( " interface prefixed by '%'. Examples:"); info( " -t 22 Forward local port 22 to port 22 in netns"); info( " -t 22:23 Forward local port 22 to port 23"); info( " -t 22,25 Forward ports 22, 25 to ports 22, 25"); diff --git a/icmp.c b/icmp.c index f02f89f..6bd87fd 100644 --- a/icmp.c +++ b/icmp.c @@ -169,7 +169,7 @@ int icmp_tap_handler(const struct ctx *c, int af, const void *addr, iref.icmp.id = id = ntohs(ih->un.echo.id); if ((s = icmp_id_map[V4][id].sock) <= 0) { - s = sock_l4(c, AF_INET, IPPROTO_ICMP, NULL, id, + s = sock_l4(c, AF_INET, IPPROTO_ICMP, NULL, NULL, id, iref.u32); if (s < 0) goto fail_sock; @@ -207,7 +207,7 @@ int icmp_tap_handler(const struct ctx *c, int af, const void *addr, iref.icmp.id = id = ntohs(ih->icmp6_identifier); if ((s = icmp_id_map[V6][id].sock) <= 0) { - s = sock_l4(c, AF_INET6, IPPROTO_ICMPV6, NULL, id, + s = sock_l4(c, AF_INET6, IPPROTO_ICMPV6, NULL, NULL, id, iref.u32); if (s < 0) goto fail_sock; diff --git a/passt.1 b/passt.1 index 41f8836..daef55d 100644 --- a/passt.1 +++ b/passt.1 @@ -325,7 +325,8 @@ For low (< 1024) ports, see \fBNOTES\fR. .BR ports A comma-separated list of ports, optionally ranged with \fI-\fR, and, optionally, with target ports after \fI:\fR, if they differ. Specific addresses -can be bound as well, separated by \fI/\fR. Within given ranges, selected ports +can be bound as well, separated by \fI/\fR, and also, since Linux 5.7, limited +to specific interfaces, prefixed by \fI%\fR. Within given ranges, selected ports and ranges can be excluded by an additional specification prefixed by \fI~\fR. Specifying excluded ranges only implies that all other ports are forwarded. Examples: @@ -349,6 +350,9 @@ Forward local ports 22 to 80 to corresponding ports on the guest plus 10 -t 192.0.2.1/22 Forward local port 22, bound to 192.0.2.1, to port 22 on the guest .TP +-t 192.0.2.1%eth0/22 +Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22 +.TP -t 2000-5000,~3000-3010 Forward local ports 2000 to 5000, but not 3000 to 3010 .TP @@ -399,7 +403,8 @@ periodically derived (every second) from listening sockets reported by .BR ports A comma-separated list of ports, optionally ranged with \fI-\fR, and, optionally, with target ports after \fI:\fR, if they differ. Specific addresses -can be bound as well, separated by \fI/\fR. Within given ranges, selected ports +can be bound as well, separated by \fI/\fR, and also, since Linux 5.7, limited +to specific interfaces, prefixed by \fI%\fR. Within given ranges, selected ports and ranges can be excluded by an additional specification prefixed by \fI~\fR. Specifying excluded ranges only implies that all other ports are forwarded. Examples: @@ -424,6 +429,9 @@ namespace -t 192.0.2.1/22 Forward local port 22, bound to 192.0.2.1, to port 22 in the target namespace .TP +-t 192.0.2.1%eth0/22 +Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22 +.TP -t 2000-5000,~3000-3010 Forward local ports 2000 to 5000, but not 3000 to 3010 .TP diff --git a/tcp.c b/tcp.c index 830dc88..7e82589 100644 --- a/tcp.c +++ b/tcp.c @@ -3078,10 +3078,11 @@ void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events, * @ns: In pasta mode, if set, bind with loopback address in namespace * @af: Address family to select a specific IP version, or AF_UNSPEC * @addr: Pointer to address for binding, NULL if not configured + * @ifname: Name of interface to bind to, NULL if not configured * @port: Port, host order */ void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af, - const void *addr, in_port_t port) + const void *addr, const char *ifname, in_port_t port) { union tcp_epoll_ref tref = { .tcp.listen = 1 }; const void *bind_addr; @@ -3103,8 +3104,8 @@ void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af, tref.tcp.splice = 0; if (!ns) { - s = sock_l4(c, AF_INET, IPPROTO_TCP, bind_addr, port, - tref.u32); + s = sock_l4(c, AF_INET, IPPROTO_TCP, bind_addr, ifname, + port, tref.u32); if (s >= 0) tcp_sock_set_bufsize(c, s); else @@ -3118,8 +3119,8 @@ void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af, bind_addr = &(uint32_t){ htonl(INADDR_LOOPBACK) }; tref.tcp.splice = 1; - s = sock_l4(c, AF_INET, IPPROTO_TCP, bind_addr, port, - tref.u32); + s = sock_l4(c, AF_INET, IPPROTO_TCP, bind_addr, ifname, + port, tref.u32); if (s >= 0) tcp_sock_set_bufsize(c, s); else @@ -3144,8 +3145,8 @@ void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af, tref.tcp.splice = 0; if (!ns) { - s = sock_l4(c, AF_INET6, IPPROTO_TCP, bind_addr, port, - tref.u32); + s = sock_l4(c, AF_INET6, IPPROTO_TCP, bind_addr, ifname, + port, tref.u32); if (s >= 0) tcp_sock_set_bufsize(c, s); else @@ -3159,8 +3160,8 @@ void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af, bind_addr = &in6addr_loopback; tref.tcp.splice = 1; - s = sock_l4(c, AF_INET6, IPPROTO_TCP, bind_addr, port, - tref.u32); + s = sock_l4(c, AF_INET6, IPPROTO_TCP, bind_addr, ifname, + port, tref.u32); if (s >= 0) tcp_sock_set_bufsize(c, s); else @@ -3193,7 +3194,7 @@ static int tcp_sock_init_ns(void *arg) if (!bitmap_isset(c->tcp.fwd_out.map, port)) continue; - tcp_sock_init(c, 1, AF_UNSPEC, NULL, port); + tcp_sock_init(c, 1, AF_UNSPEC, NULL, NULL, port); } return 0; @@ -3410,7 +3411,8 @@ static int tcp_port_rebind(void *arg) if ((a->c->ifi4 && tcp_sock_ns[port][V4] == -1) || (a->c->ifi6 && tcp_sock_ns[port][V6] == -1)) - tcp_sock_init(a->c, 1, AF_UNSPEC, NULL, port); + tcp_sock_init(a->c, 1, AF_UNSPEC, NULL, NULL, + port); } } else { for (port = 0; port < NUM_PORTS; port++) { @@ -3443,7 +3445,8 @@ static int tcp_port_rebind(void *arg) if ((a->c->ifi4 && tcp_sock_init_ext[port][V4] == -1) || (a->c->ifi6 && tcp_sock_init_ext[port][V6] == -1)) - tcp_sock_init(a->c, 0, AF_UNSPEC, NULL, port); + tcp_sock_init(a->c, 0, AF_UNSPEC, NULL, NULL, + port); } } diff --git a/tcp.h b/tcp.h index 2548d4d..7ba7ab7 100644 --- a/tcp.h +++ b/tcp.h @@ -21,7 +21,7 @@ void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events, int tcp_tap_handler(struct ctx *c, int af, const void *addr, const struct pool *p, const struct timespec *now); void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af, - const void *addr, in_port_t port); + const void *addr, const char *ifname, in_port_t port); int tcp_init(struct ctx *c); void tcp_timer(struct ctx *c, const struct timespec *ts); void tcp_defer_handler(struct ctx *c); diff --git a/udp.c b/udp.c index 5422fdd..cac9c65 100644 --- a/udp.c +++ b/udp.c @@ -1005,7 +1005,7 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr, union udp_epoll_ref uref = { .udp.bound = 1, .udp.port = src }; - s = sock_l4(c, AF_INET, IPPROTO_UDP, NULL, src, + s = sock_l4(c, AF_INET, IPPROTO_UDP, NULL, NULL, src, uref.u32); if (s < 0) return p->count; @@ -1057,8 +1057,8 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr, .udp.v6 = 1, .udp.port = src }; - s = sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, src, - uref.u32); + s = sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, NULL, + src, uref.u32); if (s < 0) return p->count; @@ -1111,10 +1111,11 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr, * @ns: In pasta mode, if set, bind with loopback address in namespace * @af: Address family to select a specific IP version, or AF_UNSPEC * @addr: Pointer to address for binding, NULL if not configured + * @ifname: Name of interface to bind to, NULL if not configured * @port: Port, host order */ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, - const void *addr, in_port_t port) + const void *addr, const char *ifname, in_port_t port) { union udp_epoll_ref uref = { .udp.bound = 1 }; const void *bind_addr; @@ -1138,8 +1139,8 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, if (!ns) { uref.udp.splice = 0; - s = sock_l4(c, AF_INET, IPPROTO_UDP, bind_addr, port, - uref.u32); + s = sock_l4(c, AF_INET, IPPROTO_UDP, bind_addr, ifname, + port, uref.u32); udp_tap_map[V4][uref.udp.port].sock = s; } @@ -1148,16 +1149,16 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, bind_addr = &(uint32_t){ htonl(INADDR_LOOPBACK) }; uref.udp.splice = UDP_TO_NS; - sock_l4(c, AF_INET, IPPROTO_UDP, bind_addr, port, - uref.u32); + sock_l4(c, AF_INET, IPPROTO_UDP, bind_addr, ifname, + port, uref.u32); } if (ns) { bind_addr = &(uint32_t){ htonl(INADDR_LOOPBACK) }; uref.udp.splice = UDP_TO_INIT; - sock_l4(c, AF_INET, IPPROTO_UDP, bind_addr, port, - uref.u32); + sock_l4(c, AF_INET, IPPROTO_UDP, bind_addr, ifname, + port, uref.u32); } } @@ -1171,8 +1172,8 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, if (!ns) { uref.udp.splice = 0; - s = sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, port, - uref.u32); + s = sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, ifname, + port, uref.u32); udp_tap_map[V6][uref.udp.port].sock = s; } @@ -1181,16 +1182,16 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, bind_addr = &in6addr_loopback; uref.udp.splice = UDP_TO_NS; - sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, port, - uref.u32); + sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, ifname, + port, uref.u32); } if (ns) { bind_addr = &in6addr_loopback; uref.udp.splice = UDP_TO_INIT; - sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, port, - uref.u32); + sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, ifname, + port, uref.u32); } } } @@ -1213,7 +1214,7 @@ int udp_sock_init_ns(void *arg) if (!bitmap_isset(c->udp.fwd_out.f.map, dst)) continue; - udp_sock_init(c, 1, AF_UNSPEC, NULL, dst); + udp_sock_init(c, 1, AF_UNSPEC, NULL, NULL, dst); } return 0; diff --git a/udp.h b/udp.h index d14df0a..b4ee8b7 100644 --- a/udp.h +++ b/udp.h @@ -13,7 +13,7 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events, int udp_tap_handler(struct ctx *c, int af, const void *addr, const struct pool *p, const struct timespec *now); void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, - const void *addr, in_port_t port); + const void *addr, const char *ifname, in_port_t port); int udp_init(struct ctx *c); void udp_timer(struct ctx *c, const struct timespec *ts); void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s, diff --git a/util.c b/util.c index 5b1e08a..76057be 100644 --- a/util.c +++ b/util.c @@ -90,13 +90,15 @@ found: * @af: Address family, AF_INET or AF_INET6 * @proto: Protocol number * @bind_addr: Address for binding, NULL for any + * @ifname: Interface for binding, NULL for any * @port: Port, host order * @data: epoll reference portion for protocol handlers * * Return: newly created socket, -1 on error */ int sock_l4(const struct ctx *c, int af, uint8_t proto, - const void *bind_addr, uint16_t port, uint32_t data) + const void *bind_addr, const char *ifname, uint16_t port, + uint32_t data) { union epoll_ref ref = { .r.proto = proto, .r.p.data = data }; struct sockaddr_in addr4 = { @@ -163,6 +165,21 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto, if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &y, sizeof(y))) debug("Failed to set SO_REUSEADDR on socket %i", fd); + if (ifname) { + /* Supported since kernel version 5.7, commit c427bfec18f2 + * ("net: core: enable SO_BINDTODEVICE for non-root users"). If + * it's unsupported, don't bind the socket at all, because the + * user might rely on this to filter incoming connections. + */ + if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, + ifname, strlen(ifname))) { + warn("Can't bind socket for %s port %u to %s, closing", + ip_proto_str[proto], port, ifname); + close(fd); + return -1; + } + } + if (bind(fd, sa, sl) < 0) { /* We'll fail to bind to low ports if we don't have enough * capabilities, and we'll fail to bind on already bound ports, diff --git a/util.h b/util.h index e8f99b6..a4b9256 100644 --- a/util.h +++ b/util.h @@ -194,7 +194,8 @@ __attribute__ ((weak)) int ffsl(long int i) { return __builtin_ffsl(i); } char *ipv6_l4hdr(const struct pool *p, int index, size_t offset, uint8_t *proto, size_t *dlen); int sock_l4(const struct ctx *c, int af, uint8_t proto, - const void *bind_addr, uint16_t port, uint32_t data); + const void *bind_addr, const char *ifname, uint16_t port, + uint32_t data); void sock_probe_mem(struct ctx *c); int timespec_diff_ms(const struct timespec *a, const struct timespec *b); void bitmap_set(uint8_t *map, int bit); -- 2.35.1

1 0

[PATCH v2 00/28] Fixes for static checkers
by David Gibson 30 Sep '22

30 Sep '22

The passt tests include two static checking tools: clang-tidy and cppcheck. However, newer versions of those tools have introduced extra checks, and may cause these tests to fail. This series fixes all the clang-tidy and cppcheck warnings, either by altering our code, or by suppressing them with relevant options to the checkers. With this series, the checks are now clean on both my Fedora 36 machine (clang-tools-extra-14.0.5-1.fc36.x86_64 and cppcheck-2.7.4-2.fc36.x86_64) and my Debian machine (bookworm with some pieces from sid: clang-tidy 1:14.0-55.1 and cppcheck 2.9-1). Changes since v1: * Fixed a whitespace error * Added extra background information and details to comments and commit messages when removing old suppressions * Improved conf_runas() rework to give better error messages David Gibson (28): Clean up parsing of port ranges clang-tidy: Suppress warning about unchecked error in logfn macro clang-tidy: Fix spurious null pointer warning in pasta_start_ns() clang-tidy: Remove duplicate #include from icmp.c Catch failures when installing signal handlers Pack DHCPv6 "on wire" structures Clean up parsing in conf_runas() cppcheck: Reduce scope of some variables Don't shadow 'i' in conf_ports() Don't shadow global function names Stricter checking for nsholder.c cppcheck: Work around false positive NULL pointer dereference error cppcheck: Use inline suppression for ffsl() cppcheck: Use inline suppressions for qrap.c cppcheck: Use inline suppression for strtok() in conf.c Avoid ugly 'end' members in netlink structures cppcheck: Broaden suppression for unused struct members cppcheck: Remove localtime suppression for pcap.c qrap: Handle case of PATH environment variable being unset cppcheck: Suppress same-value-in-ternary branches warning cppcheck: Suppress NULL pointer warning in tcp_sock_consume() Regenerate seccomp.h if seccomp.sh changes cppcheck: Avoid errors due to zeroes in bitwise ORs cppcheck: Remove unused knownConditionTrueFalse suppression cppcheck: Remove unused objectIndex suppressions cppcheck: Remove unused va_list_usedBeforeStarted suppression Mark unused functions for cppcheck cppcheck: Remove unused unmatchedSuppression suppressions Makefile | 26 +--- arch.c | 4 +- conf.c | 359 +++++++++++++++++++++++------------------------- dhcpv6.c | 26 ++-- icmp.c | 1 - igmp.c | 1 + netlink.c | 22 +-- passt.c | 7 +- pasta.c | 5 +- qrap.c | 18 ++- seccomp.sh | 2 + siphash.c | 1 + tap.c | 5 +- tcp.c | 5 + test/Makefile | 2 +- test/nsholder.c | 2 +- util.c | 2 +- util.h | 1 + 18 files changed, 236 insertions(+), 253 deletions(-) -- 2.37.3

2 29

[PATCH] Makefile: Hack for optimised-away store in ndp() before checksum calculation
by Stefano Brivio 29 Sep '22

29 Sep '22

With gcc 11 and 12, passing -flto, or -flto=auto, and -O2, intra-procedural optimisation gets rid of a fundamental bit in ndp(): the store of hop_limit in the IPv6 header, before the checksum is calculated, which on x86_64 looks like this: ip6hr->hop_limit = IPPROTO_ICMPV6; b8c0: c6 44 24 35 3a movb $0x3a,0x35(%rsp) Here, hop_limit is temporarily set to the protocol number, to conveniently get the IPv6 pseudo-header for ICMPv6 checksum calculation in memory. With LTO, the assignment just disappears from the binary. This is rather visible as NDP messages get a wrong checksum, namely the expected checksum plus 58, and they're ignored by the guest or in the namespace, meaning we can't get any IPv6 routes, as reported by Wenli Quan. The issue affects a significant number of distribution builds, including the ones for CentOS Stream 9, EPEL 9, Fedora >= 35, Mageia Cauldron, and openSUSE Tumbleweed. As a quick workaround, declare csum_unaligned() as "noipa" for gcc 11 and 12, with -flto and -O2. This disables inlining and cloning, which causes the assignment to be compiled again. Leave a TODO item: we should figure out if a gcc issue has already been reported, and report one otherwise. There's no apparent justification as to why the store could go away. Reported-by: Wenli Quan <wquan(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- Makefile | 7 +++++++ checksum.c | 3 +++ 2 files changed, 10 insertions(+) diff --git a/Makefile b/Makefile index 1d45f17..d4b623f 100644 --- a/Makefile +++ b/Makefile @@ -50,11 +50,18 @@ HEADERS = $(PASST_HEADERS) seccomp.h # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78993 # from the pointer arithmetic used from the tcp_tap_handler() path to get the # remote connection address. +# +# TODO: With the same combination, in ndp(), gcc optimises away the store of +# hop_limit in the IPv6 header (temporarily set to the protocol number for +# convenience, to mimic the ICMPv6 checksum pseudo-header) before the call to +# csum_unaligned(). Mark csum_unaligned() as "noipa" as a quick work-around, +# while we figure out if a corresponding gcc issue has already been reported. ifeq (,$(filter-out 11 12, $(shell $(CC) -dumpversion))) ifneq (,$(filter -flto%,$(FLAGS) $(CFLAGS))) ifneq (,$(filter -O2,$(FLAGS) $(CFLAGS))) FLAGS += -DTCP_HASH_NOINLINE FLAGS += -DSIPHASH_20B_NOINLINE + FLAGS += -DCSUM_UNALIGNED_NO_IPA endif endif endif diff --git a/checksum.c b/checksum.c index acb1e3e..56ad01e 100644 --- a/checksum.c +++ b/checksum.c @@ -97,6 +97,9 @@ uint16_t csum_fold(uint32_t sum) * * Return: 16-bit IPv4-style checksum */ +#if CSUM_UNALIGNED_NO_IPA +__attribute__((__noipa__)) /* See comment in Makefile */ +#endif uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init) { return (uint16_t)~csum_fold(sum_16b(buf, len) + init); -- 2.35.1

1 0

[PATCH 00/28] Fixes for static checkers
by David Gibson 29 Sep '22

29 Sep '22

The passt tests include two static checking tools: clang-tidy and cppcheck. However, newer versions of those tools have introduced extra checks, and may cause these tests to fail. This series fixes all the clang-tidy and cppcheck warnings, either by altering our code, or by suppressing them with relevant options to the checkers. With this series, the checks are now clean on both my Fedora 36 machine (clang-tools-extra-14.0.5-1.fc36.x86_64 and cppcheck-2.7.4-2.fc36.x86_64) and my Debian machine (bookworm with some pieces from sid: clang-tidy 1:14.0-55.1 and cppcheck 2.9-1). David Gibson (28): Clean up parsing of port ranges clang-tidy: Suppress warning about unchecked error in logfn macro clang-tidy: Fix spurious null pointer warning in pasta_start_ns() clang-tidy: Remove duplicate #include from icmp.c Catch failures when installing signal handlers Pack DHCPv6 "on wire" structures Clean up parsing in conf_runas() cppcheck: Reduce scope of some variables Don't shadow 'i' in conf_ports() Don't shadow global function names Stricter checking for nsholder.c cppcheck: Work around false positive NULL pointer dereference error cppcheck: Use inline suppression for ffsl() cppcheck: Use inline suppressions for qrap.c cppcheck: Use inline suppression for strtok() in conf.c Avoid ugly 'end' members in netlink structures cppcheck: Broaden suppression for unused struct members cppcheck: Remove localtime suppression for pcap.c qrap: Handle case of PATH environment variable being unset cppcheck: Suppress same-value-in-ternary branches warning cppcheck: Suppress NULL pointer warning in tcp_sock_consume() Regenerate seccomp.h if seccomp.sh changes cppcheck: Avoid errors due to zeroes in bitwise ORs cppcheck: Remove unused knownConditionTrueFalse suppression cppcheck: Remove unused objectIndex suppressions cppcheck: Remove unused va_list_usedBeforeStarted suppression Mark unused functions for cppcheck cppcheck: Remove unused unmatchedSuppression suppressions Makefile | 26 +--- arch.c | 4 +- conf.c | 344 ++++++++++++++++++++++-------------------------- dhcpv6.c | 26 ++-- icmp.c | 1 - igmp.c | 1 + netlink.c | 22 ++-- passt.c | 7 +- pasta.c | 5 +- qrap.c | 18 ++- seccomp.sh | 2 + siphash.c | 1 + tap.c | 5 +- tcp.c | 2 + test/Makefile | 2 +- test/nsholder.c | 2 +- util.c | 2 +- util.h | 1 + 18 files changed, 222 insertions(+), 249 deletions(-) -- 2.37.3

2 40

[PATCH 0/2] Address warnings from gcc 12.2 with -flto
by Stefano Brivio 28 Sep '22

28 Sep '22

The first one is a bogus report of an uninitialised field passed as input to hash functions for TCP connections, already seen with gcc 11. The second one is a bogus stringop-overread which already happened with gcc 12.1, but this time a "cute" pragma won't make it go away. Both harmless, but noisy. Based on pending series posted by David, if it matters. Stefano Brivio (2): Makefile: Extend noinline workarounds for LTO and -O2 to gcc 12 udp: Replace pragma to ignore bogus stringop-overread warning with workaround Makefile | 6 +++--- udp.c | 26 ++++++++++++++++++-------- util.h | 23 ----------------------- 3 files changed, 21 insertions(+), 34 deletions(-) -- 2.35.1

1 2