[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 9: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/7263655c_446a9895 PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); > Test to reproduce your issue (even with the old wqueue code) and proposed fix > here: […] the fix for that other issue (disabling log / endless loop) has been merged -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 9 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Comment-Date: Thu, 09 Nov 2023 12:48:17 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
laforge has submitted this change. (
https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email )
Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues
..
gsmtap_util: Use Osmo IO instead of Osmo write queues
- Adapt decl. of 'struct gsmtap_inst' for usage of Osmo IO while maintaining
backwards compatibility
- Maintain legacy behavior without any message queues if osmo_io_mode is zero
Related: OS#6213
Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0
---
M src/core/gsmtap_util.c
1 file changed, 53 insertions(+), 37 deletions(-)
Approvals:
laforge: Looks good to me, approved
daniel: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/src/core/gsmtap_util.c b/src/core/gsmtap_util.c
index dcbd304..458d1d7 100644
--- a/src/core/gsmtap_util.c
+++ b/src/core/gsmtap_util.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
@@ -46,13 +47,28 @@
*
* \file gsmtap_util.c */
-/*! one gsmtap instance */
+/*! one gsmtap instance
+ * Until gsmtap_inst_fd() is removed from the API at some point in the
future, we have to keep the first member as
+ * 'int' and the second as 'struct osmo_wqueue' (this effectively makes sure
that the struct member wq.bfd.fd maintains
+ * the same memory offset from the start of the struct) to ensure that
inlined static 'instances' of gsmtap_inst_fd() in
+ * old binaries keep working the way they used to even with gsmtap_inst
objects obtained from newer versions of libosmocore */
struct gsmtap_inst {
- int ofd_wq_mode;/*!< wait queue mode? This field member may not
be changed or moved (backwards compatibility) */
- struct osmo_wqueue wq; /*!< the wait queue. This field member may not
be changed or moved (backwards compatibility) */
- struct osmo_fd sink_ofd; /*!< file descriptor */
+ int osmo_io_mode; /*!< Indicates whether or not to use Osmo IO
mode for message output (thus enabling use of tx queues).
+ * This field member may not be changed or
moved (backwards compatibility) */
+ struct osmo_wqueue wq;/*!< the wait queue. This field member may
not be changed or moved (backwards compatibility) */
+
+ struct osmo_io_fd *out; /*!< Used when osmo_io_mode is nonzero */
+ struct osmo_fd sink_ofd;
};
+struct _gsmtap_inst_legacy {
+ int ofd_wq_mode;
+ struct osmo_wqueue wq;
+ struct osmo_fd sink_ofd;
+};
+osmo_static_assert(offsetof(struct gsmtap_inst, wq) == offsetof(struct
_gsmtap_inst_legacy, wq),
+ gsmtap_inst_new_wq_offset_equals_legacy_wq_offset);
+
/*! Deprecated, use gsmtap_inst_fd2() instead
* \param[in] gti GSMTAP instance
* \returns file descriptor of GSMTAP instance */
@@ -253,6 +269,7 @@
#include
#include
+#include
/*! Create a new (sending) GSMTAP source socket
* \param[in] host host name or IP address in string format
@@ -346,8 +363,8 @@
if (!gti)
return -ENODEV;
- if (gti->ofd_wq_mode)
- return osmo_wqueue_enqueue(>wq, msg);
+ if (gti->osmo_io_mode)
+ return osmo_iofd_write_msgb(gti->out, msg);
else {
/* try immediate send and return error if any */
int rc;
@@ -416,35 +433,17 @@
signal_dbm, snr, data, len);
}
-/* Callback from select layer if we can write to the socket */
-static int gsmtap_wq_w_cb(struct osmo_fd *ofd, struct msgb *msg)
-{
- int rc;
-
- rc = write(ofd->fd, msg->data, msg->len);
- if (rc < 0) {
- return rc;
- }
- if (rc != msg->len) {
- return -EIO;
- }
-
- return 0;
-}
-
/* Callback from select layer if we can read from the sink socket */
static int gsmtap_sink_fd_cb(struct osmo_fd *fd, unsigned int flags)
{
int rc;
uint8_t buf[4096];
-
if (!(flags & OSMO_FD_READ))
return 0;
rc = read(fd->fd, buf, sizeof(buf));
- if (rc < 0) {
+ if (rc < 0)
return rc;
- }
/* simply discard any data arriving on the socket */
return 0;
@@ -473,7 +472,7 @@
if (fd < 0)
return fd;
- if (gti->ofd_wq_mode) {
+ if (gti->osmo_io_mode) {
struct osmo_fd *sink_ofd;
sink_ofd = >sink_ofd;
@@ -491,6 +490,12 @@
return fd;
}
+/* Registered in Osmo IO as a no-op to set the write callback. */
+static void gsmtap_ops_noop_cb(struct osmo_io_fd *iofd, int res, struct msgb
*msg)
+{
+}
+
+static struct osmo_io_ops gsmtap_ops = { .write_cb = gsmtap_ops_noop_cb };
/*! Open GSMTAP source socket, connect and register osmo_fd
* \param[in] local_host IP address in string format
@@ -509,27 +514,27 @@
const char *rem_host, uint16_t
rem_port, int
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 8: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: pespin Gerrit-Comment-Date: Thu, 09 Nov 2023 12:47:28 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, laforge, pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 8: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/b190b3cc_c6fef766 PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); > This would have also happened if the old code used write_queues, maybe that's > the reason they didn't […] Test to reproduce your issue (even with the old wqueue code) and proposed fix here: https://gerrit.osmocom.org/c/libosmocore/+/34967 https://gerrit.osmocom.org/c/libosmocore/+/34966 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Fri, 03 Nov 2023 20:27:17 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, laforge, pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 8: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/ea79b08c_33cae098 PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); > @dwillmann@sysmocom. […] This would have also happened if the old code used write_queues, maybe that's the reason they didn't? In any case, I would suggest we temporarily disable the log target (by setting loglevel to LOGL_FATAL + 1) while we're in _gsmtap_raw_output(). That has the benefit that this log message would appear in other log targets such as stderr. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 31 Oct 2023 15:49:54 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: daniel, laforge, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 8: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/8a7aef64_440b964c PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); > @dwillm...@sysmocom.de just tried changing the VTY command... […] @dwillm...@sysmocom.de there's a bug in Osmo IO, basically if gsmtap logging is used, then if the queue overflows, we get an infinite call loop ``` #3729 0x77e0e0f4 in _gsmtap_raw_output (target=0x55617c60, subsys=31, level=7, file=0x77e45249 "osmo_io.c", line=377, cont=0, format=0x77e453d8 "iofd(%s)enqueueing message failed (%d). Rejecting msgb\n", ap=0x7f873e20) at logging_gsmtap.c:117 #3730 0x77e0c335 in osmo_vlogp (subsys=31, level=7, file=0x77e45249 "osmo_io.c", line=377, cont=0, format=0x77e453d8 "iofd(%s)enqueueing message failed (%d). Rejecting msgb\n", ap=0x7f873e98) at logging.c:728 #3731 0x77e0c52b in logp2 (subsys=-29, level=7, file=0x77e45249 "osmo_io.c", line=377, cont=0, format=0x77e453d8 "iofd(%s)enqueueing message failed (%d). Rejecting msgb\n") at logging.c:766 #3732 0x77e1643e in osmo_iofd_write_msgb (iofd=0x55635910, msg=0x58bf1ec0) at osmo_io.c:377 ``` (see backtrace numbers :)) because of the LOGPIO call in case of overflow. Not sure if this was also a bug when Osmo write queues were used (if LOGPIO was called to log a full-queue error, then yes...) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 31 Oct 2023 09:56:11 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: daniel, laforge, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 8: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/2c898517_6a7f6f4e PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); > @dwillm...@sysmocom.de just tried changing the VTY command... […] (currently investigating...) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 31 Oct 2023 09:17:22 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: daniel, laforge, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 8: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/bba3c6b5_64b01e61 PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); > > In that case I guess we want to limit somehow the read buffer queue in the > > kernel to the minimum ( […] @dwillm...@sysmocom.de just tried changing the VTY command... makes the TTCN3 mgw test fail (-ENOSPC being returned, I suppose because the queue length of 64 isn't enough for MGCP_Test.TC_crcx_wildcarded_exhaust) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 31 Oct 2023 08:56:35 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 8: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 24 Oct 2023 19:17:54 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, laforge, pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 8: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: arehbein Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 24 Oct 2023 13:22:28 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, laforge, pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 8: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/8a07c099_d0014ba2 PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); > In that case I guess we want to limit somehow the read buffer queue in the > kernel to the minimum (even 0) to avoid wasting kb/mb of memory for each > gsmtap socket sink with filled message that are never read. Yeah, might be a good idea, but out of scope for this patch. @arehb...@sysmocom.de gsmtap_source_add_sink is called in logging_gsmtap.c and also in osmo_pcu and osmo_bts. In libosmocore/src/vty/logging_vty.c cfg_log_gsmtap calls log_target_create_gsmtap with false, true as last parameters. That means ofd_wq_mode false and add_sink true. We should change the ofd_wq_mode parameter to true here, otherwise it doesn't use osmo_io at all. Not sure if we need a VTY parameter for that. Since add_sink is true the code will call gsmtap_source_add_sink() which creates the sink only if the gsmtap target is local. So simply enabling gsmtap logging to 127.0.0.1 should call the code in question. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: arehbein Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 24 Oct 2023 12:05:02 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, daniel, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 8: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/1e28f701_1c259d04 PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); > it may make sense to look at what shutdown(sink_fd, SHUT_RD) may do to a UDP > socket, perhaps it alre […] hmm, looks like shutdown is not really implemented in UDP sockets: https://stackoverflow.com/questions/60105082/using-shutdown-for-a-udp-socket In that case I guess we want to limit somehow the read buffer queue in the kernel to the minimum (even 0) to avoid wasting kb/mb of memory for each gsmtap socket sink with filled message that are never read. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: arehbein Gerrit-Attention: laforge Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 24 Oct 2023 11:31:41 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: laforge Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, daniel, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 8: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/814c540a_ef231d55 PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); > I have restored pre-patch functionality for the sink in this patch and added > https://gerrit.osmocom. […] it may make sense to look at what shutdown(sink_fd, SHUT_RD) may do to a UDP socket, perhaps it already instructs the kernel to directly discard wathever is received. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: arehbein Gerrit-Attention: laforge Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 24 Oct 2023 11:09:51 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: laforge Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: daniel, laforge, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 8: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/f3aa93d6_8c2fdd5c PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); > I agree, it probably make sense to […] I have restored pre-patch functionality for the sink in this patch and added https://gerrit.osmocom.org/c/libosmocore/+/34881 to simplify it. It's WIP, because from what I could tell, the only project currently using a sink is Osmocom-BB and that is running its own version of libosmocore anyways so I'm not sure how to best test this quickly. I could probably add a sink to osmo-mgw locally, and then manually send random packets to the socket to see if the behavior changes (?) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 24 Oct 2023 10:07:18 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: laforge Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, daniel, pespin. Hello Jenkins Builder, daniel, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email to look at the new patch set (#8). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. gsmtap_util: Use Osmo IO instead of Osmo write queues - Adapt decl. of 'struct gsmtap_inst' for usage of Osmo IO while maintaining backwards compatibility - Maintain legacy behavior without any message queues if osmo_io_mode is zero Related: OS#6213 Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 --- M src/core/gsmtap_util.c 1 file changed, 53 insertions(+), 37 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/43/34743/8 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: arehbein Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, daniel, pespin. Hello Jenkins Builder, daniel, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email to look at the new patch set (#7). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. gsmtap_util: Use Osmo IO instead of Osmo write queues - Adapt decl. of 'struct gsmtap_inst' for usage of Osmo IO while maintaining backwards compatibility - Maintain legacy behavior without any message queues if osmo_io_mode is zero Related: OS#6213 Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 --- M src/core/gsmtap_util.c 1 file changed, 52 insertions(+), 37 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/43/34743/7 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 7 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: arehbein Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, daniel, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 6: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/c1668f0e_26b57ac4 PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); > > simply never read from that fd and just keep it open there so no ICMP > > errors are sent […] I agree, it probably make sense to * keep the current code for the sink as-is for this patch * consider developing/testing/merging a patch that disables reading from that socket altogether. I would be surprised if there would be ICMP unreachable errors for a socket that's open. So it should work and reduced the load on the osmocom processes as they don't have to receive back each of those packets just to discard it. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 6 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: arehbein Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 24 Oct 2023 09:13:55 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, laforge, pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 6: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/60b927bc_93d7e161 PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); > simply never read from that fd and just keep it open there so no ICMP errors > are sent Yeah, one option is to keep it like it was not using osmo_io (so that the read doesn't allocate). I was wondering what happens if you open the socket and simply never read from it since we don't care about the data anyway. I don't think it's worth the effort here because we might need to decrease the kernel receive buffer and even then the kernel could send ICMP errors. So I'd suggest to keep the source_add_sink code as it was before (using osmo_fd with the read callback). -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 6 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: arehbein Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 24 Oct 2023 08:50:22 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: daniel, laforge, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 6: (1 comment) Patchset: PS1: > So, I took a short look again and it appears to be the `#if (!EMBEDDED)` in > `include/osmocom/core/so […] (solved via patch update here instead) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 6 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 24 Oct 2023 08:37:05 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: daniel, laforge, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 5: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/c7aff446_40c571ac PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); I have (for now) fixed the memleak keeping Osmo IO, since it was a quick fix and I'm not entirely I understand what you mean by > simply never read from that fd and just keep it open there so no ICMP errors > are sent ? I would think it means returning after the `fd = gsmtap_source_add_sink_fd(gsmtap_inst_fd2(gti));` line in `gsmtap_source_add_sink()`, but then I wouldn't understand why the call to `read()` was added to the Osmo FD callback in the original code. I can of course also just go back to what the code did before (regarding sink functionality), but I thought I'd ask, since you mentioned these two options. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Sun, 22 Oct 2023 13:51:16 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: daniel, laforge, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 6: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/ad6358bb_936cc4f4 PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); > I have (for now) fixed the memleak keeping Osmo IO, since it was a quick fix > and I'm not entirely I […] P.s.: I assume this isn't anything Osmo IO should be implementing, because this is more of a fringe case (actually reading data just to discard it)? -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 6 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Sun, 22 Oct 2023 13:53:27 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, laforge, pespin. Hello Jenkins Builder, daniel, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Code-Review+1 by pespin, Verified+1 by Jenkins Builder Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. gsmtap_util: Use Osmo IO instead of Osmo write queues - Adapt decl. of 'struct gsmtap_inst' for usage of Osmo IO while maintaining backwards compatibility - Maintain legacy behavior without any message queues if osmo_io_mode is zero Related: OS#6213 Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 --- M src/core/gsmtap_util.c 1 file changed, 66 insertions(+), 64 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/43/34743/6 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 6 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: arehbein Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, laforge. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 5: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/d4de8de3_f6d6e4ca PS5, Line 478: osmo_iofd_setup(gti, gti->sink_fd, "gsmtap_inst.out", OSMO_IO_FD_MODE_READ_WRITE, _sink_ops, NULL); This will leak iofds returned by osmo_iofd_setup(). Also having a read_cb() will cause (de-)allocations for each message, which will impact performance. Maybe the sink should stay a struct osmo_fd? The alternative would be to simply never read from that fd and just keep it open there so no ICMP errors are sent. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: arehbein Gerrit-Attention: laforge Gerrit-Comment-Date: Fri, 20 Oct 2023 20:27:38 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: daniel, laforge. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 5: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/f34c565e_66e9d891 PS1, Line 483: TODO > https://gerrit.osmocom. […] patch above was abandoned in favor of https://gerrit.osmocom.org/c/libosmocore/+/34835 (part of this upload series) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: laforge Gerrit-Attention: daniel Gerrit-Comment-Date: Thu, 19 Oct 2023 16:31:33 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: laforge Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, daniel, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: arehbein Gerrit-Attention: laforge Gerrit-Attention: daniel Gerrit-Comment-Date: Thu, 19 Oct 2023 16:22:29 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, daniel, laforge, pespin. Hello Jenkins Builder, daniel, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Code-Review+1 by pespin, Verified-1 by Jenkins Builder Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. gsmtap_util: Use Osmo IO instead of Osmo write queues - Adapt decl. of 'struct gsmtap_inst' for usage of Osmo IO while maintaining backwards compatibility - Maintain legacy behavior without any message queues if osmo_io_mode is zero Related: OS#6213 Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 --- M src/core/gsmtap_util.c 1 file changed, 63 insertions(+), 64 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/43/34743/5 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: arehbein Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: arehbein, daniel, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 3 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: arehbein Gerrit-Attention: laforge Gerrit-Attention: daniel Gerrit-Comment-Date: Mon, 16 Oct 2023 14:28:35 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: daniel, laforge, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 3: (3 comments) This change is ready for review. Patchset: PS1: > WIP, see comment on write callback https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/c820b4d9_2ce6f4a0/ File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/fd6f571f_2548a01d PS2, Line 441: } > msg doesn't need to be freed here? Ah yes, thanks. The case `OSMO_FD_READ` doesn't free the msgb, unlike `OSMO_FD_WRITE`, I was only looking at the latter. https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/9c2cf47b_92c44199 PS2, Line 493: * \param[in] osmo_io_mode Register \ref osmo_wqueue (1) or not (0) > Don't change the API param name, it's not really needed. Done -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 3 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Mon, 16 Oct 2023 13:51:06 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: daniel. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 1: (1 comment) File src/core/gsmtap_util.c: https://gerrit.osmocom.org/c/libosmocore/+/34743/comment/c820b4d9_2ce6f4a0 PS1, Line 483: TODO I think we need to add this kind of function to the Osmo IO API, because with write queues, a return value of -EAGAIN would have led to the write queue backend putting the message in front of the queue again. As far as I can see, Osmo IO does not do this after the write callback failed. So in order to make gsmtap_util behave as it used to, we need this kind of function for Osmo IO. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Attention: daniel Gerrit-Comment-Date: Fri, 13 Oct 2023 16:09:43 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in libosmocore[master]: gsmtap_util: Use Osmo IO instead of Osmo write queues
Attention is currently required from: daniel. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email ) Change subject: gsmtap_util: Use Osmo IO instead of Osmo write queues .. Patch Set 1: (1 comment) This change is ready for review. Patchset: PS1: WIP, see comment on write callback -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34743?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Iadbbef74e3add7001d84dd6b68f51eac293e44d0 Gerrit-Change-Number: 34743 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Attention: daniel Gerrit-Comment-Date: Fri, 13 Oct 2023 16:06:48 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
