On Fri, 8 Mar 2024 17:53:22 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:tap_send_frames() takes a vector of buffers and requires exactly one frame per buffer. We have future plans where we want to have multiple buffers per frame in some circumstances, so extend tap_send_frames() to take the number of buffers per frame as a parameter. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- tap.c | 83 +++++++++++++++++++++++++++++++++++++---------------------- tap.h | 3 ++- tcp.c | 8 +++--- udp.c | 2 +- 4 files changed, 59 insertions(+), 37 deletions(-) diff --git a/tap.c b/tap.c index f4051cec..f9e2a8d9 100644 --- a/tap.c +++ b/tap.c @@ -309,21 +309,28 @@ void tap_icmp6_send(const struct ctx *c, /** * tap_send_frames_pasta() - Send multiple frames to the pasta tap - * @c: Execution context - * @iov: Array of buffers, each containing one frame - * @n: Number of buffers/frames in @iov + * @c: Execution context + * @iov: Array of buffers + * @bufs_per_frame: Number of buffers (iovec entries) per frame + * @nframes: Number of frames to send * + * @iov must have total length @bufs_per_frame * @nframes, with each set of + * @bufs_per_frame contiguous buffers representing a single frame.Oh, this does pretty much what I was suggesting as a comment to Laurent's "tcp: Replace TCP buffer structure by an iovec array" -- I should have reviewed this first.+ * * Return: number of frames successfully sent * * #syscalls:pasta write */ static size_t tap_send_frames_pasta(const struct ctx *c, - const struct iovec *iov, size_t n) + const struct iovec *iov, + size_t bufs_per_frame, size_t nframes) { + size_t nbufs = bufs_per_frame * nframes; size_t i; - for (i = 0; i < n; i++) { - ssize_t rc = write(c->fd_tap, iov[i].iov_base, iov[i].iov_len); + for (i = 0; i < nbufs; i += bufs_per_frame) { + ssize_t rc = writev(c->fd_tap, iov + i, bufs_per_frame); + size_t framelen = iov_size(iov + i, bufs_per_frame); if (rc < 0) { debug("tap write: %s", strerror(errno)); @@ -340,32 +347,37 @@ static size_t tap_send_frames_pasta(const struct ctx *c, default: die("Write error on tap device, exiting"); } - } else if ((size_t)rc < iov[i].iov_len) { - debug("short write on tuntap: %zd/%zu", - rc, iov[i].iov_len); + } else if ((size_t)rc < framelen) { + debug("short write on tuntap: %zd/%zu", rc, framelen); break; } } - return i; + return i / bufs_per_frame; } /** * 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 + * @c: Execution context + * @iov: Array of buffers, each containing one frame + * @bufs_per_frame: Number of buffers (iovec entries) per frame + * @nframes: Number of frames to send * + * @iov must have total length @bufs_per_frame * @nframes, with each set of + * @bufs_per_frame contiguous buffers representing a single frame. + * * Return: number of frames successfully sent * * #syscalls:passt sendmsg */ static size_t tap_send_frames_passt(const struct ctx *c, - const struct iovec *iov, size_t n) + const struct iovec *iov, + size_t bufs_per_frame, size_t nframes) { + size_t nbufs = bufs_per_frame * nframes; struct msghdr mh = { .msg_iov = (void *)iov, - .msg_iovlen = n, + .msg_iovlen = nbufs, }; size_t buf_offset; unsigned int i; @@ -376,44 +388,53 @@ static size_t tap_send_frames_passt(const struct ctx *c, return 0; /* Check for any partial frames due to short send */ - i = iov_skip_bytes(iov, n, sent, &buf_offset); + i = iov_skip_bytes(iov, nbufs, sent, &buf_offset); + + if (i < nbufs && (buf_offset || (i % bufs_per_frame))) { + /* Number of not-fully-sent buffers in the frame */Strictly speaking, this comment is correct, but "not-fully-sent" seems to imply that rembufs only counts partially sent buffers. It also counts the ones that weren't sent at all. What about: /* Number of partially sent or not sent buffers for the frame */ ? The rest of the series looks good to me, I can change this text on merge if you like the proposal, or even apply it as it is (well, it's correct, after all). -- Stefano