From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202408 header.b=V0rgelO6; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id F2FBE5A0275 for ; Mon, 09 Sep 2024 03:17:55 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202408; t=1725844659; bh=BlHvCP9mMZT8SNHrkACZBmAlTuBmrX+3DlRA++0eO4E=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=V0rgelO6xabLfc87Xkpt0LGKPqUYfLy9yCitMtWjHnV5sxEOmTVJ3WHcWe65AyvJt 7BHAdThXM0kiDXQBPb+l4ujQeCiLiHa0sRyotGkc2UmlDLtvwJRpnRHgIzjP9OfHxX zzJkVPqosAFPB46vlM3EowhGXOHBf8sKaV3f5283JTVBygZJQIkhbIwPkQtL29hVNe +liWTVTucl2zeNdhHH2RzZPKXL7z5MJr0xLwmgF4Ib0fEw3Y0rD/BnKvddpAi3B1Qu smq8Q2Y4YRHM7U5aX/aS/5NnSPWsAay5L/jGXKOUL6OgaPS1TySeUvCFYeiB+K1WlA DBk7WuEqpb40w== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4X288M26w5z4wd6; Mon, 9 Sep 2024 11:17:39 +1000 (AEST) Date: Mon, 9 Sep 2024 11:17:19 +1000 From: David Gibson To: Jon Maloy Subject: Re: [PATCH 3/4] tcp: unify l2 TCPv4 and TCPv6 queues and structures Message-ID: References: <20240906213427.1915806-1-jmaloy@redhat.com> <20240906213427.1915806-4-jmaloy@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="v2MJObsNvWrALtxm" Content-Disposition: inline In-Reply-To: <20240906213427.1915806-4-jmaloy@redhat.com> Message-ID-Hash: KMZGDPGZ4QCX56HO6AJ5MQDHAOYUNEZD X-Message-ID-Hash: KMZGDPGZ4QCX56HO6AJ5MQDHAOYUNEZD 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, sbrivio@redhat.com, lvivier@redhat.com, dgibson@redhat.com 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: --v2MJObsNvWrALtxm Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 06, 2024 at 05:34:26PM -0400, Jon Maloy wrote: > Following the preparations in the previous commits, we can now > remove the queues dedicated for TCPv6 and move that traffic over > to the queues currently used for TCPv4. >=20 > Signed-off-by: Jon Maloy > --- > tcp.c | 8 ++- > tcp_buf.c | 158 +++++++++--------------------------------------------- > tcp_buf.h | 1 + > 3 files changed, 28 insertions(+), 139 deletions(-) >=20 > diff --git a/tcp.c b/tcp.c > index 006e503..19cf9e5 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -998,12 +998,14 @@ size_t tcp_l2_buf_fill_headers(const struct tcp_tap= _conn *conn, > =20 > if (a4) { > iov[TCP_IOV_IP].iov_len =3D sizeof(struct iphdr); > + tcp4_eth_src.h_proto =3D htons_constant(ETH_P_IP); > return tcp_fill_headers4(conn, iov[TCP_IOV_TAP].iov_base, > iov[TCP_IOV_IP].iov_base, > iov[TCP_IOV_PAYLOAD].iov_base, dlen, > check, seq); > } else { > iov[TCP_IOV_IP].iov_len =3D sizeof(struct ipv6hdr); > + tcp4_eth_src.h_proto =3D htons_constant(ETH_P_IPV6); > return tcp_fill_headers6(conn, iov[TCP_IOV_TAP].iov_base, > iov[TCP_IOV_IP].iov_base, > iov[TCP_IOV_PAYLOAD].iov_base, dlen, > @@ -2508,11 +2510,7 @@ int tcp_init(struct ctx *c) > { > ASSERT(!c->no_tcp); > =20 > - if (c->ifi4) > - tcp_sock4_iov_init(c); > - > - if (c->ifi6) > - tcp_sock6_iov_init(c); > + tcp_sock4_iov_init(c); I'd prefer to see the renames to remove the now incorrect '4' and '6' folded in here, rather than waiting for the next patch. > =20 > memset(init_sock_pool4, 0xff, sizeof(init_sock_pool4)); > memset(init_sock_pool6, 0xff, sizeof(init_sock_pool6)); > diff --git a/tcp_buf.c b/tcp_buf.c > index 6e6549f..92c4d73 100644 > --- a/tcp_buf.c > +++ b/tcp_buf.c > @@ -80,7 +80,7 @@ struct tcp_flags_t { > #endif > =20 > /* Ethernet header for IPv4 frames */ > -static struct ethhdr tcp4_eth_src; > +struct ethhdr tcp4_eth_src; Ditto. Also, as with the IP header, for UDP we still have separate ethernet header structures for v4 and v6 and just update the IOV per-packet to point to the right one. > =20 > static struct tap_hdr tcp4_payload_tap_hdr[TCP_FRAMES_MEM]; > /* IPv4 headers */ > @@ -104,36 +104,14 @@ static struct tcp_flags_t tcp4_flags[TCP_FRAMES_MEM= ]; > static unsigned int tcp4_flags_used; > =20 > /* Ethernet header for IPv6 frames */ > -static struct ethhdr tcp6_eth_src; > - > -static struct tap_hdr tcp6_payload_tap_hdr[TCP_FRAMES_MEM]; > -/* IPv6 headers */ > -struct ipv6hdr tcp_payload_ip6; > -static struct iphdr_t tcp6_payload_ip[TCP_FRAMES_MEM]; > -/* TCP headers and data for IPv6 frames */ > -static struct tcp_payload_t tcp6_payload[TCP_FRAMES_MEM]; > - > -static_assert(MSS6 <=3D sizeof(tcp6_payload[0].data), "MSS6 is greater t= han 65516"); > - > -/* References tracking the owner connection of frames in the tap outqueu= e */ > -static struct tcp_tap_conn *tcp6_frame_conns[TCP_FRAMES_MEM]; > -static unsigned int tcp6_payload_used; > - > -static struct tap_hdr tcp6_flags_tap_hdr[TCP_FRAMES_MEM]; > -/* IPv6 headers for TCP segment without payload */ > -static struct iphdr_t tcp6_flags_ip[TCP_FRAMES_MEM]; > -/* TCP segment without payload for IPv6 frames */ > -static struct tcp_flags_t tcp6_flags[TCP_FRAMES_MEM]; > - > -static unsigned int tcp6_flags_used; > +struct ipv6hdr tcp_payload_ip6; > =20 > /* recvmsg()/sendmsg() data for tap */ > static struct iovec iov_sock [TCP_FRAMES_MEM + 1]; > =20 > static struct iovec tcp4_l2_iov [TCP_FRAMES_MEM][TCP_NUM_IOVS]; > -static struct iovec tcp6_l2_iov [TCP_FRAMES_MEM][TCP_NUM_IOVS]; > static struct iovec tcp4_l2_flags_iov [TCP_FRAMES_MEM][TCP_NUM_IOVS]; > -static struct iovec tcp6_l2_flags_iov [TCP_FRAMES_MEM][TCP_NUM_IOVS]; > + > /** > * tcp_update_l2_buf() - Update Ethernet header buffers with addresses > * @eth_d: Ethernet destination address, NULL if unchanged > @@ -142,7 +120,6 @@ static struct iovec tcp6_l2_flags_iov [TCP_FRAMES_MEM= ][TCP_NUM_IOVS]; > void tcp_update_l2_buf(const unsigned char *eth_d, const unsigned char *= eth_s) > { > eth_update_mac(&tcp4_eth_src, eth_d, eth_s); > - eth_update_mac(&tcp6_eth_src, eth_d, eth_s); > } > =20 > /** > @@ -191,61 +168,12 @@ void tcp_sock4_iov_init(const struct ctx *c) > } > } > =20 > -/** > - * tcp_sock6_iov_init() - Initialise scatter-gather L2 buffers for IPv6 = sockets > - * @c: Execution context > - */ > -void tcp_sock6_iov_init(const struct ctx *c) > -{ > - struct ipv6hdr ip6 =3D L2_BUF_IP6_INIT(IPPROTO_TCP); > - struct iovec *iov; > - int i; > - > - tcp6_eth_src.h_proto =3D htons_constant(ETH_P_IPV6); > - tcp_payload_ip6 =3D ip6; > - > - for (i =3D 0; i < ARRAY_SIZE(tcp6_payload); i++) { > - tcp6_payload[i].th.doff =3D sizeof(struct tcphdr) / 4; > - tcp6_payload[i].th.ack =3D 1; > - } > - > - for (i =3D 0; i < ARRAY_SIZE(tcp6_flags); i++) { > - tcp6_flags_ip[i].ip6 =3D ip6; > - tcp6_flags[i].th.doff =3D sizeof(struct tcphdr) / 4; > - tcp6_flags[i].th .ack =3D 1; > - } > - > - for (i =3D 0; i < TCP_FRAMES_MEM; i++) { > - iov =3D tcp6_l2_iov[i]; > - > - iov[TCP_IOV_TAP] =3D tap_hdr_iov(c, &tcp6_payload_tap_hdr[i]); > - iov[TCP_IOV_ETH] =3D IOV_OF_LVALUE(tcp6_eth_src); > - iov[TCP_IOV_IP].iov_base =3D &tcp6_payload_ip[i]; > - iov[TCP_IOV_IP].iov_len =3D sizeof(tcp6_payload_ip[i].ip6); > - iov[TCP_IOV_PAYLOAD].iov_base =3D &tcp6_payload[i]; > - } > - > - for (i =3D 0; i < TCP_FRAMES_MEM; i++) { > - iov =3D tcp6_l2_flags_iov[i]; > - > - iov[TCP_IOV_TAP] =3D tap_hdr_iov(c, &tcp6_flags_tap_hdr[i]); > - iov[TCP_IOV_ETH] =3D IOV_OF_LVALUE(tcp6_eth_src); > - iov[TCP_IOV_IP].iov_base =3D &tcp6_flags_ip[i]; > - iov[TCP_IOV_IP].iov_len =3D sizeof(tcp6_flags_ip[i].ip6); > - iov[TCP_IOV_PAYLOAD].iov_base =3D &tcp6_flags[i]; > - } > -} > - > /** > * tcp_flags_flush() - Send out buffers for segments with no data (flags) > * @c: Execution context > */ > void tcp_flags_flush(const struct ctx *c) > { > - tap_send_frames(c, &tcp6_l2_flags_iov[0][0], TCP_NUM_IOVS, > - tcp6_flags_used); > - tcp6_flags_used =3D 0; > - > tap_send_frames(c, &tcp4_l2_flags_iov[0][0], TCP_NUM_IOVS, > tcp4_flags_used); > tcp4_flags_used =3D 0; > @@ -287,14 +215,6 @@ void tcp_payload_flush(struct ctx *c) > { > size_t m; > =20 > - m =3D tap_send_frames(c, &tcp6_l2_iov[0][0], TCP_NUM_IOVS, > - tcp6_payload_used); > - if (m !=3D tcp6_payload_used) { > - tcp_revert_seq(c, &tcp6_frame_conns[m], &tcp6_l2_iov[m], > - tcp6_payload_used - m); > - } > - tcp6_payload_used =3D 0; > - > m =3D tap_send_frames(c, &tcp4_l2_iov[0][0], TCP_NUM_IOVS, > tcp4_payload_used); > if (m !=3D tcp4_payload_used) { > @@ -321,21 +241,13 @@ int tcp_buf_send_flag(struct ctx *c, struct tcp_tap= _conn *conn, int flags) > uint32_t seq; > int ret; > =20 > - if (CONN_V4(conn)) > - iov =3D tcp4_l2_flags_iov[tcp4_flags_used++]; > - else > - iov =3D tcp6_l2_flags_iov[tcp6_flags_used++]; > - > + iov =3D tcp4_l2_flags_iov[tcp4_flags_used++]; > payload =3D iov[TCP_IOV_PAYLOAD].iov_base; > - > seq =3D conn->seq_to_tap; > ret =3D tcp_prepare_flags(c, conn, flags, &payload->th, > payload->opts, &optlen); > if (ret <=3D 0) { > - if (CONN_V4(conn)) > - tcp4_flags_used--; > - else > - tcp6_flags_used--; > + tcp4_flags_used--; > return ret; > } > =20 > @@ -346,10 +258,7 @@ int tcp_buf_send_flag(struct ctx *c, struct tcp_tap_= conn *conn, int flags) > struct iovec *dup_iov; > int i; > =20 > - if (CONN_V4(conn)) > - dup_iov =3D tcp4_l2_flags_iov[tcp4_flags_used++]; > - else > - dup_iov =3D tcp6_l2_flags_iov[tcp6_flags_used++]; > + dup_iov =3D tcp4_l2_flags_iov[tcp4_flags_used++]; > =20 > for (i =3D 0; i < TCP_NUM_IOVS; i++) > memcpy(dup_iov[i].iov_base, iov[i].iov_base, > @@ -357,13 +266,8 @@ int tcp_buf_send_flag(struct ctx *c, struct tcp_tap_= conn *conn, int flags) > dup_iov[TCP_IOV_PAYLOAD].iov_len =3D iov[TCP_IOV_PAYLOAD].iov_len; > } > =20 > - if (CONN_V4(conn)) { > - if (tcp4_flags_used > TCP_FRAMES_MEM - 2) > - tcp_flags_flush(c); > - } else { > - if (tcp6_flags_used > TCP_FRAMES_MEM - 2) > - tcp_flags_flush(c); > - } > + if (tcp4_flags_used > TCP_FRAMES_MEM - 2) > + tcp_flags_flush(c); > =20 > return 0; > } > @@ -379,36 +283,26 @@ int tcp_buf_send_flag(struct ctx *c, struct tcp_tap= _conn *conn, int flags) > static void tcp_data_to_tap(struct ctx *c, struct tcp_tap_conn *conn, > ssize_t dlen, int no_csum, uint32_t seq) > { > + struct iovec *iov_prev =3D tcp4_l2_iov[tcp4_payload_used - 1]; > + const uint16_t *check =3D NULL; > struct iovec *iov; > size_t l4len; > =20 > conn->seq_to_tap =3D seq + dlen; > =20 > - if (CONN_V4(conn)) { > - struct iovec *iov_prev =3D tcp4_l2_iov[tcp4_payload_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; > - } > + if (CONN_V4(conn) && no_csum) { > + struct iphdr *iph =3D iov_prev[TCP_IOV_IP].iov_base; > =20 > - tcp4_frame_conns[tcp4_payload_used] =3D conn; > - > - iov =3D tcp4_l2_iov[tcp4_payload_used++]; > - l4len =3D tcp_l2_buf_fill_headers(conn, iov, dlen, check, seq); > - iov[TCP_IOV_PAYLOAD].iov_len =3D l4len; > - if (tcp4_payload_used > TCP_FRAMES_MEM - 1) > - tcp_payload_flush(c); > - } else if (CONN_V6(conn)) { > - tcp6_frame_conns[tcp6_payload_used] =3D conn; > - > - iov =3D tcp6_l2_iov[tcp6_payload_used++]; > - l4len =3D tcp_l2_buf_fill_headers(conn, iov, dlen, NULL, seq); > - iov[TCP_IOV_PAYLOAD].iov_len =3D l4len; > - if (tcp6_payload_used > TCP_FRAMES_MEM - 1) > - tcp_payload_flush(c); > + check =3D &iph->check; > } > + > + tcp4_frame_conns[tcp4_payload_used] =3D conn; > + > + iov =3D tcp4_l2_iov[tcp4_payload_used++]; > + l4len =3D tcp_l2_buf_fill_headers(conn, iov, dlen, check, seq); > + iov[TCP_IOV_PAYLOAD].iov_len =3D l4len; > + if (tcp4_payload_used > TCP_FRAMES_MEM - 1) > + tcp_payload_flush(c); > } > =20 > /** > @@ -472,19 +366,15 @@ int tcp_buf_data_from_sock(struct ctx *c, struct tc= p_tap_conn *conn) > mh_sock.msg_iovlen =3D fill_bufs; > } > =20 > - if (( v4 && tcp4_payload_used + fill_bufs > TCP_FRAMES_MEM) || > - (!v4 && tcp6_payload_used + fill_bufs > TCP_FRAMES_MEM)) { > + if ((v4 && tcp4_payload_used + fill_bufs > TCP_FRAMES_MEM)) { > tcp_payload_flush(c); > =20 > /* Silence Coverity CWE-125 false positive */ > - tcp4_payload_used =3D tcp6_payload_used =3D 0; > + tcp4_payload_used =3D 0; > } > =20 > for (i =3D 0, iov =3D iov_sock + 1; i < fill_bufs; i++, iov++) { > - if (v4) > - iov->iov_base =3D &tcp4_payload[tcp4_payload_used + i].data; > - else > - iov->iov_base =3D &tcp6_payload[tcp6_payload_used + i].data; > + iov->iov_base =3D &tcp4_payload[tcp4_payload_used + i].data; > iov->iov_len =3D mss; > } > if (iov_rem) > diff --git a/tcp_buf.h b/tcp_buf.h > index d3d0d7f..d7cdbaf 100644 > --- a/tcp_buf.h > +++ b/tcp_buf.h > @@ -13,6 +13,7 @@ void tcp_payload_flush(struct ctx *c); > int tcp_buf_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn); > int tcp_buf_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flag= s); > =20 > +extern struct ethhdr tcp4_eth_src; > extern struct iphdr tcp_payload_ip4; > extern struct ipv6hdr tcp_payload_ip6; > #endif /*TCP_BUF_H */ --=20 David Gibson (he or they) | 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 --v2MJObsNvWrALtxm Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmbeTJ4ACgkQzQJF27ox 2GeANA/7Baj4cCyEu2EQ+WX6Wl4VoN4i/8e/cwH3BtuqXkq5D0poSZGSZ19AenaY UgZWW2Lb/cVeP4oqXFrBLPBuQMf7enh7i75mYny7ikrc33Z7BO9Z77e3QXL76r+F F8X7FKtRtETRvum5m5iaLx/v1uknHrlPyvX15IO0DObAmjhvltyVE3D4qC7ZXw5n GIEkexXQWdHufFwlCibOtuvUZEvCa2mCZBNdTQMmLZApPlOdqW/8Y6xIhAcr1TjO amGagfLZE7kdw8+ga74ndXS5o2SIu0JYdMH6kelBYNJzHg+QeYZRq6H+4Mvdy48a o3NX9Eo05TGjYyRCJ+sjfvcV4TuN8s2GVqT/Bzf5/MxAJ0C4hXTgJlC0N6YAj8Fj Nbtz4o2plhjp+XFsxjoIKqyLExE70X2s1hiStbq93wtRPF1rLEa+VSNaWwAu4D21 AOBaBdYD0HYKzfeFtmZqcxuC6DLrguTe9MnByQcjz+SIJDsdJpdhD4QlmUpQuy1l jq+TKcQgca2fQ1rI/0D3cvo4SoaMKgWBUYnEGLoA2xKn1Om0mrpOrtYeZwOUzQ94 tCtjH4DNFOrBn8RdbJbAZYNwiv0vxtgxVn7ThBkAdUhSF3APj58bheNU6Id3HoxZ f+iEV2HjO/5MJxuiwkjP9yqWSdHSQkcJWsiOCFI+0ayi4jOzWDQ= =6KdB -----END PGP SIGNATURE----- --v2MJObsNvWrALtxm--