September 2025 - gerrit-log - lists.osmocom.org

[S] Change in libosmocore[master]: osmo-io: Put together message buffers when dequeued from tx queue

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/40760?usp=email ) Change subject: osmo-io: Put together message buffers when dequeued from tx queue ...................................................................... osmo-io: Put together message buffers when dequeued from tx queue Write operations may be incomplete. osmo-io processs will remove complete message buffers after a write operation from msghdr and put the msghdr with the remaining buffers back into tx_queue. If the user requests multiple messages buffers per write operation, the msghdr of an incomplete write may have less message buffers than the user requested. The remaining buffers are buffers are taken from next msghdr in the queue, if exists. Change-Id: I97c366211dd266fd58ec252890ec017d6d334534 --- M src/core/osmo_io.c 1 file changed, 36 insertions(+), 5 deletions(-) Approvals: pespin: Looks good to me, approved laforge: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index bfc75c6..62d2362 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -247,21 +247,52 @@ */ struct iofd_msghdr *iofd_txqueue_dequeue(struct osmo_io_fd *iofd) { - struct llist_head *lh; + struct iofd_msghdr *msghdr; if (iofd->tx_queue.current_length == 0) return NULL; - lh = iofd->tx_queue.msg_queue.next; + msghdr = llist_first_entry_or_null(&iofd->tx_queue.msg_queue, struct iofd_msghdr, list); - OSMO_ASSERT(lh); + OSMO_ASSERT(msghdr); iofd->tx_queue.current_length--; - llist_del(lh); + llist_del(&msghdr->list); + + /* Fill up empty buffers in dequeued msghdr with buffers from the next msghdr. + * There can be empty buffers, when a msghdr is queued to the front with incomplete write. */ + while (OSMO_UNLIKELY(msghdr->io_len < iofd->io_write_buffers)) { + struct iofd_msghdr *next; + int i; + + if (iofd->tx_queue.current_length == 0) + break; + next = llist_first_entry_or_null(&iofd->tx_queue.msg_queue, struct iofd_msghdr, list); + OSMO_ASSERT(next->io_len > 0); + /* Get first message buffer from next msghdr and store them in the dequeued one. */ + msghdr->iov[msghdr->io_len] = next->iov[0]; + msghdr->msg[msghdr->io_len] = next->msg[0]; + msghdr->hdr.msg_iovlen = ++msghdr->io_len; + /* Remove the message buffer from the next msghdr and free, if empty. */ + next->io_len--; + for (i = 0; i < next->io_len; i++) { + next->iov[i] = next->iov[i + 1]; + next->msg[i] = next->msg[i + 1]; + } + if (next->io_len == 0) { + iofd->tx_queue.current_length--; + llist_del(&next->list); + iofd_msghdr_free(next); + } else { + memset(&next->iov[next->io_len], 0, sizeof(struct iovec)); + next->msg[next->io_len] = NULL; + next->hdr.msg_iovlen = --next->io_len; + } + } if (iofd->tx_queue.current_length == 0) osmo_iofd_ops.write_disable(iofd); - return llist_entry(lh, struct iofd_msghdr, list); + return msghdr; } /*! Handle segmentation of the msg. If this function returns *_HANDLE_ONE or MORE then the data in msg will contain -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40760?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I97c366211dd266fd58ec252890ec017d6d334534 Gerrit-Change-Number: 40760 Gerrit-PatchSet: 7 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

3 days, 5 hours

1
0
0 0

[M] Change in libosmocore[master]: Add multiple messages buffers to struct iofd_msghdr

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/40491?usp=email ) Change subject: Add multiple messages buffers to struct iofd_msghdr ...................................................................... Add multiple messages buffers to struct iofd_msghdr This is a prerequisite for transfering multiple buffers with a single SQE or CQE when using osmo_io with io_uring. The message buffers at struct iofd_msghr, as well as the io vectors are arrays. A maximum number is stored, as well as the current number. As this patch does not implement multiple buffers yet, only index 0 of a message buffer is used. (msg[0]) Having more than one buffer is optional and the number can be controlled via environment variable. Related: OS#6705 Change-Id: I4fb1067de4615cc22cc6caf99b481491e7f2ef92 --- M include/osmocom/core/osmo_io.h M src/core/libosmocore.map 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 6 files changed, 74 insertions(+), 16 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/include/osmocom/core/osmo_io.h b/include/osmocom/core/osmo_io.h index c8b8517..0b3dc83 100644 --- a/include/osmocom/core/osmo_io.h +++ b/include/osmocom/core/osmo_io.h @@ -70,6 +70,14 @@ OSMO_IO_FD_MODE_RECVMSG_SENDMSG, }; +/*! The _operation_ of an osmo_io_fd determines if read or write operation are to be configured used. */ +enum osmo_io_op { + /*! change parameters for read() */ + OSMO_IO_OP_READ, + /*! change parameters for write() */ + OSMO_IO_OP_WRITE, +}; + /*! The back-end used by osmo_io. There can be multiple different back-ends available on a given system; * only one of it is used for all I/O performed via osmo_io in one given process. */ enum osmo_io_backend { @@ -204,6 +212,7 @@ struct osmo_io_fd *osmo_iofd_setup(const void *ctx, int fd, const char *name, enum osmo_io_fd_mode mode, const struct osmo_io_ops *ioops, void *data); int osmo_iofd_set_cmsg_size(struct osmo_io_fd *iofd, size_t cmsg_size); +int osmo_iofd_set_io_buffers(struct osmo_io_fd *iofd, enum osmo_io_op op, uint8_t buffers); int osmo_iofd_register(struct osmo_io_fd *iofd, int fd); int osmo_iofd_unregister(struct osmo_io_fd *iofd); unsigned int osmo_iofd_txqueue_len(struct osmo_io_fd *iofd); diff --git a/src/core/libosmocore.map b/src/core/libosmocore.map index b48c4e6..239597c 100644 --- a/src/core/libosmocore.map +++ b/src/core/libosmocore.map @@ -275,6 +275,7 @@ osmo_iofd_sendmsg_msgb; osmo_iofd_set_alloc_info; osmo_iofd_set_cmsg_size; +osmo_iofd_set_io_buffers; osmo_iofd_set_data; osmo_iofd_set_ioops; osmo_iofd_set_priv_nr; diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index 6b4b919..72e30c1 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -159,7 +159,8 @@ hdr->action = action; hdr->iofd = iofd; - hdr->msg = msg; + hdr->msg[0] = msg; + hdr->io_len = 1; return hdr; } @@ -407,7 +408,7 @@ */ void iofd_handle_send_completion(struct osmo_io_fd *iofd, int rc, struct iofd_msghdr *msghdr) { - struct msgb *msg = msghdr->msg; + struct msgb *msg = msghdr->msg[0]; /* Incomplete write */ if (rc > 0 && rc < msgb_length(msg)) { @@ -442,7 +443,7 @@ OSMO_ASSERT(0); } - msgb_free(msghdr->msg); + msgb_free(msghdr->msg[0]); iofd_msghdr_free(msghdr); } @@ -478,8 +479,8 @@ return -ENOMEM; msghdr->flags = MSG_NOSIGNAL; - msghdr->iov[0].iov_base = msgb_data(msghdr->msg); - msghdr->iov[0].iov_len = msgb_length(msghdr->msg); + msghdr->iov[0].iov_base = msgb_data(msghdr->msg[0]); + msghdr->iov[0].iov_len = msgb_length(msghdr->msg[0]); msghdr->hdr.msg_iov = &msghdr->iov[0]; msghdr->hdr.msg_iovlen = 1; @@ -530,8 +531,8 @@ msghdr->hdr.msg_namelen = osmo_sockaddr_size(&msghdr->osa); } msghdr->flags = sendto_flags; - msghdr->iov[0].iov_base = msgb_data(msghdr->msg); - msghdr->iov[0].iov_len = msgb_length(msghdr->msg); + msghdr->iov[0].iov_base = msgb_data(msghdr->msg[0]); + msghdr->iov[0].iov_len = msgb_length(msghdr->msg[0]); msghdr->hdr.msg_iov = &msghdr->iov[0]; msghdr->hdr.msg_iovlen = 1; @@ -595,8 +596,8 @@ } /* build iov from msgb */ - msghdr->iov[0].iov_base = msgb_data(msghdr->msg); - msghdr->iov[0].iov_len = msgb_length(msghdr->msg); + msghdr->iov[0].iov_base = msgb_data(msghdr->msg[0]); + msghdr->iov[0].iov_len = msgb_length(msghdr->msg[0]); msghdr->hdr.msg_iov = &msghdr->iov[0]; msghdr->hdr.msg_iovlen = 1; @@ -724,6 +725,9 @@ iofd->tx_queue.max_length = 1024; INIT_LLIST_HEAD(&iofd->tx_queue.msg_queue); + iofd->io_read_buffers = 1; + iofd->io_write_buffers = 1; + return iofd; } @@ -741,6 +745,41 @@ return 0; } +/*! Set the number of buffers that can be used in a single read or write operation. + * + * If the osmo_io_fd is in OSMO_IO_FD_MODE_READ_WRITE mode, this API function can be used to tell the + * osmo_io proecess how many buffers should be read or written with a single read or write operation. + * This feature is supported with io_uring backend only. + * + * \param[in] iofd the iofd file descriptor + * \param[in] op the osmo_io_op (read or write) to set the number of IO buffers for + * \param[in] buffers the number of IO buffer for each specified operation + * \returns zero on success, a negative value on error + */ +int osmo_iofd_set_io_buffers(struct osmo_io_fd *iofd, enum osmo_io_op op, uint8_t buffers) +{ + if (iofd->mode != OSMO_IO_FD_MODE_READ_WRITE) + return -EINVAL; + + if (g_io_backend != OSMO_IO_BACKEND_IO_URING) + return -EINVAL; + + if (buffers < 1 || buffers > IOFD_MSGHDR_IO_BUFFERS) + return -EINVAL; + + switch (op) { + case OSMO_IO_OP_READ: + iofd->io_read_buffers = buffers; + break; + case OSMO_IO_OP_WRITE: + iofd->io_write_buffers = buffers; + break; + default: + return -EINVAL; + } + return 0; +} + /*! Register the osmo_io_fd for active I/O. * * Calling this function will register a previously initialized osmo_io_fd for performing I/O. @@ -836,7 +875,7 @@ { struct iofd_msghdr *hdr; while ((hdr = iofd_txqueue_dequeue(iofd))) { - msgb_free(hdr->msg); + msgb_free(hdr->msg[0]); iofd_msghdr_free(hdr); } } diff --git a/src/core/osmo_io_internal.h b/src/core/osmo_io_internal.h index 79b3fa9..d28e3d7 100644 --- a/src/core/osmo_io_internal.h +++ b/src/core/osmo_io_internal.h @@ -76,6 +76,12 @@ /*! size of iofd_msghdr.cmsg[] when allocated in recvmsg path */ size_t cmsg_size; + /*! maximum number of message-buffers per read operation */ + uint8_t io_read_buffers; + + /*! maximum number of message-buffers per write operation */ + uint8_t io_write_buffers; + struct { /*! talloc context from which to allocate msgb when reading */ const void *ctx; @@ -119,6 +125,7 @@ IOFD_ACT_SENDMSG, }; +#define IOFD_MSGHDR_IO_BUFFERS 8 /*! serialized version of 'struct msghdr' employed by sendmsg/recvmsg */ struct iofd_msghdr { @@ -131,12 +138,14 @@ struct osmo_sockaddr osa; /*! io-vector we need to pass as argument to sendmsg/recvmsg; is set up * to point into msg below */ - struct iovec iov[1]; + struct iovec iov[IOFD_MSGHDR_IO_BUFFERS]; /*! flags we pass as argument to sendmsg / recvmsg */ int flags; + /*! current number of message-buffers that are stored */ + uint8_t io_len; /*! message-buffer containing data for this I/O operation */ - struct msgb *msg; + struct msgb *msg[IOFD_MSGHDR_IO_BUFFERS]; /*! I/O file descriptor on which we perform this I/O operation */ struct osmo_io_fd *iofd; diff --git a/src/core/osmo_io_poll.c b/src/core/osmo_io_poll.c index 466973d..d27a5d6 100644 --- a/src/core/osmo_io_poll.c +++ b/src/core/osmo_io_poll.c @@ -65,7 +65,7 @@ break; case OSMO_IO_FD_MODE_RECVFROM_SENDTO: case OSMO_IO_FD_MODE_RECVMSG_SENDMSG: - hdr.msg = msg; + hdr.msg[0] = msg; hdr.iov[0].iov_base = msg->tail; hdr.iov[0].iov_len = msgb_tailroom(msg); hdr.hdr = (struct msghdr) { diff --git a/src/core/osmo_io_uring.c b/src/core/osmo_io_uring.c index 17853b6..fff632c 100644 --- a/src/core/osmo_io_uring.c +++ b/src/core/osmo_io_uring.c @@ -201,7 +201,7 @@ static void iofd_uring_handle_recv(struct iofd_msghdr *msghdr, int rc) { struct osmo_io_fd *iofd = msghdr->iofd; - struct msgb *msg = msghdr->msg; + struct msgb *msg = msghdr->msg[0]; if (rc > 0) msgb_put(msg, rc); @@ -235,7 +235,7 @@ iofd->u.uring.write_msghdr = NULL; if (OSMO_UNLIKELY(IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED))) { - msgb_free(msghdr->msg); + msgb_free(msghdr->msg[0]); iofd_msghdr_free(msghdr); } else { iofd_handle_send_completion(iofd, rc, msghdr); @@ -431,7 +431,7 @@ LOGPIO(iofd, LOGL_DEBUG, "Cancelling write\n"); iofd->u.uring.write_msghdr = NULL; talloc_steal(OTC_GLOBAL, msghdr); - msgb_free(msghdr->msg); + msgb_free(msghdr->msg[0]); msghdr->iofd = NULL; io_uring_prep_cancel(sqe, msghdr, 0); } -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40491?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I4fb1067de4615cc22cc6caf99b481491e7f2ef92 Gerrit-Change-Number: 40491 Gerrit-PatchSet: 13 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

