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 split across several iovec and it's easier to rebuild it in a buffer than updating an existing iovec array. Signed-off-by: Laurent Vivier Reviewed-by: David Gibson --- arp.c | 84 ++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 55 insertions(+), 29 deletions(-) diff --git a/arp.c b/arp.c index fc482bbd9938..721ff2d529b5 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 + * accept_arp() - Check if we should accept this 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 accept_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 (accept_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

These utilities enhance iov_tail manipulation, useful for efficient packet processing by enabling iovec array cloning and header stripping without data copies. - iov_tail_drop(): Discards a specified number of bytes from the beginning of an iov_tail by advancing its internal offset and pruning consumed elements. - iov_tail_clone(): Clone an iov_tail into an iovec array, adjusting the first iovec entry to remove the iov_tail offset. Signed-off-by: Laurent Vivier --- iov.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ iov.h | 3 +++ 2 files changed, 55 insertions(+) diff --git a/iov.c b/iov.c index 91e87a740f0a..3b89f8126ee7 100644 --- a/iov.c +++ b/iov.c @@ -192,6 +192,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 item 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 @@ -248,3 +263,40 @@ void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align) tail->off = tail->off + len; return p; } + +/** + * iov_tail_clone() - Assign iov references referencing a subset of the data + * in an iov_tail + * + * @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. + * @tail: Pointer to the source iov_tail + * + * Returns: The number of elements successfully copied to the destination + * iov array, a negative value if there is not enough room in the + * destination iov array + */ +/* cppcheck-suppress unusedFunction */ +ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt, + struct iov_tail *tail) +{ + const struct iovec *iov = &tail->iov[0]; + size_t iov_cnt = tail->cnt; + size_t offset = tail->off; + unsigned int i, j; + + i = iov_skip_bytes(iov, iov_cnt, offset, &offset); + + /* assign iov references referencing a subset of the source one */ + for (j = 0; i < iov_cnt && j < dst_iov_cnt; i++, j++) { + dst_iov[j].iov_base = (char *)iov[i].iov_base + offset; + dst_iov[j].iov_len = iov[i].iov_len - offset; + offset = 0; + } + + if (j == dst_iov_cnt && i != iov_cnt) + return -1; + + return j; +} diff --git a/iov.h b/iov.h index 9855bf0c0c32..11a8567aa183 100644 --- a/iov.h +++ b/iov.h @@ -72,8 +72,11 @@ 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); +ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt, + struct iov_tail *tail); /** * IOV_PEEK_HEADER() - Get typed pointer to a header from an IOV tail -- 2.49.0

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 --- iov.c | 53 ++++++++++++++++++++++++++++++++++++++++++++--------- iov.h | 55 +++++++++++++++++++++++++++++++++++++++++-------------- tcp_buf.c | 2 +- 3 files changed, 86 insertions(+), 24 deletions(-) diff --git a/iov.c b/iov.c index 3b89f8126ee7..541ca8aa9119 100644 --- a/iov.c +++ b/iov.c @@ -109,7 +109,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) { @@ -127,6 +127,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; @@ -208,7 +209,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 @@ -219,8 +220,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; @@ -240,27 +240,62 @@ 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 11a8567aa183..7d5bce2f841c 100644 --- a/iov.h +++ b/iov.h @@ -70,41 +70,68 @@ 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); ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt, struct iov_tail *tail); /** * 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..ac6d73130f55 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 th_storage, *th = IOV_REMOVE_HEADER(&tail, th_storage); 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

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 Reviewed-by: David Gibson --- 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 541ca8aa9119..cc50ea9e3e6c 100644 --- a/iov.c +++ b/iov.c @@ -200,7 +200,6 @@ size_t iov_tail_size(struct iov_tail *tail) * * Returns: true if the item 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 d630f6df97f1..db4f6e40781f 100644 --- a/tap.c +++ b/tap.c @@ -1068,24 +1068,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) { + struct ethhdr eh_storage; const struct ethhdr *eh; - pcap(p, l2len); + pcap_iov(data->iov, data->cnt, data->off); - eh = (struct ethhdr *)p; + eh = IOV_PEEK_HEADER(data, eh_storage); + 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: @@ -1093,14 +1098,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; @@ -1166,6 +1173,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"); @@ -1180,7 +1188,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; @@ -1224,6 +1233,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) { @@ -1245,7 +1256,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++; } -- 2.49.0

packet_data() 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 --- packet.c | 42 ++++++++++++++++++++++++++++++++++++++++++ packet.h | 5 +++++ 2 files changed, 47 insertions(+) diff --git a/packet.c b/packet.c index 98ded4e27aae..82adc9fd1a39 100644 --- a/packet.c +++ b/packet.c @@ -190,6 +190,48 @@ void *packet_get_do(const struct pool *p, const size_t idx, return r; } +/** + * packet_data_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: false if packet index is invalid, true otherwise. + * If something wrong with @data, don't return at all (assert). + */ +/* cppcheck-suppress unusedFunction */ +bool packet_data_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, + "Corrupted 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..062afb978124 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_data_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_data(p, idx, data) \ + packet_data_do(p, idx, data, __func__, __LINE__) #define PACKET_POOL_DECL(_name, _size, _buf) \ struct _name ## _t { \ -- 2.49.0

