[PATCH v2 0/1] RFC: IOV tail helpers

[PATCH v2 0/1] RFC: IOV tail helpers

[David Gibson]

As mentioned here's my draft patch adding a helper to pull headers off
an IOV, checking for contiguity, alignment and so forth.  It's based
around the idea of an "iov tail", that is an iov from which you've
already consumed (in some sense) some data from the beginning.  It
uses an explicit offset to track this, so we don't have to copy an
entire iovec array in order to trim a bit off it.  But, it's built so
that if you pull enough off the front to remove some entire buffers,
it won't repeatedly have to step through them working out later
offsets.

It may be somewhat overengineered, let me know what you think of the
interface.

v2:
 * Substantial changes to comments and names, to make the purpose clearer

David Gibson (1):
  iov: iov tail helpers

 iov.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 iov.h | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 169 insertions(+)

-- 
2.47.0

[PATCH v2 1/1] iov: iov tail helpers

[David Gibson]

In the vhost-user code we have a number of places where we need to locate
a particular header within the guest-supplied IO vector.  We need to work
out which buffer the header is in, and verify that it's contiguous and
aligned as we need.  At the moment this is open-coded, but introduce a
helper to make this more straightforward.

We add a new datatype 'struct iov_tail' representing an IO vector from
which we've logically consumed some number of headers.  The IOV_PULL_HEADER
macro consumes a new header from the vector, returning a pointer and
updating the iov_tail.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 iov.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 iov.h | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 169 insertions(+)

diff --git a/iov.c b/iov.c
index 3f9e229..0a70f5c 100644
--- a/iov.c
+++ b/iov.c
@@ -156,3 +156,96 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt)
 
 	return len;
 }
+
+/**
+ * iov_tail_prune() - Remove any unneeded buffers from an IOV tail
+ * @tail:	IO vector tail (modified)
+ *
+ * If an IOV tail's offset is large enough, it may not include any bytes from
+ * the first (or first several) buffers in the underlying IO vector.  Modify the
+ * tail's representation so it contains the same logical bytes, but only
+ * includes buffers that are actually needed.  This will avoid stepping through
+ * unnecessary elements of the underlying IO vector on future operations.
+ *
+ * Return:	true if the tail still contains any bytes, otherwise false
+ */
+bool iov_tail_prune(struct iov_tail *tail)
+{
+	size_t i;
+
+	i = iov_skip_bytes(tail->iov, tail->cnt, tail->off, &tail->off);
+	tail->iov += i;
+	tail->cnt -= i;
+
+	return !!tail->cnt;
+}
+
+/**
+ * iov_tail_size - Calculate the total size of an IO vector tail
+ * @tail:	IO vector tail
+ *
+ * Returns:    The total size in bytes.
+ */
+/* cppcheck-suppress unusedFunction */
+size_t iov_tail_size(struct iov_tail *tail)
+{
+	iov_tail_prune(tail);
+	return iov_size(tail->iov, tail->cnt) - tail->off;
+}
+
+/**
+ * iov_peek_header_() - Get pointer to a header from an IOV tail
+ * @tail:	IOV tail to get header from
+ * @len:	Length of header to get, in bytes
+ * @align:	Required alignment of header, in bytes
+ *
+ * @tail may be pruned, but will represent the same bytes as before.
+ *
+ * Returns: Pointer to the first @len logical bytes of the tail, NULL if that
+ *	    overruns the IO vector, is not contiguous or doesn't have the
+ *	    requested alignment.
+ */
+void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align)
+{
+	char *p;
+
+	if (!iov_tail_prune(tail))
+		return NULL; /* Nothing left */
+
+	if (tail->off + len < tail->off)
+		return NULL; /* Overflow */
+
+	if (tail->off + len > tail->iov[0].iov_len)
+		return NULL; /* Not contiguous */
+
+	p = (char *)tail->iov[0].iov_base + tail->off;
+	if ((uintptr_t)p % align)
+		return NULL; /* not aligned */
+
+	return p;
+}
+
+/**
+ * iov_remove_header_() - Remove a header from an IOV tail
+ * @tail:	IOV tail to remove header from (modified)
+ * @len:	Length of header to remove, in bytes
+ * @align:	Required alignment of header, in bytes
+ *
+ * On success, @tail is updated so that it longer includes the bytes of the
+ * returned header.
+ *
+ * Returns: Pointer to the first @len logical bytes of the tail, NULL if that
+ *	    overruns the IO vector, is not contiguous or doesn't have the
+ *	    requested alignment.
+ */
+/* cppcheck-suppress unusedFunction */
+void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align)
+{
+	char *p = iov_peek_header_(tail, len, align);
+
+	if (!p)
+		return NULL;
+
+	tail->off = tail->off + len;
+	return p;
+}
diff --git a/iov.h b/iov.h
index a9e1722..9855bf0 100644
--- a/iov.h
+++ b/iov.h
@@ -28,4 +28,80 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
 size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
                   size_t offset, void *buf, size_t bytes);
 size_t iov_size(const struct iovec *iov, size_t iov_cnt);
