[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
daniel has submitted this change. (
https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
..
Add osmo_io with initial poll backend
* make backend configurable for later
* segmentation callback for chunked streams
* logging target for osmo_io
* support partial writes
Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0
Related: SYS#5094, OS#5751
---
M TODO-RELEASE
M include/osmocom/core/Makefile.am
M include/osmocom/core/logging.h
A include/osmocom/core/osmo_io.h
M src/core/Makefile.am
M src/core/libosmocore.map
M src/core/logging.c
A src/core/osmo_io.c
A src/core/osmo_io_internal.h
A src/core/osmo_io_poll.c
M tests/logging/logging_vty_test.vty
11 files changed, 1,064 insertions(+), 3 deletions(-)
Approvals:
laforge: Looks good to me, approved
fixeria: Looks good to me, but someone else must approve
dexter: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/TODO-RELEASE b/TODO-RELEASE
index bade6f5..7270257 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -10,3 +10,4 @@
libosmogsm new header osmocom/gsm/protocol/gsm_44_060.h
libosmocore ADD new defines in osmocom/gsm/protocol/gsm_04_08.h (old ones
marked deprecated)
libosmovty drop APIdrop struct vty_parent_node from public
API, it should never have been public
+libosmocore ADD new API osmo_io_*()
diff --git a/include/osmocom/core/Makefile.am b/include/osmocom/core/Makefile.am
index e1cd92a..1d394cd 100644
--- a/include/osmocom/core/Makefile.am
+++ b/include/osmocom/core/Makefile.am
@@ -37,6 +37,7 @@
msgb.h \
netdev.h \
netns.h \
+ osmo_io.h \
panic.h \
prbs.h \
prim.h \
diff --git a/include/osmocom/core/logging.h b/include/osmocom/core/logging.h
index 755564d..e8433a1 100644
--- a/include/osmocom/core/logging.h
+++ b/include/osmocom/core/logging.h
@@ -154,7 +154,8 @@
#define DLCSN1 -26 /*!< CSN.1 (Concrete Syntax Notation 1) codec */
#define DLM2PA -27 /*!< Osmocom M2PA (libosmo-sigtran) */
#define DLM2UA -28 /*!< Reserved for future Osmocom M2UA
(libosmo-sigtran) */
-#define OSMO_NUM_DLIB 28 /*!< Number of logging sub-systems in libraries
*/
+#define DLIO -29 /*!< Osmocom IO sub-system */
+#define OSMO_NUM_DLIB 29 /*!< Number of logging sub-systems in libraries
*/
/* Colors that can be used in log_info_cat.color */
#define OSMO_LOGCOLOR_NORMAL NULL
diff --git a/include/osmocom/core/osmo_io.h b/include/osmocom/core/osmo_io.h
new file mode 100644
index 000..ffc8cfa
--- /dev/null
+++ b/include/osmocom/core/osmo_io.h
@@ -0,0 +1,90 @@
+/*! \file osmo_io.h
+ * io(_uring) abstraction osmo fd compatibility
+ */
+
+#pragma once
+
+#include
+#include
+#include
+#include
+#include
+
+
+#define LOGPIO(iofd, level, fmt, args...) \
+ LOGP(DLIO, level, "iofd(%s)" fmt, iofd->name, ## args)
+
+struct osmo_io_fd;
+
+enum osmo_io_fd_mode {
+ /*! use read() / write() calls */
+ OSMO_IO_FD_MODE_READ_WRITE,
+ /*! use recvfrom() / sendto() calls */
+ OSMO_IO_FD_MODE_RECVFROM_SENDTO,
+ /*! emulate sctp_recvmsg() and sctp_sendmsg() */
+ OSMO_IO_FD_MODE_SCTP_RECVMSG_SENDMSG,
+};
+
+enum osmo_io_backend {
+ OSMO_IO_BACKEND_POLL,
+};
+
+extern const struct value_string osmo_io_backend_names[];
+static inline const char *osmo_io_backend_name(enum osmo_io_backend val)
+{ return get_value_string(osmo_io_backend_names, val); }
+
+struct osmo_io_ops {
+ union {
+ /* mode OSMO_IO_FD_MODE_READ_WRITE: */
+ struct {
+ /*! call-back function when something was read from fd
*/
+ void (*read_cb)(struct osmo_io_fd *iofd, int res,
struct msgb *msg);
+ /*! call-back function when write has completed on fd */
+ void (*write_cb)(struct osmo_io_fd *iofd, int res,
+const struct msgb *msg);
+ /*! call-back function to segment the data returned by
read_cb */
+ int (*segmentation_cb)(struct msgb *msg, int data_len);
+ };
+
+ /* mode OSMO_IO_FD_MODE_RECVFROM_SENDTO: */
+ struct {
+ /*! call-back function emulating sendto */
+ void (*sendto_cb)(struct osmo_io_fd *iofd, int res,
+ const struct msgb *msg,
+ const struct osmo_sockaddr *daddr);
+ /*! call-back function emulating recvfrom */
+ void (*recvfrom_cb)(struct osmo_io_fd *iofd, int res,
+ struct msgb *msg,
+ const struct osmo_sockaddr *saddr);
+ };
+ };
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, neels, fixeria, pespin, daniel. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 17: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 17 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: neels Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 09 May 2023 23:20:37 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, neels, laforge, fixeria, pespin, daniel. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 17: Code-Review+1 (1 comment) Patchset: PS17: looks very clean to me, but unfortunately I can not say too much since I do not have the full background. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 17 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 09 May 2023 15:24:27 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, neels, laforge, fixeria, pespin, daniel. Hoernchen has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 17: (1 comment) File src/core/osmo_io_poll.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/a0dd9200_50a768d5 PS14, Line 44: inline > Sure, we can always re-add it if it becomes an issue "inline" does not force anything anyway and causes as much inlining as "static" - it just specifies linkage. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 17 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 09 May 2023 09:47:30 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, daniel. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 17: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 17 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 09 May 2023 09:37:40 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. Hello osmith, Jenkins Builder, Hoernchen, neels, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/30934 to look at the new patch set (#17). Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/osmocom/core/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/core/Makefile.am M src/core/libosmocore.map M src/core/logging.c A src/core/osmo_io.c A src/core/osmo_io_internal.h A src/core/osmo_io_poll.c M tests/logging/logging_vty_test.vty 11 files changed, 1,064 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/17 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 17 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-MessageType: newpatchset
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 17: (2 comments) File src/core/osmo_io_internal.h: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/1dc4c5be_96f3577e PS14, Line 40: /*! flags to guard closing/freeing of iofd */ > Not saying this needs to be done, but IMO a bitmask would be a better fit > here because adding new fl […] Sounds good, I added a TODO in my local repo so I don't forget. Still like to push this for now. File src/core/osmo_io_poll.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/17951f7b_269e6a88 PS14, Line 44: inline > Do we really need to force inlining here? Given that this function is used > only once, compilers will […] Sure, we can always re-add it if it becomes an issue -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 17 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Tue, 09 May 2023 08:07:02 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. Hello osmith, Jenkins Builder, Hoernchen, neels, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/30934 to look at the new patch set (#16). Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/osmocom/core/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/core/Makefile.am M src/core/libosmocore.map M src/core/logging.c A src/core/osmo_io.c A src/core/osmo_io_internal.h A src/core/osmo_io_poll.c M tests/logging/logging_vty_test.vty 11 files changed, 1,064 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/16 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 16 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-MessageType: newpatchset
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. Hello osmith, Jenkins Builder, Hoernchen, neels, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/30934 to look at the new patch set (#15). Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/osmocom/core/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/core/Makefile.am M src/core/libosmocore.map M src/core/logging.c A src/core/osmo_io.c A src/core/osmo_io_internal.h A src/core/osmo_io_poll.c M tests/logging/logging_vty_test.vty 11 files changed, 1,065 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/15 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 15 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-MessageType: newpatchset
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge,
pespin, fixeria.
daniel has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
..
Patch Set 15:
(6 comments)
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/eb053956_89f2ab50
PS14, Line 150: msg = iofd->pending;
> use `msg = iofd_msgb_pending(iofd)` here?
Ack
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/0c8a24b0_8bd03c54
PS14, Line 324: flags
> `sendto_flags`
Done
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/7e0004c8_5823af6f
PS14, Line 516: underlyiny
> typo: underlying
Done
File src/core/osmo_io_poll.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/64aaa79d_d159d9e9
PS14, Line 175: struct iofd_backend_ops iofd_poll_ops = {
> const?
Ack
File src/osmo_io_poll.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/7b4a10a7_6bfa17b7
PS3, Line 84: /
> This seems to map to a POLLPRI event in poll() […]
Marking resolved
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/89486973_cd65446e
PS3, Line 143: /
> Ack
Done
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0
Gerrit-Change-Number: 30934
Gerrit-PatchSet: 15
Gerrit-Owner: daniel
Gerrit-Reviewer: Hoernchen
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: neels
Gerrit-Reviewer: osmith
Gerrit-Reviewer: pespin
Gerrit-CC: laforge
Gerrit-Attention: osmith
Gerrit-Attention: Hoernchen
Gerrit-Attention: neels
Gerrit-Attention: laforge
Gerrit-Attention: pespin
Gerrit-Attention: fixeria
Gerrit-Comment-Date: Tue, 09 May 2023 06:12:27 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge
Comment-In-Reply-To: fixeria
Comment-In-Reply-To: pespin
Comment-In-Reply-To: daniel
Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, daniel.
fixeria has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
..
Patch Set 14:
(6 comments)
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/5f8e6f97_4bec484d
PS14, Line 150: msg = iofd->pending;
use `msg = iofd_msgb_pending(iofd)` here?
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/d2ae9a37_d45f3a58
PS14, Line 324: flags
`sendto_flags`
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/e3c76b19_80f9ef5b
PS14, Line 516: underlyiny
typo: underlying
File src/core/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/8220d3f6_846c09cc
PS14, Line 40: /*! flags to guard closing/freeing of iofd */
Not saying this needs to be done, but IMO a bitmask would be a better fit here
because adding new flags would not require adding new fields and thus breaking
ABI.
File src/core/osmo_io_poll.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/96c515c9_320de74b
PS14, Line 44: inline
Do we really need to force inlining here? Given that this function is used only
once, compilers will likely inline it anyway...
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/3a82b046_20ac5f35
PS14, Line 175: struct iofd_backend_ops iofd_poll_ops = {
const?
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0
Gerrit-Change-Number: 30934
Gerrit-PatchSet: 14
Gerrit-Owner: daniel
Gerrit-Reviewer: Hoernchen
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: neels
Gerrit-Reviewer: osmith
Gerrit-Reviewer: pespin
Gerrit-CC: laforge
Gerrit-Attention: osmith
Gerrit-Attention: Hoernchen
Gerrit-Attention: neels
Gerrit-Attention: laforge
Gerrit-Attention: daniel
Gerrit-Comment-Date: Sun, 07 May 2023 22:06:36 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge,
fixeria, daniel.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
..
Patch Set 14: Code-Review+1
(1 comment)
File src/core/osmo_io_poll.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/eecf4e09_5fc6b455
PS13, Line 93: if (rc < 0) {
> I though I had msg == NULL for a reason, but this is the sending side... […]
ACK
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0
Gerrit-Change-Number: 30934
Gerrit-PatchSet: 14
Gerrit-Owner: daniel
Gerrit-Reviewer: Hoernchen
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: neels
Gerrit-Reviewer: osmith
Gerrit-Reviewer: pespin
Gerrit-CC: laforge
Gerrit-Attention: osmith
Gerrit-Attention: Hoernchen
Gerrit-Attention: neels
Gerrit-Attention: laforge
Gerrit-Attention: fixeria
Gerrit-Attention: daniel
Gerrit-Comment-Date: Fri, 05 May 2023 14:44:36 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: pespin
Comment-In-Reply-To: daniel
Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. Hello osmith, Jenkins Builder, Hoernchen, neels, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/30934 to look at the new patch set (#14). Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/osmocom/core/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/core/Makefile.am M src/core/libosmocore.map M src/core/logging.c A src/core/osmo_io.c A src/core/osmo_io_internal.h A src/core/osmo_io_poll.c M tests/logging/logging_vty_test.vty 11 files changed, 1,067 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/14 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 14 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-MessageType: newpatchset
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge,
pespin, fixeria.
daniel has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
..
Patch Set 13:
(4 comments)
Patchset:
PS13:
Sorry for the back and forth changes, I tried to merge your changes and still
keep the lost changes from
https://gerrit.osmocom.org/c/libosmocore/+/30934/8..9 and I had some trouble
seeing what was intentional and what was merely code from a previous version.
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/fcdac5d7_ddc8be5c
PS8, Line 206: iofd->tx_queue.current_length--;
> Could be msgb_dequeue_count(), but can stay this way too.
See discusion in iofd_txqueue_enqueue()
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/e883e478_f7a2eaf4
PS13, Line 82: fprintf(stderr, "Invalid osmo_io backend requested:
\"%s\nCheck the environment variable %s\n", backend, OSMO_IO_BACKEND_ENV);
> You are missing a \" before first \n.
Thanks, fixed
File src/core/osmo_io_poll.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/2d358e27_50d3d0df
PS13, Line 93: if (rc < 0) {
> I think I changed this and you are changing it again. […]
I though I had msg == NULL for a reason, but this is the sending side...
Thinking about it some more we definitely want to report a send rc of 0 to the
user since that can indicate a broken connection. If we re-enqueue in that case
we'll busy loop trying to send and send returning 0.
Keeping the msg does make sense so I'll change it to reenqueue only if (rc > 0
&& rc < msgb_length(msg))
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0
Gerrit-Change-Number: 30934
Gerrit-PatchSet: 13
Gerrit-Owner: daniel
Gerrit-Reviewer: Hoernchen
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: neels
Gerrit-Reviewer: osmith
Gerrit-Reviewer: pespin
Gerrit-CC: laforge
Gerrit-Attention: osmith
Gerrit-Attention: Hoernchen
Gerrit-Attention: neels
Gerrit-Attention: laforge
Gerrit-Attention: pespin
Gerrit-Attention: fixeria
Gerrit-Comment-Date: Fri, 05 May 2023 14:00:44 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin
Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge,
fixeria, daniel.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
..
Patch Set 13:
(2 comments)
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/918a8e10_308b954e
PS13, Line 82: fprintf(stderr, "Invalid osmo_io backend requested:
\"%s\nCheck the environment variable %s\n", backend, OSMO_IO_BACKEND_ENV);
You are missing a \" before first \n.
File src/core/osmo_io_poll.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/8fb39dcd_9f3bee08
PS13, Line 93: if (rc < 0) {
I think I changed this and you are changing it again.
I really don't understand why are you doing this here. Is there some specific
reason?)
I also don't remember why exactly I changed it, but I recall there was a reason
why I did so :/ I think this way you can, in the callback, actually dump the
content of the message which is being dropped, which can be handy.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0
Gerrit-Change-Number: 30934
Gerrit-PatchSet: 13
Gerrit-Owner: daniel
Gerrit-Reviewer: Hoernchen
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: neels
Gerrit-Reviewer: osmith
Gerrit-Reviewer: pespin
Gerrit-CC: laforge
Gerrit-Attention: osmith
Gerrit-Attention: Hoernchen
Gerrit-Attention: neels
Gerrit-Attention: laforge
Gerrit-Attention: fixeria
Gerrit-Attention: daniel
Gerrit-Comment-Date: Fri, 05 May 2023 10:33:19 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 13: (1 comment) File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/e639b742_c81c078b PS12, Line 71: LIBOSMO_IO_BACKEND > I guess this environment variable would need to be documented somewhere. […] Done -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 13 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Fri, 05 May 2023 07:25:17 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. Hello osmith, Jenkins Builder, Hoernchen, neels, laforge, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/30934 to look at the new patch set (#13). Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/osmocom/core/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/core/Makefile.am M src/core/libosmocore.map M src/core/logging.c A src/core/osmo_io.c A src/core/osmo_io_internal.h A src/core/osmo_io_poll.c M tests/logging/logging_vty_test.vty 11 files changed, 1,069 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/13 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 13 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-MessageType: newpatchset
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, pespin, fixeria, daniel. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 12: Code-Review+1 (1 comment) File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/4a133366_4b240bd6 PS12, Line 71: LIBOSMO_IO_BACKEND I guess this environment variable would need to be documented somewhere. At least in doxygen for now (developer documentation), but for sure once we add support for other backends also in some common chapter of the osmo-gsm-manuals.git (user documentation) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 12 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Sun, 30 Apr 2023 00:21:05 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, pespin, fixeria, daniel. pespin has uploaded a new patch set (#12) to the change originally created by daniel. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/osmocom/core/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/core/Makefile.am M src/core/libosmocore.map M src/core/logging.c A src/core/osmo_io.c A src/core/osmo_io_internal.h A src/core/osmo_io_poll.c M tests/logging/logging_vty_test.vty 11 files changed, 1,061 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/12 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 12 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, pespin, fixeria, daniel. pespin has uploaded a new patch set (#11) to the change originally created by daniel. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/osmocom/core/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/core/Makefile.am M src/core/libosmocore.map M src/core/logging.c A src/core/osmo_io.c A src/core/osmo_io_internal.h A src/core/osmo_io_poll.c M tests/logging/logging_vty_test.vty 11 files changed, 1,063 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/11 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 11 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, pespin, fixeria, daniel. pespin has uploaded a new patch set (#10) to the change originally created by daniel. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/osmocom/core/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/core/Makefile.am M src/core/libosmocore.map M src/core/logging.c A src/core/osmo_io.c A src/core/osmo_io_internal.h A src/core/osmo_io_poll.c M tests/logging/logging_vty_test.vty 11 files changed, 1,046 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/10 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 10 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, fixeria, daniel. Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 9: (2 comments) File include/osmocom/core/osmo_io.h: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-6584): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/4cf94a61_b6282fda PS9, Line 73: const char *osmo_iofd_get_name(const struct osmo_io_fd *iofd); adding a line without newline at end of file File src/core/osmo_io.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-6584): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/adac0cad_2468acf8 PS9, Line 460: int osmo_iofd_unregister(struct osmo_io_fd *iofd) please, no spaces at the start of a line -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 9 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Thu, 27 Apr 2023 13:14:33 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, fixeria, daniel. pespin has uploaded a new patch set (#9) to the change originally created by daniel. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/osmocom/core/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/core/Makefile.am M src/core/libosmocore.map M src/core/logging.c A src/core/osmo_io.c A src/core/osmo_io_internal.h A src/core/osmo_io_poll.c M tests/logging/logging_vty_test.vty 11 files changed, 1,043 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/9 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 9 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, fixeria, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 8: Code-Review+2 (1 comment) File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/09aac23c_b3f6595f PS8, Line 169: llist_add_tail(>list, >tx_queue.msg_queue); > I had that at first, but msgb_enqueue_count() takes a struct msgb * and > derefereces the ->list point […] Ack -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 8 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 04 Apr 2023 11:48:25 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, fixeria, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 8: (1 comment) File src/core/osmo_io_poll.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/845e0276_d33c2d06 PS8, Line 85: if (iofd->closed) > closed, in_callback, and to_free […] Nice, thanks. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 8 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 04 Apr 2023 11:48:13 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, pespin, fixeria. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 8: (2 comments) File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/f516f35b_750d09f6 PS8, Line 169: llist_add_tail(>list, >tx_queue.msg_queue); > Could be msgb_enqueue_count(), but it can stay this way too. I had that at first, but msgb_enqueue_count() takes a struct msgb * and derefereces the ->list pointer in msgb_enqueue(). So this would require casting a msghdr * to a msgb * and relying on the fact that the list member is in the same position in both structs which I quite dislike. File src/core/osmo_io_poll.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/b043b551_197e07ec PS8, Line 85: if (iofd->closed) > is this the new -EBADF to handle case where recvmsg_cb closes th iofd? What > if iofd is freed by recv […] closed, in_callback, and to_free Closed is the early-return that prevents the write from being considered when the fd is already closed. in_callback is set to true while inside this function. If osmo_iofd_free() is called while in_callback is true then it will only close the fd and defer actually freeing the data and just set to_free to true. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 8 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Tue, 04 Apr 2023 11:43:25 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, fixeria, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 8: Code-Review+1 (3 comments) File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/f3774e3f_863afd46 PS8, Line 169: llist_add_tail(>list, >tx_queue.msg_queue); Could be msgb_enqueue_count(), but it can stay this way too. https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/97d19e82_37b0b785 PS8, Line 206: iofd->tx_queue.current_length--; Could be msgb_dequeue_count(), but can stay this way too. File src/core/osmo_io_poll.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/25c87e73_39e41f78 PS8, Line 85: if (iofd->closed) is this the new -EBADF to handle case where recvmsg_cb closes th iofd? What if iofd is freed by recvmsg_cb(), can that happen? you would be accessing freed memory here. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 8 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Mon, 03 Apr 2023 08:57:20 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, pespin, fixeria, daniel. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 8: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 8 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Sat, 01 Apr 2023 14:04:32 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. Hello osmith, Jenkins Builder, Hoernchen, neels, laforge, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/30934 to look at the new patch set (#8). Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/osmocom/core/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/core/Makefile.am M src/core/libosmocore.map M src/core/logging.c A src/core/osmo_io.c A src/core/osmo_io_internal.h A src/core/osmo_io_poll.c M tests/logging/logging_vty_test.vty 11 files changed, 1,049 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/8 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 8 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-MessageType: newpatchset
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 8: (3 comments) File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/3806d858_1cc24bd2 PS7, Line 5: 2022 > probably also 2023 now. Not super important. Ack https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/975d52b3_a82d08b7 PS7, Line 349: iofd(%s) > I see multiple log lines manually encoding this prefix. […] Yeah, I also thought about that. https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/c70fa3fe_9f428703 PS7, Line 432: using backend %s > as the backend is global, I'm not sure we need to log this every time? ACK, I guess that's left over from an older version where it wasn't yet global -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 8 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Sat, 01 Apr 2023 12:27:32 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, pespin, fixeria, daniel. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 7: Code-Review+1 (4 comments) Patchset: PS7: looks good, just some cosmetic stuff... File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/1c82c63f_91fa25d1 PS7, Line 5: 2022 probably also 2023 now. Not super important. https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/e1854e91_c7880c79 PS7, Line 349: iofd(%s) I see multiple log lines manually encoding this prefix. Guess it's time for a LOGPIO(iofd, level, fmt , ...) macro that encapsulates that? https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/bb8db996_872484f7 PS7, Line 432: using backend %s as the backend is global, I'm not sure we need to log this every time? -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 7 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Fri, 31 Mar 2023 21:51:40 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria, daniel. Hello osmith, Jenkins Builder, Hoernchen, neels, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/30934 to look at the new patch set (#7). Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/osmocom/core/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/core/Makefile.am M src/core/libosmocore.map M src/core/logging.c A src/core/osmo_io.c A src/core/osmo_io_internal.h A src/core/osmo_io_poll.c M tests/logging/logging_vty_test.vty 11 files changed, 1,043 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/7 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 7 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 6: (2 comments) File include/osmocom/core/osmo_io.h: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-5659): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/f96e35e9_08303690 PS6, Line 73: const char *osmo_iofd_get_name(const struct osmo_io_fd *iofd); adding a line without newline at end of file File src/core/osmo_io.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-5659): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/0dd7b7a0_4e84460c PS6, Line 460: int osmo_iofd_unregister(struct osmo_io_fd *iofd) please, no spaces at the start of a line -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 6 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Fri, 31 Mar 2023 15:50:09 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. Hello osmith, Jenkins Builder, Hoernchen, neels, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/30934 to look at the new patch set (#6). Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/osmocom/core/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/core/Makefile.am M src/core/libosmocore.map M src/core/logging.c A src/core/osmo_io.c A src/core/osmo_io_internal.h A src/core/osmo_io_poll.c M tests/logging/logging_vty_test.vty 11 files changed, 1,043 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/6 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 6 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-MessageType: newpatchset
[XL] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 6: (6 comments) Patchset: PS3: > Are you talking about a map file for all of libosmocore or just a libosmoio > map file? We have a libosmocore.map file now File include/osmocom/core/osmo_io.h: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/a894a935_681ff3b8 PS5, Line 57: int osmo_iofd_sendto_msgb(struct osmo_io_fd *iofd, struct msgb *msg, int flags, > maybe name it sendto_flags to clarify what are the flags for, but not that > important. Done File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/74a82377_353ddfb5 PS5, Line 210: * If there are bytes left over *pending_out will point to a msgb with the remaining data. > " If there are bytes left over, *pending_out will" (see extra comma) Done https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/32c493f5_ebe9662c PS5, Line 426: if (!osmo_iofd_ops.close || (osmo_iofd_ops.close && osmo_iofd_ops.close(iofd))) > There seems to be a lot of expected stuff happening here with regards to > struct configuration and re […] I reworked this and now have separate free and close functions which also works with (un-)registering an iofd so users don't need to go through free/alloc every time a connection is reconnected (e.g. by libosmo-netif). File src/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/803e3585_bf78bf05 PS3, Line 157: */ > not critical: I usually put the end-of-comment on the previous line to save > extraneous lines in the […] I actually prefer it like this if you don't mind. In select.c both styles are used throughout the file. File src/osmo_io_poll.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/8baf0854_c27cc953 PS3, Line 84: / > Ack This seems to map to a POLLPRI event in poll() man poll says this: ``` POLLPRI There is some exceptional condition on the file descriptor. Possibilities include: • There is out-of-band data on a TCP socket (see tcp(7)). • A pseudoterminal master in packet mode has seen a state change on the slave (see ioctl_tty(2)). • A cgroup.events file has been modified (see cgroups(7)). ``` I don't see this being relevant for us in any way so I don't think we need to support FD_EXCEPT here. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 6 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Fri, 31 Mar 2023 15:49:31 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[L] Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, fixeria, daniel. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 5: (1 comment) Patchset: PS5: any update here? -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 5 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Mon, 27 Feb 2023 10:05:03 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, fixeria, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 5: (6 comments) File include/osmocom/core/osmo_io.h: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/d9628e2d_f17a91bf PS5, Line 57: int osmo_iofd_sendto_msgb(struct osmo_io_fd *iofd, struct msgb *msg, int flags, maybe name it sendto_flags to clarify what are the flags for, but not that important. File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/ca524f17_c67c82bf PS5, Line 210: * If there are bytes left over *pending_out will point to a msgb with the remaining data. " If there are bytes left over, *pending_out will" (see extra comma) https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/7c6dffd2_0e001c04 PS5, Line 426: if (!osmo_iofd_ops.close || (osmo_iofd_ops.close && osmo_iofd_ops.close(iofd))) There seems to be a lot of expected stuff happening here with regards to struct configuration and return codes from function pointers. I think this deserves a comment here explaining the expected lifecycle. File src/osmo_io_poll.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/1fa54ade_1463fb7d PS3, Line 71: rc = write(ofd->fd, msgb_data(msg), msgb_length(msg)); > the normal "read/write" case might be worth a dedicated function as we don't > need a serialized msghe […] read/write is basically recv/send with flags=0, so there's no need to have read/write imho. https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/f4a749b6_e40208a1 PS3, Line 84: / > maybe still look into this before merging it? Ack https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/c7a750bf_aea0ba83 PS3, Line 143: / > same as above Ack -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 5 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Fri, 03 Feb 2023 15:29:55 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 5: (4 comments) File src/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/0581e24a_90af351f PS3, Line 50: struct iofd_backend_ops g_iofd_ops; > if a map file is introduced, this symbol will still be exported in order to > be used by tests AFAIU. […] Done https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/9356aa10_89e91727 PS3, Line 111: talloc_free(msghdr); > Better describe it in the code then. Done https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/b331d7b9_90749472 PS3, Line 225: msg_pending = iofd_msgb_alloc(iofd); > Ack. Better describe this in the code. I tried https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/b986ca7f_22960d5c PS3, Line 228: iofd->pending = msg_pending; > you may want to add OSMO_ASSERT(iofd->pending == NULL) perhaps. ACK -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 5 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Comment-Date: Fri, 03 Feb 2023 15:17:00 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. Hello osmith, Jenkins Builder, Hoernchen, neels, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/30934 to look at the new patch set (#5). Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/osmocom/core/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/core/Makefile.am M src/core/libosmocore.map M src/core/logging.c A src/core/osmo_io.c A src/core/osmo_io_internal.h A src/core/osmo_io_poll.c M tests/logging/logging_vty_test.vty 11 files changed, 947 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/5 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 5 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-MessageType: newpatchset
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, fixeria, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 4: (4 comments) File src/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/4f24affb_91ab9c56 PS3, Line 50: struct iofd_backend_ops g_iofd_ops; > This should be resolved if we end up introducing a . […] if a map file is introduced, this symbol will still be exported in order to be used by tests AFAIU. And what we want to avoid is exporting symbols not prefixed with osmo_ which would collide with other libs. https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/64a7ffd2_9590d754 PS3, Line 111: talloc_free(msghdr); > For io_uring the msghdr will also be used for read requests and then the msg > might continue to live […] Better describe it in the code then. https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/9cc09570_70a0a0c3 PS3, Line 225: msg_pending = iofd_msgb_alloc(iofd); > This path is chosen if we receive more than one message. […] Ack. Better describe this in the code. https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/3332e966_daeee86f PS3, Line 228: iofd->pending = msg_pending; you may want to add OSMO_ASSERT(iofd->pending == NULL) perhaps. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 4 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Wed, 25 Jan 2023 16:17:07 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge,
pespin, fixeria.
daniel has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
..
Patch Set 4:
(11 comments)
This change is ready for review.
Patchset:
PS3:
> In general I think we need to think about naming / namespace pollution. […]
Are you talking about a map file for all of libosmocore or just a libosmoio map
file?
File src/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/8c9b0167_87a37287
PS3, Line 50: struct iofd_backend_ops g_iofd_ops;
> then better name it with an osmo_ prefix.
This should be resolved if we end up introducing a .map file, right?
File src/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/cdabab1f_b2c19701
PS4, Line 228: msg_pending = iofd_msgb_alloc(iofd);
This function now sets pending_out to the remaining msgb and
iofd_handle_segmented_read is responsible for setting this. This also saves us
one msgb_pending call in the handle_more case below.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/0a9f73e5_028dca84
PS4, Line 299: * \returns 0 in case of success; a negative value in case of
error
Even though almost no code uses it I guess you're right.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/1115e0e0_632590fc
PS4, Line 356:
: struct os
Done
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/b7a0d6ae_03597afb
PS4, Line 367: iofd->io_ops = *ioops;
I agree, the name should be mandatory.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/8471d7a4_6109cd5a
PS4, Line 414: * \param[in] headroom the headroom of the msgb when receiving
data
Ack
File src/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/8e8e3792_75bbe29d
PS3, Line 17: in
> I'm wondering if it had any advantage to have only a single function with an
> argument stating the "o […]
I'll keep it as it is
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/337a22b0_b3a229c7
PS3, Line 72: struct {
: bool read_enabled;
: bool read_pending;
: bool write_pending;
: bool write_enabled;
: /* TODO: index into array of registered fd's? */
: } uring;
> Ack
Done
File src/osmo_io_poll.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/66e76cf0_7c9f532a
PS4, Line 104: struct osmo_sockaddr saddr;
Done
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/f3a3dc66_965873ff
PS4, Line 128: if (rc < 0) {
Does the kernel use addrlen for more than just checking if it's long enough for
the variant? In any case I guess we only want to copy to the kernel what we
really need.
I think should be a helper function in libosmocore like osmo_sockaddr_size()
that returns the size depending on the family that way we can reuse it
elsewhere.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0
Gerrit-Change-Number: 30934
Gerrit-PatchSet: 4
Gerrit-Owner: daniel
Gerrit-Reviewer: Hoernchen
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: neels
Gerrit-Reviewer: osmith
Gerrit-Reviewer: pespin
Gerrit-CC: laforge
Gerrit-Attention: osmith
Gerrit-Attention: Hoernchen
Gerrit-Attention: neels
Gerrit-Attention: laforge
Gerrit-Attention: pespin
Gerrit-Attention: fixeria
Gerrit-Comment-Date: Wed, 25 Jan 2023 16:03:50 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge
Comment-In-Reply-To: pespin
Gerrit-MessageType: comment
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, pespin, fixeria, daniel. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 4: (6 comments) File src/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/6d24eba8_18c9e1a3 PS3, Line 356: unsigned int size, unsigned int headroom, :int fd > I guess we could even have defaults for msgb size/headroom and have > osmo_iofd_set_alloc_info() avail […] yes, we could go for that. https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/c2d9f460_4318f3f3 PS3, Line 414: talloc_free(iofd); > The close callback return value indicates whether the free() should be > deferred or not, it does not […] I think Pau's comment was related to the ops.close == NULL case, where you don't free the iofd at all. So I guess talloc_free should be called if (close == NULL || (close && close(iofd)) File src/osmo_io_internal.h: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/ea5723dc_ec22fb88 PS3, Line 87: // TODO: SCTP_* > I wanted to keep this patch as-is and add sctp support in a separate one. […] Ack File src/osmo_io_poll.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/3c922bdc_5f9c8cd9 PS3, Line 40: what > In socket.c it's called what in the callback, when in the ofd and flags in > the internal code. […] Ack https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/06a0b455_8e16eca4 PS3, Line 71: rc = write(ofd->fd, msgb_data(msg), msgb_length(msg)); > There is no iofd->flags, did you mean msghdr->flags? […] the normal "read/write" case might be worth a dedicated function as we don't need a serialized msgheader heap allocation for those, right? https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/0f9dbbd6_88b2d852 PS3, Line 104: socklen_t addrlen = sizeof(struct sockaddr); > I'd use sizeof(saddr) since that most accurately describes the size recvfrom > is allowed to write to. Ack -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 4 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Fri, 20 Jan 2023 16:35:51 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge,
pespin, fixeria.
Jenkins Builder has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
..
Patch Set 4:
(1 comment)
File src/osmo_io.c:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2884):
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/37b415fc_6cadbe92
PS4, Line 146: if (!msg) {
braces {} are not necessary for single statement blocks
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0
Gerrit-Change-Number: 30934
Gerrit-PatchSet: 4
Gerrit-Owner: daniel
Gerrit-Reviewer: Hoernchen
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: neels
Gerrit-Reviewer: osmith
Gerrit-Reviewer: pespin
Gerrit-CC: laforge
Gerrit-Attention: osmith
Gerrit-Attention: Hoernchen
Gerrit-Attention: neels
Gerrit-Attention: laforge
Gerrit-Attention: pespin
Gerrit-Attention: fixeria
Gerrit-Comment-Date: Fri, 20 Jan 2023 13:58:49 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. Hello osmith, Jenkins Builder, Hoernchen, neels, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/30934 to look at the new patch set (#4). Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/Makefile.am M src/logging.c A src/osmo_io.c A src/osmo_io_internal.h A src/osmo_io_poll.c M tests/logging/logging_vty_test.vty 10 files changed, 882 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/4 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 4 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-MessageType: newpatchset
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge,
pespin, fixeria.
daniel has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
..
Patch Set 3:
(33 comments)
This change is ready for review.
File include/osmocom/core/osmo_io.h:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/a0e3d24c_0c13d2a8
PS3, Line 49: void osmo_iofd_read_enable(struct osmo_io_fd *iofd);
> duplicated declaration? one probably should be "disable"
Ack
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/77b6da4e_53027caf
PS3, Line 55: unsigned int osmo_iofd_get_priv(struct osmo_io_fd *iofd);
> const iofd
Done
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/2e098e12_c9327515
PS3, Line 56: int osmo_iofd_get_fd(struct osmo_io_fd *iofd);
> const iofd
Done
File src/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/d1fe65a8_5f61b0c1
PS3, Line 44: extern struct iofd_backend_ops iofd_poll_ops;
> we have an internal header, maybe move this there?
Ack
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/9faaa528_cda404be
PS3, Line 47: static enum osmo_io_backend g_backend;
> g_io_backend?
Ack
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/38b482c4_bdaa1ff1
PS3, Line 85: * \returns the newly allocated msghdr or NULL in case of error
*/
> I suggest printing an error message and exit(1) here, since this may be the
> result of an unexpected […]
Ack
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/d5b4285c_c62e8f3a
PS3, Line 121: iofd->msgb_alloc.size + headroom,
headroom, iofd->name);
> the ownserhip of msghdr->msg is not really clear here. […]
For io_uring the msghdr will also be used for read requests and then the msg
might continue to live in the read_cb after the msghdr has already been freed.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/93a037c3_1ed3de9e
PS3, Line 181:
> always
Done
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/7f3b57f6_8f6e35ca
PS3, Line 203: e
> Ack
It can be static
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/880fcbfd_46071b9b
PS3, Line 224: superfluous
> superflous IMHO means that it's not needed. […]
Ack
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/9c49029c_dd87789b
PS3, Line 225: msg_pending = iofd_msgb_alloc(iofd);
> I wonder why do we need to allocate + copy to a new msgb here.
This path is chosen if we receive more than one message. The original msgb is
passed on to the recv callback (where it will be freed) and the pending data is
stored in a new msgb.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/0e19c65a_be3c04f1
PS3, Line 230: /* Trim the original msgb to size */
> iofd->pending may already be set here? in that case may leak the previous
> iofd->pending?
pending should always be NULL here because either iofd_msgb_pending() of
iofd_msgb_pending_or_alloc() has been called before.
I'll see if I can restructure the code to make it clearer, like passing msgbs
in and out here and setting iofd->pending in the functions that call this one.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/56f23a5c_0255cef3
PS3, Line 271: {
> coding style
Done
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/05bd9ad5_b68c6af8
PS3, Line 273:
> coding style
Done
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/2cc0a478_cf885f44
PS3, Line 306: {
> coding style
Done
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/fcfdae14_dfe16f0e
PS3, Line 308:
> coding style
Done
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/36a34157_d300dbb4
PS3, Line 360: t osmo_io_fd);
> given that there's only one known legacy user of the priv_nr (libosmo-abis),
> maybe we remove this fr […]
Ack
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/db9e9630_ce96f7e9
PS3, Line 360: smo_io_fd);
: if (!iofd
> we have three integer argiuments after each other, and lots of arguments,
> which makes it easy to hav […]
I guess we could even have defaults for msgb size/headroom and have
osmo_iofd_set_alloc_info() available to set it manually?
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/87d6056c_515bbebe
PS3, Line 391:
> Ack
Done
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/b19c7579_2ddc1353
PS3, Line 408: i
> c++ style comment
Done
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/804141de_c1216e69
PS3, Line 410:
> no need for this if, must probably (free function)
Ack
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/d83e1f13_af9084b0
PS3, Line 416:
> you still want to free the memory if close somehow fails, otherwise you'll
> leak it?
The close callback return value indicates whether the free() should be deferred
or not, it does not indicate failure. E.g. if a user calls
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: Hoernchen, neels, pespin, fixeria, daniel. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 3: (3 comments) File src/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/a198e86e_38f0e34e PS3, Line 75: OSMO_ASSERT(0); I suggest printing an error message and exit(1) here, since this may be the result of an unexpected value in the env var https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/47b73328_8f30fc2f PS3, Line 171: allways always File src/osmo_io_poll.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/a2c4b60e_98b2fb3c PS3, Line 40: what might be just me, but I find "flags" much more descriptive than "what" -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 3 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Wed, 18 Jan 2023 17:39:50 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, pespin, fixeria, daniel. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 3: (2 comments) File src/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/eb441ae7_22c37abb PS3, Line 367: iofd->name = talloc_strdup(iofd, name); > This can be split into a separate func osmo_iofd_set_name(), simplifying the > current osmo_iofd_setup […] I actually think it is rather important to encourage all useres to provide a name. If it's a separate API call, it means many users will froget about it. Having meaningful names allows the osmo_io provider to do meaningful logging (with context) in error conditions, etc. File src/osmo_io_internal.h: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/f801206b_17d8d58c PS3, Line 87: // TODO: SCTP_* > I'd say this needs to be added before merging the patch? or not needed? it is only needed later one when we start to support sctp_sendmsg/sctp_recvmsg, which are just API wrappers around normal sendmsg/recvmsg. But in order to provide the same level of API abstraction, we thought it would be good to offer a sctp_recvmsg callback for the applications. The TODO is therefore intentionally for a later follow-up patch. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 3 Gerrit-Owner: daniel Gerrit-Reviewer: Hoernchen Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: osmith Gerrit-Attention: Hoernchen Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 17 Jan 2023 17:33:58 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: osmith, Hoernchen, neels, laforge,
fixeria, daniel.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
..
Patch Set 3:
(26 comments)
File include/osmocom/core/osmo_io.h:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/8d04803d_26ccd6a7
PS3, Line 49: void osmo_iofd_read_enable(struct osmo_io_fd *iofd);
duplicated declaration? one probably should be "disable"
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/a893cd75_7d281ec1
PS3, Line 55: unsigned int osmo_iofd_get_priv(struct osmo_io_fd *iofd);
const iofd
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/9779c049_4723f660
PS3, Line 56: int osmo_iofd_get_fd(struct osmo_io_fd *iofd);
const iofd
File src/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/4eb23142_d4708c38
PS3, Line 44: extern struct iofd_backend_ops iofd_poll_ops;
we have an internal header, maybe move this there?
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/b45183be_4c0b39cd
PS3, Line 47: static enum osmo_io_backend g_backend;
g_io_backend?
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/325f6a28_d6e94902
PS3, Line 50: struct iofd_backend_ops g_iofd_ops;
then better name it with an osmo_ prefix.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/5f694a6e_7c527ff6
PS3, Line 111: talloc_free(msghdr);
the ownserhip of msghdr->msg is not really clear here. If a msg ptr is passed
in iofd_msghdr_alloc, is it expected to pass ownsership of msg to msghdr? If
so, either:
- call talloc_steal in iofd_msghdr_alloc
- explicitly talloc_free msghdr->msg here.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/c5f10141_edbf357f
PS3, Line 203: e
> no doxygen docs, but function is exported
Ack
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/3989be04_0f4f8bb4
PS3, Line 225: msg_pending = iofd_msgb_alloc(iofd);
I wonder why do we need to allocate + copy to a new msgb here.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/d9474b17_ea77ad96
PS3, Line 228: iofd->pending = msg_pending;
iofd->pending may already be set here? in that case may leak the previous
iofd->pending?
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/9cf86edf_f55914c5
PS3, Line 299: int osmo_iofd_sendto_msgb(struct osmo_io_fd *iofd, struct msgb
*msg, const struct osmo_sockaddr *dest)
we may be missing "int flags" here?
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/8d0ed2a8_3583faa5
PS3, Line 367: iofd->name = talloc_strdup(iofd, name);
This can be split into a separate func osmo_iofd_set_name(), simplifying the
current osmo_iofd_setup param list. Some apps may not even care nor want to
spend cycles allocating memory and setting names.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/9dd628e7_0023c4fa
PS3, Line 389: g_backend == OSMO_IO_BACKEND_POLL ? "poll" : "uring"
> there might be more in the future, it might be worth having a separate
> value_string for that rather […]
Ack
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/f517d93a_a320919a
PS3, Line 408: if (iofd->pending)
no need for this if, must probably (free function)
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/5e7e43dd_5e60bd63
PS3, Line 414: talloc_free(iofd);
you still want to free the memory if close somehow fails, otherwise you'll leak
it?
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/7decfc04_2f211349
PS3, Line 430: osmo_iofd_get_priv
> please rename to osmo_iofd_get_priv_nr(). […]
Ack
File src/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/f501f63f_e7dff15c
PS3, Line 72: struct {
: bool read_enabled;
: bool read_pending;
: bool write_pending;
: bool write_enabled;
: /* TODO: index into array of registered fd's? */
: } uring;
> should this maybe introduced only in the actual io_oring patch?
Ack
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/7ff40994_73fcb0ba
PS3, Line 87: // TODO: SCTP_*
I'd say this needs to be added before merging the patch? or not needed?
File src/osmo_io_poll.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/c4b0bb76_56537732
PS3, Line 63: if (iofd->closing) {
Remove {}
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/4f2e3acc_e61c1809
PS3, Line 71: rc = write(ofd->fd, msgb_data(msg),
msgb_length(msg));
this should be send() in order to pass iofd->flags to it.
Actually, you should have aseparate read_flags and write_flags fields most
probably.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/ef75ef5d_767d6bba
PS3, Line 72: if (rc < msgb_length(msg)) {
if write
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: daniel.
laforge has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
..
Patch Set 3:
(10 comments)
File src/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/9bae02a3_ce85f35f
PS3, Line 389: g_backend == OSMO_IO_BACKEND_POLL ? "poll" : "uring"
there might be more in the future, it might be worth having a separate
value_string for that rather than explicitly hard-coding the names on the
caller side here. Also, it doesn't hurt to log the actaul fd number while
we're logging something anyway.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/a750a7b0_dc9a555c
PS3, Line 406: /
c++ style comment
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/c23a484a_4615b9f0
PS3, Line 430: osmo_iofd_get_priv
please rename to osmo_iofd_get_priv_nr(). "priv" is also sometimes used as
alias to "data" in kernel and osmocom.
If we remove the argument from the setup function, then we also need a
set_priv_nr().
File src/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/221f9c34_279a479c
PS3, Line 17: in
I'm wondering if it had any advantage to have only a single function with an
argument stating the "operation"? The prototype is almost identical (just
int/void return).
I'm not sure it would have any advantage. Probably none.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/4851bdb7_ac9ffd4a
PS3, Line 72: struct {
: bool read_enabled;
: bool read_pending;
: bool write_pending;
: bool write_enabled;
: /* TODO: index into array of registered fd's? */
: } uring;
should this maybe introduced only in the actual io_oring patch?
File src/osmo_io_poll.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/620c95b7_ad90f478
PS3, Line 84: /
maybe still look into this before merging it?
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/26ed1231_29e5d8c9
PS3, Line 114:
extraneous empty line
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/85d8716d_adce74ab
PS3, Line 117: {
coding style
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/d4b0e7dc_8388e60e
PS3, Line 143: /
same as above
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/13c1983d_94d3470c
PS3, Line 146: i
should be static? same as all of the iofd_poll_ functions which are exposed as
callback via iofd_poll_ops and don't need to be exported as global functions
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0
Gerrit-Change-Number: 30934
Gerrit-PatchSet: 3
Gerrit-Owner: daniel
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge
Gerrit-Attention: daniel
Gerrit-Comment-Date: Tue, 17 Jan 2023 11:24:18 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: daniel.
laforge has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
..
Patch Set 3:
(8 comments)
Patchset:
PS3:
In general I think we need to think about naming / namespace pollution. I
guess the iofd_* functions are non-static as they will end up geting used form
the io_uring backend in another file?
Unfortunately, for historical reasons, libosmocore doesn't have a .map file so
all the non-static symbols become publicly accessible to applications. And the
general rule for (new) library-exported symbols is to use the osmo_ prefix.
So I think the cleanest approach would actually be to introduce a .map file and
then only export the osmo_iofd_* functions that way, while keeping the iofd_*
non-static but not exported?
File src/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/82a7173e_92c7c48e
PS3, Line 157: */
not critical: I usually put the end-of-comment on the previous line to save
extraneous lines in the file (more context on the screen) and to optically have
the doxygen-docs directly above the function definition, without a (mostly)
empty line.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/6f6a2141_d40e53cb
PS3, Line 203: e
no doxygen docs, but function is exported
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/d3f6a27e_69281393
PS3, Line 224: superfluous
superflous IMHO means that it's not needed. Maybe "extraneous"? "leftover"?
"pending"?
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/d3baccb7_7397c510
PS3, Line 271: {
coding style
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/7903a154_ac2309c6
PS3, Line 306: {
coding style
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/430479e2_d7e320ad
PS3, Line 356: unsigned int size, unsigned int headroom,
:int fd
we have three integer argiuments after each other, and lots of arguments, which
makes it easy to have bugs on the caller side. Maybe we should re-order them
somehow (or reduce, if possible)? Logically I would say that "ctx, fd" should
be the two first arguments, as we usually have the object the call relates
to/on as the first argument, prefixed by th ctx.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/d5e0682b_122c0ff0
PS3, Line 358: unsigned int priv_nr
given that there's only one known legacy user of the priv_nr (libosmo-abis),
maybe we remove this from the function prototype fore new API. Those few
callers that actually care can still set it in the io_fd that is returned from
this function?
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0
Gerrit-Change-Number: 30934
Gerrit-PatchSet: 3
Gerrit-Owner: daniel
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge
Gerrit-Attention: daniel
Gerrit-Comment-Date: Tue, 17 Jan 2023 11:15:28 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Change in libosmocore[master]: Add osmo_io with initial poll backend
Attention is currently required from: daniel. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/30934 to look at the new patch set (#3). Change subject: Add osmo_io with initial poll backend .. Add osmo_io with initial poll backend * make backend configurable for later * segmentation callback for chunked streams * logging target for osmo_io * support partial writes Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Related: SYS#5094, OS#5751 --- M TODO-RELEASE M include/Makefile.am M include/osmocom/core/logging.h A include/osmocom/core/osmo_io.h M src/Makefile.am M src/logging.c A src/osmo_io.c A src/osmo_io_internal.h A src/osmo_io_poll.c M tests/logging/logging_vty_test.vty 10 files changed, 854 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/3 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 3 Gerrit-Owner: daniel Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
Change in libosmocore[master]: Add osmo_io with initial poll backend
daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Set Ready For Review -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 2 Gerrit-Owner: daniel Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge Gerrit-Comment-Date: Wed, 11 Jan 2023 16:13:32 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in libosmocore[master]: Add osmo_io with initial poll backend
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend .. Patch Set 1: (24 comments) File include/osmocom/core/osmo_io.h: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/a06005a2_b630304e PS1, Line 28: void (*read_cb)(struct osmo_io_fd *, int res, struct msgb *); function definition argument 'struct osmo_io_fd *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/a6882882_97172bed PS1, Line 28: void (*read_cb)(struct osmo_io_fd *, int res, struct msgb *); function definition argument 'struct msgb *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/34dae675_42dee70d PS1, Line 30: void (*write_cb)(struct osmo_io_fd *, int res, struct msgb *); function definition argument 'struct osmo_io_fd *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/4daaa2a1_cf98da01 PS1, Line 30: void (*write_cb)(struct osmo_io_fd *, int res, struct msgb *); function definition argument 'struct msgb *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/e73886e1_eeeaf2c4 PS1, Line 33: void (*sendmsg_cb)(struct osmo_io_fd *, int res, struct msgb *); function definition argument 'struct osmo_io_fd *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/90bd2fce_ca06ee12 PS1, Line 33: void (*sendmsg_cb)(struct osmo_io_fd *, int res, struct msgb *); function definition argument 'struct msgb *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/e4f803f5_59a8b316 PS1, Line 35: void (*recvmsg_cb)(struct osmo_io_fd *, int res, struct msgb *, struct osmo_sockaddr *); function definition argument 'struct osmo_io_fd *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/dfd5ab6a_2d4e0773 PS1, Line 35: void (*recvmsg_cb)(struct osmo_io_fd *, int res, struct msgb *, struct osmo_sockaddr *); function definition argument 'struct msgb *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/cc035462_005f9fb0 PS1, Line 35: void (*recvmsg_cb)(struct osmo_io_fd *, int res, struct msgb *, struct osmo_sockaddr *); function definition argument 'struct osmo_sockaddr *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/d82689fd_e87daffe PS1, Line 38: int (*segmentation_cb)(struct msgb *, int data_len); function definition argument 'struct msgb *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/36042dd0_0772c714 PS1, Line 41: void osmo_io_init(); Bad function definition - void osmo_io_init() should probably be void osmo_io_init(void) Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/a936804e_ce32a8bc PS1, Line 52: const struct osmo_sockaddr *dest); code indent should use tabs where possible Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/d3eef3f3_083169f7 PS1, Line 52: const struct osmo_sockaddr *dest); please, no spaces at the start of a line File src/osmo_io.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/99590406_b38a3166 PS1, Line 53: void osmo_iofd_init() Bad function definition - void osmo_iofd_init() should probably be void osmo_iofd_init(void) Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/a6271d9a_bb9bafff PS1, Line 226: memcpy(msgb_data(msg_pending), msgb_data(msg)+ len, pending_len); need consistent spacing around '+' (ctx:VxW) (or typedef missing in osmo-ci/lint/checkpatch/typedefs_osmo.txt?) Robot Comment from checkpatch (run ID jenkins-gerrit-lint-2606): https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/bd552a2b_5ca6e479 PS1, Line
Change in libosmocore[master]: Add osmo_io with initial poll backend
daniel has uploaded this change for review. (
https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
..
Add osmo_io with initial poll backend
* make backend configurable for later
* segmentation callback for chunked streams
* logging target for osmo_io
* support partial writes
Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0
Related: SYS#5094, OS#5751
---
M TODO-RELEASE
M include/Makefile.am
M include/osmocom/core/logging.h
A include/osmocom/core/osmo_io.h
M src/Makefile.am
M src/logging.c
A src/osmo_io.c
A src/osmo_io_internal.h
A src/osmo_io_poll.c
9 files changed, 851 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/34/30934/1
diff --git a/TODO-RELEASE b/TODO-RELEASE
index 6ac8db8..ef629c7 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -9,3 +9,4 @@
#library whatdescription / commit summary line
libosmocore new API osmo_sockaddr_is_any()
libosmocoreABI breakageOSMO_NUM_DLIB change affecting
internal_cat[]
+libosmocorenew API osmo_io_*
diff --git a/include/Makefile.am b/include/Makefile.am
index a234014..b59009d 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -37,6 +37,7 @@
osmocom/core/log2.h \
osmocom/core/logging.h \
osmocom/core/loggingrb.h \
+ osmocom/core/osmo_io.h \
osmocom/core/stats.h \
osmocom/core/macaddr.h \
osmocom/core/msgb.h \
diff --git a/include/osmocom/core/logging.h b/include/osmocom/core/logging.h
index 5ae38c7..8e30a20 100644
--- a/include/osmocom/core/logging.h
+++ b/include/osmocom/core/logging.h
@@ -154,7 +154,8 @@
#define DLCSN1 -26 /*!< CSN.1 (Concrete Syntax Notation 1) codec */
#define DLM2PA -27 /*!< Osmocom M2PA (libosmo-sigtran) */
#define DLM2UA -28 /*!< Reserved for future Osmocom M2UA
(libosmo-sigtran) */
-#define OSMO_NUM_DLIB 28 /*!< Number of logging sub-systems in libraries
*/
+#define DLIO -29 /*!< Osmocom IO sub-system */
+#define OSMO_NUM_DLIB 29 /*!< Number of logging sub-systems in libraries
*/
/* Colors that can be used in log_info_cat.color */
#define OSMO_LOGCOLOR_NORMAL NULL
diff --git a/include/osmocom/core/osmo_io.h b/include/osmocom/core/osmo_io.h
new file mode 100644
index 000..4fac3f1
--- /dev/null
+++ b/include/osmocom/core/osmo_io.h
@@ -0,0 +1,56 @@
+/*! \file osmo_io.h
+ * io(_uring) abstraction osmo fd compatibility
+ */
+
+#pragma once
+
+#include
+#include
+#include
+
+struct osmo_io_fd;
+
+enum osmo_io_fd_mode {
+ /*! use read() / write() calls */
+ OSMO_IO_FD_MODE_READ_WRITE,
+ /*! use recvfrom() / sendto() calls */
+ OSMO_IO_FD_MODE_RECVFROM_SENDTO,
+ /*! emulate sctp_recvmsg() and sctp_sendmsg() */
+ OSMO_IO_FD_MODE_SCTP_RECVMSG_SENDMSG,
+};
+
+enum osmo_io_backend {
+ OSMO_IO_BACKEND_POLL,
+};
+
+struct osmo_io_ops {
+ /*! call-back function when something was read from fd */
+ void (*read_cb)(struct osmo_io_fd *, int res, struct msgb *);
+ /*! call-back function when write has completed on fd */
+ void (*write_cb)(struct osmo_io_fd *, int res, struct msgb *);
+
+ /*! call-back function emulating sendto */
+ void (*sendmsg_cb)(struct osmo_io_fd *, int res, struct msgb *);
+ /*! call-back function emulating recvfrom */
+ void (*recvmsg_cb)(struct osmo_io_fd *, int res, struct msgb *, struct
osmo_sockaddr *);
+
+ /*! call-back function to segment the data returned by read_cb */
+ int (*segmentation_cb)(struct msgb *, int data_len);
+};
+
+void osmo_io_init();
+
+struct osmo_io_fd *osmo_iofd_setup(const void *ctx, unsigned int size,
unsigned int headroom,
+ int fd, const char *name, enum osmo_io_fd_mode mode,
+ const struct osmo_io_ops *ioops, void *data, unsigned int
priv_nr);
+void osmo_iofd_close(struct osmo_io_fd *iofd);
+int osmo_iofd_write_msgb(struct osmo_io_fd *iofd, struct msgb *msg);
+void osmo_iofd_read_enable(struct osmo_io_fd *iofd);
+void osmo_iofd_read_enable(struct osmo_io_fd *iofd);
+
+int osmo_iofd_sendto_msgb(struct osmo_io_fd *iofd, struct msgb *msg,
+ const struct osmo_sockaddr *dest);
+
+void *osmo_iofd_get_data(struct osmo_io_fd *iofd);
+unsigned int osmo_iofd_get_priv(struct osmo_io_fd *iofd);
+int osmo_iofd_get_fd(struct osmo_io_fd *iofd);
diff --git a/src/Makefile.am b/src/Makefile.am
index 2c73af6..ba0926e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -34,6 +34,7 @@
it_q.c \
probes.d \
base64.c \
+osmo_io.c osmo_io_poll.c \
$(NULL)
