- gerrit-log - lists.osmocom.org

[S] Change in libosmocore[master]: osmo_io: Add osmo_io_get_ioops() function
by laforge 29 Feb '24

29 Feb '24

Attention is currently required from: jolly. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36076?usp=email ) Change subject: osmo_io: Add osmo_io_get_ioops() function ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36076?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I03398c811b9534f50c6644b21eea89a04be29fb0 Gerrit-Change-Number: 36076 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> 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, 29 Feb 2024 15:18:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in libosmo-netif[master]: stream_{client,server} example: Cleanup on exit
by Jenkins Builder 29 Feb '24

29 Feb '24

Attention is currently required from: jolly. Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/36124?usp=email ) Change subject: stream_{client,server} example: Cleanup on exit ...................................................................... Patch Set 3: (2 comments) File examples/stream-client.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-14780): https://gerrit.osmocom.org/c/libosmo-netif/+/36124/comment/79bec3b0_8133845d PS3, Line 172: while(!quit) { space required before the open parenthesis '(' File examples/stream-server.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-14780): https://gerrit.osmocom.org/c/libosmo-netif/+/36124/comment/3146a486_4688a15e PS3, Line 189: while(!quit) { space required before the open parenthesis '(' -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/36124?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I9dbb7f46b2a798e88ad4df8ff73c6ee40c07b843 Gerrit-Change-Number: 36124 Gerrit-PatchSet: 3 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Thu, 29 Feb 2024 15:13:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: osmo_io: Assign const name when stealing TX msg from iofd ctx
by pespin 29 Feb '24

29 Feb '24

Attention is currently required from: jolly, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36121?usp=email ) Change subject: osmo_io: Assign const name when stealing TX msg from iofd ctx ...................................................................... Patch Set 1: (1 comment) File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/36121/comment/48c0a6f7_d477cdb8 PS1, Line 339: talloc_set_name(msg, "osmo_io_rx_msgb"); > I think it would be better to not even set the msgb name to the name of the iofd in the first place, […] Yeah, just set it to some const string. I thought the name was copied/allocated when set. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36121?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib8dae924fa2d94a7f636136ba7279b965a18cf5b Gerrit-Change-Number: 36121 Gerrit-PatchSet: 1 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Thu, 29 Feb 2024 14:44:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in libosmo-netif[master]: stream_{cli,srv}: Add support for SCTP in OSMO_IO mode
by laforge 29 Feb '24

29 Feb '24

Attention is currently required from: arehbein, daniel, fixeria, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/35069?usp=email ) Change subject: stream_{cli,srv}: Add support for SCTP in OSMO_IO mode ...................................................................... Patch Set 10: This change is ready for review. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/35069?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I6cf5bad5f618e71c80017960c38009b089dbd6a1 Gerrit-Change-Number: 35069 Gerrit-PatchSet: 10 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: jolly <andreas(a)eversberg.eu> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Feb 2024 14:32:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: osmo_io: Reject writing messages with length of 0
by laforge 29 Feb '24