3 days, 5 hours

1
0
0 0

[M] Change in libosmocore[master]: Add multiple messages buffers to io_uring read operations

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/40492?usp=email ) Change subject: Add multiple messages buffers to io_uring read operations ...................................................................... Add multiple messages buffers to io_uring read operations Multiple message buffers can be read by receiving a single CQE when using io_uring. If there is less data available than available buffers, not all buffers will be filled. Having more than one buffer is optional and the number can be controlled via environment variable. Related: OS#6705 Change-Id: Ic4544b8fcbad5a266db748d6864d3ae93ee06bce --- M src/core/osmo_io.c M src/core/osmo_io_uring.c 2 files changed, 60 insertions(+), 24 deletions(-) Approvals: pespin: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index 72e30c1..46885aa 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -138,29 +138,38 @@ struct iofd_msghdr *iofd_msghdr_alloc(struct osmo_io_fd *iofd, enum iofd_msg_action action, struct msgb *msg, size_t cmsg_size) { - bool free_msg = false; struct iofd_msghdr *hdr; + uint8_t idx, io_len; - if (!msg) { - msg = iofd_msgb_alloc(iofd); - if (!msg) - return NULL; - free_msg = true; - } else { - talloc_steal(iofd, msg); - } hdr = talloc_zero_size(iofd, sizeof(struct iofd_msghdr) + cmsg_size); - if (!hdr) { - if (free_msg) - talloc_free(msg); + if (!hdr) return NULL; - } hdr->action = action; hdr->iofd = iofd; - hdr->msg[0] = msg; - hdr->io_len = 1; + + /* Allocate the number of read buffers, configured by the user. Use msg as first buffer, if not NULL. + * Only READ may have multiple buffers, because packets will only be written to the first buffer. */ + io_len = (action == IOFD_ACT_READ) ? iofd->io_read_buffers : 1; + for (idx = 0; idx < io_len; idx++) { + if (msg) { + talloc_steal(iofd, msg); + hdr->msg[idx] = msg; + msg = NULL; + } else { + hdr->msg[idx] = iofd_msgb_alloc(iofd); + if (!hdr->msg[idx]) + break; + } + } + /* If at least one msgb is allocated, we can continue with only one msgb, instead of completely failing. */ + if (idx == 0) { + talloc_free(hdr); + return NULL; + } + + hdr->io_len = idx; return hdr; } diff --git a/src/core/osmo_io_uring.c b/src/core/osmo_io_uring.c index fff632c..754a39a 100644 --- a/src/core/osmo_io_uring.c +++ b/src/core/osmo_io_uring.c @@ -139,6 +139,7 @@ struct msgb *msg; struct iofd_msghdr *msghdr; struct io_uring_sqe *sqe; + uint8_t idx; msg = iofd_msgb_alloc(iofd); if (!msg) { @@ -152,8 +153,10 @@ OSMO_ASSERT(0); } - msghdr->iov[0].iov_base = msg->tail; - msghdr->iov[0].iov_len = msgb_tailroom(msg); + for (idx = 0; idx < msghdr->io_len; idx++) { + msghdr->iov[idx].iov_base = msghdr->msg[idx]->tail; + msghdr->iov[idx].iov_len = msgb_tailroom(msghdr->msg[idx]); + } switch (action) { case IOFD_ACT_RECVMSG: @@ -166,7 +169,7 @@ /* fall-through */ case IOFD_ACT_READ: msghdr->hdr.msg_iov = &msghdr->iov[0]; - msghdr->hdr.msg_iovlen = 1; + msghdr->hdr.msg_iovlen = msghdr->io_len; break; default: OSMO_ASSERT(0); @@ -180,7 +183,7 @@ switch (action) { case IOFD_ACT_READ: - io_uring_prep_readv(sqe, iofd->fd, msghdr->iov, 1, -1); + io_uring_prep_readv(sqe, iofd->fd, msghdr->iov, msghdr->hdr.msg_iovlen, -1); break; case IOFD_ACT_RECVMSG: case IOFD_ACT_RECVFROM: @@ -201,13 +204,37 @@ static void iofd_uring_handle_recv(struct iofd_msghdr *msghdr, int rc) { struct osmo_io_fd *iofd = msghdr->iofd; - struct msgb *msg = msghdr->msg[0]; + uint8_t idx; - if (rc > 0) - msgb_put(msg, rc); + for (idx = 0; idx < msghdr->io_len; idx++) { + struct msgb *msg = msghdr->msg[idx]; + int chunk; - if (!IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) - iofd_handle_recv(iofd, msg, rc, msghdr); + msghdr->msg[idx] = NULL; + if (rc > 0) { + if (rc > msghdr->iov[idx].iov_len) + chunk = msghdr->iov[idx].iov_len; + else + chunk = rc; + rc -= chunk; + msgb_put(msg, chunk); + } else { + chunk = rc; + } + + /* Check for every iteration, because iofd might get unregistered/closed during receive function. */ + if (iofd->u.uring.read_enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) + iofd_handle_recv(iofd, msg, chunk, msghdr); + else + msgb_free(msg); + + if (rc <= 0) + break; + } + while (++idx < msghdr->io_len) { + msgb_free(msghdr->msg[idx]); + msghdr->msg[idx] = NULL; + } if (iofd->u.uring.read_enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) iofd_uring_submit_recv(iofd, msghdr->action); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40492?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ic4544b8fcbad5a266db748d6864d3ae93ee06bce Gerrit-Change-Number: 40492 Gerrit-PatchSet: 13 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

