- dev - passt

Problematic throughput numbers
by Jon Maloy 26 Sep '24

26 Sep '24

I made many runs with iperf3 ns->host, and the results are puzzling me. Over and over again, I see throughput practically collapse, with a two orders of magnitude decrease. Just to make sure this wasn't something introduced by me, I went back to the master branch and disabled the SO_PEEK_OFF feature. The result was the same. The below log shows typical run, but it is highly variable. Sometimes almost the whole series is in the 50-70 Gb/s range, and sometimes almost all in the 100-300 Mb/s range. When I added the kernel fix it didn't seem to make any difference. To me this is really worrying, and should be investigated. ///jon pasta NS->host (master branch, SO_PEEK_OFF disabled ----------------------------------------------------------- Server listening on 5201 (test #2) ----------------------------------------------------------- Accepted connection from 127.0.0.1, port 48354 [ 5] local 127.0.0.1 port 5201 connected to 127.0.0.1 port 48360 [ ID] Interval Transfer Bitrate [ 5] 0.00-1.00 sec 7.61 GBytes 65.3 Gbits/sec [ 5] 1.00-2.00 sec 7.59 GBytes 65.2 Gbits/sec [ 5] 2.00-3.00 sec 7.55 GBytes 64.8 Gbits/sec [ 5] 3.00-4.00 sec 7.69 GBytes 66.1 Gbits/sec [ 5] 4.00-5.00 sec 7.55 GBytes 64.8 Gbits/sec [ 5] 5.00-6.00 sec 7.60 GBytes 65.3 Gbits/sec [ 5] 6.00-7.00 sec 7.52 GBytes 64.6 Gbits/sec [ 5] 7.00-8.00 sec 1.02 GBytes 8.73 Gbits/sec [ 5] 8.00-9.00 sec 3.50 MBytes 29.4 Mbits/sec [ 5] 9.00-10.00 sec 17.2 MBytes 145 Mbits/sec [ 5] 10.00-11.00 sec 64.0 MBytes 537 Mbits/sec [ 5] 11.00-12.00 sec 24.1 MBytes 202 Mbits/sec [ 5] 12.00-13.00 sec 25.4 MBytes 213 Mbits/sec [ 5] 13.00-14.00 sec 24.9 MBytes 209 Mbits/sec [ 5] 14.00-15.00 sec 21.4 MBytes 179 Mbits/sec [ 5] 15.00-16.00 sec 32.0 MBytes 268 Mbits/sec [ 5] 16.00-17.00 sec 10.9 MBytes 91.2 Mbits/sec [ 5] 17.00-18.00 sec 27.6 MBytes 232 Mbits/sec [ 5] 18.00-19.00 sec 75.6 MBytes 634 Mbits/sec [ 5] 19.00-20.00 sec 21.1 MBytes 177 Mbits/sec [ 5] 20.00-21.00 sec 109 MBytes 912 Mbits/sec [ 5] 21.00-22.00 sec 23.9 MBytes 200 Mbits/sec [ 5] 22.00-23.00 sec 62.6 MBytes 525 Mbits/sec [ 5] 23.00-24.00 sec 34.5 MBytes 289 Mbits/sec [ 5] 24.00-25.00 sec 54.4 MBytes 456 Mbits/sec [ 5] 25.00-26.00 sec 14.1 MBytes 118 Mbits/sec [ 5] 26.00-27.00 sec 38.8 MBytes 325 Mbits/sec [ 5] 27.00-28.00 sec 95.0 MBytes 797 Mbits/sec [ 5] 28.00-29.00 sec 23.2 MBytes 195 Mbits/sec [ 5] 29.00-30.00 sec 71.9 MBytes 603 Mbits/sec [ 5] 30.00-31.00 sec 28.8 MBytes 241 Mbits/sec [ 5] 31.00-32.00 sec 34.8 MBytes 292 Mbits/sec [ 5] 32.00-33.00 sec 19.4 MBytes 163 Mbits/sec [ 5] 33.00-34.00 sec 39.1 MBytes 328 Mbits/sec [ 5] 34.00-35.00 sec 31.4 MBytes 263 Mbits/sec [ 5] 35.00-36.00 sec 28.2 MBytes 237 Mbits/sec [ 5] 36.00-37.00 sec 48.5 MBytes 407 Mbits/sec [ 5] 37.00-38.00 sec 23.4 MBytes 196 Mbits/sec [ 5] 38.00-39.00 sec 71.4 MBytes 599 Mbits/sec [ 5] 39.00-40.00 sec 41.4 MBytes 347 Mbits/sec [ 5] 40.00-41.00 sec 15.2 MBytes 128 Mbits/sec [ 5] 41.00-42.00 sec 34.1 MBytes 286 Mbits/sec [ 5] 42.00-43.00 sec 31.1 MBytes 261 Mbits/sec [ 5] 43.00-44.00 sec 61.5 MBytes 516 Mbits/sec [ 5] 44.00-45.00 sec 38.2 MBytes 321 Mbits/sec [ 5] 45.00-46.00 sec 34.2 MBytes 287 Mbits/sec [ 5] 46.00-47.00 sec 33.8 MBytes 283 Mbits/sec [ 5] 47.00-48.00 sec 7.12 MBytes 59.8 Mbits/sec [ 5] 48.00-49.00 sec 97.4 MBytes 817 Mbits/sec [ 5] 49.00-50.00 sec 13.6 MBytes 114 Mbits/sec - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bitrate [ 5] 0.00-50.00 sec 55.7 GBytes 9.57 Gbits/sec receiver ----------------------------------------------------------- Server listening on 5201 (test #3) -----------------------------------------------------------

