December 2024 - gerrit-log - lists.osmocom.org

[M] Change in libosmocore[master]: osmo_io: uring: Setup connect_notify internal ofd during register() op
by pespin 09 Dec '24

09 Dec '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/39068?usp=email ) Change subject: osmo_io: uring: Setup connect_notify internal ofd during register() op ...................................................................... osmo_io: uring: Setup connect_notify internal ofd during register() op osmo_fd used internally should only be registered during osmo_iofd_register() time, not before. Unregistering was already placed at the proper place. Change-Id: Ifac374170736a9fe922362e95059a18c2a3ccaac --- M src/core/osmo_io_uring.c 1 file changed, 59 insertions(+), 52 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/68/39068/1 diff --git a/src/core/osmo_io_uring.c b/src/core/osmo_io_uring.c index 1b1f997..be7e1d8 100644 --- a/src/core/osmo_io_uring.c +++ b/src/core/osmo_io_uring.c @@ -324,8 +324,65 @@ static void iofd_uring_write_enable(struct osmo_io_fd *iofd); static void iofd_uring_read_enable(struct osmo_io_fd *iofd); + +/* 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.\n"); + + 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 int iofd_uring_register(struct osmo_io_fd *iofd) { + if (iofd->mode != OSMO_IO_FD_MODE_RECVMSG_SENDMSG) + return 0; /* Nothing to be done */ + + /* OSMO_IO_FD_MODE_RECVMSG_SENDMSG: + * 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); + if (osmo_fd_register(&iofd->u.uring.connect_ofd) < 0) { + LOGPIO(iofd, LOGL_ERROR, "Failed to register FD for connect event.\n"); + return -EBADFD; + } + } return 0; } @@ -456,46 +513,6 @@ 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.\n"); - - 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) { @@ -506,18 +523,8 @@ /* 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); - if (osmo_fd_register(&iofd->u.uring.connect_ofd) < 0) - LOGPIO(iofd, LOGL_ERROR, "Failed to register FD for connect event.\n"); - else - IOFD_FLAG_SET(iofd, IOFD_FLAG_NOTIFY_CONNECTED); - } + /* Set flag to enable temporary osmo_fd during register() time: */ + IOFD_FLAG_SET(iofd, IOFD_FLAG_NOTIFY_CONNECTED); } const struct iofd_backend_ops iofd_uring_ops = { -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/39068?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ifac374170736a9fe922362e95059a18c2a3ccaac Gerrit-Change-Number: 39068 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in libosmocore[master]: osmo_io: close() op in backend only takes care of closing
by pespin 09 Dec '24

09 Dec '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/39070?usp=email ) Change subject: osmo_io: close() op in backend only takes care of closing ...................................................................... osmo_io: close() op in backend only takes care of closing Move dependent steps such as unregister to be done at the common path, and leave the close() op on each backend to implement only the specific close operations. Change-Id: I0150afcc0b83ea8b2d00d108658ed688ce487f7f --- M src/core/osmo_io.c M src/core/osmo_io_poll.c M src/core/osmo_io_uring.c 3 files changed, 4 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/70/39070/1 diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index cef9bec..387cefb 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -845,6 +845,10 @@ iofd->pending = NULL; + osmo_iofd_ops.read_disable(iofd); + osmo_iofd_ops.write_disable(iofd); + osmo_iofd_unregister(iofd); + rc = osmo_iofd_ops.close(iofd); iofd->fd = -1; return rc; diff --git a/src/core/osmo_io_poll.c b/src/core/osmo_io_poll.c index 89ff466..92f03f8 100644 --- a/src/core/osmo_io_poll.c +++ b/src/core/osmo_io_poll.c @@ -156,9 +156,7 @@ static int iofd_poll_close(struct osmo_io_fd *iofd) { - iofd_poll_unregister(iofd); osmo_fd_close(&iofd->u.poll.ofd); - return 0; } diff --git a/src/core/osmo_io_uring.c b/src/core/osmo_io_uring.c index be7e1d8..72a465e 100644 --- a/src/core/osmo_io_uring.c +++ b/src/core/osmo_io_uring.c @@ -507,9 +507,6 @@ static int iofd_uring_close(struct osmo_io_fd *iofd) { - iofd_uring_read_disable(iofd); - iofd_uring_write_disable(iofd); - iofd_uring_unregister(iofd); return close(iofd->fd); } -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/39070?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0150afcc0b83ea8b2d00d108658ed688ce487f7f Gerrit-Change-Number: 39070 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in libosmocore[master]: osmo_io: segmented_read: Avoid triggering read events if user unregis...
by pespin 09 Dec '24

09 Dec '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/39072?usp=email ) Change subject: osmo_io: segmented_read: Avoid triggering read events if user unregisters ...................................................................... osmo_io: segmented_read: Avoid triggering read events if user unregisters The user code doesn't expect to receive any more read cb events after unregistering the FD. Avoid keep calling the read cb if several entire messages were received at once in a single tcp chunk. Related: SYS#7063 Change-Id: Id2199b9aa805cc7e7793c1a8aecd10b61f2b4c90 --- M src/core/osmo_io.c 1 file changed, 9 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/72/39072/1 diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index c61c8e9..d1767d2 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -340,8 +340,16 @@ do { pending = NULL; res = iofd_handle_segmentation(iofd, msg, &pending); - if (res != IOFD_SEG_ACT_DEFER || rc < 0) + if (res != IOFD_SEG_ACT_DEFER || rc < 0) { iofd->io_ops.read_cb(iofd, rc, msg); + /* The user could unregister/close the iofd during read_cb() above. + * Once that's done, it doesn't expect to receive any more events, + * so discard it: */ + if (!IOFD_FLAG_ISSET(iofd, IOFD_FLAG_FD_REGISTERED)) { + msgb_free(pending); + return; + } + } if (res == IOFD_SEG_ACT_HANDLE_MORE) msg = pending; } while (res == IOFD_SEG_ACT_HANDLE_MORE); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/39072?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id2199b9aa805cc7e7793c1a8aecd10b61f2b4c90 Gerrit-Change-Number: 39072 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in libosmocore[master]: osmo_io: Track IOFD_FLAG_FD_REGISTERED in all backends
by pespin 09 Dec '24

09 Dec '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/39071?usp=email ) Change subject: osmo_io: Track IOFD_FLAG_FD_REGISTERED in all backends ...................................................................... osmo_io: Track IOFD_FLAG_FD_REGISTERED in all backends This will be used in common segmentation handling code to figure out whether the iofd is still registered or was unregistered by the user during the read cb, in order to know whether to continue submitting read events upwards or to discard the handling. Change-Id: Id5e92aa22ce1c5d76028c539784118be227b9d5a --- M src/core/osmo_io.c M src/core/osmo_io_poll.c 2 files changed, 21 insertions(+), 15 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/71/39071/1 diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index 387cefb..c61c8e9 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -737,19 +737,29 @@ { int rc = 0; - if (fd >= 0) - iofd->fd = fd; - else if (iofd->fd < 0) { + if (fd < 0 && iofd->fd < 0) { /* this might happen if both osmo_iofd_setup() and osmo_iofd_register() are called with -1 */ LOGPIO(iofd, LOGL_ERROR, "Cannot register io_fd using invalid fd == %d\n", iofd->fd); return -EBADF; } + if (fd < 0) + fd = iofd->fd; + else if (iofd->fd < 0) + iofd->fd = fd; + + if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_FD_REGISTERED)) { + /* If re-registering same fd, handle as NO-OP. + * New FD should go through unregister() first. */ + return iofd->fd == fd ? 0 : -ENOTSUP; + } rc = osmo_iofd_ops.register_fd(iofd); if (rc) return rc; IOFD_FLAG_UNSET(iofd, IOFD_FLAG_CLOSED); + IOFD_FLAG_SET(iofd, IOFD_FLAG_FD_REGISTERED); + if ((iofd->mode == OSMO_IO_FD_MODE_READ_WRITE && iofd->io_ops.read_cb) || (iofd->mode == OSMO_IO_FD_MODE_RECVFROM_SENDTO && iofd->io_ops.recvfrom_cb) || (iofd->mode == OSMO_IO_FD_MODE_RECVMSG_SENDMSG && iofd->io_ops.recvmsg_cb)) { @@ -772,7 +782,14 @@ */ int osmo_iofd_unregister(struct osmo_io_fd *iofd) { - return osmo_iofd_ops.unregister_fd(iofd); + int rc; + + if (!IOFD_FLAG_ISSET(iofd, IOFD_FLAG_FD_REGISTERED)) + return 0; + + rc = osmo_iofd_ops.unregister_fd(iofd); + IOFD_FLAG_UNSET(iofd, IOFD_FLAG_FD_REGISTERED); + return rc; } /*! Retrieve the number of messages pending in the transmit queue. diff --git a/src/core/osmo_io_poll.c b/src/core/osmo_io_poll.c index 92f03f8..bf0291e 100644 --- a/src/core/osmo_io_poll.c +++ b/src/core/osmo_io_poll.c @@ -128,29 +128,18 @@ struct osmo_fd *ofd = &iofd->u.poll.ofd; int rc; - if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_FD_REGISTERED)) - return 0; - osmo_fd_setup(ofd, iofd->fd, 0, &iofd_poll_ofd_cb_dispatch, iofd, 0); if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_NOTIFY_CONNECTED)) osmo_fd_write_enable(&iofd->u.poll.ofd); rc = osmo_fd_register(ofd); - if (!rc) - IOFD_FLAG_SET(iofd, IOFD_FLAG_FD_REGISTERED); - return rc; } static int iofd_poll_unregister(struct osmo_io_fd *iofd) { struct osmo_fd *ofd = &iofd->u.poll.ofd; - - if (!IOFD_FLAG_ISSET(iofd, IOFD_FLAG_FD_REGISTERED)) - return 0; osmo_fd_unregister(ofd); - IOFD_FLAG_UNSET(iofd, IOFD_FLAG_FD_REGISTERED); - return 0; } -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/39071?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id5e92aa22ce1c5d76028c539784118be227b9d5a Gerrit-Change-Number: 39071 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in libosmocore[master]: osmo_io: Simplify early return path
by pespin 09 Dec '24