29 Feb '24

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35912?usp=email ) ( 7 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: osmo_io: Reject writing messages with length of 0 ...................................................................... osmo_io: Reject writing messages with length of 0 io_uring will reject to transmit messages with length of 0. Change-Id: I94be5ec7344d92157f7853c6c0ddf7007513ba8e Related: OS#5751 --- M src/core/osmo_io.c 1 file changed, 27 insertions(+), 0 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 5d25f66..472edf9 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -409,6 +409,11 @@ { int rc; + if (OSMO_UNLIKELY(msgb_length(msg) == 0)) { + LOGPIO(iofd, LOGL_ERROR, "Length is 0, rejecting msgb.\n"); + return -EINVAL; + } + OSMO_ASSERT(iofd->mode == OSMO_IO_FD_MODE_READ_WRITE); if (OSMO_UNLIKELY(!iofd->io_ops.write_cb)) { LOGPIO(iofd, LOGL_ERROR, "write_cb not set, Rejecting msgb\n"); @@ -451,6 +456,11 @@ { int rc; + if (OSMO_UNLIKELY(msgb_length(msg) == 0)) { + LOGPIO(iofd, LOGL_ERROR, "Length is 0, rejecting msgb.\n"); + return -EINVAL; + } + OSMO_ASSERT(iofd->mode == OSMO_IO_FD_MODE_RECVFROM_SENDTO); if (OSMO_UNLIKELY(!iofd->io_ops.sendto_cb)) { LOGPIO(iofd, LOGL_ERROR, "sendto_cb not set, Rejecting msgb\n"); @@ -499,6 +509,11 @@ int rc; struct iofd_msghdr *msghdr; + if (OSMO_UNLIKELY(msgb_length(msg) == 0)) { + LOGPIO(iofd, LOGL_ERROR, "Length is 0, rejecting msgb.\n"); + return -EINVAL; + } + OSMO_ASSERT(iofd->mode == OSMO_IO_FD_MODE_RECVMSG_SENDMSG); if (OSMO_UNLIKELY(!iofd->io_ops.sendmsg_cb)) { LOGPIO(iofd, LOGL_ERROR, "sendmsg_cb not set, Rejecting msgb\n"); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35912?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I94be5ec7344d92157f7853c6c0ddf7007513ba8e Gerrit-Change-Number: 35912 Gerrit-PatchSet: 9 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in libosmocore[master]: osmo_io_poll: Use -errno as result on read error
by laforge 29 Feb '24

29 Feb '24

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35980?usp=email ) ( 5 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: osmo_io_poll: Use -errno as result on read error ...................................................................... osmo_io_poll: Use -errno as result on read error Related: OS#5751 Change-Id: Ida63b74feecddf96bab7b2ade4e9ad216fe56e06 --- M src/core/osmo_io.c M src/core/osmo_io_poll.c 2 files changed, 12 insertions(+), 1 deletion(-) Approvals: daniel: Looks good to me, but someone else must approve Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index 472edf9..b6b5908 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -330,6 +330,7 @@ /*! completion handler: Called by osmo_io backend after a given I/O operation has completed * \param[in] iofd I/O file-descriptor on which I/O has completed * \param[in] msg message buffer containing data related to completed I/O + * \param[in] rc result code with read size or error (-errno) * \param[in] hdr serialized msghdr containing state of completed I/O */ void iofd_handle_recv(struct osmo_io_fd *iofd, struct msgb *msg, int rc, struct iofd_msghdr *hdr) { diff --git a/src/core/osmo_io_poll.c b/src/core/osmo_io_poll.c index 52e806d..74f137b 100644 --- a/src/core/osmo_io_poll.c +++ b/src/core/osmo_io_poll.c @@ -74,7 +74,7 @@ if (rc > 0) msgb_put(msg, rc); - iofd_handle_recv(iofd, msg, rc, &hdr); + iofd_handle_recv(iofd, msg, (rc < 0 && errno > 0) ? -errno : rc, &hdr); } if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35980?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ida63b74feecddf96bab7b2ade4e9ad216fe56e06 Gerrit-Change-Number: 35980 Gerrit-PatchSet: 8 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in libosmocore[master]: osmo_io_uring: Cancel pending request, free msghdr on completion
by laforge 29 Feb '24

29 Feb '24

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35981?usp=email ) ( 4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: osmo_io_uring: Cancel pending request, free msghdr on completion ...................................................................... osmo_io_uring: Cancel pending request, free msghdr on completion There is always a completion after cancelling a uring request. Because uring requests use msghdr pointer as user data, we cannot just free the msghdr after cancelling. Upon completion (received after cancelling), the user data still points to the msghdr. To prevent a use-after-free bug, msghdr is not freed, but detached from iofd instance. Then upon completion, the msghdr (if it was detached from iofd) is freed. Additionally it is not required to keep IOFD_FLAG_IN_CALLBACK set anymore, if there is a msghdr attached to iofd. As described above, all msghdr get detached, if iofd is freed (uring request get cancelled) during callback. Related: OS#5751 Change-Id: Ic253f085dd6362db85f029f46350951472210a02 --- M src/core/osmo_io_uring.c 1 file changed, 43 insertions(+), 4 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve daniel: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/core/osmo_io_uring.c b/src/core/osmo_io_uring.c index af1c790..3812b50 100644 --- a/src/core/osmo_io_uring.c +++ b/src/core/osmo_io_uring.c @@ -230,8 +230,7 @@ OSMO_ASSERT(0) } - if (!iofd->u.uring.read_msghdr && !iofd->u.uring.write_msghdr) - IOFD_FLAG_UNSET(iofd, IOFD_FLAG_IN_CALLBACK); + IOFD_FLAG_UNSET(iofd, IOFD_FLAG_IN_CALLBACK); if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_TO_FREE) && !iofd->u.uring.read_msghdr && !iofd->u.uring.write_msghdr) talloc_free(iofd); @@ -252,6 +251,11 @@ io_uring_cqe_seen(ring, cqe); continue; } + if (!msghdr->iofd) { + io_uring_cqe_seen(ring, cqe); + iofd_msghdr_free(msghdr); + continue; + } rc = cqe->res; /* Hand the entry back to the kernel before */ @@ -307,20 +311,31 @@ static int iofd_uring_unregister(struct osmo_io_fd *iofd) { struct io_uring_sqe *sqe; + struct iofd_msghdr *msghdr; + if (iofd->u.uring.read_msghdr) { + msghdr = iofd->u.uring.read_msghdr; sqe = io_uring_get_sqe(&g_ring.ring); OSMO_ASSERT(sqe != NULL); io_uring_sqe_set_data(sqe, NULL); LOGPIO(iofd, LOGL_DEBUG, "Cancelling read\n"); - io_uring_prep_cancel(sqe, iofd->u.uring.read_msghdr, 0); + iofd->u.uring.read_msghdr = NULL; + talloc_steal(OTC_GLOBAL, msghdr); + msghdr->iofd = NULL; + io_uring_prep_cancel(sqe, msghdr, 0); } if (iofd->u.uring.write_msghdr) { + msghdr = iofd->u.uring.write_msghdr; sqe = io_uring_get_sqe(&g_ring.ring); OSMO_ASSERT(sqe != NULL); io_uring_sqe_set_data(sqe, NULL); LOGPIO(iofd, LOGL_DEBUG, "Cancelling write\n"); - io_uring_prep_cancel(sqe, iofd->u.uring.write_msghdr, 0); + iofd->u.uring.write_msghdr = NULL; + talloc_steal(OTC_GLOBAL, msghdr); + msgb_free(msghdr->msg); + msghdr->iofd = NULL; + io_uring_prep_cancel(sqe, msghdr, 0); } io_uring_submit(&g_ring.ring); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35981?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ic253f085dd6362db85f029f46350951472210a02 Gerrit-Change-Number: 35981 Gerrit-PatchSet: 9 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-MessageType: merged

1 0

[S] Change in libosmocore[master]: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_sen...
by laforge 29 Feb '24

29 Feb '24

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email ) Change subject: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() ...................................................................... osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() msghdr must be detached, because subsequent callback at iofd_handle_send_completion() may destroy the iofd (which in turn frees this msghdr, if still attached) and frees the msghdr, causing a double free. Related: OS#5751 Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 --- M src/core/osmo_io_uring.c 1 file changed, 24 insertions(+), 1 deletion(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/src/core/osmo_io_uring.c b/src/core/osmo_io_uring.c index 3812b50..cb636da 100644 --- a/src/core/osmo_io_uring.c +++ b/src/core/osmo_io_uring.c @@ -195,6 +195,15 @@ { struct osmo_io_fd *iofd = msghdr->iofd; + /* Detach msghdr from iofd. It might get freed here or it is freed during iofd_handle_send_completion(). + * If there is pending data to send, iofd_uring_submit_tx() will attach it again. + * iofd_handle_send_completion() will invoke a callback function to signal the possibility of write/send. + * This callback function might close iofd, leading to the potential freeing of iofd->u.uring.write_msghdr if + * still attached. Since iofd_handle_send_completion() frees msghdr at the end of the function, detaching + * msghdr here prevents a double-free bug. */ + if (iofd->u.uring.write_msghdr == msghdr) + iofd->u.uring.write_msghdr = NULL; + if (OSMO_UNLIKELY(IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED))) { msgb_free(msghdr->msg); iofd_msghdr_free(msghdr); @@ -202,7 +211,6 @@ iofd_handle_send_completion(iofd, rc, msghdr); } - iofd->u.uring.write_msghdr = NULL; /* submit the next to-be-transmitted message for this file descriptor */ if (iofd->u.uring.write_enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) iofd_uring_submit_tx(iofd); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 Gerrit-Change-Number: 35982 Gerrit-PatchSet: 9 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in libosmocore[master]: osmo_io_poll: Declare local functions "static"
by laforge 29 Feb '24