3 days, 5 hours

1
0
0 0

[M] Change in libosmocore[master]: osmo_io: Add unit test to verify segmentation process

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/40854?usp=email ) Change subject: osmo_io: Add unit test to verify segmentation process ...................................................................... osmo_io: Add unit test to verify segmentation process Change-Id: I7d8feba9c8e8386c9fd144669f6ccd01d6bbbabb --- M tests/osmo_io/osmo_io_test.c M tests/osmo_io/osmo_io_test.ok 2 files changed, 126 insertions(+), 0 deletions(-) Approvals: laforge: Looks good to me, approved Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve diff --git a/tests/osmo_io/osmo_io_test.c b/tests/osmo_io/osmo_io_test.c index 553368c..d3e22aa 100644 --- a/tests/osmo_io/osmo_io_test.c +++ b/tests/osmo_io/osmo_io_test.c @@ -247,6 +247,112 @@ for (int i = 0; i < 128; i++) osmo_select_main(1); } + +int segmentation_cb(struct osmo_io_fd *iofd, struct msgb *msg) +{ + printf("%s: segmentation_cb() returning %d\n", osmo_iofd_get_name(iofd), 4); + return 4; +} + +static void segment_read_cb(struct osmo_io_fd *iofd, int rc, struct msgb *msg) +{ + static int seg_number = 0; + + printf("%s: read() msg with rc=%d\n", osmo_iofd_get_name(iofd), rc); + if (rc < 0) { + printf("%s: error: %s\n", osmo_iofd_get_name(iofd), strerror(-rc)); + OSMO_ASSERT(0); + } + OSMO_ASSERT(msg); + if (seg_number < 3) { + printf("%s\n", osmo_hexdump(msgb_data(msg), msgb_length(msg))); + printf("tailroom = %d\n", msgb_tailroom(msg)); + /* Our read buffer is 6 bytes, Our segment is 4 bytes, this results in tailroom of 2 bytes. + * When the pending 2 bytes are combined with subsequent read of 6 bytes, an extra buffer + * with 8 bytes is allocated. Our segment is 4 byte, then this results in a tailroom of 4 + * bytes. */ + if (seg_number == 1) + OSMO_ASSERT(msgb_tailroom(msg) == 4) + else + OSMO_ASSERT(msgb_tailroom(msg) == 2) + OSMO_ASSERT(msgb_length(msg) == sizeof(TESTDATA) / 4); + seg_number++; + } else { + OSMO_ASSERT(rc == 0); + file_eof_read = true; + } + talloc_free(msg); +} + +static void test_segmentation(void) +{ + struct osmo_io_fd *iofd; + struct msgb *msg; + uint8_t *buf; + int fd[2] = { 0, 0 }; + int rc; + struct osmo_io_ops ioops; + + TEST_START(); + + /* Create pipe */ + rc = pipe(fd); + OSMO_ASSERT(rc == 0); + OSMO_ASSERT(fd[0]); + OSMO_ASSERT(fd[1]); + + /* First test writing to the pipe: */ + printf("Enable write\n"); + ioops = (struct osmo_io_ops){ .write_cb = file_write_cb }; + iofd = osmo_iofd_setup(ctx, fd[1], "seg_iofd", OSMO_IO_FD_MODE_READ_WRITE, &ioops, NULL); + osmo_iofd_register(iofd, fd[1]); + + msg = msgb_alloc(12, "Test data"); + buf = msgb_put(msg, 12); + memcpy(buf, TESTDATA, 12); + osmo_iofd_write_msgb(iofd, msg); + /* Allow enough cycles to handle the messages */ + file_bytes_write_compl = 0; + for (int i = 0; i < 128; i++) { + OSMO_ASSERT(file_bytes_write_compl <= 12); + if (file_bytes_write_compl == 12) + break; + osmo_select_main(1); + usleep(100 * 1000); + } + fflush(stdout); + OSMO_ASSERT(file_bytes_write_compl == 12); + + osmo_iofd_unregister(iofd); + close(fd[1]); + + /* Now, re-configure iofd to only read from the pipe. + * Reduce the read buffer size, to verify correct segmentation operation: */ + printf("Enable read\n"); + osmo_iofd_set_alloc_info(iofd, 6, 0); + osmo_iofd_register(iofd, fd[0]); + ioops = (struct osmo_io_ops){ .read_cb = segment_read_cb, .segmentation_cb2 = segmentation_cb }; + rc = osmo_iofd_set_ioops(iofd, &ioops); + OSMO_ASSERT(rc == 0); + /* Allow enough cycles to handle the message. We expect 3 reads, 4th read will return 0. */ + file_bytes_read = 0; + file_eof_read = false; + for (int i = 0; i < 128; i++) { + if (file_eof_read) + break; + osmo_select_main(1); + usleep(100 * 1000); + } + fflush(stdout); + OSMO_ASSERT(file_eof_read); + + osmo_iofd_free(iofd); + + for (int i = 0; i < 128; i++) + osmo_select_main(1); +} + + static const struct log_info_cat default_categories[] = { }; @@ -267,6 +373,7 @@ test_file(); test_connected(); test_unconnected(); + test_segmentation(); return EXIT_SUCCESS; } diff --git a/tests/osmo_io/osmo_io_test.ok b/tests/osmo_io/osmo_io_test.ok index 6b9e591..b24bfd2 100644 --- a/tests/osmo_io/osmo_io_test.ok +++ b/tests/osmo_io/osmo_io_test.ok @@ -16,3 +16,22 @@ ep1: sendto() returned rc=16 ep2: recvfrom() msg with len=16 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 +Running test_segmentation +Enable write +seg_iofd: write() returned rc=12 +01 02 03 04 05 06 07 08 09 0a 0b 0c +Enable read +seg_iofd: segmentation_cb() returning 4 +seg_iofd: read() msg with rc=6 +01 02 03 04 +tailroom = 2 +seg_iofd: segmentation_cb() returning 4 +seg_iofd: segmentation_cb() returning 4 +seg_iofd: read() msg with rc=6 +05 06 07 08 +tailroom = 4 +seg_iofd: segmentation_cb() returning 4 +seg_iofd: read() msg with rc=6 +09 0a 0b 0c +tailroom = 2 +seg_iofd: read() msg with rc=0 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40854?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I7d8feba9c8e8386c9fd144669f6ccd01d6bbbabb Gerrit-Change-Number: 40854 Gerrit-PatchSet: 4 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

