pcapm() captures multiple frames from a msghdr, however the only thing it cares about in the msghdr is the list of buffers, where it assumes there is one frame to capture per buffer. That's what we want for its single caller but it's not the only obvious choice here (one frame per msghdr would arguably make more sense in isolation). In addition pcapm() has logic that only makes sense in the context of the passt specific path its called from: it skips the first 4 bytes of each buffer, because those have the qemu vnet_len rather than the frame proper. Make this clearer by replacing pcapm() with pcap_multiple() which more explicitly takes one struct iovec per frame, and parameterizes how much of each buffer to skip (i.e. the offset of the frame within the buffer). Signed-off-by: David Gibson --- pcap.c | 16 ++++++++-------- pcap.h | 2 +- tcp.c | 3 ++- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/pcap.c b/pcap.c index c9ac5cf..8af9021 100644 --- a/pcap.c +++ b/pcap.c @@ -100,10 +100,12 @@ void pcap(const char *pkt, size_t len) } /** - * pcapm() - Capture multiple frames from message header to pcap file - * @mh: Pointer to sendmsg() message header buffer + * pcap_multiple() - Capture multiple frames + * @iov: Array of iovecs, one entry per frame + * @n: Number of frames to capture + * @offset: Offset of the frame within each iovec buffer */ -void pcapm(const struct msghdr *mh) +void pcap_multiple(const struct iovec *iov, unsigned int n, size_t offset) { struct timeval tv; unsigned int i; @@ -113,11 +115,9 @@ void pcapm(const struct msghdr *mh) gettimeofday(&tv, NULL); - for (i = 0; i < mh->msg_iovlen; i++) { - const struct iovec *iov = &mh->msg_iov[i]; - - pcap_frame((char *)iov->iov_base + 4, iov->iov_len - 4, &tv); - } + for (i = 0; i < n; i++) + pcap_frame((char *)iov[i].iov_base + offset, + iov[i].iov_len - offset, &tv); } /** diff --git a/pcap.h b/pcap.h index 9e1736c..eafc89b 100644 --- a/pcap.h +++ b/pcap.h @@ -7,7 +7,7 @@ #define PCAP_H void pcap(const char *pkt, size_t len); -void pcapm(const struct msghdr *mh); +void pcap_multiple(const struct iovec *iov, unsigned int n, size_t offset); void pcapmm(const struct mmsghdr *mmh, unsigned int vlen); void pcap_init(struct ctx *c); diff --git a/tcp.c b/tcp.c index cfdae06..ed65a9e 100644 --- a/tcp.c +++ b/tcp.c @@ -1468,7 +1468,8 @@ static void tcp_l2_buf_flush(struct ctx *c, struct msghdr *mh, } } *buf_used = *buf_bytes = 0; - pcapm(mh); + + pcap_multiple(mh->msg_iov, mh->msg_iovlen, sizeof(uint32_t)); } /** -- 2.38.1

tcp[46]_l2_buf_bytes keep track of the total number of bytes we have queued to send to the tap interface. tcp_l2_buf_flush_passt() uses this to determine if sendmsg() has sent all the data we requested, or whether we need to resend a trailing portion. However, the logic for finding where we're up to in the case of a short sendmsg() can equally well tell whether we've had one at all, without knowing the total number in advance. This does require an extra loop after each sendmsg(), but it's doing simple arithmetic on values we've already been accessing, and it leads to overall simpler code. tcp[46]_l2_flags_buf_bytes were being calculated, but never used for anything, so simply remove them. Signed-off-by: David Gibson --- tcp.c | 55 +++++++++++++++++++------------------------------------ 1 file changed, 19 insertions(+), 36 deletions(-) diff --git a/tcp.c b/tcp.c index fe788b4..4050ea6 100644 --- a/tcp.c +++ b/tcp.c @@ -476,7 +476,6 @@ static struct tcp4_l2_buf_t { tcp4_l2_buf[TCP_FRAMES_MEM]; static unsigned int tcp4_l2_buf_used; -static size_t tcp4_l2_buf_bytes; /** * tcp6_l2_buf_t - Pre-cooked IPv6 packet buffers for tap connections @@ -507,7 +506,6 @@ struct tcp6_l2_buf_t { tcp6_l2_buf[TCP_FRAMES_MEM]; static unsigned int tcp6_l2_buf_used; -static size_t tcp6_l2_buf_bytes; /* recvmsg()/sendmsg() data for tap */ static char tcp_buf_discard [MAX_WINDOW]; @@ -555,7 +553,6 @@ static struct tcp4_l2_flags_buf_t { tcp4_l2_flags_buf[TCP_FRAMES_MEM]; static unsigned int tcp4_l2_flags_buf_used; -static size_t tcp4_l2_flags_buf_bytes; /** * tcp6_l2_flags_buf_t - IPv6 packet buffers for segments without data (flags) @@ -585,7 +582,6 @@ static struct tcp6_l2_flags_buf_t { tcp6_l2_flags_buf[TCP_FRAMES_MEM]; static unsigned int tcp6_l2_flags_buf_used; -static size_t tcp6_l2_flags_buf_bytes; /* TCP connections */ union tcp_conn tc[TCP_MAX_CONNS]; @@ -1418,19 +1414,17 @@ static int tcp_l2_buf_write_one(struct ctx *c, const struct iovec *iov) * tcp_l2_buf_flush_passt() - Send a message on the passt tap interface * @c: Execution context * @mh: Message header that was partially sent by sendmsg() - * @buf_bytes: Total number of bytes to send */ -static void tcp_l2_buf_flush_passt(const struct ctx *c, - const struct msghdr *mh, size_t buf_bytes) +static void tcp_l2_buf_flush_passt(const struct ctx *c, const struct msghdr *mh) { - size_t end = 0, missing, sent; + size_t end = 0, missing; struct iovec *iov; unsigned int i; - ssize_t n; + ssize_t sent; char *p; - n = sendmsg(c->fd_tap, mh, MSG_NOSIGNAL | MSG_DONTWAIT); - if (n < 0 || ((sent = (size_t)n) == buf_bytes)) + sent = sendmsg(c->fd_tap, mh, MSG_NOSIGNAL | MSG_DONTWAIT); + if (sent < 0) return; /* Ensure a complete last message on partial sendmsg() */ @@ -1441,6 +1435,9 @@ static void tcp_l2_buf_flush_passt(const struct ctx *c, } missing = end - sent; + if (!missing) + return; + p = (char *)iov->iov_base + iov->iov_len - missing; if (send(c->fd_tap, p, missing, MSG_NOSIGNAL)) debug("TCP: failed to flush %lu missing bytes to tap", missing); @@ -1451,19 +1448,18 @@ static void tcp_l2_buf_flush_passt(const struct ctx *c, * @c: Execution context * @mh: Message header pointing to buffers, msg_iovlen not set * @buf_used: Pointer to count of used buffers, set to 0 on return - * @buf_bytes: Pointer to count of buffer bytes, set to 0 on return */ static void tcp_l2_buf_flush(struct ctx *c, struct msghdr *mh, - unsigned int *buf_used, size_t *buf_bytes) + unsigned int *buf_used) { + size_t i; + if (!(mh->msg_iovlen = *buf_used)) return; if (c->mode == MODE_PASST) { - tcp_l2_buf_flush_passt(c, mh, *buf_bytes); + tcp_l2_buf_flush_passt(c, mh); } else { - size_t i; - for (i = 0; i < mh->msg_iovlen; i++) { struct iovec *iov = &mh->msg_iov[i]; @@ -1471,7 +1467,7 @@ static void tcp_l2_buf_flush(struct ctx *c, struct msghdr *mh, i--; } } - *buf_used = *buf_bytes = 0; + *buf_used = 0; pcap_multiple(mh->msg_iov, mh->msg_iovlen, sizeof(uint32_t)); } @@ -1484,17 +1480,14 @@ static void tcp_l2_flags_buf_flush(struct ctx *c) { struct msghdr mh = { 0 }; unsigned int *buf_used; - size_t *buf_bytes; mh.msg_iov = tcp6_l2_flags_iov; buf_used = &tcp6_l2_flags_buf_used; - buf_bytes = &tcp6_l2_flags_buf_bytes; - tcp_l2_buf_flush(c, &mh, buf_used, buf_bytes); + tcp_l2_buf_flush(c, &mh, buf_used); mh.msg_iov = tcp4_l2_flags_iov; buf_used = &tcp4_l2_flags_buf_used; - buf_bytes = &tcp4_l2_flags_buf_bytes; - tcp_l2_buf_flush(c, &mh, buf_used, buf_bytes); + tcp_l2_buf_flush(c, &mh, buf_used); } /** @@ -1505,17 +1498,14 @@ static void tcp_l2_data_buf_flush(struct ctx *c) { struct msghdr mh = { 0 }; unsigned int *buf_used; - size_t *buf_bytes; mh.msg_iov = tcp6_l2_iov; buf_used = &tcp6_l2_buf_used; - buf_bytes = &tcp6_l2_buf_bytes; - tcp_l2_buf_flush(c, &mh, buf_used, buf_bytes); + tcp_l2_buf_flush(c, &mh, buf_used); mh.msg_iov = tcp4_l2_iov; buf_used = &tcp4_l2_buf_used; - buf_bytes = &tcp4_l2_buf_bytes; - tcp_l2_buf_flush(c, &mh, buf_used, buf_bytes); + tcp_l2_buf_flush(c, &mh, buf_used); } /** @@ -1829,11 +1819,6 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags) NULL, conn->seq_to_tap); iov->iov_len = eth_len + sizeof(uint32_t); - if (CONN_V4(conn)) - tcp4_l2_flags_buf_bytes += iov->iov_len; - else - tcp6_l2_flags_buf_bytes += iov->iov_len; - if (th->ack) conn_flag(c, conn, ~ACK_TO_TAP_DUE); @@ -1849,7 +1834,6 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags) memcpy(b4 + 1, b4, sizeof(*b4)); (iov + 1)->iov_len = iov->iov_len; tcp4_l2_flags_buf_used++; - tcp4_l2_flags_buf_bytes += iov->iov_len; } if (tcp4_l2_flags_buf_used > ARRAY_SIZE(tcp4_l2_flags_buf) - 2) @@ -1859,7 +1843,6 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags) memcpy(b6 + 1, b6, sizeof(*b6)); (iov + 1)->iov_len = iov->iov_len; tcp6_l2_flags_buf_used++; - tcp6_l2_flags_buf_bytes += iov->iov_len; } if (tcp6_l2_flags_buf_used > ARRAY_SIZE(tcp6_l2_flags_buf) - 2) @@ -2203,7 +2186,7 @@ static void tcp_data_to_tap(struct ctx *c, struct tcp_tap_conn *conn, len = tcp_l2_buf_fill_headers(c, conn, b, plen, check, seq); iov = tcp4_l2_iov + tcp4_l2_buf_used++; - tcp4_l2_buf_bytes += iov->iov_len = len + sizeof(b->vnet_len); + iov->iov_len = len + sizeof(b->vnet_len); if (tcp4_l2_buf_used > ARRAY_SIZE(tcp4_l2_buf) - 1) tcp_l2_data_buf_flush(c); } else if (CONN_V6(conn)) { @@ -2212,7 +2195,7 @@ static void tcp_data_to_tap(struct ctx *c, struct tcp_tap_conn *conn, len = tcp_l2_buf_fill_headers(c, conn, b, plen, NULL, seq); iov = tcp6_l2_iov + tcp6_l2_buf_used++; - tcp6_l2_buf_bytes += iov->iov_len = len + sizeof(b->vnet_len); + iov->iov_len = len + sizeof(b->vnet_len); if (tcp6_l2_buf_used > ARRAY_SIZE(tcp6_l2_buf) - 1) tcp_l2_data_buf_flush(c); } -- 2.38.1

tcp_l2_buf_flush() open codes the loop across each frame in a group, but but calls tcp_l2_buf_write_one() to send each frame to the pasta tuntap device. Combine these two pasta-specific operations into tcp_l2_buf_flush_pasta() which is a little cleaner and will enable further cleanups. Signed-off-by: David Gibson --- tcp.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/tcp.c b/tcp.c index d96122d..9960a35 100644 --- a/tcp.c +++ b/tcp.c @@ -1391,23 +1391,25 @@ static void tcp_rst_do(struct ctx *c, struct tcp_tap_conn *conn); } while (0) /** - * tcp_l2_buf_write_one() - Write a single buffer to tap file descriptor + * tcp_l2_buf_flush_pasta() - Send frames on the pasta tap interface * @c: Execution context - * @iov: struct iovec item pointing to buffer - * @ts: Current timestamp - * - * Return: 0 on success, negative error code on failure (tap reset possible) + * @iov: Pointer to array of buffers, one per frame + * @n: Number of buffers/frames to flush */ -static int tcp_l2_buf_write_one(struct ctx *c, const struct iovec *iov) +static void tcp_l2_buf_flush_pasta(struct ctx *c, + const struct iovec *iov, size_t n) { - if (write(c->fd_tap, (char *)iov->iov_base + 4, iov->iov_len - 4) < 0) { - debug("tap write: %s", strerror(errno)); - if (errno != EAGAIN && errno != EWOULDBLOCK) - tap_handler(c, c->fd_tap, EPOLLERR, NULL); - return -errno; - } + size_t i; - return 0; + for (i = 0; i < n; i++) { + if (write(c->fd_tap, (char *)iov->iov_base + 4, + iov->iov_len - 4) < 0) { + debug("tap write: %s", strerror(errno)); + if (errno != EAGAIN && errno != EWOULDBLOCK) + tap_handler(c, c->fd_tap, EPOLLERR, NULL); + i--; + } + } } /** @@ -1454,19 +1456,13 @@ static void tcp_l2_buf_flush_passt(const struct ctx *c, */ static void tcp_l2_buf_flush(struct ctx *c, const struct iovec *iov, size_t n) { - size_t i; - if (!n) return; - if (c->mode == MODE_PASST) { + if (c->mode == MODE_PASST) tcp_l2_buf_flush_passt(c, iov, n); - } else { - for (i = 0; i < n; i++) { - if (tcp_l2_buf_write_one(c, iov + i)) - i--; - } - } + else + tcp_l2_buf_flush_pasta(c, iov, n); pcap_multiple(iov, n, sizeof(uint32_t)); } -- 2.38.1

We have several places where we have fairly ugly #ifdefs on __BYTE_ORDER where we need network order values in a constant expression (so we can't use htons() or htonl()). We can do this more cleanly by using a single __BYTE_ORDER ifdef to define htons_constant() and htonl_constant() macros, then using those in all the other places. Signed-off-by: David Gibson --- dhcpv6.c | 50 +++++++++++++------------------------------------- util.h | 37 +++++++++++-------------------------- 2 files changed, 24 insertions(+), 63 deletions(-) diff --git a/dhcpv6.c b/dhcpv6.c index e763aed..d0e6c7f 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -39,37 +39,21 @@ */ struct opt_hdr { uint16_t t; -#if __BYTE_ORDER == __BIG_ENDIAN -# define OPT_CLIENTID 1 -# define OPT_SERVERID 2 -# define OPT_IA_NA 3 -# define OPT_IA_TA 4 -# define OPT_IAAADR 5 -# define OPT_STATUS_CODE 13 -# define STATUS_NOTONLINK 4 -# define OPT_DNS_SERVERS 23 -# define OPT_DNS_SEARCH 24 -#else -# define OPT_CLIENTID __bswap_constant_16(1) -# define OPT_SERVERID __bswap_constant_16(2) -# define OPT_IA_NA __bswap_constant_16(3) -# define OPT_IA_TA __bswap_constant_16(4) -# define OPT_IAAADR __bswap_constant_16(5) -# define OPT_STATUS_CODE __bswap_constant_16(13) -# define STATUS_NOTONLINK __bswap_constant_16(4) -# define OPT_DNS_SERVERS __bswap_constant_16(23) -# define OPT_DNS_SEARCH __bswap_constant_16(24) -#endif +# define OPT_CLIENTID htons_constant(1) +# define OPT_SERVERID htons_constant(2) +# define OPT_IA_NA htons_constant(3) +# define OPT_IA_TA htons_constant(4) +# define OPT_IAAADR htons_constant(5) +# define OPT_STATUS_CODE htons_constant(13) +# define STATUS_NOTONLINK htons_constant(4) +# define OPT_DNS_SERVERS htons_constant(23) +# define OPT_DNS_SEARCH htons_constant(24) #define STR_NOTONLINK "Prefix not appropriate for link." uint16_t l; } __attribute__((packed)); -#if __BYTE_ORDER == __BIG_ENDIAN -# define OPT_SIZE_CONV(x) (x) -#else -# define OPT_SIZE_CONV(x) (__bswap_constant_16(x)) -#endif +# 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) - \ @@ -103,19 +87,11 @@ struct opt_server_id { uint8_t duid_lladdr[ETH_ALEN]; } __attribute__ ((packed)); -#if __BYTE_ORDER == __BIG_ENDIAN -#define SERVER_ID { \ +#define SERVER_ID { \ { OPT_SERVERID, OPT_SIZE(server_id) }, \ - DUID_TYPE_LLT, ARPHRD_ETHER, 0, { 0 } \ + htons_constant(DUID_TYPE_LLT), \ + htons_constant(ARPHRD_ETHER), 0, { 0 } \ } -#else -#define SERVER_ID { \ - { OPT_SERVERID, OPT_SIZE(server_id) }, \ - __bswap_constant_16(DUID_TYPE_LLT), \ - __bswap_constant_16(ARPHRD_ETHER), \ - 0, { 0 } \ -} -#endif /** * struct opt_ia_na - Identity Association for Non-temporary Addresses Option diff --git a/util.h b/util.h index 3baec91..b4c139d 100644 --- a/util.h +++ b/util.h @@ -69,6 +69,14 @@ #define MAC_ZERO ((uint8_t [ETH_ALEN]){ 0 }) #define MAC_IS_ZERO(addr) (!memcmp((addr), MAC_ZERO, ETH_ALEN)) +#if __BYTE_ORDER == __BIG_ENDIAN +#define htons_constant(x) (x) +#define htonl_constant(x) (x) +#else +#define htons_constant(x) (__bswap_constant_16(x)) +#define htonl_constant(x) (__bswap_constant_32(x)) +#endif + #define IN4_IS_ADDR_UNSPECIFIED(a) \ ((a)->s_addr == htonl(INADDR_ANY)) #define IN4_IS_ADDR_BROADCAST(a) \ @@ -79,13 +87,8 @@ (IN_MULTICAST(ntohl((a)->s_addr))) #define IN4_ARE_ADDR_EQUAL(a, b) \ (((struct in_addr *)(a))->s_addr == ((struct in_addr *)b)->s_addr) -#if __BYTE_ORDER == __BIG_ENDIAN -#define IN4ADDR_LOOPBACK_INIT \ - { .s_addr = INADDR_LOOPBACK } -#else #define IN4ADDR_LOOPBACK_INIT \ - { .s_addr = __bswap_constant_32(INADDR_LOOPBACK) } -#endif + { .s_addr = htonl_constant(INADDR_LOOPBACK) } #define NS_FN_STACK_SIZE (RLIMIT_STACK_VAL * 1024 / 8) int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags, @@ -99,37 +102,19 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags, (void *)(arg)); \ } while (0) -#if __BYTE_ORDER == __BIG_ENDIAN -#define L2_BUF_ETH_IP4_INIT \ - { \ - .h_dest = { 0 }, \ - .h_source = { 0 }, \ - .h_proto = ETH_P_IP, \ - } -#else #define L2_BUF_ETH_IP4_INIT \ { \ .h_dest = { 0 }, \ .h_source = { 0 }, \ - .h_proto = __bswap_constant_16(ETH_P_IP), \ + .h_proto = htons_constant(ETH_P_IP), \ } -#endif -#if __BYTE_ORDER == __BIG_ENDIAN -#define L2_BUF_ETH_IP6_INIT \ - { \ - .h_dest = { 0 }, \ - .h_source = { 0 }, \ - .h_proto = ETH_P_IPV6, \ - } -#else #define L2_BUF_ETH_IP6_INIT \ { \ .h_dest = { 0 }, \ .h_source = { 0 }, \ - .h_proto = __bswap_constant_16(ETH_P_IPV6), \ + .h_proto = htons_constant(ETH_P_IPV6), \ } -#endif #define L2_BUF_IP4_INIT(proto) \ { \ -- 2.38.1

We have separate IPv4 and IPv6 versions of a macro to construct an initializer for ethernet headers. However, now that we have htons_constant it's easy to simply paramterize this with the ethernet protocol number. Signed-off-by: David Gibson --- tcp.c | 8 ++++---- udp.c | 4 ++-- util.h | 11 ++--------- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/tcp.c b/tcp.c index 79fc733..b30e8e6 100644 --- a/tcp.c +++ b/tcp.c @@ -1059,7 +1059,7 @@ static void tcp_sock4_iov_init(void) for (i = 0; i < ARRAY_SIZE(tcp4_l2_buf); i++) { tcp4_l2_buf[i] = (struct tcp4_l2_buf_t) { - .eh = L2_BUF_ETH_IP4_INIT, + .eh = L2_BUF_ETH_INIT(ETH_P_IP), .iph = L2_BUF_IP4_INIT(IPPROTO_TCP), .th = { .doff = sizeof(struct tcphdr) / 4, .ack = 1 } }; @@ -1067,7 +1067,7 @@ static void tcp_sock4_iov_init(void) for (i = 0; i < ARRAY_SIZE(tcp4_l2_flags_buf); i++) { tcp4_l2_flags_buf[i] = (struct tcp4_l2_flags_buf_t) { - .eh = L2_BUF_ETH_IP4_INIT, + .eh = L2_BUF_ETH_INIT(ETH_P_IP), .iph = L2_BUF_IP4_INIT(IPPROTO_TCP) }; } @@ -1091,7 +1091,7 @@ static void tcp_sock6_iov_init(void) for (i = 0; i < ARRAY_SIZE(tcp6_l2_buf); i++) { tcp6_l2_buf[i] = (struct tcp6_l2_buf_t) { - .eh = L2_BUF_ETH_IP6_INIT, + .eh = L2_BUF_ETH_INIT(ETH_P_IPV6), .ip6h = L2_BUF_IP6_INIT(IPPROTO_TCP), .th = { .doff = sizeof(struct tcphdr) / 4, .ack = 1 } }; @@ -1099,7 +1099,7 @@ static void tcp_sock6_iov_init(void) for (i = 0; i < ARRAY_SIZE(tcp6_l2_flags_buf); i++) { tcp6_l2_flags_buf[i] = (struct tcp6_l2_flags_buf_t) { - .eh = L2_BUF_ETH_IP6_INIT, + .eh = L2_BUF_ETH_INIT(ETH_P_IPV6), .ip6h = L2_BUF_IP6_INIT(IPPROTO_TCP) }; } diff --git a/udp.c b/udp.c index ffedca1..5609836 100644 --- a/udp.c +++ b/udp.c @@ -329,7 +329,7 @@ static void udp_sock4_iov_init(const struct ctx *c) for (i = 0; i < ARRAY_SIZE(udp4_l2_buf); i++) { udp4_l2_buf[i] = (struct udp4_l2_buf_t) { - .eh = L2_BUF_ETH_IP4_INIT, + .eh = L2_BUF_ETH_INIT(ETH_P_IP), .iph = L2_BUF_IP4_INIT(IPPROTO_UDP) }; } @@ -370,7 +370,7 @@ static void udp_sock6_iov_init(const struct ctx *c) for (i = 0; i < ARRAY_SIZE(udp6_l2_buf); i++) { udp6_l2_buf[i] = (struct udp6_l2_buf_t) { - .eh = L2_BUF_ETH_IP6_INIT, + .eh = L2_BUF_ETH_INIT(ETH_P_IPV6), .ip6h = L2_BUF_IP6_INIT(IPPROTO_UDP) }; } diff --git a/util.h b/util.h index b4c139d..b5be7a6 100644 --- a/util.h +++ b/util.h @@ -102,18 +102,11 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags, (void *)(arg)); \ } while (0) -#define L2_BUF_ETH_IP4_INIT \ +#define L2_BUF_ETH_INIT(proto) \ { \ .h_dest = { 0 }, \ .h_source = { 0 }, \ - .h_proto = htons_constant(ETH_P_IP), \ - } - -#define L2_BUF_ETH_IP6_INIT \ - { \ - .h_dest = { 0 }, \ - .h_source = { 0 }, \ - .h_proto = htons_constant(ETH_P_IPV6), \ + .h_proto = htons_constant(proto), \ } #define L2_BUF_IP4_INIT(proto) \ -- 2.38.1

tcp_l2_buf_fill_headers() returns the size of the generated frame including the ethernet header. The caller then adds on the size of the vnet_len field to get the total frame size to be passed to the tap device. Outside the tap code, though, we never care about the ethernet header size only the final total size we need to put into an iovec. So, consolidate the total frame size calculation within tcp_l2_buf_fill_headers(). Signed-off-by: David Gibson --- tcp.c | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/tcp.c b/tcp.c index 0920c2a..8bcb039 100644 --- a/tcp.c +++ b/tcp.c @@ -1447,7 +1447,7 @@ void tcp_defer_handler(struct ctx *c) * @check: Checksum, if already known * @seq: Sequence number for this segment * - * Return: 802.3 length, host order + * Return: frame length including L2 headers, host order */ static size_t tcp_l2_buf_fill_headers(const struct ctx *c, const struct tcp_tap_conn *conn, @@ -1455,7 +1455,7 @@ static size_t tcp_l2_buf_fill_headers(const struct ctx *c, const uint16_t *check, uint32_t seq) { const struct in_addr *a4 = inany_v4(&conn->addr); - size_t ip_len, eth_len; + size_t ip_len, tlen; #define SET_TCP_HEADER_COMMON_V4_V6(b, conn, seq) \ do { \ @@ -1489,9 +1489,10 @@ do { \ tcp_update_check_tcp4(b); - eth_len = ip_len + sizeof(struct ethhdr); + tlen = ip_len + sizeof(struct ethhdr); if (c->mode == MODE_PASST) - b->vnet_len = htonl(eth_len); + b->vnet_len = htonl(tlen); + tlen += sizeof(b->vnet_len); } else { struct tcp6_l2_buf_t *b = (struct tcp6_l2_buf_t *)p; @@ -1514,14 +1515,14 @@ do { \ b->ip6h.flow_lbl[1] = (conn->sock >> 8) & 0xff; b->ip6h.flow_lbl[2] = (conn->sock >> 0) & 0xff; - eth_len = ip_len + sizeof(struct ethhdr); + tlen = ip_len + sizeof(struct ethhdr); if (c->mode == MODE_PASST) - b->vnet_len = htonl(eth_len); + b->vnet_len = htonl(tlen); + tlen += sizeof(b->vnet_len); } - #undef SET_TCP_HEADER_COMMON_V4_V6 - return eth_len; + return tlen; } /** @@ -1627,8 +1628,8 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags) struct tcp6_l2_flags_buf_t *b6 = NULL; struct tcp_info tinfo = { 0 }; socklen_t sl = sizeof(tinfo); - size_t optlen = 0, eth_len; int s = conn->sock; + size_t optlen = 0; struct iovec *iov; struct tcphdr *th; char *data; @@ -1717,9 +1718,8 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags) th->syn = !!(flags & SYN); th->fin = !!(flags & FIN); - eth_len = tcp_l2_buf_fill_headers(c, conn, p, optlen, - NULL, conn->seq_to_tap); - iov->iov_len = eth_len + sizeof(uint32_t); + iov->iov_len = tcp_l2_buf_fill_headers(c, conn, p, optlen, + NULL, conn->seq_to_tap); if (th->ack) conn_flag(c, conn, ~ACK_TO_TAP_DUE); @@ -2079,25 +2079,22 @@ static void tcp_data_to_tap(struct ctx *c, struct tcp_tap_conn *conn, ssize_t plen, int no_csum, uint32_t seq) { struct iovec *iov; - size_t len; if (CONN_V4(conn)) { struct tcp4_l2_buf_t *b = &tcp4_l2_buf[tcp4_l2_buf_used]; uint16_t *check = no_csum ? &(b - 1)->iph.check : NULL; - len = tcp_l2_buf_fill_headers(c, conn, b, plen, check, seq); - iov = tcp4_l2_iov + tcp4_l2_buf_used++; - iov->iov_len = len + sizeof(b->vnet_len); + iov->iov_len = tcp_l2_buf_fill_headers(c, conn, b, plen, + check, seq); if (tcp4_l2_buf_used > ARRAY_SIZE(tcp4_l2_buf) - 1) tcp_l2_data_buf_flush(c); } else if (CONN_V6(conn)) { struct tcp6_l2_buf_t *b = &tcp6_l2_buf[tcp6_l2_buf_used]; - len = tcp_l2_buf_fill_headers(c, conn, b, plen, NULL, seq); - iov = tcp6_l2_iov + tcp6_l2_buf_used++; - iov->iov_len = len + sizeof(b->vnet_len); + iov->iov_len = tcp_l2_buf_fill_headers(c, conn, b, plen, + NULL, seq); if (tcp6_l2_buf_used > ARRAY_SIZE(tcp6_l2_buf) - 1) tcp_l2_data_buf_flush(c); } -- 2.38.1

Update the TCP code to use the tap layer abstractions for initializing and updating the L2 and lower headers. This will make adding other tap backends in future easier. Signed-off-by: David Gibson --- tcp.c | 85 +++++++++++++++++++++-------------------------------------- 1 file changed, 30 insertions(+), 55 deletions(-) diff --git a/tcp.c b/tcp.c index 8bcb039..26037b3 100644 --- a/tcp.c +++ b/tcp.c @@ -330,8 +330,7 @@ struct tcp4_l2_head { /* For MSS4 macro: keep in sync with tcp4_l2_buf_t */ #else uint8_t pad[2]; #endif - uint32_t vnet_len; - struct ethhdr eh; + struct tap_hdr taph; struct iphdr iph; struct tcphdr th; #ifdef __AVX2__ @@ -346,8 +345,7 @@ struct tcp6_l2_head { /* For MSS6 macro: keep in sync with tcp6_l2_buf_t */ #else uint8_t pad[2]; #endif - uint32_t vnet_len; - struct ethhdr eh; + struct tap_hdr taph; struct ipv6hdr ip6h; struct tcphdr th; #ifdef __AVX2__ @@ -448,8 +446,7 @@ static union inany_addr low_rtt_dst[LOW_RTT_TABLE_SIZE]; * @psum: Partial IP header checksum (excluding tot_len and saddr) * @tsum: Partial TCP header checksum (excluding length and saddr) * @pad: Align TCP header to 32 bytes, for AVX2 checksum calculation only - * @vnet_len: 4-byte qemu vnet buffer length descriptor, only for passt mode - * @eh: Pre-filled Ethernet header + * @taph: Tap-level headers (partially pre-filled) * @iph: Pre-filled IP header (except for tot_len and saddr) * @uh: Headroom for TCP header * @data: Storage for TCP payload @@ -462,8 +459,7 @@ static struct tcp4_l2_buf_t { #else uint8_t pad[2]; /* align iph to 4 bytes 8 */ #endif - uint32_t vnet_len; /* 26 10 */ - struct ethhdr eh; /* 30 14 */ + struct tap_hdr taph; /* 26 10 */ struct iphdr iph; /* 44 28 */ struct tcphdr th; /* 64 48 */ uint8_t data[MSS4]; /* 84 68 */ @@ -480,8 +476,7 @@ static unsigned int tcp4_l2_buf_used; /** * tcp6_l2_buf_t - Pre-cooked IPv6 packet buffers for tap connections * @pad: Align IPv6 header for checksum calculation to 32B (AVX2) or 4B - * @vnet_len: 4-byte qemu vnet buffer length descriptor, only for passt mode - * @eh: Pre-filled Ethernet header + * @taph: Tap-level headers (partially pre-filled) * @ip6h: Pre-filled IP header (except for payload_len and addresses) * @th: Headroom for TCP header * @data: Storage for TCP payload @@ -492,8 +487,7 @@ struct tcp6_l2_buf_t { #else uint8_t pad[2]; /* align ip6h to 4 bytes 0 */ #endif - uint32_t vnet_len; /* 14 2 */ - struct ethhdr eh; /* 18 6 */ + struct tap_hdr taph; /* 14 2 */ struct ipv6hdr ip6h; /* 32 20 */ struct tcphdr th; /* 72 60 */ uint8_t data[MSS6]; /* 92 80 */ @@ -526,8 +520,7 @@ static struct iovec tcp_iov [UIO_MAXIOV]; * @psum: Partial IP header checksum (excluding tot_len and saddr) * @tsum: Partial TCP header checksum (excluding length and saddr) * @pad: Align TCP header to 32 bytes, for AVX2 checksum calculation only - * @vnet_len: 4-byte qemu vnet buffer length descriptor, only for passt mode - * @eh: Pre-filled Ethernet header + * @taph: Tap-level headers (partially pre-filled) * @iph: Pre-filled IP header (except for tot_len and saddr) * @th: Headroom for TCP header * @opts: Headroom for TCP options @@ -540,8 +533,7 @@ static struct tcp4_l2_flags_buf_t { #else uint8_t pad[2]; /* align iph to 4 bytes 8 */ #endif - uint32_t vnet_len; /* 26 10 */ - struct ethhdr eh; /* 30 14 */ + struct tap_hdr taph; /* 26 10 */ struct iphdr iph; /* 44 28 */ struct tcphdr th; /* 64 48 */ char opts[OPT_MSS_LEN + OPT_WS_LEN + 1]; @@ -557,8 +549,7 @@ static unsigned int tcp4_l2_flags_buf_used; /** * tcp6_l2_flags_buf_t - IPv6 packet buffers for segments without data (flags) * @pad: Align IPv6 header for checksum calculation to 32B (AVX2) or 4B - * @vnet_len: 4-byte qemu vnet buffer length descriptor, only for passt mode - * @eh: Pre-filled Ethernet header + * @taph: Tap-level headers (partially pre-filled) * @ip6h: Pre-filled IP header (except for payload_len and addresses) * @th: Headroom for TCP header * @opts: Headroom for TCP options @@ -569,8 +560,7 @@ static struct tcp6_l2_flags_buf_t { #else uint8_t pad[2]; /* align ip6h to 4 bytes 0 */ #endif - uint32_t vnet_len; /* 14 2 */ - struct ethhdr eh; /* 18 6 */ + struct tap_hdr taph; /* 14 2 */ struct ipv6hdr ip6h; /* 32 20 */ struct tcphdr th /* 72 */ __attribute__ ((aligned(4))); /* 60 */ char opts[OPT_MSS_LEN + OPT_WS_LEN + 1]; @@ -1013,21 +1003,10 @@ void tcp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s, struct tcp4_l2_buf_t *b4 = &tcp4_l2_buf[i]; struct tcp6_l2_buf_t *b6 = &tcp6_l2_buf[i]; - if (eth_d) { - memcpy(b4->eh.h_dest, eth_d, ETH_ALEN); - memcpy(b6->eh.h_dest, eth_d, ETH_ALEN); - - memcpy(b4f->eh.h_dest, eth_d, ETH_ALEN); - memcpy(b6f->eh.h_dest, eth_d, ETH_ALEN); - } - - if (eth_s) { - memcpy(b4->eh.h_source, eth_s, ETH_ALEN); - memcpy(b6->eh.h_source, eth_s, ETH_ALEN); - - memcpy(b4f->eh.h_source, eth_s, ETH_ALEN); - memcpy(b6f->eh.h_source, eth_s, ETH_ALEN); - } + tap_update_mac(&b4->taph, eth_d, eth_s); + tap_update_mac(&b6->taph, eth_d, eth_s); + tap_update_mac(&b4f->taph, eth_d, eth_s); + tap_update_mac(&b6f->taph, eth_d, eth_s); if (ip_da) { b4f->iph.daddr = b4->iph.daddr = ip_da->s_addr; @@ -1051,15 +1030,16 @@ void tcp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s, /** * tcp_sock4_iov_init() - Initialise scatter-gather L2 buffers for IPv4 sockets + * @c: Execution context */ -static void tcp_sock4_iov_init(void) +static void tcp_sock4_iov_init(const struct ctx *c) { struct iovec *iov; int i; for (i = 0; i < ARRAY_SIZE(tcp4_l2_buf); i++) { tcp4_l2_buf[i] = (struct tcp4_l2_buf_t) { - .eh = L2_BUF_ETH_INIT(ETH_P_IP), + .taph = TAP_HDR_INIT(ETH_P_IP), .iph = L2_BUF_IP4_INIT(IPPROTO_TCP), .th = { .doff = sizeof(struct tcphdr) / 4, .ack = 1 } }; @@ -1067,29 +1047,30 @@ static void tcp_sock4_iov_init(void) for (i = 0; i < ARRAY_SIZE(tcp4_l2_flags_buf); i++) { tcp4_l2_flags_buf[i] = (struct tcp4_l2_flags_buf_t) { - .eh = L2_BUF_ETH_INIT(ETH_P_IP), + .taph = TAP_HDR_INIT(ETH_P_IP), .iph = L2_BUF_IP4_INIT(IPPROTO_TCP) }; } for (i = 0, iov = tcp4_l2_iov; i < TCP_FRAMES_MEM; i++, iov++) - iov->iov_base = &tcp4_l2_buf[i].vnet_len; + iov->iov_base = tap_iov_base(c, &tcp4_l2_buf[i].taph); for (i = 0, iov = tcp4_l2_flags_iov; i < TCP_FRAMES_MEM; i++, iov++) - iov->iov_base = &tcp4_l2_flags_buf[i].vnet_len; + iov->iov_base = tap_iov_base(c, &tcp4_l2_flags_buf[i].taph); } /** * tcp_sock6_iov_init() - Initialise scatter-gather L2 buffers for IPv6 sockets + * @c: Execution context */ -static void tcp_sock6_iov_init(void) +static void tcp_sock6_iov_init(const struct ctx *c) { struct iovec *iov; int i; for (i = 0; i < ARRAY_SIZE(tcp6_l2_buf); i++) { tcp6_l2_buf[i] = (struct tcp6_l2_buf_t) { - .eh = L2_BUF_ETH_INIT(ETH_P_IPV6), + .taph = TAP_HDR_INIT(ETH_P_IPV6), .ip6h = L2_BUF_IP6_INIT(IPPROTO_TCP), .th = { .doff = sizeof(struct tcphdr) / 4, .ack = 1 } }; @@ -1097,16 +1078,16 @@ static void tcp_sock6_iov_init(void) for (i = 0; i < ARRAY_SIZE(tcp6_l2_flags_buf); i++) { tcp6_l2_flags_buf[i] = (struct tcp6_l2_flags_buf_t) { - .eh = L2_BUF_ETH_INIT(ETH_P_IPV6), + .taph = TAP_HDR_INIT(ETH_P_IPV6), .ip6h = L2_BUF_IP6_INIT(IPPROTO_TCP) }; } for (i = 0, iov = tcp6_l2_iov; i < TCP_FRAMES_MEM; i++, iov++) - iov->iov_base = &tcp6_l2_buf[i].vnet_len; + iov->iov_base = tap_iov_base(c, &tcp6_l2_buf[i].taph); for (i = 0, iov = tcp6_l2_flags_iov; i < TCP_FRAMES_MEM; i++, iov++) - iov->iov_base = &tcp6_l2_flags_buf[i].vnet_len; + iov->iov_base = tap_iov_base(c, &tcp6_l2_flags_buf[i].taph); } /** @@ -1489,10 +1470,7 @@ do { \ tcp_update_check_tcp4(b); - tlen = ip_len + sizeof(struct ethhdr); - if (c->mode == MODE_PASST) - b->vnet_len = htonl(tlen); - tlen += sizeof(b->vnet_len); + tlen = tap_iov_len(c, &b->taph, ip_len); } else { struct tcp6_l2_buf_t *b = (struct tcp6_l2_buf_t *)p; @@ -1515,10 +1493,7 @@ do { \ b->ip6h.flow_lbl[1] = (conn->sock >> 8) & 0xff; b->ip6h.flow_lbl[2] = (conn->sock >> 0) & 0xff; - tlen = ip_len + sizeof(struct ethhdr); - if (c->mode == MODE_PASST) - b->vnet_len = htonl(tlen); - tlen += sizeof(b->vnet_len); + tlen = tap_iov_len(c, &b->taph, ip_len); } #undef SET_TCP_HEADER_COMMON_V4_V6 @@ -3113,10 +3088,10 @@ int tcp_init(struct ctx *c) tcp_l2_mh[i] = (struct mmsghdr) { .msg_hdr.msg_iovlen = 1 }; if (c->ifi4) - tcp_sock4_iov_init(); + tcp_sock4_iov_init(c); if (c->ifi6) - tcp_sock6_iov_init(); + tcp_sock6_iov_init(c); memset(init_sock_pool4, 0xff, sizeof(init_sock_pool4)); memset(init_sock_pool6, 0xff, sizeof(init_sock_pool6)); -- 2.38.1

Update the UDP code to use the tap layer abstractions for initializing and updating the L2 and lower headers. This will make adding other tap backends in future easier. Signed-off-by: David Gibson --- udp.c | 79 ++++++++++++++++++---------------------------------------- util.h | 7 ------ 2 files changed, 24 insertions(+), 62 deletions(-) diff --git a/udp.c b/udp.c index 5609836..910f15f 100644 --- a/udp.c +++ b/udp.c @@ -169,8 +169,7 @@ static uint8_t udp_act[IP_VERSIONS][UDP_ACT_TYPE_MAX][DIV_ROUND_UP(NUM_PORTS, 8) * udp4_l2_buf_t - Pre-cooked IPv4 packet buffers for tap connections * @s_in: Source socket address, filled in by recvmmsg() * @psum: Partial IP header checksum (excluding tot_len and saddr) - * @vnet_len: 4-byte qemu vnet buffer length descriptor, only for passt mode - * @eh: Pre-filled Ethernet header + * @taph: Tap-level headers (partially pre-filled) * @iph: Pre-filled IP header (except for tot_len and saddr) * @uh: Headroom for UDP header * @data: Storage for UDP payload @@ -179,8 +178,7 @@ static struct udp4_l2_buf_t { struct sockaddr_in s_in; uint32_t psum; - uint32_t vnet_len; - struct ethhdr eh; + struct tap_hdr taph; struct iphdr iph; struct udphdr uh; uint8_t data[USHRT_MAX - @@ -191,8 +189,7 @@ udp4_l2_buf[UDP_MAX_FRAMES]; /** * udp6_l2_buf_t - Pre-cooked IPv6 packet buffers for tap connections * @s_in6: Source socket address, filled in by recvmmsg() - * @vnet_len: 4-byte qemu vnet buffer length descriptor, only for passt mode - * @eh: Pre-filled Ethernet header + * @taph: Tap-level headers (partially pre-filled) * @ip6h: Pre-filled IP header (except for payload_len and addresses) * @uh: Headroom for UDP header * @data: Storage for UDP payload @@ -205,8 +202,7 @@ struct udp6_l2_buf_t { sizeof(uint32_t))]; #endif - uint32_t vnet_len; - struct ethhdr eh; + struct tap_hdr taph; struct ipv6hdr ip6h; struct udphdr uh; uint8_t data[USHRT_MAX - @@ -294,15 +290,8 @@ void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s, struct udp4_l2_buf_t *b4 = &udp4_l2_buf[i]; struct udp6_l2_buf_t *b6 = &udp6_l2_buf[i]; - if (eth_d) { - memcpy(b4->eh.h_dest, eth_d, ETH_ALEN); - memcpy(b6->eh.h_dest, eth_d, ETH_ALEN); - } - - if (eth_s) { - memcpy(b4->eh.h_source, eth_s, ETH_ALEN); - memcpy(b6->eh.h_source, eth_s, ETH_ALEN); - } + tap_update_mac(&b4->taph, eth_d, eth_s); + tap_update_mac(&b6->taph, eth_d, eth_s); if (ip_da) { b4->iph.daddr = ip_da->s_addr; @@ -329,7 +318,7 @@ static void udp_sock4_iov_init(const struct ctx *c) for (i = 0; i < ARRAY_SIZE(udp4_l2_buf); i++) { udp4_l2_buf[i] = (struct udp4_l2_buf_t) { - .eh = L2_BUF_ETH_INIT(ETH_P_IP), + .taph = TAP_HDR_INIT(ETH_P_IP), .iph = L2_BUF_IP4_INIT(IPPROTO_UDP) }; } @@ -346,16 +335,13 @@ static void udp_sock4_iov_init(const struct ctx *c) mh->msg_iovlen = 1; } - for (i = 0, h = udp4_l2_mh_tap; i < UDP_MAX_FRAMES; i++, h++) { - struct msghdr *mh = &h->msg_hdr; - - if (c->mode == MODE_PASTA) - udp4_l2_iov_tap[i].iov_base = &udp4_l2_buf[i].eh; - else - udp4_l2_iov_tap[i].iov_base = &udp4_l2_buf[i].vnet_len; + for (i = 0; i < UDP_MAX_FRAMES; i++) { + struct msghdr *mh = &udp4_l2_mh_tap[i].msg_hdr; + struct iovec *iov = &udp4_l2_iov_tap[i]; - mh->msg_iov = &udp4_l2_iov_tap[i]; - mh->msg_iovlen = 1; + iov->iov_base = tap_iov_base(c, &udp4_l2_buf[i].taph); + mh->msg_iov = iov; + mh->msg_iovlen = 1; } } @@ -370,7 +356,7 @@ static void udp_sock6_iov_init(const struct ctx *c) for (i = 0; i < ARRAY_SIZE(udp6_l2_buf); i++) { udp6_l2_buf[i] = (struct udp6_l2_buf_t) { - .eh = L2_BUF_ETH_INIT(ETH_P_IPV6), + .taph = TAP_HDR_INIT(ETH_P_IPV6), .ip6h = L2_BUF_IP6_INIT(IPPROTO_UDP) }; } @@ -387,16 +373,13 @@ static void udp_sock6_iov_init(const struct ctx *c) mh->msg_iovlen = 1; } - for (i = 0, h = udp6_l2_mh_tap; i < UDP_MAX_FRAMES; i++, h++) { - struct msghdr *mh = &h->msg_hdr; - - if (c->mode == MODE_PASTA) - udp6_l2_iov_tap[i].iov_base = &udp6_l2_buf[i].eh; - else - udp6_l2_iov_tap[i].iov_base = &udp6_l2_buf[i].vnet_len; + for (i = 0; i < UDP_MAX_FRAMES; i++) { + struct msghdr *mh = &udp6_l2_mh_tap[i].msg_hdr; + struct iovec *iov = &udp6_l2_iov_tap[i]; - mh->msg_iov = &udp6_l2_iov_tap[i]; - mh->msg_iovlen = 1; + iov->iov_base = tap_iov_base(c, &udp6_l2_buf[i].taph); + mh->msg_iov = iov; + mh->msg_iovlen = 1; } } @@ -609,8 +592,8 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport, const struct timespec *now) { struct udp4_l2_buf_t *b = &udp4_l2_buf[n]; - size_t ip_len, buf_len; in_port_t src_port; + size_t ip_len; ip_len = udp4_l2_mh_sock[n].msg_len + sizeof(b->iph) + sizeof(b->uh); @@ -644,14 +627,7 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport, b->uh.dest = htons(dstport); b->uh.len = htons(udp4_l2_mh_sock[n].msg_len + sizeof(b->uh)); - buf_len = ip_len + sizeof(b->eh); - - if (c->mode == MODE_PASST) { - b->vnet_len = htonl(buf_len); - buf_len += sizeof(b->vnet_len); - } - - return buf_len; + return tap_iov_len(c, &b->taph, ip_len); } /** @@ -667,9 +643,9 @@ static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport, const struct timespec *now) { struct udp6_l2_buf_t *b = &udp6_l2_buf[n]; - size_t ip_len, buf_len; struct in6_addr *src; in_port_t src_port; + size_t ip_len; src = &b->s_in6.sin6_addr; src_port = ntohs(b->s_in6.sin6_port); @@ -726,14 +702,7 @@ static size_t udp_update_hdr6(const struct ctx *c, int n, in_port_t dstport, b->ip6h.nexthdr = IPPROTO_UDP; b->ip6h.hop_limit = 255; - buf_len = ip_len + sizeof(b->eh); - - if (c->mode == MODE_PASST) { - b->vnet_len = htonl(buf_len); - buf_len += sizeof(b->vnet_len); - } - - return buf_len; + return tap_iov_len(c, &b->taph, ip_len); } /** diff --git a/util.h b/util.h index b5be7a6..4a1c03e 100644 --- a/util.h +++ b/util.h @@ -102,13 +102,6 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags, (void *)(arg)); \ } while (0) -#define L2_BUF_ETH_INIT(proto) \ - { \ - .h_dest = { 0 }, \ - .h_source = { 0 }, \ - .h_proto = htons_constant(proto), \ - } - #define L2_BUF_IP4_INIT(proto) \ { \ .version = 4, \ -- 2.38.1

In passt mode, when writing frames to the qemu socket, we might get a short send. If we ignored this and carried on, the qemu socket would get out of sync, because the bytes we actually sent wouldn't correspond to the length header we already sent. tap_send_frames_passt() handles that by doing a a blocking send to complete the message, but it has a few flaws: * We only attempt to resend once: although it's unlikely in practice, nothing prevents the blocking send() from also being short * We print a debug error if send() returns non-zero.. but send() returns the number of bytes sent, so we actually want it to return the length of the remaining data. Correct those flaws and also be a bit more thorough about reporting problems here. Signed-off-by: David Gibson --- tap.c | 51 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/tap.c b/tap.c index 5ec6b70..4fb27ca 100644 --- a/tap.c +++ b/tap.c @@ -326,13 +326,39 @@ static void tap_send_frames_pasta(struct ctx *c, } } +/** + * tap_send_remainder() - Send remainder of a partiall sent frame + * @c: Execution context + * @iov: Partially sent buffer + * @offset: Number of bytes already sent from @iov + * + * #syscalls:passt sendto + */ +static void tap_send_remainder(const struct ctx *c, const struct iovec *iov, + size_t offset) +{ + const char *base = (char *)iov->iov_base; + size_t len = iov->iov_len; + + while (offset < len) { + ssize_t sent = send(c->fd_tap, base + offset, len - offset, + MSG_NOSIGNAL); + if (sent < 0) { + err("tap: partial frame send (missing %lu bytes): %s", + len - offset, strerror(errno)); + return; + } + offset += sent; + } +} + /** * tap_send_frames_passt() - Send multiple frames to the passt tap * @c: Execution context * @iov: Array of buffers, each containing one frame * @n: Number of buffers/frames in @iov * - * #syscalls:passt sendmsg send + * #syscalls:passt sendmsg */ static void tap_send_frames_passt(const struct ctx *c, const struct iovec *iov, size_t n) @@ -341,29 +367,28 @@ static void tap_send_frames_passt(const struct ctx *c, .msg_iov = (void *)iov, .msg_iovlen = n, }; - size_t end = 0, missing; unsigned int i; ssize_t sent; - char *p; sent = sendmsg(c->fd_tap, &mh, MSG_NOSIGNAL | MSG_DONTWAIT); if (sent < 0) return; - /* Ensure a complete last message on partial sendmsg() */ - for (i = 0; i < n; i++, iov++) { - end += iov->iov_len; - if (end >= (size_t)sent) + /* Check for any partial frames due to short send */ + for (i = 0; i < n; i++) { + if ((size_t)sent < iov[i].iov_len) break; + sent -= iov[i].iov_len; } - missing = end - sent; - if (!missing) - return; + if (i < n && sent) { + /* A partial frame was sent */ + tap_send_remainder(c, &iov[i], sent); + i++; + } - p = (char *)iov->iov_base + iov->iov_len - missing; - if (send(c->fd_tap, p, missing, MSG_NOSIGNAL)) - debug("tap: failed to flush %lu missing bytes to tap", missing); + if (i < n) + debug("tap: dropped %lu frames due to short send", n - i); } /** -- 2.38.1