From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 2FFE05A0307 for ; Mon, 03 Jun 2024 06:13:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1717387979; bh=x+ntZF1G43VIVYxmKQjwxirtckukVzg+GyrPL1iKrFo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Gi5sk5LcTBk9Olm3UocYf1FtffWbpJeMnNVT22x12oBbRBuzP9X1ah6ty0vopB1dF dS+P5UnEDabsNp9uM2jFlc8UkZucCSN68w67prvIC2bfJ9wcWUsHPZiWT5ojgPbIAw jj2RdJIaLPTa+kCkOBl5HiQ6Sl97qBUpM+ag9wiltm5+tzepCf1MYpahOy7kzFqtQu UpmLAOPppFFHKQQZvamwP7fLdxIDlr6aq/ecaK2JhlE15SSqflkLEys8cO5Ys352a9 2Xfxjucqfs8c26toF6+SH7yx03JSARYg/2U62HSk2V7GScghl1ROebvJChPcvjm7U4 EcZ7tfUp3lSSg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Vt0gv0yd1z4wyw; Mon, 3 Jun 2024 14:12:59 +1000 (AEST) Date: Mon, 3 Jun 2024 14:12:51 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v4 09/10] tcp: remove tap_hdr parameter Message-ID: References: <20240531142344.1420034-1-lvivier@redhat.com> <20240531142344.1420034-10-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="eXJpY7qz7d5NxBpa" Content-Disposition: inline In-Reply-To: <20240531142344.1420034-10-lvivier@redhat.com> Message-ID-Hash: LXO5M7PQPIAHSMF2E5M37SEHBGJIDO3O X-Message-ID-Hash: LXO5M7PQPIAHSMF2E5M37SEHBGJIDO3O 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: --eXJpY7qz7d5NxBpa Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, May 31, 2024 at 04:23:43PM +0200, Laurent Vivier wrote: > As tap_hdr is not used with vhost-user, remove it from tcp_fill_headers4() > and tcp_fill_headers6() So, this is kind of at odds with how I intended tap_hdr to work. The idea was that it would cover any sort of specific headers we needed for the tap backend. Currently that's either a length field (qemu socket) or nothing (tuntap), but it could be other things if we need them. At the moment tap_hdr_update() fills in the vnet_len in all cases, even MODE_PASTA, because my guess was that it's cheaper to do that than to test the mode every time. If it matters we could make it do nothing in both PASTA and VU modes, and that's more what I'd expect rather than extracting it from the path here. I don't know if VU mode has any use for some backend specific "header" (would it make sense to put the descriptor ring entry here?). > Signed-off-by: Laurent Vivier > --- > tcp.c | 8 -------- > tcp_buf.c | 18 ++++++++++++++---- > tcp_internal.h | 2 -- > 3 files changed, 14 insertions(+), 14 deletions(-) >=20 > diff --git a/tcp.c b/tcp.c > index 48d8f7c6d696..433ab1fab30f 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -1017,7 +1017,6 @@ static void tcp_fill_header(struct tcphdr *th, > * tcp_fill_headers4() - Fill 802.3, IPv4, TCP headers in pre-cooked buf= fers > * @c: Execution context > * @conn: Connection pointer > - * @taph: tap backend specific header > * @iph: Pointer to IPv4 header > * @th: Pointer to TCP header > * @dlen: TCP payload length > @@ -1028,7 +1027,6 @@ static void tcp_fill_header(struct tcphdr *th, > */ > size_t tcp_fill_headers4(const struct ctx *c, > const struct tcp_tap_conn *conn, > - struct tap_hdr *taph, > struct iphdr *iph, struct tcphdr *th, > size_t dlen, const uint16_t *check, > uint32_t seq) > @@ -1051,8 +1049,6 @@ size_t tcp_fill_headers4(const struct ctx *c, > =20 > tcp_update_check_tcp4(iph, th); > =20 > - tap_hdr_update(taph, l3len + sizeof(struct ethhdr)); > - > return l4len; > } > =20 > @@ -1060,7 +1056,6 @@ size_t tcp_fill_headers4(const struct ctx *c, > * tcp_fill_headers6() - Fill 802.3, IPv6, TCP headers in pre-cooked buf= fers > * @c: Execution context > * @conn: Connection pointer > - * @taph: tap backend specific header > * @ip6h: Pointer to IPv6 header > * @th: Pointer to TCP header > * @dlen: TCP payload length > @@ -1071,7 +1066,6 @@ size_t tcp_fill_headers4(const struct ctx *c, > */ > size_t tcp_fill_headers6(const struct ctx *c, > const struct tcp_tap_conn *conn, > - struct tap_hdr *taph, > struct ipv6hdr *ip6h, struct tcphdr *th, > size_t dlen, uint32_t seq) > { > @@ -1096,8 +1090,6 @@ size_t tcp_fill_headers6(const struct ctx *c, > =20 > tcp_update_check_tcp6(ip6h, th); > =20 > - tap_hdr_update(taph, l4len + sizeof(*ip6h) + sizeof(struct ethhdr)); > - > return l4len; > } > =20 > diff --git a/tcp_buf.c b/tcp_buf.c > index 630e83e9a01a..cd4549c06035 100644 > --- a/tcp_buf.c > +++ b/tcp_buf.c > @@ -298,10 +298,12 @@ int tcp_buf_send_flag(struct ctx *c, struct tcp_tap= _conn *conn, int flags) > if (ret <=3D 0) > return ret; > =20 > - l4len =3D tcp_fill_headers4(c, conn, iov[TCP_IOV_TAP].iov_base, > + l4len =3D tcp_fill_headers4(c, conn, > iov[TCP_IOV_IP].iov_base, > iov[TCP_IOV_PAYLOAD].iov_base, optlen, > NULL, conn->seq_to_tap); > + tap_hdr_update(iov[TCP_IOV_TAP].iov_base, > + l4len + sizeof(struct iphdr) + sizeof(struct ethhdr)); > } else { > iov =3D tcp6_l2_flags_iov[tcp6_flags_used++]; > =20 > @@ -312,10 +314,13 @@ int tcp_buf_send_flag(struct ctx *c, struct tcp_tap= _conn *conn, int flags) > if (ret <=3D 0) > return ret; > =20 > - l4len =3D tcp_fill_headers6(c, conn, iov[TCP_IOV_TAP].iov_base, > + l4len =3D tcp_fill_headers6(c, conn, > iov[TCP_IOV_IP].iov_base, > iov[TCP_IOV_PAYLOAD].iov_base, optlen, > conn->seq_to_tap); > + tap_hdr_update(iov[TCP_IOV_TAP].iov_base, > + l4len + sizeof(struct ipv6hdr) + > + sizeof(struct ethhdr)); > } > iov[TCP_IOV_PAYLOAD].iov_len =3D l4len; > =20 > @@ -373,10 +378,12 @@ void tcp_data_to_tap(const struct ctx *c, struct tc= p_tap_conn *conn, > tcp4_seq_update[tcp4_payload_used].len =3D dlen; > =20 > iov =3D tcp4_l2_iov[tcp4_payload_used++]; > - l4len =3D tcp_fill_headers4(c, conn, iov[TCP_IOV_TAP].iov_base, > + l4len =3D tcp_fill_headers4(c, conn, > iov[TCP_IOV_IP].iov_base, > iov[TCP_IOV_PAYLOAD].iov_base, dlen, > check, seq); > + tap_hdr_update(iov[TCP_IOV_TAP].iov_base, > + l4len + sizeof(struct iphdr) + sizeof(struct ethhdr)); > iov[TCP_IOV_PAYLOAD].iov_len =3D l4len; > if (tcp4_payload_used > TCP_FRAMES_MEM - 1) > tcp_payload_flush(c); > @@ -385,10 +392,13 @@ void tcp_data_to_tap(const struct ctx *c, struct tc= p_tap_conn *conn, > tcp6_seq_update[tcp6_payload_used].len =3D dlen; > =20 > iov =3D tcp6_l2_iov[tcp6_payload_used++]; > - l4len =3D tcp_fill_headers6(c, conn, iov[TCP_IOV_TAP].iov_base, > + l4len =3D tcp_fill_headers6(c, conn, > iov[TCP_IOV_IP].iov_base, > iov[TCP_IOV_PAYLOAD].iov_base, dlen, > seq); > + tap_hdr_update(iov[TCP_IOV_TAP].iov_base, > + l4len + sizeof(struct ipv6hdr) + > + sizeof(struct ethhdr)); > iov[TCP_IOV_PAYLOAD].iov_len =3D l4len; > if (tcp6_payload_used > TCP_FRAMES_MEM - 1) > tcp_payload_flush(c); > diff --git a/tcp_internal.h b/tcp_internal.h > index e47b64a68afd..5c7a52b8293c 100644 > --- a/tcp_internal.h > +++ b/tcp_internal.h > @@ -70,13 +70,11 @@ void tcp_rst_do(struct ctx *c, struct tcp_tap_conn *c= onn); > =20 > size_t tcp_fill_headers4(const struct ctx *c, > const struct tcp_tap_conn *conn, > - struct tap_hdr *taph, > struct iphdr *iph, struct tcphdr *th, > size_t dlen, const uint16_t *check, > uint32_t seq); > size_t tcp_fill_headers6(const struct ctx *c, > const struct tcp_tap_conn *conn, > - struct tap_hdr *taph, > struct ipv6hdr *ip6h, struct tcphdr *th, > size_t dlen, uint32_t seq); > int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, --=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 --eXJpY7qz7d5NxBpa Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmZdQsIACgkQzQJF27ox 2GefmQ/+OH4aL859T/W1u2UWfv8xvqMxS1jzksL0/iGndw2laCl3rid0U6C4P5dP 0VBvBQprTHgOo04mdx7L/1rLqIzuHBCrmJf+ht0myhEZ8bWxFRmblRwMX7Y3/ERW QIOqAnjh0xLIFMexFjODUcj9amvH/xQ3x3wRPmbZx2EHGTxYFj0h/rgcshpwB1ng vjez4ngFa34AXxgNjnpQpB06XLDjk/qzfIerDZcaabeoQpOE4W4fvbJRuMLpTBth MmQwsRgdNcM1XIZFWAM+UCWUpXm7oUnI/MITQuA5ZWtUnTupKtV9ubShB1IvL1v9 sWNVtxIHBYw7/A0SieTuAX5Lc+NnEBA3FcEn/5o+Gghv4co6nOLkGMWtQt0mM65+ cRQUCm6X6evw6G+9mKFX0XjBvs9wK1j6JJ/o6MYiWVSgl6JZZ922J1fwAJ+1waZ1 +4pKYqsfP04cW6/NnIRVtx/Fv5eVjcGmI4tEcS8m09AUJJ9WM2XTPiJqVYoHQ5aQ E17sn3p/q7Eo6eDlStSOtNnM0PmSH/b+KHFVWDQyFiAUslEiIQ7EmbmY4XHoyi+k MMSRWVUe8pQNaQ9T59BT9qIv3i78QLJoStfvYBo2bxaEzlj6gHpQng7MbHXR+u/o v06gVM11ckfUAbCiyfJKNZOx6CAWOKhOmML3tWoJxGq1IfGx6jY= =LfIV -----END PGP SIGNATURE----- --eXJpY7qz7d5NxBpa--