09 Dec '24

pespin has abandoned this change. ( https://gerrit.osmocom.org/c/libosmocore/+/39065?usp=email ) Change subject: osmo_io: Simplify early return path ...................................................................... Abandoned -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/39065?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: abandon Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I985077cc0e2c11c4b6937f7e0f9bca61518716e0 Gerrit-Change-Number: 39065 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in libosmocore[master]: osmo_io: Simplify early return path
by pespin 09 Dec '24

09 Dec '24

pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmocore/+/39065?usp=email ) Change subject: osmo_io: Simplify early return path ...................................................................... Patch Set 1: Code-Review-2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/39065?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: I985077cc0e2c11c4b6937f7e0f9bca61518716e0 Gerrit-Change-Number: 39065 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 09 Dec 2024 17:30:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in libosmocore[master]: osmo_io: Use early return to simplify code
by pespin 09 Dec '24

09 Dec '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/39067?usp=email ) Change subject: osmo_io: Use early return to simplify code ...................................................................... osmo_io: Use early return to simplify code Change-Id: Idd4c4f2da7f15b86ddd4765c60680130af08b22d --- M src/core/osmo_io_uring.c 1 file changed, 19 insertions(+), 17 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/67/39067/1 diff --git a/src/core/osmo_io_uring.c b/src/core/osmo_io_uring.c index 569f150..1b1f997 100644 --- a/src/core/osmo_io_uring.c +++ b/src/core/osmo_io_uring.c @@ -498,24 +498,26 @@ 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); - if (osmo_fd_register(&iofd->u.uring.connect_ofd) < 0) - LOGPIO(iofd, LOGL_ERROR, "Failed to register FD for connect event.\n"); - else - IOFD_FLAG_SET(iofd, IOFD_FLAG_NOTIFY_CONNECTED); - } - } else + if (iofd->mode != OSMO_IO_FD_MODE_RECVMSG_SENDMSG) { iofd_uring_write_enable(iofd); + return; + } + + /* 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); + if (osmo_fd_register(&iofd->u.uring.connect_ofd) < 0) + LOGPIO(iofd, LOGL_ERROR, "Failed to register FD for connect event.\n"); + else + IOFD_FLAG_SET(iofd, IOFD_FLAG_NOTIFY_CONNECTED); + } } const struct iofd_backend_ops iofd_uring_ops = { -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/39067?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Idd4c4f2da7f15b86ddd4765c60680130af08b22d Gerrit-Change-Number: 39067 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in libosmocore[master]: osmo_io: segmentation cb: Initialize null ptr every iteration
by pespin 09 Dec '24

