Attention is currently required from: jolly, laforge.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/35910?usp=email )
Change subject: osmo_io: Use poll/select to notify socket connection at osmo_io_uring.c
......................................................................
Patch Set 4:
(4 comments)
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/35910/comment/62900f2c_f007201e
PS4, Line 327: if (osmo_fd_is_registered(&iofd->u.uring.connect_ofd))
Again this is expensive! Should be removed (or conditionally enabled through ifdef).
https://gerrit.osmocom.org/c/libosmocore/+/35910/comment/0b38f7db_8c1db15b
PS4, Line 341: if (osmo_fd_is_registered(&iofd->u.uring.connect_ofd))
Again this is expensive! Should be removed (or conditionally enabled through ifdef).
https://gerrit.osmocom.org/c/libosmocore/+/35910/comment/6ebeb0dc_70109e55
PS4, Line 398: if (osmo_fd_is_registered(&iofd->u.uring.connect_ofd))
Again this is expensive! Should be removed (or conditionally enabled through ifdef).
https://gerrit.osmocom.org/c/libosmocore/+/35910/comment/cd75808a_4abffff4
PS4, Line 498: /* Use a temporary osmo_fd which we can use to notify us once the
connection is established
I am not sure this is correct. According to man2 connect, the way to go is:
"""
The socket is nonblocking and the connection cannot be completed
immediately. (UNIX domain sockets failed with EAGAIN instead.) It is pos‐
sible to select(2) or poll(2) for completion by selecting the socket for
writing. After select(2) indicates writability, use getsockopt(2)
to read the SO_ERROR option at level SOL_SOCKET to determine whether
connect() completed successfully (SO_ERROR is zero) or unsuccessfully
(SO_ERROR is one of the usual error codes listed here, explaining the reason
for the failure).
"""
I don't see anywhere the mention of "fd becoming readable" as you said.
Seems you should be checking the SO_ERROR instead?
--
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: 4
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-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Tue, 20 Feb 2024 10:19:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment