- dev - passt

[PATCH] tcp: Explicitly check option length field values in tcp_opt_get()
by Stefano Brivio 05 Jan '23

05 Jan '23

Reported by Coverity (CWE-606, Untrusted loop bound), and actually harmless because we'll exit the option-scanning loop if the remaining length is not enough for a new option, instead of reading past the header. In any case, it looks like a good idea to explicitly check for reasonable values of option lengths. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tcp.c b/tcp.c index 26037b3..1e0a338 100644 --- a/tcp.c +++ b/tcp.c @@ -1117,6 +1117,10 @@ static int tcp_opt_get(const char *opts, size_t len, uint8_t type_find, break; default: type = *(opts++); + + if (*(uint8_t *)opts < 2 || *(uint8_t *)opts > len) + return -1; + optlen = *(opts++) - 2; len -= 2; -- 2.35.1

2 1

[PATCH v3 0/8] Don't use additional sockets for receiving "spliced" UDP communications
by David Gibson 05 Jan '23

05 Jan '23

At present, the UDP "splice" and "tap" paths are quite separate. We have separate sockets to receive packets bound for the tap and splice paths. This leads to some code duplication, and extra open sockets. This series partially unifies the two paths, allowing us to use a single (host side) socket, bound to 0.0.0.0 or :: to receive packets for both cases. Changes since v2: * Don't receive multiple packets at once for pasta mode - seems to hurt throughput on balance. * Add some comments clarifying reasoning here. Changes since v1: * Renamed udp_localname[46] to udp[46]_localname * Allow handling of UDP port 0 * Fix a bug which could misidentify certain v6 packets as v4-spliceable * Some minor cosmetic fixes to code and commit messages David Gibson (8): udp: Move sending pasta tap frames to the end of udp_sock_handler() udp: Split sending to passt tap interface into separate function udp: Split receive from preparation and send in udp_sock_handler() udp: Don't handle tap receive batch size calculation within a #define udp: Pre-populate msg_names with local address udp: Unify udp_sock_handler_splice() with udp_sock_handler() udp: Decide whether to "splice" per datagram rather than per socket udp: Don't use separate sockets to listen for spliced packets udp.c | 389 ++++++++++++++++++++++++++++++--------------------------- udp.h | 2 +- util.h | 7 ++ 3 files changed, 214 insertions(+), 184 deletions(-) -- 2.39.0

2 10

[PATCH] test/perf/pasta_tcp: Add host to namespace cases for traffic via tap
by Stefano Brivio 05 Jan '23

05 Jan '23

Similarly to UDP cases, these were missing as it wasn't clear, when the other tests were introduced, if using the global address of a namespace, from the host, should have resulted in connections being routed via the tap interface. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- test/perf/pasta_tcp | 57 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/test/perf/pasta_tcp b/test/perf/pasta_tcp index 1847c83..8ebb25c 100644 --- a/test/perf/pasta_tcp +++ b/test/perf/pasta_tcp @@ -240,4 +240,61 @@ nsout LAT tcp_crr --nolog -P 10003 -C 10013 -4 -c -H __GW__ | sed -n 's/^through hostw lat __LAT__ 1500 500 + +tr TCP throughput over IPv6: host to ns +nsout IFNAME ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname' +nsout ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .prefixlen == 64).local' +bw - +bw - +bw - +iperf3 BW host ns __ADDR6__ 100${i}2 __THREADS__ __TIME__ __OPTS__ +bw __BW__ 8.0 10.0 + +tl TCP RR latency over IPv6: host to ns +lat - +lat - +lat - +nsb tcp_rr --nolog -P 10002 -C 10012 -6 +hout LAT tcp_rr --nolog -P 10002 -C 10012 -6 -c -H __ADDR6__ | sed -n 's/^throughput=$.*$/\1/p' +nsw +lat __LAT__ 150 100 + +tl TCP CRR latency over IPv6: host to ns +lat - +lat - +lat - +sleep 1 +nsb tcp_crr --nolog -P 10002 -C 10012 -6 +hout LAT tcp_crr --nolog -P 10002 -C 10012 -6 -c -H __ADDR6__ | sed -n 's/^throughput=$.*$/\1/p' +nsw +lat __LAT__ 5000 10000 + + +tr TCP throughput over IPv4: host to ns +nsout ADDR ip -j -4 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[0].local' +bw - +bw - +bw - +iperf3 BW host ns __ADDR__ 100${i}2 __THREADS__ __TIME__ __OPTS__ +bw __BW__ 8.0 10.0 + +tl TCP RR latency over IPv4: host to ns +lat - +lat - +lat - +nsb tcp_rr --nolog -P 10002 -C 10012 -4 +hout LAT tcp_rr --nolog -P 10002 -C 10012 -4 -c -H __ADDR__ | sed -n 's/^throughput=$.*$/\1/p' +nsw +lat __LAT__ 150 100 + +tl TCP CRR latency over IPv4: host to ns +lat - +lat - +lat - +sleep 1 +nsb tcp_crr --nolog -P 10002 -C 10012 -4 +hout LAT tcp_crr --nolog -P 10002 -C 10012 -4 -c -H __ADDR__ | sed -n 's/^throughput=$.*$/\1/p' +nsw +lat __LAT__ 5000 10000 + te -- 2.35.1

2 1

[PATCH 0/8] Don't use additional sockets for receiving "spliced" UDP communications
by David Gibson 04 Jan '23

04 Jan '23

At present, the UDP "splice" and "tap" paths are quite separate. We have separate sockets to receive packets bound for the tap and splice paths. This leads to some code duplication, and extra open sockets. This series partially unifies the two paths, allowing us to use a single (host side) socket, bound to 0.0.0.0 or :: to receive packets for both cases. This is based on my earlier series with some fixes for the tap path. David Gibson (8): udp: Move sending pasta tap frames to the end of udp_sock_handler() udp: Split sending to passt tap interface into separate function udp: Split receive from preparation and send in udp_sock_handler() udp: Receive multiple datagrams at once on the pasta sock->tap path udp: Pre-populate msg_names with local address udp: Unify udp_sock_handler_splice() with udp_sock_handler() udp: Decide whether to "splice" per datagram rather than per socket udp: Don't use separate sockets to listen for spliced packets udp.c | 382 ++++++++++++++++++++++++++++++--------------------------- udp.h | 2 +- util.h | 7 ++ 3 files changed, 207 insertions(+), 184 deletions(-) -- 2.38.1

2 27

[PATCH] test/perf/pasta_udp: Add host to namespace cases for traffic via tap
by Stefano Brivio 04 Jan '23

04 Jan '23

These were missing as it wasn't clear, when the other tests were introduced, if using the global address of a namespace, from the host, should have resulted in traffic being routed via the tap interface (as opposed to the loopback interface). We now clarified that's actually the case. Use same values and thresholds as the tests for loopback traffic, as throughput figures currently indicate there isn't much difference. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- Contrary to what I stated a while ago, there's actually no need to adapt the "code" writing the JavaScript snippets for performance reports -- the trick is simply to keep those ordered like the previous ones, and we'll get the right output. test/perf/pasta_udp | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/perf/pasta_udp b/test/perf/pasta_udp index 27ea724..112d70d 100644 --- a/test/perf/pasta_udp +++ b/test/perf/pasta_udp @@ -179,4 +179,41 @@ nsout LAT udp_rr --nolog -P 10003 -C 10013 -4 -c -H __GW__ | sed -n 's/^throughp hostw lat __LAT__ 200 150 + +tr UDP throughput over IPv6: host to ns +nsout IFNAME ip -j link show | jq -rM '.[] | select(.link_type == "ether").ifname' +nsout ADDR6 ip -j -6 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[] | select(.scope == "global" and .prefixlen == 64).local' +bw - +bw - +bw - +iperf3 BW host ns __ADDR6__ 100${i}2 __THREADS__ __TIME__ __OPTS__ -b 15G +bw __BW__ 7.0 9.0 + +tl UDP RR latency over IPv6: host to ns +lat - +lat - +lat - +nsb udp_rr --nolog -P 10002 -C 10012 -6 +hout LAT udp_rr --nolog -P 10002 -C 10012 -6 -c -H __ADDR6__ | sed -n 's/^throughput=$.*$/\1/p' +nsw +lat __LAT__ 200 150 + + +tr UDP throughput over IPv4: host to ns +nsout ADDR ip -j -4 addr show|jq -rM '.[] | select(.ifname == "__IFNAME__").addr_info[0].local' +bw - +bw - +bw - +iperf3 BW host ns __ADDR__ 100${i}2 __THREADS__ __TIME__ __OPTS__ -b 15G +bw __BW__ 7.0 9.0 + +tl UDP RR latency over IPv4: host to ns +lat - +lat - +lat - +nsb udp_rr --nolog -P 10002 -C 10012 -4 +hout LAT udp_rr --nolog -P 10002 -C 10012 -4 -c -H __ADDR__ | sed -n 's/^throughput=$.*$/\1/p' +nsw +lat __LAT__ 200 150 + te -- 2.35.1

