Re: [PATCH v4 09/10] tcp: remove tap_hdr parameter

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 6506 bytes --]

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 <lvivier@redhat.com>
> ---
>  tcp.c          |  8 --------
>  tcp_buf.c      | 18 ++++++++++++++----
>  tcp_internal.h |  2 --
>  3 files changed, 14 insertions(+), 14 deletions(-)
> 
> 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 buffers
>   * @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,
>  
>  	tcp_update_check_tcp4(iph, th);
>  
> -	tap_hdr_update(taph, l3len + sizeof(struct ethhdr));
> -
>  	return l4len;
>  }
>  
> @@ -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 buffers
>   * @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,
>  
>  	tcp_update_check_tcp6(ip6h, th);
>  
> -	tap_hdr_update(taph, l4len + sizeof(*ip6h) + sizeof(struct ethhdr));
> -
>  	return l4len;
>  }
>  
> 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 <= 0)
>  			return ret;
>  
> -		l4len = tcp_fill_headers4(c, conn, iov[TCP_IOV_TAP].iov_base,
> +		l4len = 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 = tcp6_l2_flags_iov[tcp6_flags_used++];
>  
> @@ -312,10 +314,13 @@ int tcp_buf_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags)
>  		if (ret <= 0)
>  			return ret;
>  
> -		l4len = tcp_fill_headers6(c, conn, iov[TCP_IOV_TAP].iov_base,
> +		l4len = 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 = l4len;
>  
> @@ -373,10 +378,12 @@ void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn,
>  		tcp4_seq_update[tcp4_payload_used].len = dlen;
>  
>  		iov = tcp4_l2_iov[tcp4_payload_used++];
> -		l4len = tcp_fill_headers4(c, conn, iov[TCP_IOV_TAP].iov_base,
> +		l4len = 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 = 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 tcp_tap_conn *conn,
>  		tcp6_seq_update[tcp6_payload_used].len = dlen;
>  
>  		iov = tcp6_l2_iov[tcp6_payload_used++];
> -		l4len = tcp_fill_headers6(c, conn, iov[TCP_IOV_TAP].iov_base,
> +		l4len = 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 = 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 *conn);
>  
>  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,

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]