Attention is currently required from: Hoernchen, fixeria, laforge.
pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-uecups/+/40787?usp=email )
Change subject: tun_device: Make sure struct iphdr access is 4-byte aligned
......................................................................
Patch Set 2:
(2 comments)
Patchset:
PS2:
> What about __attribute__((aligned(4))) or aligned alloca? This looks as brittle as the first version […]
Because I don't want to align the start of the buffer variable, but make sure an intermediate point in the buffer is aligned.
File daemon/tun_device.c:
https://gerrit.osmocom.org/c/osmo-uecups/+/40787/comment/999416a8_ffe92148?… :
PS2, Line 208: uint8_t base_buffer[payload_off_4byte_aligned + MAX_UDP_PACKET];
> I would expect there are nice compiler pragmas/macros that allow for such alignment? I know the Linu […]
It's not about the initial buffer address, which I expect to be aligned, but the middle pointer where the iphdr gets stored.
--
To view, visit https://gerrit.osmocom.org/c/osmo-uecups/+/40787?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-uecups
Gerrit-Branch: master
Gerrit-Change-Id: I610c8e9b150c234b7d4997e0b1c4d4a9ce4de9ec
Gerrit-Change-Number: 40787
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 31 Jul 2025 14:47:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hoernchen <ewild(a)sysmocom.de>
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
laforge has submitted this change. ( https://gerrit.osmocom.org/c/erlang/osmo_dia2gsup/+/20020?usp=email )
Change subject: osmo_dia2gsup.app.src: Add missing 'crypto'
......................................................................
osmo_dia2gsup.app.src: Add missing 'crypto'
In a new clean environment initialized with rebar3,
I ran into dependency errors related to missing libraries.
Change-Id: I4a6b963daf64695acd626a434bc4718fb5be40d6
---
0 files changed, 0 insertions(+), 0 deletions(-)
Approvals:
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
laforge: Looks good to me, approved
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo_dia2gsup/+/20020?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: erlang/osmo_dia2gsup
Gerrit-Branch: master
Gerrit-Change-Id: I4a6b963daf64695acd626a434bc4718fb5be40d6
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 3
Gerrit-Owner: matt9j <matt9j(a)cs.washington.edu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
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 6: Code-Review-1
(5 comments)
File src/core/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/6799a0a9_4e2cf970?… :
PS6, Line 116: /* ! array of rings the submitted read SQEs have been submitted */
> "have been submitted to"
same comment like in the other patch regarding "*!" vs "* !"
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/14dd5fc1_ad400304?… :
PS2, Line 175:
> No, it doesn't, because I don't track the number of submissions and the number of completions. […]
ok, then please mention this fact explicitly in this c ommit in a TODO-comment and also in the commit log. At least this way it's clear that there is no related cleanup yet. Implementation could then follow at a later patch, but at least it is known to everyone that this is the situation.
I would consider having a single per-ring counter of the number of not-yet-completed sqe's would be sufficient. If that counter reaches zero, and we are not the currently active ring, we can destroy it?
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/c5d522d0_aeb3e8ef?… :
PS6, Line 172: sqe = io_uring_get_sqe(&g_ring->ring);
> so here we end up with 2 read SQEs on 2 different queues. […]
does it matter? Do we ever care? I think there's no such guarantee even with two read sqe's for the same fd on a single ring? In the end, they are all empty msgb's waiting for some incoming data. It doesn't matter which is used/filled/reported-completed first?
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/5501dd65_4c9eabef?… :
PS6, Line 527: while ((sqe = io_uring_get_sqe(iofd->u.uring.read_ring[idx])) == NULL);
> that looks like a really nice tight non-blocking loop filling the CPU?
yeah. It's one problem stalling the process itself by waiting for something to happen eventually - it's another problem to waste a full CPU core while doing that, potentially slowing down the event that we're waiting for.
It seems `io_uring_sqring_wait` was introduced for this: https://man7.org/linux/man-pages/man3/io_uring_sqring_wait.3.html
- we may have to check
1) at compile time that a sufficient enough liburing version is present
2) at runtime that the kernel supports it (-EINVAL is returned otherwise)
It might also make sense to log something just before we call io_uiring_sqring_wait... that might help debugging if we ever ended up spending significant time in it.
So my strategy would be:
* fast path trying io_uring_get_sqe
* only if that fails, log the situation and use io_uring_sqring_wait
* if io_uring_sqring_wait fails, fall-back to busy-waiting, but maybe with some usleep in the loop
Alternatively, we could try calling io_uring_sqring_wait once at ring creation time (when we know it will never block as there are always free SQEs inside), and cache whether or not it is supported (positive value) or unsupported (-EINVAL) in a global variable. The code here and below could then avoid the trial-and-error every time we unregister an fd.
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/c930b6b7_3dd558ea?… :
PS6, Line 543: while ((sqe = io_uring_get_sqe(iofd->u.uring.write_ring)) == NULL);
> that looks like a really nice tight non-blocking loop filling the CPU?
same as above
--
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: 6
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: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 31 Jul 2025 14:00:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: jolly <andreas(a)eversberg.eu>
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>