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

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 --- ndp.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ndp.c b/ndp.c index ded2081ddd1d..b09a1a133f56 100644 --- a/ndp.c +++ b/ndp.c @@ -351,8 +351,13 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih, if (ih->icmp6_type == NS) { const struct ndp_ns *ns; + struct iov_tail data; + struct ndp_ns nsc; - ns = packet_get(p, 0, 0, sizeof(struct ndp_ns), NULL); + if (!packet_data(p, 0, &data)) + return -1; + + ns = IOV_REMOVE_HEADER(&data, nsc); 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 | 34 +++++++++++++++++++++++----------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/iov.c b/iov.c index 147177ab7c0d..80590acfb2ae 100644 --- a/iov.c +++ b/iov.c @@ -389,7 +389,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 */ int iov_tail_slice(struct iovec *dst_iov, size_t dst_iov_cnt, struct iov_tail *tail, size_t *bytes) { diff --git a/udp.c b/udp.c index 40af7dfc260f..27e31bc5d92d 100644 --- a/udp.c +++ b/udp.c @@ -872,15 +872,20 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, struct iovec m[UIO_MAXIOV]; const struct udphdr *uh; struct udp_flow *uflow; - int i, s, count = 0; + int i, j, s, count = 0; + struct iov_tail data; flow_sidx_t tosidx; in_port_t src, dst; + struct udphdr uhc; uint8_t topif; socklen_t sl; ASSERT(!c->no_udp); - uh = packet_get(p, idx, 0, sizeof(*uh), NULL); + if (!packet_data(p, idx, &data)) + return 1; + + uh = IOV_PEEK_HEADER(&data, uhc); if (!uh) return 1; @@ -917,23 +922,30 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, pif_sockaddr(c, &to_sa, &sl, topif, &toside->eaddr, toside->eport); - for (i = 0; i < (int)p->count - idx; i++) { - struct udphdr *uh_send; - size_t len; + for (i = 0, j = 0; i < (int)p->count - idx && j < UIO_MAXIOV; i++) { + const struct udphdr *uh_send; - uh_send = packet_get(p, idx + i, 0, sizeof(*uh), &len); + if (!packet_data(p, idx + i, &data)) + return p->count - idx; + + uh_send = IOV_REMOVE_HEADER(&data, uhc); if (!uh_send) return p->count - idx; mm[i].msg_hdr.msg_name = &to_sa; mm[i].msg_hdr.msg_namelen = sl; - if (len) { - m[i].iov_base = (char *)(uh_send + 1); - m[i].iov_len = len; + if (data.cnt) { + int cnt; + + cnt = iov_tail_slice(&m[j], UIO_MAXIOV - j, + &data, NULL); + 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 67d656a1cf97..a25fbc7ae182 100644 --- a/tcp.c +++ b/tcp.c @@ -1651,15 +1651,21 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn, for (i = idx, iov_i = 0; i < (int)p->count; i++) { uint32_t seq, seq_offset, ack_seq; const struct tcphdr *th; - char *data; - size_t off; + struct iov_tail data; + struct tcphdr thc; + size_t off, size; + int count; - th = packet_get(p, i, 0, sizeof(*th), &len); + if (!packet_data(p, i, &data)) + return -1; + + th = IOV_PEEK_HEADER(&data, thc); if (!th) return -1; - len += sizeof(*th); + len = iov_tail_size(&data); off = th->doff * 4UL; + if (off < sizeof(*th) || off > len) return -1; @@ -1669,9 +1675,7 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn, } len -= off; - data = packet_get(p, i, off, len, NULL); - if (!data) - continue; + iov_tail_drop(&data, off); seq = ntohl(th->seq); if (SEQ_LT(seq, conn->seq_from_tap) && len <= 1) { @@ -1745,10 +1749,14 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn, continue; } - tcp_iov[iov_i].iov_base = data + seq_offset; - tcp_iov[iov_i].iov_len = len - seq_offset; - seq_from_tap += tcp_iov[iov_i].iov_len; - iov_i++; + iov_tail_drop(&data, seq_offset); + size = len - seq_offset; + count = iov_tail_slice(&tcp_iov[iov_i], UIO_MAXIOV - iov_i, + &data, &size); + 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 be51c278af7a..549ddbbea735 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -355,6 +355,44 @@ err: return ia; } +/** + * dhcpv6_send_ia_notonlink() - Send NotOnLink status + * @c: Execution context + * @ia: Pointer to non-appropriate IA_NA or IA_TA + * @client_id: Client ID message option + * xid: Transaction ID for message exchange + */ +static void dhcpv6_send_ia_notonlink(struct ctx *c, struct opt_hdr *ia, + const struct opt_hdr *client_id, + uint32_t xid) +{ + const struct in6_addr *src = &c->ip6.our_tap_ll; + size_t n; + + info("DHCPv6: received CONFIRM with inappropriate IA," + " sending NotOnLink status in REPLY"); + + ia->l = htons(OPT_VSIZE(ia_na) + sizeof(sc_not_on_link)); + + n = sizeof(struct opt_ia_na); + memcpy(resp_not_on_link.var, ia, n); + memcpy(resp_not_on_link.var + n, &sc_not_on_link, + sizeof(sc_not_on_link)); + + n += sizeof(sc_not_on_link); + memcpy(resp_not_on_link.var + n, client_id, + sizeof(struct opt_hdr) + ntohs(client_id->l)); + + n += sizeof(struct opt_hdr) + ntohs(client_id->l); + + n = offsetof(struct resp_not_on_link_t, var) + n; + + resp_not_on_link.hdr.xid = xid; + + tap_udp6_send(c, src, 547, tap_ip6_daddr(c, src), 546, + xid, &resp_not_on_link, n); +} + /** * dhcpv6_dns_fill() - Fill in DNS Servers and Domain Search list options * @c: Execution context @@ -547,28 +585,8 @@ int dhcpv6(struct ctx *c, const struct pool *p, return -1; if ((bad_ia = dhcpv6_ia_notonlink(p, &c->ip6.addr))) { - info("DHCPv6: received CONFIRM with inappropriate IA," - " sending NotOnLink status in REPLY"); - - bad_ia->l = htons(OPT_VSIZE(ia_na) + - sizeof(sc_not_on_link)); - n = sizeof(struct opt_ia_na); - memcpy(resp_not_on_link.var, bad_ia, n); - - memcpy(resp_not_on_link.var + n, - &sc_not_on_link, sizeof(sc_not_on_link)); - n += sizeof(sc_not_on_link); - - memcpy(resp_not_on_link.var + n, client_id, - sizeof(struct opt_hdr) + ntohs(client_id->l)); - n += sizeof(struct opt_hdr) + ntohs(client_id->l); - - n = offsetof(struct resp_not_on_link_t, var) + n; - - resp_not_on_link.hdr.xid = mh->xid; - tap_udp6_send(c, src, 547, tap_ip6_daddr(c, src), 546, - mh->xid, &resp_not_on_link, n); + dhcpv6_send_ia_notonlink(c, bad_ia, client_id, mh->xid); return 1; } -- 2.49.0

Signed-off-by: Laurent Vivier --- dhcpv6.c | 179 ++++++++++++++++++++++++++++++++----------------------- iov.c | 1 - 2 files changed, 103 insertions(+), 77 deletions(-) diff --git a/dhcpv6.c b/dhcpv6.c index a52af1b7a0fe..365b609addda 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -278,112 +278,122 @@ static struct resp_not_on_link_t { /** * dhcpv6_opt() - Get option from DHCPv6 message - * @p: Packet pool, single packet with UDP header - * @offset: Offset to look at, 0: end of header, set to option start + * @data: Data to look at (input/output) * @type: Option type to look up, network order * - * Return: pointer to option header, or NULL on malformed or missing option + * Return: true if found and @data points to the option header, + * or false on malformed or missing option and @data is + * unmodified. */ -static struct opt_hdr *dhcpv6_opt(const struct pool *p, size_t *offset, - uint16_t type) +static bool dhcpv6_opt(struct iov_tail *data, uint16_t type) { - struct opt_hdr *o; - size_t left; + struct iov_tail head = *data; + const struct opt_hdr *o; + struct opt_hdr oc; - ASSERT(*offset >= UDP_MSG_HDR_SIZE); - - while ((o = packet_get_try(p, 0, *offset, sizeof(*o), &left))) { + while ((o = IOV_PEEK_HEADER(data, oc))) { unsigned int opt_len = ntohs(o->l) + sizeof(*o); - if (ntohs(o->l) > left) - return NULL; + if (opt_len > iov_tail_size(data)) + break; if (o->t == type) - return o; + return true; - *offset += opt_len; + iov_tail_drop(data, opt_len); } - return NULL; + *data = head; + return false; } /** * dhcpv6_ia_notonlink() - Check if any IA contains non-appropriate addresses - * @p: Packet pool, single packet starting from UDP header + * @data: Data to look at, packet starting from UDP header (input/output) * @la: Address we want to lease to the client * - * Return: pointer to non-appropriate IA_NA or IA_TA, if any, NULL otherwise + * Return: true and @data points to non-appropriate IA_NA or IA_TA, if any, + * false otherwise and @data is unmodified */ -static struct opt_hdr *dhcpv6_ia_notonlink(const struct pool *p, - struct in6_addr *la) +static bool dhcpv6_ia_notonlink(struct iov_tail *data, + struct in6_addr *la) { int ia_types[2] = { OPT_IA_NA, OPT_IA_TA }, *ia_type; const struct opt_ia_addr *opt_addr; + struct iov_tail current, ia_base; + struct opt_ia_addr opt_addrc; char buf[INET6_ADDRSTRLEN]; + const struct opt_ia_na *ia; struct in6_addr req_addr; const struct opt_hdr *h; - struct opt_hdr *ia; - size_t offset; + struct opt_ia_na iac; + struct opt_hdr hc; foreach(ia_type, ia_types) { - offset = UDP_MSG_HDR_SIZE; - while ((ia = dhcpv6_opt(p, &offset, *ia_type))) { - if (ntohs(ia->l) < OPT_VSIZE(ia_na)) - return NULL; - - offset += sizeof(struct opt_ia_na); - - while ((h = dhcpv6_opt(p, &offset, OPT_IAAADR))) { - if (ntohs(h->l) != OPT_VSIZE(ia_addr)) - return NULL; - - opt_addr = (const struct opt_ia_addr *)h; + current = *data; + while (dhcpv6_opt(¤t, *ia_type)) { + ia_base = current; + ia = IOV_REMOVE_HEADER(¤t, iac); + if (!ia || ntohs(ia->hdr.l) < OPT_VSIZE(ia_na)) + goto notfound; + + while (dhcpv6_opt(¤t, OPT_IAAADR)) { + h = IOV_PEEK_HEADER(¤t, hc); + if (!h || ntohs(h->l) != OPT_VSIZE(ia_addr)) + goto notfound; + + opt_addr = IOV_REMOVE_HEADER(¤t, + opt_addrc); req_addr = opt_addr->addr; if (!IN6_ARE_ADDR_EQUAL(la, &req_addr)) - goto err; - - offset += sizeof(struct opt_ia_addr); + goto notonlink; } } } - return NULL; +notfound: + return false; -err: +notonlink: info("DHCPv6: requested address %s not on link", inet_ntop(AF_INET6, &req_addr, buf, sizeof(buf))); - return ia; + *data = ia_base; + return true; } /** * dhcpv6_send_ia_notonlink() - Send NotOnLink status - * @c: Execution context - * @ia: Pointer to non-appropriate IA_NA or IA_TA - * @client_id: Client ID message option - * xid: Transaction ID for message exchange + * @c: Execution context + * @ia_base: Non-appropriate IA_NA or IA_TA base + * @client_id_base: Client ID message option base + * @len: Client ID length + * @xid: Transaction ID for message exchange */ -static void dhcpv6_send_ia_notonlink(struct ctx *c, struct opt_hdr *ia, - const struct opt_hdr *client_id, - uint32_t xid) +static void dhcpv6_send_ia_notonlink(struct ctx *c, + const struct iov_tail *ia_base, + const struct iov_tail *client_id_base, + int len, uint32_t xid) { const struct in6_addr *src = &c->ip6.our_tap_ll; + struct opt_hdr *ia = (struct opt_hdr *)resp_not_on_link.var; size_t n; info("DHCPv6: received CONFIRM with inappropriate IA," " sending NotOnLink status in REPLY"); - ia->l = htons(OPT_VSIZE(ia_na) + sizeof(sc_not_on_link)); - n = sizeof(struct opt_ia_na); - memcpy(resp_not_on_link.var, ia, n); + iov_to_buf(&ia_base->iov[0], ia_base->cnt, ia_base->off, + resp_not_on_link.var, n); + ia->l = htons(OPT_VSIZE(ia_na) + sizeof(sc_not_on_link)); memcpy(resp_not_on_link.var + n, &sc_not_on_link, sizeof(sc_not_on_link)); n += sizeof(sc_not_on_link); - memcpy(resp_not_on_link.var + n, client_id, - sizeof(struct opt_hdr) + ntohs(client_id->l)); + iov_to_buf(&client_id_base->iov[0], client_id_base->cnt, + client_id_base->off, resp_not_on_link.var + n, + sizeof(struct opt_hdr) + len); - n += sizeof(struct opt_hdr) + ntohs(client_id->l); + n += sizeof(struct opt_hdr) + len; n = offsetof(struct resp_not_on_link_t, var) + n; @@ -472,17 +482,19 @@ search: /** * dhcpv6_client_fqdn_fill() - Fill in client FQDN option + * @data: Data to look at * @c: Execution context * @buf: Response message buffer where options will be appended * @offset: Offset in message buffer for new options * * Return: updated length of response message buffer. */ -static size_t dhcpv6_client_fqdn_fill(const struct pool *p, const struct ctx *c, +static size_t dhcpv6_client_fqdn_fill(const struct iov_tail *data, + const struct ctx *c, char *buf, int offset) { - struct opt_client_fqdn const *req_opt; + struct iov_tail current = *data; struct opt_client_fqdn *o; size_t opt_len; @@ -501,13 +513,16 @@ static size_t dhcpv6_client_fqdn_fill(const struct pool *p, const struct ctx *c, o = (struct opt_client_fqdn *)(buf + offset); encode_domain_name(o->domain_name, c->fqdn); - req_opt = (struct opt_client_fqdn *)dhcpv6_opt(p, - &(size_t){ UDP_MSG_HDR_SIZE }, - OPT_CLIENT_FQDN); - if (req_opt && req_opt->flags & 0x01 /* S flag */) - o->flags = 0x02 /* O flag */; - else - o->flags = 0x00; + if (dhcpv6_opt(¤t, OPT_CLIENT_FQDN)) { + struct opt_client_fqdn const *req_opt; + struct opt_client_fqdn req_optc; + + req_opt = IOV_PEEK_HEADER(¤t, req_optc); + if (req_opt && req_opt->flags & 0x01 /* S flag */) + o->flags = 0x02 /* O flag */; + else + o->flags = 0x00; + } opt_len++; @@ -529,12 +544,16 @@ static size_t dhcpv6_client_fqdn_fill(const struct pool *p, const struct ctx *c, int dhcpv6(struct ctx *c, const struct pool *p, const struct in6_addr *saddr, const struct in6_addr *daddr) { - const struct opt_hdr *client_id, *server_id, *ia; + const struct opt_server_id *server_id = NULL; + struct iov_tail data, opt, client_id_base; + const struct opt_hdr *client_id = NULL; + const struct opt_ia_na *ia = NULL; + struct opt_server_id server_idc; + struct opt_hdr client_idc; const struct in6_addr *src; const struct msg_hdr *mh; const struct udphdr *uh; - struct opt_hdr *bad_ia; - struct iov_tail data; + struct opt_ia_na iac; struct msg_hdr mhc; struct udphdr uhc; size_t mlen, n; @@ -563,20 +582,26 @@ int dhcpv6(struct ctx *c, const struct pool *p, src = &c->ip6.our_tap_ll; - mh = IOV_PEEK_HEADER(&data, mhc); + mh = IOV_REMOVE_HEADER(&data, mhc); if (!mh) return -1; - client_id = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_CLIENTID); + client_id_base = data; + if (dhcpv6_opt(&client_id_base, OPT_CLIENTID)) + client_id = IOV_PEEK_HEADER(&client_id_base, client_idc); if (!client_id || ntohs(client_id->l) > OPT_VSIZE(client_id)) return -1; - server_id = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_SERVERID); - if (server_id && ntohs(server_id->l) != OPT_VSIZE(server_id)) + opt = data; + if (dhcpv6_opt(&opt, OPT_SERVERID)) + server_id = IOV_PEEK_HEADER(&opt, server_idc); + if (server_id && ntohs(server_id->hdr.l) != OPT_VSIZE(server_id)) return -1; - ia = dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_IA_NA); - if (ia && ntohs(ia->l) < MIN(OPT_VSIZE(ia_na), OPT_VSIZE(ia_ta))) + opt = data; + if (dhcpv6_opt(&opt, OPT_IA_NA)) + ia = IOV_PEEK_HEADER(&opt, iac); + if (ia && ntohs(ia->hdr.l) < MIN(OPT_VSIZE(ia_na), OPT_VSIZE(ia_ta))) return -1; resp.hdr.type = TYPE_REPLY; @@ -591,9 +616,10 @@ int dhcpv6(struct ctx *c, const struct pool *p, if (mh->type == TYPE_CONFIRM && server_id) return -1; - if ((bad_ia = dhcpv6_ia_notonlink(p, &c->ip6.addr))) { + if (dhcpv6_ia_notonlink(&data, &c->ip6.addr)) { - dhcpv6_send_ia_notonlink(c, bad_ia, client_id, mh->xid); + dhcpv6_send_ia_notonlink(c, &data, &client_id_base, + ntohs(client_id->l), mh->xid); return 1; } @@ -605,7 +631,7 @@ int dhcpv6(struct ctx *c, const struct pool *p, memcmp(&resp.server_id, server_id, sizeof(resp.server_id))) return -1; - if (ia || dhcpv6_opt(p, &(size_t){ UDP_MSG_HDR_SIZE }, OPT_IA_TA)) + if (ia || dhcpv6_opt(&data, OPT_IA_TA)) return -1; info("DHCPv6: received INFORMATION_REQUEST, sending REPLY"); @@ -631,13 +657,14 @@ int dhcpv6(struct ctx *c, const struct pool *p, if (ia) resp.ia_na.iaid = ((struct opt_ia_na *)ia)->iaid; - memcpy(&resp.client_id, client_id, - ntohs(client_id->l) + sizeof(struct opt_hdr)); + iov_to_buf(&client_id_base.iov[0], client_id_base.cnt, + client_id_base.off, &resp.client_id, + ntohs(client_id->l) + sizeof(struct opt_hdr)); n = offsetof(struct resp_t, client_id) + sizeof(struct opt_hdr) + ntohs(client_id->l); n = dhcpv6_dns_fill(c, (char *)&resp, n); - n = dhcpv6_client_fqdn_fill(p, c, (char *)&resp, n); + n = dhcpv6_client_fqdn_fill(&data, c, (char *)&resp, n); resp.hdr.xid = mh->xid; diff --git a/iov.c b/iov.c index 80590acfb2ae..f25ece225a4a 100644 --- a/iov.c +++ b/iov.c @@ -108,7 +108,6 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt, * * Returns: The number of bytes successfully copied. */ -/* cppcheck-suppress [staticFunction] */ size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt, size_t offset, void *buf, size_t bytes) { -- 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 | 27 ++++++++++++--------------- ip.h | 3 +-- packet.c | 1 + tap.c | 4 +++- 4 files changed, 17 insertions(+), 18 deletions(-) diff --git a/ip.c b/ip.c index 2cc7f6548aff..b63fbe79fb5f 100644 --- a/ip.c +++ b/ip.c @@ -23,40 +23,37 @@ /** * ipv6_l4hdr() - Find pointer to L4 header in IPv6 packet and extract protocol - * @p: Packet pool, packet number @idx has IPv6 header at @offset - * @idx: Index of packet in pool - * @offset: Pre-calculated IPv6 header offset + * @data: IPv6 packet * @proto: Filled with L4 protocol number * @dlen: Data length (payload excluding header extensions), set on return * * Return: pointer to L4 header, NULL if not found */ -char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto, - size_t *dlen) +bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen) { const struct ipv6_opt_hdr *o; + struct ipv6_opt_hdr oc; const struct ipv6hdr *ip6h; - char *base; + struct ipv6hdr ip6hc; int hdrlen; uint8_t nh; - base = packet_get(p, idx, 0, 0, NULL); - ip6h = packet_get(p, idx, offset, sizeof(*ip6h), dlen); + ip6h = IOV_REMOVE_HEADER(data, ip6hc); if (!ip6h) - return NULL; - - offset += sizeof(*ip6h); + return false; + *dlen = iov_tail_size(data); nh = ip6h->nexthdr; if (!IPV6_NH_OPT(nh)) goto found; - while ((o = packet_get_try(p, idx, offset, sizeof(*o), dlen))) { + while ((o = IOV_PEEK_HEADER(data, oc))) { + *dlen = iov_tail_size(data) - sizeof(*o); nh = o->nexthdr; hdrlen = (o->hdrlen + 1) * 8; if (IPV6_NH_OPT(nh)) - offset += hdrlen; + iov_tail_drop(data, hdrlen); else goto found; } @@ -65,8 +62,8 @@ char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto, found: if (nh == 59) - return NULL; + return false; *proto = nh; - return base + offset; + return true; } diff --git a/ip.h b/ip.h index 471c57ee4c71..874cbe476a6b 100644 --- a/ip.h +++ b/ip.h @@ -115,8 +115,7 @@ static inline uint32_t ip6_get_flow_lbl(const struct ipv6hdr *ip6h) ip6h->flow_lbl[2]; } -char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto, - size_t *dlen); +bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen); /* IPv6 link-local all-nodes multicast adddress, ff02::1 */ static const struct in6_addr in6addr_ll_all_nodes = { diff --git a/packet.c b/packet.c index b6de869db176..7489010e02eb 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 162f19d68115..ca25ff9e90d9 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 | 23 +++++++++++-------- 3 files changed, 14 insertions(+), 90 deletions(-) diff --git a/packet.c b/packet.c index 7489010e02eb..53ede9fcb509 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 aad6d4dc1f61..56b8cb21d9dd 100644 --- a/tap.c +++ b/tap.c @@ -898,17 +898,24 @@ resume: const struct udphdr *uh; struct iov_tail data; struct ipv6hdr *ip6h; + struct ipv6hdr ip6hc; + struct ethhdr ehc; + struct udphdr uhc; uint8_t proto; - char *l4h; - eh = packet_get(in, i, 0, sizeof(*eh), NULL); + if (!packet_data(in, i, &data)) + return -1; + + eh = IOV_REMOVE_HEADER(&data, ehc); if (!eh) continue; - ip6h = packet_get(in, i, sizeof(*eh), sizeof(*ip6h), &check); + ip6h = IOV_PEEK_HEADER(&data, ip6hc); if (!ip6h) continue; + check = iov_tail_size(&data) - sizeof(*ip6h); + saddr = &ip6h->saddr; daddr = &ip6h->daddr; @@ -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) { + const struct icmp6hdr *l4h; + struct icmp6hdr l4hc; 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, l4hc); if (ndp(c, (struct icmp6hdr *)l4h, saddr, pkt)) continue; @@ -967,12 +974,11 @@ resume: if (l4len < sizeof(*uh)) continue; - uh = (struct udphdr *)l4h; + uh = IOV_PEEK_HEADER(&data, uhc); if (proto == IPPROTO_UDP) { PACKET_POOL_P(pkt, 1, in->buf, in->buf_size); - data = IOV_TAIL_FROM_BUF(l4h, l4len, 0); packet_add(pkt, &data); if (dhcpv6(c, pkt, saddr, daddr)) @@ -1029,7 +1035,6 @@ resume: #undef L4_SET append: - data = IOV_TAIL_FROM_BUF(l4h, l4len, 0); packet_add((struct pool *)&seq->p, &data); } -- 2.49.0

Signed-off-by: Laurent Vivier --- dhcp.c | 32 ++++++++++++++------------------ dhcp.h | 2 +- tap.c | 5 +---- 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/dhcp.c b/dhcp.c index e8408ef04839..e7ea7723ebd3 100644 --- a/dhcp.c +++ b/dhcp.c @@ -296,11 +296,11 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len) /** * dhcp() - Check if this is a DHCP message, reply as needed * @c: Execution context - * @p: Packet pool, single packet with Ethernet buffer + * @data: Single packet with Ethernet buffer * * Return: 0 if it's not a DHCP message, 1 if handled, -1 on failure */ -int dhcp(const struct ctx *c, const struct pool *p) +int dhcp(const struct ctx *c, struct iov_tail *data) { size_t mlen, dlen, opt_len; char macstr[ETH_ADDRSTRLEN]; @@ -308,7 +308,6 @@ int dhcp(const struct ctx *c, const struct pool *p) 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; @@ -317,18 +316,15 @@ int dhcp(const struct ctx *c, const struct pool *p) struct msg reply; unsigned int i; - if (!packet_data(p, 0, &data)) - return -1; - - eh = IOV_REMOVE_HEADER(&data, ehc); - iph = IOV_PEEK_HEADER(&data, iphc); + eh = IOV_REMOVE_HEADER(data, ehc); + iph = IOV_PEEK_HEADER(data, iphc); if (!eh || !iph) return -1; - if (!iov_tail_drop(&data, iph->ihl * 4UL)) + if (!iov_tail_drop(data, iph->ihl * 4UL)) return -1; - uh = IOV_REMOVE_HEADER(&data, uhc); + uh = IOV_REMOVE_HEADER(data, uhc); if (!uh) return -1; @@ -338,8 +334,8 @@ int dhcp(const struct ctx *c, const struct pool *p) if (c->no_dhcp) return 1; - mlen = iov_tail_size(&data); - m = (struct msg const *)iov_remove_header_(&data, &mc, offsetof(struct msg, o), + 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) || @@ -366,24 +362,24 @@ int dhcp(const struct ctx *c, const struct pool *p) for (i = 0; i < ARRAY_SIZE(opts); i++) opts[i].clen = -1; - opt_len = iov_tail_size(&data); + opt_len = iov_tail_size(data); while (opt_len >= 2) { uint8_t olenc, typec; const uint8_t *olen; uint8_t *type; - type = IOV_REMOVE_HEADER(&data, typec); - olen = IOV_REMOVE_HEADER(&data, olenc); + type = IOV_REMOVE_HEADER(data, typec); + olen = IOV_REMOVE_HEADER(data, olenc); if (!type || !olen) return -1; - opt_len = iov_tail_size(&data); + opt_len = iov_tail_size(data); if (opt_len < *olen) return -1; - iov_to_buf(&data.iov[0], data.cnt, data.off, &opts[*type].c, *olen); + iov_to_buf(&data->iov[0], data->cnt, data->off, &opts[*type].c, *olen); opts[*type].clen = *olen; - iov_tail_drop(&data, *olen); + iov_tail_drop(data, *olen); opt_len -= *olen; } diff --git a/dhcp.h b/dhcp.h index 87aeecd8dec8..cd50c99b8856 100644 --- a/dhcp.h +++ b/dhcp.h @@ -6,7 +6,7 @@ #ifndef DHCP_H #define DHCP_H -int dhcp(const struct ctx *c, const struct pool *p); +int dhcp(const struct ctx *c, struct iov_tail *data); void dhcp_init(void); #endif /* DHCP_H */ diff --git a/tap.c b/tap.c index b1c817451031..846d4b35dda7 100644 --- a/tap.c +++ b/tap.c @@ -783,11 +783,8 @@ resume: if (iph->protocol == IPPROTO_UDP) { struct iov_tail eh_data; - PACKET_POOL_P(pkt, 1, in->buf, in->buf_size); - packet_data(in, i, &eh_data); - packet_add(pkt, &eh_data); - if (dhcp(c, pkt)) + if (dhcp(c, &eh_data)) continue; } -- 2.49.0

As the iov_tail has a non zero offset (because of the presence of packet headers in the iov array), we must copy it to a new iov array (using iov_splice()) to pass it to sendmsg(). We cannot use anymore iov_tail_msghdr(), so remove it. Signed-off-by: Laurent Vivier --- icmp.c | 30 +++++++++++++++++++----------- icmp.h | 2 +- iov.c | 23 ----------------------- iov.h | 2 -- tap.c | 7 ++----- 5 files changed, 22 insertions(+), 42 deletions(-) diff --git a/icmp.c b/icmp.c index d27d860b4af4..44d431d53c96 100644 --- a/icmp.c +++ b/icmp.c @@ -44,6 +44,7 @@ #define ICMP_ECHO_TIMEOUT 60 /* s, timeout for ICMP socket activity */ #define ICMP_NUM_IDS (1U << 16) +#define MAX_IOV_ICMP 16 /** * ping_at_sidx() - Get ping specific flow at given sidx @@ -229,36 +230,33 @@ cancel: * @af: Address family, AF_INET or AF_INET6 * @saddr: Source address * @daddr: Destination address - * @p: Packet pool, single packet with ICMP/ICMPv6 header + * @data: Single packet with ICMP/ICMPv6 header * @now: Current timestamp * * Return: count of consumed packets (always 1, even if malformed) */ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, const void *saddr, const void *daddr, - const struct pool *p, const struct timespec *now) + struct iov_tail *data, const struct timespec *now) { + struct iovec iov[MAX_IOV_ICMP]; struct icmp_ping_flow *pingf; const struct flowside *tgt; union sockaddr_inany sa; - struct iov_tail data; struct msghdr msh; uint16_t id, seq; union flow *flow; uint8_t proto; - socklen_t sl; + int cnt; (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; - ih = IOV_PEEK_HEADER(&data, ihc); + ih = IOV_PEEK_HEADER(data, ihc); if (!ih) return 1; @@ -272,7 +270,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, const struct icmp6hdr *ih; struct icmp6hdr ihc; - ih = IOV_PEEK_HEADER(&data, ihc); + ih = IOV_PEEK_HEADER(data, ihc); if (!ih) return 1; @@ -286,6 +284,10 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, ASSERT(0); } + cnt = iov_tail_slice(&iov[0], MAX_IOV_ICMP, data, NULL); + if (cnt < 0) + return 1; + flow = flow_at_sidx(flow_lookup_af(c, proto, PIF_TAP, af, saddr, daddr, id, id)); @@ -300,8 +302,14 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, pingf->ts = now->tv_sec; - pif_sockaddr(c, &sa, &sl, PIF_HOST, &tgt->eaddr, 0); - iov_tail_msghdr(&msh, &data, &sa, sl); + pif_sockaddr(c, &sa, &msh.msg_namelen, PIF_HOST, &tgt->eaddr, 0); + msh.msg_name = &sa; + msh.msg_iov = iov; + msh.msg_iovlen = cnt; + msh.msg_control = NULL; + msh.msg_controllen = 0; + msh.msg_flags = 0; + if (sendmsg(pingf->sock, &msh, MSG_NOSIGNAL) < 0) { flow_dbg_perror(pingf, "failed to relay request to socket"); } else { diff --git a/icmp.h b/icmp.h index 5ce22b5eca1f..d1cecb20e29d 100644 --- a/icmp.h +++ b/icmp.h @@ -14,7 +14,7 @@ struct icmp_ping_flow; void icmp_sock_handler(const struct ctx *c, union epoll_ref ref); int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, const void *saddr, const void *daddr, - const struct pool *p, const struct timespec *now); + struct iov_tail *data, const struct timespec *now); void icmp_init(void); /** diff --git a/iov.c b/iov.c index f25ece225a4a..303351850e50 100644 --- a/iov.c +++ b/iov.c @@ -209,29 +209,6 @@ 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 8b73f99b063e..8e8c7efe6639 100644 --- a/iov.h +++ b/iov.h @@ -83,8 +83,6 @@ 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); diff --git a/tap.c b/tap.c index 5ab49935d318..d70e387fa7cc 100644 --- a/tap.c +++ b/tap.c @@ -762,17 +762,14 @@ resume: continue; if (iph->protocol == IPPROTO_ICMP) { - PACKET_POOL_P(pkt, 1, in->buf, in->buf_size); - if (c->no_icmp) continue; tap_packet_debug(iph, NULL, NULL, 0, NULL, 1); - packet_add(pkt, &data); icmp_tap_handler(c, PIF_TAP, AF_INET, &iph->saddr, &iph->daddr, - pkt, now); + &data, now); continue; } @@ -962,7 +959,7 @@ resume: tap_packet_debug(NULL, ip6h, NULL, proto, NULL, 1); icmp_tap_handler(c, PIF_TAP, AF_INET6, - saddr, daddr, pkt, now); + saddr, daddr, &data, now); continue; } -- 2.49.0

Signed-off-by: Laurent Vivier --- packet.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packet.h b/packet.h index dab8274fa5c5..ce910699b4d7 100644 --- a/packet.h +++ b/packet.h @@ -60,19 +60,10 @@ struct _name ## _t { \ .size = _size, \ } -#define PACKET_POOL(name, size, buf, buf_size) \ - PACKET_POOL_DECL(name, size, buf) name = \ - PACKET_POOL_INIT_NOCAST(size, buf, buf_size) - #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; \ static struct pool *name = (struct pool *)&name ## _storage - -#define PACKET_POOL_P(name, size, buf, buf_size) \ - PACKET_POOL(name ## _storage, size, buf, buf_size); \ - struct pool *name = (struct pool *)&name ## _storage - #endif /* PACKET_H */ -- 2.49.0

Do not hack anymore the buf pointer to detect and manage memory regions Signed-off-by: Laurent Vivier --- packet.c | 4 ++-- packet.h | 16 ++++++++++++---- tap.c | 23 +++++++++++++++-------- tap.h | 1 - vhost_user.c | 28 +++++++++++----------------- virtio.c | 4 ++-- virtio.h | 8 ++++++-- vu_common.c | 20 +++++++++++--------- 8 files changed, 59 insertions(+), 45 deletions(-) diff --git a/packet.c b/packet.c index 53ede9fcb509..0a2ad28d3d25 100644 --- a/packet.c +++ b/packet.c @@ -41,10 +41,10 @@ static int packet_check_range(const struct pool *p, const char *ptr, size_t len, return -1; } - if (p->buf_size == 0) { + if (p->memory) { int ret; - ret = vu_packet_check_range((void *)p->buf, ptr, len); + ret = vu_packet_check_range(p->memory, ptr, len); if (ret == -1) debug("cannot find region, %s:%i", func, line); diff --git a/packet.h b/packet.h index bbea04eac455..d553521a30c5 100644 --- a/packet.h +++ b/packet.h @@ -9,6 +9,8 @@ #include #include "iov.h" +struct vdev_memory; + /* Maximum size of a single packet stored in pool, including headers */ #define PACKET_MAX_LEN ((size_t)UINT16_MAX) @@ -20,6 +22,7 @@ * 0 for passt vhost-user mode * @size: Number of usable descriptors for the pool * @count: Number of used descriptors for the pool + * @memory: Memory regions defined for vhost-user, NULL otherwise * @pkt: Descriptors: see macros below */ struct pool { @@ -27,10 +30,12 @@ struct pool { size_t buf_size; size_t size; size_t count; + struct vdev_memory *memory; struct iovec pkt[]; }; -int vu_packet_check_range(void *buf, const char *ptr, size_t len); +int vu_packet_check_range(struct vdev_memory *memory, + const char *ptr, size_t len); void packet_add_do(struct pool *p, struct iov_tail *data, const char *func, int line); bool packet_data_do(const struct pool *p, const size_t idx, @@ -50,18 +55,21 @@ struct _name ## _t { \ size_t buf_size; \ size_t size; \ size_t count; \ + struct vdev_memory *memory; \ struct iovec pkt[_size]; \ } -#define PACKET_POOL_INIT_NOCAST(_size, _buf, _buf_size) \ +#define PACKET_POOL_INIT_NOCAST(_size, _buf, _buf_size, _memory) \ { \ .buf_size = _buf_size, \ .buf = _buf, \ .size = _size, \ + .memory = _memory \ } -#define PACKET_INIT(name, size, buf, buf_size) \ - (struct name ## _t) PACKET_POOL_INIT_NOCAST(size, buf, buf_size) +#define PACKET_INIT(name, size, buf, buf_size, memory) \ + (struct name ## _t) \ + PACKET_POOL_INIT_NOCAST(size, buf, buf_size, memory) #define PACKET_POOL_NOINIT(name, size) \ PACKET_POOL_DECL(name, size) name ## _storage; \ diff --git a/tap.c b/tap.c index 615609d4ded2..419b09b4cee7 100644 --- a/tap.c +++ b/tap.c @@ -1453,17 +1453,23 @@ static void tap_sock_tun_init(struct ctx *c) * tap_sock_update_pool() - Set the buffer base and size for the pool of packets * @base: Buffer base * @size Buffer size + * @memory: vhost-user memory regions */ -void tap_sock_update_pool(void *base, size_t size) +static void tap_sock_update_pool(void *base, size_t size, + struct vdev_memory *memory) { int i; - pool_tap4_storage = PACKET_INIT(pool_tap4, TAP_MSGS_IP4, base, size); - pool_tap6_storage = PACKET_INIT(pool_tap6, TAP_MSGS_IP6, base, size); + pool_tap4_storage = PACKET_INIT(pool_tap4, TAP_MSGS_IP4, base, size, + memory); + pool_tap6_storage = PACKET_INIT(pool_tap6, TAP_MSGS_IP6, base, size, + memory); for (i = 0; i < TAP_SEQS; i++) { - tap4_l4[i].p = PACKET_INIT(pool_l4, UIO_MAXIOV, base, size); - tap6_l4[i].p = PACKET_INIT(pool_l4, UIO_MAXIOV, base, size); + tap4_l4[i].p = PACKET_INIT(pool_l4, UIO_MAXIOV, base, size, + memory); + tap6_l4[i].p = PACKET_INIT(pool_l4, UIO_MAXIOV, base, size, + memory); } } @@ -1474,12 +1480,13 @@ void tap_sock_update_pool(void *base, size_t size) */ void tap_backend_init(struct ctx *c) { + struct vdev_memory *memory = NULL; + if (c->mode == MODE_VU) { - tap_sock_update_pool(NULL, 0); vu_init(c); - } else { - tap_sock_update_pool(pkt_buf, sizeof(pkt_buf)); + memory = &c->vdev->memory; } + tap_sock_update_pool(pkt_buf, sizeof(pkt_buf), memory); if (c->fd_tap != -1) { /* Passed as --fd */ ASSERT(c->one_off); diff --git a/tap.h b/tap.h index 095190eea44e..38f560b678f0 100644 --- a/tap.h +++ b/tap.h @@ -116,7 +116,6 @@ void tap_handler_passt(struct ctx *c, uint32_t events, int tap_sock_unix_open(char *sock_path); void tap_sock_reset(struct ctx *c); void tap_sock_set_mr(struct vhost_user_memory *mr, int nb); -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); diff --git a/vhost_user.c b/vhost_user.c index 105f77a5df2b..e53ea21909e3 100644 --- a/vhost_user.c +++ b/vhost_user.c @@ -137,8 +137,8 @@ static void *qva_to_va(struct vu_dev *dev, uint64_t qemu_addr) unsigned int i; /* Find matching memory region. */ - for (i = 0; i < dev->nregions; i++) { - const struct vu_dev_region *r = &dev->regions[i]; + for (i = 0; i < dev->memory.nregions; i++) { + const struct vu_dev_region *r = &dev->memory.regions[i]; if ((qemu_addr >= r->qva) && (qemu_addr < (r->qva + r->size))) { /* NOLINTNEXTLINE(performance-no-int-to-ptr) */ @@ -428,8 +428,8 @@ static bool vu_set_mem_table_exec(struct vu_dev *vdev, struct vhost_user_memory m = msg->payload.memory, *memory = &m; unsigned int i; - for (i = 0; i < vdev->nregions; i++) { - const struct vu_dev_region *r = &vdev->regions[i]; + for (i = 0; i < vdev->memory.nregions; i++) { + const struct vu_dev_region *r = &vdev->memory.regions[i]; if (r->mmap_addr) { /* NOLINTNEXTLINE(performance-no-int-to-ptr) */ @@ -437,12 +437,12 @@ static bool vu_set_mem_table_exec(struct vu_dev *vdev, r->size + r->mmap_offset); } } - vdev->nregions = memory->nregions; + vdev->memory.nregions = memory->nregions; debug("vhost-user nregions: %u", memory->nregions); - for (i = 0; i < vdev->nregions; i++) { + for (i = 0; i < vdev->memory.nregions; i++) { struct vhost_user_memory_region *msg_region = &memory->regions[i]; - struct vu_dev_region *dev_region = &vdev->regions[i]; + struct vu_dev_region *dev_region = &vdev->memory.regions[i]; void *mmap_addr; debug("vhost-user region %d", i); @@ -484,13 +484,7 @@ static bool vu_set_mem_table_exec(struct vu_dev *vdev, } } - /* As vu_packet_check_range() has no access to the number of - * memory regions, mark the end of the array with mmap_addr = 0 - */ - ASSERT(vdev->nregions < VHOST_USER_MAX_RAM_SLOTS - 1); - vdev->regions[vdev->nregions].mmap_addr = 0; - - tap_sock_update_pool(vdev->regions, 0); + ASSERT(vdev->memory.nregions < VHOST_USER_MAX_RAM_SLOTS); return false; } @@ -1107,8 +1101,8 @@ void vu_cleanup(struct vu_dev *vdev) vq->vring.avail = 0; } - for (i = 0; i < vdev->nregions; i++) { - const struct vu_dev_region *r = &vdev->regions[i]; + for (i = 0; i < vdev->memory.nregions; i++) { + const struct vu_dev_region *r = &vdev->memory.regions[i]; if (r->mmap_addr) { /* NOLINTNEXTLINE(performance-no-int-to-ptr) */ @@ -1116,7 +1110,7 @@ void vu_cleanup(struct vu_dev *vdev) r->size + r->mmap_offset); } } - vdev->nregions = 0; + vdev->memory.nregions = 0; vu_close_log(vdev); diff --git a/virtio.c b/virtio.c index bc2b89a7f4a7..2b0a8d0dc4c5 100644 --- a/virtio.c +++ b/virtio.c @@ -102,8 +102,8 @@ static void *vu_gpa_to_va(const struct vu_dev *dev, uint64_t *plen, return NULL; /* Find matching memory region. */ - for (i = 0; i < dev->nregions; i++) { - const struct vu_dev_region *r = &dev->regions[i]; + for (i = 0; i < dev->memory.nregions; i++) { + const struct vu_dev_region *r = &dev->memory.regions[i]; if ((guest_addr >= r->gpa) && (guest_addr < (r->gpa + r->size))) { diff --git a/virtio.h b/virtio.h index 7a370bdeae65..5acd4f19f0ed 100644 --- a/virtio.h +++ b/virtio.h @@ -96,6 +96,11 @@ struct vu_dev_region { */ #define VHOST_USER_MAX_RAM_SLOTS 32 +struct vdev_memory { + uint32_t nregions; + struct vu_dev_region regions[VHOST_USER_MAX_RAM_SLOTS]; +}; + /** * struct vu_dev - vhost-user device information * @context: Execution context @@ -109,8 +114,7 @@ struct vu_dev_region { */ struct vu_dev { struct ctx *context; - uint32_t nregions; - struct vu_dev_region regions[VHOST_USER_MAX_RAM_SLOTS]; + struct vdev_memory memory; struct vu_virtq vq[VHOST_USER_MAX_QUEUES]; uint64_t features; uint64_t protocol_features; diff --git a/vu_common.c b/vu_common.c index b77b21420c57..19fe22935a1e 100644 --- a/vu_common.c +++ b/vu_common.c @@ -25,26 +25,28 @@ /** * vu_packet_check_range() - Check if a given memory zone is contained in * a mapped guest memory region - * @buf: Array of the available memory regions + * @memory: Allowed memory regions * @ptr: Start of desired data range * @size: Length of desired data range * * Return: 0 if the zone is in a mapped memory region, -1 otherwise */ -int vu_packet_check_range(void *buf, const char *ptr, size_t len) +int vu_packet_check_range(struct vdev_memory *memory, + const char *ptr, size_t len) { - struct vu_dev_region *dev_region; + struct vu_dev_region *dev_region = memory->regions; + unsigned int i; - for (dev_region = buf; dev_region->mmap_addr; dev_region++) { - uintptr_t base_addr = dev_region->mmap_addr + - dev_region->mmap_offset; + for (i = 0; i < memory->nregions; i++) { + uintptr_t base_addr = dev_region[i].mmap_addr + + dev_region[i].mmap_offset; /* NOLINTNEXTLINE(performance-no-int-to-ptr) */ const char *base = (const char *)base_addr; - ASSERT(base_addr >= dev_region->mmap_addr); + ASSERT(base_addr >= dev_region[i].mmap_addr); - if (len <= dev_region->size && base <= ptr && - (size_t)(ptr - base) <= dev_region->size - len) + if (len <= dev_region[i].size && base <= ptr && + (size_t)(ptr - base) <= dev_region[i].size - len) return 0; } -- 2.49.0