With vhost-user multibuffer frames, the iov can be larger than the
actual L2 frame. The previous approach of computing L2 length as
iov_size() - offset would overcount and write extra bytes into the
pcap file.
Pass the L2 frame length explicitly to pcap_frame() and pcap_iov(),
and write exactly that many bytes instead of the full iov remainder.
Signed-off-by: Laurent Vivier
---
pcap.c | 28 +++++++++++++++++++---------
pcap.h | 2 +-
tap.c | 6 ++++--
tcp_vu.c | 14 ++++++++------
udp_vu.c | 4 +++-
util.c | 31 +++++++++++++++++++++++++++----
util.h | 3 ++-
vu_common.c | 2 +-
8 files changed, 65 insertions(+), 25 deletions(-)
diff --git a/pcap.c b/pcap.c
index a026f17e7974..d1717a003faf 100644
--- a/pcap.c
+++ b/pcap.c
@@ -52,12 +52,12 @@ struct pcap_pkthdr {
* @iov: IO vector containing frame (with L2 headers and tap headers)
* @iovcnt: Number of buffers (@iov entries) in frame
* @offset: Byte offset of the L2 headers within @iov
+ * @l2len: Length of L2 frame data to capture
* @now: Timestamp
*/
static void pcap_frame(const struct iovec *iov, size_t iovcnt,
- size_t offset, const struct timespec *now)
+ size_t offset, size_t l2len, const struct timespec *now)
{
- size_t l2len = iov_size(iov, iovcnt) - offset;
struct pcap_pkthdr h = {
.tv_sec = now->tv_sec,
.tv_usec = DIV_ROUND_CLOSEST(now->tv_nsec, 1000),
@@ -65,9 +65,15 @@ static void pcap_frame(const struct iovec *iov, size_t iovcnt,
.len = l2len
};
- if (write_all_buf(pcap_fd, &h, sizeof(h)) < 0 ||
- write_remainder(pcap_fd, iov, iovcnt, offset) < 0)
+ if (write_all_buf(pcap_fd, &h, sizeof(h)) < 0) {
+ debug_perror("Cannot log packet, packet header error");
+ return;
+ }
+
+ if (write_remainder(pcap_fd, iov, iovcnt, offset, l2len) < 0) {
debug_perror("Cannot log packet, length %zu", l2len);
+ return;
+ }
}
/**
@@ -87,7 +93,7 @@ void pcap(const char *pkt, size_t l2len)
if (clock_gettime(CLOCK_REALTIME, &now))
err_perror("Failed to get CLOCK_REALTIME time");
- pcap_frame(&iov, 1, 0, &now);
+ pcap_frame(&iov, 1, 0, l2len, &now);
}
/**
@@ -109,8 +115,11 @@ void pcap_multiple(const struct iovec *iov, size_t frame_parts, unsigned int n,
if (clock_gettime(CLOCK_REALTIME, &now))
err_perror("Failed to get CLOCK_REALTIME time");
- for (i = 0; i < n; i++)
- pcap_frame(iov + i * frame_parts, frame_parts, offset, &now);
+ for (i = 0; i < n; i++) {
+ pcap_frame(iov + i * frame_parts, frame_parts, offset,
+ iov_size(iov + i * frame_parts, frame_parts) - offset,
+ &now);
+ }
}
/**
@@ -120,8 +129,9 @@ void pcap_multiple(const struct iovec *iov, size_t frame_parts, unsigned int n,
* containing packet data to write, including L2 header
* @iovcnt: Number of buffers (@iov entries)
* @offset: Offset of the L2 frame within the full data length
+ * @l2len: Length of L2 frame data to capture
*/
-void pcap_iov(const struct iovec *iov, size_t iovcnt, size_t offset)
+void pcap_iov(const struct iovec *iov, size_t iovcnt, size_t offset, size_t l2len)
{
struct timespec now = { 0 };
@@ -131,7 +141,7 @@ void pcap_iov(const struct iovec *iov, size_t iovcnt, size_t offset)
if (clock_gettime(CLOCK_REALTIME, &now))
err_perror("Failed to get CLOCK_REALTIME time");
- pcap_frame(iov, iovcnt, offset, &now);
+ pcap_frame(iov, iovcnt, offset, l2len, &now);
}
/**
diff --git a/pcap.h b/pcap.h
index dface5df4ee6..c171257cbd73 100644
--- a/pcap.h
+++ b/pcap.h
@@ -13,7 +13,7 @@ extern int pcap_fd;
void pcap(const char *pkt, size_t l2len);
void pcap_multiple(const struct iovec *iov, size_t frame_parts, unsigned int n,
size_t offset);
-void pcap_iov(const struct iovec *iov, size_t iovcnt, size_t offset);
+void pcap_iov(const struct iovec *iov, size_t iovcnt, size_t offset, size_t l2len);
void pcap_init(struct ctx *c);
#endif /* PCAP_H */
diff --git a/tap.c b/tap.c
index 41a61a36c279..41ba2b8666a5 100644
--- a/tap.c
+++ b/tap.c
@@ -500,7 +500,8 @@ static size_t tap_send_frames_passt(const struct ctx *c,
/* Number of unsent or partially sent buffers for the frame */
size_t rembufs = bufs_per_frame - (i % bufs_per_frame);
- if (write_remainder(c->fd_tap, &iov[i], rembufs, buf_offset) < 0) {
+ if (write_remainder(c->fd_tap, &iov[i], rembufs, buf_offset,
+ SIZE_MAX) < 0) {
err_perror("tap: partial frame send");
return i;
}
@@ -1102,10 +1103,11 @@ void tap_handler(struct ctx *c, const struct timespec *now)
void tap_add_packet(struct ctx *c, struct iov_tail *data,
const struct timespec *now)
{
+ size_t l2len = iov_tail_size(data);
struct ethhdr eh_storage;
const struct ethhdr *eh;
- pcap_iov(data->iov, data->cnt, data->off);
+ pcap_iov(data->iov, data->cnt, data->off, l2len);
eh = IOV_PEEK_HEADER(data, eh_storage);
if (!eh)
diff --git a/tcp_vu.c b/tcp_vu.c
index 0cd01190d612..7b7ea9c789b1 100644
--- a/tcp_vu.c
+++ b/tcp_vu.c
@@ -128,7 +128,8 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
return ret;
}
- iov_truncate(&flags_iov[0], 1, hdrlen + optlen);
+ l2len = hdrlen + optlen - VNET_HLEN;
+ iov_truncate(&flags_iov[0], 1, l2len + VNET_HLEN);
payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);
if (flags & KEEPALIVE)
@@ -137,13 +138,12 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &payload,
NULL, seq, !*c->pcap);
- l2len = optlen + hdrlen - VNET_HLEN;
vu_pad(&flags_elem[0].in_sg[0], l2len);
vu_flush(vdev, vq, flags_elem, 1);
if (*c->pcap)
- pcap_iov(&flags_elem[0].in_sg[0], 1, VNET_HLEN);
+ pcap_iov(&flags_elem[0].in_sg[0], 1, VNET_HLEN, l2len);
if (flags & DUP_ACK) {
elem_cnt = vu_collect(vdev, vq, &flags_elem[1], 1,
@@ -158,8 +158,10 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
vu_flush(vdev, vq, &flags_elem[1], 1);
- if (*c->pcap)
- pcap_iov(&flags_elem[1].in_sg[0], 1, VNET_HLEN);
+ if (*c->pcap) {
+ pcap_iov(&flags_elem[1].in_sg[0], 1, VNET_HLEN,
+ l2len);
+ }
}
}
vu_queue_notify(vdev, vq);
@@ -464,7 +466,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
vu_flush(vdev, vq, &elem[head[i]], buf_cnt);
if (*c->pcap)
- pcap_iov(iov, buf_cnt, VNET_HLEN);
+ pcap_iov(iov, buf_cnt, VNET_HLEN, l2len);
conn->seq_to_tap += dlen;
}
diff --git a/udp_vu.c b/udp_vu.c
index 1a73d997f683..76242d423778 100644
--- a/udp_vu.c
+++ b/udp_vu.c
@@ -182,6 +182,7 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s, int n, flow_sidx_t tosidx)
static struct iovec iov_vu[VIRTQUEUE_MAX_SIZE];
struct vu_dev *vdev = c->vdev;
struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE];
+ size_t hdrlen = udp_vu_hdrlen(v6);
int i;
assert(!c->no_udp);
@@ -227,7 +228,8 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s, int n, flow_sidx_t tosidx)
udp_vu_prepare(c, iov_vu, toside, dlen);
if (*c->pcap) {
udp_vu_csum(toside, iov_vu, iov_cnt, dlen);
- pcap_iov(iov_vu, iov_cnt, VNET_HLEN);
+ pcap_iov(iov_vu, iov_cnt, VNET_HLEN,
+ hdrlen + dlen - VNET_HLEN);
}
vu_flush(vdev, vq, elem, elem_used);
vu_queue_notify(vdev, vq);
diff --git a/util.c b/util.c
index 73c9d51d7b4a..141aad275869 100644
--- a/util.c
+++ b/util.c
@@ -722,31 +722,54 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags,
* @iov: IO vector
* @iovcnt: Number of entries in @iov
* @skip: Number of bytes of the vector to skip writing
+ * @length: Number of bytes of the vector to write
*
* Return: 0 on success, -1 on error (with errno set)
*
* #syscalls writev
*/
-int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip)
+int write_remainder(int fd, const struct iovec *iov, size_t iovcnt,
+ size_t skip, size_t length)
{
size_t i = 0, offset;
- while ((i += iov_skip_bytes(iov + i, iovcnt - i, skip, &offset)) < iovcnt) {
+ while (length &&
+ (i += iov_skip_bytes(iov + i, iovcnt - i, skip, &offset)) < iovcnt) {
ssize_t rc;
+ size_t end;
if (offset) {
+ size_t len = MIN(length, iov[i].iov_len - offset);
+
/* Write the remainder of the partially written buffer */
if (write_all_buf(fd, (char *)iov[i].iov_base + offset,
- iov[i].iov_len - offset) < 0)
+ len) < 0)
return -1;
+
+ length -= len;
i++;
+
+ if (!length || i >= iovcnt)
+ break;
+ }
+
+ end = iov_skip_bytes(iov + i, iovcnt - i, length, NULL);
+
+ /* Write a trailing partial buffer */
+ if (!end) {
+ size_t len = MIN(length, iov[i].iov_len);
+
+ if (write_all_buf(fd, iov[i].iov_base, len) < 0)
+ return -1;
+ break;
}
/* Write as much of the remaining whole buffers as we can */
- rc = writev(fd, &iov[i], iovcnt - i);
+ rc = writev(fd, &iov[i], end);
if (rc < 0)
return -1;
+ length -= rc;
skip = rc;
}
return 0;
diff --git a/util.h b/util.h
index 92aeabc86b52..888093277896 100644
--- a/util.h
+++ b/util.h
@@ -235,7 +235,8 @@ int fls(unsigned long x);
int ilog2(unsigned long x);
int write_file(const char *path, const char *buf);
intmax_t read_file_integer(const char *path, intmax_t fallback);
-int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip);
+int write_remainder(int fd, const struct iovec *iov, size_t iovcnt,
+ size_t skip, size_t length);
int read_remainder(int fd, const struct iovec *iov, size_t cnt, size_t skip);
void close_open_files(int argc, char **argv);
bool snprintf_check(char *str, size_t size, const char *format, ...);
diff --git a/vu_common.c b/vu_common.c
index 57949ca32309..f254cb67ec78 100644
--- a/vu_common.c
+++ b/vu_common.c
@@ -268,7 +268,7 @@ int vu_send_single(const struct ctx *c, const void *buf, size_t size)
iov_from_buf(in_sg, in_total, VNET_HLEN, buf, total);
if (*c->pcap)
- pcap_iov(in_sg, in_total, VNET_HLEN);
+ pcap_iov(in_sg, in_total, VNET_HLEN, size);
vu_flush(vdev, vq, elem, elem_cnt);
vu_queue_notify(vdev, vq);
--
2.53.0