* [PATCH v2 01/20] arp: Don't mix incoming and outgoing buffers
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 02/20] iov: Introduce iov_tail_drop() and iov_slice() Laurent Vivier
` (18 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier, David Gibson
Don't use the memory of the incoming packet to build the outgoing buffer
as it can be memory of the TX queue in the case of vhost-user.
Moreover with vhost-user, the packet can be splitted accross several
iovec and it's easier to rebuild it in a buffer than updating an
existing iovec array.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
arp.c | 84 ++++++++++++++++++++++++++++++++++++++---------------------
1 file changed, 55 insertions(+), 29 deletions(-)
diff --git a/arp.c b/arp.c
index fc482bbd9938..9d68d7c3b602 100644
--- a/arp.c
+++ b/arp.c
@@ -31,56 +31,82 @@
#include "tap.h"
/**
- * arp() - Check if this is a supported ARP message, reply as needed
+ * ignore_arp() - Check if this is a supported ARP message
* @c: Execution context
- * @p: Packet pool, single packet with Ethernet buffer
+ * @ah: ARP header
+ * @am: ARP message
*
- * Return: 1 if handled, -1 on failure
+ * Return: true if the message is supported, false otherwise.
*/
-int arp(const struct ctx *c, const struct pool *p)
+static bool ignore_arp(const struct ctx *c,
+ const struct arphdr *ah, const struct arpmsg *am)
{
- unsigned char swap[4];
- struct ethhdr *eh;
- struct arphdr *ah;
- struct arpmsg *am;
- size_t l2len;
-
- eh = packet_get(p, 0, 0, sizeof(*eh), NULL);
- ah = packet_get(p, 0, sizeof(*eh), sizeof(*ah), NULL);
- am = packet_get(p, 0, sizeof(*eh) + sizeof(*ah), sizeof(*am), NULL);
-
- if (!eh || !ah || !am)
- return -1;
-
if (ah->ar_hrd != htons(ARPHRD_ETHER) ||
ah->ar_pro != htons(ETH_P_IP) ||
ah->ar_hln != ETH_ALEN ||
ah->ar_pln != 4 ||
ah->ar_op != htons(ARPOP_REQUEST))
- return 1;
+ return true;
/* Discard announcements, but not 0.0.0.0 "probes" */
if (memcmp(am->sip, &in4addr_any, sizeof(am->sip)) &&
!memcmp(am->sip, am->tip, sizeof(am->sip)))
- return 1;
+ return true;
/* Don't resolve the guest's assigned address, either. */
if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip)))
+ return true;
+
+ return false;
+}
+
+/**
+ * arp() - Check if this is a supported ARP message, reply as needed
+ * @c: Execution context
+ * @p: Packet pool, single packet with Ethernet buffer
+ *
+ * Return: 1 if handled, -1 on failure
+ */
+int arp(const struct ctx *c, const struct pool *p)
+{
+ struct {
+ struct ethhdr eh;
+ struct arphdr ah;
+ struct arpmsg am;
+ } __attribute__((__packed__)) resp;
+ const struct ethhdr *eh;
+ const struct arphdr *ah;
+ const struct arpmsg *am;
+
+ eh = packet_get(p, 0, 0, sizeof(*eh), NULL);
+ ah = packet_get(p, 0, sizeof(*eh), sizeof(*ah), NULL);
+ am = packet_get(p, 0, sizeof(*eh) + sizeof(*ah), sizeof(*am), NULL);
+
+ if (!eh || !ah || !am)
+ return -1;
+
+ if (ignore_arp(c, ah, am))
return 1;
- ah->ar_op = htons(ARPOP_REPLY);
- memcpy(am->tha, am->sha, sizeof(am->tha));
- memcpy(am->sha, c->our_tap_mac, sizeof(am->sha));
+ /* ethernet header */
+ resp.eh.h_proto = htons(ETH_P_ARP);
+ memcpy(resp.eh.h_dest, eh->h_source, sizeof(resp.eh.h_dest));
+ memcpy(resp.eh.h_source, c->our_tap_mac, sizeof(resp.eh.h_source));
- memcpy(swap, am->tip, sizeof(am->tip));
- memcpy(am->tip, am->sip, sizeof(am->tip));
- memcpy(am->sip, swap, sizeof(am->sip));
+ /* ARP header */
+ resp.ah.ar_op = htons(ARPOP_REPLY);
+ resp.ah.ar_hrd = ah->ar_hrd;
+ resp.ah.ar_pro = ah->ar_pro;
+ resp.ah.ar_hln = ah->ar_hln;
+ resp.ah.ar_pln = ah->ar_pln;
- l2len = sizeof(*eh) + sizeof(*ah) + sizeof(*am);
- memcpy(eh->h_dest, eh->h_source, sizeof(eh->h_dest));
- memcpy(eh->h_source, c->our_tap_mac, sizeof(eh->h_source));
+ /* ARP message */
+ memcpy(resp.am.sha, c->our_tap_mac, sizeof(resp.am.sha));
+ memcpy(resp.am.sip, am->tip, sizeof(resp.am.sip));
+ memcpy(resp.am.tha, am->sha, sizeof(resp.am.tha));
+ memcpy(resp.am.tip, am->sip, sizeof(resp.am.tip));
- tap_send_single(c, eh, l2len);
+ tap_send_single(c, &resp, sizeof(resp));
return 1;
}
--
@@ -31,56 +31,82 @@
#include "tap.h"
/**
- * arp() - Check if this is a supported ARP message, reply as needed
+ * ignore_arp() - Check if this is a supported ARP message
* @c: Execution context
- * @p: Packet pool, single packet with Ethernet buffer
+ * @ah: ARP header
+ * @am: ARP message
*
- * Return: 1 if handled, -1 on failure
+ * Return: true if the message is supported, false otherwise.
*/
-int arp(const struct ctx *c, const struct pool *p)
+static bool ignore_arp(const struct ctx *c,
+ const struct arphdr *ah, const struct arpmsg *am)
{
- unsigned char swap[4];
- struct ethhdr *eh;
- struct arphdr *ah;
- struct arpmsg *am;
- size_t l2len;
-
- eh = packet_get(p, 0, 0, sizeof(*eh), NULL);
- ah = packet_get(p, 0, sizeof(*eh), sizeof(*ah), NULL);
- am = packet_get(p, 0, sizeof(*eh) + sizeof(*ah), sizeof(*am), NULL);
-
- if (!eh || !ah || !am)
- return -1;
-
if (ah->ar_hrd != htons(ARPHRD_ETHER) ||
ah->ar_pro != htons(ETH_P_IP) ||
ah->ar_hln != ETH_ALEN ||
ah->ar_pln != 4 ||
ah->ar_op != htons(ARPOP_REQUEST))
- return 1;
+ return true;
/* Discard announcements, but not 0.0.0.0 "probes" */
if (memcmp(am->sip, &in4addr_any, sizeof(am->sip)) &&
!memcmp(am->sip, am->tip, sizeof(am->sip)))
- return 1;
+ return true;
/* Don't resolve the guest's assigned address, either. */
if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip)))
+ return true;
+
+ return false;
+}
+
+/**
+ * arp() - Check if this is a supported ARP message, reply as needed
+ * @c: Execution context
+ * @p: Packet pool, single packet with Ethernet buffer
+ *
+ * Return: 1 if handled, -1 on failure
+ */
+int arp(const struct ctx *c, const struct pool *p)
+{
+ struct {
+ struct ethhdr eh;
+ struct arphdr ah;
+ struct arpmsg am;
+ } __attribute__((__packed__)) resp;
+ const struct ethhdr *eh;
+ const struct arphdr *ah;
+ const struct arpmsg *am;
+
+ eh = packet_get(p, 0, 0, sizeof(*eh), NULL);
+ ah = packet_get(p, 0, sizeof(*eh), sizeof(*ah), NULL);
+ am = packet_get(p, 0, sizeof(*eh) + sizeof(*ah), sizeof(*am), NULL);
+
+ if (!eh || !ah || !am)
+ return -1;
+
+ if (ignore_arp(c, ah, am))
return 1;
- ah->ar_op = htons(ARPOP_REPLY);
- memcpy(am->tha, am->sha, sizeof(am->tha));
- memcpy(am->sha, c->our_tap_mac, sizeof(am->sha));
+ /* ethernet header */
+ resp.eh.h_proto = htons(ETH_P_ARP);
+ memcpy(resp.eh.h_dest, eh->h_source, sizeof(resp.eh.h_dest));
+ memcpy(resp.eh.h_source, c->our_tap_mac, sizeof(resp.eh.h_source));
- memcpy(swap, am->tip, sizeof(am->tip));
- memcpy(am->tip, am->sip, sizeof(am->tip));
- memcpy(am->sip, swap, sizeof(am->sip));
+ /* ARP header */
+ resp.ah.ar_op = htons(ARPOP_REPLY);
+ resp.ah.ar_hrd = ah->ar_hrd;
+ resp.ah.ar_pro = ah->ar_pro;
+ resp.ah.ar_hln = ah->ar_hln;
+ resp.ah.ar_pln = ah->ar_pln;
- l2len = sizeof(*eh) + sizeof(*ah) + sizeof(*am);
- memcpy(eh->h_dest, eh->h_source, sizeof(eh->h_dest));
- memcpy(eh->h_source, c->our_tap_mac, sizeof(eh->h_source));
+ /* ARP message */
+ memcpy(resp.am.sha, c->our_tap_mac, sizeof(resp.am.sha));
+ memcpy(resp.am.sip, am->tip, sizeof(resp.am.sip));
+ memcpy(resp.am.tha, am->sha, sizeof(resp.am.tha));
+ memcpy(resp.am.tip, am->sip, sizeof(resp.am.tip));
- tap_send_single(c, eh, l2len);
+ tap_send_single(c, &resp, sizeof(resp));
return 1;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 02/20] iov: Introduce iov_tail_drop() and iov_slice()
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 01/20] arp: Don't mix incoming and outgoing buffers Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-14 2:14 ` David Gibson
2025-04-11 13:10 ` [PATCH v2 03/20] iov: Update IOV_REMOVE_HEADER() and IOV_PEEK_HEADER() Laurent Vivier
` (17 subsequent siblings)
19 siblings, 1 reply; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
iov_tail_drop() discards a header from the iov_tail,
iov_slice() makes a new iov referencing a subset of the data
in the original iov.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
iov.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
iov.h | 4 ++++
2 files changed, 72 insertions(+)
diff --git a/iov.c b/iov.c
index 8c63b7ea6e31..d2aca59e42ed 100644
--- a/iov.c
+++ b/iov.c
@@ -156,6 +156,59 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt)
return len;
}
+/**
+ * iov_slice - Make a new iov referencing a subset of the data in the original iov
+ *
+ * @dst_iov: Pointer to the destination array of struct iovec describing
+ * the scatter/gather I/O vector to copy to.
+ * @dst_iov_cnt: Maximum number of elements in the destination iov array.
+ * @iov: Pointer to the source array of struct iovec describing
+ * the scatter/gather I/O vector to copy from.
+ * @iov_cnt: Maximum number of elements in the source iov array.
+ * @offset: Offset within the source iov from where copying should start.
+ * @bytes: On input, total number of bytes to copy from @iov to @dst_iov,
+ * if @bytes is NULL, copy all the content of @iov,
+ * on output actual number of bytes copied.
+ *
+ * Returns: The number of elements successfully copied to the destination
+ * iov array, a negative value if there is no enough room in the
+ * destination iov array
+ */
+/* cppcheck-suppress unusedFunction */
+int iov_slice(struct iovec *dst_iov, size_t dst_iov_cnt,
+ const struct iovec *iov, size_t iov_cnt,
+ size_t offset, size_t *bytes)
+{
+ unsigned int i, j;
+ size_t remaining = bytes ? *bytes : SIZE_MAX;
+
+ i = iov_skip_bytes(iov, iov_cnt, offset, &offset);
+
+ /* copying data */
+ for (j = 0; i < iov_cnt && j < dst_iov_cnt && remaining; i++) {
+ size_t len = MIN(remaining, iov[i].iov_len - offset);
+
+ dst_iov[j].iov_base = (char *)iov[i].iov_base + offset;
+ dst_iov[j].iov_len = len;
+ j++;
+ remaining -= len;
+ offset = 0;
+ }
+ if (j == dst_iov_cnt) {
+ if (bytes) {
+ if (remaining)
+ return -1;
+ } else if (i != iov_cnt) {
+ return -1;
+ }
+ }
+
+ if (bytes)
+ *bytes -= remaining;
+
+ return j;
+}
+
/**
* iov_tail_prune() - Remove any unneeded buffers from an IOV tail
* @tail: IO vector tail (modified)
@@ -191,6 +244,21 @@ size_t iov_tail_size(struct iov_tail *tail)
return iov_size(tail->iov, tail->cnt) - tail->off;
}
+/**
+ * iov_tail_drop() - Discard a header from an IOV tail
+ * @tail: IO vector tail
+ * @len: length to move the head of the tail
+ *
+ * Returns: true if the tail still contains any bytes, otherwise false
+ */
+/* cppcheck-suppress unusedFunction */
+bool iov_tail_drop(struct iov_tail *tail, size_t len)
+{
+ tail->off = tail->off + len;
+
+ return iov_tail_prune(tail);
+}
+
/**
* iov_peek_header_() - Get pointer to a header from an IOV tail
* @tail: IOV tail to get header from
diff --git a/iov.h b/iov.h
index 9855bf0c0c32..3f8259aa8d28 100644
--- a/iov.h
+++ b/iov.h
@@ -28,6 +28,9 @@ 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);
+int iov_slice(struct iovec *dst_iov, size_t dst_iov_cnt,
+ const struct iovec *iov, size_t iov_cnt,
+ size_t offset, size_t *bytes);
/*
* DOC: Theory of Operation, struct iov_tail
@@ -72,6 +75,7 @@ struct iov_tail {
bool iov_tail_prune(struct iov_tail *tail);
size_t iov_tail_size(struct iov_tail *tail);
+bool iov_tail_drop(struct iov_tail *tail, size_t len);
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);
--
@@ -28,6 +28,9 @@ 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);
+int iov_slice(struct iovec *dst_iov, size_t dst_iov_cnt,
+ const struct iovec *iov, size_t iov_cnt,
+ size_t offset, size_t *bytes);
/*
* DOC: Theory of Operation, struct iov_tail
@@ -72,6 +75,7 @@ struct iov_tail {
bool iov_tail_prune(struct iov_tail *tail);
size_t iov_tail_size(struct iov_tail *tail);
+bool iov_tail_drop(struct iov_tail *tail, size_t len);
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);
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 02/20] iov: Introduce iov_tail_drop() and iov_slice()
2025-04-11 13:10 ` [PATCH v2 02/20] iov: Introduce iov_tail_drop() and iov_slice() Laurent Vivier
@ 2025-04-14 2:14 ` David Gibson
0 siblings, 0 replies; 29+ messages in thread
From: David Gibson @ 2025-04-14 2:14 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 4656 bytes --]
On Fri, Apr 11, 2025 at 03:10:12PM +0200, Laurent Vivier wrote:
> iov_tail_drop() discards a header from the iov_tail,
>
> iov_slice() makes a new iov referencing a subset of the data
> in the original iov.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
For the actual behaviour, couple of cosmetic nits noted below.
> ---
> iov.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> iov.h | 4 ++++
> 2 files changed, 72 insertions(+)
>
> diff --git a/iov.c b/iov.c
> index 8c63b7ea6e31..d2aca59e42ed 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -156,6 +156,59 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt)
> return len;
> }
>
> +/**
> + * iov_slice - Make a new iov referencing a subset of the data in the original iov
> + *
> + * @dst_iov: Pointer to the destination array of struct iovec describing
> + * the scatter/gather I/O vector to copy to.
> + * @dst_iov_cnt: Maximum number of elements in the destination iov array.
> + * @iov: Pointer to the source array of struct iovec describing
> + * the scatter/gather I/O vector to copy from.
> + * @iov_cnt: Maximum number of elements in the source iov array.
> + * @offset: Offset within the source iov from where copying should start.
> + * @bytes: On input, total number of bytes to copy from @iov to @dst_iov,
> + * if @bytes is NULL, copy all the content of @iov,
> + * on output actual number of bytes copied.
> + *
> + * Returns: The number of elements successfully copied to the destination
> + * iov array, a negative value if there is no enough room in the
s/no/not/
> + * destination iov array
> + */
> +/* cppcheck-suppress unusedFunction */
> +int iov_slice(struct iovec *dst_iov, size_t dst_iov_cnt,
> + const struct iovec *iov, size_t iov_cnt,
> + size_t offset, size_t *bytes)
> +{
> + unsigned int i, j;
> + size_t remaining = bytes ? *bytes : SIZE_MAX;
> +
> + i = iov_skip_bytes(iov, iov_cnt, offset, &offset);
> +
> + /* copying data */
I think this comment is still a bit misleading.
> + for (j = 0; i < iov_cnt && j < dst_iov_cnt && remaining; i++) {
> + size_t len = MIN(remaining, iov[i].iov_len - offset);
> +
> + dst_iov[j].iov_base = (char *)iov[i].iov_base + offset;
> + dst_iov[j].iov_len = len;
> + j++;
> + remaining -= len;
> + offset = 0;
> + }
> + if (j == dst_iov_cnt) {
> + if (bytes) {
> + if (remaining)
> + return -1;
> + } else if (i != iov_cnt) {
> + return -1;
> + }
> + }
> +
> + if (bytes)
> + *bytes -= remaining;
> +
> + return j;
> +}
> +
> /**
> * iov_tail_prune() - Remove any unneeded buffers from an IOV tail
> * @tail: IO vector tail (modified)
> @@ -191,6 +244,21 @@ size_t iov_tail_size(struct iov_tail *tail)
> return iov_size(tail->iov, tail->cnt) - tail->off;
> }
>
> +/**
> + * iov_tail_drop() - Discard a header from an IOV tail
> + * @tail: IO vector tail
> + * @len: length to move the head of the tail
> + *
> + * Returns: true if the tail still contains any bytes, otherwise false
> + */
> +/* cppcheck-suppress unusedFunction */
> +bool iov_tail_drop(struct iov_tail *tail, size_t len)
> +{
> + tail->off = tail->off + len;
> +
> + return iov_tail_prune(tail);
> +}
> +
> /**
> * iov_peek_header_() - Get pointer to a header from an IOV tail
> * @tail: IOV tail to get header from
> diff --git a/iov.h b/iov.h
> index 9855bf0c0c32..3f8259aa8d28 100644
> --- a/iov.h
> +++ b/iov.h
> @@ -28,6 +28,9 @@ 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);
> +int iov_slice(struct iovec *dst_iov, size_t dst_iov_cnt,
> + const struct iovec *iov, size_t iov_cnt,
> + size_t offset, size_t *bytes);
>
> /*
> * DOC: Theory of Operation, struct iov_tail
> @@ -72,6 +75,7 @@ struct iov_tail {
>
> bool iov_tail_prune(struct iov_tail *tail);
> size_t iov_tail_size(struct iov_tail *tail);
> +bool iov_tail_drop(struct iov_tail *tail, size_t len);
> 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);
>
--
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 --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 03/20] iov: Update IOV_REMOVE_HEADER() and IOV_PEEK_HEADER()
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 01/20] arp: Don't mix incoming and outgoing buffers Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 02/20] iov: Introduce iov_tail_drop() and iov_slice() Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-14 2:19 ` David Gibson
2025-04-11 13:10 ` [PATCH v2 04/20] tap: Use iov_tail with tap_add_packet() Laurent Vivier
` (16 subsequent siblings)
19 siblings, 1 reply; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
Provide a temporary variable of the wanted type to store
the header if the memory in the iovec array is not contiguous.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
iov.c | 53 ++++++++++++++++++++++++++++++++++++++++++++---------
iov.h | 52 ++++++++++++++++++++++++++++++++++++++--------------
tcp_buf.c | 2 +-
3 files changed, 83 insertions(+), 24 deletions(-)
diff --git a/iov.c b/iov.c
index d2aca59e42ed..ae8eca26f295 100644
--- a/iov.c
+++ b/iov.c
@@ -108,7 +108,7 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
*
* Returns: The number of bytes successfully copied.
*/
-/* cppcheck-suppress unusedFunction */
+/* cppcheck-suppress [staticFunction] */
size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
size_t offset, void *buf, size_t bytes)
{
@@ -126,6 +126,7 @@ size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
/* copying data */
for (copied = 0; copied < bytes && i < iov_cnt; i++) {
size_t len = MIN(iov[i].iov_len - offset, bytes - copied);
+ /* NOLINTNEXTLINE(clang-analyzer-core.NonNullParamChecker) */
memcpy((char *)buf + copied, (char *)iov[i].iov_base + offset,
len);
copied += len;
@@ -260,7 +261,7 @@ bool iov_tail_drop(struct iov_tail *tail, size_t len)
}
/**
- * iov_peek_header_() - Get pointer to a header from an IOV tail
+ * iov_check_header() - Check if a header can be accessed
* @tail: IOV tail to get header from
* @len: Length of header to get, in bytes
* @align: Required alignment of header, in bytes
@@ -271,8 +272,7 @@ bool iov_tail_drop(struct iov_tail *tail, size_t len)
* overruns the IO vector, is not contiguous or doesn't have the
* requested alignment.
*/
-/* cppcheck-suppress [staticFunction,unmatchedSuppression] */
-void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align)
+static void *iov_check_header(struct iov_tail *tail, size_t len, size_t align)
{
char *p;
@@ -292,26 +292,61 @@ void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align)
return p;
}
+/**
+ * iov_peek_header_() - Get pointer to a header from an IOV tail
+ * @tail: IOV tail to get header from
+ * @v: Temporary memory to use if the memory in @tail
+ * is discontinuous
+ * @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, or to
+ * a copy if that overruns the IO vector, is not contiguous or
+ * doesn't have the requested alignment. NULL if that overruns the
+ * IO vector.
+ */
+/* cppcheck-suppress [staticFunction,unmatchedSuppression] */
+void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, size_t align)
+{
+ char *p = iov_check_header(tail, len, align);
+ size_t l;
+
+ if (p)
+ return p;
+
+ l = iov_to_buf(tail->iov, tail->cnt, tail->off, v, len);
+ if (l != len)
+ return NULL;
+
+ return v;
+}
+
/**
* iov_remove_header_() - Remove a header from an IOV tail
* @tail: IOV tail to remove header from (modified)
+ * @v: Temporary memory to use if the memory in @tail
+ * is discontinuous
* @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.
+ * Returns: Pointer to the first @len logical bytes of the tail, or to
+ * a copy if that overruns the IO vector, is not contiguous or
+ * doesn't have the requested alignment. NULL if that overruns the
+ * IO vector.
*/
-void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align)
+void *iov_remove_header_(struct iov_tail *tail, void *v, size_t len, size_t align)
{
- char *p = iov_peek_header_(tail, len, align);
+ char *p = iov_peek_header_(tail, v, len, align);
if (!p)
return NULL;
tail->off = tail->off + len;
+
return p;
}
diff --git a/iov.h b/iov.h
index 3f8259aa8d28..a63d8f21857a 100644
--- a/iov.h
+++ b/iov.h
@@ -73,39 +73,63 @@ struct iov_tail {
#define IOV_TAIL(iov_, cnt_, off_) \
(struct iov_tail){ .iov = (iov_), .cnt = (cnt_), .off = (off_) }
+/**
+ * IOV_TAIL_FROM_BUF() - Create a new IOV tail from a buffer
+ * @buf_: Buffer address to use in the iovec
+ * @len_: Buffer size
+ * @off_: Byte offset in the buffer where the tail begins
+ */
+#define IOV_TAIL_FROM_BUF(buf_, len_, off_) \
+ IOV_TAIL((&(const struct iovec){ .iov_base = (buf_), .iov_len = (len_) }), 1, (off_))
+
bool iov_tail_prune(struct iov_tail *tail);
size_t iov_tail_size(struct iov_tail *tail);
bool iov_tail_drop(struct iov_tail *tail, size_t len);
-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);
+void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, size_t align);
+void *iov_remove_header_(struct iov_tail *tail, void *v, 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
+ * @var_: Temporary buffer of the type of the header to use if
+ * the memory in the iovec array is not contiguous.
*
* @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.
+ * Returns: Pointer of type (@type_ *) located at the start of @tail_
+ * or to @var_ if iovec memory is not contiguous, NULL if
+ * that overruns the iovec.
*/
-#define IOV_PEEK_HEADER(tail_, type_) \
- ((type_ *)(iov_peek_header_((tail_), \
- sizeof(type_), __alignof__(type_))))
+
+#define IOV_PEEK_HEADER(tail_, var_) \
+ ((__typeof__(var_) *)(iov_peek_header_((tail_), &(var_), \
+ sizeof(var_), __alignof__(var_))))
/**
* 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
+ * @var_: Temporary buffer of the type of the header to use if
+ * the memory in the iovec array is not contiguous.
*
* 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.
+ * Returns: Pointer of type (@type_ *) located at the start of @tail_
+ * or to @var_ if iovec memory is not contiguous, NULL if
+ * that overruns the iovec.
+ */
+
+#define IOV_REMOVE_HEADER(tail_, var_) \
+ ((__typeof__(var_) *)(iov_remove_header_((tail_), &(var_), \
+ sizeof(var_), __alignof__(var_))))
+
+/** IOV_DROP_HEADER() -- Remove a typed header from an IOV tail
+ * @tail_: IOV tail to remove header from (modified)
+ * @type_: Data type of the header to remove
+ *
+ * Returns: true if the tail still contains any bytes, otherwise false
*/
-#define IOV_REMOVE_HEADER(tail_, type_) \
- ((type_ *)(iov_remove_header_((tail_), \
- sizeof(type_), __alignof__(type_))))
+#define IOV_DROP_HEADER(tail_, type_) \
+ iov_tail_drop((tail_), sizeof(type_))
#endif /* IOVEC_H */
diff --git a/tcp_buf.c b/tcp_buf.c
index 05305636b503..4bcc1acb245a 100644
--- a/tcp_buf.c
+++ b/tcp_buf.c
@@ -160,7 +160,7 @@ static void tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn,
uint32_t seq, bool no_tcp_csum)
{
struct iov_tail tail = IOV_TAIL(&iov[TCP_IOV_PAYLOAD], 1, 0);
- struct tcphdr *th = IOV_REMOVE_HEADER(&tail, struct tcphdr);
+ struct tcphdr thc, *th = IOV_REMOVE_HEADER(&tail, thc);
struct tap_hdr *taph = iov[TCP_IOV_TAP].iov_base;
const struct flowside *tapside = TAPFLOW(conn);
const struct in_addr *a4 = inany_v4(&tapside->oaddr);
--
@@ -160,7 +160,7 @@ static void tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn,
uint32_t seq, bool no_tcp_csum)
{
struct iov_tail tail = IOV_TAIL(&iov[TCP_IOV_PAYLOAD], 1, 0);
- struct tcphdr *th = IOV_REMOVE_HEADER(&tail, struct tcphdr);
+ struct tcphdr thc, *th = IOV_REMOVE_HEADER(&tail, thc);
struct tap_hdr *taph = iov[TCP_IOV_TAP].iov_base;
const struct flowside *tapside = TAPFLOW(conn);
const struct in_addr *a4 = inany_v4(&tapside->oaddr);
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 03/20] iov: Update IOV_REMOVE_HEADER() and IOV_PEEK_HEADER()
2025-04-11 13:10 ` [PATCH v2 03/20] iov: Update IOV_REMOVE_HEADER() and IOV_PEEK_HEADER() Laurent Vivier
@ 2025-04-14 2:19 ` David Gibson
0 siblings, 0 replies; 29+ messages in thread
From: David Gibson @ 2025-04-14 2:19 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 9196 bytes --]
On Fri, Apr 11, 2025 at 03:10:13PM +0200, Laurent Vivier wrote:
> Provide a temporary variable of the wanted type to store
> the header if the memory in the iovec array is not contiguous.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> iov.c | 53 ++++++++++++++++++++++++++++++++++++++++++++---------
> iov.h | 52 ++++++++++++++++++++++++++++++++++++++--------------
> tcp_buf.c | 2 +-
> 3 files changed, 83 insertions(+), 24 deletions(-)
>
> diff --git a/iov.c b/iov.c
> index d2aca59e42ed..ae8eca26f295 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -108,7 +108,7 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
> *
> * Returns: The number of bytes successfully copied.
> */
> -/* cppcheck-suppress unusedFunction */
> +/* cppcheck-suppress [staticFunction] */
> size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
> size_t offset, void *buf, size_t bytes)
> {
> @@ -126,6 +126,7 @@ size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
> /* copying data */
> for (copied = 0; copied < bytes && i < iov_cnt; i++) {
> size_t len = MIN(iov[i].iov_len - offset, bytes - copied);
> + /* NOLINTNEXTLINE(clang-analyzer-core.NonNullParamChecker) */
It's not clear to me why this extra lint suppression is needed. Is it
related to the changes in this patch, or is it an unrelated fix?
> memcpy((char *)buf + copied, (char *)iov[i].iov_base + offset,
> len);
> copied += len;
> @@ -260,7 +261,7 @@ bool iov_tail_drop(struct iov_tail *tail, size_t len)
> }
>
> /**
> - * iov_peek_header_() - Get pointer to a header from an IOV tail
> + * iov_check_header() - Check if a header can be accessed
> * @tail: IOV tail to get header from
> * @len: Length of header to get, in bytes
> * @align: Required alignment of header, in bytes
> @@ -271,8 +272,7 @@ bool iov_tail_drop(struct iov_tail *tail, size_t len)
> * overruns the IO vector, is not contiguous or doesn't have the
> * requested alignment.
> */
> -/* cppcheck-suppress [staticFunction,unmatchedSuppression] */
> -void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align)
> +static void *iov_check_header(struct iov_tail *tail, size_t len, size_t align)
> {
> char *p;
>
> @@ -292,26 +292,61 @@ void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align)
> return p;
> }
>
> +/**
> + * iov_peek_header_() - Get pointer to a header from an IOV tail
> + * @tail: IOV tail to get header from
> + * @v: Temporary memory to use if the memory in @tail
> + * is discontinuous
> + * @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, or to
> + * a copy if that overruns the IO vector, is not contiguous or
> + * doesn't have the requested alignment. NULL if that overruns the
> + * IO vector.
> + */
> +/* cppcheck-suppress [staticFunction,unmatchedSuppression] */
> +void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, size_t align)
> +{
> + char *p = iov_check_header(tail, len, align);
> + size_t l;
> +
> + if (p)
> + return p;
> +
> + l = iov_to_buf(tail->iov, tail->cnt, tail->off, v, len);
> + if (l != len)
> + return NULL;
> +
> + return v;
> +}
> +
> /**
> * iov_remove_header_() - Remove a header from an IOV tail
> * @tail: IOV tail to remove header from (modified)
> + * @v: Temporary memory to use if the memory in @tail
> + * is discontinuous
> * @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.
> + * Returns: Pointer to the first @len logical bytes of the tail, or to
> + * a copy if that overruns the IO vector, is not contiguous or
> + * doesn't have the requested alignment. NULL if that overruns the
> + * IO vector.
> */
> -void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align)
> +void *iov_remove_header_(struct iov_tail *tail, void *v, size_t len, size_t align)
> {
> - char *p = iov_peek_header_(tail, len, align);
> + char *p = iov_peek_header_(tail, v, len, align);
>
> if (!p)
> return NULL;
>
> tail->off = tail->off + len;
> +
> return p;
> }
> diff --git a/iov.h b/iov.h
> index 3f8259aa8d28..a63d8f21857a 100644
> --- a/iov.h
> +++ b/iov.h
> @@ -73,39 +73,63 @@ struct iov_tail {
> #define IOV_TAIL(iov_, cnt_, off_) \
> (struct iov_tail){ .iov = (iov_), .cnt = (cnt_), .off = (off_) }
>
> +/**
> + * IOV_TAIL_FROM_BUF() - Create a new IOV tail from a buffer
> + * @buf_: Buffer address to use in the iovec
> + * @len_: Buffer size
> + * @off_: Byte offset in the buffer where the tail begins
> + */
> +#define IOV_TAIL_FROM_BUF(buf_, len_, off_) \
> + IOV_TAIL((&(const struct iovec){ .iov_base = (buf_), .iov_len = (len_) }), 1, (off_))
> +
> bool iov_tail_prune(struct iov_tail *tail);
> size_t iov_tail_size(struct iov_tail *tail);
> bool iov_tail_drop(struct iov_tail *tail, size_t len);
> -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);
> +void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, size_t align);
> +void *iov_remove_header_(struct iov_tail *tail, void *v, 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
> + * @var_: Temporary buffer of the type of the header to use if
> + * the memory in the iovec array is not contiguous.
> *
> * @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.
> + * Returns: Pointer of type (@type_ *) located at the start of @tail_
> + * or to @var_ if iovec memory is not contiguous, NULL if
> + * that overruns the iovec.
> */
> -#define IOV_PEEK_HEADER(tail_, type_) \
> - ((type_ *)(iov_peek_header_((tail_), \
> - sizeof(type_), __alignof__(type_))))
> +
> +#define IOV_PEEK_HEADER(tail_, var_) \
> + ((__typeof__(var_) *)(iov_peek_header_((tail_), &(var_), \
> + sizeof(var_), __alignof__(var_))))
>
> /**
> * 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
> + * @var_: Temporary buffer of the type of the header to use if
> + * the memory in the iovec array is not contiguous.
> *
> * 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.
> + * Returns: Pointer of type (@type_ *) located at the start of @tail_
> + * or to @var_ if iovec memory is not contiguous, NULL if
> + * that overruns the iovec.
> + */
> +
> +#define IOV_REMOVE_HEADER(tail_, var_) \
> + ((__typeof__(var_) *)(iov_remove_header_((tail_), &(var_), \
> + sizeof(var_), __alignof__(var_))))
> +
> +/** IOV_DROP_HEADER() -- Remove a typed header from an IOV tail
> + * @tail_: IOV tail to remove header from (modified)
> + * @type_: Data type of the header to remove
> + *
> + * Returns: true if the tail still contains any bytes, otherwise false
> */
> -#define IOV_REMOVE_HEADER(tail_, type_) \
> - ((type_ *)(iov_remove_header_((tail_), \
> - sizeof(type_), __alignof__(type_))))
> +#define IOV_DROP_HEADER(tail_, type_) \
> + iov_tail_drop((tail_), sizeof(type_))
>
> #endif /* IOVEC_H */
> diff --git a/tcp_buf.c b/tcp_buf.c
> index 05305636b503..4bcc1acb245a 100644
> --- a/tcp_buf.c
> +++ b/tcp_buf.c
> @@ -160,7 +160,7 @@ static void tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn,
> uint32_t seq, bool no_tcp_csum)
> {
> struct iov_tail tail = IOV_TAIL(&iov[TCP_IOV_PAYLOAD], 1, 0);
> - struct tcphdr *th = IOV_REMOVE_HEADER(&tail, struct tcphdr);
> + struct tcphdr thc, *th = IOV_REMOVE_HEADER(&tail, thc);
> struct tap_hdr *taph = iov[TCP_IOV_TAP].iov_base;
> const struct flowside *tapside = TAPFLOW(conn);
> const struct in_addr *a4 = inany_v4(&tapside->oaddr);
--
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 --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 04/20] tap: Use iov_tail with tap_add_packet()
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (2 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 03/20] iov: Update IOV_REMOVE_HEADER() and IOV_PEEK_HEADER() Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 05/20] packet: Use iov_tail with packet_add() Laurent Vivier
` (15 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier, David Gibson
Use IOV_PEEK_HEADER() to get the ethernet header from the iovec.
Move the workaround about multiple iovec array from vu_handle_tx() to
tap_add_packet(). Removing the offset out of the iovec array should
reduce the iovec count to 1.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
iov.c | 1 -
pcap.c | 1 +
tap.c | 30 +++++++++++++++++++++---------
tap.h | 3 +--
vu_common.c | 26 +++++---------------------
5 files changed, 28 insertions(+), 33 deletions(-)
diff --git a/iov.c b/iov.c
index ae8eca26f295..98d5fecbdc8f 100644
--- a/iov.c
+++ b/iov.c
@@ -252,7 +252,6 @@ size_t iov_tail_size(struct iov_tail *tail)
*
* Returns: true if the tail still contains any bytes, otherwise false
*/
-/* cppcheck-suppress unusedFunction */
bool iov_tail_drop(struct iov_tail *tail, size_t len)
{
tail->off = tail->off + len;
diff --git a/pcap.c b/pcap.c
index e95aa6fe29a6..404043a27e22 100644
--- a/pcap.c
+++ b/pcap.c
@@ -76,6 +76,7 @@ static void pcap_frame(const struct iovec *iov, size_t iovcnt,
* @pkt: Pointer to data buffer, including L2 headers
* @l2len: L2 frame length
*/
+/* cppcheck-suppress unusedFunction */
void pcap(const char *pkt, size_t l2len)
{
struct iovec iov = { (char *)pkt, l2len };
diff --git a/tap.c b/tap.c
index 3a6fcbe85a42..e247a8f5ebc2 100644
--- a/tap.c
+++ b/tap.c
@@ -1059,24 +1059,29 @@ void tap_handler(struct ctx *c, const struct timespec *now)
/**
* tap_add_packet() - Queue/capture packet, update notion of guest MAC address
* @c: Execution context
- * @l2len: Total L2 packet length
- * @p: Packet buffer
+ * @data: Packet to add to the pool
* @now: Current timestamp
*/
-void tap_add_packet(struct ctx *c, ssize_t l2len, char *p,
+void tap_add_packet(struct ctx *c, struct iov_tail *data,
const struct timespec *now)
{
const struct ethhdr *eh;
+ struct ethhdr ehc;
- pcap(p, l2len);
+ pcap_iov(data->iov, data->cnt, data->off);
- eh = (struct ethhdr *)p;
+ eh = IOV_PEEK_HEADER(data, ehc);
+ if (!eh)
+ return;
if (memcmp(c->guest_mac, eh->h_source, ETH_ALEN)) {
memcpy(c->guest_mac, eh->h_source, ETH_ALEN);
proto_update_l2_buf(c->guest_mac, NULL);
}
+ iov_tail_prune(data);
+ ASSERT(data->cnt == 1); /* packet_add() doesn't support iovec */
+
switch (ntohs(eh->h_proto)) {
case ETH_P_ARP:
case ETH_P_IP:
@@ -1084,14 +1089,16 @@ void tap_add_packet(struct ctx *c, ssize_t l2len, char *p,
tap4_handler(c, pool_tap4, now);
pool_flush(pool_tap4);
}
- packet_add(pool_tap4, l2len, p);
+ packet_add(pool_tap4, data->iov[0].iov_len - data->off,
+ (char *)data->iov[0].iov_base + data->off);
break;
case ETH_P_IPV6:
if (pool_full(pool_tap6)) {
tap6_handler(c, pool_tap6, now);
pool_flush(pool_tap6);
}
- packet_add(pool_tap6, l2len, p);
+ packet_add(pool_tap6, data->iov[0].iov_len - data->off,
+ (char *)data->iov[0].iov_base + data->off);
break;
default:
break;
@@ -1157,6 +1164,7 @@ static void tap_passt_input(struct ctx *c, const struct timespec *now)
while (n >= (ssize_t)sizeof(uint32_t)) {
uint32_t l2len = ntohl_unaligned(p);
+ struct iov_tail data;
if (l2len < sizeof(struct ethhdr) || l2len > L2_MAX_LEN_PASST) {
err("Bad frame size from guest, resetting connection");
@@ -1171,7 +1179,8 @@ static void tap_passt_input(struct ctx *c, const struct timespec *now)
p += sizeof(uint32_t);
n -= sizeof(uint32_t);
- tap_add_packet(c, l2len, p, now);
+ data = IOV_TAIL_FROM_BUF(p, l2len, 0);
+ tap_add_packet(c, &data, now);
p += l2len;
n -= l2len;
@@ -1215,6 +1224,8 @@ static void tap_pasta_input(struct ctx *c, const struct timespec *now)
for (n = 0;
n <= (ssize_t)(sizeof(pkt_buf) - L2_MAX_LEN_PASTA);
n += len) {
+ struct iov_tail data;
+
len = read(c->fd_tap, pkt_buf + n, L2_MAX_LEN_PASTA);
if (len == 0) {
@@ -1236,7 +1247,8 @@ static void tap_pasta_input(struct ctx *c, const struct timespec *now)
len > (ssize_t)L2_MAX_LEN_PASTA)
continue;
- tap_add_packet(c, len, pkt_buf + n, now);
+ data = IOV_TAIL_FROM_BUF(pkt_buf + n, len, 0);
+ tap_add_packet(c, &data, now);
}
tap_handler(c, now);
diff --git a/tap.h b/tap.h
index 6fe3d15d1337..954ad440a287 100644
--- a/tap.h
+++ b/tap.h
@@ -119,7 +119,6 @@ void tap_sock_update_pool(void *base, size_t size);
void tap_backend_init(struct ctx *c);
void tap_flush_pools(void);
void tap_handler(struct ctx *c, const struct timespec *now);
-void tap_add_packet(struct ctx *c, ssize_t l2len, char *p,
+void tap_add_packet(struct ctx *c, struct iov_tail *data,
const struct timespec *now);
-
#endif /* TAP_H */
diff --git a/vu_common.c b/vu_common.c
index 5e6fd4a8261f..b77b21420c57 100644
--- a/vu_common.c
+++ b/vu_common.c
@@ -163,7 +163,6 @@ static void vu_handle_tx(struct vu_dev *vdev, int index,
struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZE];
struct iovec out_sg[VIRTQUEUE_MAX_SIZE];
struct vu_virtq *vq = &vdev->vq[index];
- int hdrlen = sizeof(struct virtio_net_hdr_mrg_rxbuf);
int out_sg_count;
int count;
@@ -176,6 +175,7 @@ static void vu_handle_tx(struct vu_dev *vdev, int index,
while (count < VIRTQUEUE_MAX_SIZE &&
out_sg_count + VU_MAX_TX_BUFFER_NB <= VIRTQUEUE_MAX_SIZE) {
int ret;
+ struct iov_tail data;
elem[count].out_num = VU_MAX_TX_BUFFER_NB;
elem[count].out_sg = &out_sg[out_sg_count];
@@ -191,26 +191,10 @@ static void vu_handle_tx(struct vu_dev *vdev, int index,
warn("virtio-net transmit queue contains no out buffers");
break;
}
- if (elem[count].out_num == 1) {
- tap_add_packet(vdev->context,
- elem[count].out_sg[0].iov_len - hdrlen,
- (char *)elem[count].out_sg[0].iov_base +
- hdrlen, now);
- } else {
- /* vnet header can be in a separate iovec */
- if (elem[count].out_num != 2) {
- debug("virtio-net transmit queue contains more than one buffer ([%d]: %u)",
- count, elem[count].out_num);
- } else if (elem[count].out_sg[0].iov_len != (size_t)hdrlen) {
- debug("virtio-net transmit queue entry not aligned on hdrlen ([%d]: %d != %zu)",
- count, hdrlen, elem[count].out_sg[0].iov_len);
- } else {
- tap_add_packet(vdev->context,
- elem[count].out_sg[1].iov_len,
- (char *)elem[count].out_sg[1].iov_base,
- now);
- }
- }
+
+ data = IOV_TAIL(elem[count].out_sg, elem[count].out_num, 0);
+ if (IOV_DROP_HEADER(&data, struct virtio_net_hdr_mrg_rxbuf))
+ tap_add_packet(vdev->context, &data, now);
count++;
}
--
@@ -163,7 +163,6 @@ static void vu_handle_tx(struct vu_dev *vdev, int index,
struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZE];
struct iovec out_sg[VIRTQUEUE_MAX_SIZE];
struct vu_virtq *vq = &vdev->vq[index];
- int hdrlen = sizeof(struct virtio_net_hdr_mrg_rxbuf);
int out_sg_count;
int count;
@@ -176,6 +175,7 @@ static void vu_handle_tx(struct vu_dev *vdev, int index,
while (count < VIRTQUEUE_MAX_SIZE &&
out_sg_count + VU_MAX_TX_BUFFER_NB <= VIRTQUEUE_MAX_SIZE) {
int ret;
+ struct iov_tail data;
elem[count].out_num = VU_MAX_TX_BUFFER_NB;
elem[count].out_sg = &out_sg[out_sg_count];
@@ -191,26 +191,10 @@ static void vu_handle_tx(struct vu_dev *vdev, int index,
warn("virtio-net transmit queue contains no out buffers");
break;
}
- if (elem[count].out_num == 1) {
- tap_add_packet(vdev->context,
- elem[count].out_sg[0].iov_len - hdrlen,
- (char *)elem[count].out_sg[0].iov_base +
- hdrlen, now);
- } else {
- /* vnet header can be in a separate iovec */
- if (elem[count].out_num != 2) {
- debug("virtio-net transmit queue contains more than one buffer ([%d]: %u)",
- count, elem[count].out_num);
- } else if (elem[count].out_sg[0].iov_len != (size_t)hdrlen) {
- debug("virtio-net transmit queue entry not aligned on hdrlen ([%d]: %d != %zu)",
- count, hdrlen, elem[count].out_sg[0].iov_len);
- } else {
- tap_add_packet(vdev->context,
- elem[count].out_sg[1].iov_len,
- (char *)elem[count].out_sg[1].iov_base,
- now);
- }
- }
+
+ data = IOV_TAIL(elem[count].out_sg, elem[count].out_num, 0);
+ if (IOV_DROP_HEADER(&data, struct virtio_net_hdr_mrg_rxbuf))
+ tap_add_packet(vdev->context, &data, now);
count++;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 05/20] packet: Use iov_tail with packet_add()
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (3 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 04/20] tap: Use iov_tail with tap_add_packet() Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 06/20] packet: Add packet_base() Laurent Vivier
` (14 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier, David Gibson
Modify the interface of packet_add_do() to take an iov_tail
rather than a memory pointer aand length.
Internally it only supports iovec array with only one entries,
after being pruned. We can accept iovec array with several
entries if the offset allows the function to reduce the number
of entries to 1.
tap4_handler() is updated to create an iov_tail value using
IOV_TAIL_FROM_BUF() from the buffer and the length.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
iov.h | 1 +
packet.c | 15 ++++++++++++---
packet.h | 7 ++++---
tap.c | 32 ++++++++++++++++++--------------
4 files changed, 35 insertions(+), 20 deletions(-)
diff --git a/iov.h b/iov.h
index a63d8f21857a..b412a96b1090 100644
--- a/iov.h
+++ b/iov.h
@@ -17,6 +17,7 @@
#include <unistd.h>
#include <string.h>
+#include <stdbool.h>
#define IOV_OF_LVALUE(lval) \
(struct iovec){ .iov_base = &(lval), .iov_len = sizeof(lval) }
diff --git a/packet.c b/packet.c
index 72c61580be1e..98ded4e27aae 100644
--- a/packet.c
+++ b/packet.c
@@ -87,15 +87,16 @@ bool pool_full(const struct pool *p)
/**
* packet_add_do() - Add data as packet descriptor to given pool
* @p: Existing pool
- * @len: Length of new descriptor
- * @start: Start of data
+ * @data: Data to add
* @func: For tracing: name of calling function
* @line: For tracing: caller line of function call
*/
-void packet_add_do(struct pool *p, size_t len, const char *start,
+void packet_add_do(struct pool *p, struct iov_tail *data,
const char *func, int line)
{
size_t idx = p->count;
+ const char *start;
+ size_t len;
if (pool_full(p)) {
debug("add packet index %zu to pool with size %zu, %s:%i",
@@ -103,6 +104,14 @@ void packet_add_do(struct pool *p, size_t len, const char *start,
return;
}
+ if (!iov_tail_prune(data))
+ return;
+
+ ASSERT(data->cnt == 1); /* we don't support iovec */
+
+ len = data->iov[0].iov_len - data->off;
+ start = (char *)data->iov[0].iov_base + data->off;
+
if (packet_check_range(p, start, len, func, line))
return;
diff --git a/packet.h b/packet.h
index c94780a5ea54..af40b39b5251 100644
--- a/packet.h
+++ b/packet.h
@@ -7,6 +7,7 @@
#define PACKET_H
#include <stdbool.h>
+#include "iov.h"
/* Maximum size of a single packet stored in pool, including headers */
#define PACKET_MAX_LEN ((size_t)UINT16_MAX)
@@ -30,7 +31,7 @@ struct pool {
};
int vu_packet_check_range(void *buf, const char *ptr, size_t len);
-void packet_add_do(struct pool *p, size_t len, const char *start,
+void packet_add_do(struct pool *p, struct iov_tail *data,
const char *func, int line);
void *packet_get_try_do(const struct pool *p, const size_t idx,
size_t offset, size_t len, size_t *left,
@@ -41,8 +42,8 @@ void *packet_get_do(const struct pool *p, const size_t idx,
bool pool_full(const struct pool *p);
void pool_flush(struct pool *p);
-#define packet_add(p, len, start) \
- packet_add_do(p, len, start, __func__, __LINE__)
+#define packet_add(p, data) \
+ packet_add_do(p, data, __func__, __LINE__)
#define packet_get_try(p, idx, offset, len, left) \
packet_get_try_do(p, idx, offset, len, left, __func__, __LINE__)
diff --git a/tap.c b/tap.c
index e247a8f5ebc2..52a94f9f8be2 100644
--- a/tap.c
+++ b/tap.c
@@ -702,6 +702,7 @@ resume:
size_t l2len, l3len, hlen, l4len;
const struct ethhdr *eh;
const struct udphdr *uh;
+ struct iov_tail data;
struct iphdr *iph;
const char *l4h;
@@ -713,7 +714,8 @@ resume:
if (ntohs(eh->h_proto) == ETH_P_ARP) {
PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
- packet_add(pkt, l2len, (char *)eh);
+ data = IOV_TAIL_FROM_BUF((void *)eh, l2len, 0);
+ packet_add(pkt, &data);
arp(c, pkt);
continue;
}
@@ -758,7 +760,8 @@ resume:
tap_packet_debug(iph, NULL, NULL, 0, NULL, 1);
- packet_add(pkt, l4len, l4h);
+ data = IOV_TAIL_FROM_BUF((void *)l4h, l4len, 0);
+ packet_add(pkt, &data);
icmp_tap_handler(c, PIF_TAP, AF_INET,
&iph->saddr, &iph->daddr,
pkt, now);
@@ -772,7 +775,8 @@ resume:
if (iph->protocol == IPPROTO_UDP) {
PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
- packet_add(pkt, l2len, (char *)eh);
+ data = IOV_TAIL_FROM_BUF((void *)eh, l2len, 0);
+ packet_add(pkt, &data);
if (dhcp(c, pkt))
continue;
}
@@ -821,7 +825,8 @@ resume:
#undef L4_SET
append:
- packet_add((struct pool *)&seq->p, l4len, l4h);
+ data = IOV_TAIL_FROM_BUF((void *)l4h, l4len, 0);
+ packet_add((struct pool *)&seq->p, &data);
}
for (j = 0, seq = tap4_l4; j < seq_count; j++, seq++) {
@@ -877,6 +882,7 @@ resume:
struct in6_addr *saddr, *daddr;
const struct ethhdr *eh;
const struct udphdr *uh;
+ struct iov_tail data;
struct ipv6hdr *ip6h;
uint8_t proto;
char *l4h;
@@ -930,7 +936,8 @@ resume:
if (l4len < sizeof(struct icmp6hdr))
continue;
- packet_add(pkt, l4len, l4h);
+ data = IOV_TAIL_FROM_BUF(l4h, l4len, 0);
+ packet_add(pkt, &data);
if (ndp(c, (struct icmp6hdr *)l4h, saddr, pkt))
continue;
@@ -949,7 +956,8 @@ resume:
if (proto == IPPROTO_UDP) {
PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
- packet_add(pkt, l4len, l4h);
+ data = IOV_TAIL_FROM_BUF(l4h, l4len, 0);
+ packet_add(pkt, &data);
if (dhcpv6(c, pkt, saddr, daddr))
continue;
@@ -1003,7 +1011,8 @@ resume:
#undef L4_SET
append:
- packet_add((struct pool *)&seq->p, l4len, l4h);
+ data = IOV_TAIL_FROM_BUF(l4h, l4len, 0);
+ packet_add((struct pool *)&seq->p, &data);
}
for (j = 0, seq = tap6_l4; j < seq_count; j++, seq++) {
@@ -1079,9 +1088,6 @@ void tap_add_packet(struct ctx *c, struct iov_tail *data,
proto_update_l2_buf(c->guest_mac, NULL);
}
- iov_tail_prune(data);
- ASSERT(data->cnt == 1); /* packet_add() doesn't support iovec */
-
switch (ntohs(eh->h_proto)) {
case ETH_P_ARP:
case ETH_P_IP:
@@ -1089,16 +1095,14 @@ void tap_add_packet(struct ctx *c, struct iov_tail *data,
tap4_handler(c, pool_tap4, now);
pool_flush(pool_tap4);
}
- packet_add(pool_tap4, data->iov[0].iov_len - data->off,
- (char *)data->iov[0].iov_base + data->off);
+ packet_add(pool_tap4, data);
break;
case ETH_P_IPV6:
if (pool_full(pool_tap6)) {
tap6_handler(c, pool_tap6, now);
pool_flush(pool_tap6);
}
- packet_add(pool_tap6, data->iov[0].iov_len - data->off,
- (char *)data->iov[0].iov_base + data->off);
+ packet_add(pool_tap6, data);
break;
default:
break;
--
@@ -702,6 +702,7 @@ resume:
size_t l2len, l3len, hlen, l4len;
const struct ethhdr *eh;
const struct udphdr *uh;
+ struct iov_tail data;
struct iphdr *iph;
const char *l4h;
@@ -713,7 +714,8 @@ resume:
if (ntohs(eh->h_proto) == ETH_P_ARP) {
PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
- packet_add(pkt, l2len, (char *)eh);
+ data = IOV_TAIL_FROM_BUF((void *)eh, l2len, 0);
+ packet_add(pkt, &data);
arp(c, pkt);
continue;
}
@@ -758,7 +760,8 @@ resume:
tap_packet_debug(iph, NULL, NULL, 0, NULL, 1);
- packet_add(pkt, l4len, l4h);
+ data = IOV_TAIL_FROM_BUF((void *)l4h, l4len, 0);
+ packet_add(pkt, &data);
icmp_tap_handler(c, PIF_TAP, AF_INET,
&iph->saddr, &iph->daddr,
pkt, now);
@@ -772,7 +775,8 @@ resume:
if (iph->protocol == IPPROTO_UDP) {
PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
- packet_add(pkt, l2len, (char *)eh);
+ data = IOV_TAIL_FROM_BUF((void *)eh, l2len, 0);
+ packet_add(pkt, &data);
if (dhcp(c, pkt))
continue;
}
@@ -821,7 +825,8 @@ resume:
#undef L4_SET
append:
- packet_add((struct pool *)&seq->p, l4len, l4h);
+ data = IOV_TAIL_FROM_BUF((void *)l4h, l4len, 0);
+ packet_add((struct pool *)&seq->p, &data);
}
for (j = 0, seq = tap4_l4; j < seq_count; j++, seq++) {
@@ -877,6 +882,7 @@ resume:
struct in6_addr *saddr, *daddr;
const struct ethhdr *eh;
const struct udphdr *uh;
+ struct iov_tail data;
struct ipv6hdr *ip6h;
uint8_t proto;
char *l4h;
@@ -930,7 +936,8 @@ resume:
if (l4len < sizeof(struct icmp6hdr))
continue;
- packet_add(pkt, l4len, l4h);
+ data = IOV_TAIL_FROM_BUF(l4h, l4len, 0);
+ packet_add(pkt, &data);
if (ndp(c, (struct icmp6hdr *)l4h, saddr, pkt))
continue;
@@ -949,7 +956,8 @@ resume:
if (proto == IPPROTO_UDP) {
PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
- packet_add(pkt, l4len, l4h);
+ data = IOV_TAIL_FROM_BUF(l4h, l4len, 0);
+ packet_add(pkt, &data);
if (dhcpv6(c, pkt, saddr, daddr))
continue;
@@ -1003,7 +1011,8 @@ resume:
#undef L4_SET
append:
- packet_add((struct pool *)&seq->p, l4len, l4h);
+ data = IOV_TAIL_FROM_BUF(l4h, l4len, 0);
+ packet_add((struct pool *)&seq->p, &data);
}
for (j = 0, seq = tap6_l4; j < seq_count; j++, seq++) {
@@ -1079,9 +1088,6 @@ void tap_add_packet(struct ctx *c, struct iov_tail *data,
proto_update_l2_buf(c->guest_mac, NULL);
}
- iov_tail_prune(data);
- ASSERT(data->cnt == 1); /* packet_add() doesn't support iovec */
-
switch (ntohs(eh->h_proto)) {
case ETH_P_ARP:
case ETH_P_IP:
@@ -1089,16 +1095,14 @@ void tap_add_packet(struct ctx *c, struct iov_tail *data,
tap4_handler(c, pool_tap4, now);
pool_flush(pool_tap4);
}
- packet_add(pool_tap4, data->iov[0].iov_len - data->off,
- (char *)data->iov[0].iov_base + data->off);
+ packet_add(pool_tap4, data);
break;
case ETH_P_IPV6:
if (pool_full(pool_tap6)) {
tap6_handler(c, pool_tap6, now);
pool_flush(pool_tap6);
}
- packet_add(pool_tap6, data->iov[0].iov_len - data->off,
- (char *)data->iov[0].iov_base + data->off);
+ packet_add(pool_tap6, data);
break;
default:
break;
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 06/20] packet: Add packet_base()
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (4 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 05/20] packet: Use iov_tail with packet_add() Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-14 2:29 ` David Gibson
2025-04-11 13:10 ` [PATCH v2 07/20] arp: Convert to iov_tail Laurent Vivier
` (13 subsequent siblings)
19 siblings, 1 reply; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
packet_base() gets the data range from a packet descriptor from a
given pool.
It uses iov_tail to return the packet memory.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
packet.c | 41 +++++++++++++++++++++++++++++++++++++++++
packet.h | 5 +++++
2 files changed, 46 insertions(+)
diff --git a/packet.c b/packet.c
index 98ded4e27aae..c3f329d53507 100644
--- a/packet.c
+++ b/packet.c
@@ -190,6 +190,47 @@ void *packet_get_do(const struct pool *p, const size_t idx,
return r;
}
+/**
+ * packet_base_do() - Get data range from packet descriptor from given pool
+ * @p: Packet pool
+ * @idx: Index of packet descriptor in pool
+ * @data: IOV tail to store the address of the data (output)
+ * @func: For tracing: name of calling function, NULL means no trace()
+ * @line: For tracing: caller line of function call
+ *
+ * Return: true if @data contains valid data, false otherwise
+ */
+/* cppcheck-suppress unusedFunction */
+bool packet_base_do(const struct pool *p, size_t idx,
+ struct iov_tail *data,
+ const char *func, int line)
+{
+ size_t i;
+
+ ASSERT_WITH_MSG(p->count <= p->size,
+ "Corrupt pool count: %zu, size: %zu, %s:%i",
+ p->count, p->size, func, line);
+
+ if (idx >= p->count) {
+ debug("packet %zu from pool size: %zu, count: %zu, "
+ "%s:%i", idx, p->size, p->count, func, line);
+ return false;
+ }
+
+ data->cnt = 1;
+ data->off = 0;
+ data->iov = &p->pkt[idx];
+
+ for (i = 0; i < data->cnt; i++) {
+ ASSERT_WITH_MSG(!packet_check_range(p, data->iov[i].iov_base,
+ data->iov[i].iov_len,
+ func, line),
+ "Corrupt packet pool, %s:%i", func, line);
+ }
+
+ return true;
+}
+
/**
* pool_flush() - Flush a packet pool
* @p: Pointer to packet pool
diff --git a/packet.h b/packet.h
index af40b39b5251..ee7a5304a034 100644
--- a/packet.h
+++ b/packet.h
@@ -39,6 +39,9 @@ void *packet_get_try_do(const struct pool *p, const size_t idx,
void *packet_get_do(const struct pool *p, const size_t idx,
size_t offset, size_t len, size_t *left,
const char *func, int line);
+bool packet_base_do(const struct pool *p, const size_t idx,
+ struct iov_tail *data,
+ const char *func, int line);
bool pool_full(const struct pool *p);
void pool_flush(struct pool *p);
@@ -49,6 +52,8 @@ void pool_flush(struct pool *p);
packet_get_try_do(p, idx, offset, len, left, __func__, __LINE__)
#define packet_get(p, idx, offset, len, left) \
packet_get_do(p, idx, offset, len, left, __func__, __LINE__)
+#define packet_base(p, idx, data) \
+ packet_base_do(p, idx, data, __func__, __LINE__)
#define PACKET_POOL_DECL(_name, _size, _buf) \
struct _name ## _t { \
--
@@ -39,6 +39,9 @@ void *packet_get_try_do(const struct pool *p, const size_t idx,
void *packet_get_do(const struct pool *p, const size_t idx,
size_t offset, size_t len, size_t *left,
const char *func, int line);
+bool packet_base_do(const struct pool *p, const size_t idx,
+ struct iov_tail *data,
+ const char *func, int line);
bool pool_full(const struct pool *p);
void pool_flush(struct pool *p);
@@ -49,6 +52,8 @@ void pool_flush(struct pool *p);
packet_get_try_do(p, idx, offset, len, left, __func__, __LINE__)
#define packet_get(p, idx, offset, len, left) \
packet_get_do(p, idx, offset, len, left, __func__, __LINE__)
+#define packet_base(p, idx, data) \
+ packet_base_do(p, idx, data, __func__, __LINE__)
#define PACKET_POOL_DECL(_name, _size, _buf) \
struct _name ## _t { \
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 06/20] packet: Add packet_base()
2025-04-11 13:10 ` [PATCH v2 06/20] packet: Add packet_base() Laurent Vivier
@ 2025-04-14 2:29 ` David Gibson
0 siblings, 0 replies; 29+ messages in thread
From: David Gibson @ 2025-04-14 2:29 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 3488 bytes --]
On Fri, Apr 11, 2025 at 03:10:16PM +0200, Laurent Vivier wrote:
> packet_base() gets the data range from a packet descriptor from a
> given pool.
>
> It uses iov_tail to return the packet memory.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Logic looks fine. I kind of hate the name though; "base" of what?
Maybe packet_data()?
> ---
> packet.c | 41 +++++++++++++++++++++++++++++++++++++++++
> packet.h | 5 +++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/packet.c b/packet.c
> index 98ded4e27aae..c3f329d53507 100644
> --- a/packet.c
> +++ b/packet.c
> @@ -190,6 +190,47 @@ void *packet_get_do(const struct pool *p, const size_t idx,
> return r;
> }
>
> +/**
> + * packet_base_do() - Get data range from packet descriptor from given pool
> + * @p: Packet pool
> + * @idx: Index of packet descriptor in pool
> + * @data: IOV tail to store the address of the data (output)
> + * @func: For tracing: name of calling function, NULL means no trace()
> + * @line: For tracing: caller line of function call
> + *
> + * Return: true if @data contains valid data, false otherwise
> + */
> +/* cppcheck-suppress unusedFunction */
> +bool packet_base_do(const struct pool *p, size_t idx,
> + struct iov_tail *data,
> + const char *func, int line)
> +{
> + size_t i;
> +
> + ASSERT_WITH_MSG(p->count <= p->size,
> + "Corrupt pool count: %zu, size: %zu, %s:%i",
> + p->count, p->size, func, line);
> +
> + if (idx >= p->count) {
> + debug("packet %zu from pool size: %zu, count: %zu, "
> + "%s:%i", idx, p->size, p->count, func, line);
> + return false;
> + }
I'm kind of tempted to suggest making this also an ASSERT(), in which
case this function could return an iov_tail by value, which is
probably a more natural interface. Stefano, any opinion?
> +
> + data->cnt = 1;
> + data->off = 0;
> + data->iov = &p->pkt[idx];
> +
> + for (i = 0; i < data->cnt; i++) {
> + ASSERT_WITH_MSG(!packet_check_range(p, data->iov[i].iov_base,
> + data->iov[i].iov_len,
> + func, line),
> + "Corrupt packet pool, %s:%i", func, line);
> + }
> +
> + return true;
> +}
> +
> /**
> * pool_flush() - Flush a packet pool
> * @p: Pointer to packet pool
> diff --git a/packet.h b/packet.h
> index af40b39b5251..ee7a5304a034 100644
> --- a/packet.h
> +++ b/packet.h
> @@ -39,6 +39,9 @@ void *packet_get_try_do(const struct pool *p, const size_t idx,
> void *packet_get_do(const struct pool *p, const size_t idx,
> size_t offset, size_t len, size_t *left,
> const char *func, int line);
> +bool packet_base_do(const struct pool *p, const size_t idx,
> + struct iov_tail *data,
> + const char *func, int line);
> bool pool_full(const struct pool *p);
> void pool_flush(struct pool *p);
>
> @@ -49,6 +52,8 @@ void pool_flush(struct pool *p);
> packet_get_try_do(p, idx, offset, len, left, __func__, __LINE__)
> #define packet_get(p, idx, offset, len, left) \
> packet_get_do(p, idx, offset, len, left, __func__, __LINE__)
> +#define packet_base(p, idx, data) \
> + packet_base_do(p, idx, data, __func__, __LINE__)
>
> #define PACKET_POOL_DECL(_name, _size, _buf) \
> struct _name ## _t { \
--
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 --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 07/20] arp: Convert to iov_tail
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (5 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 06/20] packet: Add packet_base() Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 08/20] ndp: " Laurent Vivier
` (12 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier, David Gibson
Use packet_base() and extract headers using IOV_REMOVE_HEADER()
rather than packet_get().
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
arp.c | 12 +++++++++---
packet.c | 1 -
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arp.c b/arp.c
index 9d68d7c3b602..cd0e15de7de0 100644
--- a/arp.c
+++ b/arp.c
@@ -77,11 +77,17 @@ int arp(const struct ctx *c, const struct pool *p)
const struct ethhdr *eh;
const struct arphdr *ah;
const struct arpmsg *am;
+ struct iov_tail data;
+ struct arphdr ahc;
+ struct ethhdr ehc;
+ struct arpmsg amc;
- eh = packet_get(p, 0, 0, sizeof(*eh), NULL);
- ah = packet_get(p, 0, sizeof(*eh), sizeof(*ah), NULL);
- am = packet_get(p, 0, sizeof(*eh) + sizeof(*ah), sizeof(*am), NULL);
+ if (!packet_base(p, 0, &data))
+ return -1;
+ eh = IOV_REMOVE_HEADER(&data, ehc);
+ ah = IOV_REMOVE_HEADER(&data, ahc);
+ am = IOV_REMOVE_HEADER(&data, amc);
if (!eh || !ah || !am)
return -1;
diff --git a/packet.c b/packet.c
index c3f329d53507..c238696c58bb 100644
--- a/packet.c
+++ b/packet.c
@@ -200,7 +200,6 @@ void *packet_get_do(const struct pool *p, const size_t idx,
*
* Return: true if @data contains valid data, false otherwise
*/
-/* cppcheck-suppress unusedFunction */
bool packet_base_do(const struct pool *p, size_t idx,
struct iov_tail *data,
const char *func, int line)
--
@@ -200,7 +200,6 @@ void *packet_get_do(const struct pool *p, const size_t idx,
*
* Return: true if @data contains valid data, false otherwise
*/
-/* cppcheck-suppress unusedFunction */
bool packet_base_do(const struct pool *p, size_t idx,
struct iov_tail *data,
const char *func, int line)
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 08/20] ndp: Convert to iov_tail
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (6 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 07/20] arp: Convert to iov_tail Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 09/20] icmp: " Laurent Vivier
` (11 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier, David Gibson
Use packet_base() and extract headers using IOV_REMOVE_HEADER()
rather than packet_get().
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
ndp.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/ndp.c b/ndp.c
index ded2081ddd1d..64e25d5455b4 100644
--- a/ndp.c
+++ b/ndp.c
@@ -351,8 +351,13 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
if (ih->icmp6_type == NS) {
const struct ndp_ns *ns;
+ struct iov_tail data;
+ struct ndp_ns nsc;
- ns = packet_get(p, 0, 0, sizeof(struct ndp_ns), NULL);
+ if (!packet_base(p, 0, &data))
+ return -1;
+
+ ns = IOV_REMOVE_HEADER(&data, nsc);
if (!ns)
return -1;
--
@@ -351,8 +351,13 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
if (ih->icmp6_type == NS) {
const struct ndp_ns *ns;
+ struct iov_tail data;
+ struct ndp_ns nsc;
- ns = packet_get(p, 0, 0, sizeof(struct ndp_ns), NULL);
+ if (!packet_base(p, 0, &data))
+ return -1;
+
+ ns = IOV_REMOVE_HEADER(&data, nsc);
if (!ns)
return -1;
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 09/20] icmp: Convert to iov_tail
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (7 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 08/20] ndp: " Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-14 3:14 ` David Gibson
2025-04-11 13:10 ` [PATCH v2 10/20] udp: " Laurent Vivier
` (10 subsequent siblings)
19 siblings, 1 reply; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
Use packet_base() and extract headers using IOV_PEEK_HEADER()
rather than packet_get().
Introduce iov_tail_msghdr() to convert iov_tail to msghdr
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
icmp.c | 32 +++++++++++++++++---------------
iov.c | 21 +++++++++++++++++++++
iov.h | 2 ++
3 files changed, 40 insertions(+), 15 deletions(-)
diff --git a/icmp.c b/icmp.c
index 7e2b3423a8d1..64ad738b6809 100644
--- a/icmp.c
+++ b/icmp.c
@@ -241,24 +241,25 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
struct icmp_ping_flow *pingf;
const struct flowside *tgt;
union sockaddr_inany sa;
- size_t dlen, l4len;
+ struct iov_tail data;
+ struct msghdr msh;
uint16_t id, seq;
union flow *flow;
uint8_t proto;
- socklen_t sl;
- void *pkt;
(void)saddr;
ASSERT(pif == PIF_TAP);
+ if (!packet_base(p, 0, &data))
+ return -1;
+
if (af == AF_INET) {
const struct icmphdr *ih;
+ struct icmphdr ihc;
- if (!(pkt = packet_get(p, 0, 0, sizeof(*ih), &dlen)))
- return 1;
-
- ih = (struct icmphdr *)pkt;
- l4len = dlen + sizeof(*ih);
+ ih = IOV_PEEK_HEADER(&data, ihc);
+ if (!ih)
+ return -1;
if (ih->type != ICMP_ECHO)
return 1;
@@ -268,12 +269,11 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
seq = ntohs(ih->un.echo.sequence);
} else if (af == AF_INET6) {
const struct icmp6hdr *ih;
+ struct icmp6hdr ihc;
- if (!(pkt = packet_get(p, 0, 0, sizeof(*ih), &dlen)))
- return 1;
-
- ih = (struct icmp6hdr *)pkt;
- l4len = dlen + sizeof(*ih);
+ ih = IOV_PEEK_HEADER(&data, ihc);
+ if (!ih)
+ return -1;
if (ih->icmp6_type != ICMPV6_ECHO_REQUEST)
return 1;
@@ -298,8 +298,10 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
ASSERT(flow_proto[pingf->f.type] == proto);
pingf->ts = now->tv_sec;
- pif_sockaddr(c, &sa, &sl, PIF_HOST, &tgt->eaddr, 0);
- if (sendto(pingf->sock, pkt, l4len, MSG_NOSIGNAL, &sa.sa, sl) < 0) {
+ iov_tail_msghdr(&msh, &data, &sa);
+
+ pif_sockaddr(c, &sa, &msh.msg_namelen, PIF_HOST, &tgt->eaddr, 0);
+ if (sendmsg(pingf->sock, &msh, MSG_NOSIGNAL) < 0) {
flow_dbg_perror(pingf, "failed to relay request to socket");
} else {
flow_dbg(pingf,
diff --git a/iov.c b/iov.c
index 98d5fecbdc8f..9e3786e6efe8 100644
--- a/iov.c
+++ b/iov.c
@@ -210,6 +210,27 @@ int iov_slice(struct iovec *dst_iov, size_t dst_iov_cnt,
return j;
}
+/**
+ * iov_tail_msghdr - Initialize a msghdr from an IOV tail structure
+ * @msh: msghdr to initialize
+ * @tail: iov_tail to use to set msg_iov and msg_iovlen
+ * @msg_name: Pointer to set to msg_name
+ */
+void iov_tail_msghdr(struct msghdr *msh, struct iov_tail *tail,
+ void *msg_name)
+{
+ iov_tail_prune(tail);
+
+ ASSERT(tail->off == 0);
+
+ msh->msg_name = msg_name;
+ msh->msg_iov = (struct iovec *)tail->iov;
+ msh->msg_iovlen = tail->cnt;
+ msh->msg_control = NULL;
+ msh->msg_controllen = 0;
+ msh->msg_flags = 0;
+}
+
/**
* iov_tail_prune() - Remove any unneeded buffers from an IOV tail
* @tail: IO vector tail (modified)
diff --git a/iov.h b/iov.h
index b412a96b1090..acba2ea4240b 100644
--- a/iov.h
+++ b/iov.h
@@ -83,6 +83,8 @@ struct iov_tail {
#define IOV_TAIL_FROM_BUF(buf_, len_, off_) \
IOV_TAIL((&(const struct iovec){ .iov_base = (buf_), .iov_len = (len_) }), 1, (off_))
+void iov_tail_msghdr(struct msghdr *msh, struct iov_tail *tail,
+ void *msg_name);
bool iov_tail_prune(struct iov_tail *tail);
size_t iov_tail_size(struct iov_tail *tail);
bool iov_tail_drop(struct iov_tail *tail, size_t len);
--
@@ -83,6 +83,8 @@ struct iov_tail {
#define IOV_TAIL_FROM_BUF(buf_, len_, off_) \
IOV_TAIL((&(const struct iovec){ .iov_base = (buf_), .iov_len = (len_) }), 1, (off_))
+void iov_tail_msghdr(struct msghdr *msh, struct iov_tail *tail,
+ void *msg_name);
bool iov_tail_prune(struct iov_tail *tail);
size_t iov_tail_size(struct iov_tail *tail);
bool iov_tail_drop(struct iov_tail *tail, size_t len);
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 09/20] icmp: Convert to iov_tail
2025-04-11 13:10 ` [PATCH v2 09/20] icmp: " Laurent Vivier
@ 2025-04-14 3:14 ` David Gibson
0 siblings, 0 replies; 29+ messages in thread
From: David Gibson @ 2025-04-14 3:14 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 4656 bytes --]
On Fri, Apr 11, 2025 at 03:10:19PM +0200, Laurent Vivier wrote:
> Use packet_base() and extract headers using IOV_PEEK_HEADER()
> rather than packet_get().
>
> Introduce iov_tail_msghdr() to convert iov_tail to msghdr
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> icmp.c | 32 +++++++++++++++++---------------
> iov.c | 21 +++++++++++++++++++++
> iov.h | 2 ++
> 3 files changed, 40 insertions(+), 15 deletions(-)
>
> diff --git a/icmp.c b/icmp.c
> index 7e2b3423a8d1..64ad738b6809 100644
> --- a/icmp.c
> +++ b/icmp.c
> @@ -241,24 +241,25 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
> struct icmp_ping_flow *pingf;
> const struct flowside *tgt;
> union sockaddr_inany sa;
> - size_t dlen, l4len;
> + struct iov_tail data;
> + struct msghdr msh;
> uint16_t id, seq;
> union flow *flow;
> uint8_t proto;
> - socklen_t sl;
> - void *pkt;
>
> (void)saddr;
> ASSERT(pif == PIF_TAP);
>
> + if (!packet_base(p, 0, &data))
> + return -1;
> +
> if (af == AF_INET) {
> const struct icmphdr *ih;
> + struct icmphdr ihc;
>
> - if (!(pkt = packet_get(p, 0, 0, sizeof(*ih), &dlen)))
> - return 1;
> -
> - ih = (struct icmphdr *)pkt;
> - l4len = dlen + sizeof(*ih);
> + ih = IOV_PEEK_HEADER(&data, ihc);
> + if (!ih)
> + return -1;
You've replaced a 'return 1' with a 'return -1' which doesn't seem correct.
>
> if (ih->type != ICMP_ECHO)
> return 1;
> @@ -268,12 +269,11 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
> seq = ntohs(ih->un.echo.sequence);
> } else if (af == AF_INET6) {
> const struct icmp6hdr *ih;
> + struct icmp6hdr ihc;
>
> - if (!(pkt = packet_get(p, 0, 0, sizeof(*ih), &dlen)))
> - return 1;
> -
> - ih = (struct icmp6hdr *)pkt;
> - l4len = dlen + sizeof(*ih);
> + ih = IOV_PEEK_HEADER(&data, ihc);
> + if (!ih)
> + return -1;
Same here.
>
> if (ih->icmp6_type != ICMPV6_ECHO_REQUEST)
> return 1;
> @@ -298,8 +298,10 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
> ASSERT(flow_proto[pingf->f.type] == proto);
> pingf->ts = now->tv_sec;
>
> - pif_sockaddr(c, &sa, &sl, PIF_HOST, &tgt->eaddr, 0);
> - if (sendto(pingf->sock, pkt, l4len, MSG_NOSIGNAL, &sa.sa, sl) < 0) {
> + iov_tail_msghdr(&msh, &data, &sa);
> +
> + pif_sockaddr(c, &sa, &msh.msg_namelen, PIF_HOST, &tgt->eaddr, 0);
> + if (sendmsg(pingf->sock, &msh, MSG_NOSIGNAL) < 0) {
> flow_dbg_perror(pingf, "failed to relay request to socket");
> } else {
> flow_dbg(pingf,
> diff --git a/iov.c b/iov.c
> index 98d5fecbdc8f..9e3786e6efe8 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -210,6 +210,27 @@ int iov_slice(struct iovec *dst_iov, size_t dst_iov_cnt,
> return j;
> }
>
> +/**
> + * iov_tail_msghdr - Initialize a msghdr from an IOV tail structure
> + * @msh: msghdr to initialize
> + * @tail: iov_tail to use to set msg_iov and msg_iovlen
> + * @msg_name: Pointer to set to msg_name
> + */
> +void iov_tail_msghdr(struct msghdr *msh, struct iov_tail *tail,
> + void *msg_name)
> +{
> + iov_tail_prune(tail);
> +
> + ASSERT(tail->off == 0);
> +
> + msh->msg_name = msg_name;
This doesn't initialise msg_namelen. I see that you do it outside
from pif_sockaddr(), but that makes it extremely easy to misuse this
function.
Not immediately sure how to make an interface that's both safe and
convenient here. If it helps, it's probably reasonable to assume here
that you'll always use a union sockaddr_inany for the address.
> + msh->msg_iov = (struct iovec *)tail->iov;
> + msh->msg_iovlen = tail->cnt;
> + msh->msg_control = NULL;
> + msh->msg_controllen = 0;
> + msh->msg_flags = 0;
> +}
> +
> /**
> * iov_tail_prune() - Remove any unneeded buffers from an IOV tail
> * @tail: IO vector tail (modified)
> diff --git a/iov.h b/iov.h
> index b412a96b1090..acba2ea4240b 100644
> --- a/iov.h
> +++ b/iov.h
> @@ -83,6 +83,8 @@ struct iov_tail {
> #define IOV_TAIL_FROM_BUF(buf_, len_, off_) \
> IOV_TAIL((&(const struct iovec){ .iov_base = (buf_), .iov_len = (len_) }), 1, (off_))
>
> +void iov_tail_msghdr(struct msghdr *msh, struct iov_tail *tail,
> + void *msg_name);
> bool iov_tail_prune(struct iov_tail *tail);
> size_t iov_tail_size(struct iov_tail *tail);
> bool iov_tail_drop(struct iov_tail *tail, size_t len);
--
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 --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 10/20] udp: Convert to iov_tail
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (8 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 09/20] icmp: " Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-14 3:21 ` David Gibson
2025-04-11 13:10 ` [PATCH v2 11/20] tcp: Convert tcp_tap_handler() to use iov_tail Laurent Vivier
` (9 subsequent siblings)
19 siblings, 1 reply; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
Use packet_base() and extract headers using IOV_REMOVE_HEADER()
and IOV_PEEK_HEADER() rather than packet_get().
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
iov.c | 1 -
udp.c | 34 +++++++++++++++++++++++-----------
2 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/iov.c b/iov.c
index 9e3786e6efe8..d09e51190d54 100644
--- a/iov.c
+++ b/iov.c
@@ -175,7 +175,6 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt)
* iov array, a negative value if there is no enough room in the
* destination iov array
*/
-/* cppcheck-suppress unusedFunction */
int iov_slice(struct iovec *dst_iov, size_t dst_iov_cnt,
const struct iovec *iov, size_t iov_cnt,
size_t offset, size_t *bytes)
diff --git a/udp.c b/udp.c
index ab3e9d20fd74..75526f743994 100644
--- a/udp.c
+++ b/udp.c
@@ -862,15 +862,20 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
struct iovec m[UIO_MAXIOV];
const struct udphdr *uh;
struct udp_flow *uflow;
- int i, s, count = 0;
+ int i, j, s, count = 0;
+ struct iov_tail data;
flow_sidx_t tosidx;
in_port_t src, dst;
+ struct udphdr uhc;
uint8_t topif;
socklen_t sl;
ASSERT(!c->no_udp);
- uh = packet_get(p, idx, 0, sizeof(*uh), NULL);
+ if (!packet_base(p, idx, &data))
+ return 1;
+
+ uh = IOV_PEEK_HEADER(&data, uhc);
if (!uh)
return 1;
@@ -907,23 +912,30 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
pif_sockaddr(c, &to_sa, &sl, topif, &toside->eaddr, toside->eport);
- for (i = 0; i < (int)p->count - idx; i++) {
- struct udphdr *uh_send;
- size_t len;
+ for (i = 0, j = 0; i < (int)p->count - idx && j < UIO_MAXIOV; i++) {
+ const struct udphdr *uh_send;
- uh_send = packet_get(p, idx + i, 0, sizeof(*uh), &len);
+ if (!packet_base(p, idx + i, &data))
+ return p->count - idx;
+
+ uh_send = IOV_REMOVE_HEADER(&data, uhc);
if (!uh_send)
return p->count - idx;
mm[i].msg_hdr.msg_name = &to_sa;
mm[i].msg_hdr.msg_namelen = sl;
- if (len) {
- m[i].iov_base = (char *)(uh_send + 1);
- m[i].iov_len = len;
+ if (data.cnt) {
+ int len;
+
+ len = iov_slice(&m[j], UIO_MAXIOV - j,
+ &data.iov[0], data.cnt, data.off, NULL);
+ if (len < 0)
+ return p->count - idx;
- mm[i].msg_hdr.msg_iov = m + i;
- mm[i].msg_hdr.msg_iovlen = 1;
+ mm[i].msg_hdr.msg_iov = &m[j];
+ mm[i].msg_hdr.msg_iovlen = len;
+ j += len;
} else {
mm[i].msg_hdr.msg_iov = NULL;
mm[i].msg_hdr.msg_iovlen = 0;
--
@@ -862,15 +862,20 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
struct iovec m[UIO_MAXIOV];
const struct udphdr *uh;
struct udp_flow *uflow;
- int i, s, count = 0;
+ int i, j, s, count = 0;
+ struct iov_tail data;
flow_sidx_t tosidx;
in_port_t src, dst;
+ struct udphdr uhc;
uint8_t topif;
socklen_t sl;
ASSERT(!c->no_udp);
- uh = packet_get(p, idx, 0, sizeof(*uh), NULL);
+ if (!packet_base(p, idx, &data))
+ return 1;
+
+ uh = IOV_PEEK_HEADER(&data, uhc);
if (!uh)
return 1;
@@ -907,23 +912,30 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
pif_sockaddr(c, &to_sa, &sl, topif, &toside->eaddr, toside->eport);
- for (i = 0; i < (int)p->count - idx; i++) {
- struct udphdr *uh_send;
- size_t len;
+ for (i = 0, j = 0; i < (int)p->count - idx && j < UIO_MAXIOV; i++) {
+ const struct udphdr *uh_send;
- uh_send = packet_get(p, idx + i, 0, sizeof(*uh), &len);
+ if (!packet_base(p, idx + i, &data))
+ return p->count - idx;
+
+ uh_send = IOV_REMOVE_HEADER(&data, uhc);
if (!uh_send)
return p->count - idx;
mm[i].msg_hdr.msg_name = &to_sa;
mm[i].msg_hdr.msg_namelen = sl;
- if (len) {
- m[i].iov_base = (char *)(uh_send + 1);
- m[i].iov_len = len;
+ if (data.cnt) {
+ int len;
+
+ len = iov_slice(&m[j], UIO_MAXIOV - j,
+ &data.iov[0], data.cnt, data.off, NULL);
+ if (len < 0)
+ return p->count - idx;
- mm[i].msg_hdr.msg_iov = m + i;
- mm[i].msg_hdr.msg_iovlen = 1;
+ mm[i].msg_hdr.msg_iov = &m[j];
+ mm[i].msg_hdr.msg_iovlen = len;
+ j += len;
} else {
mm[i].msg_hdr.msg_iov = NULL;
mm[i].msg_hdr.msg_iovlen = 0;
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 10/20] udp: Convert to iov_tail
2025-04-11 13:10 ` [PATCH v2 10/20] udp: " Laurent Vivier
@ 2025-04-14 3:21 ` David Gibson
0 siblings, 0 replies; 29+ messages in thread
From: David Gibson @ 2025-04-14 3:21 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 3228 bytes --]
On Fri, Apr 11, 2025 at 03:10:20PM +0200, Laurent Vivier wrote:
> Use packet_base() and extract headers using IOV_REMOVE_HEADER()
> and IOV_PEEK_HEADER() rather than packet_get().
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> iov.c | 1 -
> udp.c | 34 +++++++++++++++++++++++-----------
> 2 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/iov.c b/iov.c
> index 9e3786e6efe8..d09e51190d54 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -175,7 +175,6 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt)
> * iov array, a negative value if there is no enough room in the
> * destination iov array
> */
> -/* cppcheck-suppress unusedFunction */
> int iov_slice(struct iovec *dst_iov, size_t dst_iov_cnt,
> const struct iovec *iov, size_t iov_cnt,
> size_t offset, size_t *bytes)
> diff --git a/udp.c b/udp.c
> index ab3e9d20fd74..75526f743994 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -862,15 +862,20 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
> struct iovec m[UIO_MAXIOV];
> const struct udphdr *uh;
> struct udp_flow *uflow;
> - int i, s, count = 0;
> + int i, j, s, count = 0;
> + struct iov_tail data;
> flow_sidx_t tosidx;
> in_port_t src, dst;
> + struct udphdr uhc;
> uint8_t topif;
> socklen_t sl;
>
> ASSERT(!c->no_udp);
>
> - uh = packet_get(p, idx, 0, sizeof(*uh), NULL);
> + if (!packet_base(p, idx, &data))
> + return 1;
> +
> + uh = IOV_PEEK_HEADER(&data, uhc);
> if (!uh)
> return 1;
>
> @@ -907,23 +912,30 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
>
> pif_sockaddr(c, &to_sa, &sl, topif, &toside->eaddr, toside->eport);
>
> - for (i = 0; i < (int)p->count - idx; i++) {
> - struct udphdr *uh_send;
> - size_t len;
> + for (i = 0, j = 0; i < (int)p->count - idx && j < UIO_MAXIOV; i++) {
> + const struct udphdr *uh_send;
>
> - uh_send = packet_get(p, idx + i, 0, sizeof(*uh), &len);
> + if (!packet_base(p, idx + i, &data))
> + return p->count - idx;
> +
> + uh_send = IOV_REMOVE_HEADER(&data, uhc);
> if (!uh_send)
> return p->count - idx;
>
> mm[i].msg_hdr.msg_name = &to_sa;
> mm[i].msg_hdr.msg_namelen = sl;
>
> - if (len) {
> - m[i].iov_base = (char *)(uh_send + 1);
> - m[i].iov_len = len;
> + if (data.cnt) {
> + int len;
Variables named 'len' or similar are usually byte counts, not item
counts. So maybe a different name here?
> +
> + len = iov_slice(&m[j], UIO_MAXIOV - j,
> + &data.iov[0], data.cnt, data.off, NULL);
Might be nice to have a wrapper around iov_slice() that turns the
whole of an iov_tail into a new plain iov.
> + if (len < 0)
> + return p->count - idx;
>
> - mm[i].msg_hdr.msg_iov = m + i;
> - mm[i].msg_hdr.msg_iovlen = 1;
> + mm[i].msg_hdr.msg_iov = &m[j];
> + mm[i].msg_hdr.msg_iovlen = len;
> + j += len;
> } else {
> mm[i].msg_hdr.msg_iov = NULL;
> mm[i].msg_hdr.msg_iovlen = 0;
--
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 --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 11/20] tcp: Convert tcp_tap_handler() to use iov_tail
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (9 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 10/20] udp: " Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-14 3:26 ` David Gibson
2025-04-11 13:10 ` [PATCH v2 12/20] tcp: Convert tcp_data_from_tap() " Laurent Vivier
` (8 subsequent siblings)
19 siblings, 1 reply; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
Use packet_base() and extract headers using IOV_REMOVE_HEADER()
and iov_peek_header_() rather than packet_get().
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
tcp.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/tcp.c b/tcp.c
index 35626c914c6a..1838b73cf766 100644
--- a/tcp.c
+++ b/tcp.c
@@ -310,6 +310,16 @@
#include "tcp_buf.h"
#include "tcp_vu.h"
+/*
+ * The size of TCP header (including options) is given by doff (Data Offset)
+ * that is a 4-bit value specifying the number of 32-bit words in the header.
+ * The maximum value of doff is 15 [(1 << 4) - 1].
+ * The maximum length in bytes of options is 15 minus the number of 32-bit
+ * words in the minimal TCP header (5) multiplied by the length of a 32-bit
+ * word (4).
+ */
+#define OPTLEN_MAX (((1UL << 4) - 6) * 4UL)
+
#ifndef __USE_MISC
/* From Linux UAPI, missing in netinet/tcp.h provided by musl */
struct tcp_repair_opt {
@@ -1957,7 +1967,10 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
{
struct tcp_tap_conn *conn;
const struct tcphdr *th;
- size_t optlen, len;
+ char optsc[OPTLEN_MAX];
+ struct iov_tail data;
+ size_t optlen, l4len;
+ struct tcphdr thc;
const char *opts;
union flow *flow;
flow_sidx_t sidx;
@@ -1966,15 +1979,19 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
(void)pif;
- th = packet_get(p, idx, 0, sizeof(*th), &len);
+ if (!packet_base(p, idx, &data))
+ return 1;
+
+ l4len = iov_tail_size(&data);
+
+ th = IOV_REMOVE_HEADER(&data, thc);
if (!th)
return 1;
- len += sizeof(*th);
optlen = th->doff * 4UL - sizeof(*th);
/* Static checkers might fail to see this: */
- optlen = MIN(optlen, ((1UL << 4) /* from doff width */ - 6) * 4UL);
- opts = packet_get(p, idx, sizeof(*th), optlen, NULL);
+ optlen = MIN(optlen, OPTLEN_MAX);
+ opts = (char *)iov_peek_header_(&data, &optsc[0], optlen, 1);
sidx = flow_lookup_af(c, IPPROTO_TCP, PIF_TAP, af, saddr, daddr,
ntohs(th->source), ntohs(th->dest));
@@ -1986,7 +2003,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
tcp_conn_from_tap(c, af, saddr, daddr, th,
opts, optlen, now);
else
- tcp_rst_no_conn(c, af, saddr, daddr, flow_lbl, th, len);
+ tcp_rst_no_conn(c, af, saddr, daddr, flow_lbl, th, l4len);
return 1;
}
@@ -1994,7 +2011,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
ASSERT(pif_at_sidx(sidx) == PIF_TAP);
conn = &flow->tcp;
- flow_trace(conn, "packet length %zu from tap", len);
+ flow_trace(conn, "packet length %zu from tap", l4len);
if (th->rst) {
conn_event(c, conn, CLOSED);
--
@@ -310,6 +310,16 @@
#include "tcp_buf.h"
#include "tcp_vu.h"
+/*
+ * The size of TCP header (including options) is given by doff (Data Offset)
+ * that is a 4-bit value specifying the number of 32-bit words in the header.
+ * The maximum value of doff is 15 [(1 << 4) - 1].
+ * The maximum length in bytes of options is 15 minus the number of 32-bit
+ * words in the minimal TCP header (5) multiplied by the length of a 32-bit
+ * word (4).
+ */
+#define OPTLEN_MAX (((1UL << 4) - 6) * 4UL)
+
#ifndef __USE_MISC
/* From Linux UAPI, missing in netinet/tcp.h provided by musl */
struct tcp_repair_opt {
@@ -1957,7 +1967,10 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
{
struct tcp_tap_conn *conn;
const struct tcphdr *th;
- size_t optlen, len;
+ char optsc[OPTLEN_MAX];
+ struct iov_tail data;
+ size_t optlen, l4len;
+ struct tcphdr thc;
const char *opts;
union flow *flow;
flow_sidx_t sidx;
@@ -1966,15 +1979,19 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
(void)pif;
- th = packet_get(p, idx, 0, sizeof(*th), &len);
+ if (!packet_base(p, idx, &data))
+ return 1;
+
+ l4len = iov_tail_size(&data);
+
+ th = IOV_REMOVE_HEADER(&data, thc);
if (!th)
return 1;
- len += sizeof(*th);
optlen = th->doff * 4UL - sizeof(*th);
/* Static checkers might fail to see this: */
- optlen = MIN(optlen, ((1UL << 4) /* from doff width */ - 6) * 4UL);
- opts = packet_get(p, idx, sizeof(*th), optlen, NULL);
+ optlen = MIN(optlen, OPTLEN_MAX);
+ opts = (char *)iov_peek_header_(&data, &optsc[0], optlen, 1);
sidx = flow_lookup_af(c, IPPROTO_TCP, PIF_TAP, af, saddr, daddr,
ntohs(th->source), ntohs(th->dest));
@@ -1986,7 +2003,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
tcp_conn_from_tap(c, af, saddr, daddr, th,
opts, optlen, now);
else
- tcp_rst_no_conn(c, af, saddr, daddr, flow_lbl, th, len);
+ tcp_rst_no_conn(c, af, saddr, daddr, flow_lbl, th, l4len);
return 1;
}
@@ -1994,7 +2011,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
ASSERT(pif_at_sidx(sidx) == PIF_TAP);
conn = &flow->tcp;
- flow_trace(conn, "packet length %zu from tap", len);
+ flow_trace(conn, "packet length %zu from tap", l4len);
if (th->rst) {
conn_event(c, conn, CLOSED);
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 11/20] tcp: Convert tcp_tap_handler() to use iov_tail
2025-04-11 13:10 ` [PATCH v2 11/20] tcp: Convert tcp_tap_handler() to use iov_tail Laurent Vivier
@ 2025-04-14 3:26 ` David Gibson
0 siblings, 0 replies; 29+ messages in thread
From: David Gibson @ 2025-04-14 3:26 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 3330 bytes --]
On Fri, Apr 11, 2025 at 03:10:21PM +0200, Laurent Vivier wrote:
> Use packet_base() and extract headers using IOV_REMOVE_HEADER()
> and iov_peek_header_() rather than packet_get().
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> tcp.c | 31 ++++++++++++++++++++++++-------
> 1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/tcp.c b/tcp.c
> index 35626c914c6a..1838b73cf766 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -310,6 +310,16 @@
> #include "tcp_buf.h"
> #include "tcp_vu.h"
>
> +/*
> + * The size of TCP header (including options) is given by doff (Data Offset)
> + * that is a 4-bit value specifying the number of 32-bit words in the header.
> + * The maximum value of doff is 15 [(1 << 4) - 1].
> + * The maximum length in bytes of options is 15 minus the number of 32-bit
> + * words in the minimal TCP header (5) multiplied by the length of a 32-bit
> + * word (4).
> + */
> +#define OPTLEN_MAX (((1UL << 4) - 6) * 4UL)
> +
> #ifndef __USE_MISC
> /* From Linux UAPI, missing in netinet/tcp.h provided by musl */
> struct tcp_repair_opt {
> @@ -1957,7 +1967,10 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
> {
> struct tcp_tap_conn *conn;
> const struct tcphdr *th;
> - size_t optlen, len;
> + char optsc[OPTLEN_MAX];
> + struct iov_tail data;
> + size_t optlen, l4len;
> + struct tcphdr thc;
> const char *opts;
> union flow *flow;
> flow_sidx_t sidx;
> @@ -1966,15 +1979,19 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
>
> (void)pif;
>
> - th = packet_get(p, idx, 0, sizeof(*th), &len);
> + if (!packet_base(p, idx, &data))
> + return 1;
> +
> + l4len = iov_tail_size(&data);
> +
> + th = IOV_REMOVE_HEADER(&data, thc);
> if (!th)
> return 1;
> - len += sizeof(*th);
>
> optlen = th->doff * 4UL - sizeof(*th);
> /* Static checkers might fail to see this: */
> - optlen = MIN(optlen, ((1UL << 4) /* from doff width */ - 6) * 4UL);
> - opts = packet_get(p, idx, sizeof(*th), optlen, NULL);
> + optlen = MIN(optlen, OPTLEN_MAX);
> + opts = (char *)iov_peek_header_(&data, &optsc[0], optlen, 1);
Can you use remove_header here? AFAICT it makes logical sense to move
the tail on to covering just the payload at this point.
> sidx = flow_lookup_af(c, IPPROTO_TCP, PIF_TAP, af, saddr, daddr,
> ntohs(th->source), ntohs(th->dest));
> @@ -1986,7 +2003,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
> tcp_conn_from_tap(c, af, saddr, daddr, th,
> opts, optlen, now);
> else
> - tcp_rst_no_conn(c, af, saddr, daddr, flow_lbl, th, len);
> + tcp_rst_no_conn(c, af, saddr, daddr, flow_lbl, th, l4len);
> return 1;
> }
>
> @@ -1994,7 +2011,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
> ASSERT(pif_at_sidx(sidx) == PIF_TAP);
> conn = &flow->tcp;
>
> - flow_trace(conn, "packet length %zu from tap", len);
> + flow_trace(conn, "packet length %zu from tap", l4len);
>
> if (th->rst) {
> conn_event(c, conn, CLOSED);
--
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 --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 12/20] tcp: Convert tcp_data_from_tap() to use iov_tail
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (10 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 11/20] tcp: Convert tcp_tap_handler() to use iov_tail Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-14 3:28 ` David Gibson
2025-04-11 13:10 ` [PATCH v2 13/20] dhcpv6: move offset initialization out of dhcpv6_opt() Laurent Vivier
` (7 subsequent siblings)
19 siblings, 1 reply; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
Use packet_base() and extract headers using IOV_PEEK_HEADER()
rather than packet_get().
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
tcp.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/tcp.c b/tcp.c
index 1838b73cf766..bebbb01b39f5 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1651,15 +1651,21 @@ 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;
- size_t off;
+ struct iov_tail data;
+ struct tcphdr thc;
+ size_t off, size;
+ int count;
- 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;
@@ -1669,9 +1675,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;
+ iov_tail_drop(&data, off);
seq = ntohl(th->seq);
if (SEQ_LT(seq, conn->seq_from_tap) && len <= 1) {
@@ -1745,10 +1749,14 @@ 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++;
+ size = len - seq_offset;
+ count = iov_slice(&tcp_iov[iov_i], UIO_MAXIOV - iov_i,
+ &data.iov[0], data.cnt, data.off + seq_offset,
+ &size);
+ if (count < 0)
+ break;
+ seq_from_tap += size;
+ iov_i += count;
if (keep == i)
keep = -1;
--
@@ -1651,15 +1651,21 @@ 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;
- size_t off;
+ struct iov_tail data;
+ struct tcphdr thc;
+ size_t off, size;
+ int count;
- 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;
@@ -1669,9 +1675,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;
+ iov_tail_drop(&data, off);
seq = ntohl(th->seq);
if (SEQ_LT(seq, conn->seq_from_tap) && len <= 1) {
@@ -1745,10 +1749,14 @@ 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++;
+ size = len - seq_offset;
+ count = iov_slice(&tcp_iov[iov_i], UIO_MAXIOV - iov_i,
+ &data.iov[0], data.cnt, data.off + seq_offset,
+ &size);
+ if (count < 0)
+ break;
+ seq_from_tap += size;
+ iov_i += count;
if (keep == i)
keep = -1;
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 12/20] tcp: Convert tcp_data_from_tap() to use iov_tail
2025-04-11 13:10 ` [PATCH v2 12/20] tcp: Convert tcp_data_from_tap() " Laurent Vivier
@ 2025-04-14 3:28 ` David Gibson
0 siblings, 0 replies; 29+ messages in thread
From: David Gibson @ 2025-04-14 3:28 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 2329 bytes --]
On Fri, Apr 11, 2025 at 03:10:22PM +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 | 30 +++++++++++++++++++-----------
> 1 file changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/tcp.c b/tcp.c
> index 1838b73cf766..bebbb01b39f5 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -1651,15 +1651,21 @@ 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;
> - size_t off;
> + struct iov_tail data;
> + struct tcphdr thc;
> + size_t off, size;
> + int count;
>
> - 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;
>
> @@ -1669,9 +1675,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;
> + iov_tail_drop(&data, off);
>
> seq = ntohl(th->seq);
> if (SEQ_LT(seq, conn->seq_from_tap) && len <= 1) {
> @@ -1745,10 +1749,14 @@ 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++;
> + size = len - seq_offset;
> + count = iov_slice(&tcp_iov[iov_i], UIO_MAXIOV - iov_i,
> + &data.iov[0], data.cnt, data.off + seq_offset,
> + &size);
Here's another use case for that iov_slice() wrapper
> + if (count < 0)
> + break;
> + seq_from_tap += size;
> + 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 --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 13/20] dhcpv6: move offset initialization out of dhcpv6_opt()
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (11 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 12/20] tcp: Convert tcp_data_from_tap() " Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-14 4:07 ` David Gibson
2025-04-11 13:10 ` [PATCH v2 14/20] dhcpv6: Extract sending of NotOnLink status Laurent Vivier
` (6 subsequent siblings)
19 siblings, 1 reply; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
No functional change.
Currently, if dhcpv6_opt() is called with offset set to 0, it will set the
offset to point to DHCPv6 options offset.
To simplify the use of iovec_tail in a later patch, move the initialization
out of the function. Replace all the call using 0 by a call using
the offset of the DHCPv6 options.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
dhcpv6.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/dhcpv6.c b/dhcpv6.c
index 373a98869f9b..be51c278af7a 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -54,14 +54,14 @@ struct opt_hdr {
uint16_t l;
} __attribute__((packed));
+#define UDP_MSG_HDR_SIZE (sizeof(struct udphdr) + sizeof(struct msg_hdr))
# define OPT_SIZE_CONV(x) (htons_constant(x))
#define OPT_SIZE(x) OPT_SIZE_CONV(sizeof(struct opt_##x) - \
sizeof(struct opt_hdr))
#define OPT_VSIZE(x) (sizeof(struct opt_##x) - \
sizeof(struct opt_hdr))
#define OPT_MAX_SIZE IPV6_MIN_MTU - (sizeof(struct ipv6hdr) + \
- sizeof(struct udphdr) + \
- sizeof(struct msg_hdr))
+ UDP_MSG_HDR_SIZE)
/**
* struct opt_client_id - DHCPv6 Client Identifier option
@@ -290,8 +290,7 @@ static struct opt_hdr *dhcpv6_opt(const struct pool *p, size_t *offset,
struct opt_hdr *o;
size_t left;
- if (!*offset)
- *offset = sizeof(struct udphdr) + sizeof(struct msg_hdr);
+ ASSERT(*offset >= UDP_MSG_HDR_SIZE);
while ((o = packet_get_try(p, 0, *offset, sizeof(*o), &left))) {
unsigned int opt_len = ntohs(o->l) + sizeof(*o);
@@ -327,7 +326,7 @@ static struct opt_hdr *dhcpv6_ia_notonlink(const struct pool *p,
size_t offset;
foreach(ia_type, ia_types) {
- offset = 0;
+ offset = UDP_MSG_HDR_SIZE;
while ((ia = dhcpv6_opt(p, &offset, *ia_type))) {
if (ntohs(ia->l) < OPT_VSIZE(ia_na))
return NULL;
@@ -464,8 +463,9 @@ static size_t dhcpv6_client_fqdn_fill(const struct pool *p, const struct ctx *c,
o = (struct opt_client_fqdn *)(buf + offset);
encode_domain_name(o->domain_name, c->fqdn);
- req_opt = (struct opt_client_fqdn *)dhcpv6_opt(p, &(size_t){ 0 },
- OPT_CLIENT_FQDN);
+ req_opt = (struct opt_client_fqdn *)dhcpv6_opt(p,
+ &(size_t){ UDP_MSG_HDR_SIZE },
+ OPT_CLIENT_FQDN);
if (req_opt && req_opt->flags & 0x01 /* S flag */)
o->flags = 0x02 /* O flag */;
else
@@ -522,15 +522,15 @@ int dhcpv6(struct ctx *c, const struct pool *p,
if (!mh)
return -1;
- client_id = dhcpv6_opt(p, &(size_t){ 0 }, OPT_CLIENTID);
+ client_id = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_CLIENTID);
if (!client_id || ntohs(client_id->l) > OPT_VSIZE(client_id))
return -1;
- server_id = dhcpv6_opt(p, &(size_t){ 0 }, OPT_SERVERID);
+ server_id = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_SERVERID);
if (server_id && ntohs(server_id->l) != OPT_VSIZE(server_id))
return -1;
- ia = dhcpv6_opt(p, &(size_t){ 0 }, OPT_IA_NA);
+ ia = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_IA_NA);
if (ia && ntohs(ia->l) < MIN(OPT_VSIZE(ia_na), OPT_VSIZE(ia_ta)))
return -1;
@@ -580,7 +580,7 @@ int dhcpv6(struct ctx *c, const struct pool *p,
memcmp(&resp.server_id, server_id, sizeof(resp.server_id)))
return -1;
- if (ia || dhcpv6_opt(p, &(size_t){ 0 }, OPT_IA_TA))
+ if (ia || dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_IA_TA))
return -1;
info("DHCPv6: received INFORMATION_REQUEST, sending REPLY");
--
@@ -54,14 +54,14 @@ struct opt_hdr {
uint16_t l;
} __attribute__((packed));
+#define UDP_MSG_HDR_SIZE (sizeof(struct udphdr) + sizeof(struct msg_hdr))
# define OPT_SIZE_CONV(x) (htons_constant(x))
#define OPT_SIZE(x) OPT_SIZE_CONV(sizeof(struct opt_##x) - \
sizeof(struct opt_hdr))
#define OPT_VSIZE(x) (sizeof(struct opt_##x) - \
sizeof(struct opt_hdr))
#define OPT_MAX_SIZE IPV6_MIN_MTU - (sizeof(struct ipv6hdr) + \
- sizeof(struct udphdr) + \
- sizeof(struct msg_hdr))
+ UDP_MSG_HDR_SIZE)
/**
* struct opt_client_id - DHCPv6 Client Identifier option
@@ -290,8 +290,7 @@ static struct opt_hdr *dhcpv6_opt(const struct pool *p, size_t *offset,
struct opt_hdr *o;
size_t left;
- if (!*offset)
- *offset = sizeof(struct udphdr) + sizeof(struct msg_hdr);
+ ASSERT(*offset >= UDP_MSG_HDR_SIZE);
while ((o = packet_get_try(p, 0, *offset, sizeof(*o), &left))) {
unsigned int opt_len = ntohs(o->l) + sizeof(*o);
@@ -327,7 +326,7 @@ static struct opt_hdr *dhcpv6_ia_notonlink(const struct pool *p,
size_t offset;
foreach(ia_type, ia_types) {
- offset = 0;
+ offset = UDP_MSG_HDR_SIZE;
while ((ia = dhcpv6_opt(p, &offset, *ia_type))) {
if (ntohs(ia->l) < OPT_VSIZE(ia_na))
return NULL;
@@ -464,8 +463,9 @@ static size_t dhcpv6_client_fqdn_fill(const struct pool *p, const struct ctx *c,
o = (struct opt_client_fqdn *)(buf + offset);
encode_domain_name(o->domain_name, c->fqdn);
- req_opt = (struct opt_client_fqdn *)dhcpv6_opt(p, &(size_t){ 0 },
- OPT_CLIENT_FQDN);
+ req_opt = (struct opt_client_fqdn *)dhcpv6_opt(p,
+ &(size_t){ UDP_MSG_HDR_SIZE },
+ OPT_CLIENT_FQDN);
if (req_opt && req_opt->flags & 0x01 /* S flag */)
o->flags = 0x02 /* O flag */;
else
@@ -522,15 +522,15 @@ int dhcpv6(struct ctx *c, const struct pool *p,
if (!mh)
return -1;
- client_id = dhcpv6_opt(p, &(size_t){ 0 }, OPT_CLIENTID);
+ client_id = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_CLIENTID);
if (!client_id || ntohs(client_id->l) > OPT_VSIZE(client_id))
return -1;
- server_id = dhcpv6_opt(p, &(size_t){ 0 }, OPT_SERVERID);
+ server_id = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_SERVERID);
if (server_id && ntohs(server_id->l) != OPT_VSIZE(server_id))
return -1;
- ia = dhcpv6_opt(p, &(size_t){ 0 }, OPT_IA_NA);
+ ia = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_IA_NA);
if (ia && ntohs(ia->l) < MIN(OPT_VSIZE(ia_na), OPT_VSIZE(ia_ta)))
return -1;
@@ -580,7 +580,7 @@ int dhcpv6(struct ctx *c, const struct pool *p,
memcmp(&resp.server_id, server_id, sizeof(resp.server_id)))
return -1;
- if (ia || dhcpv6_opt(p, &(size_t){ 0 }, OPT_IA_TA))
+ if (ia || dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_IA_TA))
return -1;
info("DHCPv6: received INFORMATION_REQUEST, sending REPLY");
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 13/20] dhcpv6: move offset initialization out of dhcpv6_opt()
2025-04-11 13:10 ` [PATCH v2 13/20] dhcpv6: move offset initialization out of dhcpv6_opt() Laurent Vivier
@ 2025-04-14 4:07 ` David Gibson
0 siblings, 0 replies; 29+ messages in thread
From: David Gibson @ 2025-04-14 4:07 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 4112 bytes --]
On Fri, Apr 11, 2025 at 03:10:23PM +0200, Laurent Vivier wrote:
> No functional change.
>
> Currently, if dhcpv6_opt() is called with offset set to 0, it will set the
> offset to point to DHCPv6 options offset.
>
> To simplify the use of iovec_tail in a later patch, move the initialization
> out of the function. Replace all the call using 0 by a call using
> the offset of the DHCPv6 options.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
As a rule, I dislike automagical special cases like this one in any case.
> ---
> dhcpv6.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/dhcpv6.c b/dhcpv6.c
> index 373a98869f9b..be51c278af7a 100644
> --- a/dhcpv6.c
> +++ b/dhcpv6.c
> @@ -54,14 +54,14 @@ struct opt_hdr {
> uint16_t l;
> } __attribute__((packed));
>
> +#define UDP_MSG_HDR_SIZE (sizeof(struct udphdr) + sizeof(struct msg_hdr))
> # define OPT_SIZE_CONV(x) (htons_constant(x))
> #define OPT_SIZE(x) OPT_SIZE_CONV(sizeof(struct opt_##x) - \
> sizeof(struct opt_hdr))
> #define OPT_VSIZE(x) (sizeof(struct opt_##x) - \
> sizeof(struct opt_hdr))
> #define OPT_MAX_SIZE IPV6_MIN_MTU - (sizeof(struct ipv6hdr) + \
> - sizeof(struct udphdr) + \
> - sizeof(struct msg_hdr))
> + UDP_MSG_HDR_SIZE)
>
> /**
> * struct opt_client_id - DHCPv6 Client Identifier option
> @@ -290,8 +290,7 @@ static struct opt_hdr *dhcpv6_opt(const struct pool *p, size_t *offset,
> struct opt_hdr *o;
> size_t left;
>
> - if (!*offset)
> - *offset = sizeof(struct udphdr) + sizeof(struct msg_hdr);
> + ASSERT(*offset >= UDP_MSG_HDR_SIZE);
>
> while ((o = packet_get_try(p, 0, *offset, sizeof(*o), &left))) {
> unsigned int opt_len = ntohs(o->l) + sizeof(*o);
> @@ -327,7 +326,7 @@ static struct opt_hdr *dhcpv6_ia_notonlink(const struct pool *p,
> size_t offset;
>
> foreach(ia_type, ia_types) {
> - offset = 0;
> + offset = UDP_MSG_HDR_SIZE;
> while ((ia = dhcpv6_opt(p, &offset, *ia_type))) {
> if (ntohs(ia->l) < OPT_VSIZE(ia_na))
> return NULL;
> @@ -464,8 +463,9 @@ static size_t dhcpv6_client_fqdn_fill(const struct pool *p, const struct ctx *c,
>
> o = (struct opt_client_fqdn *)(buf + offset);
> encode_domain_name(o->domain_name, c->fqdn);
> - req_opt = (struct opt_client_fqdn *)dhcpv6_opt(p, &(size_t){ 0 },
> - OPT_CLIENT_FQDN);
> + req_opt = (struct opt_client_fqdn *)dhcpv6_opt(p,
> + &(size_t){ UDP_MSG_HDR_SIZE },
> + OPT_CLIENT_FQDN);
> if (req_opt && req_opt->flags & 0x01 /* S flag */)
> o->flags = 0x02 /* O flag */;
> else
> @@ -522,15 +522,15 @@ int dhcpv6(struct ctx *c, const struct pool *p,
> if (!mh)
> return -1;
>
> - client_id = dhcpv6_opt(p, &(size_t){ 0 }, OPT_CLIENTID);
> + client_id = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_CLIENTID);
> if (!client_id || ntohs(client_id->l) > OPT_VSIZE(client_id))
> return -1;
>
> - server_id = dhcpv6_opt(p, &(size_t){ 0 }, OPT_SERVERID);
> + server_id = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_SERVERID);
> if (server_id && ntohs(server_id->l) != OPT_VSIZE(server_id))
> return -1;
>
> - ia = dhcpv6_opt(p, &(size_t){ 0 }, OPT_IA_NA);
> + ia = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_IA_NA);
> if (ia && ntohs(ia->l) < MIN(OPT_VSIZE(ia_na), OPT_VSIZE(ia_ta)))
> return -1;
>
> @@ -580,7 +580,7 @@ int dhcpv6(struct ctx *c, const struct pool *p,
> memcmp(&resp.server_id, server_id, sizeof(resp.server_id)))
> return -1;
>
> - if (ia || dhcpv6_opt(p, &(size_t){ 0 }, OPT_IA_TA))
> + if (ia || dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_IA_TA))
> return -1;
>
> info("DHCPv6: received INFORMATION_REQUEST, sending REPLY");
--
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 --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 14/20] dhcpv6: Extract sending of NotOnLink status
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (12 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 13/20] dhcpv6: move offset initialization out of dhcpv6_opt() Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 15/20] dhcpv6: Convert to iov_tail Laurent Vivier
` (5 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
Extract code from dhcpv6() into a new function, dhcpv6_send_ia_notonlink()
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
dhcpv6.c | 60 ++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 39 insertions(+), 21 deletions(-)
diff --git a/dhcpv6.c b/dhcpv6.c
index be51c278af7a..549ddbbea735 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -355,6 +355,44 @@ err:
return ia;
}
+/**
+ * dhcpv6_send_ia_notonlink() - Send NotOnLink status
+ * @c: Execution context
+ * @ia: Pointer to non-appropriate IA_NA or IA_TA
+ * @client_id: Client ID message option
+ * xid: Transaction ID for message exchange
+ */
+static void dhcpv6_send_ia_notonlink(struct ctx *c, struct opt_hdr *ia,
+ const struct opt_hdr *client_id,
+ uint32_t xid)
+{
+ const struct in6_addr *src = &c->ip6.our_tap_ll;
+ size_t n;
+
+ info("DHCPv6: received CONFIRM with inappropriate IA,"
+ " sending NotOnLink status in REPLY");
+
+ ia->l = htons(OPT_VSIZE(ia_na) + sizeof(sc_not_on_link));
+
+ n = sizeof(struct opt_ia_na);
+ memcpy(resp_not_on_link.var, ia, n);
+ memcpy(resp_not_on_link.var + n, &sc_not_on_link,
+ sizeof(sc_not_on_link));
+
+ n += sizeof(sc_not_on_link);
+ memcpy(resp_not_on_link.var + n, client_id,
+ sizeof(struct opt_hdr) + ntohs(client_id->l));
+
+ n += sizeof(struct opt_hdr) + ntohs(client_id->l);
+
+ n = offsetof(struct resp_not_on_link_t, var) + n;
+
+ resp_not_on_link.hdr.xid = xid;
+
+ tap_udp6_send(c, src, 547, tap_ip6_daddr(c, src), 546,
+ xid, &resp_not_on_link, n);
+}
+
/**
* dhcpv6_dns_fill() - Fill in DNS Servers and Domain Search list options
* @c: Execution context
@@ -547,28 +585,8 @@ int dhcpv6(struct ctx *c, const struct pool *p,
return -1;
if ((bad_ia = dhcpv6_ia_notonlink(p, &c->ip6.addr))) {
- info("DHCPv6: received CONFIRM with inappropriate IA,"
- " sending NotOnLink status in REPLY");
-
- bad_ia->l = htons(OPT_VSIZE(ia_na) +
- sizeof(sc_not_on_link));
- n = sizeof(struct opt_ia_na);
- memcpy(resp_not_on_link.var, bad_ia, n);
-
- memcpy(resp_not_on_link.var + n,
- &sc_not_on_link, sizeof(sc_not_on_link));
- n += sizeof(sc_not_on_link);
-
- memcpy(resp_not_on_link.var + n, client_id,
- sizeof(struct opt_hdr) + ntohs(client_id->l));
- n += sizeof(struct opt_hdr) + ntohs(client_id->l);
-
- n = offsetof(struct resp_not_on_link_t, var) + n;
-
- resp_not_on_link.hdr.xid = mh->xid;
- tap_udp6_send(c, src, 547, tap_ip6_daddr(c, src), 546,
- mh->xid, &resp_not_on_link, n);
+ dhcpv6_send_ia_notonlink(c, bad_ia, client_id, mh->xid);
return 1;
}
--
@@ -355,6 +355,44 @@ err:
return ia;
}
+/**
+ * dhcpv6_send_ia_notonlink() - Send NotOnLink status
+ * @c: Execution context
+ * @ia: Pointer to non-appropriate IA_NA or IA_TA
+ * @client_id: Client ID message option
+ * xid: Transaction ID for message exchange
+ */
+static void dhcpv6_send_ia_notonlink(struct ctx *c, struct opt_hdr *ia,
+ const struct opt_hdr *client_id,
+ uint32_t xid)
+{
+ const struct in6_addr *src = &c->ip6.our_tap_ll;
+ size_t n;
+
+ info("DHCPv6: received CONFIRM with inappropriate IA,"
+ " sending NotOnLink status in REPLY");
+
+ ia->l = htons(OPT_VSIZE(ia_na) + sizeof(sc_not_on_link));
+
+ n = sizeof(struct opt_ia_na);
+ memcpy(resp_not_on_link.var, ia, n);
+ memcpy(resp_not_on_link.var + n, &sc_not_on_link,
+ sizeof(sc_not_on_link));
+
+ n += sizeof(sc_not_on_link);
+ memcpy(resp_not_on_link.var + n, client_id,
+ sizeof(struct opt_hdr) + ntohs(client_id->l));
+
+ n += sizeof(struct opt_hdr) + ntohs(client_id->l);
+
+ n = offsetof(struct resp_not_on_link_t, var) + n;
+
+ resp_not_on_link.hdr.xid = xid;
+
+ tap_udp6_send(c, src, 547, tap_ip6_daddr(c, src), 546,
+ xid, &resp_not_on_link, n);
+}
+
/**
* dhcpv6_dns_fill() - Fill in DNS Servers and Domain Search list options
* @c: Execution context
@@ -547,28 +585,8 @@ int dhcpv6(struct ctx *c, const struct pool *p,
return -1;
if ((bad_ia = dhcpv6_ia_notonlink(p, &c->ip6.addr))) {
- info("DHCPv6: received CONFIRM with inappropriate IA,"
- " sending NotOnLink status in REPLY");
-
- bad_ia->l = htons(OPT_VSIZE(ia_na) +
- sizeof(sc_not_on_link));
- n = sizeof(struct opt_ia_na);
- memcpy(resp_not_on_link.var, bad_ia, n);
-
- memcpy(resp_not_on_link.var + n,
- &sc_not_on_link, sizeof(sc_not_on_link));
- n += sizeof(sc_not_on_link);
-
- memcpy(resp_not_on_link.var + n, client_id,
- sizeof(struct opt_hdr) + ntohs(client_id->l));
- n += sizeof(struct opt_hdr) + ntohs(client_id->l);
-
- n = offsetof(struct resp_not_on_link_t, var) + n;
-
- resp_not_on_link.hdr.xid = mh->xid;
- tap_udp6_send(c, src, 547, tap_ip6_daddr(c, src), 546,
- mh->xid, &resp_not_on_link, n);
+ dhcpv6_send_ia_notonlink(c, bad_ia, client_id, mh->xid);
return 1;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 15/20] dhcpv6: Convert to iov_tail
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (13 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 14/20] dhcpv6: Extract sending of NotOnLink status Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 16/20] dhcpv6: Use iov_tail in dhcpv6_opt() Laurent Vivier
` (4 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
Use packet_base() and extract headers using IOV_REMOVE_HEADER()
and IOV_PEEK_HEADER() rather than packet_get().
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
dhcpv6.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/dhcpv6.c b/dhcpv6.c
index 549ddbbea735..cf5c77d5af72 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -534,9 +534,15 @@ int dhcpv6(struct ctx *c, const struct pool *p,
const struct msg_hdr *mh;
const struct udphdr *uh;
struct opt_hdr *bad_ia;
+ struct iov_tail data;
+ struct msg_hdr mhc;
+ struct udphdr uhc;
size_t mlen, n;
- uh = packet_get(p, 0, 0, sizeof(*uh), &mlen);
+ if (!packet_base(p, 0, &data))
+ return -1;
+
+ uh = IOV_REMOVE_HEADER(&data, uhc);
if (!uh)
return -1;
@@ -549,6 +555,7 @@ int dhcpv6(struct ctx *c, const struct pool *p,
if (!IN6_IS_ADDR_MULTICAST(daddr))
return -1;
+ mlen = iov_tail_size(&data);
if (mlen + sizeof(*uh) != ntohs(uh->len) || mlen < sizeof(*mh))
return -1;
@@ -556,7 +563,7 @@ int dhcpv6(struct ctx *c, const struct pool *p,
src = &c->ip6.our_tap_ll;
- mh = packet_get(p, 0, sizeof(*uh), sizeof(*mh), NULL);
+ mh = IOV_PEEK_HEADER(&data, mhc);
if (!mh)
return -1;
--
@@ -534,9 +534,15 @@ int dhcpv6(struct ctx *c, const struct pool *p,
const struct msg_hdr *mh;
const struct udphdr *uh;
struct opt_hdr *bad_ia;
+ struct iov_tail data;
+ struct msg_hdr mhc;
+ struct udphdr uhc;
size_t mlen, n;
- uh = packet_get(p, 0, 0, sizeof(*uh), &mlen);
+ if (!packet_base(p, 0, &data))
+ return -1;
+
+ uh = IOV_REMOVE_HEADER(&data, uhc);
if (!uh)
return -1;
@@ -549,6 +555,7 @@ int dhcpv6(struct ctx *c, const struct pool *p,
if (!IN6_IS_ADDR_MULTICAST(daddr))
return -1;
+ mlen = iov_tail_size(&data);
if (mlen + sizeof(*uh) != ntohs(uh->len) || mlen < sizeof(*mh))
return -1;
@@ -556,7 +563,7 @@ int dhcpv6(struct ctx *c, const struct pool *p,
src = &c->ip6.our_tap_ll;
- mh = packet_get(p, 0, sizeof(*uh), sizeof(*mh), NULL);
+ mh = IOV_PEEK_HEADER(&data, mhc);
if (!mh)
return -1;
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 16/20] dhcpv6: Use iov_tail in dhcpv6_opt()
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (14 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 15/20] dhcpv6: Convert to iov_tail Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 17/20] dhcp: Convert to iov_tail Laurent Vivier
` (3 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
dhcpv6.c | 179 ++++++++++++++++++++++++++++++++-----------------------
iov.c | 1 -
2 files changed, 103 insertions(+), 77 deletions(-)
diff --git a/dhcpv6.c b/dhcpv6.c
index cf5c77d5af72..6458697d348e 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -278,112 +278,122 @@ static struct resp_not_on_link_t {
/**
* dhcpv6_opt() - Get option from DHCPv6 message
- * @p: Packet pool, single packet with UDP header
- * @offset: Offset to look at, 0: end of header, set to option start
+ * @data: Data to look at (input/output)
* @type: Option type to look up, network order
*
- * Return: pointer to option header, or NULL on malformed or missing option
+ * Return: true if found and @data points to the option header,
+ * or false on malformed or missing option and @data is
+ * unmodified.
*/
-static struct opt_hdr *dhcpv6_opt(const struct pool *p, size_t *offset,
- uint16_t type)
+static bool dhcpv6_opt(struct iov_tail *data, uint16_t type)
{
- struct opt_hdr *o;
- size_t left;
+ struct iov_tail head = *data;
+ const struct opt_hdr *o;
+ struct opt_hdr oc;
- ASSERT(*offset >= UDP_MSG_HDR_SIZE);
-
- while ((o = packet_get_try(p, 0, *offset, sizeof(*o), &left))) {
+ while ((o = IOV_PEEK_HEADER(data, oc))) {
unsigned int opt_len = ntohs(o->l) + sizeof(*o);
- if (ntohs(o->l) > left)
- return NULL;
+ if (opt_len > iov_tail_size(data))
+ break;
if (o->t == type)
- return o;
+ return true;
- *offset += opt_len;
+ iov_tail_drop(data, opt_len);
}
- return NULL;
+ *data = head;
+ return false;
}
/**
* dhcpv6_ia_notonlink() - Check if any IA contains non-appropriate addresses
- * @p: Packet pool, single packet starting from UDP header
+ * @data: Data to look at, packet starting from UDP header (input/output)
* @la: Address we want to lease to the client
*
- * Return: pointer to non-appropriate IA_NA or IA_TA, if any, NULL otherwise
+ * Return: true and @data points to non-appropriate IA_NA or IA_TA, if any,
+ * false otherwise and @data is unmodified
*/
-static struct opt_hdr *dhcpv6_ia_notonlink(const struct pool *p,
- struct in6_addr *la)
+static bool dhcpv6_ia_notonlink(struct iov_tail *data,
+ struct in6_addr *la)
{
int ia_types[2] = { OPT_IA_NA, OPT_IA_TA }, *ia_type;
const struct opt_ia_addr *opt_addr;
+ struct iov_tail current, ia_base;
+ struct opt_ia_addr opt_addrc;
char buf[INET6_ADDRSTRLEN];
+ const struct opt_ia_na *ia;
struct in6_addr req_addr;
const struct opt_hdr *h;
- struct opt_hdr *ia;
- size_t offset;
+ struct opt_ia_na iac;
+ struct opt_hdr hc;
foreach(ia_type, ia_types) {
- offset = UDP_MSG_HDR_SIZE;
- while ((ia = dhcpv6_opt(p, &offset, *ia_type))) {
- if (ntohs(ia->l) < OPT_VSIZE(ia_na))
- return NULL;
-
- offset += sizeof(struct opt_ia_na);
-
- while ((h = dhcpv6_opt(p, &offset, OPT_IAAADR))) {
- if (ntohs(h->l) != OPT_VSIZE(ia_addr))
- return NULL;
-
- opt_addr = (const struct opt_ia_addr *)h;
+ current = *data;
+ while (dhcpv6_opt(¤t, *ia_type)) {
+ ia_base = current;
+ ia = IOV_REMOVE_HEADER(¤t, iac);
+ if (!ia || ntohs(ia->hdr.l) < OPT_VSIZE(ia_na))
+ goto notfound;
+
+ while (dhcpv6_opt(¤t, OPT_IAAADR)) {
+ h = IOV_PEEK_HEADER(¤t, hc);
+ if (!h || ntohs(h->l) != OPT_VSIZE(ia_addr))
+ goto notfound;
+
+ opt_addr = IOV_REMOVE_HEADER(¤t,
+ opt_addrc);
req_addr = opt_addr->addr;
if (!IN6_ARE_ADDR_EQUAL(la, &req_addr))
- goto err;
-
- offset += sizeof(struct opt_ia_addr);
+ goto notonlink;
}
}
}
- return NULL;
+notfound:
+ return false;
-err:
+notonlink:
info("DHCPv6: requested address %s not on link",
inet_ntop(AF_INET6, &req_addr, buf, sizeof(buf)));
- return ia;
+ *data = ia_base;
+ return true;
}
/**
* dhcpv6_send_ia_notonlink() - Send NotOnLink status
- * @c: Execution context
- * @ia: Pointer to non-appropriate IA_NA or IA_TA
- * @client_id: Client ID message option
- * xid: Transaction ID for message exchange
+ * @c: Execution context
+ * @ia_base: Non-appropriate IA_NA or IA_TA base
+ * @client_id_base: Client ID message option base
+ * @len: Client ID length
+ * @xid: Transaction ID for message exchange
*/
-static void dhcpv6_send_ia_notonlink(struct ctx *c, struct opt_hdr *ia,
- const struct opt_hdr *client_id,
- uint32_t xid)
+static void dhcpv6_send_ia_notonlink(struct ctx *c,
+ const struct iov_tail *ia_base,
+ const struct iov_tail *client_id_base,
+ int len, uint32_t xid)
{
const struct in6_addr *src = &c->ip6.our_tap_ll;
+ struct opt_hdr *ia = (struct opt_hdr *)resp_not_on_link.var;
size_t n;
info("DHCPv6: received CONFIRM with inappropriate IA,"
" sending NotOnLink status in REPLY");
- ia->l = htons(OPT_VSIZE(ia_na) + sizeof(sc_not_on_link));
-
n = sizeof(struct opt_ia_na);
- memcpy(resp_not_on_link.var, ia, n);
+ iov_to_buf(&ia_base->iov[0], ia_base->cnt, ia_base->off,
+ resp_not_on_link.var, n);
+ ia->l = htons(OPT_VSIZE(ia_na) + sizeof(sc_not_on_link));
memcpy(resp_not_on_link.var + n, &sc_not_on_link,
sizeof(sc_not_on_link));
n += sizeof(sc_not_on_link);
- memcpy(resp_not_on_link.var + n, client_id,
- sizeof(struct opt_hdr) + ntohs(client_id->l));
+ iov_to_buf(&client_id_base->iov[0], client_id_base->cnt,
+ client_id_base->off, resp_not_on_link.var + n,
+ sizeof(struct opt_hdr) + len);
- n += sizeof(struct opt_hdr) + ntohs(client_id->l);
+ n += sizeof(struct opt_hdr) + len;
n = offsetof(struct resp_not_on_link_t, var) + n;
@@ -472,17 +482,19 @@ search:
/**
* dhcpv6_client_fqdn_fill() - Fill in client FQDN option
+ * @data: Data to look at
* @c: Execution context
* @buf: Response message buffer where options will be appended
* @offset: Offset in message buffer for new options
*
* Return: updated length of response message buffer.
*/
-static size_t dhcpv6_client_fqdn_fill(const struct pool *p, const struct ctx *c,
+static size_t dhcpv6_client_fqdn_fill(const struct iov_tail *data,
+ const struct ctx *c,
char *buf, int offset)
{
- struct opt_client_fqdn const *req_opt;
+ struct iov_tail current = *data;
struct opt_client_fqdn *o;
size_t opt_len;
@@ -501,13 +513,16 @@ static size_t dhcpv6_client_fqdn_fill(const struct pool *p, const struct ctx *c,
o = (struct opt_client_fqdn *)(buf + offset);
encode_domain_name(o->domain_name, c->fqdn);
- req_opt = (struct opt_client_fqdn *)dhcpv6_opt(p,
- &(size_t){ UDP_MSG_HDR_SIZE },
- OPT_CLIENT_FQDN);
- if (req_opt && req_opt->flags & 0x01 /* S flag */)
- o->flags = 0x02 /* O flag */;
- else
- o->flags = 0x00;
+ if (dhcpv6_opt(¤t, OPT_CLIENT_FQDN)) {
+ struct opt_client_fqdn const *req_opt;
+ struct opt_client_fqdn req_optc;
+
+ req_opt = IOV_PEEK_HEADER(¤t, req_optc);
+ if (req_opt && req_opt->flags & 0x01 /* S flag */)
+ o->flags = 0x02 /* O flag */;
+ else
+ o->flags = 0x00;
+ }
opt_len++;
@@ -529,12 +544,16 @@ static size_t dhcpv6_client_fqdn_fill(const struct pool *p, const struct ctx *c,
int dhcpv6(struct ctx *c, const struct pool *p,
const struct in6_addr *saddr, const struct in6_addr *daddr)
{
- const struct opt_hdr *client_id, *server_id, *ia;
+ const struct opt_server_id *server_id = NULL;
+ struct iov_tail data, opt, client_id_base;
+ const struct opt_hdr *client_id = NULL;
+ const struct opt_ia_na *ia = NULL;
+ struct opt_server_id server_idc;
+ struct opt_hdr client_idc;
const struct in6_addr *src;
const struct msg_hdr *mh;
const struct udphdr *uh;
- struct opt_hdr *bad_ia;
- struct iov_tail data;
+ struct opt_ia_na iac;
struct msg_hdr mhc;
struct udphdr uhc;
size_t mlen, n;
@@ -563,20 +582,26 @@ int dhcpv6(struct ctx *c, const struct pool *p,
src = &c->ip6.our_tap_ll;
- mh = IOV_PEEK_HEADER(&data, mhc);
+ mh = IOV_REMOVE_HEADER(&data, mhc);
if (!mh)
return -1;
- client_id = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_CLIENTID);
+ client_id_base = data;
+ if (dhcpv6_opt(&client_id_base, OPT_CLIENTID))
+ client_id = IOV_PEEK_HEADER(&client_id_base, client_idc);
if (!client_id || ntohs(client_id->l) > OPT_VSIZE(client_id))
return -1;
- server_id = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_SERVERID);
- if (server_id && ntohs(server_id->l) != OPT_VSIZE(server_id))
+ opt = data;
+ if (dhcpv6_opt(&opt, OPT_SERVERID))
+ server_id = IOV_PEEK_HEADER(&opt, server_idc);
+ if (server_id && ntohs(server_id->hdr.l) != OPT_VSIZE(server_id))
return -1;
- ia = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_IA_NA);
- if (ia && ntohs(ia->l) < MIN(OPT_VSIZE(ia_na), OPT_VSIZE(ia_ta)))
+ opt = data;
+ if (dhcpv6_opt(&opt, OPT_IA_NA))
+ ia = IOV_PEEK_HEADER(&opt, iac);
+ if (ia && ntohs(ia->hdr.l) < MIN(OPT_VSIZE(ia_na), OPT_VSIZE(ia_ta)))
return -1;
resp.hdr.type = TYPE_REPLY;
@@ -591,9 +616,10 @@ int dhcpv6(struct ctx *c, const struct pool *p,
if (mh->type == TYPE_CONFIRM && server_id)
return -1;
- if ((bad_ia = dhcpv6_ia_notonlink(p, &c->ip6.addr))) {
+ if (dhcpv6_ia_notonlink(&data, &c->ip6.addr)) {
- dhcpv6_send_ia_notonlink(c, bad_ia, client_id, mh->xid);
+ dhcpv6_send_ia_notonlink(c, &data, &client_id_base,
+ ntohs(client_id->l), mh->xid);
return 1;
}
@@ -605,7 +631,7 @@ int dhcpv6(struct ctx *c, const struct pool *p,
memcmp(&resp.server_id, server_id, sizeof(resp.server_id)))
return -1;
- if (ia || dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_IA_TA))
+ if (ia || dhcpv6_opt(&data, OPT_IA_TA))
return -1;
info("DHCPv6: received INFORMATION_REQUEST, sending REPLY");
@@ -631,13 +657,14 @@ int dhcpv6(struct ctx *c, const struct pool *p,
if (ia)
resp.ia_na.iaid = ((struct opt_ia_na *)ia)->iaid;
- memcpy(&resp.client_id, client_id,
- ntohs(client_id->l) + sizeof(struct opt_hdr));
+ iov_to_buf(&client_id_base.iov[0], client_id_base.cnt,
+ client_id_base.off, &resp.client_id,
+ ntohs(client_id->l) + sizeof(struct opt_hdr));
n = offsetof(struct resp_t, client_id) +
sizeof(struct opt_hdr) + ntohs(client_id->l);
n = dhcpv6_dns_fill(c, (char *)&resp, n);
- n = dhcpv6_client_fqdn_fill(p, c, (char *)&resp, n);
+ n = dhcpv6_client_fqdn_fill(&data, c, (char *)&resp, n);
resp.hdr.xid = mh->xid;
diff --git a/iov.c b/iov.c
index d09e51190d54..adb47293dd28 100644
--- a/iov.c
+++ b/iov.c
@@ -108,7 +108,6 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
*
* Returns: The number of bytes successfully copied.
*/
-/* cppcheck-suppress [staticFunction] */
size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
size_t offset, void *buf, size_t bytes)
{
--
@@ -108,7 +108,6 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
*
* Returns: The number of bytes successfully copied.
*/
-/* cppcheck-suppress [staticFunction] */
size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
size_t offset, void *buf, size_t bytes)
{
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 17/20] dhcp: Convert to iov_tail
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (15 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 16/20] dhcpv6: Use iov_tail in dhcpv6_opt() Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 18/20] ip: Use iov_tail in ipv6_l4hdr() Laurent Vivier
` (2 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
Use packet_base() and extract headers using IOV_REMOVE_HEADER()
and IOV_PEEK_HEADER() rather than packet_get().
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
dhcp.c | 45 +++++++++++++++++++++++++++------------------
1 file changed, 27 insertions(+), 18 deletions(-)
diff --git a/dhcp.c b/dhcp.c
index b0de04be6f27..3365b0632c74 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -302,27 +302,33 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len)
*/
int dhcp(const struct ctx *c, const struct pool *p)
{
- size_t mlen, dlen, offset = 0, opt_len, opt_off = 0;
+ size_t mlen, dlen, opt_len;
char macstr[ETH_ADDRSTRLEN];
struct in_addr mask, dst;
const struct ethhdr *eh;
const struct iphdr *iph;
const struct udphdr *uh;
+ struct iov_tail data;
struct msg const *m;
+ struct msg mc;
+ struct ethhdr ehc;
+ struct iphdr iphc;
+ struct udphdr uhc;
struct msg reply;
unsigned int i;
- eh = packet_get(p, 0, offset, sizeof(*eh), NULL);
- offset += sizeof(*eh);
+ if (!packet_base(p, 0, &data))
+ return -1;
- iph = packet_get(p, 0, offset, sizeof(*iph), NULL);
+ eh = IOV_REMOVE_HEADER(&data, ehc);
+ iph = IOV_PEEK_HEADER(&data, iphc);
if (!eh || !iph)
return -1;
- offset += iph->ihl * 4UL;
- uh = packet_get(p, 0, offset, sizeof(*uh), &mlen);
- offset += sizeof(*uh);
+ if (!iov_tail_drop(&data, iph->ihl * 4UL))
+ return -1;
+ uh = IOV_REMOVE_HEADER(&data, uhc);
if (!uh)
return -1;
@@ -332,7 +338,9 @@ int dhcp(const struct ctx *c, const struct pool *p)
if (c->no_dhcp)
return 1;
- m = packet_get(p, 0, offset, offsetof(struct msg, o), &opt_len);
+ mlen = iov_tail_size(&data);
+ m = (struct msg const *)iov_remove_header_(&data, &mc, offsetof(struct msg, o),
+ __alignof__(struct msg));
if (!m ||
mlen != ntohs(uh->len) - sizeof(*uh) ||
mlen < offsetof(struct msg, o) ||
@@ -355,27 +363,28 @@ int dhcp(const struct ctx *c, const struct pool *p)
memset(&reply.file, 0, sizeof(reply.file));
reply.magic = m->magic;
- offset += offsetof(struct msg, o);
-
for (i = 0; i < ARRAY_SIZE(opts); i++)
opts[i].clen = -1;
- while (opt_off + 2 < opt_len) {
- const uint8_t *olen, *val;
+ opt_len = iov_tail_size(&data);
+ while (opt_len >= 2) {
+ uint8_t olenc, typec;
+ const uint8_t *olen;
uint8_t *type;
- type = packet_get(p, 0, offset + opt_off, 1, NULL);
- olen = packet_get(p, 0, offset + opt_off + 1, 1, NULL);
+ type = IOV_REMOVE_HEADER(&data, typec);
+ olen = IOV_REMOVE_HEADER(&data, olenc);
if (!type || !olen)
return -1;
- val = packet_get(p, 0, offset + opt_off + 2, *olen, NULL);
- if (!val)
+ opt_len = iov_tail_size(&data);
+ if (opt_len < *olen)
return -1;
- memcpy(&opts[*type].c, val, *olen);
+ iov_to_buf(&data.iov[0], data.cnt, data.off, &opts[*type].c, *olen);
opts[*type].clen = *olen;
- opt_off += *olen + 2;
+ iov_tail_drop(&data, *olen);
+ opt_len -= *olen;
}
opts[80].slen = -1;
--
@@ -302,27 +302,33 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len)
*/
int dhcp(const struct ctx *c, const struct pool *p)
{
- size_t mlen, dlen, offset = 0, opt_len, opt_off = 0;
+ size_t mlen, dlen, opt_len;
char macstr[ETH_ADDRSTRLEN];
struct in_addr mask, dst;
const struct ethhdr *eh;
const struct iphdr *iph;
const struct udphdr *uh;
+ struct iov_tail data;
struct msg const *m;
+ struct msg mc;
+ struct ethhdr ehc;
+ struct iphdr iphc;
+ struct udphdr uhc;
struct msg reply;
unsigned int i;
- eh = packet_get(p, 0, offset, sizeof(*eh), NULL);
- offset += sizeof(*eh);
+ if (!packet_base(p, 0, &data))
+ return -1;
- iph = packet_get(p, 0, offset, sizeof(*iph), NULL);
+ eh = IOV_REMOVE_HEADER(&data, ehc);
+ iph = IOV_PEEK_HEADER(&data, iphc);
if (!eh || !iph)
return -1;
- offset += iph->ihl * 4UL;
- uh = packet_get(p, 0, offset, sizeof(*uh), &mlen);
- offset += sizeof(*uh);
+ if (!iov_tail_drop(&data, iph->ihl * 4UL))
+ return -1;
+ uh = IOV_REMOVE_HEADER(&data, uhc);
if (!uh)
return -1;
@@ -332,7 +338,9 @@ int dhcp(const struct ctx *c, const struct pool *p)
if (c->no_dhcp)
return 1;
- m = packet_get(p, 0, offset, offsetof(struct msg, o), &opt_len);
+ mlen = iov_tail_size(&data);
+ m = (struct msg const *)iov_remove_header_(&data, &mc, offsetof(struct msg, o),
+ __alignof__(struct msg));
if (!m ||
mlen != ntohs(uh->len) - sizeof(*uh) ||
mlen < offsetof(struct msg, o) ||
@@ -355,27 +363,28 @@ int dhcp(const struct ctx *c, const struct pool *p)
memset(&reply.file, 0, sizeof(reply.file));
reply.magic = m->magic;
- offset += offsetof(struct msg, o);
-
for (i = 0; i < ARRAY_SIZE(opts); i++)
opts[i].clen = -1;
- while (opt_off + 2 < opt_len) {
- const uint8_t *olen, *val;
+ opt_len = iov_tail_size(&data);
+ while (opt_len >= 2) {
+ uint8_t olenc, typec;
+ const uint8_t *olen;
uint8_t *type;
- type = packet_get(p, 0, offset + opt_off, 1, NULL);
- olen = packet_get(p, 0, offset + opt_off + 1, 1, NULL);
+ type = IOV_REMOVE_HEADER(&data, typec);
+ olen = IOV_REMOVE_HEADER(&data, olenc);
if (!type || !olen)
return -1;
- val = packet_get(p, 0, offset + opt_off + 2, *olen, NULL);
- if (!val)
+ opt_len = iov_tail_size(&data);
+ if (opt_len < *olen)
return -1;
- memcpy(&opts[*type].c, val, *olen);
+ iov_to_buf(&data.iov[0], data.cnt, data.off, &opts[*type].c, *olen);
opts[*type].clen = *olen;
- opt_off += *olen + 2;
+ iov_tail_drop(&data, *olen);
+ opt_len -= *olen;
}
opts[80].slen = -1;
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 18/20] ip: Use iov_tail in ipv6_l4hdr()
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (16 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 17/20] dhcp: Convert to iov_tail Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 19/20] tap: Convert tap4_handler() to iov_tail Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 20/20] tap: Convert tap6_handler() " Laurent Vivier
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
Use packet_base() and extract headers using IOV_REMOVE_HEADER()
and IOV_PEEK_HEADER() rather than packet_get().
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
ip.c | 27 ++++++++++++---------------
ip.h | 3 +--
packet.c | 1 +
tap.c | 4 +++-
4 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/ip.c b/ip.c
index 2cc7f6548aff..b63fbe79fb5f 100644
--- a/ip.c
+++ b/ip.c
@@ -23,40 +23,37 @@
/**
* ipv6_l4hdr() - Find pointer to L4 header in IPv6 packet and extract protocol
- * @p: Packet pool, packet number @idx has IPv6 header at @offset
- * @idx: Index of packet in pool
- * @offset: Pre-calculated IPv6 header offset
+ * @data: IPv6 packet
* @proto: Filled with L4 protocol number
* @dlen: Data length (payload excluding header extensions), set on return
*
* Return: pointer to L4 header, NULL if not found
*/
-char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto,
- size_t *dlen)
+bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen)
{
const struct ipv6_opt_hdr *o;
+ struct ipv6_opt_hdr oc;
const struct ipv6hdr *ip6h;
- char *base;
+ struct ipv6hdr ip6hc;
int hdrlen;
uint8_t nh;
- base = packet_get(p, idx, 0, 0, NULL);
- ip6h = packet_get(p, idx, offset, sizeof(*ip6h), dlen);
+ ip6h = IOV_REMOVE_HEADER(data, ip6hc);
if (!ip6h)
- return NULL;
-
- offset += sizeof(*ip6h);
+ return false;
+ *dlen = iov_tail_size(data);
nh = ip6h->nexthdr;
if (!IPV6_NH_OPT(nh))
goto found;
- while ((o = packet_get_try(p, idx, offset, sizeof(*o), dlen))) {
+ while ((o = IOV_PEEK_HEADER(data, oc))) {
+ *dlen = iov_tail_size(data) - sizeof(*o);
nh = o->nexthdr;
hdrlen = (o->hdrlen + 1) * 8;
if (IPV6_NH_OPT(nh))
- offset += hdrlen;
+ iov_tail_drop(data, hdrlen);
else
goto found;
}
@@ -65,8 +62,8 @@ char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto,
found:
if (nh == 59)
- return NULL;
+ return false;
*proto = nh;
- return base + offset;
+ return true;
}
diff --git a/ip.h b/ip.h
index 471c57ee4c71..874cbe476a6b 100644
--- a/ip.h
+++ b/ip.h
@@ -115,8 +115,7 @@ static inline uint32_t ip6_get_flow_lbl(const struct ipv6hdr *ip6h)
ip6h->flow_lbl[2];
}
-char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto,
- size_t *dlen);
+bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen);
/* IPv6 link-local all-nodes multicast adddress, ff02::1 */
static const struct in6_addr in6addr_ll_all_nodes = {
diff --git a/packet.c b/packet.c
index c238696c58bb..75944328326e 100644
--- a/packet.c
+++ b/packet.c
@@ -133,6 +133,7 @@ void packet_add_do(struct pool *p, struct iov_tail *data,
*
* Return: pointer to start of data range, NULL on invalid range or descriptor
*/
+/* cppcheck-suppress [staticFunction] */
void *packet_get_try_do(const struct pool *p, size_t idx, size_t offset,
size_t len, size_t *left, const char *func, int line)
{
diff --git a/tap.c b/tap.c
index 52a94f9f8be2..e8726ae35cef 100644
--- a/tap.c
+++ b/tap.c
@@ -902,8 +902,10 @@ resume:
if (plen != check)
continue;
- if (!(l4h = ipv6_l4hdr(in, i, sizeof(*eh), &proto, &l4len)))
+ data = IOV_TAIL_FROM_BUF(ip6h, sizeof(*ip6h) + check, 0);
+ if (!ipv6_l4hdr(&data, &proto, &l4len))
continue;
+ l4h = (char *)data.iov[0].iov_base + data.off;
if (IN6_IS_ADDR_LOOPBACK(saddr) || IN6_IS_ADDR_LOOPBACK(daddr)) {
char sstr[INET6_ADDRSTRLEN], dstr[INET6_ADDRSTRLEN];
--
@@ -902,8 +902,10 @@ resume:
if (plen != check)
continue;
- if (!(l4h = ipv6_l4hdr(in, i, sizeof(*eh), &proto, &l4len)))
+ data = IOV_TAIL_FROM_BUF(ip6h, sizeof(*ip6h) + check, 0);
+ if (!ipv6_l4hdr(&data, &proto, &l4len))
continue;
+ l4h = (char *)data.iov[0].iov_base + data.off;
if (IN6_IS_ADDR_LOOPBACK(saddr) || IN6_IS_ADDR_LOOPBACK(daddr)) {
char sstr[INET6_ADDRSTRLEN], dstr[INET6_ADDRSTRLEN];
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 19/20] tap: Convert tap4_handler() to iov_tail
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (17 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 18/20] ip: Use iov_tail in ipv6_l4hdr() Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
2025-04-11 13:10 ` [PATCH v2 20/20] tap: Convert tap6_handler() " Laurent Vivier
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
Use packet_base() and extract headers using IOV_PEEK_HEADER()
rather than packet_get().
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
tap.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/tap.c b/tap.c
index e8726ae35cef..7899902e1042 100644
--- a/tap.c
+++ b/tap.c
@@ -699,28 +699,34 @@ static int tap4_handler(struct ctx *c, const struct pool *in,
i = 0;
resume:
for (seq_count = 0, seq = NULL; i < in->count; i++) {
- size_t l2len, l3len, hlen, l4len;
+ size_t l3len, hlen, l4len;
const struct ethhdr *eh;
const struct udphdr *uh;
struct iov_tail data;
+ struct ethhdr ehc;
struct iphdr *iph;
- const char *l4h;
+ struct iphdr iphc;
+ struct udphdr uhc;
- packet_get(in, i, 0, 0, &l2len);
+ if (!packet_base(in, i, &data))
+ continue;
- eh = packet_get(in, i, 0, sizeof(*eh), &l3len);
+ eh = IOV_PEEK_HEADER(&data, ehc);
if (!eh)
continue;
if (ntohs(eh->h_proto) == ETH_P_ARP) {
PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
- data = IOV_TAIL_FROM_BUF((void *)eh, l2len, 0);
packet_add(pkt, &data);
arp(c, pkt);
continue;
}
- iph = packet_get(in, i, sizeof(*eh), sizeof(*iph), NULL);
+ if (!iov_tail_drop(&data, sizeof(*eh)))
+ continue;
+ l3len = iov_tail_size(&data);
+
+ iph = IOV_PEEK_HEADER(&data, iphc);
if (!iph)
continue;
@@ -748,8 +754,9 @@ resume:
if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr)
c->ip4.addr_seen.s_addr = iph->saddr;
- l4h = packet_get(in, i, sizeof(*eh) + hlen, l4len, NULL);
- if (!l4h)
+ if (!iov_tail_drop(&data, hlen))
+ continue;
+ if (iov_tail_size(&data) != l4len)
continue;
if (iph->protocol == IPPROTO_ICMP) {
@@ -760,7 +767,6 @@ resume:
tap_packet_debug(iph, NULL, NULL, 0, NULL, 1);
- data = IOV_TAIL_FROM_BUF((void *)l4h, l4len, 0);
packet_add(pkt, &data);
icmp_tap_handler(c, PIF_TAP, AF_INET,
&iph->saddr, &iph->daddr,
@@ -768,15 +774,17 @@ resume:
continue;
}
- uh = packet_get(in, i, sizeof(*eh) + hlen, sizeof(*uh), NULL);
+ uh = IOV_PEEK_HEADER(&data, uhc);
if (!uh)
continue;
if (iph->protocol == IPPROTO_UDP) {
+ struct iov_tail eh_data;
+
PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
- data = IOV_TAIL_FROM_BUF((void *)eh, l2len, 0);
- packet_add(pkt, &data);
+ packet_base(in, i, &eh_data);
+ packet_add(pkt, &eh_data);
if (dhcp(c, pkt))
continue;
}
@@ -825,7 +833,6 @@ resume:
#undef L4_SET
append:
- data = IOV_TAIL_FROM_BUF((void *)l4h, l4len, 0);
packet_add((struct pool *)&seq->p, &data);
}
--
@@ -699,28 +699,34 @@ static int tap4_handler(struct ctx *c, const struct pool *in,
i = 0;
resume:
for (seq_count = 0, seq = NULL; i < in->count; i++) {
- size_t l2len, l3len, hlen, l4len;
+ size_t l3len, hlen, l4len;
const struct ethhdr *eh;
const struct udphdr *uh;
struct iov_tail data;
+ struct ethhdr ehc;
struct iphdr *iph;
- const char *l4h;
+ struct iphdr iphc;
+ struct udphdr uhc;
- packet_get(in, i, 0, 0, &l2len);
+ if (!packet_base(in, i, &data))
+ continue;
- eh = packet_get(in, i, 0, sizeof(*eh), &l3len);
+ eh = IOV_PEEK_HEADER(&data, ehc);
if (!eh)
continue;
if (ntohs(eh->h_proto) == ETH_P_ARP) {
PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
- data = IOV_TAIL_FROM_BUF((void *)eh, l2len, 0);
packet_add(pkt, &data);
arp(c, pkt);
continue;
}
- iph = packet_get(in, i, sizeof(*eh), sizeof(*iph), NULL);
+ if (!iov_tail_drop(&data, sizeof(*eh)))
+ continue;
+ l3len = iov_tail_size(&data);
+
+ iph = IOV_PEEK_HEADER(&data, iphc);
if (!iph)
continue;
@@ -748,8 +754,9 @@ resume:
if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr)
c->ip4.addr_seen.s_addr = iph->saddr;
- l4h = packet_get(in, i, sizeof(*eh) + hlen, l4len, NULL);
- if (!l4h)
+ if (!iov_tail_drop(&data, hlen))
+ continue;
+ if (iov_tail_size(&data) != l4len)
continue;
if (iph->protocol == IPPROTO_ICMP) {
@@ -760,7 +767,6 @@ resume:
tap_packet_debug(iph, NULL, NULL, 0, NULL, 1);
- data = IOV_TAIL_FROM_BUF((void *)l4h, l4len, 0);
packet_add(pkt, &data);
icmp_tap_handler(c, PIF_TAP, AF_INET,
&iph->saddr, &iph->daddr,
@@ -768,15 +774,17 @@ resume:
continue;
}
- uh = packet_get(in, i, sizeof(*eh) + hlen, sizeof(*uh), NULL);
+ uh = IOV_PEEK_HEADER(&data, uhc);
if (!uh)
continue;
if (iph->protocol == IPPROTO_UDP) {
+ struct iov_tail eh_data;
+
PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
- data = IOV_TAIL_FROM_BUF((void *)eh, l2len, 0);
- packet_add(pkt, &data);
+ packet_base(in, i, &eh_data);
+ packet_add(pkt, &eh_data);
if (dhcp(c, pkt))
continue;
}
@@ -825,7 +833,6 @@ resume:
#undef L4_SET
append:
- data = IOV_TAIL_FROM_BUF((void *)l4h, l4len, 0);
packet_add((struct pool *)&seq->p, &data);
}
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 20/20] tap: Convert tap6_handler() to iov_tail
2025-04-11 13:10 [PATCH v2 00/20] Introduce discontiguous frames management Laurent Vivier
` (18 preceding siblings ...)
2025-04-11 13:10 ` [PATCH v2 19/20] tap: Convert tap4_handler() to iov_tail Laurent Vivier
@ 2025-04-11 13:10 ` Laurent Vivier
19 siblings, 0 replies; 29+ messages in thread
From: Laurent Vivier @ 2025-04-11 13:10 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
Use packet_base() and extract headers using IOV_REMOVE_HEADER()
and IOV_PEEK_HEADER() rather than packet_get().
Remove packet_get() as it is not used anymore.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
packet.c | 70 --------------------------------------------------------
packet.h | 11 ---------
tap.c | 23 +++++++++++--------
3 files changed, 14 insertions(+), 90 deletions(-)
diff --git a/packet.c b/packet.c
index 75944328326e..99d8a6e1fcb0 100644
--- a/packet.c
+++ b/packet.c
@@ -121,76 +121,6 @@ void packet_add_do(struct pool *p, struct iov_tail *data,
p->count++;
}
-/**
- * packet_get_try_do() - Get data range from packet descriptor from given pool
- * @p: Packet pool
- * @idx: Index of packet descriptor in pool
- * @offset: Offset of data range in packet descriptor
- * @len: Length of desired data range
- * @left: Length of available data after range, set on return, can be NULL
- * @func: For tracing: name of calling function
- * @line: For tracing: caller line of function call
- *
- * Return: pointer to start of data range, NULL on invalid range or descriptor
- */
-/* cppcheck-suppress [staticFunction] */
-void *packet_get_try_do(const struct pool *p, size_t idx, size_t offset,
- size_t len, size_t *left, const char *func, int line)
-{
- char *ptr;
-
- ASSERT_WITH_MSG(p->count <= p->size,
- "Corrupt pool count: %zu, size: %zu, %s:%i",
- p->count, p->size, func, line);
-
- if (idx >= p->count) {
- debug("packet %zu from pool count: %zu, %s:%i",
- idx, p->count, func, line);
- return NULL;
- }
-
- if (offset > p->pkt[idx].iov_len ||
- len > (p->pkt[idx].iov_len - offset))
- return NULL;
-
- ptr = (char *)p->pkt[idx].iov_base + offset;
-
- ASSERT_WITH_MSG(!packet_check_range(p, ptr, len, func, line),
- "Corrupt packet pool, %s:%i", func, line);
-
- if (left)
- *left = p->pkt[idx].iov_len - offset - len;
-
- return ptr;
-}
-
-/**
- * packet_get_do() - Get data range from packet descriptor from given pool
- * @p: Packet pool
- * @idx: Index of packet descriptor in pool
- * @offset: Offset of data range in packet descriptor
- * @len: Length of desired data range
- * @left: Length of available data after range, set on return, can be NULL
- * @func: For tracing: name of calling function
- * @line: For tracing: caller line of function call
- *
- * Return: as packet_get_try_do() but log a trace message when returning NULL
- */
-void *packet_get_do(const struct pool *p, const size_t idx,
- size_t offset, size_t len, size_t *left,
- const char *func, int line)
-{
- void *r = packet_get_try_do(p, idx, offset, len, left, func, line);
-
- if (!r) {
- trace("missing packet data length %zu, offset %zu from "
- "length %zu, %s:%i",
- len, offset, p->pkt[idx].iov_len, func, line);
- }
-
- return r;
-}
-
/**
* packet_base_do() - Get data range from packet descriptor from given pool
* @p: Packet pool
diff --git a/packet.h b/packet.h
index ee7a5304a034..8a891822efa5 100644
--- a/packet.h
+++ b/packet.h
@@ -33,12 +33,6 @@ struct pool {
int vu_packet_check_range(void *buf, const char *ptr, size_t len);
void packet_add_do(struct pool *p, struct iov_tail *data,
const char *func, int line);
-void *packet_get_try_do(const struct pool *p, const size_t idx,
- size_t offset, size_t len, size_t *left,
- const char *func, int line);
-void *packet_get_do(const struct pool *p, const size_t idx,
- size_t offset, size_t len, size_t *left,
- const char *func, int line);
bool packet_base_do(const struct pool *p, const size_t idx,
struct iov_tail *data,
const char *func, int line);
@@ -47,11 +41,6 @@ void pool_flush(struct pool *p);
#define packet_add(p, data) \
packet_add_do(p, data, __func__, __LINE__)
-
-#define packet_get_try(p, idx, offset, len, left) \
- packet_get_try_do(p, idx, offset, len, left, __func__, __LINE__)
-#define packet_get(p, idx, offset, len, left) \
- packet_get_do(p, idx, offset, len, left, __func__, __LINE__)
#define packet_base(p, idx, data) \
packet_base_do(p, idx, data, __func__, __LINE__)
diff --git a/tap.c b/tap.c
index 7899902e1042..8eb101222b50 100644
--- a/tap.c
+++ b/tap.c
@@ -891,17 +891,24 @@ resume:
const struct udphdr *uh;
struct iov_tail data;
struct ipv6hdr *ip6h;
+ struct ipv6hdr ip6hc;
+ struct ethhdr ehc;
+ struct udphdr uhc;
uint8_t proto;
- char *l4h;
- eh = packet_get(in, i, 0, sizeof(*eh), NULL);
+ if (!packet_base(in, i, &data))
+ return -1;
+
+ eh = IOV_REMOVE_HEADER(&data, ehc);
if (!eh)
continue;
- ip6h = packet_get(in, i, sizeof(*eh), sizeof(*ip6h), &check);
+ ip6h = IOV_PEEK_HEADER(&data, ip6hc);
if (!ip6h)
continue;
+ check = iov_tail_size(&data) - sizeof(*ip6h);
+
saddr = &ip6h->saddr;
daddr = &ip6h->daddr;
@@ -909,10 +916,8 @@ resume:
if (plen != check)
continue;
- data = IOV_TAIL_FROM_BUF(ip6h, sizeof(*ip6h) + check, 0);
if (!ipv6_l4hdr(&data, &proto, &l4len))
continue;
- l4h = (char *)data.iov[0].iov_base + data.off;
if (IN6_IS_ADDR_LOOPBACK(saddr) || IN6_IS_ADDR_LOOPBACK(daddr)) {
char sstr[INET6_ADDRSTRLEN], dstr[INET6_ADDRSTRLEN];
@@ -937,6 +942,8 @@ resume:
}
if (proto == IPPROTO_ICMPV6) {
+ const struct icmp6hdr *l4h;
+ struct icmp6hdr l4hc;
PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
if (c->no_icmp)
@@ -945,9 +952,9 @@ resume:
if (l4len < sizeof(struct icmp6hdr))
continue;
- data = IOV_TAIL_FROM_BUF(l4h, l4len, 0);
packet_add(pkt, &data);
+ l4h = IOV_PEEK_HEADER(&data, l4hc);
if (ndp(c, (struct icmp6hdr *)l4h, saddr, pkt))
continue;
@@ -960,12 +967,11 @@ resume:
if (l4len < sizeof(*uh))
continue;
- uh = (struct udphdr *)l4h;
+ uh = IOV_PEEK_HEADER(&data, uhc);
if (proto == IPPROTO_UDP) {
PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
- data = IOV_TAIL_FROM_BUF(l4h, l4len, 0);
packet_add(pkt, &data);
if (dhcpv6(c, pkt, saddr, daddr))
@@ -1020,7 +1026,6 @@ resume:
#undef L4_SET
append:
- data = IOV_TAIL_FROM_BUF(l4h, l4len, 0);
packet_add((struct pool *)&seq->p, &data);
}
--
@@ -891,17 +891,24 @@ resume:
const struct udphdr *uh;
struct iov_tail data;
struct ipv6hdr *ip6h;
+ struct ipv6hdr ip6hc;
+ struct ethhdr ehc;
+ struct udphdr uhc;
uint8_t proto;
- char *l4h;
- eh = packet_get(in, i, 0, sizeof(*eh), NULL);
+ if (!packet_base(in, i, &data))
+ return -1;
+
+ eh = IOV_REMOVE_HEADER(&data, ehc);
if (!eh)
continue;
- ip6h = packet_get(in, i, sizeof(*eh), sizeof(*ip6h), &check);
+ ip6h = IOV_PEEK_HEADER(&data, ip6hc);
if (!ip6h)
continue;
+ check = iov_tail_size(&data) - sizeof(*ip6h);
+
saddr = &ip6h->saddr;
daddr = &ip6h->daddr;
@@ -909,10 +916,8 @@ resume:
if (plen != check)
continue;
- data = IOV_TAIL_FROM_BUF(ip6h, sizeof(*ip6h) + check, 0);
if (!ipv6_l4hdr(&data, &proto, &l4len))
continue;
- l4h = (char *)data.iov[0].iov_base + data.off;
if (IN6_IS_ADDR_LOOPBACK(saddr) || IN6_IS_ADDR_LOOPBACK(daddr)) {
char sstr[INET6_ADDRSTRLEN], dstr[INET6_ADDRSTRLEN];
@@ -937,6 +942,8 @@ resume:
}
if (proto == IPPROTO_ICMPV6) {
+ const struct icmp6hdr *l4h;
+ struct icmp6hdr l4hc;
PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
if (c->no_icmp)
@@ -945,9 +952,9 @@ resume:
if (l4len < sizeof(struct icmp6hdr))
continue;
- data = IOV_TAIL_FROM_BUF(l4h, l4len, 0);
packet_add(pkt, &data);
+ l4h = IOV_PEEK_HEADER(&data, l4hc);
if (ndp(c, (struct icmp6hdr *)l4h, saddr, pkt))
continue;
@@ -960,12 +967,11 @@ resume:
if (l4len < sizeof(*uh))
continue;
- uh = (struct udphdr *)l4h;
+ uh = IOV_PEEK_HEADER(&data, uhc);
if (proto == IPPROTO_UDP) {
PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
- data = IOV_TAIL_FROM_BUF(l4h, l4len, 0);
packet_add(pkt, &data);
if (dhcpv6(c, pkt, saddr, daddr))
@@ -1020,7 +1026,6 @@ resume:
#undef L4_SET
append:
- data = IOV_TAIL_FROM_BUF(l4h, l4len, 0);
packet_add((struct pool *)&seq->p, &data);
}
--
2.49.0
^ permalink raw reply related [flat|nested] 29+ messages in thread