2 1

[PATCH v2 0/8] Don't use additional sockets for receiving "spliced" UDP communications
by David Gibson 16 Dec '22

16 Dec '22

At present, the UDP "splice" and "tap" paths are quite separate. We have separate sockets to receive packets bound for the tap and splice paths. This leads to some code duplication, and extra open sockets. This series partially unifies the two paths, allowing us to use a single (host side) socket, bound to 0.0.0.0 or :: to receive packets for both cases. This is based on my earlier series with some fixes for the tap path. Changes since v1: * Renamed udp_localname[46] to udp[46]_localname * Allow handling of UDP port 0 * Fix a bug which could misidentify certain v6 packets as v4-spliceable * Some minor cosmetic fixes to code and commit messages David Gibson (8): udp: Move sending pasta tap frames to the end of udp_sock_handler() udp: Split sending to passt tap interface into separate function udp: Split receive from preparation and send in udp_sock_handler() udp: Receive multiple datagrams at once on the pasta sock->tap path udp: Pre-populate msg_names with local address udp: Unify udp_sock_handler_splice() with udp_sock_handler() udp: Decide whether to "splice" per datagram rather than per socket udp: Don't use separate sockets to listen for spliced packets udp.c | 380 ++++++++++++++++++++++++++++++--------------------------- udp.h | 2 +- util.h | 7 ++ 3 files changed, 205 insertions(+), 184 deletions(-) -- 2.38.1

2 9

[PATCH 00/10] RFC: Unify and simplify tap send path
by David Gibson 08 Dec '22

08 Dec '22

Although we have an abstraction for the "slow path" (DHCP, NDP) guest bound packets, the TCP and UDP forwarding paths write directly to the tap fd. However, it turns out how they send frames to the tap device is more similar than it originally appears. This series unifies the low-level tap send functions for TCP and UDP, and makes some clean ups along the way. David Gibson (10): pcap: Introduce pcap_frame() helper pcap: Replace pcapm() with pcap_multiple() tcp: Combine two parts of passt tap send path together tcp: Don't keep compute total bytes in a message until we need it tcp: Improve interface to tcp_l2_buf_flush() tcp: Combine two parts of pasta tap send path together tap, tcp: Move tap send path to tap.c tcp,tap: Use different io vector bases depending on tap type udp: Use tap_send_frames() tap: Improve handling of partial frame sends pcap.c | 78 ++++++++----------------------- pcap.h | 3 +- tap.c | 108 ++++++++++++++++++++++++++++++++++++++++++ tap.h | 1 + tcp.c | 145 +++++++++++++-------------------------------------------- udp.c | 145 +++------------------------------------------------------ udp.h | 2 +- 7 files changed, 169 insertions(+), 313 deletions(-) -- 2.38.1

1 10

[PATCH 0/4] udp: Fix some confusion of IPv4 and IPv6 control structures
by David Gibson 06 Dec '22

06 Dec '22

It turns out a couple of places on the IPv4 specific inbound path accidentally use control structures that are supposed to be for IPv6. That could lead to weird behaviour in a rather complex set of circumstances. Path 1/4 here is the actual fix, the rest makes some clean ups to the code that should make similar mistakes harder errors harder to commit in future. This is based on my earlier cleanup of the UDP splicing code, although I think it will rebase trivially. David Gibson (4): udp: Fix inorrect use of IPv6 mh buffers in IPv4 path udp: Better factor IPv4 and IPv6 paths in udp_sock_handler() udp: Preadjust udp[46]_l2_iov_tap[].iov_base for pasta mode udp: Factor out control structure management from udp_sock_fill_data_v[46] udp.c | 184 ++++++++++++++++++++++++++-------------------------------- 1 file changed, 81 insertions(+), 103 deletions(-) -- 2.38.1

2 7

[PATCH v4 00/16] Simplify and correct handling of "spliced" UDP forwarding
by David Gibson 06 Dec '22

06 Dec '22