09 Dec '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/39066?usp=email ) Change subject: osmo_io: segmentation cb: Initialize null ptr every iteration ...................................................................... osmo_io: segmentation cb: Initialize null ptr every iteration Change-Id: I0fcaa6afb9cf2f88243280f01c278fa2d72e69a6 --- M src/core/osmo_io.c 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/66/39066/1 diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index bff86d2..f374cf4 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -328,7 +328,7 @@ void iofd_handle_segmented_read(struct osmo_io_fd *iofd, struct msgb *msg, int rc) { int res; - struct msgb *pending = NULL; + struct msgb *pending; OSMO_ASSERT(iofd->mode == OSMO_IO_FD_MODE_READ_WRITE); @@ -338,6 +338,7 @@ } do { + pending = NULL; res = iofd_handle_segmentation(iofd, msg, &pending); if (res != IOFD_SEG_ACT_DEFER || rc < 0) iofd->io_ops.read_cb(iofd, rc, msg); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/39066?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0fcaa6afb9cf2f88243280f01c278fa2d72e69a6 Gerrit-Change-Number: 39066 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in libosmocore[master]: osmo_io: Simplify early return path
by pespin 09 Dec '24

09 Dec '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/39065?usp=email ) Change subject: osmo_io: Simplify early return path ...................................................................... osmo_io: Simplify early return path Change-Id: I985077cc0e2c11c4b6937f7e0f9bca61518716e0 --- M src/core/osmo_io.c 1 file changed, 2 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/65/39065/1 diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index 9c93e25..bff86d2 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -736,13 +736,12 @@ { int rc = 0; - if (fd >= 0) - iofd->fd = fd; - else if (iofd->fd < 0) { + if (iofd->fd < 0) { /* this might happen if both osmo_iofd_setup() and osmo_iofd_register() are called with -1 */ LOGPIO(iofd, LOGL_ERROR, "Cannot register io_fd using invalid fd == %d\n", iofd->fd); return -EBADF; } + iofd->fd = fd; rc = osmo_iofd_ops.register_fd(iofd); if (rc) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/39065?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I985077cc0e2c11c4b6937f7e0f9bca61518716e0 Gerrit-Change-Number: 39065 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[L] Change in libosmo-netif[master]: ipa: Add osmo_ipa_ka_fsm_inst APIs
by pespin 09 Dec '24