Use packet_data() and extract headers using IOV_REMOVE_HEADER() rather than packet_get(). Signed-off-by: Laurent Vivier Reviewed-by: David Gibson --- ndp.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ndp.c b/ndp.c index b664034b00b1..77722353be7b 100644 --- a/ndp.c +++ b/ndp.c @@ -351,9 +351,14 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih, return 1; if (ih->icmp6_type == NS) { + struct ndp_ns ns_storage; const struct ndp_ns *ns; + struct iov_tail data; - ns = packet_get(p, 0, 0, sizeof(struct ndp_ns), NULL); + if (!packet_data(p, 0, &data)) + return -1; + + ns = IOV_REMOVE_HEADER(&data, ns_storage); if (!ns) return -1; -- 2.49.0

Use packet_data() and extract headers using IOV_REMOVE_HEADER() and IOV_PEEK_HEADER() rather than packet_get(). Signed-off-by: Laurent Vivier --- iov.c | 1 - udp.c | 33 ++++++++++++++++++++++----------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/iov.c b/iov.c index 910103883827..ca6a35574f1b 100644 --- a/iov.c +++ b/iov.c @@ -334,7 +334,6 @@ void *iov_remove_header_(struct iov_tail *tail, void *v, size_t len, size_t alig * iov array, a negative value if there is not enough room in the * destination iov array */ -/* cppcheck-suppress unusedFunction */ ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt, struct iov_tail *tail) { diff --git a/udp.c b/udp.c index ca28b37c142d..cfa653508d9c 100644 --- a/udp.c +++ b/udp.c @@ -978,9 +978,11 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, struct mmsghdr mm[UIO_MAXIOV]; union sockaddr_inany to_sa; struct iovec m[UIO_MAXIOV]; + struct udphdr uh_storage; 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; uint8_t topif; @@ -988,7 +990,10 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, ASSERT(!c->no_udp); - uh = packet_get(p, idx, 0, sizeof(*uh), NULL); + if (!packet_data(p, idx, &data)) + return 1; + + uh = IOV_PEEK_HEADER(&data, uh_storage); if (!uh) return 1; @@ -1025,23 +1030,29 @@ 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_data(p, idx + i, &data)) + return p->count - idx; + + uh_send = IOV_REMOVE_HEADER(&data, uh_storage); 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 cnt; + + cnt = iov_tail_clone(&m[j], UIO_MAXIOV - j, &data); + if (cnt < 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 = cnt; + j += cnt; } else { mm[i].msg_hdr.msg_iov = NULL; mm[i].msg_hdr.msg_iovlen = 0; -- 2.49.0

Use packet_data() and extract headers using IOV_PEEK_HEADER() rather than packet_get(). Signed-off-by: Laurent Vivier --- tcp.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/tcp.c b/tcp.c index 7c543e4a5a57..be1d3e0c0d32 100644 --- a/tcp.c +++ b/tcp.c @@ -1649,16 +1649,22 @@ 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; + struct tcphdr th_storage; const struct tcphdr *th; - char *data; - size_t off; + struct iov_tail data; + size_t off, size; + int count; - th = packet_get(p, i, 0, sizeof(*th), &len); + if (!packet_data(p, i, &data)) + return -1; + + th = IOV_PEEK_HEADER(&data, th_storage); if (!th) return -1; - len += sizeof(*th); + len = iov_tail_size(&data); off = th->doff * 4UL; + if (off < sizeof(*th) || off > len) return -1; @@ -1668,9 +1674,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) { @@ -1744,10 +1748,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++; + iov_tail_drop(&data, seq_offset); + size = len - seq_offset; + count = iov_tail_clone(&tcp_iov[iov_i], UIO_MAXIOV - iov_i, + &data); + if (count < 0) + break; + seq_from_tap += size; + iov_i += count; if (keep == i) keep = -1; -- 2.49.0

Extract code from dhcpv6() into a new function, dhcpv6_send_ia_notonlink() Signed-off-by: Laurent Vivier Reviewed-by: David Gibson --- dhcpv6.c | 60 ++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/dhcpv6.c b/dhcpv6.c index 1e540bb84f0d..8e6c29df205d 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -357,6 +357,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 @@ -549,28 +587,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

dhcpv6_opt() and its callers are refactored for iov_tail option parsing, replacing direct offset management for improved robustness. Its signature is now `bool dhcpv6_opt(iov_tail *data, type)`. `*data` (in/out) points to a found option on `true` return or is restored on `false`. The main dhcpv6() function uses IOV_REMOVE_HEADER for the msg_hdr, then passes the iov_tail (now at options start) to the new dhcpv6_opt(). Signed-off-by: Laurent Vivier --- dhcpv6.c | 178 +++++++++++++++++++++++++++++++------------------------ iov.c | 1 - 2 files changed, 102 insertions(+), 77 deletions(-) diff --git a/dhcpv6.c b/dhcpv6.c index ae06e646f92f..bd6268804d95 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -280,112 +280,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: Buffer with options, set to matching option on return * @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; + struct opt_hdr o_storage; + const struct opt_hdr *o; - ASSERT(*offset >= UDP_MSG_HDR_SIZE); - - while ((o = packet_get_try(p, 0, *offset, sizeof(*o), &left))) { + while ((o = IOV_PEEK_HEADER(data, o_storage))) { 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; + struct opt_ia_addr opt_addr_storage; const struct opt_ia_addr *opt_addr; + struct iov_tail current, ia_base; + struct opt_ia_na ia_storage; char buf[INET6_ADDRSTRLEN]; + const struct opt_ia_na *ia; struct in6_addr req_addr; + struct opt_hdr h_storage; const struct opt_hdr *h; - struct opt_hdr *ia; - size_t offset; 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, ia_storage); + if (!ia || ntohs(ia->hdr.l) < OPT_VSIZE(ia_na)) + goto notfound; + + while (dhcpv6_opt(¤t, OPT_IAAADR)) { + h = IOV_PEEK_HEADER(¤t, h_storage); + if (!h || ntohs(h->l) != OPT_VSIZE(ia_addr)) + goto notfound; + + opt_addr = IOV_REMOVE_HEADER(¤t, + opt_addr_storage); 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; @@ -474,17 +484,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; @@ -502,14 +514,16 @@ static size_t dhcpv6_client_fqdn_fill(const struct pool *p, const struct ctx *c, } o = (struct opt_client_fqdn *)(buf + offset); + o->flags = 0x00; 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 req_opt_storage; + struct opt_client_fqdn const *req_opt; + + req_opt = IOV_PEEK_HEADER(¤t, req_opt_storage); + if (req_opt && req_opt->flags & 0x01 /* S flag */) + o->flags = 0x02 /* O flag */; + } opt_len++; @@ -531,14 +545,18 @@ 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; + struct opt_server_id server_id_storage; + const struct opt_ia_na *ia = NULL; + struct opt_hdr client_id_storage; + struct opt_ia_na ia_storage; const struct in6_addr *src; struct msg_hdr mh_storage; const struct msg_hdr *mh; struct udphdr uh_storage; const struct udphdr *uh; - struct opt_hdr *bad_ia; - struct iov_tail data; size_t mlen, n; if (!packet_data(p, 0, &data)) @@ -565,20 +583,26 @@ int dhcpv6(struct ctx *c, const struct pool *p, src = &c->ip6.our_tap_ll; - mh = IOV_PEEK_HEADER(&data, mh_storage); + mh = IOV_REMOVE_HEADER(&data, mh_storage); 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_id_storage); 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_id_storage); + 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, ia_storage); + if (ia && ntohs(ia->hdr.l) < MIN(OPT_VSIZE(ia_na), OPT_VSIZE(ia_ta))) return -1; resp.hdr.type = TYPE_REPLY; @@ -593,9 +617,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; } @@ -607,7 +632,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"); @@ -633,13 +658,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 ca6a35574f1b..76d261f804ca 100644 --- a/iov.c +++ b/iov.c @@ -109,7 +109,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