3 days, 5 hours

1
0
0 0

[M] Change in libosmocore[master]: Avoid reusing pending buffer; append incoming data instead

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/40584?usp=email ) Change subject: Avoid reusing pending buffer; append incoming data instead ...................................................................... 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(-) Approvals: pespin: Looks good to me, approved Jenkins Builder: Verified 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 https://gerrit.osmocom.org/c/libosmocore/+/40584?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I08df9736ccc5e9a7df61ca6dcf94629ee010752f Gerrit-Change-Number: 40584 Gerrit-PatchSet: 8 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

3 days, 5 hours

1
0
0 0

[M] Change in libosmocore[master]: Allow io_uring_submit batching just ahead of poll/select

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/40489?usp=email ) Change subject: Allow io_uring_submit batching just ahead of poll/select ...................................................................... Allow io_uring_submit batching just ahead of poll/select Let's add a mode (enabled via the LIBOSMO_IO_URING_BATCH environment variable), where we don't call io_uring_submit() after every operation we add to the submission queue. Rather, do that once before we go into poll. This should massively reduce the amount of io_uring_enter() syscalls we're seeing. Related: OS#6705 Change-Id: Id34fe2ced32c63d15b14810e145744f7509064cc --- M src/core/osmo_io_uring.c M src/core/select.c 2 files changed, 47 insertions(+), 4 deletions(-) Approvals: pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, approved diff --git a/src/core/osmo_io_uring.c b/src/core/osmo_io_uring.c index b8b240f..347461e 100644 --- a/src/core/osmo_io_uring.c +++ b/src/core/osmo_io_uring.c @@ -54,6 +54,11 @@ #define IOFD_URING_ENTRIES 4096 +#define OSMO_IO_URING_BATCH "LIBOSMO_IO_URING_BATCH" + +bool g_io_uring_batch = false; +bool g_io_uring_submit_needed = false; + struct osmo_io_uring { struct osmo_fd event_ofd; struct io_uring ring; @@ -88,8 +93,12 @@ /*! initialize the uring and tie it into our event loop */ void osmo_iofd_uring_init(void) { + const char *env; int rc, evfd; + if ((env = getenv(OSMO_IO_URING_BATCH))) + g_io_uring_batch = true; + rc = io_uring_queue_init(IOFD_URING_ENTRIES, &g_ring.ring, 0); if (rc < 0) osmo_panic("failure during io_uring_queue_init(): %s\n", strerror(-rc)); @@ -117,6 +126,13 @@ } } +static inline void iofd_io_uring_submit(void) +{ + if (OSMO_LIKELY(!g_io_uring_batch)) + io_uring_submit(&g_ring.ring); + else + g_io_uring_submit_needed = true; +} static void iofd_uring_submit_recv(struct osmo_io_fd *iofd, enum iofd_msg_action action) { @@ -175,7 +191,8 @@ } io_uring_sqe_set_data(sqe, msghdr); - io_uring_submit(&g_ring.ring); + iofd_io_uring_submit(); + /* NOTE: This only works if we have one read per fd */ iofd->u.uring.read_msghdr = msghdr; } @@ -317,7 +334,8 @@ OSMO_ASSERT(0); } - io_uring_submit(&g_ring.ring); + iofd_io_uring_submit(); + iofd->u.uring.write_msghdr = msghdr; return 0; @@ -417,7 +435,8 @@ msghdr->iofd = NULL; io_uring_prep_cancel(sqe, msghdr, 0); } - io_uring_submit(&g_ring.ring); + + iofd_io_uring_submit(); if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_NOTIFY_CONNECTED)) { osmo_fd_unregister(&iofd->u.uring.connect_ofd); @@ -466,7 +485,8 @@ io_uring_prep_writev(sqe, iofd->fd, msghdr->iov, 1, 0); io_uring_sqe_set_data(sqe, msghdr); - io_uring_submit(&g_ring.ring); + iofd_io_uring_submit(); + iofd->u.uring.write_msghdr = msghdr; } } @@ -537,4 +557,12 @@ .notify_connected = iofd_uring_notify_connected, }; +void osmo_io_uring_submit(void) +{ + if (OSMO_LIKELY(g_io_uring_submit_needed)) { + io_uring_submit(&g_ring.ring); + g_io_uring_submit_needed = false; + } +} + #endif /* defined(__linux__) */ diff --git a/src/core/select.c b/src/core/select.c index 70047f0..267f041 100644 --- a/src/core/select.c +++ b/src/core/select.c @@ -426,12 +426,22 @@ return work; } +#if defined(HAVE_URING) +void osmo_io_uring_submit(void); +extern bool g_io_uring_batch; +#endif + static int _osmo_select_main(int polling) { unsigned int n_poll; int rc; int timeout = 0; +#if defined(HAVE_URING) + if (OSMO_UNLIKELY(g_io_uring_batch)) + osmo_io_uring_submit(); +#endif + /* prepare read and write fdsets */ n_poll = poll_fill_fds(); @@ -464,6 +474,11 @@ int rc; struct timeval no_time = {0, 0}; +#if defined(HAVE_URING) + if (OSMO_UNLIKELY(g_io_uring_batch)) + osmo_io_uring_submit(); +#endif + FD_ZERO(&readset); FD_ZERO(&writeset); FD_ZERO(&exceptset); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40489?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id34fe2ced32c63d15b14810e145744f7509064cc Gerrit-Change-Number: 40489 Gerrit-PatchSet: 7 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

