pespin submitted this change.

View Change

Approvals: laforge: Looks good to me, but someone else must approve Jenkins Builder: Verified fixeria: Looks good to me, approved
logging_vty: Allow setting gsmtap log tgt as (blocking-io|nonblocking-io|wq)

The current patch adds the possibility to configure it, and leaves the
previous behavior as default: blocking-io.

Related: OS#6213
Change-Id: Id5d31bedd7d265d18f6e475ccbc94ced80598d04
---
M TODO-RELEASE
M include/osmocom/core/gsmtap_util.h
M include/osmocom/core/socket.h
M src/core/gsmtap_util.c
M src/core/libosmocore.map
M src/core/socket.c
M src/vty/logging_vty.c
7 files changed, 88 insertions(+), 7 deletions(-)

diff --git a/TODO-RELEASE b/TODO-RELEASE
index 1562b38..4859444 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -7,4 +7,5 @@
# If any interfaces have been added since the last public release: c:r:a + 1.
# If any interfaces have been removed or changed since the last public release: c:r:0.
#library what description / commit summary line
-core added osmo_sock_set_nonblock()
+core added osmo_sock_set_nonblock(), osmo_sock_get_nonblock()
+core added gsmtap_source_set_nonblock(), gsmtap_source_using_wq()
diff --git a/include/osmocom/core/gsmtap_util.h b/include/osmocom/core/gsmtap_util.h
index d24ee95..5cad23b 100644
--- a/include/osmocom/core/gsmtap_util.h
+++ b/include/osmocom/core/gsmtap_util.h
@@ -1,6 +1,7 @@
#pragma once

#include <stdint.h>
+#include <stdbool.h>
#include <osmocom/core/write_queue.h>
#include <osmocom/core/select.h>

@@ -43,8 +44,11 @@

void gsmtap_source_free(struct gsmtap_inst *gti);

+int gsmtap_source_set_nonblock(struct gsmtap_inst *gti, int on);
int gsmtap_source_add_sink(struct gsmtap_inst *gti);

+bool gsmtap_source_using_wq(const struct gsmtap_inst *gti);
+
int gsmtap_sendmsg(struct gsmtap_inst *gti, struct msgb *msg);
int gsmtap_sendmsg_free(struct gsmtap_inst *gti, struct msgb *msg);

diff --git a/include/osmocom/core/socket.h b/include/osmocom/core/socket.h
index 088a444..e9979f5 100644
--- a/include/osmocom/core/socket.h
+++ b/include/osmocom/core/socket.h
@@ -219,6 +219,7 @@

int osmo_sock_set_dscp(int fd, uint8_t dscp);
int osmo_sock_set_priority(int fd, int prio);
+int osmo_sock_get_nonblock(int fd);
int osmo_sock_set_nonblock(int fd, int on);

int osmo_sock_sctp_get_peer_addr_info(int fd, struct sctp_paddrinfo *pinfo, size_t *pinfo_cnt);
diff --git a/src/core/gsmtap_util.c b/src/core/gsmtap_util.c
index 903821d..b21de74 100644
--- a/src/core/gsmtap_util.c
+++ b/src/core/gsmtap_util.c
@@ -419,6 +419,20 @@
signal_dbm, snr, data, len);
}