Use packet_data() and extract headers using IOV_REMOVE_HEADER() and IOV_PEEK_HEADER() rather than packet_get(). Signed-off-by: Laurent Vivier --- dhcp.c | 45 +++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/dhcp.c b/dhcp.c index b0de04be6f27..7c92f30d0b39 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; char macstr[ETH_ADDRSTRLEN]; + size_t mlen, dlen, opt_len; struct in_addr mask, dst; + struct ethhdr eh_storage; + struct iphdr iph_storage; + struct udphdr uh_storage; const struct ethhdr *eh; const struct iphdr *iph; const struct udphdr *uh; + struct iov_tail data; struct msg const *m; struct msg reply; unsigned int i; + struct msg mc; - eh = packet_get(p, 0, offset, sizeof(*eh), NULL); - offset += sizeof(*eh); + if (!packet_data(p, 0, &data)) + return -1; - iph = packet_get(p, 0, offset, sizeof(*iph), NULL); + eh = IOV_REMOVE_HEADER(&data, eh_storage); + iph = IOV_PEEK_HEADER(&data, iph_storage); 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, uh_storage); 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 olen_storage, type_storage; + 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, type_storage); + olen = IOV_REMOVE_HEADER(&data, olen_storage); 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

Use packet_data() and extract headers using IOV_REMOVE_HEADER() and IOV_PEEK_HEADER() rather than packet_get(). Signed-off-by: Laurent Vivier --- ip.c | 32 +++++++++++++++----------------- ip.h | 3 +-- packet.c | 1 + tap.c | 4 +++- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/ip.c b/ip.c index 2cc7f6548aff..50bd69a70596 100644 --- a/ip.c +++ b/ip.c @@ -23,50 +23,48 @@ /** * 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 + * Return: true if the L4 header is found and @data, @proto, @dlen are set, + * false on error. Outputs are indeterminate on failure. */ -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) { + struct ipv6_opt_hdr o_storage; const struct ipv6_opt_hdr *o; + struct ipv6hdr ip6h_storage; const struct ipv6hdr *ip6h; - char *base; 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, ip6h_storage); 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, o_storage))) { + *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; } - return NULL; + return false; found: if (nh == 59) - return NULL; + return false; *proto = nh; - return base + offset; + return true; } diff --git a/ip.h b/ip.h index 24509d9c11cd..5830b92302e2 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 address, ff02::1 */ static const struct in6_addr in6addr_ll_all_nodes = { diff --git a/packet.c b/packet.c index 34b1722b9a03..014b353cdf8b 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 6a3dd37a0c45..3f090e1bf65d 100644 --- a/tap.c +++ b/tap.c @@ -909,8 +909,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

Use packet_data() 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 --- packet.c | 70 -------------------------------------------------------- packet.h | 11 --------- tap.c | 25 ++++++++++++-------- 3 files changed, 16 insertions(+), 90 deletions(-) diff --git a/packet.c b/packet.c index 014b353cdf8b..5da18bafa576 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_data_do() - Get data range from packet descriptor from given pool * @p: Packet pool diff --git a/packet.h b/packet.h index 062afb978124..dab8274fa5c5 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_data_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_data(p, idx, data) \ packet_data_do(p, idx, data, __func__, __LINE__) diff --git a/tap.c b/tap.c index 30031c6e514e..7cf807149879 100644 --- a/tap.c +++ b/tap.c @@ -894,21 +894,28 @@ resume: for (seq_count = 0, seq = NULL; i < in->count; i++) { size_t l4len, plen, check; struct in6_addr *saddr, *daddr; + struct ipv6hdr ip6h_storage; + struct ethhdr eh_storage; + struct udphdr uh_storage; const struct ethhdr *eh; const struct udphdr *uh; struct iov_tail data; struct ipv6hdr *ip6h; uint8_t proto; - char *l4h; - eh = packet_get(in, i, 0, sizeof(*eh), NULL); + if (!packet_data(in, i, &data)) + return -1; + + eh = IOV_REMOVE_HEADER(&data, eh_storage); if (!eh) continue; - ip6h = packet_get(in, i, sizeof(*eh), sizeof(*ip6h), &check); + ip6h = IOV_PEEK_HEADER(&data, ip6h_storage); if (!ip6h) continue; + check = iov_tail_size(&data) - sizeof(*ip6h); + saddr = &ip6h->saddr; daddr = &ip6h->daddr; @@ -916,10 +923,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]; @@ -944,6 +949,8 @@ resume: } if (proto == IPPROTO_ICMPV6) { + struct icmp6hdr l4h_storage; + const struct icmp6hdr *l4h; PACKET_POOL_P(pkt, 1, in->buf, in->buf_size); if (c->no_icmp) @@ -952,9 +959,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, l4h_storage); if (ndp(c, (struct icmp6hdr *)l4h, saddr, pkt)) continue; @@ -967,12 +974,13 @@ resume: if (l4len < sizeof(*uh)) continue; - uh = (struct udphdr *)l4h; + uh = IOV_PEEK_HEADER(&data, uh_storage); + if (!uh) + continue; 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)) @@ -1029,7 +1037,6 @@ resume: #undef L4_SET append: - data = IOV_TAIL_FROM_BUF(l4h, l4len, 0); packet_add((struct pool *)&seq->p, &data); } -- 2.49.0