3 days, 5 hours

1
0
0 0

[M] Change in libosmocore[master]: Put all io_uring related read and write states into sub structures

by laforge

Attention is currently required from: jolly. laforge has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/40856?usp=email ) Change subject: Put all io_uring related read and write states into sub structures ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40856?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3cf4ab6d9aebf5532ad174c90c7b0e9633491c88 Gerrit-Change-Number: 40856 Gerrit-PatchSet: 5 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Thu, 18 Sep 2025 09:41:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

3 days, 5 hours

1
0
0 0

[S] Change in libosmocore[master]: Remove old empty io_uring

by laforge

Attention is currently required from: jolly. laforge has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/40855?usp=email ) Change subject: Remove old empty io_uring ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40855?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id2d2a0400ad442198c684ea0ead4eaeaead4c53d Gerrit-Change-Number: 40855 Gerrit-PatchSet: 6 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Thu, 18 Sep 2025 09:41:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

3 days, 5 hours

1
0
0 0

[M] Change in libosmocore[master]: Automatically increase io_uring, if too small.

by laforge

Attention is currently required from: jolly, pespin. laforge has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/40725?usp=email ) Change subject: Automatically increase io_uring, if too small. ...................................................................... Patch Set 14: Code-Review+2 (1 comment) Patchset: PS2: > I think having two rings and gradually migrating over is clean, and we can test the scenario by runn […] Done -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40725?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id9230146acc8d54bfd44834e783c31b37bd64bca Gerrit-Change-Number: 40725 Gerrit-PatchSet: 14 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 18 Sep 2025 09:41:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>

3 days, 5 hours

1
0
0 0

[S] Change in libosmocore[master]: Add environment variable to set io_uring size

by laforge

Attention is currently required from: jolly. laforge has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/40782?usp=email ) Change subject: Add environment variable to set io_uring size ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40782?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I55289d9282e13aa1bf82f3931c85c196752f1484 Gerrit-Change-Number: 40782 Gerrit-PatchSet: 7 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Thu, 18 Sep 2025 09:40:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

3 days, 6 hours

1
0
0 0

2025

2024

2023

2022

gerrit-log September 2025