Don't use the memory of the incoming packet to build the outgoing buffer as it can be memory of the TX queue in the case of vhost-user. Moreover with vhost-user, the packet can be split across several iovec and it's easier to rebuild it in a buffer than updating an existing iovec array. Signed-off-by: Laurent Vivier Reviewed-by: David Gibson --- arp.c | 84 ++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 55 insertions(+), 29 deletions(-) diff --git a/arp.c b/arp.c index fc482bbd9938..9f1fedeafec0 100644 --- a/arp.c +++ b/arp.c @@ -31,56 +31,82 @@ #include "tap.h" /** - * arp() - Check if this is a supported ARP message, reply as needed + * ignore_arp() - Check if we should ignore this ARP message * @c: Execution context - * @p: Packet pool, single packet with Ethernet buffer + * @ah: ARP header + * @am: ARP message * - * Return: 1 if handled, -1 on failure + * Return: true if the ARP message should be ignored, false otherwise */ -int arp(const struct ctx *c, const struct pool *p) +static bool ignore_arp(const struct ctx *c, + const struct arphdr *ah, const struct arpmsg *am) { - unsigned char swap[4]; - struct ethhdr *eh; - struct arphdr *ah; - struct arpmsg *am; - size_t l2len; - - eh = packet_get(p, 0, 0, sizeof(*eh), NULL); - ah = packet_get(p, 0, sizeof(*eh), sizeof(*ah), NULL); - am = packet_get(p, 0, sizeof(*eh) + sizeof(*ah), sizeof(*am), NULL); - - if (!eh || !ah || !am) - return -1; - if (ah->ar_hrd != htons(ARPHRD_ETHER) || ah->ar_pro != htons(ETH_P_IP) || ah->ar_hln != ETH_ALEN || ah->ar_pln != 4 || ah->ar_op != htons(ARPOP_REQUEST)) - return 1; + return true; /* Discard announcements, but not 0.0.0.0 "probes" */ if (memcmp(am->sip, &in4addr_any, sizeof(am->sip)) && !memcmp(am->sip, am->tip, sizeof(am->sip))) - return 1; + return true; /* Don't resolve the guest's assigned address, either. */ if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip))) + return true; + + return false; +} + +/** + * arp() - Check if this is a supported ARP message, reply as needed + * @c: Execution context + * @p: Packet pool, single packet with Ethernet buffer + * + * Return: 1 if handled, -1 on failure + */ +int arp(const struct ctx *c, const struct pool *p) +{ + struct { + struct ethhdr eh; + struct arphdr ah; + struct arpmsg am; + } __attribute__((__packed__)) resp; + const struct ethhdr *eh; + const struct arphdr *ah; + const struct arpmsg *am; + + eh = packet_get(p, 0, 0, sizeof(*eh), NULL); + ah = packet_get(p, 0, sizeof(*eh), sizeof(*ah), NULL); + am = packet_get(p, 0, sizeof(*eh) + sizeof(*ah), sizeof(*am), NULL); + + if (!eh || !ah || !am) + return -1; + + if (ignore_arp(c, ah, am)) return 1; - ah->ar_op = htons(ARPOP_REPLY); - memcpy(am->tha, am->sha, sizeof(am->tha)); - memcpy(am->sha, c->our_tap_mac, sizeof(am->sha)); + /* Ethernet header */ + resp.eh.h_proto = htons(ETH_P_ARP); + memcpy(resp.eh.h_dest, eh->h_source, sizeof(resp.eh.h_dest)); + memcpy(resp.eh.h_source, c->our_tap_mac, sizeof(resp.eh.h_source)); - memcpy(swap, am->tip, sizeof(am->tip)); - memcpy(am->tip, am->sip, sizeof(am->tip)); - memcpy(am->sip, swap, sizeof(am->sip)); + /* ARP header */ + resp.ah.ar_op = htons(ARPOP_REPLY); + resp.ah.ar_hrd = ah->ar_hrd; + resp.ah.ar_pro = ah->ar_pro; + resp.ah.ar_hln = ah->ar_hln; + resp.ah.ar_pln = ah->ar_pln; - l2len = sizeof(*eh) + sizeof(*ah) + sizeof(*am); - memcpy(eh->h_dest, eh->h_source, sizeof(eh->h_dest)); - memcpy(eh->h_source, c->our_tap_mac, sizeof(eh->h_source)); + /* ARP message */ + memcpy(resp.am.sha, c->our_tap_mac, sizeof(resp.am.sha)); + memcpy(resp.am.sip, am->tip, sizeof(resp.am.sip)); + memcpy(resp.am.tha, am->sha, sizeof(resp.am.tha)); + memcpy(resp.am.tip, am->sip, sizeof(resp.am.tip)); - tap_send_single(c, eh, l2len); + tap_send_single(c, &resp, sizeof(resp)); return 1; } -- 2.50.1

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 Reviewed-by: David Gibson --- iov.c | 53 ++++++++++++++++++++++++++++++++++++++++++++--------- iov.h | 35 +++++++++++++++++++++-------------- tcp_buf.c | 2 +- 3 files changed, 66 insertions(+), 24 deletions(-) diff --git a/iov.c b/iov.c index 9a91eb63026a..d20fbb854933 100644 --- a/iov.c +++ b/iov.c @@ -109,7 +109,7 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt, * * Return: the number of bytes successfully copied. */ -/* cppcheck-suppress unusedFunction */ +/* cppcheck-suppress [staticFunction] */ size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt, size_t offset, void *buf, size_t bytes) { @@ -127,6 +127,7 @@ size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt, /* copying data */ for (copied = 0; copied < bytes && i < iov_cnt; i++) { size_t len = MIN(iov[i].iov_len - offset, bytes - copied); + ASSERT(iov[i].iov_base); memcpy((char *)buf + copied, (char *)iov[i].iov_base + offset, len); copied += len; @@ -208,7 +209,7 @@ bool iov_drop_header(struct iov_tail *tail, size_t len) } /** - * iov_peek_header_() - Get pointer to a header from an IOV tail + * iov_check_header() - Check if a header can be accessed * @tail: IOV tail to get header from * @len: Length of header to get, in bytes * @align: Required alignment of header, in bytes @@ -219,8 +220,7 @@ bool iov_drop_header(struct iov_tail *tail, size_t len) * overruns the IO vector, is not contiguous or doesn't have the * requested alignment. */ -/* cppcheck-suppress [staticFunction,unmatchedSuppression] */ -void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align) +static void *iov_check_header(struct iov_tail *tail, size_t len, size_t align) { char *p; @@ -240,27 +240,62 @@ void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align) return p; } +/** + * iov_peek_header_() - Get pointer to a header from an IOV tail + * @tail: IOV tail to get header from + * @v: Temporary memory to use if the memory in @tail + * is discontinuous + * @len: Length of header to get, in bytes + * @align: Required alignment of header, in bytes + * + * @tail may be pruned, but will represent the same bytes as before. + * + * Return: 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. * - * Return: 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. + * Return: 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 c94fabe1eb43..1821b40b1cb5 100644 --- a/iov.h +++ b/iov.h @@ -73,38 +73,45 @@ struct iov_tail { bool iov_tail_prune(struct iov_tail *tail); size_t iov_tail_size(struct iov_tail *tail); bool iov_drop_header(struct iov_tail *tail, size_t len); -void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align); -void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align); +void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, size_t align); +void *iov_remove_header_(struct iov_tail *tail, void *v, size_t len, size_t align); ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt, struct iov_tail *tail); /** * IOV_PEEK_HEADER() - Get typed pointer to a header from an IOV tail * @tail_: IOV tail to get header from - * @type_: Data type of the header + * @var_: Temporary buffer of the type of the header to use if + * the memory in the iovec array is not contiguous. * * @tail_ may be pruned, but will represent the same bytes as before. * - * Return: pointer of type (@type_ *) located at the start of @tail_, NULL if - * we can't get a contiguous and aligned pointer. + * Return: 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. * - * Return: pointer of type (@type_ *) located at the old start of @tail_, NULL - * if we can't get a contiguous and aligned pointer. + * Return: 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_, type_) \ - ((type_ *)(iov_remove_header_((tail_), \ - sizeof(type_), __alignof__(type_)))) + +#define IOV_REMOVE_HEADER(tail_, var_) \ + ((__typeof__(var_) *)(iov_remove_header_((tail_), &(var_), \ + sizeof(var_), __alignof__(var_)))) #endif /* IOVEC_H */ diff --git a/tcp_buf.c b/tcp_buf.c index d1fca676c9a7..bc898de86919 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -160,7 +160,7 @@ static void tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn, uint32_t seq, bool no_tcp_csum) { struct iov_tail tail = IOV_TAIL(&iov[TCP_IOV_PAYLOAD], 1, 0); - struct tcphdr *th = IOV_REMOVE_HEADER(&tail, struct tcphdr); + struct tcphdr th_storage, *th = IOV_REMOVE_HEADER(&tail, th_storage); struct tap_hdr *taph = iov[TCP_IOV_TAP].iov_base; const struct flowside *tapside = TAPFLOW(conn); const struct in_addr *a4 = inany_v4(&tapside->oaddr); -- 2.50.1

Modify the interface of packet_add_do() to take an iov_tail rather than a memory pointer and length. Internally it only supports iovec array with only one entry, after being pruned. We can accept iovec array with several entries if the offset allows the function to reduce the number of entries to 1. tap4_handler() is updated to create an iov_tail value using IOV_TAIL_FROM_BUF() from the buffer and the length. Signed-off-by: Laurent Vivier Reviewed-by: David Gibson --- packet.c | 15 ++++++++++++--- packet.h | 7 ++++--- tap.c | 32 ++++++++++++++++++-------------- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/packet.c b/packet.c index 72c61580be1e..98ded4e27aae 100644 --- a/packet.c +++ b/packet.c @@ -87,15 +87,16 @@ bool pool_full(const struct pool *p) /** * packet_add_do() - Add data as packet descriptor to given pool * @p: Existing pool - * @len: Length of new descriptor - * @start: Start of data + * @data: Data to add * @func: For tracing: name of calling function * @line: For tracing: caller line of function call */ -void packet_add_do(struct pool *p, size_t len, const char *start, +void packet_add_do(struct pool *p, struct iov_tail *data, const char *func, int line) { size_t idx = p->count; + const char *start; + size_t len; if (pool_full(p)) { debug("add packet index %zu to pool with size %zu, %s:%i", @@ -103,6 +104,14 @@ void packet_add_do(struct pool *p, size_t len, const char *start, return; } + if (!iov_tail_prune(data)) + return; + + ASSERT(data->cnt == 1); /* we don't support iovec */ + + len = data->iov[0].iov_len - data->off; + start = (char *)data->iov[0].iov_base + data->off; + if (packet_check_range(p, start, len, func, line)) return; diff --git a/packet.h b/packet.h index c94780a5ea54..af40b39b5251 100644 --- a/packet.h +++ b/packet.h @@ -7,6 +7,7 @@ #define PACKET_H #include +#include "iov.h" /* Maximum size of a single packet stored in pool, including headers */ #define PACKET_MAX_LEN ((size_t)UINT16_MAX) @@ -30,7 +31,7 @@ struct pool { }; int vu_packet_check_range(void *buf, const char *ptr, size_t len); -void packet_add_do(struct pool *p, size_t len, const char *start, +void packet_add_do(struct pool *p, struct iov_tail *data, const char *func, int line); void *packet_get_try_do(const struct pool *p, const size_t idx, size_t offset, size_t len, size_t *left, @@ -41,8 +42,8 @@ void *packet_get_do(const struct pool *p, const size_t idx, bool pool_full(const struct pool *p); void pool_flush(struct pool *p); -#define packet_add(p, len, start) \ - packet_add_do(p, len, start, __func__, __LINE__) +#define packet_add(p, data) \ + packet_add_do(p, data, __func__, __LINE__) #define packet_get_try(p, idx, offset, len, left) \ packet_get_try_do(p, idx, offset, len, left, __func__, __LINE__) diff --git a/tap.c b/tap.c index b1de0c5b61a2..7cb74949231b 100644 --- a/tap.c +++ b/tap.c @@ -709,6 +709,7 @@ resume: size_t l2len, l3len, hlen, l4len; const struct ethhdr *eh; const struct udphdr *uh; + struct iov_tail data; struct iphdr *iph; const char *l4h; @@ -720,7 +721,8 @@ resume: if (ntohs(eh->h_proto) == ETH_P_ARP) { PACKET_POOL_P(pkt, 1, in->buf, in->buf_size); - packet_add(pkt, l2len, (char *)eh); + data = IOV_TAIL_FROM_BUF((void *)eh, l2len, 0); + packet_add(pkt, &data); arp(c, pkt); continue; } @@ -765,7 +767,8 @@ resume: tap_packet_debug(iph, NULL, NULL, 0, NULL, 1); - packet_add(pkt, l4len, l4h); + data = IOV_TAIL_FROM_BUF((void *)l4h, l4len, 0); + packet_add(pkt, &data); icmp_tap_handler(c, PIF_TAP, AF_INET, &iph->saddr, &iph->daddr, pkt, now); @@ -779,7 +782,8 @@ resume: if (iph->protocol == IPPROTO_UDP) { PACKET_POOL_P(pkt, 1, in->buf, in->buf_size); - packet_add(pkt, l2len, (char *)eh); + data = IOV_TAIL_FROM_BUF((void *)eh, l2len, 0); + packet_add(pkt, &data); if (dhcp(c, pkt)) continue; } @@ -830,7 +834,8 @@ resume: #undef L4_SET append: - packet_add((struct pool *)&seq->p, l4len, l4h); + data = IOV_TAIL_FROM_BUF((void *)l4h, l4len, 0); + packet_add((struct pool *)&seq->p, &data); } for (j = 0, seq = tap4_l4; j < seq_count; j++, seq++) { @@ -886,6 +891,7 @@ resume: struct in6_addr *saddr, *daddr; const struct ethhdr *eh; const struct udphdr *uh; + struct iov_tail data; struct ipv6hdr *ip6h; uint8_t proto; char *l4h; @@ -939,7 +945,8 @@ resume: if (l4len < sizeof(struct icmp6hdr)) continue; - packet_add(pkt, l4len, l4h); + data = IOV_TAIL_FROM_BUF(l4h, l4len, 0); + packet_add(pkt, &data); if (ndp(c, (struct icmp6hdr *)l4h, saddr, pkt)) continue; @@ -958,7 +965,8 @@ resume: if (proto == IPPROTO_UDP) { PACKET_POOL_P(pkt, 1, in->buf, in->buf_size); - packet_add(pkt, l4len, l4h); + data = IOV_TAIL_FROM_BUF(l4h, l4len, 0); + packet_add(pkt, &data); if (dhcpv6(c, pkt, saddr, daddr)) continue; @@ -1014,7 +1022,8 @@ resume: #undef L4_SET append: - packet_add((struct pool *)&seq->p, l4len, l4h); + data = IOV_TAIL_FROM_BUF(l4h, l4len, 0); + packet_add((struct pool *)&seq->p, &data); } for (j = 0, seq = tap6_l4; j < seq_count; j++, seq++) { @@ -1090,9 +1099,6 @@ void tap_add_packet(struct ctx *c, struct iov_tail *data, proto_update_l2_buf(c->guest_mac, NULL); } - iov_tail_prune(data); - ASSERT(data->cnt == 1); /* packet_add() doesn't support iovec */ - switch (ntohs(eh->h_proto)) { case ETH_P_ARP: case ETH_P_IP: @@ -1100,16 +1106,14 @@ void tap_add_packet(struct ctx *c, struct iov_tail *data, tap4_handler(c, pool_tap4, now); pool_flush(pool_tap4); } - packet_add(pool_tap4, data->iov[0].iov_len - data->off, - (char *)data->iov[0].iov_base + data->off); + packet_add(pool_tap4, data); break; case ETH_P_IPV6: if (pool_full(pool_tap6)) { tap6_handler(c, pool_tap6, now); pool_flush(pool_tap6); } - packet_add(pool_tap6, data->iov[0].iov_len - data->off, - (char *)data->iov[0].iov_base + data->off); + packet_add(pool_tap6, data); break; default: break; -- 2.50.1

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 9f1fedeafec0..b3ac42082841 100644 --- a/arp.c +++ b/arp.c @@ -74,14 +74,20 @@ int arp(const struct ctx *c, const struct pool *p) struct arphdr ah; struct arpmsg am; } __attribute__((__packed__)) resp; + struct arphdr ah_storage; + struct ethhdr eh_storage; + struct arpmsg am_storage; const struct ethhdr *eh; const struct arphdr *ah; const struct arpmsg *am; + struct iov_tail data; - 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, eh_storage); + ah = IOV_REMOVE_HEADER(&data, ah_storage); + am = IOV_REMOVE_HEADER(&data, am_storage); if (!eh || !ah || !am) return -1; diff --git a/packet.c b/packet.c index 82adc9fd1a39..34b1722b9a03 100644 --- a/packet.c +++ b/packet.c @@ -201,7 +201,6 @@ void *packet_get_do(const struct pool *p, const size_t idx, * Return: false if packet index is invalid, true otherwise. * If something wrong with @data, don't return at all (assert). */ -/* cppcheck-suppress unusedFunction */ bool packet_data_do(const struct pool *p, size_t idx, struct iov_tail *data, const char *func, int line) -- 2.50.1

Use packet_data() and extract headers using IOV_PEEK_HEADER() rather than packet_get(). Signed-off-by: Laurent Vivier Reviewed-by: David Gibson --- icmp.c | 40 +++++++++++++++++++++++++++------------- iov.c | 1 - 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/icmp.c b/icmp.c index 95f38c1e2a3a..7791e88733d7 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 /* Arbitrary, should be enough */ /** * ping_at_sidx() - Get ping specific flow at given sidx @@ -238,28 +239,31 @@ 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 iovec iov[MAX_IOV_ICMP]; 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; + int cnt; (void)saddr; ASSERT(pif == PIF_TAP); + if (!packet_data(p, 0, &data)) + return -1; + if (af == AF_INET) { + struct icmphdr ih_storage; const struct icmphdr *ih; - if (!(pkt = packet_get(p, 0, 0, sizeof(*ih), &dlen))) + ih = IOV_PEEK_HEADER(&data, ih_storage); + if (!ih) return 1; - ih = (struct icmphdr *)pkt; - l4len = dlen + sizeof(*ih); - if (ih->type != ICMP_ECHO) return 1; @@ -267,14 +271,13 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, id = ntohs(ih->un.echo.id); seq = ntohs(ih->un.echo.sequence); } else if (af == AF_INET6) { + struct icmp6hdr ih_storage; const struct icmp6hdr *ih; - if (!(pkt = packet_get(p, 0, 0, sizeof(*ih), &dlen))) + ih = IOV_PEEK_HEADER(&data, ih_storage); + if (!ih) return 1; - ih = (struct icmp6hdr *)pkt; - l4len = dlen + sizeof(*ih); - if (ih->icmp6_type != ICMPV6_ECHO_REQUEST) return 1; @@ -285,6 +288,10 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, ASSERT(0); } + cnt = iov_tail_clone(&iov[0], MAX_IOV_ICMP, &data); + if (cnt < 0) + return 1; + flow = flow_at_sidx(flow_lookup_af(c, proto, PIF_TAP, af, saddr, daddr, id, id)); @@ -298,8 +305,15 @@ 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) { + 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 { flow_dbg(pingf, diff --git a/iov.c b/iov.c index 0329613153a8..35dc60380c8c 100644 --- a/iov.c +++ b/iov.c @@ -310,7 +310,6 @@ void *iov_remove_header_(struct iov_tail *tail, void *v, size_t len, size_t alig * iov array, a negative value if there is not enough room in the * destination iov array */ -/* cppcheck-suppress unusedFunction */ ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt, struct iov_tail *tail) { -- 2.50.1

Use packet_data() and extract headers using IOV_REMOVE_HEADER() and iov_remove_header_() rather than packet_get(). Signed-off-by: Laurent Vivier Reviewed-by: David Gibson --- tcp.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/tcp.c b/tcp.c index 957b498db32d..f1048d7230c9 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) - 1 - 5) * 4UL) + #ifndef __USE_MISC /* From Linux UAPI, missing in netinet/tcp.h provided by musl */ struct tcp_repair_opt { @@ -1957,8 +1967,11 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, const struct pool *p, int idx, const struct timespec *now) { struct tcp_tap_conn *conn; + struct tcphdr th_storage; const struct tcphdr *th; - size_t optlen, len; + char optsc[OPTLEN_MAX]; + struct iov_tail data; + size_t optlen, l4len; const char *opts; union flow *flow; flow_sidx_t sidx; @@ -1967,15 +1980,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, th_storage); 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)); @@ -1987,7 +2004,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; } @@ -1995,7 +2012,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.50.1

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 ba16c664ee24..1e540bb84f0d 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 @@ -292,8 +292,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); @@ -329,7 +328,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; @@ -466,8 +465,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 @@ -524,15 +524,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; @@ -582,7 +582,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.50.1

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 8e6c29df205d..ae06e646f92f 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -533,12 +533,18 @@ int dhcpv6(struct ctx *c, const struct pool *p, { const struct opt_hdr *client_id, *server_id, *ia; const struct in6_addr *src; + struct msg_hdr mh_storage; const struct msg_hdr *mh; + struct udphdr uh_storage; const struct udphdr *uh; struct opt_hdr *bad_ia; + struct iov_tail data; size_t mlen, n; - uh = packet_get(p, 0, 0, sizeof(*uh), &mlen); + if (!packet_data(p, 0, &data)) + return -1; + + uh = IOV_REMOVE_HEADER(&data, uh_storage); if (!uh) return -1; @@ -551,6 +557,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; @@ -558,7 +565,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, mh_storage); if (!mh) return -1; -- 2.50.1

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 --- dhcp.c | 46 ++++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/dhcp.c b/dhcp.c index b0de04be6f27..d33e23ee4d48 100644 --- a/dhcp.c +++ b/dhcp.c @@ -302,27 +302,33 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len) */ int dhcp(const struct ctx *c, const struct pool *p) { - size_t mlen, dlen, offset = 0, opt_len, opt_off = 0; char macstr[ETH_ADDRSTRLEN]; + size_t mlen, dlen, opt_len; struct in_addr mask, dst; + struct ethhdr eh_storage; + struct iphdr iph_storage; + struct udphdr uh_storage; const struct ethhdr *eh; const struct iphdr *iph; const struct udphdr *uh; + struct iov_tail data; struct msg const *m; struct msg reply; unsigned int i; + struct msg m_storage; - eh = packet_get(p, 0, offset, sizeof(*eh), NULL); - offset += sizeof(*eh); + if (!packet_data(p, 0, &data)) + return -1; - iph = packet_get(p, 0, offset, sizeof(*iph), NULL); + eh = IOV_REMOVE_HEADER(&data, eh_storage); + iph = IOV_PEEK_HEADER(&data, iph_storage); if (!eh || !iph) return -1; - offset += iph->ihl * 4UL; - uh = packet_get(p, 0, offset, sizeof(*uh), &mlen); - offset += sizeof(*uh); + if (!iov_drop_header(&data, iph->ihl * 4UL)) + return -1; + uh = IOV_REMOVE_HEADER(&data, uh_storage); if (!uh) return -1; @@ -332,7 +338,10 @@ 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, &m_storage, + offsetof(struct msg, o), + __alignof__(struct msg)); if (!m || mlen != ntohs(uh->len) - sizeof(*uh) || mlen < offsetof(struct msg, o) || @@ -355,27 +364,28 @@ int dhcp(const struct ctx *c, const struct pool *p) memset(&reply.file, 0, sizeof(reply.file)); reply.magic = m->magic; - offset += offsetof(struct msg, o); - for (i = 0; i < ARRAY_SIZE(opts); i++) opts[i].clen = -1; - while (opt_off + 2 < opt_len) { - const uint8_t *olen, *val; + opt_len = iov_tail_size(&data); + while (opt_len >= 2) { + uint8_t olen_storage, type_storage; + const uint8_t *olen; uint8_t *type; - type = packet_get(p, 0, offset + opt_off, 1, NULL); - olen = packet_get(p, 0, offset + opt_off + 1, 1, NULL); + type = IOV_REMOVE_HEADER(&data, type_storage); + olen = IOV_REMOVE_HEADER(&data, olen_storage); if (!type || !olen) return -1; - val = packet_get(p, 0, offset + opt_off + 2, *olen, NULL); - if (!val) + opt_len = iov_tail_size(&data); + if (opt_len < *olen) return -1; - memcpy(&opts[*type].c, val, *olen); + iov_to_buf(&data.iov[0], data.cnt, data.off, &opts[*type].c, *olen); opts[*type].clen = *olen; - opt_off += *olen + 2; + iov_drop_header(&data, *olen); + opt_len -= *olen; } opts[80].slen = -1; -- 2.50.1

Use packet_data() and extract headers using IOV_PEEK_HEADER() rather than packet_get(). Signed-off-by: Laurent Vivier Reviewed-by: David Gibson --- tap.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/tap.c b/tap.c index 497440a00dd8..bb18b76676e8 100644 --- a/tap.c +++ b/tap.c @@ -706,28 +706,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; + struct ethhdr eh_storage; + struct iphdr iph_storage; + struct udphdr uh_storage; const struct ethhdr *eh; const struct udphdr *uh; struct iov_tail data; struct iphdr *iph; - const char *l4h; - 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, eh_storage); 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_drop_header(&data, sizeof(*eh))) + continue; + l3len = iov_tail_size(&data); + + iph = IOV_PEEK_HEADER(&data, iph_storage); if (!iph) continue; @@ -755,8 +761,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_drop_header(&data, hlen)) + continue; + if (iov_tail_size(&data) != l4len) continue; if (iph->protocol == IPPROTO_ICMP) { @@ -767,7 +774,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, @@ -775,15 +781,17 @@ resume: continue; } - uh = packet_get(in, i, sizeof(*eh) + hlen, sizeof(*uh), NULL); + uh = IOV_PEEK_HEADER(&data, uh_storage); 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; } @@ -834,7 +842,6 @@ resume: #undef L4_SET append: - data = IOV_TAIL_FROM_BUF((void *)l4h, l4len, 0); packet_add((struct pool *)&seq->p, &data); } -- 2.50.1

