Re: [PATCH 11/18] tcp: Convert tcp_data_from_tap() to use iov_tail

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

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

On Wed, Apr 02, 2025 at 07:23:36PM +0200, Laurent Vivier wrote:
> Use packet_base() and extract headers using IOV_PEEK_HEADER()
> rather than packet_get().
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  tcp.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 790714a08793..a9c04551d9d6 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -1643,14 +1643,19 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn,
>  	for (i = idx, iov_i = 0; i < (int)p->count; i++) {
>  		uint32_t seq, seq_offset, ack_seq;
>  		const struct tcphdr *th;
> -		char *data;
> +		struct iov_tail data;
> +		unsigned int count;
> +		struct tcphdr thc;
>  		size_t off;
>  
> -		th = packet_get(p, i, 0, sizeof(*th), &len);
> +		if (!packet_base(p, i, &data))
> +			return -1;
> +
> +		th = IOV_PEEK_HEADER(&data, thc);
>  		if (!th)
>  			return -1;
> -		len += sizeof(*th);
>  
> +		len = iov_tail_size(&data);
>  		off = th->doff * 4UL;
>  		if (off < sizeof(*th) || off > len)
>  			return -1;
> @@ -1661,9 +1666,7 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn,
>  		}
>  
>  		len -= off;
> -		data = packet_get(p, i, off, len, NULL);
> -		if (!data)
> -			continue;
> +		data.off = off;

You can use iov_drop() rather than reaching into the internals of
iov_tail here, no?

>  		seq = ntohl(th->seq);
>  		if (SEQ_LT(seq, conn->seq_from_tap) && len <= 1) {
> @@ -1737,10 +1740,11 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn,
>  			continue;
>  		}
>  
> -		tcp_iov[iov_i].iov_base = data + seq_offset;
> -		tcp_iov[iov_i].iov_len = len - seq_offset;
> -		seq_from_tap += tcp_iov[iov_i].iov_len;
> -		iov_i++;
> +		count = iov_copy(&tcp_iov[iov_i], UIO_MAXIOV - iov_i,
> +				 &data.iov[0], data.cnt, data.off + seq_offset,
> +				 len - seq_offset);

Here again it matters if you run out of space in the destination iov,
and I don't think you have a check above which prevents it.

> +		seq_from_tap += iov_size(&tcp_iov[iov_i], count);

We already called iov_size() on &data above.  We should be able to
derive the total length here from that minus headers, without having
to recount the IOV, no?

> +		iov_i += count;
>  
>  		if (keep == i)
>  			keep = -1;

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

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