The arp() function signature is changed to accept `struct iov_tail *data` directly, replacing the previous `const struct pool *p` parameter. Consequently, arp() no longer fetches packet data internally using packet_data(), streamlining its logic. This simplifies callers like tap4_handler(), which now pass the iov_tail for the L2 ARP frame directly, removing intermediate pool handling. Signed-off-by: Laurent Vivier --- arp.c | 14 +++++--------- arp.h | 2 +- tap.c | 5 +---- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/arp.c b/arp.c index 65f0f2627db6..b255e30b739d 100644 --- a/arp.c +++ b/arp.c @@ -63,11 +63,11 @@ static bool accept_arp(const struct ctx *c, /** * arp() - Check if this is a supported ARP message, reply as needed * @c: Execution context - * @p: Packet pool, single packet with Ethernet buffer + * @data: Single packet with Ethernet buffer * * Return: 1 if handled, -1 on failure */ -int arp(const struct ctx *c, const struct pool *p) +int arp(const struct ctx *c, struct iov_tail *data) { struct { struct ethhdr eh; @@ -80,14 +80,10 @@ 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; - if (!packet_get(p, 0, &data)) - return -1; - - eh = IOV_REMOVE_HEADER(&data, eh_storage); - ah = IOV_REMOVE_HEADER(&data, ah_storage); - am = IOV_REMOVE_HEADER(&data, am_storage); + eh = IOV_REMOVE_HEADER(data, eh_storage); + ah = IOV_REMOVE_HEADER(data, ah_storage); + am = IOV_REMOVE_HEADER(data, am_storage); if (!eh || !ah || !am) return -1; diff --git a/arp.h b/arp.h index ac5cd16e47f4..86bcbf878eda 100644 --- a/arp.h +++ b/arp.h @@ -20,6 +20,6 @@ struct arpmsg { unsigned char tip[4]; } __attribute__((__packed__)); -int arp(const struct ctx *c, const struct pool *p); +int arp(const struct ctx *c, struct iov_tail *data); #endif /* ARP_H */ diff --git a/tap.c b/tap.c index 09da76ea23bc..1daa3009cc65 100644 --- a/tap.c +++ b/tap.c @@ -720,10 +720,7 @@ resume: if (!eh) continue; if (ntohs(eh->h_proto) == ETH_P_ARP) { - PACKET_POOL_P(pkt, 1, in->buf, in->buf_size); - - packet_add(pkt, &data); - arp(c, pkt); + arp(c, &data); continue; } -- 2.49.0

This patch refactors the dhcpv6() function to accept `struct iov_tail *data` directly as its packet input, replacing the `const struct pool *p` parameter. Consequently, dhcpv6() no longer fetches packet data internally using packet_data(). This change simplifies callers, such as tap6_handler(), which now pass the iov_tail representing the L4 UDP segment (DHCPv6 message) directly. This removes the need for intermediate packet pool handling. Signed-off-by: Laurent Vivier --- dhcpv6.c | 29 +++++++++++++---------------- dhcpv6.h | 2 +- tap.c | 6 ++---- 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/dhcpv6.c b/dhcpv6.c index f73aa68a298e..a54db137061c 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -536,19 +536,19 @@ static size_t dhcpv6_client_fqdn_fill(const struct iov_tail *data, /** * dhcpv6() - Check if this is a DHCPv6 message, reply as needed * @c: Execution context - * @p: Packet pool, single packet starting from UDP header + * @data: Single packet starting from UDP header * @saddr: Source IPv6 address of original message * @daddr: Destination IPv6 address of original message * * Return: 0 if it's not a DHCPv6 message, 1 if handled, -1 on failure */ -int dhcpv6(struct ctx *c, const struct pool *p, +int dhcpv6(struct ctx *c, struct iov_tail *data, const struct in6_addr *saddr, const struct in6_addr *daddr) { const struct opt_server_id *server_id = NULL; - struct iov_tail data, opt, client_id_base; const struct opt_hdr *client_id = NULL; struct opt_server_id server_id_storage; + struct iov_tail opt, client_id_base; const struct opt_ia_na *ia = NULL; struct opt_hdr client_id_storage; struct opt_ia_na ia_storage; @@ -559,10 +559,7 @@ int dhcpv6(struct ctx *c, const struct pool *p, const struct udphdr *uh; size_t mlen, n; - if (!packet_get(p, 0, &data)) - return -1; - - uh = IOV_REMOVE_HEADER(&data, uh_storage); + uh = IOV_REMOVE_HEADER(data, uh_storage); if (!uh) return -1; @@ -575,7 +572,7 @@ int dhcpv6(struct ctx *c, const struct pool *p, if (!IN6_IS_ADDR_MULTICAST(daddr)) return -1; - mlen = iov_tail_size(&data); + mlen = iov_tail_size(data); if (mlen + sizeof(*uh) != ntohs(uh->len) || mlen < sizeof(*mh)) return -1; @@ -583,23 +580,23 @@ int dhcpv6(struct ctx *c, const struct pool *p, src = &c->ip6.our_tap_ll; - mh = IOV_REMOVE_HEADER(&data, mh_storage); + mh = IOV_REMOVE_HEADER(data, mh_storage); if (!mh) return -1; - client_id_base = data; + client_id_base = *data; if (dhcpv6_opt(&client_id_base, OPT_CLIENTID)) client_id = IOV_PEEK_HEADER(&client_id_base, client_id_storage); if (!client_id || ntohs(client_id->l) > OPT_VSIZE(client_id)) return -1; - opt = data; + opt = *data; if (dhcpv6_opt(&opt, OPT_SERVERID)) server_id = IOV_PEEK_HEADER(&opt, server_id_storage); if (server_id && ntohs(server_id->hdr.l) != OPT_VSIZE(server_id)) return -1; - opt = data; + opt = *data; if (dhcpv6_opt(&opt, OPT_IA_NA)) ia = IOV_PEEK_HEADER(&opt, ia_storage); if (ia && ntohs(ia->hdr.l) < MIN(OPT_VSIZE(ia_na), OPT_VSIZE(ia_ta))) @@ -617,9 +614,9 @@ int dhcpv6(struct ctx *c, const struct pool *p, if (mh->type == TYPE_CONFIRM && server_id) return -1; - if (dhcpv6_ia_notonlink(&data, &c->ip6.addr)) { + if (dhcpv6_ia_notonlink(data, &c->ip6.addr)) { - dhcpv6_send_ia_notonlink(c, &data, &client_id_base, + dhcpv6_send_ia_notonlink(c, data, &client_id_base, ntohs(client_id->l), mh->xid); return 1; @@ -632,7 +629,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(&data, OPT_IA_TA)) + if (ia || dhcpv6_opt(data, OPT_IA_TA)) return -1; info("DHCPv6: received INFORMATION_REQUEST, sending REPLY"); @@ -665,7 +662,7 @@ int dhcpv6(struct ctx *c, const struct pool *p, 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(&data, c, (char *)&resp, n); + n = dhcpv6_client_fqdn_fill(data, c, (char *)&resp, n); resp.hdr.xid = mh->xid; diff --git a/dhcpv6.h b/dhcpv6.h index 580998862227..c706dfdbb2ac 100644 --- a/dhcpv6.h +++ b/dhcpv6.h @@ -6,7 +6,7 @@ #ifndef DHCPV6_H #define DHCPV6_H -int dhcpv6(struct ctx *c, const struct pool *p, +int dhcpv6(struct ctx *c, struct iov_tail *data, struct in6_addr *saddr, struct in6_addr *daddr); void dhcpv6_init(const struct ctx *c); diff --git a/tap.c b/tap.c index daeba0eb6078..f0dc4d157cea 100644 --- a/tap.c +++ b/tap.c @@ -973,11 +973,9 @@ resume: continue; if (proto == IPPROTO_UDP) { - PACKET_POOL_P(pkt, 1, in->buf, in->buf_size); - - packet_add(pkt, &data); + struct iov_tail uh_data = data; - if (dhcpv6(c, pkt, saddr, daddr)) + if (dhcpv6(c, &uh_data, saddr, daddr)) continue; } -- 2.49.0

The ndp() function signature is changed to accept `struct iov_tail *data` directly, replacing the previous `const struct pool *p` and `const struct icmp6hdr *ih` parameters. This change simplifies callers, like tap6_handler(), which now provide the iov_tail representing the L4 ICMPv6 segment directly to ndp(). Signed-off-by: Laurent Vivier --- ndp.c | 19 +++++++++++-------- ndp.h | 4 ++-- tap.c | 10 +++------- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/ndp.c b/ndp.c index fb2bf1161b66..09d43546b91e 100644 --- a/ndp.c +++ b/ndp.c @@ -337,13 +337,20 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) * @c: Execution context * @ih: ICMPv6 header * @saddr: Source IPv6 address - * @p: Packet pool + * @data: Single packet with ICMPv6 header * * Return: 0 if not handled here, 1 if handled, -1 on failure */ -int ndp(const struct ctx *c, const struct icmp6hdr *ih, - const struct in6_addr *saddr, const struct pool *p) +int ndp(const struct ctx *c, const struct in6_addr *saddr, + struct iov_tail *data) { + struct icmp6hdr ih_storage; + const struct icmp6hdr *ih; + + ih = IOV_PEEK_HEADER(data, ih_storage); + if (!ih) + return -1; + if (ih->icmp6_type < RS || ih->icmp6_type > NA) return 0; @@ -353,12 +360,8 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih, if (ih->icmp6_type == NS) { struct ndp_ns ns_storage; const struct ndp_ns *ns; - struct iov_tail data; - - if (!packet_get(p, 0, &data)) - return -1; - ns = IOV_REMOVE_HEADER(&data, ns_storage); + ns = IOV_REMOVE_HEADER(data, ns_storage); if (!ns) return -1; diff --git a/ndp.h b/ndp.h index 41c2000356ec..b1dd5e82c085 100644 --- a/ndp.h +++ b/ndp.h @@ -8,8 +8,8 @@ struct icmp6hdr; -int ndp(const struct ctx *c, const struct icmp6hdr *ih, - const struct in6_addr *saddr, const struct pool *p); +int ndp(const struct ctx *c, const struct in6_addr *saddr, + struct iov_tail *data); void ndp_timer(const struct ctx *c, const struct timespec *now); #endif /* NDP_H */ diff --git a/tap.c b/tap.c index 683da5b6d431..964e036d8de5 100644 --- a/tap.c +++ b/tap.c @@ -940,9 +940,7 @@ resume: } if (proto == IPPROTO_ICMPV6) { - struct icmp6hdr l4h_storage; - const struct icmp6hdr *l4h; - PACKET_POOL_P(pkt, 1, in->buf, in->buf_size); + struct iov_tail ndp_data; if (c->no_icmp) continue; @@ -950,10 +948,8 @@ resume: if (l4len < sizeof(struct icmp6hdr)) continue; - packet_add(pkt, &data); - - l4h = IOV_PEEK_HEADER(&data, l4h_storage); - if (ndp(c, (struct icmp6hdr *)l4h, saddr, pkt)) + ndp_data = data; + if (ndp(c, saddr, &ndp_data)) continue; tap_packet_debug(NULL, ip6h, NULL, proto, NULL, 1); -- 2.49.0

_buf is not used in the macro. Remove it. Remove it also from PACKET_POOL_NOINIT() as it was needed for PACKET_POOL_DECL(). Signed-off-by: Laurent Vivier --- packet.h | 6 +++--- tap.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packet.h b/packet.h index 286b6b9994db..43b9022075d1 100644 --- a/packet.h +++ b/packet.h @@ -43,7 +43,7 @@ void pool_flush(struct pool *p); #define packet_get(p, idx, data) \ packet_get_do(p, idx, data, __func__, __LINE__) -#define PACKET_POOL_DECL(_name, _size, _buf) \ +#define PACKET_POOL_DECL(_name, _size) \ struct _name ## _t { \ char *buf; \ size_t buf_size; \ @@ -62,7 +62,7 @@ struct _name ## _t { \ #define PACKET_INIT(name, size, buf, buf_size) \ (struct name ## _t) PACKET_POOL_INIT_NOCAST(size, buf, buf_size) -#define PACKET_POOL_NOINIT(name, size, buf) \ - PACKET_POOL_DECL(name, size, buf) name ## _storage; \ +#define PACKET_POOL_NOINIT(name, size) \ + PACKET_POOL_DECL(name, size) name ## _storage; \ static struct pool *name = (struct pool *)&name ## _storage #endif /* PACKET_H */ diff --git a/tap.c b/tap.c index 964e036d8de5..bbcdeeaae54d 100644 --- a/tap.c +++ b/tap.c @@ -95,8 +95,8 @@ CHECK_FRAME_LEN(L2_MAX_LEN_VU); ETH_HLEN + sizeof(struct ipv6hdr) + sizeof(struct udphdr)) /* IPv4 (plus ARP) and IPv6 message batches from tap/guest to IP handlers */ -static PACKET_POOL_NOINIT(pool_tap4, TAP_MSGS_IP4, pkt_buf); -static PACKET_POOL_NOINIT(pool_tap6, TAP_MSGS_IP6, pkt_buf); +static PACKET_POOL_NOINIT(pool_tap4, TAP_MSGS_IP4); +static PACKET_POOL_NOINIT(pool_tap6, TAP_MSGS_IP6); #define TAP_SEQS 128 /* Different L4 tuples in one batch */ #define FRAGMENT_MSG_RATE 10 /* # seconds between fragment warnings */ @@ -553,7 +553,7 @@ void eth_update_mac(struct ethhdr *eh, memcpy(eh->h_source, eth_s, sizeof(eh->h_source)); } -PACKET_POOL_DECL(pool_l4, UIO_MAXIOV, pkt_buf); +PACKET_POOL_DECL(pool_l4, UIO_MAXIOV); /** * struct l4_seq4_t - Message sequence for one protocol handler call, IPv4 -- 2.49.0

In vhost-user mode, where `pool->buf` doesn't store packet data directly, this patch repurposes it to hold the `struct iovec` array describing a scattered packet. This enables pools to manage true scatter-gather descriptors, with iovecs pointing to guest memory. `p->pkt[idx].iov_base` is now an index into this `pool->buf` iovec array, and `p->pkt[idx].iov_len` is the count of iovecs. `packet_add_do` uses `iov_tail_slice` to store these iovec descriptors from an input iov_tail into `pool->buf`. `packet_data_do` reconstructs the iov_tail for a packet by pointing to this stored array of iovecs. Signed-off-by: Laurent Vivier --- packet.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 148 insertions(+), 16 deletions(-) diff --git a/packet.c b/packet.c index fd3b6db57c30..8dbe00af12c6 100644 --- a/packet.c +++ b/packet.c @@ -84,6 +84,122 @@ bool pool_full(const struct pool *p) return p->count >= p->size; } +/** + * packet_iov_max_cnt() - Return the maximum number of iovec entries we can + * store + * @p: Pointer to packet pool + * + * Return: the maximum number of iovec entries we can store in the memory of + * the pool buffer + */ +static size_t packet_iov_max_cnt(const struct pool *p) +{ + return p->buf_size / sizeof(struct iovec); +} + +/** + * packet_iov_idx() - For a given packet index, return the iovec index and + * the number of iovec entry of the packet + * @p: Pointer to packet pool + * @idx: Index of packet descriptor in pool + * @iov_cnt: Pointer to store the number of the iovec entry of the packet + * @func: For tracing: name of calling function + * @line: For tracing: caller line of function call + * + * Return: the iovec index for the given packet index, @iov_cnt is set + * to the number of the iovec entry of the packet + */ +static int packet_iov_idx(const struct pool *p, size_t idx, size_t *iov_cnt, + const char *func, int line) +{ + size_t iov_idx, max = packet_iov_max_cnt(p); + + iov_idx = (size_t)p->pkt[idx].iov_base; + *iov_cnt = p->pkt[idx].iov_len; + + ASSERT_WITH_MSG(iov_idx + *iov_cnt <= max, + "Corrupt iov entry: (%zu, %zu), max: %zu, %s:%i", + iov_idx, *iov_cnt, max, func, line); + + return iov_idx; +} + +/** + * packet_iov_next_idx() - Give the the next available iovec index + * @p: Pointer to packet pool + * @idx: Index of packet descriptor in pool + * @func: For tracing: name of calling function + * @line: For tracing: caller line of function call + * + * Return: the next available iovec index + */ +static size_t packet_iov_next_idx(const struct pool *p, size_t idx, + const char *func, int line) +{ + size_t iov_idx, iov_cnt; + + if (idx == 0) + return 0; + + iov_idx = packet_iov_idx(p, idx - 1, &iov_cnt, func, line); + + return iov_idx + iov_cnt; +} + +/** + * packet_iov_data() - For a given packet index, provide the iovec array + * @p: Pointer to packet pool + * @idx: Index of packet descriptor in pool + * @data: iov_tail to store iovec array and count + * (offset is always set to 0) + * @func: For tracing: name of calling function + * @line: For tracing: caller line of function call + */ +static void packet_iov_data(const struct pool *p, size_t idx, + struct iov_tail *data, + const char *func, int line) +{ + struct iovec *iov = (struct iovec *)p->buf; + size_t iov_idx, iov_cnt; + + iov_idx = packet_iov_idx(p, idx, &iov_cnt, func, line); + + data->iov = &iov[iov_idx]; + data->cnt = iov_cnt; + data->off = 0; +} + +/** + * packet_iov_check_range() - Check if iovec array is valid for a pool + * @p: Pointer to packet pool + * @data: iov_tail that stores the iovec array to check + * @func: For tracing: name of calling function + * @line: For tracing: caller line of function call + * + * Return: 0 if the range is valid, -1 otherwise + */ +static int packet_iov_check_range(const struct pool *p, + const struct iov_tail *data, + const char *func, int line) +{ + size_t offset, i; + + offset = data->off; + for (i = 0; i < data->cnt; i++) { + int ret; + + ret = packet_check_range(p, + (char *)data->iov[i].iov_base + offset, + data->iov[i].iov_len - offset, + func, line); + if (ret) + return ret; + offset = 0; + } + + return 0; +} + /** * packet_add_do() - Add data as packet descriptor to given pool * @p: Existing pool @@ -107,14 +223,32 @@ void packet_add_do(struct pool *p, struct iov_tail *data, if (!iov_tail_prune(data)) return; - ASSERT(data->cnt == 1); /* we don't support iovec */ + if (packet_iov_check_range(p, data, func, line)) + return; + + if (p->memory) { + size_t iov_max_cnt = packet_iov_max_cnt(p); + struct iovec *iov = (struct iovec *)p->buf; + size_t iov_idx; + int iov_cnt; - len = data->iov[0].iov_len - data->off; - start = (char *)data->iov[0].iov_base + data->off; + iov_idx = packet_iov_next_idx(p, idx, func, line); - if (packet_check_range(p, start, len, func, line)) - return; + iov_cnt = iov_tail_clone(&iov[iov_idx], iov_max_cnt - iov_idx, + data); + if (iov_cnt < 0) { + debug("add iov (%zu,%zu) to buf with size %zu, %s:%i", + iov_idx, data->cnt, iov_max_cnt, func, line); + return; + } + len = iov_cnt; + /* NOLINTNEXTLINE(performance-no-int-to-ptr) */ + start = (char *)iov_idx; + } else { + len = data->iov[0].iov_len - data->off; + start = (char *)data->iov[0].iov_base + data->off; + } p->pkt[idx].iov_base = (void *)start; p->pkt[idx].iov_len = len; @@ -136,8 +270,6 @@ bool packet_get_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, "Corrupted pool count: %zu, size: %zu, %s:%i", p->count, p->size, func, line); @@ -148,17 +280,17 @@ bool packet_get_do(const struct pool *p, size_t idx, 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); + if (p->memory) { + packet_iov_data(p, idx, data, func, line); + } else { + data->cnt = 1; + data->off = 0; + data->iov = &p->pkt[idx]; } + ASSERT_WITH_MSG(!packet_iov_check_range(p, data, func, line), + "Corrupt packet pool, %s:%i", func, line); + return true; } -- 2.49.0