As we have removed packet_get(), we can rename packet_data() to packet_get() as the name is clearer. Signed-off-by: Laurent Vivier Reviewed-by: David Gibson --- arp.c | 2 +- dhcp.c | 2 +- dhcpv6.c | 2 +- icmp.c | 2 +- ndp.c | 2 +- packet.c | 8 ++++---- packet.h | 9 ++++----- tap.c | 6 +++--- tcp.c | 4 ++-- udp.c | 4 ++-- 10 files changed, 20 insertions(+), 21 deletions(-) diff --git a/arp.c b/arp.c index b3ac42082841..8b97df633e70 100644 --- a/arp.c +++ b/arp.c @@ -82,7 +82,7 @@ int arp(const struct ctx *c, const struct pool *p) const struct arpmsg *am; struct iov_tail data; - if (!packet_data(p, 0, &data)) + if (!packet_get(p, 0, &data)) return -1; eh = IOV_REMOVE_HEADER(&data, eh_storage); diff --git a/dhcp.c b/dhcp.c index d33e23ee4d48..81175296bc2b 100644 --- a/dhcp.c +++ b/dhcp.c @@ -317,7 +317,7 @@ int dhcp(const struct ctx *c, const struct pool *p) unsigned int i; struct msg m_storage; - if (!packet_data(p, 0, &data)) + if (!packet_get(p, 0, &data)) return -1; eh = IOV_REMOVE_HEADER(&data, eh_storage); diff --git a/dhcpv6.c b/dhcpv6.c index b94c8bc84b28..fea4a84bc654 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -562,7 +562,7 @@ int dhcpv6(struct ctx *c, const struct pool *p, const struct udphdr *uh; size_t mlen, n; - if (!packet_data(p, 0, &data)) + if (!packet_get(p, 0, &data)) return -1; uh = IOV_REMOVE_HEADER(&data, uh_storage); diff --git a/icmp.c b/icmp.c index 7791e88733d7..9d90141542ff 100644 --- a/icmp.c +++ b/icmp.c @@ -253,7 +253,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, (void)saddr; ASSERT(pif == PIF_TAP); - if (!packet_data(p, 0, &data)) + if (!packet_get(p, 0, &data)) return -1; if (af == AF_INET) { diff --git a/ndp.c b/ndp.c index 5de4e508dc52..ba87a0aaa6e9 100644 --- a/ndp.c +++ b/ndp.c @@ -354,7 +354,7 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih, const struct ndp_ns *ns; struct iov_tail data; - if (!packet_data(p, 0, &data)) + if (!packet_get(p, 0, &data)) return -1; ns = IOV_REMOVE_HEADER(&data, ns_storage); diff --git a/packet.c b/packet.c index 5da18bafa576..cbc43c2fc22d 100644 --- a/packet.c +++ b/packet.c @@ -122,7 +122,7 @@ void packet_add_do(struct pool *p, struct iov_tail *data, } /** - * packet_data_do() - Get data range from packet descriptor from given pool + * packet_get_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) @@ -132,9 +132,9 @@ void packet_add_do(struct pool *p, struct iov_tail *data, * Return: false if packet index is invalid, true otherwise. * If something wrong with @data, don't return at all (assert). */ -bool packet_data_do(const struct pool *p, size_t idx, - struct iov_tail *data, - const char *func, int line) +bool packet_get_do(const struct pool *p, size_t idx, + struct iov_tail *data, + const char *func, int line) { size_t i; diff --git a/packet.h b/packet.h index dab8274fa5c5..7afe80ef3fcf 100644 --- a/packet.h +++ b/packet.h @@ -33,16 +33,15 @@ 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); -bool packet_data_do(const struct pool *p, const size_t idx, - struct iov_tail *data, - const char *func, int line); +bool packet_get_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); #define packet_add(p, data) \ packet_add_do(p, data, __func__, __LINE__) -#define packet_data(p, idx, data) \ - packet_data_do(p, idx, data, __func__, __LINE__) +#define packet_get(p, idx, data) \ + packet_get_do(p, idx, data, __func__, __LINE__) #define PACKET_POOL_DECL(_name, _size, _buf) \ struct _name ## _t { \ diff --git a/tap.c b/tap.c index 5010c45d1fa3..4b3f08787384 100644 --- a/tap.c +++ b/tap.c @@ -715,7 +715,7 @@ resume: struct iov_tail data; struct iphdr *iph; - if (!packet_data(in, i, &data)) + if (!packet_get(in, i, &data)) continue; eh = IOV_PEEK_HEADER(&data, eh_storage); @@ -790,7 +790,7 @@ resume: PACKET_POOL_P(pkt, 1, in->buf, in->buf_size); - packet_data(in, i, &eh_data); + packet_get(in, i, &eh_data); packet_add(pkt, &eh_data); if (dhcp(c, pkt)) continue; @@ -905,7 +905,7 @@ resume: struct ipv6hdr *ip6h; uint8_t proto; - if (!packet_data(in, i, &data)) + if (!packet_get(in, i, &data)) return -1; eh = IOV_REMOVE_HEADER(&data, eh_storage); diff --git a/tcp.c b/tcp.c index 80f0c1ff2774..dba5fdcfa957 100644 --- a/tcp.c +++ b/tcp.c @@ -1657,7 +1657,7 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn, size_t off, size; int count; - if (!packet_data(p, i, &data)) + if (!packet_get(p, i, &data)) return -1; th = IOV_PEEK_HEADER(&data, th_storage); @@ -1988,7 +1988,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, (void)pif; - if (!packet_data(p, idx, &data)) + if (!packet_get(p, idx, &data)) return 1; l4len = iov_tail_size(&data); diff --git a/udp.c b/udp.c index 3c25f2e0ae97..86585b7e0942 100644 --- a/udp.c +++ b/udp.c @@ -990,7 +990,7 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, ASSERT(!c->no_udp); - if (!packet_data(p, idx, &data)) + if (!packet_get(p, idx, &data)) return 1; uh = IOV_PEEK_HEADER(&data, uh_storage); @@ -1033,7 +1033,7 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, for (i = 0, j = 0; i < (int)p->count - idx && j < UIO_MAXIOV; i++) { const struct udphdr *uh_send; - if (!packet_data(p, idx + i, &data)) + if (!packet_get(p, idx + i, &data)) return p->count - idx; uh_send = IOV_REMOVE_HEADER(&data, uh_storage); -- 2.50.1

This patch refactors the dhcp() function to accept `struct iov_tail *data` directly as its packet input, replacing the previous `const struct pool *p` parameter. Consequently, dhcp() no longer fetches packet data internally using packet_data(). This change simplifies callers, such as tap4_handler(), which now pass the iov_tail representing the L2 frame directly to dhcp(). This removes the need for intermediate packet pool handling for DHCP processing. Signed-off-by: Laurent Vivier Reviewed-by: David Gibson --- 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 81175296bc2b..012cec6c0d04 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) { char macstr[ETH_ADDRSTRLEN]; size_t mlen, dlen, opt_len; @@ -311,24 +311,20 @@ 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 reply; unsigned int i; struct msg m_storage; - if (!packet_get(p, 0, &data)) - return -1; - - eh = IOV_REMOVE_HEADER(&data, eh_storage); - iph = IOV_PEEK_HEADER(&data, iph_storage); + eh = IOV_REMOVE_HEADER(data, eh_storage); + iph = IOV_PEEK_HEADER(data, iph_storage); if (!eh || !iph) return -1; - if (!iov_drop_header(&data, iph->ihl * 4UL)) + if (!iov_drop_header(data, iph->ihl * 4UL)) return -1; - uh = IOV_REMOVE_HEADER(&data, uh_storage); + uh = IOV_REMOVE_HEADER(data, uh_storage); 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, &m_storage, + mlen = iov_tail_size(data); + m = (struct msg const *)iov_remove_header_(data, &m_storage, offsetof(struct msg, o), __alignof__(struct msg)); if (!m || @@ -367,24 +363,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 olen_storage, type_storage; const uint8_t *olen; uint8_t *type; - type = IOV_REMOVE_HEADER(&data, type_storage); - olen = IOV_REMOVE_HEADER(&data, olen_storage); + type = IOV_REMOVE_HEADER(data, type_storage); + olen = IOV_REMOVE_HEADER(data, olen_storage); 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_drop_header(&data, *olen); + iov_drop_header(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 7a538b20723e..14050b37483e 100644 --- a/tap.c +++ b/tap.c @@ -785,11 +785,8 @@ resume: if (iph->protocol == IPPROTO_UDP) { struct iov_tail eh_data; - PACKET_POOL_P(pkt, 1, in->buf, in->buf_size); - packet_get(in, i, &eh_data); - packet_add(pkt, &eh_data); - if (dhcp(c, pkt)) + if (dhcp(c, &eh_data)) continue; } -- 2.50.1

This patch refactors the icmp_tap_handler() function to accept `struct iov_tail *data` directly as its packet input, replacing the `const struct pool *p` parameter. This change simplifies callers, such as tap4_handler(), which now pass the iov_tail representing the L4 ICMP message directly. This removes the need for intermediate packet pool handling. Signed-off-by: Laurent Vivier Reviewed-by: David Gibson --- icmp.c | 14 +++++--------- icmp.h | 2 +- tap.c | 7 ++----- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/icmp.c b/icmp.c index 9d90141542ff..6dffafb0bf54 100644 --- a/icmp.c +++ b/icmp.c @@ -230,20 +230,19 @@ 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; @@ -253,14 +252,11 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, (void)saddr; ASSERT(pif == PIF_TAP); - if (!packet_get(p, 0, &data)) - return -1; - if (af == AF_INET) { struct icmphdr ih_storage; const struct icmphdr *ih; - ih = IOV_PEEK_HEADER(&data, ih_storage); + ih = IOV_PEEK_HEADER(data, ih_storage); if (!ih) return 1; @@ -274,7 +270,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, struct icmp6hdr ih_storage; const struct icmp6hdr *ih; - ih = IOV_PEEK_HEADER(&data, ih_storage); + ih = IOV_PEEK_HEADER(data, ih_storage); if (!ih) return 1; @@ -288,7 +284,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, ASSERT(0); } - cnt = iov_tail_clone(&iov[0], MAX_IOV_ICMP, &data); + cnt = iov_tail_clone(&iov[0], MAX_IOV_ICMP, data); if (cnt < 0) return 1; 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/tap.c b/tap.c index a340e43cde5d..1a7088a2fdaa 100644 --- a/tap.c +++ b/tap.c @@ -764,17 +764,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; } @@ -964,7 +961,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.50.1

These macros are no longer used following the refactoring of packet handlers to directly use iov_tail. Callers no longer require PACKET_POOL_P for temporary pools, and PACKET_POOL can be replaced by PACKET_POOL_DECL and separate initialization if needed. Signed-off-by: Laurent Vivier Reviewed-by: David Gibson --- packet.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packet.h b/packet.h index 7afe80ef3fcf..286b6b9994db 100644 --- a/packet.h +++ b/packet.h @@ -59,19 +59,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.50.1

This patch refactors the handling of vhost-user memory regions by introducing a new `struct vdev_memory` to encapsulate the regions array and their count (`nregions`) within the main `vu_dev` structure. This new `vdev_memory` structure is then passed to the packet pool by re-using the existing `p->buf` field. A `p->buf_size` of 0 indicates that `p->buf` holds a pointer to `struct vdev_memory` instead of a regular packet buffer. A new helper, `get_vdev_memory()`, is added to abstract this access pattern. Previous implementation was using a marker at the end of the memory regions array. We can now uses all the slots. Signed-off-by: Laurent Vivier Reviewed-by: David Gibson --- packet.c | 21 +++++++++++++++++++-- packet.h | 6 ++++-- tap.c | 4 ++-- tap.h | 1 - vhost_user.c | 28 +++++++++++----------------- virtio.c | 4 ++-- virtio.h | 18 ++++++++++++++---- vu_common.c | 22 ++++++++++++---------- 8 files changed, 64 insertions(+), 40 deletions(-) diff --git a/packet.c b/packet.c index cbc43c2fc22d..27693c55a138 100644 --- a/packet.c +++ b/packet.c @@ -22,6 +22,20 @@ #include "util.h" #include "log.h" +/** + * get_vdev_memory() - Return a pointer to the memory regions of the pool + * @p: Packet pool + * + * Return: Null if none, otherwise a pointer to vdev_memory structure + */ +static struct vdev_memory *get_vdev_memory(const struct pool *p) +{ + if (p->buf_size) + return NULL; + + return (struct vdev_memory *)p->buf; +} + /** * packet_check_range() - Check if a memory range is valid for a pool * @p: Packet pool @@ -35,16 +49,19 @@ static int packet_check_range(const struct pool *p, const char *ptr, size_t len, const char *func, int line) { + struct vdev_memory *memory; + if (len > PACKET_MAX_LEN) { debug("packet range length %zu (max %zu), %s:%i", len, PACKET_MAX_LEN, func, line); return -1; } - if (p->buf_size == 0) { + memory = get_vdev_memory(p); + if (memory) { int ret; - ret = vu_packet_check_range((void *)p->buf, ptr, len); + ret = vu_packet_check_range(memory, ptr, len); if (ret == -1) debug("cannot find region, %s:%i", func, line); diff --git a/packet.h b/packet.h index 43b9022075d1..e51cbd19fdc4 100644 --- a/packet.h +++ b/packet.h @@ -8,6 +8,7 @@ #include #include "iov.h" +#include "virtio.h" /* Maximum size of a single packet stored in pool, including headers */ #define PACKET_MAX_LEN ((size_t)UINT16_MAX) @@ -15,7 +16,7 @@ /** * struct pool - Generic pool of packets stored in a buffer * @buf: Buffer storing packet descriptors, - * a struct vu_dev_region array for passt vhost-user mode + * a struct vdev_region for passt vhost-user mode * @buf_size: Total size of buffer, * 0 for passt vhost-user mode * @size: Number of usable descriptors for the pool @@ -30,7 +31,8 @@ struct pool { 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_get_do(const struct pool *p, const size_t idx, diff --git a/tap.c b/tap.c index 51d79c2c8959..5e0173b2ce43 100644 --- a/tap.c +++ b/tap.c @@ -1460,7 +1460,7 @@ static void tap_sock_tun_init(struct ctx *c) * @base: Buffer base * @size Buffer size */ -void tap_sock_update_pool(void *base, size_t size) +static void tap_sock_update_pool(void *base, size_t size) { int i; @@ -1481,8 +1481,8 @@ void tap_sock_update_pool(void *base, size_t size) void tap_backend_init(struct ctx *c) { if (c->mode == MODE_VU) { - tap_sock_update_pool(NULL, 0); vu_init(c); + tap_sock_update_pool(&c->vdev->memory, 0); } else { tap_sock_update_pool(pkt_buf, sizeof(pkt_buf)); } diff --git a/tap.h b/tap.h index ce5510882d5d..21db4d219ecb 100644 --- a/tap.h +++ b/tap.h @@ -115,7 +115,6 @@ void tap_handler_passt(struct ctx *c, uint32_t events, const struct timespec *now); int tap_sock_unix_open(char *sock_path); void tap_sock_reset(struct ctx *c); -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 c1522d549f00..f97ec6064cac 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 = vmsg->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; } @@ -1106,8 +1100,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) */ @@ -1115,7 +1109,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 ed7842b4c78a..bd388c2dfc7f 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 32757458ea95..b55cc4042521 100644 --- a/virtio.h +++ b/virtio.h @@ -96,11 +96,22 @@ struct vu_dev_region { */ #define VHOST_USER_MAX_RAM_SLOTS 32 +/** + * struct vdev_memory - Describes the shared memory regions for a vhost-user + * device + * @nregions: Number of shared memory regions + * @regions: Guest shared memory regions + */ +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 - * @nregions: Number of shared memory regions - * @regions: Guest shared memory regions + * @memory: Shared memory regions + * @vq: Virtqueues of the device * @features: Vhost-user features * @protocol_features: Vhost-user protocol features * @log_call_fd: Eventfd to report logging update @@ -109,8 +120,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..b716070ea3c3 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: Array of the available memory regions * @ptr: Start of desired data range - * @size: Length of desired data range + * @len: 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.50.1