+
+/*
+ * DOC: Theory of Operation, struct iov_tail
+ *
+ * Sometimes a single logical network frame is split across multiple buffers,
+ * represented by an IO vector (struct iovec[]).  We often want to process this
+ * one header / network layer at a time.  So, it's useful to maintain a "tail"
+ * of the vector representing the parts we haven't yet extracted.
+ *
+ * The headers we extract need not line up with buffer boundaries (though we do
+ * assume they're contiguous within a single buffer for now).  So, we could
+ * represent that tail as another struct iovec[], but that would mean copying
+ * the whole array of struct iovecs, just so we can adjust the offset and length
+ * on the first one.
+ *
+ * So, instead represent the tail as pointer into an existing struct iovec[],
+ * with an explicit offset for where the "tail" starts within it.  If we extract
+ * enough headers that some buffers of the original vector no longer contain
+ * part of the tail, we (lazily) advance our struct iovec * to the first buffer
+ * we still need, and adjust the vector length and offset to match.
+ */
+
+/**
+ * struct iov_tail - An IO vector which may have some headers logically removed
+ * @iov:	IO vector
+ * @cnt:	Number of entries in @iov
+ * @off:	Current offset in @iov
+ */
+struct iov_tail {
+	const struct iovec *iov;
+	size_t cnt, off;
+};
+
+/**
+ * IOV_TAIL() - Create a new IOV tail
+ * @iov_:	IO vector to create tail from
+ * @cnt_:	Length of the IO vector at @iov_
+ * @off_:	Byte offset in the IO vector where the tail begins
+ */
+#define IOV_TAIL(iov_, cnt_, off_) \
+	(struct iov_tail){ .iov = (iov_), .cnt = (cnt_), .off = (off_) }
+
+bool iov_tail_prune(struct iov_tail *tail);
+size_t iov_tail_size(struct iov_tail *tail);
+void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align);
+void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align);
+
+/**
+ * IOV_PEEK_HEADER() - Get typed pointer to a header from an IOV tail
+ * @tail_:	IOV tail to get header from
+ * @type_:	Data type of the header
+ *
+ * @tail_ may be pruned, but will represent the same bytes as before.
+ *
+ * Returns: Pointer of type (@type_ *) located at the start of @tail_, NULL if
+ *          we can't get a contiguous and aligned pointer.
+ */
+#define IOV_PEEK_HEADER(tail_, type_)					\
+	((type_ *)(iov_peek_header_((tail_),				\
+				    sizeof(type_), __alignof__(type_))))
+
+/**
+ * IOV_REMOVE_HEADER() - Remove and return typed header from an IOV tail
+ * @tail_:	IOV tail to remove header from (modified)
+ * @type_:	Data type of the header to remove
+ *
+ * On success, @tail_ is updated so that it longer includes the bytes of the
+ * returned header.
+ *
+ * Returns: Pointer of type (@type_ *) located at the old start of @tail_, NULL
+ *          if we can't get a contiguous and aligned pointer.
+ */
+#define IOV_REMOVE_HEADER(tail_, type_)					\
+	((type_ *)(iov_remove_header_((tail_),				\
+				      sizeof(type_), __alignof__(type_))))
+
 #endif /* IOVEC_H */
