On Thu, 22 Feb 2024 16:56:01 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:pcap_frame() explicitly takes a single frame, and only allows a single buffer (iovec) to be passed. pcap_multiple() takes multiple buffers, but explicitly expects exactly one frame per buffer. Future changes are going to want to split single frames across multiple buffers in some circumstances, so extend the pcap functions to allow for that. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- pcap.c | 24 ++++++++++++++---------- pcap.h | 3 ++- tap.c | 2 +- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/pcap.c b/pcap.c index eeb71a3c..9eb4f3d2 100644 --- a/pcap.c +++ b/pcap.c @@ -31,6 +31,7 @@ #include "util.h" #include "passt.h" #include "log.h" +#include "pcap.h" #define PCAP_VERSION_MINOR 4 @@ -67,14 +68,15 @@ struct pcap_pkthdr { /** * pcap_frame() - Capture a single frame to pcap file with given timestamp - * @iov: iovec referencing buffer containing frame (with L2 headers) - * @offset: Offset of the frame from @iov->iov_base + * @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 @iovI swear I didn't read "Byte offset" from here before commenting on 1/6, which is rather promising.* @tv: Timestamp * * Returns: 0 on success, -errno on error writing to the file */ -static void pcap_frame(const struct iovec *iov, size_t offset, - const struct timeval *tv) +static void pcap_frame(const struct iovec *iov, size_t iovcnt, + size_t offset, const struct timeval *tv) { size_t len = iov->iov_len - offset; struct pcap_pkthdr h = { @@ -86,7 +88,7 @@ static void pcap_frame(const struct iovec *iov, size_t offset, struct iovec hiov = { &h, sizeof(h) }; if (write_remainder(pcap_fd, &hiov, 1, 0) < 0 || - write_remainder(pcap_fd, iov, 1, offset) < 0) + write_remainder(pcap_fd, iov, iovcnt, offset) < 0) debug("Cannot log packet, length %zu: %s", len, strerror(errno)); } @@ -105,16 +107,18 @@ void pcap(const char *pkt, size_t len) return; gettimeofday(&tv, NULL); - pcap_frame(&iov, 0, &tv); + pcap_frame(&iov, 1, 0, &tv); } /** * pcap_multiple() - Capture multiple frames - * @iov: Array of iovecs, one entry per frame + * @iov: Array of iovecs, every @framebufs entries is one frame + * @framebufs: Number of buffers per frameI found this a bit hard to understand. What about: * @iov: Array of IO vectors, with item count @frame_parts * @n * @frame_parts: Number of IO vector items for each frame ? The rest of the series looks good to me. -- Stefano