09 Dec '24

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/39035?usp=email ) Change subject: ipa: Add osmo_ipa_ka_fsm_inst APIs ...................................................................... ipa: Add osmo_ipa_ka_fsm_inst APIs This new object and its APIs are meant to replace ipa_keepalive_fsm_* APIs from libosmo-abis, because: 1- libosmo-abis is not the proper place to have IPA generic code, since other protocols than Abis also use IPA underneath. 2- the existing API in libosmo-abis is actually a set of 3 interfaces, 2 of which rely on ipa_client_conn and ipa_server_conn, which are provided by libosmo-abis itself and which will eventually be deprecated. Hence, only the "generic" one is actually reimplemented here, so it is backend agnsotic. A notable difference from the FSM implementation in libosmo-abis is that this fsm instnace object doesn't terminate itself, but leaves the decision to the user, in order to allow for more flexibility during object destruction. Change-Id: I5c36e06e0dc29ec4679b20ad6c426f051b659acd --- M TODO-RELEASE M include/osmocom/netif/ipa.h M src/Makefile.am A src/ipa_keepalive.c 4 files changed, 379 insertions(+), 0 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved osmith: Looks good to me, but someone else must approve diff --git a/TODO-RELEASE b/TODO-RELEASE index 268b142..5092ee7 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -9,3 +9,4 @@ #library what description / commit summary line libosmo-netif add API osmo_stream_cli_set_{ip_dscp,priority}(), osmo_stream_srv_link_set_{ip_dscp,priority}() libosmo-netif add API osmo-stream_cli_set_tx_queue_max_length(), osmo_stream_srv_link_set_tx_queue_max_length() +libosmo-netif add API struct osmo_ipa_ka_fsm_inst \ No newline at end of file diff --git a/include/osmocom/netif/ipa.h b/include/osmocom/netif/ipa.h index 038b9ac..6ec75f8 100644 --- a/include/osmocom/netif/ipa.h +++ b/include/osmocom/netif/ipa.h @@ -67,4 +67,28 @@ void osmo_ipa_msg_push_headers(struct msgb *msg, enum ipaccess_proto p, enum ipaccess_proto_ext pe); +/*********************************************************************** + * IPA Keep-Alive FSM + ***********************************************************************/ +struct osmo_ipa_ka_fsm_inst; +typedef int (*osmo_ipa_ka_fsm_timeout_cb_t)(struct osmo_ipa_ka_fsm_inst *ka_fi, void *data); + +typedef int (*osmo_ipa_ka_fsm_send_cb_t)(struct osmo_ipa_ka_fsm_inst *ka_fi, struct msgb *msg, void *data); + +struct osmo_ipa_ka_fsm_inst *osmo_ipa_ka_fsm_alloc(void *ctx, const char *id); +void osmo_ipa_ka_fsm_free(struct osmo_ipa_ka_fsm_inst *ka_fi); + +int osmo_ipa_ka_fsm_set_id(struct osmo_ipa_ka_fsm_inst *ka_fi, const char *id); +int osmo_ipa_ka_fsm_set_ping_interval(struct osmo_ipa_ka_fsm_inst *ka_fi, unsigned int interval); +int osmo_ipa_ka_fsm_set_pong_timeout(struct osmo_ipa_ka_fsm_inst *ka_fi, unsigned int timeout); +void osmo_ipa_ka_fsm_set_data(struct osmo_ipa_ka_fsm_inst *ka_fi, void *cb_data); +void *osmo_ipa_ka_fsm_get_data(const struct osmo_ipa_ka_fsm_inst *ka_fi); + +void osmo_ipa_ka_fsm_set_send_cb(struct osmo_ipa_ka_fsm_inst *ka_fi, osmo_ipa_ka_fsm_send_cb_t send_cb); +void osmo_ipa_ka_fsm_set_timeout_cb(struct osmo_ipa_ka_fsm_inst *ka_fi, osmo_ipa_ka_fsm_timeout_cb_t timeout_cb); + +void osmo_ipa_ka_fsm_start(struct osmo_ipa_ka_fsm_inst *ka_fi); +void osmo_ipa_ka_fsm_pong_received(struct osmo_ipa_ka_fsm_inst *ka_fi); +void osmo_ipa_ka_fsm_stop(struct osmo_ipa_ka_fsm_inst *ka_fi); + #endif diff --git a/src/Makefile.am b/src/Makefile.am index 258d04a..bd3f272 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -14,6 +14,7 @@ libosmonetif_la_SOURCES = amr.c \ datagram.c \ ipa.c \ + ipa_keepalive.c \ ipa_unit.c \ jibuf.c \ osmux.c \ diff --git a/src/ipa_keepalive.c b/src/ipa_keepalive.c new file mode 100644 index 0000000..ac056ed --- /dev/null +++ b/src/ipa_keepalive.c @@ -0,0 +1,353 @@ +/* IPA keep-alive FSM; Periodically transmit IPA_PING and expect IPA_PONG in return. + * + * (C) 2024 by sysmocom - s.f.m.c. GmbH <info(a)sysmocom.de + * (C) 2019 by Harald Welte <laforge(a)gnumonks.org> + * + * All Rights Reserved + * + * SPDX-License-Identifier: GPL-2.0+ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <osmocom/core/fsm.h> +#include <osmocom/core/timer.h> +#include <osmocom/core/msgb.h> +#include <osmocom/core/logging.h> + +#include <osmocom/gsm/protocol/ipaccess.h> + +#include <osmocom/netif/ipa.h> + +#define KA_FI_DEFAULT_PING_INTERVAL 30 /* seconds */ +#define KA_FI_DEFAULT_PONG_TIMEOUT 10 /* seconds */ + +enum osmo_ipa_keepalive_event { + OSMO_IPA_KA_E_START, + OSMO_IPA_KA_E_STOP, + OSMO_IPA_KA_E_PONG, +}; + +static const struct value_string ipa_keepalive_event_names[] = { + OSMO_VALUE_STRING(OSMO_IPA_KA_E_START), + OSMO_VALUE_STRING(OSMO_IPA_KA_E_STOP), + OSMO_VALUE_STRING(OSMO_IPA_KA_E_PONG), + { 0, NULL } +}; + +struct osmo_ipa_ka_fsm_inst { + struct osmo_fsm_inst *fi; + /*! interval in which to send IPA CCM PING requests to the peer. */ + unsigned int ping_interval; + /*! time to wait for an IPA CCM PONG in response to a IPA CCM PING before giving up. */ + unsigned int pong_timeout; + osmo_ipa_ka_fsm_send_cb_t send_cb; + osmo_ipa_ka_fsm_timeout_cb_t timeout_cb; + void *cb_data; +}; + +/* generate a msgb containing an IPA CCM PING message */ +static struct msgb *gen_ipa_ping(void) +{ + struct msgb *msg = msgb_alloc_headroom(64, 32, "IPA PING"); + if (!msg) + return NULL; + + msgb_put_u8(msg, IPAC_MSGT_PING); + ipa_prepend_header(msg, IPAC_PROTO_IPACCESS); + + return msg; +} + +/******** + * FSM: + *******/ + +#define S(x) (1 << (x)) + +enum osmo_ipa_keepalive_state { + OSMO_IPA_KA_S_INIT, + OSMO_IPA_KA_S_IDLE, /* waiting for next interval */ + OSMO_IPA_KA_S_WAIT_RESP, /* waiting for response to keepalive */ +}; + +enum ipa_fsm_timer { + T_SEND_NEXT_PING = 1, + T_PONG_NOT_RECEIVED = 2, +}; + +static void ipa_ka_init(struct osmo_fsm_inst *fi, uint32_t event, void *data) +{ + struct osmo_ipa_ka_fsm_inst *ka_fi = fi->priv; + + switch (event) { + case OSMO_IPA_KA_E_START: + osmo_fsm_inst_state_chg(fi, OSMO_IPA_KA_S_WAIT_RESP, + ka_fi->pong_timeout, T_PONG_NOT_RECEIVED); + break; + default: + OSMO_ASSERT(0); + break; + } +} + +static void ipa_ka_wait_resp_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state) +{ + struct osmo_ipa_ka_fsm_inst *ka_fi = fi->priv; + struct msgb *msg; + + if (!ka_fi->send_cb) + osmo_panic("osmo_ipa_ka_fsm_inst running without send_cb, fix your code!"); + + /* Send an IPA PING to the peer */ + msg = gen_ipa_ping(); + OSMO_ASSERT(msg); + + ka_fi->send_cb(ka_fi, msg, ka_fi->cb_data); +} + +static void ipa_ka_wait_resp(struct osmo_fsm_inst *fi, uint32_t event, void *data) +{ + struct osmo_ipa_ka_fsm_inst *ka_fi = fi->priv; + + switch (event) { + case OSMO_IPA_KA_E_PONG: + osmo_fsm_inst_state_chg(fi, OSMO_IPA_KA_S_IDLE, + ka_fi->ping_interval, T_SEND_NEXT_PING); + break; + default: + OSMO_ASSERT(0); + } +} + +static int ipa_ka_fsm_timer_cb(struct osmo_fsm_inst *fi) +{ + struct osmo_ipa_ka_fsm_inst *ka_fi = fi->priv; + + switch (fi->T) { + case T_SEND_NEXT_PING: + /* send another PING */ + osmo_fsm_inst_state_chg(fi, OSMO_IPA_KA_S_WAIT_RESP, + ka_fi->pong_timeout, T_PONG_NOT_RECEIVED); + return 0; + case T_PONG_NOT_RECEIVED: + /* PONG not received within time */ + LOGPFSML(fi, LOGL_NOTICE, "IPA keep-alive FSM timed out: PONG not received\n"); + /* Keep FSM alive, move to INIT state */ + osmo_fsm_inst_state_chg(fi, OSMO_IPA_KA_S_INIT, 0, 0); + if (ka_fi->timeout_cb) + ka_fi->timeout_cb(ka_fi, ka_fi->cb_data); + return 0; + default: + OSMO_ASSERT(0); + } +} + +static void ipa_ka_allstate_action(struct osmo_fsm_inst *fi, uint32_t event, void *data) +{ + switch (event) { + case OSMO_IPA_KA_E_STOP: + osmo_fsm_inst_state_chg(fi, OSMO_IPA_KA_S_INIT, 0, 0); + break; + default: + OSMO_ASSERT(0); + break; + } +} + +static const struct osmo_fsm_state ipa_keepalive_states[] = { + [OSMO_IPA_KA_S_INIT] = { + .name = "INIT", + .in_event_mask = S(OSMO_IPA_KA_E_START), + .out_state_mask = S(OSMO_IPA_KA_S_WAIT_RESP) | S(OSMO_IPA_KA_S_INIT), + .action = ipa_ka_init, + }, + [OSMO_IPA_KA_S_IDLE] = { + .name = "IDLE", + .out_state_mask = S(OSMO_IPA_KA_S_WAIT_RESP) | S(OSMO_IPA_KA_S_INIT), + /* no permitted events aside from E_STOP, which is handled in allstate_events */ + }, + [OSMO_IPA_KA_S_WAIT_RESP] = { + .name = "WAIT_RESP", + .in_event_mask = S(OSMO_IPA_KA_E_PONG), + .out_state_mask = S(OSMO_IPA_KA_S_IDLE) | S(OSMO_IPA_KA_S_INIT), + .action = ipa_ka_wait_resp, + .onenter = ipa_ka_wait_resp_onenter, + }, +}; + +static struct osmo_fsm ipa_keepalive_fsm = { + .name = "IPA-KA", + .states = ipa_keepalive_states, + .num_states = ARRAY_SIZE(ipa_keepalive_states), + .log_subsys = DLINP, + .allstate_event_mask = S(OSMO_IPA_KA_E_STOP), + .allstate_action = ipa_ka_allstate_action, + .event_names = ipa_keepalive_event_names, + .timer_cb = ipa_ka_fsm_timer_cb, +}; + +static __attribute__((constructor)) void on_dso_load(void) +{ + OSMO_ASSERT(osmo_fsm_register(&ipa_keepalive_fsm) == 0); +} + + +/***************************** + * osmo_ipa_ka_fsm_inst APIS +******************************/ + +/*! Create a new instance of an IPA keepalive FSM: Periodically transmit PING and expect PONG. + * \param[in] ctx Talloc context. + * \param[in] id String used as identifier for the FSM. + * \returns pointer to the newly-created FSM instance; NULL in case of error. + * + * Must be freed with \ref osmo_ipa_ka_fsm_free() + */ +struct osmo_ipa_ka_fsm_inst *osmo_ipa_ka_fsm_alloc(void *ctx, const char *id) +{ + struct osmo_ipa_ka_fsm_inst *ka_fi; + + ka_fi = talloc_zero(ctx, struct osmo_ipa_ka_fsm_inst); + if (!ka_fi) + goto ret_free; + + ka_fi->fi = osmo_fsm_inst_alloc(&ipa_keepalive_fsm, ka_fi, NULL, LOGL_DEBUG, id); + if (!ka_fi->fi) + goto ret_free; + ka_fi->fi->priv = ka_fi; + + ka_fi->ping_interval = KA_FI_DEFAULT_PING_INTERVAL; + ka_fi->pong_timeout = KA_FI_DEFAULT_PONG_TIMEOUT; + + return ka_fi; + +ret_free: + talloc_free(ka_fi); + return NULL; +} + +/*! Free object allocated through \ref osmo_ipa_ka_fsm_alloc(). + * \param[in] ka_fi IPA keepalive FSM instance. + * + * Does nothing if NULL is passed. + */ +void osmo_ipa_ka_fsm_free(struct osmo_ipa_ka_fsm_inst *ka_fi) +{ + if (!ka_fi) + return; + + osmo_fsm_inst_free(ka_fi->fi); + ka_fi->fi = NULL; + + talloc_free(ka_fi); +} + +/*! Set name id of the IPA keepalive FSM instance. + * \param[in] ka_fi IPA keepalive FSM instance. + * \param[in] id Name used during logging. + * \returns zero on success, negative on error. + */ +int osmo_ipa_ka_fsm_set_id(struct osmo_ipa_ka_fsm_inst *ka_fi, const char *id) +{ + return osmo_fsm_inst_update_id(ka_fi->fi, id); +} + +/*! Set PING interval value. + * \param[in] ka_fi IPA keepalive FSM instance. + * \param[in] interval PING interval value, in seconds. + * \returns zero on success, negative on error. + */ +int osmo_ipa_ka_fsm_set_ping_interval(struct osmo_ipa_ka_fsm_inst *ka_fi, unsigned int interval) +{ + ka_fi->ping_interval = interval; + return 0; +} + +/*! Set PONG timeout value. + * \param[in] ka_fi IPA keepalive FSM instance. + * \param[in] timeout PONG timeout value, in seconds. + * \returns zero on success, negative on error. + */ +int osmo_ipa_ka_fsm_set_pong_timeout(struct osmo_ipa_ka_fsm_inst *ka_fi, unsigned int timeout) +{ + ka_fi->pong_timeout = timeout; + return 0; +} + +/*! Set user private data which can be used by user of osmo_ipa_ka_fsm. + * \param[in] ka_fi IPA keepalive FSM instance. + * \param[in] cb_data User private data pointer. + */ +void osmo_ipa_ka_fsm_set_data(struct osmo_ipa_ka_fsm_inst *ka_fi, void *cb_data) +{ + ka_fi->cb_data = cb_data; +} + +/*! Get user private data set previously throuhg \ref osmo_ipa_ka_fsm_set_data. + * \param[in] ka_fi IPA keepalive FSM instance. + */ +void *osmo_ipa_ka_fsm_get_data(const struct osmo_ipa_ka_fsm_inst *ka_fi) +{ + return ka_fi->cb_data; +} + +/*! Set a custom send callback for sending pings + * \param[in] ka_fi IPA keepalive FSM instance. + * \param[in] send_cb Function to call whenever a PING needs to be sent (present in msgb param). + */ +void osmo_ipa_ka_fsm_set_send_cb(struct osmo_ipa_ka_fsm_inst *ka_fi, osmo_ipa_ka_fsm_send_cb_t send_cb) +{ + ka_fi->send_cb = send_cb; +} + +/*! Set a timeout call-back which is to be called once the peer doesn't respond anymore. + * \param[in] ka_fi IPA keepalive FSM instance. + * \param[in] timeout_cb Function to call whenever PONG timeout occurs. + * + * When the PONG timeout occurs, the FSM will stop and transition to INITIAL + * state prior to triggering the timeout_cb(). This lets the user either destroy + * the FSM (|ref osmo_ipa_ka_fsm_free()) or restart it (\ref osmo_ipa_ka_fsm_start()). + */ +void osmo_ipa_ka_fsm_set_timeout_cb(struct osmo_ipa_ka_fsm_inst *ka_fi, osmo_ipa_ka_fsm_timeout_cb_t timeout_cb) +{ + ka_fi->timeout_cb = timeout_cb; +} + +/*! Start the ping/pong procedure of the IPA Keepalive FSM. + * \param[in] ka_fi IPA keepalive FSM instance. + */ +void osmo_ipa_ka_fsm_start(struct osmo_ipa_ka_fsm_inst *ka_fi) +{ + struct osmo_fsm_inst *fi = ka_fi->fi; + LOGPFSML(fi, LOGL_INFO, "Starting IPA keep-alive FSM (interval=%us wait=%us)\n", + ka_fi->ping_interval, ka_fi->pong_timeout); + osmo_fsm_inst_dispatch(fi, OSMO_IPA_KA_E_START, NULL); +} + +/*! Inform IPA Keepalive FSM that a PONG has been received. + * \param[in] ka_fi IPA keepalive FSM instance. + */ +void osmo_ipa_ka_fsm_pong_received(struct osmo_ipa_ka_fsm_inst *ka_fi) +{ + osmo_fsm_inst_dispatch(ka_fi->fi, OSMO_IPA_KA_E_PONG, NULL); +} + +/*! Stop the ping/pong procedure of the IPA Keepalive FSM. + * \param[in] ka_fi IPA keepalive FSM instance. + */ +void osmo_ipa_ka_fsm_stop(struct osmo_ipa_ka_fsm_inst *ka_fi) +{ + struct osmo_fsm_inst *fi = ka_fi->fi; + LOGPFSML(fi, LOGL_INFO, "Stopping IPA keep-alive FSM\n"); + osmo_fsm_inst_dispatch(fi, OSMO_IPA_KA_E_STOP, NULL); +} -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/39035?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I5c36e06e0dc29ec4679b20ad6c426f051b659acd Gerrit-Change-Number: 39035 Gerrit-PatchSet: 5 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

2025

2024

2023

2022

gerrit-log December 2024