-- 
@@ -28,4 +28,80 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
 size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
                   size_t offset, void *buf, size_t bytes);
 size_t iov_size(const struct iovec *iov, size_t iov_cnt);
+
+/*
+ * DOC: Theory of Operation, struct iov_tail
+ *
+ * Sometimes a single logical network frame is split across multiple buffers,
+ * represented by an IO vector (struct iovec[]).  We often want to process this
+ * one header / network layer at a time.  So, it's useful to maintain a "tail"
+ * of the vector representing the parts we haven't yet extracted.
+ *
+ * The headers we extract need not line up with buffer boundaries (though we do
+ * assume they're contiguous within a single buffer for now).  So, we could
+ * represent that tail as another struct iovec[], but that would mean copying
+ * the whole array of struct iovecs, just so we can adjust the offset and length
+ * on the first one.
+ *
+ * So, instead represent the tail as pointer into an existing struct iovec[],
+ * with an explicit offset for where the "tail" starts within it.  If we extract
+ * enough headers that some buffers of the original vector no longer contain
+ * part of the tail, we (lazily) advance our struct iovec * to the first buffer
+ * we still need, and adjust the vector length and offset to match.
+ */
+
+/**
+ * struct iov_tail - An IO vector which may have some headers logically removed
+ * @iov:	IO vector
+ * @cnt:	Number of entries in @iov
+ * @off:	Current offset in @iov
+ */
+struct iov_tail {
+	const struct iovec *iov;
+	size_t cnt, off;
+};
+
+/**
+ * IOV_TAIL() - Create a new IOV tail
+ * @iov_:	IO vector to create tail from
+ * @cnt_:	Length of the IO vector at @iov_
+ * @off_:	Byte offset in the IO vector where the tail begins
+ */
+#define IOV_TAIL(iov_, cnt_, off_) \
+	(struct iov_tail){ .iov = (iov_), .cnt = (cnt_), .off = (off_) }
+
+bool iov_tail_prune(struct iov_tail *tail);
+size_t iov_tail_size(struct iov_tail *tail);
+void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align);
+void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align);
+
+/**
+ * IOV_PEEK_HEADER() - Get typed pointer to a header from an IOV tail
+ * @tail_:	IOV tail to get header from
+ * @type_:	Data type of the header
+ *
+ * @tail_ may be pruned, but will represent the same bytes as before.
+ *
+ * Returns: Pointer of type (@type_ *) located at the start of @tail_, NULL if
+ *          we can't get a contiguous and aligned pointer.
+ */
+#define IOV_PEEK_HEADER(tail_, type_)					\
+	((type_ *)(iov_peek_header_((tail_),				\
+				    sizeof(type_), __alignof__(type_))))
+
+/**
+ * IOV_REMOVE_HEADER() - Remove and return typed header from an IOV tail
+ * @tail_:	IOV tail to remove header from (modified)
+ * @type_:	Data type of the header to remove
+ *
+ * On success, @tail_ is updated so that it longer includes the bytes of the
+ * returned header.
+ *
+ * Returns: Pointer of type (@type_ *) located at the old start of @tail_, NULL
+ *          if we can't get a contiguous and aligned pointer.
+ */
+#define IOV_REMOVE_HEADER(tail_, type_)					\
+	((type_ *)(iov_remove_header_((tail_),				\
+				      sizeof(type_), __alignof__(type_))))
+
 #endif /* IOVEC_H */
-- 
2.47.0

Re: [PATCH v2 1/1] iov: iov tail helpers

[Stefano Brivio]

On Fri,  8 Nov 2024 15:19:30 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> In the vhost-user code we have a number of places where we need to locate
> a particular header within the guest-supplied IO vector.  We need to work
> out which buffer the header is in, and verify that it's contiguous and
> aligned as we need.  At the moment this is open-coded, but introduce a
> helper to make this more straightforward.
> 
> We add a new datatype 'struct iov_tail' representing an IO vector from
> which we've logically consumed some number of headers.  The IOV_PULL_HEADER
> macro consumes a new header from the vector, returning a pointer and
> updating the iov_tail.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  iov.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  iov.h | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 169 insertions(+)

This looks good to me now. Should I apply this, or wait until it's
actually used?

-- 
Stefano