The UDP "splicing" (forwarding packets from one L4 socket to another, rather than via the tuntap device) code assumes that any given UDP port in the init namespace will only communicate with a single port on the ns side at a time, and vice versa. This will often be the case, but since UDP is a connectionless protocol, it need not be. In fact it is not the case in our existing UDP bandwidth checks, although the specific configuration there means it's not harmful in that case. The failure mode in this case can be quite bad: we don't just fall back to an unoptimized oath, or drop packets, we will misdirect packets to the wrong destination. This series make some substantial simplifications to how we handle the splice forwarding, then corrects it to handle the case of multiple source ports sending to a single destination. This does come at a performance cost. It's not as large as I feared, and shouldn't affect the most common case where there is a 1 to 1 mapping between source and destination ports. I haven't yet been able to confirm the latter because the iperf3 bandwidth test we use *does* have interleaved streams with a common destination port. Based on the earlier series for dual stack TCP sockets. Changes since v3: * Changed interface of udp_splice_sendfrom() to slightly better separate concerns and to make some future cleanups simpler * Fixed a serious buffer overrun bug where we weren't bounds checking as we scanned for additional datagrams with the same source address. Changes since v2: * Minor style and comment revisions Changes since v1: * Added patches 12..16/16 fixing the delivery of packets, as well as just simplifying the mechanics David Gibson (16): udp: Also bind() connected ports for "splice" forwarding udp: Separate tracking of inbound and outbound packet flows udp: Always use sendto() rather than send() for forwarding spliced packets udp: Don't connect "forward" sockets for spliced flows udp: Remove the @bound field from union udp_epoll_ref udp: Split splice field in udp_epoll_ref into (mostly) independent bits udp: Don't create double sockets for -U port udp: Re-use fixed bound sockets for packet forwarding when possible udp: Don't explicitly track originating socket for spliced "connections" udp: Update UDP "connection" timestamps in both directions udp: Simplify udp_sock_handler_splice udp: Make UDP_SPLICE_FRAMES and UDP_TAP_FRAMES_MEM the same thing udp: Add helper to extract port from a sockaddr_in or sockaddr_in6 udp: Unify buffers for tap and splice paths udp: Split send half of udp_sock_handler_splice() from the receive half udp: Correct splice forwarding when receiving from multiple sources passt.h | 2 + udp.c | 522 ++++++++++++++++++++++++++------------------------------ udp.h | 16 +- 3 files changed, 248 insertions(+), 292 deletions(-) -- 2.38.1

2 17

[PATCH v2 00/16] Simplify and correct handling of "spliced" UDP forwarding
by David Gibson 01 Dec '22

01 Dec '22

The UDP "splicing" (forwarding packets from one L4 socket to another, rather than via the tuntap device) code assumes that any given UDP port in the init namespace will only communicate with a single port on the ns side at a time, and vice versa. This will often be the case, but since UDP is a connectionless protocol, it need not be. In fact it is not the case in our existing UDP bandwidth checks, although the specific configuration there means it's not harmful in that case. The failure mode in this case can be quite bad: we don't just fall back to an unoptimized oath, or drop packets, we will misdirect packets to the wrong destination. This series make some substantial simplifications to how we handle the splice forwarding, then corrects it to handle the case of multiple source ports sending to a single destination. This does come at a performance cost. It's not as large as I feared, and shouldn't affect the most common case where there is a 1 to 1 mapping between source and destination ports. I haven't yet been able to confirm the latter because the iperf3 bandwidth test we use *does* have interleaved streams with a common destination port. Based on the earlier series for dual stack TCP sockets. Changes since v1: * Added patches 12..16/16 fixing the delivery of packets, as well as just simplifying the mechanics David Gibson (16): udp: Also bind() connected ports for "splice" forwarding udp: Separate tracking of inbound and outbound packet flows udp: Always use sendto() rather than send() for forwarding spliced packets udp: Don't connect "forward" sockets for spliced flows udp: Remove the @bound field from union udp_epoll_ref udp: Split splice field in udp_epoll_ref into (mostly) independent bits udp: Don't create double sockets for -U port udp: Re-use fixed bound sockets for packet forwarding when possible udp: Don't explicitly track originating socket for spliced "connections" udp: Update UDP "connection" timestamps in both directions udp: Simplify udp_sock_handler_splice udp: Make UDP_SPLICE_FRAMES and UDP_TAP_FRAMES_MEM the same thing udp: Add helper to extract port from a sockaddr_in or sockaddr_in6 udp: Unify buffers for tap and splice paths udp: Split send half of udp_sock_handler_splice() from the receive half udp: Correct splice forwarding when receiving from multiple sources passt.h | 2 + udp.c | 518 +++++++++++++++++++++++++------------------------------- udp.h | 16 +- 3 files changed, 244 insertions(+), 292 deletions(-) -- 2.38.1

2 28

2025

2024

2023

2022

2021

dev