29 Feb '24

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35983?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: osmo_io_poll: Declare local functions "static" ...................................................................... osmo_io_poll: Declare local functions "static" Change-Id: I6ba88cd7bbd5b5ef42eb460679696f105c9158cb --- M src/core/osmo_io_poll.c 1 file changed, 17 insertions(+), 8 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved pespin: Looks good to me, but someone else must approve diff --git a/src/core/osmo_io_poll.c b/src/core/osmo_io_poll.c index 74f137b..2e726d9 100644 --- a/src/core/osmo_io_poll.c +++ b/src/core/osmo_io_poll.c @@ -123,7 +123,7 @@ return 0; } -int iofd_poll_register(struct osmo_io_fd *iofd) +static int iofd_poll_register(struct osmo_io_fd *iofd) { struct osmo_fd *ofd = &iofd->u.poll.ofd; int rc; @@ -137,7 +137,7 @@ return rc; } -int iofd_poll_unregister(struct osmo_io_fd *iofd) +static int iofd_poll_unregister(struct osmo_io_fd *iofd) { struct osmo_fd *ofd = &iofd->u.poll.ofd; @@ -149,7 +149,7 @@ return 0; } -int iofd_poll_close(struct osmo_io_fd *iofd) +static int iofd_poll_close(struct osmo_io_fd *iofd) { iofd_poll_unregister(iofd); osmo_fd_close(&iofd->u.poll.ofd); @@ -157,27 +157,27 @@ return 0; } -void iofd_poll_read_enable(struct osmo_io_fd *iofd) +static void iofd_poll_read_enable(struct osmo_io_fd *iofd) { osmo_fd_read_enable(&iofd->u.poll.ofd); } -void iofd_poll_read_disable(struct osmo_io_fd *iofd) +static void iofd_poll_read_disable(struct osmo_io_fd *iofd) { osmo_fd_read_disable(&iofd->u.poll.ofd); } -void iofd_poll_write_enable(struct osmo_io_fd *iofd) +static void iofd_poll_write_enable(struct osmo_io_fd *iofd) { osmo_fd_write_enable(&iofd->u.poll.ofd); } -void iofd_poll_write_disable(struct osmo_io_fd *iofd) +static void iofd_poll_write_disable(struct osmo_io_fd *iofd) { osmo_fd_write_disable(&iofd->u.poll.ofd); } -void iofd_poll_notify_connected(struct osmo_io_fd *iofd) +static void iofd_poll_notify_connected(struct osmo_io_fd *iofd) { int rc; -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35983?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I6ba88cd7bbd5b5ef42eb460679696f105c9158cb Gerrit-Change-Number: 35983 Gerrit-PatchSet: 9 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-MessageType: merged

1 0

[M] Change in libosmocore[master]: osmo_io: Use poll/select to notify socket connection at osmo_io_uring.c
by laforge 29 Feb '24

29 Feb '24

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35910?usp=email ) ( 8 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: osmo_io: Use poll/select to notify socket connection at osmo_io_uring.c ...................................................................... osmo_io: Use poll/select to notify socket connection at osmo_io_uring.c In order to receive a connect notification from SCTP socket, poll/select event must be used instead of a write notification via io_uring completion event. Once the connect notification has been received, subsequent write notifications via io_uring are used. Change-Id: I4eca9ea72beb0d6ea4d44cce81ed620033f07270 Related: OS#5751 --- M src/core/osmo_io_internal.h M src/core/osmo_io_uring.c 2 files changed, 93 insertions(+), 1 deletion(-) Approvals: laforge: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/core/osmo_io_internal.h b/src/core/osmo_io_internal.h index af47a3d..a4b0749 100644 --- a/src/core/osmo_io_internal.h +++ b/src/core/osmo_io_internal.h @@ -104,6 +104,8 @@ void *read_msghdr; void *write_msghdr; /* TODO: index into array of registered fd's? */ + /* osmo_fd for non-blocking connect handling */ + struct osmo_fd connect_ofd; } uring; } u; }; diff --git a/src/core/osmo_io_uring.c b/src/core/osmo_io_uring.c index aa9df85..af1c790 100644 --- a/src/core/osmo_io_uring.c +++ b/src/core/osmo_io_uring.c @@ -324,6 +324,11 @@ } io_uring_submit(&g_ring.ring); + if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_NOTIFY_CONNECTED)) { + osmo_fd_unregister(&iofd->u.uring.connect_ofd); + IOFD_FLAG_UNSET(iofd, IOFD_FLAG_NOTIFY_CONNECTED); + } + return 0; } @@ -334,6 +339,10 @@ if (iofd->u.uring.write_msghdr) return; + /* This function is called again, once the socket is connected. */ + if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_NOTIFY_CONNECTED)) + return; + if (osmo_iofd_txqueue_len(iofd) > 0) iofd_uring_submit_tx(iofd); else if (iofd->mode == OSMO_IO_FD_MODE_READ_WRITE) { @@ -379,6 +388,10 @@ if (iofd->u.uring.read_msghdr) return; + /* This function is called again, once the socket is connected. */ + if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_NOTIFY_CONNECTED)) + return; + switch (iofd->mode) { case OSMO_IO_FD_MODE_READ_WRITE: iofd_uring_submit_recv(iofd, IOFD_ACT_READ); @@ -407,6 +420,66 @@ return close(iofd->fd); } +/* called via osmocom poll/select main handling once outbound non-blocking connect() completes */ +static int iofd_uring_connected_cb(struct osmo_fd *ofd, unsigned int what) +{ + struct osmo_io_fd *iofd = ofd->data; + + LOGPIO(iofd, LOGL_DEBUG, "Socket connected or failed."); + + if (!(what & OSMO_FD_WRITE)) + return 0; + + /* Unregister from poll/select handling. */ + osmo_fd_unregister(ofd); + IOFD_FLAG_UNSET(iofd, IOFD_FLAG_NOTIFY_CONNECTED); + + /* Notify the application about this via a zero-length write completion call-back. */ + IOFD_FLAG_SET(iofd, IOFD_FLAG_IN_CALLBACK); + switch (iofd->mode) { + case OSMO_IO_FD_MODE_READ_WRITE: + iofd->io_ops.write_cb(iofd, 0, NULL); + break; + case OSMO_IO_FD_MODE_RECVFROM_SENDTO: + iofd->io_ops.sendto_cb(iofd, 0, NULL, NULL); + break; + case OSMO_IO_FD_MODE_RECVMSG_SENDMSG: + iofd->io_ops.sendmsg_cb(iofd, 0, NULL); + break; + } + IOFD_FLAG_UNSET(iofd, IOFD_FLAG_IN_CALLBACK); + + /* If write/read notifications are pending, enable it now. */ + if (iofd->u.uring.write_enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) + iofd_uring_write_enable(iofd); + if (iofd->u.uring.read_enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) + iofd_uring_read_enable(iofd); + + if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_TO_FREE) && !iofd->u.uring.read_msghdr && !iofd->u.uring.write_msghdr) + talloc_free(iofd); + return 0; +} + +static void iofd_uring_notify_connected(struct osmo_io_fd *iofd) +{ + if (iofd->mode == OSMO_IO_FD_MODE_RECVMSG_SENDMSG) { + /* Don't call this function after enabling read or write. */ + OSMO_ASSERT(!iofd->u.uring.write_enabled && !iofd->u.uring.read_enabled); + + /* Use a temporary osmo_fd which we can use to notify us once the connection is established + * or failed (indicated by FD becoming writable). + * This is needed as (at least for SCTP sockets) one cannot submit a zero-length writev/sendmsg + * in order to get notification when the socekt is writable.*/ + if (!IOFD_FLAG_ISSET(iofd, IOFD_FLAG_NOTIFY_CONNECTED)) { + osmo_fd_setup(&iofd->u.uring.connect_ofd, iofd->fd, OSMO_FD_WRITE, + iofd_uring_connected_cb, iofd, 0); + osmo_fd_register(&iofd->u.uring.connect_ofd); + IOFD_FLAG_SET(iofd, IOFD_FLAG_NOTIFY_CONNECTED); + } + } else + iofd_uring_write_enable(iofd); +} + const struct iofd_backend_ops iofd_uring_ops = { .register_fd = iofd_uring_register, .unregister_fd = iofd_uring_unregister, @@ -415,7 +488,7 @@ .write_disable = iofd_uring_write_disable, .read_enable = iofd_uring_read_enable, .read_disable = iofd_uring_read_disable, - .notify_connected = iofd_uring_write_enable, + .notify_connected = iofd_uring_notify_connected, }; #endif /* defined(__linux__) */ -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35910?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I4eca9ea72beb0d6ea4d44cce81ed620033f07270 Gerrit-Change-Number: 35910 Gerrit-PatchSet: 9 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

2026

2025

2024

2023

2022

gerrit-log