From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 38CCB5A026F for ; Thu, 14 Mar 2024 05:22:33 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1710390149; bh=lUQuZ5DJuZKaZcN3f4J+b/U80jA0jLOk+RCEzvmo2Y8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=kf8WObFh+x0iFZjjke0g9sbZXFxYkiOjGutpBr3Mla9p6alAsWe8qtgfdaL6Bbd/y FM8fgtjUAkw/kq/1rwfdjeXxkb2yf8nHUnvviGwuFq2F4KHLn+UhqYydxzP3k7N9pc OxUY1xSSTlGDopeys7tPt3KxgETSTrZ2heBRexlVIGYId25R+YnQE3KdDmNVr8CRt7 mkJhrNDaRZ4hf0vLUhd+92d+QnwLX8z1l/ImBRLUc14EonZgPUeAn/00EG5hBJB1yv o2w8j9pezl/oAFUB6VKIiuDax2yw0I9HkcvOT5Oc+nqzIfIoU10AhoCS5nJxFA889T Sa9kZICvjE63A== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4TwDkF2TcMz4wcN; Thu, 14 Mar 2024 15:22:29 +1100 (AEDT) Date: Thu, 14 Mar 2024 15:22:15 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [RFC] tcp: Replace TCP buffer structure by an iovec array Message-ID: References: <20240311133356.1405001-1-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="lJjIZHHEtvjAnGBT" Content-Disposition: inline In-Reply-To: <20240311133356.1405001-1-lvivier@redhat.com> Message-ID-Hash: CQSPI26HJQJOTOCRV3OS2UF3BAOKGPOG X-Message-ID-Hash: CQSPI26HJQJOTOCRV3OS2UF3BAOKGPOG X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --lJjIZHHEtvjAnGBT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Mar 11, 2024 at 02:33:56PM +0100, Laurent Vivier wrote: > To be able to provide pointers to TCP headers and IP headers without > worrying about alignment in the structure, split the structure into > several arrays and point to each part of the frame using an iovec array. >=20 > Using iovec also allows us to simply ignore the first entry when the > vnet length header is not needed. And as the payload buffer contains > only the TCP header and the TCP data we can increase the size of the > TCP data to USHRT_MAX - sizeof(struct tcphdr). >=20 > As a side effect, these changes improve performance by a factor of > x1.5. >=20 > Signed-off-by: Laurent Vivier > --- > tap.c | 104 ++++++++++++++ > tap.h | 16 +++ > tcp.c | 429 ++++++++++++++++++++++++---------------------------------- > 3 files changed, 296 insertions(+), 253 deletions(-) >=20 > diff --git a/tap.c b/tap.c > index c7b9372668ec..afd01cffbed6 100644 > --- a/tap.c > +++ b/tap.c > @@ -350,6 +350,30 @@ static size_t tap_send_frames_pasta(const struct ctx= *c, > return i; > } > =20 > +/** > + * tap_send_iov_pasta() - Send out multiple prepared frames > + * @c: Execution context > + * @iov: Array of frames, each frames is divided in an array of iovecs. > + * The first entry of the iovec is ignored > + * @n: Number of frames in @iov > + * > + * Return: number of frames actually sent > + */ > +static size_t tap_send_iov_pasta(const struct ctx *c, > + struct iovec iov[][TCP_IOV_NUM], size_t n) As Stefano also points out, this isn't quite right. At the very least the names are misleading, and I'm pretty sure it also won't work right: the tuntap interface requires a whole frame to be given with a single write()/writev(), you can't split it into multiple writes. I also think it's kind of a layering violation to refer to TCP_IOV_NUM here, so I think the number of iovecs per frame should be a parameter. Which.. is basically what I already did in the tap rework series I already sent. I think it should be pretty straightforward to rebase the TCP part of this patch on top of that, and drop the tap part. > +{ > + unsigned int i; > + > + for (i =3D 0; i < n; i++) { > + if (!tap_send_frames_pasta(c, &iov[i][TCP_IOV_ETH], > + TCP_IOV_NUM - TCP_IOV_ETH)) > + break; > + } > + > + return i; > + > +} > + > /** > * tap_send_frames_passt() - Send multiple frames to the passt tap > * @c: Execution context > @@ -390,6 +414,42 @@ static size_t tap_send_frames_passt(const struct ctx= *c, > return i; > } > =20 > +/** > + * tap_send_iov_passt() - Send out multiple prepared frames > + * @c: Execution context > + * @iov: Array of frames, each frames is divided in an array of iovecs. > + * The first entry of the iovec is updated to point to an > + * uint32_t storing the frame length. > + * @n: Number of frames in @iov > + * > + * Return: number of frames actually sent > + */ > +static size_t tap_send_iov_passt(const struct ctx *c, > + struct iovec iov[][TCP_IOV_NUM], > + size_t n) > +{ > + unsigned int i; > + > + for (i =3D 0; i < n; i++) { > + uint32_t vnet_len; > + int j; > + > + vnet_len =3D 0; > + for (j =3D TCP_IOV_ETH; j < TCP_IOV_NUM; j++) > + vnet_len +=3D iov[i][j].iov_len; > + > + vnet_len =3D htonl(vnet_len); > + iov[i][TCP_IOV_VNET].iov_base =3D &vnet_len; > + iov[i][TCP_IOV_VNET].iov_len =3D sizeof(vnet_len); > + > + if (!tap_send_frames_passt(c, iov[i], TCP_IOV_NUM)) > + break; > + } > + > + return i; > + > +} > + > /** > * tap_send_frames() - Send out multiple prepared frames > * @c: Execution context > @@ -418,6 +478,50 @@ size_t tap_send_frames(const struct ctx *c, const st= ruct iovec *iov, size_t n) > return m; > } > =20 > +/** > + * tap_send_iov() - Send out multiple prepared frames > + * @c: Execution context > + * @iov: Array of frames, each frames is divided in an array of iovecs. > + * iovec array is: As per Stefano, I think making these constants an enum and moving this description there would be preferable. > + * TCP_IOV_VNET (0) vnet length Maybe call this TCP_IOV_TAP, to more easily to generalise to any TAP backend specific headers we might want in future? > + * TCP_IOV_ETH (1) ethernet header > + * TCP_IOV_IP (2) IP (v4/v6) header > + * TCP_IOV_PAYLOAD (3) IP payload (TCP header + data) > + * TCP_IOV_NUM (4) is the number of entries in the iovec array > + * TCP_IOV_VNET entry is updated with passt, ignored with pasta. > + * @n: Number of frames in @iov > + * > + * Return: number of frames actually sent > + */ > +size_t tap_send_iov(const struct ctx *c, struct iovec iov[][TCP_IOV_NUM], > + size_t n) > +{ > + size_t m; > + unsigned int i; > + > + if (!n) > + return 0; > + > + switch (c->mode) { > + case MODE_PASST: > + m =3D tap_send_iov_passt(c, iov, n); > + break; > + case MODE_PASTA: > + m =3D tap_send_iov_pasta(c, iov, n); > + break; > + default: > + ASSERT(0); > + } > + > + if (m < n) > + debug("tap: failed to send %zu frames of %zu", n - m, n); > + > + for (i =3D 0; i < m; i++) > + pcap_iov(&iov[i][TCP_IOV_ETH], TCP_IOV_NUM - TCP_IOV_ETH); > + > + return m; > +} > + > /** > * eth_update_mac() - Update tap L2 header with new Ethernet addresses > * @eh: Ethernet headers to update > diff --git a/tap.h b/tap.h > index 437b9aa2b43f..2d8e7aa1101d 100644 > --- a/tap.h > +++ b/tap.h > @@ -6,6 +6,20 @@ > #ifndef TAP_H > #define TAP_H > =20 > +/* > + * TCP frame iovec array: > + * TCP_IOV_VNET vnet length > + * TCP_IOV_ETH ethernet header > + * TCP_IOV_IP IP (v4/v6) header > + * TCP_IOV_PAYLOAD IP payload (TCP header + data) > + * TCP_IOV_NUM is the number of entries in the iovec array > + */ > +#define TCP_IOV_VNET 0 > +#define TCP_IOV_ETH 1 > +#define TCP_IOV_IP 2 > +#define TCP_IOV_PAYLOAD 3 > +#define TCP_IOV_NUM 4 As with using TCP_IOV_NUM etc. inside tap.c, defining these in tap.h rather than tcp.h seems odd to me. > + > /** > * struct tap_hdr - L2 and tap specific headers > * @vnet_len: Frame length (for qemu socket transport) > @@ -74,6 +88,8 @@ void tap_icmp6_send(const struct ctx *c, > const void *in, size_t len); > int tap_send(const struct ctx *c, const void *data, size_t len); > size_t tap_send_frames(const struct ctx *c, const struct iovec *iov, siz= e_t n); > +size_t tap_send_iov(const struct ctx *c, struct iovec iov[][TCP_IOV_NUM], > + size_t n); > void eth_update_mac(struct ethhdr *eh, > const unsigned char *eth_d, const unsigned char *eth_s); > void tap_listen_handler(struct ctx *c, uint32_t events); > diff --git a/tcp.c b/tcp.c > index d5eedf4d0138..5c8488108ef7 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -318,39 +318,7 @@ > =20 > /* MSS rounding: see SET_MSS() */ > #define MSS_DEFAULT 536 > - > -struct tcp4_l2_head { /* For MSS4 macro: keep in sync with tcp4_l2_buf_t= */ > -#ifdef __AVX2__ > - uint8_t pad[26]; > -#else > - uint8_t pad[2]; > -#endif > - struct tap_hdr taph; > - struct iphdr iph; > - struct tcphdr th; > -#ifdef __AVX2__ > -} __attribute__ ((packed, aligned(32))); > -#else > -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))); > -#endif > - > -struct tcp6_l2_head { /* For MSS6 macro: keep in sync with tcp6_l2_buf_t= */ > -#ifdef __AVX2__ > - uint8_t pad[14]; > -#else > - uint8_t pad[2]; > -#endif > - struct tap_hdr taph; > - struct ipv6hdr ip6h; > - struct tcphdr th; > -#ifdef __AVX2__ > -} __attribute__ ((packed, aligned(32))); > -#else > -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))); > -#endif > - > -#define MSS4 ROUND_DOWN(USHRT_MAX - sizeof(struct tcp4_l2_head), 4) > -#define MSS6 ROUND_DOWN(USHRT_MAX - sizeof(struct tcp6_l2_head), 4) > +#define MSS (USHRT_MAX - sizeof(struct tcphdr)) As Stefano points out, the buffer size isn't the only limit on the MSS here, so this isn't quite right. In particular there's the 16-bit length field in the IPv4 header. I think making the *buffers* be exactly 64k is a good idea (page aligned on nearly everything), but the MSS has to be clamped a bit smaller. > #define WINDOW_DEFAULT 14600 /* RFC 6928 */ > #ifdef HAS_SND_WND > @@ -445,133 +413,78 @@ struct tcp_buf_seq_update { > }; > =20 > /* Static buffers */ > - > /** > - * tcp4_l2_buf_t - Pre-cooked IPv4 packet buffers for tap connections > - * @pad: Align TCP header to 32 bytes, for AVX2 checksum calculation only > - * @taph: Tap-level headers (partially pre-filled) > - * @iph: Pre-filled IP header (except for tot_len and saddr) > - * @uh: Headroom for TCP header > - * @data: Storage for TCP payload > + * tcp_l2_flags_t - TCP header and data to send option flags > + * @th: TCP header > + * @opts TCP option flags > */ > -static struct tcp4_l2_buf_t { > -#ifdef __AVX2__ > - uint8_t pad[26]; /* 0, align th to 32 bytes */ > -#else > - uint8_t pad[2]; /* align iph to 4 bytes 0 */ > -#endif > - struct tap_hdr taph; /* 26 2 */ > - struct iphdr iph; /* 44 20 */ > - struct tcphdr th; /* 64 40 */ > - uint8_t data[MSS4]; /* 84 60 */ > - /* 65536 65532 */ > +struct tcp_l2_flags_t { > + struct tcphdr th; > + char opts[OPT_MSS_LEN + OPT_WS_LEN + 1]; > +}; > +/** > + * tcp_l2_payload_t - TCP header and data to send data > + * 32 bytes aligned to be able to use AVX2 checksum > + * @th: TCP header > + * @data: TCP data > + */ > +struct tcp_l2_payload_t { > + struct tcphdr th; /* 20 bytes */ > + uint8_t data[MSS]; /* 65516 bytes */ Actually 65515 bytes - USHRT_MAX is 65535, not 65536. I think it might be an idea to explicitly set this length to 65536 - sizeof(tcphdr), then static_assert() to make sure our calculated MSS values will fit in here. > #ifdef __AVX2__ > -} __attribute__ ((packed, aligned(32))) > +} __attribute__ ((packed, aligned(32))); > #else > -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))) > +} __attribute__ ((packed, aligned(__alignof__(unsigned int)))); > #endif > -tcp4_l2_buf[TCP_FRAMES_MEM]; > =20 > -static struct tcp_buf_seq_update tcp4_l2_buf_seq_update[TCP_FRAMES_MEM]; > +/* Ethernet header for IPv4 frames */ > +static struct ethhdr tcp4_eth_src; > =20 > +/* IPv4 headers */ > +static struct iphdr tcp4_l2_ip[TCP_FRAMES_MEM]; > +/* TCP headers and data for IPv4 frames */ > +static struct tcp_l2_payload_t tcp4_l2_payload[TCP_FRAMES_MEM]; > + > +static struct tcp_buf_seq_update tcp4_l2_buf_seq_update[TCP_FRAMES_MEM]; > static unsigned int tcp4_l2_buf_used; > =20 > -/** > - * tcp6_l2_buf_t - Pre-cooked IPv6 packet buffers for tap connections > - * @pad: Align IPv6 header for checksum calculation to 32B (AVX2) or 4B > - * @taph: Tap-level headers (partially pre-filled) > - * @ip6h: Pre-filled IP header (except for payload_len and addresses) > - * @th: Headroom for TCP header > - * @data: Storage for TCP payload > - */ > -struct tcp6_l2_buf_t { > -#ifdef __AVX2__ > - uint8_t pad[14]; /* 0 align ip6h to 32 bytes */ > -#else > - uint8_t pad[2]; /* align ip6h to 4 bytes 0 */ > -#endif > - struct tap_hdr taph; /* 14 2 */ > - struct ipv6hdr ip6h; /* 32 20 */ > - struct tcphdr th; /* 72 60 */ > - uint8_t data[MSS6]; /* 92 80 */ > - /* 65536 65532 */ > -#ifdef __AVX2__ > -} __attribute__ ((packed, aligned(32))) > -#else > -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))) > -#endif > -tcp6_l2_buf[TCP_FRAMES_MEM]; > +/* IPv4 headers for TCP option flags frames */ > +static struct iphdr tcp4_l2_flags_ip[TCP_FRAMES_MEM]; > +/* TCP headers and option flags for IPv4 frames */ > +static struct tcp_l2_flags_t tcp4_l2_flags[TCP_FRAMES_MEM]; > =20 > -static struct tcp_buf_seq_update tcp6_l2_buf_seq_update[TCP_FRAMES_MEM]; > +static unsigned int tcp4_l2_flags_buf_used; > + > +/* Ethernet header for IPv6 frames */ > +static struct ethhdr tcp6_eth_src; > =20 > +/* IPv6 headers */ > +static struct ipv6hdr tcp6_l2_ip[TCP_FRAMES_MEM]; > +/* TCP headers and data for IPv6 frames */ > +static struct tcp_l2_payload_t tcp6_l2_payload[TCP_FRAMES_MEM]; We should be able to unify this with the IPv4 payload buffers (and I think we'll want to to allow v4<->v6 NAT). Makes sense to do that as a later followup, though. > + > +static struct tcp_buf_seq_update tcp6_l2_buf_seq_update[TCP_FRAMES_MEM]; > static unsigned int tcp6_l2_buf_used; > =20 > +/* IPv6 headers for TCP option flags frames */ > +static struct ipv6hdr tcp6_l2_flags_ip[TCP_FRAMES_MEM]; > +/* TCP headers and option flags for IPv6 frames */ > +static struct tcp_l2_flags_t tcp6_l2_flags[TCP_FRAMES_MEM]; > + > +static unsigned int tcp6_l2_flags_buf_used; > + > /* recvmsg()/sendmsg() data for tap */ > static char tcp_buf_discard [MAX_WINDOW]; > static struct iovec iov_sock [TCP_FRAMES_MEM + 1]; > =20 > -static struct iovec tcp4_l2_iov [TCP_FRAMES_MEM]; > -static struct iovec tcp6_l2_iov [TCP_FRAMES_MEM]; > -static struct iovec tcp4_l2_flags_iov [TCP_FRAMES_MEM]; > -static struct iovec tcp6_l2_flags_iov [TCP_FRAMES_MEM]; > +static struct iovec tcp4_l2_iov [TCP_FRAMES_MEM][TCP_IOV_NUM]; > +static struct iovec tcp6_l2_iov [TCP_FRAMES_MEM][TCP_IOV_NUM]; > +static struct iovec tcp4_l2_flags_iov [TCP_FRAMES_MEM][TCP_IOV_NUM]; > +static struct iovec tcp6_l2_flags_iov [TCP_FRAMES_MEM][TCP_IOV_NUM]; > =20 > /* sendmsg() to socket */ > static struct iovec tcp_iov [UIO_MAXIOV]; > =20 > -/** > - * tcp4_l2_flags_buf_t - IPv4 packet buffers for segments without data (= flags) > - * @pad: Align TCP header to 32 bytes, for AVX2 checksum calculation only > - * @taph: Tap-level headers (partially pre-filled) > - * @iph: Pre-filled IP header (except for tot_len and saddr) > - * @th: Headroom for TCP header > - * @opts: Headroom for TCP options > - */ > -static struct tcp4_l2_flags_buf_t { > -#ifdef __AVX2__ > - uint8_t pad[26]; /* 0, align th to 32 bytes */ > -#else > - uint8_t pad[2]; /* align iph to 4 bytes 0 */ > -#endif > - struct tap_hdr taph; /* 26 2 */ > - struct iphdr iph; /* 44 20 */ > - struct tcphdr th; /* 64 40 */ > - char opts[OPT_MSS_LEN + OPT_WS_LEN + 1]; > -#ifdef __AVX2__ > -} __attribute__ ((packed, aligned(32))) > -#else > -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))) > -#endif > -tcp4_l2_flags_buf[TCP_FRAMES_MEM]; > - > -static unsigned int tcp4_l2_flags_buf_used; > - > -/** > - * tcp6_l2_flags_buf_t - IPv6 packet buffers for segments without data (= flags) > - * @pad: Align IPv6 header for checksum calculation to 32B (AVX2) or 4B > - * @taph: Tap-level headers (partially pre-filled) > - * @ip6h: Pre-filled IP header (except for payload_len and addresses) > - * @th: Headroom for TCP header > - * @opts: Headroom for TCP options > - */ > -static struct tcp6_l2_flags_buf_t { > -#ifdef __AVX2__ > - uint8_t pad[14]; /* 0 align ip6h to 32 bytes */ > -#else > - uint8_t pad[2]; /* align ip6h to 4 bytes 0 */ > -#endif > - struct tap_hdr taph; /* 14 2 */ > - struct ipv6hdr ip6h; /* 32 20 */ > - struct tcphdr th /* 72 */ __attribute__ ((aligned(4))); /* 60 */ > - char opts[OPT_MSS_LEN + OPT_WS_LEN + 1]; > -#ifdef __AVX2__ > -} __attribute__ ((packed, aligned(32))) > -#else > -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))) > -#endif > -tcp6_l2_flags_buf[TCP_FRAMES_MEM]; > - > -static unsigned int tcp6_l2_flags_buf_used; > - > #define CONN(idx) (&(FLOW(idx)->tcp)) > =20 > /* Table for lookup from remote address, local port, remote port */ > @@ -973,19 +886,8 @@ static void tcp_update_check_tcp6(struct ipv6hdr *ip= 6h, struct tcphdr *th) > */ > void tcp_update_l2_buf(const unsigned char *eth_d, const unsigned char *= eth_s) > { > - int i; > - > - for (i =3D 0; i < TCP_FRAMES_MEM; i++) { > - struct tcp4_l2_flags_buf_t *b4f =3D &tcp4_l2_flags_buf[i]; > - struct tcp6_l2_flags_buf_t *b6f =3D &tcp6_l2_flags_buf[i]; > - struct tcp4_l2_buf_t *b4 =3D &tcp4_l2_buf[i]; > - struct tcp6_l2_buf_t *b6 =3D &tcp6_l2_buf[i]; > - > - eth_update_mac(&b4->taph.eh, eth_d, eth_s); > - eth_update_mac(&b6->taph.eh, eth_d, eth_s); > - eth_update_mac(&b4f->taph.eh, eth_d, eth_s); > - eth_update_mac(&b6f->taph.eh, eth_d, eth_s); > - } > + eth_update_mac(&tcp4_eth_src, eth_d, eth_s); > + eth_update_mac(&tcp6_eth_src, eth_d, eth_s); > } > =20 > /** > @@ -995,29 +897,42 @@ void tcp_update_l2_buf(const unsigned char *eth_d, = const unsigned char *eth_s) > static void tcp_sock4_iov_init(const struct ctx *c) > { > struct iphdr iph =3D L2_BUF_IP4_INIT(IPPROTO_TCP); > - struct iovec *iov; > int i; > =20 > - for (i =3D 0; i < ARRAY_SIZE(tcp4_l2_buf); i++) { > - tcp4_l2_buf[i] =3D (struct tcp4_l2_buf_t) { > - .taph =3D TAP_HDR_INIT(ETH_P_IP), > - .iph =3D iph, > - .th =3D { .doff =3D sizeof(struct tcphdr) / 4, .ack =3D 1 } > - }; > - } > + (void)c; > =20 > - for (i =3D 0; i < ARRAY_SIZE(tcp4_l2_flags_buf); i++) { > - tcp4_l2_flags_buf[i] =3D (struct tcp4_l2_flags_buf_t) { > - .taph =3D TAP_HDR_INIT(ETH_P_IP), > - .iph =3D L2_BUF_IP4_INIT(IPPROTO_TCP) > - }; > - } > + tcp4_eth_src.h_proto =3D htons_constant(ETH_P_IP); > + for (i =3D 0; i < TCP_FRAMES_MEM; i++) { > + struct iovec *iov; > + > + /* headers */ > + tcp4_l2_ip[i] =3D iph; > + tcp4_l2_payload[i].th =3D (struct tcphdr){ > + .doff =3D sizeof(struct tcphdr) / 4, > + .ack =3D 1 > + }; I find C struct literal syntax sufficiently awkward, that I'd probably prefer to just open code setting each field. That's only a very weak preference, though. > =20 > - for (i =3D 0, iov =3D tcp4_l2_iov; i < TCP_FRAMES_MEM; i++, iov++) > - iov->iov_base =3D tap_iov_base(c, &tcp4_l2_buf[i].taph); > + tcp4_l2_flags_ip[i] =3D iph; > + tcp4_l2_flags[i].th =3D (struct tcphdr){ > + .doff =3D sizeof(struct tcphdr) / 4, > + .ack =3D 1 > + }; > =20 > - for (i =3D 0, iov =3D tcp4_l2_flags_iov; i < TCP_FRAMES_MEM; i++, iov++) > - iov->iov_base =3D tap_iov_base(c, &tcp4_l2_flags_buf[i].taph); > + /* iovecs */ > + iov =3D tcp4_l2_iov[i]; > + iov[TCP_IOV_ETH].iov_base =3D &tcp4_eth_src; > + iov[TCP_IOV_ETH].iov_len =3D sizeof(struct ethhdr); > + iov[TCP_IOV_IP].iov_base =3D &tcp4_l2_ip[i]; > + iov[TCP_IOV_IP].iov_len =3D sizeof(struct iphdr); > + iov[TCP_IOV_PAYLOAD].iov_base =3D &tcp4_l2_payload[i]; > + > + iov =3D tcp4_l2_flags_iov[i]; > + iov[TCP_IOV_ETH].iov_base =3D &tcp4_eth_src; > + iov[TCP_IOV_ETH].iov_len =3D sizeof(struct ethhdr); > + iov[TCP_IOV_IP].iov_base =3D &tcp4_l2_flags_ip[i]; > + iov[TCP_IOV_IP].iov_len =3D sizeof(struct iphdr); > + iov[TCP_IOV_PAYLOAD].iov_base =3D &tcp4_l2_flags[i]; > + } > } > =20 > /** > @@ -1026,29 +941,43 @@ static void tcp_sock4_iov_init(const struct ctx *c) > */ > static void tcp_sock6_iov_init(const struct ctx *c) > { > - struct iovec *iov; > + struct ipv6hdr ip6 =3D L2_BUF_IP6_INIT(IPPROTO_TCP); > int i; > =20 > - for (i =3D 0; i < ARRAY_SIZE(tcp6_l2_buf); i++) { > - tcp6_l2_buf[i] =3D (struct tcp6_l2_buf_t) { > - .taph =3D TAP_HDR_INIT(ETH_P_IPV6), > - .ip6h =3D L2_BUF_IP6_INIT(IPPROTO_TCP), > - .th =3D { .doff =3D sizeof(struct tcphdr) / 4, .ack =3D 1 } > - }; > - } > + (void)c; > =20 > - for (i =3D 0; i < ARRAY_SIZE(tcp6_l2_flags_buf); i++) { > - tcp6_l2_flags_buf[i] =3D (struct tcp6_l2_flags_buf_t) { > - .taph =3D TAP_HDR_INIT(ETH_P_IPV6), > - .ip6h =3D L2_BUF_IP6_INIT(IPPROTO_TCP) > - }; > - } > + tcp6_eth_src.h_proto =3D htons_constant(ETH_P_IPV6); > + for (i =3D 0; i < TCP_FRAMES_MEM; i++) { > + struct iovec *iov; > + > + /* headers */ > + tcp6_l2_ip[i] =3D ip6; > + tcp6_l2_payload[i].th =3D (struct tcphdr){ > + .doff =3D sizeof(struct tcphdr) / 4, > + .ack =3D 1 > + }; > =20 > - for (i =3D 0, iov =3D tcp6_l2_iov; i < TCP_FRAMES_MEM; i++, iov++) > - iov->iov_base =3D tap_iov_base(c, &tcp6_l2_buf[i].taph); > + tcp6_l2_flags_ip[i] =3D ip6; > + tcp6_l2_flags[i].th =3D (struct tcphdr){ > + .doff =3D sizeof(struct tcphdr) / 4, > + .ack =3D 1 > + }; > =20 > - for (i =3D 0, iov =3D tcp6_l2_flags_iov; i < TCP_FRAMES_MEM; i++, iov++) > - iov->iov_base =3D tap_iov_base(c, &tcp6_l2_flags_buf[i].taph); > + /* iovecs */ > + iov =3D tcp6_l2_iov[i]; > + iov[TCP_IOV_ETH].iov_base =3D &tcp6_eth_src; > + iov[TCP_IOV_ETH].iov_len =3D sizeof(struct ethhdr); > + iov[TCP_IOV_IP].iov_base =3D &tcp6_l2_ip[i]; > + iov[TCP_IOV_IP].iov_len =3D sizeof(struct ipv6hdr); > + iov[TCP_IOV_PAYLOAD].iov_base =3D &tcp6_l2_payload[i]; > + > + iov =3D tcp6_l2_flags_iov[i]; > + iov[TCP_IOV_ETH].iov_base =3D &tcp6_eth_src; > + iov[TCP_IOV_ETH].iov_len =3D sizeof(struct ethhdr); > + iov[TCP_IOV_IP].iov_base =3D &tcp6_l2_flags_ip[i]; > + iov[TCP_IOV_IP].iov_len =3D sizeof(struct ipv6hdr); > + iov[TCP_IOV_PAYLOAD].iov_base =3D &tcp6_l2_flags[i]; > + } > } > =20 > /** > @@ -1289,10 +1218,10 @@ static void tcp_rst_do(struct ctx *c, struct tcp_= tap_conn *conn); > */ > static void tcp_l2_flags_buf_flush(const struct ctx *c) > { > - tap_send_frames(c, tcp6_l2_flags_iov, tcp6_l2_flags_buf_used); > + tap_send_iov(c, tcp6_l2_flags_iov, tcp6_l2_flags_buf_used); > tcp6_l2_flags_buf_used =3D 0; > =20 > - tap_send_frames(c, tcp4_l2_flags_iov, tcp4_l2_flags_buf_used); > + tap_send_iov(c, tcp4_l2_flags_iov, tcp4_l2_flags_buf_used); > tcp4_l2_flags_buf_used =3D 0; > } > =20 > @@ -1305,12 +1234,12 @@ static void tcp_l2_data_buf_flush(const struct ct= x *c) > unsigned i; > size_t m; > =20 > - m =3D tap_send_frames(c, tcp6_l2_iov, tcp6_l2_buf_used); > + m =3D tap_send_iov(c, tcp6_l2_iov, tcp6_l2_buf_used); > for (i =3D 0; i < m; i++) > *tcp6_l2_buf_seq_update[i].seq +=3D tcp6_l2_buf_seq_update[i].len; > tcp6_l2_buf_used =3D 0; > =20 > - m =3D tap_send_frames(c, tcp4_l2_iov, tcp4_l2_buf_used); > + m =3D tap_send_iov(c, tcp4_l2_iov, tcp4_l2_buf_used); > for (i =3D 0; i < m; i++) > *tcp4_l2_buf_seq_update[i].seq +=3D tcp4_l2_buf_seq_update[i].len; > tcp4_l2_buf_used =3D 0; > @@ -1433,7 +1362,7 @@ static size_t tcp_fill_headers6(const struct ctx *c, > * tcp_l2_buf_fill_headers() - Fill 802.3, IP, TCP headers in pre-cooked= buffers > * @c: Execution context > * @conn: Connection pointer > - * @p: Pointer to any type of TCP pre-cooked buffer > + * @iov: Pointer to an array of iovec of TCP pre-cooked buffer > * @plen: Payload length (including TCP header options) > * @check: Checksum, if already known > * @seq: Sequence number for this segment > @@ -1442,26 +1371,22 @@ static size_t tcp_fill_headers6(const struct ctx = *c, > */ > static size_t tcp_l2_buf_fill_headers(const struct ctx *c, > const struct tcp_tap_conn *conn, > - void *p, size_t plen, > + struct iovec *iov, size_t plen, > const uint16_t *check, uint32_t seq) > { > const struct in_addr *a4 =3D inany_v4(&conn->faddr); > size_t ip_len, tlen; > =20 > if (a4) { > - struct tcp4_l2_buf_t *b =3D (struct tcp4_l2_buf_t *)p; > - > - ip_len =3D tcp_fill_headers4(c, conn, &b->iph, &b->th, plen, > + ip_len =3D tcp_fill_headers4(c, conn, iov[TCP_IOV_IP].iov_base, > + iov[TCP_IOV_PAYLOAD].iov_base, plen, > check, seq); > - > - tlen =3D tap_iov_len(c, &b->taph, ip_len); > + tlen =3D ip_len - sizeof(struct iphdr); > } else { > - struct tcp6_l2_buf_t *b =3D (struct tcp6_l2_buf_t *)p; > - > - ip_len =3D tcp_fill_headers6(c, conn, &b->ip6h, &b->th, plen, > + ip_len =3D tcp_fill_headers6(c, conn, iov[TCP_IOV_IP].iov_base, > + iov[TCP_IOV_PAYLOAD].iov_base, plen, > seq); > - > - tlen =3D tap_iov_len(c, &b->taph, ip_len); > + tlen =3D ip_len - sizeof(struct ipv6hdr); > } > =20 > return tlen; > @@ -1595,16 +1520,15 @@ static int tcp_send_flag(struct ctx *c, struct tc= p_tap_conn *conn, int flags) > { > uint32_t prev_ack_to_tap =3D conn->seq_ack_to_tap; > uint32_t prev_wnd_to_tap =3D conn->wnd_to_tap; > - struct tcp4_l2_flags_buf_t *b4 =3D NULL; > - struct tcp6_l2_flags_buf_t *b6 =3D NULL; > + struct tcp_l2_flags_t *payload; > struct tcp_info tinfo =3D { 0 }; > socklen_t sl =3D sizeof(tinfo); > int s =3D conn->sock; > size_t optlen =3D 0; > struct iovec *iov; > + struct iovec *dup_iov; > struct tcphdr *th; > char *data; > - void *p; > =20 > if (SEQ_GE(conn->seq_ack_to_tap, conn->seq_from_tap) && > !flags && conn->wnd_to_tap) > @@ -1627,18 +1551,15 @@ static int tcp_send_flag(struct ctx *c, struct tc= p_tap_conn *conn, int flags) > return 0; > =20 > if (CONN_V4(conn)) { > - iov =3D tcp4_l2_flags_iov + tcp4_l2_flags_buf_used; > - p =3D b4 =3D tcp4_l2_flags_buf + tcp4_l2_flags_buf_used++; > - th =3D &b4->th; > - > - /* gcc 11.2 would complain on data =3D (char *)(th + 1); */ > - data =3D b4->opts; > + iov =3D tcp4_l2_flags_iov[tcp4_l2_flags_buf_used++]; > + dup_iov =3D tcp4_l2_flags_iov[tcp4_l2_flags_buf_used]; > } else { > - iov =3D tcp6_l2_flags_iov + tcp6_l2_flags_buf_used; > - p =3D b6 =3D tcp6_l2_flags_buf + tcp6_l2_flags_buf_used++; > - th =3D &b6->th; > - data =3D b6->opts; > + iov =3D tcp6_l2_flags_iov[tcp6_l2_flags_buf_used++]; > + dup_iov =3D tcp6_l2_flags_iov[tcp6_l2_flags_buf_used]; > } > + payload =3D iov[TCP_IOV_PAYLOAD].iov_base; > + th =3D &payload->th; > + data =3D payload->opts; > =20 > if (flags & SYN) { > int mss; > @@ -1653,10 +1574,6 @@ static int tcp_send_flag(struct ctx *c, struct tcp= _tap_conn *conn, int flags) > mss =3D tinfo.tcpi_snd_mss; > } else { > mss =3D c->mtu - sizeof(struct tcphdr); > - if (CONN_V4(conn)) > - mss -=3D sizeof(struct iphdr); > - else > - mss -=3D sizeof(struct ipv6hdr); > =20 > if (c->low_wmem && > !(conn->flags & LOCAL) && !tcp_rtt_dst_low(conn)) > @@ -1688,8 +1605,9 @@ static int tcp_send_flag(struct ctx *c, struct tcp_= tap_conn *conn, int flags) > th->syn =3D !!(flags & SYN); > th->fin =3D !!(flags & FIN); > =20 > - iov->iov_len =3D tcp_l2_buf_fill_headers(c, conn, p, optlen, > - NULL, conn->seq_to_tap); > + iov[TCP_IOV_PAYLOAD].iov_len =3D tcp_l2_buf_fill_headers(c, conn, iov, > + optlen, NULL, > + conn->seq_to_tap); > =20 > if (th->ack) { > if (SEQ_GE(conn->seq_ack_to_tap, conn->seq_from_tap)) > @@ -1705,23 +1623,26 @@ static int tcp_send_flag(struct ctx *c, struct tc= p_tap_conn *conn, int flags) > if (th->fin || th->syn) > conn->seq_to_tap++; > =20 > + if (flags & DUP_ACK) { > + int i; > + for (i =3D 0; i < TCP_IOV_NUM; i++) { > + memcpy(dup_iov[i].iov_base, iov[i].iov_base, > + iov[i].iov_len); > + dup_iov[i].iov_len =3D iov[i].iov_len; > + } > + } > + > if (CONN_V4(conn)) { > - if (flags & DUP_ACK) { > - memcpy(b4 + 1, b4, sizeof(*b4)); > - (iov + 1)->iov_len =3D iov->iov_len; > + if (flags & DUP_ACK) > tcp4_l2_flags_buf_used++; > - } > =20 > - if (tcp4_l2_flags_buf_used > ARRAY_SIZE(tcp4_l2_flags_buf) - 2) > + if (tcp4_l2_flags_buf_used > TCP_FRAMES_MEM - 2) > tcp_l2_flags_buf_flush(c); > } else { > - if (flags & DUP_ACK) { > - memcpy(b6 + 1, b6, sizeof(*b6)); > - (iov + 1)->iov_len =3D iov->iov_len; > + if (flags & DUP_ACK) > tcp6_l2_flags_buf_used++; > - } > =20 > - if (tcp6_l2_flags_buf_used > ARRAY_SIZE(tcp6_l2_flags_buf) - 2) > + if (tcp6_l2_flags_buf_used > TCP_FRAMES_MEM - 2) > tcp_l2_flags_buf_flush(c); > } > =20 > @@ -1887,15 +1808,14 @@ static uint16_t tcp_conn_tap_mss(const struct tcp= _tap_conn *conn, > unsigned int mss; > int ret; > =20 > + (void)conn; /* unused */ > + > if ((ret =3D tcp_opt_get(opts, optlen, OPT_MSS, NULL, NULL)) < 0) > mss =3D MSS_DEFAULT; > else > mss =3D ret; > =20 > - if (CONN_V4(conn)) > - mss =3D MIN(MSS4, mss); > - else > - mss =3D MIN(MSS6, mss); > + mss =3D MIN(MSS, mss); > =20 > return MIN(mss, USHRT_MAX); > } > @@ -2171,27 +2091,30 @@ static void tcp_data_to_tap(const struct ctx *c, = struct tcp_tap_conn *conn, > struct iovec *iov; > =20 > if (CONN_V4(conn)) { > - struct tcp4_l2_buf_t *b =3D &tcp4_l2_buf[tcp4_l2_buf_used]; > - const uint16_t *check =3D no_csum ? &(b - 1)->iph.check : NULL; > + struct iovec *iov_prev =3D tcp4_l2_iov[tcp4_l2_buf_used - 1]; > + const uint16_t *check =3D NULL; > + > + if (no_csum) { > + struct iphdr *iph =3D iov_prev[TCP_IOV_IP].iov_base; > + check =3D &iph->check; > + } > =20 > tcp4_l2_buf_seq_update[tcp4_l2_buf_used].seq =3D seq_update; > tcp4_l2_buf_seq_update[tcp4_l2_buf_used].len =3D plen; > =20 > - iov =3D tcp4_l2_iov + tcp4_l2_buf_used++; > - iov->iov_len =3D tcp_l2_buf_fill_headers(c, conn, b, plen, > - check, seq); > - if (tcp4_l2_buf_used > ARRAY_SIZE(tcp4_l2_buf) - 1) > + iov =3D tcp4_l2_iov[tcp4_l2_buf_used++]; > + iov[TCP_IOV_PAYLOAD].iov_len =3D tcp_l2_buf_fill_headers(c, conn, > + iov, plen, check, seq); > + if (tcp4_l2_buf_used > TCP_FRAMES_MEM - 1) > tcp_l2_data_buf_flush(c); > } else if (CONN_V6(conn)) { > - struct tcp6_l2_buf_t *b =3D &tcp6_l2_buf[tcp6_l2_buf_used]; > - > tcp6_l2_buf_seq_update[tcp6_l2_buf_used].seq =3D seq_update; > tcp6_l2_buf_seq_update[tcp6_l2_buf_used].len =3D plen; > =20 > - iov =3D tcp6_l2_iov + tcp6_l2_buf_used++; > - iov->iov_len =3D tcp_l2_buf_fill_headers(c, conn, b, plen, > - NULL, seq); > - if (tcp6_l2_buf_used > ARRAY_SIZE(tcp6_l2_buf) - 1) > + iov =3D tcp6_l2_iov[tcp6_l2_buf_used++]; > + iov[TCP_IOV_PAYLOAD].iov_len =3D tcp_l2_buf_fill_headers(c, conn, > + iov, plen, NULL, seq); > + if (tcp6_l2_buf_used > TCP_FRAMES_MEM - 1) > tcp_l2_data_buf_flush(c); > } > } > @@ -2247,8 +2170,8 @@ static int tcp_data_from_sock(struct ctx *c, struct= tcp_tap_conn *conn) > iov_sock[0].iov_base =3D tcp_buf_discard; > iov_sock[0].iov_len =3D already_sent; > =20 > - if (( v4 && tcp4_l2_buf_used + fill_bufs > ARRAY_SIZE(tcp4_l2_buf)) || > - (!v4 && tcp6_l2_buf_used + fill_bufs > ARRAY_SIZE(tcp6_l2_buf))) { > + if (( v4 && tcp4_l2_buf_used + fill_bufs > TCP_FRAMES_MEM) || > + (!v4 && tcp6_l2_buf_used + fill_bufs > TCP_FRAMES_MEM)) { > tcp_l2_data_buf_flush(c); > =20 > /* Silence Coverity CWE-125 false positive */ > @@ -2257,9 +2180,9 @@ static int tcp_data_from_sock(struct ctx *c, struct= tcp_tap_conn *conn) > =20 > for (i =3D 0, iov =3D iov_sock + 1; i < fill_bufs; i++, iov++) { > if (v4) > - iov->iov_base =3D &tcp4_l2_buf[tcp4_l2_buf_used + i].data; > + iov->iov_base =3D &tcp4_l2_payload[tcp4_l2_buf_used + i].data; > else > - iov->iov_base =3D &tcp6_l2_buf[tcp6_l2_buf_used + i].data; > + iov->iov_base =3D &tcp6_l2_payload[tcp6_l2_buf_used + i].data; > iov->iov_len =3D mss; > } > if (iov_rem) --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --lJjIZHHEtvjAnGBT Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmXye2QACgkQzQJF27ox 2GeNVw/+L1h8nNHZ/XqNvRnvRIqHh3W+p6WF4K8Lk+S4n/tHDynEUarbBjMCJWDV IA6EiTbZ+gDa7oBAzGfAaNsGc5dgxh3I+edMoe44pXeLK6lVqUhMH0Jp43NEy8lP fmAy9gz5W+JIiQ/VMCERPhDZsOcYDz/S+zamI03FMyjfjeHp6+Fq5XOe537Eg03s y5t5lPcAcxroecQWDaeWSrc/hjt3ykejXmt/jqDeK7TN32vtB3/EKt22G50/dqTM AhWJuHTs4T9HiNOGI6jNdVnlGJGda+oOdgGN4oc/ZnQ+dZuwyhBNP7CtUa1mUXbq aY3TgTbkcI25PotEpyY+ESv5Jc6Ty83XEMN+lTz56djqlUcHv+wKbXBx/u3aS/mu MZ5MP66PWJKNq85R/Gup5PMO4HU7jBWIGRYj579YM+nVg2ewLFkTkMCTrCwtXd5b LbffQIKOpnASIiBzb/p/P9KUuelsBpRVnXMt20OAEFTHsMNbTEqm4MszdOJmaFKa m7odTs8jQOntGdD/0Gvx4HcBxy3er2BVn8ojomSm7RPdUcddZ8YwkSwuylSmPtSv /lHcDSeHeB6p1ZFeZBc5xzi2VrJm5ZnzfH2K5r2Fwap9NVPoG1Da1uRVjX/DpH+E /MmQsEw1SD+v3hNvUmBx5EMBW+vCvcbY876pKu49L6sbxhqYF1U= =7CN6 -----END PGP SIGNATURE----- --lJjIZHHEtvjAnGBT--