Discussion:
[PATCH 3/5] libctrl: Avoid using external tall_bsc_ctx
Harald Welte
2014-08-20 18:15:08 UTC
Permalink
Instead of using one flat talloc context (and one that is specific to
openbsc), we should attach the objects to whatever parent context they
are being used in.
---
src/ctrl/control_if.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/ctrl/control_if.c b/src/ctrl/control_if.c
index 7ce8bf6..a208071 100644
--- a/src/ctrl/control_if.c
+++ b/src/ctrl/control_if.c
@@ -96,7 +96,7 @@ struct ctrl_cmd *ctrl_cmd_trap(struct ctrl_cmd *cmd)
{
struct ctrl_cmd *trap;

- trap = ctrl_cmd_cpy(tall_bsc_ctx, cmd);
+ trap = ctrl_cmd_cpy(cmd, cmd);
if (!trap)
return NULL;

@@ -281,10 +281,10 @@ static uint64_t get_rate_ctr_value(const struct rate_ctr *ctr, int intv)
}
}

-static char *get_all_rate_ctr_in_group(const struct rate_ctr_group *ctrg, int intv)
+static char *get_all_rate_ctr_in_group(void *ctx, const struct rate_ctr_group *ctrg, int intv)
{
int i;
- char *counters = talloc_strdup(tall_bsc_ctx, "");
+ char *counters = talloc_strdup(ctx, "");
if (!counters)
return NULL;

@@ -314,7 +314,7 @@ static int get_rate_ctr_group(const char *ctr_group, int intv, struct ctrl_cmd *
if (!ctrg)
break;

- counters = get_all_rate_ctr_in_group(ctrg, intv);
+ counters = get_all_rate_ctr_in_group(cmd, ctrg, intv);
if (!counters)
goto oom;

@@ -340,7 +340,7 @@ static int get_rate_ctr_group_idx(const struct rate_ctr_group *ctrg, int intv, s
{
char *counters;

- counters = get_all_rate_ctr_in_group(ctrg, intv);
+ counters = get_all_rate_ctr_in_group(cmd, ctrg, intv);
if (!counters)
goto oom;

@@ -527,7 +527,7 @@ struct ctrl_handle *controlif_setup(struct gsm_network *gsmnet, uint16_t port,
int ret;
struct ctrl_handle *ctrl;

- ctrl = talloc_zero(tall_bsc_ctx, struct ctrl_handle);
+ ctrl = talloc_zero(gsmnet, struct ctrl_handle);
if (!ctrl)
return NULL;
--
2.1.0.rc1
Harald Welte
2014-08-20 18:15:07 UTC
Permalink
---
include/osmocom/ctrl/control_cmd.h | 3 +--
include/osmocom/ctrl/control_if.h | 6 ++++--
src/ctrl/control_cmd.c | 4 +---
src/ctrl/control_if.c | 24 ++++++++----------------
4 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/include/osmocom/ctrl/control_cmd.h b/include/osmocom/ctrl/control_cmd.h
index 2e6863a..10717ac 100644
--- a/include/osmocom/ctrl/control_cmd.h
+++ b/include/osmocom/ctrl/control_cmd.h
@@ -4,11 +4,10 @@
#include <osmocom/core/msgb.h>
#include <osmocom/core/talloc.h>
#include <osmocom/core/write_queue.h>
+#include <osmocom/core/logging.h>

#include <osmocom/vty/vector.h>

-#include <openbsc/vty.h>
-
#define CTRL_CMD_ERROR -1
#define CTRL_CMD_HANDLED 0
#define CTRL_CMD_REPLY 1
diff --git a/include/osmocom/ctrl/control_if.h b/include/osmocom/ctrl/control_if.h
index d103332..9ab3b2a 100644
--- a/include/osmocom/ctrl/control_if.h
+++ b/include/osmocom/ctrl/control_if.h
@@ -2,8 +2,10 @@
#define _CONTROL_IF_H

#include <osmocom/core/write_queue.h>
-#include <openbsc/control_cmd.h>
-#include <openbsc/gsm_data.h>
+#include <osmocom/ctrl/control_cmd.h>
+
+/* FIXME: this must go */
+struct gsm_network;

typedef int (*ctrl_cmd_handler)(struct ctrl_cmd *, void *);

diff --git a/src/ctrl/control_cmd.c b/src/ctrl/control_cmd.c
index 44cfa48..45e517d 100644
--- a/src/ctrl/control_cmd.c
+++ b/src/ctrl/control_cmd.c
@@ -29,9 +29,7 @@
#include <time.h>
#include <unistd.h>

-#include <openbsc/control_cmd.h>
-#include <openbsc/debug.h>
-#include <openbsc/vty.h>
+#include <osmocom/ctrl/control_cmd.h>

#include <osmocom/core/msgb.h>
#include <osmocom/core/talloc.h>
diff --git a/src/ctrl/control_if.c b/src/ctrl/control_if.c
index 156a24f..7ce8bf6 100644
--- a/src/ctrl/control_if.c
+++ b/src/ctrl/control_if.c
@@ -38,31 +38,21 @@
#include <sys/socket.h>
#include <sys/types.h>

-#include <openbsc/control_cmd.h>
-#include <openbsc/control_if.h>
-#include <openbsc/debug.h>
-#include <openbsc/gsm_data.h>
-#include <openbsc/ipaccess.h>
-#include <openbsc/socket.h>
-#include <osmocom/abis/subchan_demux.h>
-
-#include <openbsc/abis_rsl.h>
-#include <openbsc/abis_nm.h>
+#include <osmocom/ctrl/control_cmd.h>
+#include <osmocom/ctrl/control_if.h>

#include <osmocom/core/msgb.h>
#include <osmocom/core/rate_ctr.h>
#include <osmocom/core/select.h>
#include <osmocom/core/statistics.h>
#include <osmocom/core/talloc.h>
+#include <osmocom/core/socket.h>

-#include <osmocom/gsm/tlv.h>
+#include <osmocom/gsm/protocol/ipaccess.h>

#include <osmocom/vty/command.h>
#include <osmocom/vty/vector.h>

-#include <osmocom/abis/e1_input.h>
-#include <osmocom/abis/ipa.h>
-
vector ctrl_node_vec;

/* Send command to all */
@@ -551,8 +541,10 @@ struct ctrl_handle *controlif_setup(struct gsm_network *gsmnet, uint16_t port,
goto err;

/* Listen for control connections */
- ret = make_sock(&ctrl->listen_fd, IPPROTO_TCP, INADDR_LOOPBACK, port,
- 0, listen_fd_cb, ctrl);
+ ctrl->listen_fd.cb = listen_fd_cb;
+ ctrl->listen_fd.data = ctrl;
+ ret = osmo_sock_init_ofd(&ctrl->listen_fd, AF_INET, SOCK_STREAM, IPPROTO_TCP,
+ "127.0.0.1", port, OSMO_SOCK_F_BIND);
if (ret < 0)
goto err_vec;
--
2.1.0.rc1
Harald Welte
2014-08-20 18:15:09 UTC
Permalink
... and make libctrl code use it
---
include/osmocom/core/logging.h | 3 ++-
src/ctrl/control_cmd.c | 30 +++++++++++++++---------------
src/ctrl/control_if.c | 26 +++++++++++++-------------
src/logging.c | 5 +++++
4 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/include/osmocom/core/logging.h b/include/osmocom/core/logging.h
index caeea06..3c5e7b1 100644
--- a/include/osmocom/core/logging.h
+++ b/include/osmocom/core/logging.h
@@ -67,7 +67,8 @@ void logp(int subsys, const char *file, int line, int cont, const char *format,
#define DLMI -5
#define DLMIB -6
#define DLSMS -7
-#define OSMO_NUM_DLIB 7
+#define DLCTRL -8
+#define OSMO_NUM_DLIB 8

struct log_category {
uint8_t loglevel;
diff --git a/src/ctrl/control_cmd.c b/src/ctrl/control_cmd.c
index 45e517d..8c92033 100644
--- a/src/ctrl/control_cmd.c
+++ b/src/ctrl/control_cmd.c
@@ -188,7 +188,7 @@ static void create_cmd_struct(struct ctrl_cmd_struct *cmd, const char *name)
for (cur = name, word = NULL; cur[0] != '\0'; ++cur) {
/* warn about optionals */
if (cur[0] == '(' || cur[0] == ')' || cur[0] == '|') {
- LOGP(DCTRL, LOGL_ERROR,
+ LOGP(DLCTRL, LOGL_ERROR,
"Optionals are not supported in '%s'\n", name);
goto failure;
}
@@ -223,7 +223,7 @@ int ctrl_cmd_install(enum ctrl_node_type node, struct ctrl_cmd_element *cmd)
if (!cmds_vec) {
cmds_vec = vector_init(5);
if (!cmds_vec) {
- LOGP(DCTRL, LOGL_ERROR, "vector_init failed.\n");
+ LOGP(DLCTRL, LOGL_ERROR, "vector_init failed.\n");
return -ENOMEM;
}
vector_set_index(ctrl_node_vec, node, cmds_vec);
@@ -291,7 +291,7 @@ struct ctrl_cmd *ctrl_cmd_parse(void *ctx, struct msgb *msg)

cmd = talloc_zero(ctx, struct ctrl_cmd);
if (!cmd) {
- LOGP(DCTRL, LOGL_ERROR, "Failed to allocate.\n");
+ LOGP(DLCTRL, LOGL_ERROR, "Failed to allocate.\n");
return NULL;
}

@@ -333,11 +333,11 @@ struct ctrl_cmd *ctrl_cmd_parse(void *ctx, struct msgb *msg)
if (!var) {
cmd->type = CTRL_TYPE_ERROR;
cmd->reply = "GET incomplete";
- LOGP(DCTRL, LOGL_NOTICE, "GET Command incomplete\n");
+ LOGP(DLCTRL, LOGL_NOTICE, "GET Command incomplete\n");
goto err;
}
cmd->variable = talloc_strdup(cmd, var);
- LOGP(DCTRL, LOGL_DEBUG, "Command: GET %s\n", cmd->variable);
+ LOGP(DLCTRL, LOGL_DEBUG, "Command: GET %s\n", cmd->variable);
break;
case CTRL_TYPE_SET:
var = strtok_r(NULL, " ", &saveptr);
@@ -345,14 +345,14 @@ struct ctrl_cmd *ctrl_cmd_parse(void *ctx, struct msgb *msg)
if (!var || !val) {
cmd->type = CTRL_TYPE_ERROR;
cmd->reply = "SET incomplete";
- LOGP(DCTRL, LOGL_NOTICE, "SET Command incomplete\n");
+ LOGP(DLCTRL, LOGL_NOTICE, "SET Command incomplete\n");
goto err;
}
cmd->variable = talloc_strdup(cmd, var);
cmd->value = talloc_strdup(cmd, val);
if (!cmd->variable || !cmd->value)
goto oom;
- LOGP(DCTRL, LOGL_DEBUG, "Command: SET %s = %s\n", cmd->variable, cmd->value);
+ LOGP(DLCTRL, LOGL_DEBUG, "Command: SET %s = %s\n", cmd->variable, cmd->value);
break;
case CTRL_TYPE_GET_REPLY:
case CTRL_TYPE_SET_REPLY:
@@ -362,14 +362,14 @@ struct ctrl_cmd *ctrl_cmd_parse(void *ctx, struct msgb *msg)
if (!var || !val) {
cmd->type = CTRL_TYPE_ERROR;
cmd->reply = "Trap/Reply incomplete";
- LOGP(DCTRL, LOGL_NOTICE, "Trap/Reply incomplete\n");
+ LOGP(DLCTRL, LOGL_NOTICE, "Trap/Reply incomplete\n");
goto err;
}
cmd->variable = talloc_strdup(cmd, var);
cmd->reply = talloc_strdup(cmd, val);
if (!cmd->variable || !cmd->reply)
goto oom;
- LOGP(DCTRL, LOGL_DEBUG, "Command: TRAP/REPLY %s: %s\n", cmd->variable, cmd->reply);
+ LOGP(DLCTRL, LOGL_DEBUG, "Command: TRAP/REPLY %s: %s\n", cmd->variable, cmd->reply);
break;
case CTRL_TYPE_ERROR:
var = strtok_r(NULL, "\0", &saveptr);
@@ -380,7 +380,7 @@ struct ctrl_cmd *ctrl_cmd_parse(void *ctx, struct msgb *msg)
cmd->reply = talloc_strdup(cmd, var);
if (!cmd->reply)
goto oom;
- LOGP(DCTRL, LOGL_DEBUG, "Command: ERROR %s\n", cmd->reply);
+ LOGP(DLCTRL, LOGL_DEBUG, "Command: ERROR %s\n", cmd->reply);
break;
case CTRL_TYPE_UNKNOWN:
default:
@@ -420,7 +420,7 @@ struct msgb *ctrl_cmd_make(struct ctrl_cmd *cmd)

tmp = talloc_asprintf(cmd, "%s %s %s", type, cmd->id, cmd->variable);
if (!tmp) {
- LOGP(DCTRL, LOGL_ERROR, "Failed to allocate cmd.\n");
+ LOGP(DLCTRL, LOGL_ERROR, "Failed to allocate cmd.\n");
goto err;
}

@@ -435,7 +435,7 @@ struct msgb *ctrl_cmd_make(struct ctrl_cmd *cmd)
tmp = talloc_asprintf(cmd, "%s %s %s %s", type, cmd->id, cmd->variable,
cmd->value);
if (!tmp) {
- LOGP(DCTRL, LOGL_ERROR, "Failed to allocate cmd.\n");
+ LOGP(DLCTRL, LOGL_ERROR, "Failed to allocate cmd.\n");
goto err;
}

@@ -452,7 +452,7 @@ struct msgb *ctrl_cmd_make(struct ctrl_cmd *cmd)
tmp = talloc_asprintf(cmd, "%s %s %s %s", type, cmd->id, cmd->variable,
cmd->reply);
if (!tmp) {
- LOGP(DCTRL, LOGL_ERROR, "Failed to allocate cmd.\n");
+ LOGP(DLCTRL, LOGL_ERROR, "Failed to allocate cmd.\n");
goto err;
}

@@ -467,7 +467,7 @@ struct msgb *ctrl_cmd_make(struct ctrl_cmd *cmd)
tmp = talloc_asprintf(cmd, "%s %s %s", type, cmd->id,
cmd->reply);
if (!tmp) {
- LOGP(DCTRL, LOGL_ERROR, "Failed to allocate cmd.\n");
+ LOGP(DLCTRL, LOGL_ERROR, "Failed to allocate cmd.\n");
goto err;
}

@@ -476,7 +476,7 @@ struct msgb *ctrl_cmd_make(struct ctrl_cmd *cmd)
talloc_free(tmp);
break;
default:
- LOGP(DCTRL, LOGL_NOTICE, "Unknown command type %i\n", cmd->type);
+ LOGP(DLCTRL, LOGL_NOTICE, "Unknown command type %i\n", cmd->type);
goto err;
break;
}
diff --git a/src/ctrl/control_if.c b/src/ctrl/control_if.c
index a208071..007e0fe 100644
--- a/src/ctrl/control_if.c
+++ b/src/ctrl/control_if.c
@@ -77,7 +77,7 @@ int ctrl_cmd_send(struct osmo_wqueue *queue, struct ctrl_cmd *cmd)

msg = ctrl_cmd_make(cmd);
if (!msg) {
- LOGP(DCTRL, LOGL_ERROR, "Could not generate msg\n");
+ LOGP(DLCTRL, LOGL_ERROR, "Could not generate msg\n");
return -1;
}

@@ -86,7 +86,7 @@ int ctrl_cmd_send(struct osmo_wqueue *queue, struct ctrl_cmd *cmd)

ret = osmo_wqueue_enqueue(queue, msg);
if (ret != 0) {
- LOGP(DCTRL, LOGL_ERROR, "Failed to enqueue the command.\n");
+ LOGP(DLCTRL, LOGL_ERROR, "Failed to enqueue the command.\n");
msgb_free(msg);
}
return ret;
@@ -136,27 +136,27 @@ static int handle_control_read(struct osmo_fd * bfd)
if (ret == -EAGAIN)
return 0;
if (ret == 0)
- LOGP(DCTRL, LOGL_INFO, "The control connection was closed\n");
+ LOGP(DLCTRL, LOGL_INFO, "The control connection was closed\n");
else
- LOGP(DCTRL, LOGL_ERROR, "Failed to parse ip access message: %d\n", ret);
+ LOGP(DLCTRL, LOGL_ERROR, "Failed to parse ip access message: %d\n", ret);

goto err;
}

if (msg->len < sizeof(*iph) + sizeof(*iph_ext)) {
- LOGP(DCTRL, LOGL_ERROR, "The message is too short.\n");
+ LOGP(DLCTRL, LOGL_ERROR, "The message is too short.\n");
goto err;
}

iph = (struct ipaccess_head *) msg->data;
if (iph->proto != IPAC_PROTO_OSMO) {
- LOGP(DCTRL, LOGL_ERROR, "Protocol mismatch. We got 0x%x\n", iph->proto);
+ LOGP(DLCTRL, LOGL_ERROR, "Protocol mismatch. We got 0x%x\n", iph->proto);
goto err;
}

iph_ext = (struct ipaccess_head_ext *) iph->data;
if (iph_ext->proto != IPAC_PROTO_EXT_CTRL) {
- LOGP(DCTRL, LOGL_ERROR, "Extended protocol mismatch. We got 0x%x\n", iph_ext->proto);
+ LOGP(DLCTRL, LOGL_ERROR, "Extended protocol mismatch. We got 0x%x\n", iph_ext->proto);
goto err;
}

@@ -174,7 +174,7 @@ static int handle_control_read(struct osmo_fd * bfd)
cmd = talloc_zero(ccon, struct ctrl_cmd);
if (!cmd)
goto err;
- LOGP(DCTRL, LOGL_ERROR, "Command parser error.\n");
+ LOGP(DLCTRL, LOGL_ERROR, "Command parser error.\n");
cmd->type = CTRL_TYPE_ERROR;
cmd->id = "err";
cmd->reply = "Command parser error.";
@@ -197,7 +197,7 @@ static int control_write_cb(struct osmo_fd *bfd, struct msgb *msg)

rc = write(bfd->fd, msg->data, msg->len);
if (rc != msg->len)
- LOGP(DCTRL, LOGL_ERROR, "Failed to write message to the control connection.\n");
+ LOGP(DLCTRL, LOGL_ERROR, "Failed to write message to the control connection.\n");

return rc;
}
@@ -232,19 +232,19 @@ static int listen_fd_cb(struct osmo_fd *listen_bfd, unsigned int what)
perror("accept");
return fd;
}
- LOGP(DCTRL, LOGL_INFO, "accept()ed new control connection from %s\n",
+ LOGP(DLCTRL, LOGL_INFO, "accept()ed new control connection from %s\n",
inet_ntoa(sa.sin_addr));

on = 1;
ret = setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on));
if (ret != 0) {
- LOGP(DCTRL, LOGL_ERROR, "Failed to set TCP_NODELAY: %s\n", strerror(errno));
+ LOGP(DLCTRL, LOGL_ERROR, "Failed to set TCP_NODELAY: %s\n", strerror(errno));
close(fd);
return ret;
}
ccon = ctrl_connection_alloc(listen_bfd->data);
if (!ccon) {
- LOGP(DCTRL, LOGL_ERROR, "Failed to allocate.\n");
+ LOGP(DLCTRL, LOGL_ERROR, "Failed to allocate.\n");
close(fd);
return -1;
}
@@ -258,7 +258,7 @@ static int listen_fd_cb(struct osmo_fd *listen_bfd, unsigned int what)

ret = osmo_fd_register(&ccon->write_queue.bfd);
if (ret < 0) {
- LOGP(DCTRL, LOGL_ERROR, "Could not register FD.\n");
+ LOGP(DLCTRL, LOGL_ERROR, "Could not register FD.\n");
close(ccon->write_queue.bfd.fd);
talloc_free(ccon);
}
diff --git a/src/logging.c b/src/logging.c
index 2e3a80a..f9d789d 100644
--- a/src/logging.c
+++ b/src/logging.c
@@ -106,6 +106,11 @@ static const struct log_info_cat internal_cat[OSMO_NUM_DLIB] = {
.enabled = 1, .loglevel = LOGL_NOTICE,
.color = "\033[1;38m",
},
+ [INT2IDX(DLCTRL)] = {
+ .name = "DLCTRL",
+ .description = "Control Interface",
+ .enabled = 1, .loglevel = LOGL_NOTICE,
+ },
};

/*! \brief descriptive string for each log level */
--
2.1.0.rc1
Harald Welte
2014-08-20 18:15:10 UTC
Permalink
Now we actually build the recently-imported libctrl
---
Makefile.am | 2 +-
configure.ac | 1 +
src/ctrl/Makefile.am | 15 ++++++++++-----
3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 717d3db..b7bad41 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,7 +1,7 @@
ACLOCAL_AMFLAGS = -I m4

AM_CPPFLAGS = $(all_includes) -I$(top_srcdir)/include
-SUBDIRS = include src src/vty src/codec src/gsm src/gb tests utils
+SUBDIRS = include src src/vty src/codec src/gsm src/gb src/ctrl tests utils

pkgconfigdir = $(libdir)/pkgconfig
pkgconfig_DATA = libosmocore.pc libosmocodec.pc libosmovty.pc libosmogsm.pc \
diff --git a/configure.ac b/configure.ac
index eaaab50..a902eb5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -192,6 +192,7 @@ AC_OUTPUT(
src/codec/Makefile
src/gsm/Makefile
src/gb/Makefile
+ src/ctrl/Makefile
tests/Makefile
utils/Makefile
Doxyfile.core
diff --git a/src/ctrl/Makefile.am b/src/ctrl/Makefile.am
index 4f039c8..29a8ee2 100644
--- a/src/ctrl/Makefile.am
+++ b/src/ctrl/Makefile.am
@@ -1,7 +1,12 @@
-AM_CPPFLAGS = $(all_includes) -I$(top_srcdir)/include -I$(top_builddir)
-AM_CFLAGS=-Wall $(LIBOSMOCORE_CFLAGS) $(LIBOSMOVTY_CFLAGS) $(LIBOSMOABIS_CFLAGS) $(COVERAGE_CFLAGS)
-AM_LDFLAGS = $(LIBOSMOCORE_LIBS) $(LIBOSMOABIS_LIBS) $(COVERAGE_LDFLAGS)
+# This is _NOT_ the library release version, it's an API version.
+# Please read Chapter 6 "Library interface versions" of the libtool documentation before making any modification
+LIBVERSION=0:0:0

-noinst_LIBRARIES = libctrl.a
+AM_CFLAGS = -Wall $(all_includes) -I$(top_srcdir)/include

-libctrl_a_SOURCES = control_if.c control_cmd.c
+lib_LTLIBRARIES = libosmoctrl.la
+
+libosmoctrl_la_SOURCES = control_cmd.c control_if.c
+
+libosmoctrl_la_LDFLAGS = $(LTLDFLAGS_OSMOCTRL) -version-info $(LIBVERSION) -no-undefined
+libosmoctrl_la_LIBADD = $(top_builddir)/src/libosmocore.la
--
2.1.0.rc1
Loading...