laforge submitted this change.
Avoid reusing pending buffer; append incoming data instead
When reading from a stream, a single read may return only part of a
message segment. In such cases, the partial data was stored in
'iofd->pending' and reused for subsequent reads to complete the message.
With upcoming changes that submit multiple read SQEs to io_uring,
each read uses its own pre-submitted buffer. Reusing 'iofd->pending' for
submitting next read is not possible, as the next read buffer is already
submitted.
Instead, create a new msgb which is used for the read operation and,
once completed, memcopy to the existing pending buffer, allowing message
segments to accumulate until complete.
Change-Id: I08df9736ccc5e9a7df61ca6dcf94629ee010752f
---
M src/core/osmo_io.c
M src/core/osmo_io_internal.h
M src/core/osmo_io_poll.c
M src/core/osmo_io_uring.c
4 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c
index aaf7bcc..6b4b919 100644
--- a/src/core/osmo_io.c
+++ b/src/core/osmo_io.c
@@ -174,36 +174,19 @@
talloc_free(msghdr);
}
-/*! convenience wrapper to call msgb_alloc with parameters from osmo_io_fd */
-struct msgb *iofd_msgb_alloc(struct osmo_io_fd *iofd)
+/*! convenience wrapper to call msgb_alloc with parameters from osmo_io_fd (of given size) */
+struct msgb *iofd_msgb_alloc2(struct osmo_io_fd *iofd, size_t size)
{
uint16_t headroom = iofd->msgb_alloc.headroom;
- OSMO_ASSERT(iofd->msgb_alloc.size <= 0xffff - headroom);
- return msgb_alloc_headroom_c(iofd, iofd->msgb_alloc.size + headroom, headroom, "osmo_io_msgb");
+ OSMO_ASSERT(size <= 0xffff - headroom);
+ return msgb_alloc_headroom_c(iofd, size + headroom, headroom, "osmo_io_msgb");
}
-/*! return the pending msgb in iofd or NULL if there is none*/
-struct msgb *iofd_msgb_pending(struct osmo_io_fd *iofd)
+/*! convenience wrapper to call msgb_alloc with parameters from osmo_io_fd */
+struct msgb *iofd_msgb_alloc(struct osmo_io_fd *iofd)
{
- struct msgb *msg = NULL;
-
- msg = iofd->pending;
- iofd->pending = NULL;
-
- return msg;
-}
-
-/*! Return the pending msgb or allocate and return a new one */
-struct msgb *iofd_msgb_pending_or_alloc(struct osmo_io_fd *iofd)
-{
- struct msgb *msg = NULL;
-
- msg = iofd_msgb_pending(iofd);
- if (!msg)
- msg = iofd_msgb_alloc(iofd);
-
- return msg;
+ return iofd_msgb_alloc2(iofd, iofd->msgb_alloc.size);
}
/*! Enqueue a message to be sent.
@@ -348,6 +331,25 @@
return;
}
+ /* If we have a pending message, append the received message.
+ * If the pending message is not large enough, create a larger message. */
+ if (OSMO_UNLIKELY(iofd->pending)) {
+ if (OSMO_UNLIKELY(msgb_tailroom(iofd->pending) < msgb_length(msg))) {
+ /* Data of msg does not fit into pending message. Allocate a new message that is larger.
+ * This implies that msgb_length(iofd->pending) + msgb_length(msg) > iofd.msgb_alloc.size. */
+ pending = iofd_msgb_alloc2(iofd, msgb_length(iofd->pending) + msgb_length(msg));
+ OSMO_ASSERT(pending);
+ memcpy(msgb_put(pending, msgb_length(iofd->pending)), msgb_data(iofd->pending),
+ msgb_length(iofd->pending));
+ msgb_free(iofd->pending);
+ iofd->pending = pending;
+ }
+ memcpy(msgb_put(iofd->pending, msgb_length(msg)), msgb_data(msg), msgb_length(msg));
+ msgb_free(msg);
+ msg = iofd->pending;
+ iofd->pending = NULL;
+ }
+
do {
pending = NULL;
res = iofd_handle_segmentation(iofd, msg, &pending);
diff --git a/src/core/osmo_io_internal.h b/src/core/osmo_io_internal.h
index a4b0749..79b3fa9 100644
--- a/src/core/osmo_io_internal.h
+++ b/src/core/osmo_io_internal.h
@@ -153,9 +153,8 @@
struct iofd_msghdr *iofd_msghdr_alloc(struct osmo_io_fd *iofd, enum iofd_msg_action action, struct msgb *msg, size_t cmsg_size);
void iofd_msghdr_free(struct iofd_msghdr *msghdr);
+struct msgb *iofd_msgb_alloc2(struct osmo_io_fd *iofd, size_t size);
struct msgb *iofd_msgb_alloc(struct osmo_io_fd *iofd);
-struct msgb *iofd_msgb_pending(struct osmo_io_fd *iofd);
-struct msgb *iofd_msgb_pending_or_alloc(struct osmo_io_fd *iofd);
void iofd_handle_recv(struct osmo_io_fd *iofd, struct msgb *msg, int rc, struct iofd_msghdr *msghdr);
void iofd_handle_send_completion(struct osmo_io_fd *iofd, int rc, struct iofd_msghdr *msghdr);
diff --git a/src/core/osmo_io_poll.c b/src/core/osmo_io_poll.c
index 8782adf..466973d 100644
--- a/src/core/osmo_io_poll.c
+++ b/src/core/osmo_io_poll.c
@@ -50,7 +50,7 @@
if (what & OSMO_FD_READ) {
struct iofd_msghdr hdr;
- msg = iofd_msgb_pending_or_alloc(iofd);
+ msg = iofd_msgb_alloc(iofd);
if (!msg) {
LOGPIO(iofd, LOGL_ERROR, "Could not allocate msgb for reading\n");
OSMO_ASSERT(0);
diff --git a/src/core/osmo_io_uring.c b/src/core/osmo_io_uring.c
index 347461e..17853b6 100644
--- a/src/core/osmo_io_uring.c
+++ b/src/core/osmo_io_uring.c
@@ -140,7 +140,7 @@
struct iofd_msghdr *msghdr;
struct io_uring_sqe *sqe;
- msg = iofd_msgb_pending_or_alloc(iofd);
+ msg = iofd_msgb_alloc(iofd);
if (!msg) {
LOGPIO(iofd, LOGL_ERROR, "Could not allocate msgb for reading\n");
OSMO_ASSERT(0);
To view, visit change 40584. To unsubscribe, or for help writing mail filters, visit settings.