3 4

[PATCH 0/2] Don't expose container loopback services to the host
by David Gibson 26 Sep '24

26 Sep '24

podman issue #24045 pointed out that pasta's spliced forwarding logic can expose services within the namespace bound only to 127.0.0.1 or ::1 to the host. However, the namespace probably expects those to only be accessible to itself, so that's probably not what we want. This changes our forwarding logic so as not to do this. Note that the podman tests will currently fail with this series, I've submitted podman PR https://github.com/containers/podman/pull/24064 to fix that. Link: https://github.com/containers/podman/issues/24045 David Gibson (2): test: Clarify test for spliced inbound transfers fwd: Direct inbound spliced forwards to the guest's external address fwd.c | 24 +++++++++++++++--------- test/passt_in_ns/tcp | 8 ++++---- test/passt_in_ns/udp | 4 ++-- test/pasta/tcp | 16 ++++++++-------- test/pasta/udp | 8 ++++---- 5 files changed, 33 insertions(+), 27 deletions(-) -- 2.46.1

2 5

[PATCH 0/4] Use inany addresses through port forwarding configuration
by David Gibson 25 Sep '24

25 Sep '24

We use inany addresses, which can represent either an IPv4 or IPv6 address heavily through the flow table. However, for the configuration of port forwarding: both the command line parsing and initialization of listening sockets, we use address family+(void *) pairs instead. Using inany through these paths makes them slightly neater now, but more importantly will make it easier to add more robust tracking of forwarding configuration. David Gibson (4): udp: Don't attempt to get dual-stack sockets in nonsensical cases util, pif: Replace sock_l4() with pif_sock_l4() tcp, udp: Make {tcp,udp}_sock_init() take an inany address inany: Add inany_pton() helper conf.c | 23 ++++++++--------------- inany.c | 20 ++++++++++++++++++++ inany.h | 1 + pif.c | 42 ++++++++++++++++++++++++++++++++++++++++++ pif.h | 3 +++ tcp.c | 45 +++++++++++++++++++++++---------------------- tcp.h | 2 +- udp.c | 50 ++++++++++++++++++++++---------------------------- udp.h | 4 ++-- util.c | 52 ---------------------------------------------------- util.h | 3 --- 11 files changed, 122 insertions(+), 123 deletions(-) -- 2.46.1

2 5

[PATCH 0/4] tcp: use csum_iov() in tcp_update_check_tcp[4|6]()
by Laurent Vivier 25 Sep '24

25 Sep '24

