From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 1B0D15A026C for ; Fri, 9 Dec 2022 06:42:41 +0100 (CET) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4NT0KM4QJ4z4xvh; Fri, 9 Dec 2022 16:42:31 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1670564551; bh=4knbdyKam01b3Ix+JkyogXhG02dghV++rpbR4prBG6o=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=DXpuZLwd6t5AmkK89BXKIW9rbG3n0ZsIYg/52AOdKSrPCFPXRTm56OiQx1g4YIjPs NSb2UmL9UELK8iEe/Ou87eMfjs/pyMJTPYJfUNsFhu8na1Af0vNKXByh2sZIbYgUSg g0lAEtJSo8k/4s5N11BW27YHu2CrDdF16GKBZcts= From: David Gibson To: Stefano Brivio , passt-dev@passt.top Subject: [PATCH v2 18/18] tap: Improve handling of partial frame sends Date: Fri, 9 Dec 2022 16:42:28 +1100 Message-Id: <20221209054228.4085990-19-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221209054228.4085990-1-david@gibson.dropbear.id.au> References: <20221209054228.4085990-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: XX45ANGJJE6FMJPUAG27O52U6UJVTHQA X-Message-ID-Hash: XX45ANGJJE6FMJPUAG27O52U6UJVTHQA X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: David Gibson X-Mailman-Version: 3.3.3 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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