+/*! Set the gsmtap source socket as non-blocking (see sockopt O_NONBLOCK).
+ * \param[in] gti existing GSMTAP source
+ * \param[in] on set to 1 to set as non-blocking, 0 to set as blocking.
+ * \returns 0 on success; negative on error
+ *
+ * This setting is ignored when \ref gti was initialied in ofd_wq_mode=1 mode.
+ */
+int gsmtap_source_set_nonblock(struct gsmtap_inst *gti, int on)
+{
+ if (gti->osmo_io_mode)
+ return 0;
+ return osmo_sock_set_nonblock(gti->source_fd, on);
+}
+
/*! Add a local sink to an existing GSMTAP source and return fd
* \param[in] gti existing GSMTAP source
* \returns file descriptor of locally bound receive socket; negative on error
@@ -447,6 +461,15 @@
return rc;
}

+/*! Was GSMTAP source configured to use a workqueue (ofd_wq_mode=1)?
+ * \param[in] gti existing GSMTAP source
+ * \returns Whether GSMTAP source was configured to use a workqueue (ofd_wq_mode=1)
+ */
+bool gsmtap_source_using_wq(const struct gsmtap_inst *gti)
+{
+ return gti->osmo_io_mode;
+}
+
/* 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)
{
diff --git a/src/core/libosmocore.map b/src/core/libosmocore.map
index 166eea9..6a46f6c 100644
--- a/src/core/libosmocore.map
+++ b/src/core/libosmocore.map
@@ -54,6 +54,8 @@
gsmtap_source_init2;
gsmtap_source_init_fd;
gsmtap_source_init_fd2;
+gsmtap_source_set_nonblock;
+gsmtap_source_using_wq;
gsmtap_type_names;
log_add_target;
log_category_name;
@@ -453,6 +455,7 @@
osmo_sock_multiaddr_get_ip_and_port;
osmo_sock_multiaddr_get_name_buf;
osmo_sock_sctp_get_peer_addr_info;
+osmo_sock_get_nonblock;
osmo_sock_set_dscp;
osmo_sock_set_nonblock;
osmo_sock_set_priority;
diff --git a/src/core/socket.c b/src/core/socket.c
index 01093bb..bb32185 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -46,6 +46,7 @@
#include <netinet/in.h>
#include <arpa/inet.h>

+#include <fcntl.h>
#include <stdio.h>
#include <unistd.h>
#include <stdint.h>
@@ -2756,6 +2757,17 @@
return ioctl(fd, FIONBIO, (unsigned char *)&on);
}

+/*! Find out whether the socket is configured as non-blocking or blocking.
+ * \param[in] fd File descriptor of the socket
+ * \returns 1 == nonblocking, 0 == blocking, < 0 error */
+int osmo_sock_get_nonblock(int fd)
+{
+ int flags = fcntl(fd, F_GETFL);
+ if (flags < 0)
+ return flags;
+ return (flags & O_NONBLOCK) ? 1 : 0;
+}
+
#ifdef HAVE_LIBSCTP
/*! Fill in array of struct sctp_paddrinfo with each of the remote addresses of an SCTP socket
* \param[in] fd file descriptor of SCTP socket
diff --git a/src/vty/logging_vty.c b/src/vty/logging_vty.c
index 678ae68..259d22b 100644
--- a/src/vty/logging_vty.c
+++ b/src/vty/logging_vty.c
@@ -29,7 +29,9 @@
#include <osmocom/core/strrb.h>
#include <osmocom/core/loggingrb.h>
#include <osmocom/core/gsmtap.h>
+#include <osmocom/core/gsmtap_util.h>
#include <osmocom/core/application.h>
+#include <osmocom/core/socket.h>

#include <osmocom/vty/command.h>
#include <osmocom/vty/buffer.h>
@@ -810,25 +812,50 @@
}

DEFUN(cfg_log_gsmtap, cfg_log_gsmtap_cmd,
- "log gsmtap [HOSTNAME]",
+ "log gsmtap [HOSTNAME] [(nonblocking-io|blocking-io|wq)]",
LOG_STR "Logging via GSMTAP\n"
- "Host name to send the GSMTAP logging to (UDP port 4729)\n")
+ "Host name to send the GSMTAP logging to (UDP port 4729)\n"
+ "Use non-blocking, synchronous I/O (may lose msgs if UDP socket send buffer becomes full)\n"
+ "Use blocking, synchronous I/O (only for debug purposes or when blocking is acceptable) (default)\n"
+ "Use Tx workqueue, asynchronous I/O (may lose msgs if queue becomes full)\n")
{
- const char *hostname = argc ? argv[0] : "127.0.0.1";
+ const char *hostname = argc > 0 ? argv[0] : "127.0.0.1";
+ bool ofd_wq_mode = argc > 1 && (strcmp(argv[1], "wq") == 0);
+ bool nonblocking_io = argc > 1 && (strcmp(argv[1], "nonblocking-io") == 0);
struct log_target *tgt;

log_tgt_mutex_lock();
tgt = log_target_find(LOG_TGT_TYPE_GSMTAP, hostname);
if (!tgt) {
tgt = log_target_create_gsmtap(hostname, GSMTAP_UDP_PORT,
- host.app_info->name, false,
+ host.app_info->name, ofd_wq_mode,
true);
if (!tgt) {
vty_out(vty, "%% Unable to create GSMTAP log for %s%s",
hostname, VTY_NEWLINE);
RET_WITH_UNLOCK(CMD_WARNING);
}
+ if (!ofd_wq_mode && nonblocking_io) {
+ int rc = gsmtap_source_set_nonblock(tgt->tgt_gsmtap.gsmtap_inst, 1);
+ if (rc != 0)
+ vty_out(vty, "%% Unable to configure GSMTAP log for %s as nonblocking-io%s",
+ hostname, VTY_NEWLINE);
+ }
log_add_target(tgt);
+ } else {
+ if (ofd_wq_mode != gsmtap_source_using_wq(tgt->tgt_gsmtap.gsmtap_inst)) {
+ vty_out(vty, "%% Remove and re-add the log gsmtap target in order to "
+ "change between synchronous and asynchronous IO%s", VTY_NEWLINE);
+ RET_WITH_UNLOCK(CMD_WARNING);
+ }
+ if (!ofd_wq_mode) {
+ int fd = gsmtap_inst_fd2(tgt->tgt_gsmtap.gsmtap_inst);
+ if (fd > 0 && osmo_sock_get_nonblock(fd) != nonblocking_io) {
+ vty_out(vty, "%% Remove and re-add the log gsmtap target in order to "
+ "change between blocking and non-blocking IO%s", VTY_NEWLINE);
+ RET_WITH_UNLOCK(CMD_WARNING);
+ }
+ }
}

vty->index = tgt;
@@ -1009,6 +1036,7 @@
{
char level_buf[128];
int i;
+ char *pars;

switch (tgt->type) {
case LOG_TGT_TYPE_VTY:
@@ -1039,8 +1067,17 @@
log_target_rb_avail_size(tgt), VTY_NEWLINE);
break;
case LOG_TGT_TYPE_GSMTAP:
- vty_out(vty, "log gsmtap %s%s",
- tgt->tgt_gsmtap.hostname, VTY_NEWLINE);
+ if (gsmtap_source_using_wq(tgt->tgt_gsmtap.gsmtap_inst)) {
+ pars = " wq";
+ } else {
+ int fd = gsmtap_inst_fd2(tgt->tgt_gsmtap.gsmtap_inst);
+ if (fd > 0 && osmo_sock_get_nonblock(fd))
+ pars = " nonblocking-io";
+ else
+ pars = "";
+ }
+ vty_out(vty, "log gsmtap %s%s%s",
+ tgt->tgt_gsmtap.hostname, pars, VTY_NEWLINE);
break;
case LOG_TGT_TYPE_SYSTEMD:
vty_out(vty, "log systemd-journal%s%s",

To view, visit change 41702. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: merged
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id5d31bedd7d265d18f6e475ccbc94ced80598d04
Gerrit-Change-Number: 41702
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de>
Gerrit-Reviewer: laforge <laforge@osmocom.org>
Gerrit-Reviewer: pespin <pespin@sysmocom.de>