For vhost-user, we will need to spread TCP payload over several buffers. To re-use tcp_update_check_tcp[4|6](), provide an iovec rather than a pointer to a buffer. This series updates also csum_iov() and pcap_iov() to add an offset of bytes to skip in the iovec array. It's based on top of "tcp: Use tcp_payload_t rather than tcphdr" that is added in the series for convenience. Laurent Vivier (4): tcp: Use tcp_payload_t rather than tcphdr pcap: Add an offset argument in pcap_iov() checksum: Add an offset argument in csum_iov() tcp: Update TCP checksum using an iovec array checksum.c | 14 ++++-- checksum.h | 3 +- pcap.c | 5 ++- pcap.h | 2 +- tcp.c | 118 ++++++++++++++++++++++++++++++++++--------------- tcp_buf.c | 29 ------------ tcp_internal.h | 29 ++++++++++++ 7 files changed, 128 insertions(+), 72 deletions(-) -- 2.46.0

2 11

[PATCH v3 0/2] tcp: unify IPv4 and IPv6 tap queues
by Jon Maloy 25 Sep '24

25 Sep '24

This should save us some memory and code. --- v2: - Setting pointers to pre-set IP and MAC headers on the fly instead of copying them. - Merged patch #2 and #3 from v1 v3: - Changes based on feedback from team Jon Maloy (2): tcp: set ip and eth headers in l2 tap queues on the fly tcp: unify l2 TCPv4 and TCPv6 queues and structures tcp.c | 6 +- tcp_buf.c | 257 ++++++++++++++++++------------------------------------ tcp_buf.h | 3 +- 3 files changed, 88 insertions(+), 178 deletions(-) -- 2.45.2

3 6

[PATCH v2 0/4] mbuto: Make it work on Arch Linux
by Lukasz Gut 23 Sep '24

23 Sep '24

Hi Stefano, This set of patches makes mbuto work also on Arch Linux. I have changed PATCH 4/4 based on your comments. Thank you for pointing out the correct recepient list. Cheers, Lukasz

2 12

[PATCH v5 0/4] Add vhost-user support to passt. (part 3)
by Laurent Vivier 23 Sep '24

23 Sep '24

This series of patches adds vhost-user support to passt and then allows passt to connect to QEMU network backend using virtqueue rather than a socket. With QEMU, rather than using to connect: -netdev stream,id=s,server=off,addr.type=unix,addr.path=/tmp/passt_1.socket we will use: -chardev socket,id=chr0,path=/tmp/passt_1.socket -netdev vhost-user,id=netdev0,chardev=chr0 -device virtio-net,netdev=netdev0 -object memory-backend-memfd,id=memfd0,share=on,size=$RAMSIZE -numa node,memdev=memfd0 The memory backend is needed to share data between passt and QEMU. Performance comparison between "-netdev stream" and "-netdev vhost-user": $ iperf3 -c localhost -p 10001 -t 60 -6 -u -b 50G socket: [ 5] 0.00-60.05 sec 95.6 GBytes 13.7 Gbits/sec 0.017 ms 6998988/10132413 (69%) receiver vhost-user: [ 5] 0.00-60.04 sec 237 GBytes 33.9 Gbits/sec 0.006 ms 53673/7813770 (0.69%) receiver $ iperf3 -c localhost -p 10001 -t 60 -4 -u -b 50G socket: [ 5] 0.00-60.05 sec 98.9 GBytes 14.1 Gbits/sec 0.018 ms 6260735/9501832 (66%) receiver vhost-user: [ 5] 0.00-60.05 sec 235 GBytes 33.7 Gbits/sec 0.008 ms 37581/7752699 (0.48%) receiver $ iperf3 -c localhost -p 10001 -t 60 -6 socket: [ 5] 0.00-60.00 sec 17.3 GBytes 2.48 Gbits/sec 0 sender [ 5] 0.00-60.06 sec 17.3 GBytes 2.48 Gbits/sec receiver vhost-user: [ 5] 0.00-60.00 sec 191 GBytes 27.4 Gbits/sec 0 sender [ 5] 0.00-60.05 sec 191 GBytes 27.3 Gbits/sec receiver $ iperf3 -c localhost -p 10001 -t 60 -4 socket: [ 5] 0.00-60.00 sec 15.6 GBytes 2.24 Gbits/sec 0 sender [ 5] 0.00-60.06 sec 15.6 GBytes 2.24 Gbits/sec receiver vhost-user: [ 5] 0.00-60.00 sec 189 GBytes 27.1 Gbits/sec 0 sender [ 5] 0.00-60.04 sec 189 GBytes 27.0 Gbits/sec receiver v5: - rebase on top of 2024_09_06.6b38f07 - rework udp_vu.c as ref.udp.v6 has been removed and we need to know if we receive IPv4 or IPv6 frame when we prepare the guest buffers for recvmsg() - remove vnet->hdrlen as the size is always the same with virtio-net v1 - address comments from David and Stefano v4: - rebase on top of 2024_08_21.1d6142f (rebasing on top of 620e19a1b48a ("udp: Merge udp[46]_mh_recv arrays") introduces a regression in the measure of the latency with UDP because I think I don't replace correctly ref.udp.v6 that is removed by this commit) - Addressed most of the comments from David and Stefano (I didn't want to postpone this version to next week, so I'll address the remaining comments in the next version). v3: - rebase on top of flow table - update tcp_vu.c to look like udp_vu.c (recv()/prepare()/send_frame()) - address comments from Stefano and David on version 2 v2: - remove PATCH 4 - rewrite PATCH 2 and 3 to follow passt coding style - move some code from PATCH 3 to PATCH 4 (previously PATCH 5) - partially addressed David's comment on PATCH 5 Laurent Vivier (4): packet: replace struct desc by struct iovec vhost-user: introduce virtio API vhost-user: introduce vhost-user API vhost-user: add vhost-user Makefile | 6 +- checksum.c | 1 - conf.c | 23 +- epoll_type.h | 4 + iov.c | 1 - isolation.c | 17 +- packet.c | 91 ++-- packet.h | 22 +- passt.1 | 10 +- passt.c | 26 +- passt.h | 6 + pcap.c | 1 - tap.c | 111 ++++- tap.h | 5 +- tcp.c | 31 +- tcp_buf.c | 8 +- tcp_internal.h | 3 +- tcp_vu.c | 647 +++++++++++++++++++++++++ tcp_vu.h | 12 + udp.c | 78 +-- udp.h | 8 +- udp_internal.h | 34 ++ udp_vu.c | 397 +++++++++++++++ udp_vu.h | 13 + util.h | 8 + vhost_user.c | 1247 ++++++++++++++++++++++++++++++++++++++++++++++++ vhost_user.h | 209 ++++++++ virtio.c | 659 +++++++++++++++++++++++++ virtio.h | 184 +++++++ vu_common.c | 36 ++ vu_common.h | 34 ++ 31 files changed, 3792 insertions(+), 140 deletions(-) create mode 100644 tcp_vu.c create mode 100644 tcp_vu.h create mode 100644 udp_internal.h create mode 100644 udp_vu.c create mode 100644 udp_vu.h create mode 100644 vhost_user.c create mode 100644 vhost_user.h create mode 100644 virtio.c create mode 100644 virtio.h create mode 100644 vu_common.c create mode 100644 vu_common.h -- 2.46.0

3 9

[PATCH] tcp: Use tcp_payload_t rather than tcphdr
by Laurent Vivier 23 Sep '24

23 Sep '24

As tcp_update_check_tcp4() and tcp_update_check_tcp6() compute the checksum using the TCP header and the TCP payload, it is clearer to use a pointer to tcp_payload_t that includes tcphdr and payload rather than a pointer to tcphdr (and guessing TCP header is followed by the payload). Move tcp_payload_t and tcp_flags_t to tcp_internal.h. (They will be used also by vhost-user). Signed-off-by: Laurent Vivier <lvivier(a)redhat.com> --- tcp.c | 42 ++++++++++++++++++++++-------------------- tcp_buf.c | 29 ----------------------------- tcp_internal.h | 29 +++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 49 deletions(-) diff --git a/tcp.c b/tcp.c index 1962fcc469ed..c9472d905520 100644 --- a/tcp.c +++ b/tcp.c @@ -757,32 +757,34 @@ static void tcp_sock_set_bufsize(const struct ctx *c, int s) /** * tcp_update_check_tcp4() - Update TCP checksum from stored one * @iph: IPv4 header - * @th: TCP header followed by TCP payload + * @bp: TCP header followed by TCP payload */ -static void tcp_update_check_tcp4(const struct iphdr *iph, struct tcphdr *th) +static void tcp_update_check_tcp4(const struct iphdr *iph, + struct tcp_payload_t *bp) { uint16_t l4len = ntohs(iph->tot_len) - sizeof(struct iphdr); struct in_addr saddr = { .s_addr = iph->saddr }; struct in_addr daddr = { .s_addr = iph->daddr }; uint32_t sum = proto_ipv4_header_psum(l4len, IPPROTO_TCP, saddr, daddr); - th->check = 0; - th->check = csum(th, l4len, sum); + bp->th.check = 0; + bp->th.check = csum(bp, l4len, sum); } /** * tcp_update_check_tcp6() - Calculate TCP checksum for IPv6 * @ip6h: IPv6 header - * @th: TCP header followed by TCP payload + * @bp: TCP header followed by TCP payload */ -static void tcp_update_check_tcp6(struct ipv6hdr *ip6h, struct tcphdr *th) +static void tcp_update_check_tcp6(const struct ipv6hdr *ip6h, + struct tcp_payload_t *bp) { uint16_t l4len = ntohs(ip6h->payload_len); uint32_t sum = proto_ipv6_header_psum(l4len, IPPROTO_TCP, &ip6h->saddr, &ip6h->daddr); - th->check = 0; - th->check = csum(th, l4len, sum); + bp->th.check = 0; + bp->th.check = csum(bp, l4len, sum); } /** @@ -902,7 +904,7 @@ static void tcp_fill_header(struct tcphdr *th, * @conn: Connection pointer * @taph: tap backend specific header * @iph: Pointer to IPv4 header - * @th: Pointer to TCP header + * @bp: Pointer to TCP header followed by TCP payload * @dlen: TCP payload length * @check: Checksum, if already known * @seq: Sequence number for this segment @@ -912,14 +914,14 @@ static void tcp_fill_header(struct tcphdr *th, */ static size_t tcp_fill_headers4(const struct tcp_tap_conn *conn, struct tap_hdr *taph, - struct iphdr *iph, struct tcphdr *th, + struct iphdr *iph, struct tcp_payload_t *bp, size_t dlen, const uint16_t *check, uint32_t seq, bool no_tcp_csum) { const struct flowside *tapside = TAPFLOW(conn); const struct in_addr *src4 = inany_v4(&tapside->oaddr); const struct in_addr *dst4 = inany_v4(&tapside->eaddr); - size_t l4len = dlen + sizeof(*th); + size_t l4len = dlen + sizeof(bp->th); size_t l3len = l4len + sizeof(*iph); ASSERT(src4 && dst4); @@ -931,12 +933,12 @@ static size_t tcp_fill_headers4(const struct tcp_tap_conn *conn, iph->check = check ? *check : csum_ip4_header(l3len, IPPROTO_TCP, *src4, *dst4); - tcp_fill_header(th, conn, seq); + tcp_fill_header(&bp->th, conn, seq); if (no_tcp_csum) - th->check = 0; + bp->th.check = 0; else - tcp_update_check_tcp4(iph, th); + tcp_update_check_tcp4(iph, bp); tap_hdr_update(taph, l3len + sizeof(struct ethhdr)); @@ -948,7 +950,7 @@ static size_t tcp_fill_headers4(const struct tcp_tap_conn *conn, * @conn: Connection pointer * @taph: tap backend specific header * @ip6h: Pointer to IPv6 header - * @th: Pointer to TCP header + * @bp: Pointer to TCP header followed by TCP payload * @dlen: TCP payload length * @check: Checksum, if already known * @seq: Sequence number for this segment @@ -958,11 +960,11 @@ static size_t tcp_fill_headers4(const struct tcp_tap_conn *conn, */ static size_t tcp_fill_headers6(const struct tcp_tap_conn *conn, struct tap_hdr *taph, - struct ipv6hdr *ip6h, struct tcphdr *th, + struct ipv6hdr *ip6h, struct tcp_payload_t *bp, size_t dlen, uint32_t seq, bool no_tcp_csum) { const struct flowside *tapside = TAPFLOW(conn); - size_t l4len = dlen + sizeof(*th); + size_t l4len = dlen + sizeof(bp->th); ip6h->payload_len = htons(l4len); ip6h->saddr = tapside->oaddr.a6; @@ -976,12 +978,12 @@ static size_t tcp_fill_headers6(const struct tcp_tap_conn *conn, ip6h->flow_lbl[1] = (conn->sock >> 8) & 0xff; ip6h->flow_lbl[2] = (conn->sock >> 0) & 0xff; - tcp_fill_header(th, conn, seq); + tcp_fill_header(&bp->th, conn, seq); if (no_tcp_csum) - th->check = 0; + bp->th.check = 0; else - tcp_update_check_tcp6(ip6h, th); + tcp_update_check_tcp6(ip6h, bp); tap_hdr_update(taph, l4len + sizeof(*ip6h) + sizeof(struct ethhdr)); diff --git a/tcp_buf.c b/tcp_buf.c index ffbff5e4b485..238827b01d90 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -38,35 +38,6 @@ (c->mode == MODE_PASTA ? 1 : TCP_FRAMES_MEM) /* Static buffers */ -/** - * struct tcp_payload_t - TCP header and data to send segments with payload - * @th: TCP header - * @data: TCP data - */ -struct tcp_payload_t { - struct tcphdr th; - uint8_t data[IP_MAX_MTU - sizeof(struct tcphdr)]; -#ifdef __AVX2__ -} __attribute__ ((packed, aligned(32))); /* For AVX2 checksum routines */ -#else -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))); -#endif - -/** - * struct tcp_flags_t - TCP header and data to send zero-length - * segments (flags) - * @th: TCP header - * @opts TCP options - */ -struct tcp_flags_t { - struct tcphdr th; - char opts[OPT_MSS_LEN + OPT_WS_LEN + 1]; -#ifdef __AVX2__ -} __attribute__ ((packed, aligned(32))); -#else -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))); -#endif - /* Ethernet header for IPv4 frames */ static struct ethhdr tcp4_eth_src; diff --git a/tcp_internal.h b/tcp_internal.h index de06db1438d6..2f74ffeff8f3 100644 --- a/tcp_internal.h +++ b/tcp_internal.h @@ -63,6 +63,35 @@ enum tcp_iov_parts { TCP_NUM_IOVS }; +/** + * struct tcp_payload_t - TCP header and data to send segments with payload + * @th: TCP header + * @data: TCP data + */ +struct tcp_payload_t { + struct tcphdr th; + uint8_t data[IP_MAX_MTU - sizeof(struct tcphdr)]; +#ifdef __AVX2__ +} __attribute__ ((packed, aligned(32))); /* For AVX2 checksum routines */ +#else +} __attribute__ ((packed, aligned(__alignof__(unsigned int)))); +#endif + +/** + * struct tcp_flags_t - TCP header and data to send zero-length + * segments (flags) + * @th: TCP header + * @opts TCP options + */ +struct tcp_flags_t { + struct tcphdr th; + char opts[OPT_MSS_LEN + OPT_WS_LEN + 1]; +#ifdef __AVX2__ +} __attribute__ ((packed, aligned(32))); +#else +} __attribute__ ((packed, aligned(__alignof__(unsigned int)))); +#endif + extern char tcp_buf_discard [MAX_WINDOW]; void conn_flag_do(const struct ctx *c, struct tcp_tap_conn *conn, -- 2.46.0

2 1

[PATCH] tcp: unify payload and flags l2 frames array
by Jon Maloy 23 Sep '24

23 Sep '24

In order to reduce memory and code footprint, we merge the array for l2 flag frames into the one for payload frames. This change also ensures that no flag message will be sent out over the l2 media bypassing already queued payload messages. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- tcp.c | 3 +-- tcp_buf.c | 70 ++++++++++++------------------------------------------- 2 files changed, 16 insertions(+), 57 deletions(-) diff --git a/tcp.c b/tcp.c index 12e864f..dd060d0 100644 --- a/tcp.c +++ b/tcp.c @@ -868,7 +868,6 @@ bool tcp_flow_defer(const struct tcp_tap_conn *conn) /* cppcheck-suppress [constParameterPointer, unmatchedSuppression] */ void tcp_defer_handler(struct ctx *c) { - tcp_flags_flush(c); tcp_payload_flush(c); } @@ -1148,7 +1147,7 @@ static void tcp_update_seqack_from_tap(const struct ctx *c, * 1 otherwise */ int tcp_prepare_flags(const struct ctx *c, struct tcp_tap_conn *conn, - int flags, struct tcphdr *th, char *data, + int flags, struct tcphdr *th, uint8_t *data, size_t *optlen) { struct tcp_info tinfo = { 0 }; diff --git a/tcp_buf.c b/tcp_buf.c index 625c29c..58636d1 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -33,6 +33,7 @@ #include "tcp_internal.h" #include "tcp_buf.h" +#define PAYLOAD_FLAGS htons(0x5010) /* doff = 5, ack = 1 */ #define TCP_FRAMES_MEM 128 #define TCP_FRAMES \ (c->mode == MODE_PASTA ? 1 : TCP_FRAMES_MEM) @@ -52,21 +53,6 @@ struct tcp_payload_t { } __attribute__ ((packed, aligned(__alignof__(unsigned int)))); #endif -/** - * struct tcp_flags_t - TCP header and data to send zero-length - * segments (flags) - * @th: TCP header - * @opts TCP options - */ -struct tcp_flags_t { - struct tcphdr th; - char opts[OPT_MSS_LEN + OPT_WS_LEN + 1]; -#ifdef __AVX2__ -} __attribute__ ((packed, aligned(32))); -#else -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))); -#endif - /* Ethernet header for IPv4 and IPv6 frames */ static struct ethhdr tcp4_eth_src; static struct ethhdr tcp6_eth_src; @@ -87,22 +73,10 @@ static_assert(MSS6 <= sizeof(tcp_payload[0].data), "MSS6 is greater than 65516") static struct tcp_tap_conn *tcp_frame_conns[TCP_FRAMES_MEM]; static unsigned int tcp_payload_used; -static struct tap_hdr tcp_flags_tap_hdr[TCP_FRAMES_MEM]; -/* IPv4 headers for TCP segment without payload */ -static struct iphdr tcp4_flags_ip[TCP_FRAMES_MEM]; -/* TCP segments without payload for IPv4 frames */ -static struct tcp_flags_t tcp_flags[TCP_FRAMES_MEM]; - -static unsigned int tcp_flags_used; - -/* IPv6 headers for TCP segment without payload */ -static struct ipv6hdr tcp6_flags_ip[TCP_FRAMES_MEM]; - /* recvmsg()/sendmsg() data for tap */ static struct iovec iov_sock [TCP_FRAMES_MEM + 1]; static struct iovec tcp_l2_iov[TCP_FRAMES_MEM][TCP_NUM_IOVS]; -static struct iovec tcp_l2_flags_iov[TCP_FRAMES_MEM][TCP_NUM_IOVS]; /** * tcp_update_l2_buf() - Update Ethernet header buffers with addresses @@ -135,13 +109,6 @@ void tcp_sock_iov_init(const struct ctx *c) tcp_payload[i].th.ack = 1; } - for (i = 0; i < ARRAY_SIZE(tcp_flags); i++) { - tcp6_flags_ip[i] = ip6; - tcp4_flags_ip[i] = iph; - tcp_flags[i].th.doff = sizeof(struct tcphdr) / 4; - tcp_flags[i].th.ack = 1; - } - for (i = 0; i < TCP_FRAMES_MEM; i++) { struct iovec *iov = tcp_l2_iov[i]; @@ -149,14 +116,6 @@ void tcp_sock_iov_init(const struct ctx *c) iov[TCP_IOV_ETH].iov_len = sizeof(struct ethhdr); iov[TCP_IOV_PAYLOAD].iov_base = &tcp_payload[i]; } - - for (i = 0; i < TCP_FRAMES_MEM; i++) { - struct iovec *iov = tcp_l2_flags_iov[i]; - - iov[TCP_IOV_TAP] = tap_hdr_iov(c, &tcp_flags_tap_hdr[i]); - iov[TCP_IOV_ETH].iov_len = sizeof(struct ethhdr); - iov[TCP_IOV_PAYLOAD].iov_base = &tcp_flags[i]; - } } /** @@ -165,9 +124,7 @@ void tcp_sock_iov_init(const struct ctx *c) */ void tcp_flags_flush(const struct ctx *c) { - tap_send_frames(c, &tcp_l2_flags_iov[0][0], TCP_NUM_IOVS, - tcp_flags_used); - tcp_flags_used = 0; + tcp_payload_flush(c); } /** @@ -225,37 +182,35 @@ void tcp_payload_flush(const struct ctx *c) */ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) { - struct tcp_flags_t *payload; + struct tcp_payload_t *payload; struct iovec *iov; size_t optlen; size_t l4len; uint32_t seq; int ret; - iov = tcp_l2_flags_iov[tcp_flags_used]; + iov = tcp_l2_iov[tcp_payload_used]; if (CONN_V4(conn)) { - iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp4_flags_ip[tcp_flags_used]); + iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp4_payload_ip[tcp_payload_used]); iov[TCP_IOV_ETH].iov_base = &tcp4_eth_src; } else { - iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_flags_ip[tcp_flags_used]); + iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_payload_ip[tcp_payload_used]); iov[TCP_IOV_ETH].iov_base = &tcp6_eth_src; } payload = iov[TCP_IOV_PAYLOAD].iov_base; seq = conn->seq_to_tap; ret = tcp_prepare_flags(c, conn, flags, &payload->th, - payload->opts, &optlen); + payload->data, &optlen); if (ret <= 0) return ret; - tcp_flags_used++; + tcp_payload_used++; l4len = tcp_l2_buf_fill_headers(conn, iov, optlen, NULL, seq, false); iov[TCP_IOV_PAYLOAD].iov_len = l4len; - if (flags & DUP_ACK) { - struct iovec *dup_iov; + struct iovec *dup_iov = tcp_l2_iov[tcp_payload_used++]; - dup_iov = tcp_l2_flags_iov[tcp_flags_used++]; memcpy(dup_iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_len); dup_iov[TCP_IOV_ETH].iov_base = iov[TCP_IOV_ETH].iov_base; @@ -265,7 +220,7 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) dup_iov[TCP_IOV_PAYLOAD].iov_len = l4len; } - if (tcp_flags_used > TCP_FRAMES_MEM - 2) + if (tcp_payload_used > TCP_FRAMES_MEM - 2) tcp_flags_flush(c); return 0; @@ -282,8 +237,10 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, ssize_t dlen, int no_csum, uint32_t seq) { + struct tcp_payload_t *payload; const uint16_t *check = NULL; struct iovec *iov; + uint16_t *flags; size_t l4len; conn->seq_to_tap = seq + dlen; @@ -302,6 +259,9 @@ static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_payload_ip[tcp_payload_used]); iov[TCP_IOV_ETH].iov_base = &tcp6_eth_src; } + payload = iov[TCP_IOV_PAYLOAD].iov_base; + flags = &payload->th.window - 1; + *(flags) = PAYLOAD_FLAGS; l4len = tcp_l2_buf_fill_headers(conn, iov, dlen, check, seq, false); iov[TCP_IOV_PAYLOAD].iov_len = l4len; if (++tcp_payload_used > TCP_FRAMES_MEM - 1) -- 2.45.2

3 2

[PATCH v3 0/2] util: Fix some problems in write_remainder()
by David Gibson 19 Sep '24

19 Sep '24

With Markus Armbruster's help I re-examined the Coverity warning we've had in this function for a while. While I still don't understand some steps in the report it's giving, I did spot some real problems in the vicinity. It turns out fixing those also fixes the Coverity warning, so here we go. Changes since v2: * Handle EINTR within write_all_buf() (suggested by Markus Armbruster) David Gibson (2): util: Add helper to write() all of a buffer util: Remove possible quadratic behaviour from write_remainder() pcap.c | 3 +-- util.c | 50 +++++++++++++++++++++++++++++++++++++++++--------- util.h | 1 + 3 files changed, 43 insertions(+), 11 deletions(-) -- 2.46.0

3 6