public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Laurent Vivier <lvivier@redhat.com>
Cc: passt-dev@passt.top
Subject: Re: [PATCH v3 1/4] packet: replace struct desc by struct iovec
Date: Tue, 20 Aug 2024 10:27:42 +1000	[thread overview]
Message-ID: <ZsPi_kyawUCKqB2D@zatzit.fritz.box> (raw)
In-Reply-To: <20240815155024.827956-2-lvivier@redhat.com>

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

On Thu, Aug 15, 2024 at 05:50:20PM +0200, Laurent Vivier wrote:
> To be able to manage buffers inside a shared memory provided
> by a VM via a vhost-user interface, we cannot rely on the fact
> that buffers are located in a pre-defined memory area and use
> a base address and a 32bit offset to address them.
> 
> We need a 64bit address, so replace struct desc by struct iovec
> and update range checking.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  packet.c | 80 ++++++++++++++++++++++++++++++--------------------------
>  packet.h | 14 ++--------
>  2 files changed, 45 insertions(+), 49 deletions(-)
> 
> diff --git a/packet.c b/packet.c
> index ccfc84607709..37489961a37e 100644
> --- a/packet.c
> +++ b/packet.c
> @@ -22,6 +22,35 @@
>  #include "util.h"
>  #include "log.h"
>  
> +/**
> + * packet_check_range() - Check if a packet memory range is valid
> + * @p:		Packet pool
> + * @offset:	Offset of data range in packet descriptor
> + * @len:	Length of desired data range
> + * @start:	Start of the packet descriptor
> + * @func:	For tracing: name of calling function
> + * @line:	For tracing: caller line of function call
> + *
> + * Return: 0 if the range is valid, -1 otherwise
> + */
> +static int packet_check_range(const struct pool *p, size_t offset, size_t len,
> +			      const char *start, const char *func, int line)
> +{
> +	if (start < p->buf) {
> +		trace("packet start %p before buffer start %p, "
> +		      "%s:%i", (void *)start, (void *)p->buf, func, line);
> +		return -1;
> +	}
> +
> +	if (start + len + offset > p->buf + p->buf_size) {
> +		trace("packet offset plus length %lu from size %lu, "
> +		      "%s:%i", start - p->buf + len + offset,
> +		      p->buf_size, func, line);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
>  /**
>   * packet_add_do() - Add data as packet descriptor to given pool
>   * @p:		Existing pool
> @@ -41,34 +70,16 @@ void packet_add_do(struct pool *p, size_t len, const char *start,
>  		return;
>  	}
>  
> -	if (start < p->buf) {
> -		trace("add packet start %p before buffer start %p, %s:%i",
> -		      (void *)start, (void *)p->buf, func, line);
> +	if (packet_check_range(p, 0, len, start, func, line))
>  		return;
> -	}
> -
> -	if (start + len > p->buf + p->buf_size) {
> -		trace("add packet start %p, length: %zu, buffer end %p, %s:%i",
> -		      (void *)start, len, (void *)(p->buf + p->buf_size),
> -		      func, line);
> -		return;
> -	}
>  
>  	if (len > UINT16_MAX) {
>  		trace("add packet length %zu, %s:%i", len, func, line);
>  		return;
>  	}
>  
> -#if UINTPTR_MAX == UINT64_MAX
> -	if ((uintptr_t)start - (uintptr_t)p->buf > UINT32_MAX) {
> -		trace("add packet start %p, buffer start %p, %s:%i",
> -		      (void *)start, (void *)p->buf, func, line);
> -		return;
> -	}
> -#endif
> -
> -	p->pkt[idx].offset = start - p->buf;
> -	p->pkt[idx].len = len;
> +	p->pkt[idx].iov_base = (void *)start;
> +	p->pkt[idx].iov_len = len;
>  
>  	p->count++;
>  }
> @@ -96,36 +107,31 @@ void *packet_get_do(const struct pool *p, size_t idx, size_t offset,
>  		return NULL;
>  	}
>  
> -	if (len > UINT16_MAX || len + offset > UINT32_MAX) {
> +	if (len > UINT16_MAX) {
>  		if (func) {
> -			trace("packet data length %zu, offset %zu, %s:%i",
> -			      len, offset, func, line);
> +			trace("packet data length %zu, %s:%i",
> +			      len, func, line);
>  		}
>  		return NULL;
>  	}
>  
> -	if (p->pkt[idx].offset + len + offset > p->buf_size) {
> +	if (len + offset > p->pkt[idx].iov_len) {
>  		if (func) {
> -			trace("packet offset plus length %zu from size %zu, "
> -			      "%s:%i", p->pkt[idx].offset + len + offset,
> -			      p->buf_size, func, line);
> +			trace("data length %zu, offset %zu from length %zu, "
> +			      "%s:%i", len, offset, p->pkt[idx].iov_len,
> +			      func, line);
>  		}
>  		return NULL;
>  	}
>  
> -	if (len + offset > p->pkt[idx].len) {
> -		if (func) {
> -			trace("data length %zu, offset %zu from length %u, "
> -			      "%s:%i", len, offset, p->pkt[idx].len,
> -			      func, line);
> -		}
> +	if (packet_check_range(p, offset, len, p->pkt[idx].iov_base,
> +			       func, line))
>  		return NULL;
> -	}
>  
>  	if (left)
> -		*left = p->pkt[idx].len - offset - len;
> +		*left = p->pkt[idx].iov_len - offset - len;
>  
> -	return p->buf + p->pkt[idx].offset + offset;
> +	return (char *)p->pkt[idx].iov_base + offset;
>  }
>  
>  /**
> diff --git a/packet.h b/packet.h
> index a784b07bbed5..8377dcf678bb 100644
> --- a/packet.h
> +++ b/packet.h
> @@ -6,16 +6,6 @@
>  #ifndef PACKET_H
>  #define PACKET_H
>  
> -/**
> - * struct desc - Generic offset-based descriptor within buffer
> - * @offset:	Offset of descriptor relative to buffer start, 32-bit limit
> - * @len:	Length of descriptor, host order, 16-bit limit
> - */
> -struct desc {
> -	uint32_t offset;
> -	uint16_t len;
> -};
> -
>  /**
>   * struct pool - Generic pool of packets stored in a buffer
>   * @buf:	Buffer storing packet descriptors
> @@ -29,7 +19,7 @@ struct pool {
>  	size_t buf_size;
>  	size_t size;
>  	size_t count;
> -	struct desc pkt[1];
> +	struct iovec pkt[1];
>  };
>  
>  void packet_add_do(struct pool *p, size_t len, const char *start,
> @@ -54,7 +44,7 @@ struct _name ## _t {							\
>  	size_t buf_size;						\
>  	size_t size;							\
>  	size_t count;							\
> -	struct desc pkt[_size];						\
> +	struct iovec pkt[_size];					\
>  }
>  
>  #define PACKET_POOL_INIT_NOCAST(_size, _buf, _buf_size)			\

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

  reply	other threads:[~2024-08-20  0:49 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-15 15:50 [PATCH v3 0/4] Add vhost-user support to passt. (part 3) Laurent Vivier
2024-08-15 15:50 ` [PATCH v3 1/4] packet: replace struct desc by struct iovec Laurent Vivier
2024-08-20  0:27   ` David Gibson [this message]
2024-08-15 15:50 ` [PATCH v3 2/4] vhost-user: introduce virtio API Laurent Vivier
2024-08-20  1:00   ` David Gibson
2024-08-22 22:14   ` Stefano Brivio
2024-08-15 15:50 ` [PATCH v3 3/4] vhost-user: introduce vhost-user API Laurent Vivier
2024-08-22 22:14   ` Stefano Brivio
2024-08-26  5:27     ` David Gibson
2024-08-26  7:55       ` Stefano Brivio
2024-08-26  9:53         ` David Gibson
2024-08-26  5:26   ` David Gibson
2024-08-26 22:14     ` Stefano Brivio
2024-08-27  4:42       ` David Gibson
2024-09-05  9:58     ` Laurent Vivier
2024-08-15 15:50 ` [PATCH v3 4/4] vhost-user: add vhost-user Laurent Vivier
2024-08-22  9:59   ` Stefano Brivio
2024-08-22 22:14   ` Stefano Brivio
2024-08-23 12:32   ` Stefano Brivio
2024-08-20 22:41 ` [PATCH v3 0/4] Add vhost-user support to passt. (part 3) Stefano Brivio
2024-08-22 16:53   ` Stefano Brivio
2024-08-23 12:32     ` Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZsPi_kyawUCKqB2D@zatzit.fritz.box \
    --to=david@gibson.dropbear.id.au \
    --cc=lvivier@redhat.com \
    --cc=passt-dev@passt.top \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).