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. iov_tail_slice() is a wrapper for iov_slice() using iov_tail Signed-off-by: Laurent Vivier --- iov.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ iov.h | 6 ++++ 2 files changed, 98 insertions(+) diff --git a/iov.c b/iov.c index 8c63b7ea6e31..60ab1f9af973 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 not enough room in the + * destination iov array + */ +/* cppcheck-suppress staticFunction */ +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); + + /* create a new iov array referencing a subset of the source one */ + 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 @@ -247,3 +315,27 @@ void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align) tail->off = tail->off + len; return p; } + +/** + * iov_tail_slice - Make a new iov 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 + * @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 not enough room in the + * destination iov array + */ +/* cppcheck-suppress unusedFunction */ +int iov_tail_slice(struct iovec *dst_iov, size_t dst_iov_cnt, + struct iov_tail *tail, size_t *bytes) +{ + return iov_slice(dst_iov, dst_iov_cnt, &tail->iov[0], tail->cnt, + tail->off, bytes); +} diff --git a/iov.h b/iov.h index 9855bf0c0c32..4bbdbf23f27b 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,8 +75,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); +int iov_tail_slice(struct iovec *dst_iov, size_t dst_iov_cnt, + struct iov_tail *tail, size_t *bytes); /** * 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 | 52 ++++++++++++++++++++++++++++++++++++++-------------- tcp_buf.c | 2 +- 3 files changed, 83 insertions(+), 24 deletions(-) diff --git a/iov.c b/iov.c index 60ab1f9af973..6ae792a1bf8f 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,27 +292,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 4bbdbf23f27b..03c85bd1a465 100644 --- a/iov.h +++ b/iov.h @@ -73,41 +73,65 @@ 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); int iov_tail_slice(struct iovec *dst_iov, size_t dst_iov_cnt, struct iov_tail *tail, size_t *bytes); /** * 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); -- 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 6ae792a1bf8f..c78ec2c8e4ac 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 d630f6df97f1..0cf6983c8a9f 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) { 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: @@ -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 | 41 +++++++++++++++++++++++++++++++++++++++++ packet.h | 5 +++++ 2 files changed, 46 insertions(+) diff --git a/packet.c b/packet.c index 98ded4e27aae..675bccf4d787 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_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: true if @data contains valid data, false otherwise + */ +/* 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, + "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..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 --- arp.c | 12 +++++++++--- packet.c | 1 - 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/arp.c b/arp.c index 9d68d7c3b602..4cb6965e9793 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_data(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 675bccf4d787..b6de869db176 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_data_do(const struct pool *p, size_t idx, struct iov_tail *data, const char *func, int line) -- 2.49.0

Use packet_data() 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 --- icmp.c | 25 ++++++++++++++----------- iov.c | 23 +++++++++++++++++++++++ iov.h | 2 ++ 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/icmp.c b/icmp.c index 7e2b3423a8d1..d27d860b4af4 100644 --- a/icmp.c +++ b/icmp.c @@ -241,25 +241,27 @@ 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_data(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))) + ih = IOV_PEEK_HEADER(&data, ihc); + if (!ih) return 1; - ih = (struct icmphdr *)pkt; - l4len = dlen + sizeof(*ih); - if (ih->type != ICMP_ECHO) return 1; @@ -268,13 +270,12 @@ 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))) + ih = IOV_PEEK_HEADER(&data, ihc); + if (!ih) return 1; - ih = (struct icmp6hdr *)pkt; - l4len = dlen + sizeof(*ih); - if (ih->icmp6_type != ICMPV6_ECHO_REQUEST) return 1; @@ -298,8 +299,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, sl); + 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 c78ec2c8e4ac..147177ab7c0d 100644 --- a/iov.c +++ b/iov.c @@ -210,6 +210,29 @@ 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 + * @msg_namelen: Size of @msg_name + */ +void iov_tail_msghdr(struct msghdr *msh, struct iov_tail *tail, + void *msg_name, socklen_t msg_namelen) +{ + iov_tail_prune(tail); + + ASSERT(tail->off == 0); + + msh->msg_name = msg_name; + msh->msg_namelen = msg_namelen; + 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 8e8c7efe6639..8b73f99b063e 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, socklen_t msg_namelen); 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

Use packet_data() and extract headers using IOV_REMOVE_HEADER() and iov_peek_header_() rather than packet_get(). Signed-off-by: Laurent Vivier --- tcp.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/tcp.c b/tcp.c index 9c6bc529205e..67d656a1cf97 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_data(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_remove_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

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 Reviewed-by: David Gibson --- 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"); -- 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 Reviewed-by: David Gibson --- dhcpv6.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/dhcpv6.c b/dhcpv6.c index 549ddbbea735..a52af1b7a0fe 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_data(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

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..e8408ef04839 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_data(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

Use packet_data() and extract headers using IOV_PEEK_HEADER() rather than packet_get(). Signed-off-by: Laurent Vivier --- tap.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/tap.c b/tap.c index ca25ff9e90d9..aad6d4dc1f61 100644 --- a/tap.c +++ b/tap.c @@ -704,28 +704,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_data(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; @@ -753,8 +759,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) { @@ -765,7 +772,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, @@ -773,15 +779,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_data(in, i, &eh_data); + packet_add(pkt, &eh_data); if (dhcp(c, pkt)) continue; } @@ -832,7 +840,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