Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/12761 )

Change subject: Introduce generic host config and related helpers
......................................................................

Introduce generic host config and related helpers

Add generic host config struct and related helpers for TCP-based probes
and use them for ctrl probe.

This will be used in follow-up patch for OpenVPN probe as well.

Change-Id: Ie321655a92cdbefbfaa056ac0d583397c83beccb
---
M src/Makefile.am
A src/client.c
A src/client.h
M src/osysmon_ctrl.c
M src/simple_ctrl.c
M src/simple_ctrl.h
6 files changed, 133 insertions(+), 38 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Harald Welte: Looks good to me, approved

Objections:
  Pau Espin Pedrol: I would prefer this is not merged as is



diff --git a/src/Makefile.am b/src/Makefile.am
index d0d6a22..f639023 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -21,7 +21,7 @@
        $(NULL)

 noinst_LTLIBRARIES = libintern.la
-libintern_la_SOURCES = simple_ctrl.c
+libintern_la_SOURCES = simple_ctrl.c client.c
 libintern_la_LIBADD = $(LIBOSMOCORE_LIBS) $(LIBOSMOGSM_LIBS)

 osmo_sysmon_CFLAGS = $(LIBMNL_CFLAGS) $(LIBOSMOVTY_CFLAGS) $(LIBOPING_CFLAGS) 
$(AM_CFLAGS)
@@ -44,6 +44,7 @@

 noinst_HEADERS = \
        osysmon.h \
+       client.h \
        simple_ctrl.h \
        value_node.h \
        $(NULL)
diff --git a/src/client.c b/src/client.c
new file mode 100644
index 0000000..6b37fc6
--- /dev/null
+++ b/src/client.c
@@ -0,0 +1,73 @@
+/* Generic client structure and related helpers */
+
+/* (C) 2018 by Harald Welte <[email protected]>
+ * (C) 2019 by sysmocom - s.f.m.c. GmbH.
+ * All Rights Reserved.
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ *  MA  02110-1301, USA.
+ */
+
+#include <stdbool.h>
+#include <string.h>
+#include <talloc.h>
+
+#include <osmocom/core/utils.h>
+
+#include "client.h"
+
+bool match_config(const struct host_cfg *cfg, const char *match, enum 
match_kind k)
+{
+       bool m_name = (strcmp(match, cfg->name) == 0),
+            m_host = (strcmp(match, cfg->remote_host) == 0);
+
+       switch (k) {
+       case MATCH_NAME:
+               return m_name;
+       case MATCH_HOST:
+               return m_host;
+       case MATCH_EITHER:
+               return m_name | m_host;
+       case MATCH_BOTH:
+               return m_name & m_host;
+       default:
+               return false;
+       }
+
+       return false;
+}
+
+struct host_cfg *host_cfg_alloc(void *ctx, const char *name, const char *host, 
uint16_t port)
+{
+       struct host_cfg *cfg = talloc_zero(ctx, struct host_cfg);
+       if (!cfg)
+               return NULL;
+
+       cfg->name = talloc_strdup(cfg, name);
+       cfg->remote_host = talloc_strdup(cfg, host);
+       cfg->remote_port = port;
+
+       return cfg;
+}
+
+char *make_authority(void *ctx, const struct host_cfg *cfg)
+{
+       if (!cfg->remote_host)
+               return NULL;
+
+       return talloc_asprintf(ctx, "%s:%u", cfg->remote_host, 
cfg->remote_port);
+}
diff --git a/src/client.h b/src/client.h
new file mode 100644
index 0000000..605ddd7
--- /dev/null
+++ b/src/client.h
@@ -0,0 +1,25 @@
+#pragma once
+
+#include <stdint.h>
+#include <stdbool.h>
+
+enum match_kind {
+       MATCH_NAME,
+       MATCH_HOST,
+       MATCH_BOTH,
+       MATCH_EITHER,
+};
+
+/* a client config */
+struct host_cfg {
+       /* name of this client */
+       const char *name;
+       /* remote host/IP */
+       const char *remote_host;
+       /* remote port */
+       uint16_t remote_port;
+};
+
+struct host_cfg *host_cfg_alloc(void *ctx, const char *name, const char *host, 
uint16_t port);
+bool match_config(const struct host_cfg *cfg, const char *match, enum 
match_kind k);
+char *make_authority(void *ctx, const struct host_cfg *cfg);
diff --git a/src/osysmon_ctrl.c b/src/osysmon_ctrl.c
index c2d0725..c54779c 100644
--- a/src/osysmon_ctrl.c
+++ b/src/osysmon_ctrl.c
@@ -26,6 +26,7 @@
 #include <osmocom/vty/vty.h>
 #include <osmocom/vty/command.h>

+#include "client.h"
 #include "osysmon.h"
 #include "simple_ctrl.h"
 #include "value_node.h"
@@ -38,7 +39,7 @@
 struct ctrl_client {
        /* links to osysmon.ctrl_clients */
        struct llist_head list;
-       struct ctrl_cfg cfg;
+       struct host_cfg *cfg;
        struct simple_ctrl_handle *sch;
        /* list of ctrl_client_get_var objects */
        struct llist_head get_vars;
@@ -62,7 +63,7 @@
 {
        struct ctrl_client *cc;
        llist_for_each_entry(cc, &os->ctrl_clients, list) {
-               if (!strcmp(name, cc->cfg.name))
+               if (match_config(cc->cfg, name, MATCH_NAME))
                        return cc;
        }
        return NULL;
@@ -79,9 +80,13 @@
        cc = talloc_zero(os, struct ctrl_client);
        if (!cc)
                return NULL;
-       cc->cfg.name = talloc_strdup(cc, name);
-       cc->cfg.remote_host = talloc_strdup(cc, host);
-       cc->cfg.remote_port = port;
+
+       cc->cfg = host_cfg_alloc(cc, name, host, port);
+       if (!cc->cfg) {
+               talloc_free(cc);
+               return NULL;
+       }
+
        INIT_LLIST_HEAD(&cc->get_vars);
        llist_add_tail(&cc->list, &os->ctrl_clients);
        /* FIXME */
@@ -156,10 +161,10 @@
        struct ctrl_client *cc;
        cc = ctrl_client_find(g_oss, argv[0]);
        if (cc) {
-               if ((strcmp(cc->cfg.remote_host, argv[1])) ||
-                   (cc->cfg.remote_port != atoi(argv[2]))) {
+               if ((strcmp(cc->cfg->remote_host, argv[1])) ||
+                   (cc->cfg->remote_port != atoi(argv[2]))) {
                        vty_out(vty, "Client %s has different IP/port, please 
remove it first%s",
-                               cc->cfg.name, VTY_NEWLINE);
+                               cc->cfg->name, VTY_NEWLINE);
                        return CMD_WARNING;
                }
        } else
@@ -215,8 +220,8 @@
 static void write_one_ctrl_client(struct vty *vty, struct ctrl_client *cc)
 {
        struct ctrl_client_get_var *ccgv;
-       vty_out(vty, "ctrl-client %s %s %u%s", cc->cfg.name,
-               cc->cfg.remote_host, cc->cfg.remote_port, VTY_NEWLINE);
+       vty_out(vty, "ctrl-client %s %s %u%s", cc->cfg->name,
+               cc->cfg->remote_host, cc->cfg->remote_port, VTY_NEWLINE);
        llist_for_each_entry(ccgv, &cc->get_vars, list) {
                vty_out(vty, " get-variable %s%s", ccgv->cfg.name, VTY_NEWLINE);
                if (ccgv->cfg.display_name)
@@ -259,11 +264,11 @@
 static int ctrl_client_poll(struct ctrl_client *cc, struct value_node *parent)
 {
        struct ctrl_client_get_var *ccgv;
-       struct value_node *vn_clnt = value_node_add(parent, cc->cfg.name, NULL);
+       struct value_node *vn_clnt = value_node_add(parent, cc->cfg->name, 
NULL);

        /* attempt to re-connect */
        if (!cc->sch)
-               cc->sch = simple_ctrl_open(cc, cc->cfg.remote_host, 
cc->cfg.remote_port, 1000);
+               cc->sch = simple_ctrl_open(cc, cc->cfg->remote_host, 
cc->cfg->remote_port, 1000);
        /* abort, if that failed */
        if (!cc->sch) {
                return -1;
diff --git a/src/simple_ctrl.c b/src/simple_ctrl.c
index b56fc83..883e92f 100644
--- a/src/simple_ctrl.c
+++ b/src/simple_ctrl.c
@@ -35,10 +35,11 @@
 #include <osmocom/gsm/ipa.h>
 #include <osmocom/gsm/protocol/ipaccess.h>

+#include "client.h"
 #include "simple_ctrl.h"

-#define CTRL_ERR(cfg, fmt, args...) \
-       fprintf(stderr, "CTRL %s:%u error: " fmt, cfg.remote_host, 
cfg.remote_port, ##args)
+#define CTRL_ERR(sch, fmt, args...) \
+       fprintf(stderr, "CTRL %s error: " fmt, make_authority(sch, &sch->cfg), 
##args)

 /***********************************************************************
  * blocking I/O with timeout helpers
@@ -101,7 +102,7 @@
        int fd;
        uint32_t next_id;
        uint32_t tout_msec;
-       struct ctrl_cfg cfg;
+       struct host_cfg cfg;
 };

 struct simple_ctrl_handle *simple_ctrl_open(void *ctx, const char *host, 
uint16_t dport,
@@ -122,7 +123,7 @@
        fd = osmo_sock_init(AF_INET, SOCK_STREAM, IPPROTO_TCP, host, dport,
                            OSMO_SOCK_F_CONNECT | OSMO_SOCK_F_NONBLOCK);
        if (fd < 0) {
-               CTRL_ERR(sch->cfg, "connecting socket: %s\n", strerror(errno));
+               CTRL_ERR(sch, "connecting socket: %s\n", strerror(errno));
                return NULL;
        }

@@ -131,17 +132,17 @@
        FD_SET(fd, &writeset);
        rc = select(fd+1, NULL, &writeset, NULL, timeval_from_msec(tout_msec));
        if (rc == 0) {
-               CTRL_ERR(sch->cfg, "timeout during connect\n");
+               CTRL_ERR(sch, "timeout during connect\n");
                goto out_close;
        }
        if (rc < 0) {
-               CTRL_ERR(sch->cfg, "error connecting socket: %s\n", 
strerror(errno));
+               CTRL_ERR(sch, "error connecting socket: %s\n", strerror(errno));
                goto out_close;
        }

        /* set FD blocking again */
        if (ioctl(fd, FIONBIO, (unsigned char *)&off) < 0) {
-               CTRL_ERR(sch->cfg, "cannot set socket blocking: %s\n", 
strerror(errno));
+               CTRL_ERR(sch, "cannot set socket blocking: %s\n", 
strerror(errno));
                goto out_close;
        }

@@ -173,10 +174,10 @@

        rc = read_timeout(sch->fd, (uint8_t *) &hh, sizeof(hh), sch->tout_msec);
        if (rc < 0) {
-               CTRL_ERR(sch->cfg, "read(): %d\n", rc);
+               CTRL_ERR(sch, "read(): %d\n", rc);
                return NULL;
        } else if (rc < sizeof(hh)) {
-               CTRL_ERR(sch->cfg, "short read (header)\n");
+               CTRL_ERR(sch, "short read (header)\n");
                return NULL;
        }
        len = ntohs(hh.len);
@@ -190,7 +191,7 @@
        resp->l2h = resp->tail;
        rc = read(sch->fd, resp->l2h, len);
        if (rc < len) {
-               CTRL_ERR(sch->cfg, "short read (payload)\n");
+               CTRL_ERR(sch, "short read (payload)\n");
                msgb_free(resp);
                return NULL;
        }
@@ -222,7 +223,7 @@
                        *tmp = '\0';
                        return resp;
                } else {
-                       CTRL_ERR(sch->cfg, "unknown IPA message %s\n", 
msgb_hexdump(resp));
+                       CTRL_ERR(sch, "unknown IPA message %s\n", 
msgb_hexdump(resp));
                        msgb_free(resp);
                }
        }
@@ -237,10 +238,10 @@

        rc = write_timeout(sch->fd, msg->data, msg->len, sch->tout_msec);
        if (rc < 0) {
-               CTRL_ERR(sch->cfg, "write(): %d\n", rc);
+               CTRL_ERR(sch, "write(): %d\n", rc);
                return rc;
        } else if (rc < msg->len) {
-               CTRL_ERR(sch->cfg, "short write\n");
+               CTRL_ERR(sch, "short write\n");
                msgb_free(msg);
                return -1;
        } else {
@@ -291,7 +292,7 @@
                free(rx_var);
                free(rx_val);
        } else {
-               CTRL_ERR(sch->cfg, "GET(%s) results in '%s'\n", var, (char 
*)msgb_l2(resp));
+               CTRL_ERR(sch, "GET(%s) results in '%s'\n", var, (char 
*)msgb_l2(resp));
        }

        msgb_free(resp);
@@ -329,7 +330,7 @@
                        free(rx_var);
                }
        } else {
-               CTRL_ERR(sch->cfg, "SET(%s=%s) results in '%s'\n", var, val, 
(char *) msgb_l2(resp));
+               CTRL_ERR(sch, "SET(%s=%s) results in '%s'\n", var, val, (char 
*) msgb_l2(resp));
        }

        msgb_free(resp);
diff --git a/src/simple_ctrl.h b/src/simple_ctrl.h
index f35eab1..81a759d 100644
--- a/src/simple_ctrl.h
+++ b/src/simple_ctrl.h
@@ -2,16 +2,6 @@

 #include <stdint.h>

-/* a CTRL client config */
-struct ctrl_cfg {
-       /* name of this CTRL client */
-       const char *name;
-       /* remote host/IP */
-       const char *remote_host;
-       /* remote CTRL port */
-       uint16_t remote_port;
-};
-
 struct simple_ctrl_handle;

 struct simple_ctrl_handle *simple_ctrl_open(void *ctx, const char *host, 
uint16_t dport,

--
To view, visit https://gerrit.osmocom.org/12761
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-sysmon
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie321655a92cdbefbfaa056ac0d583397c83beccb
Gerrit-Change-Number: 12761
Gerrit-PatchSet: 6
Gerrit-Owner: Max <[email protected]>
Gerrit-Reviewer: Harald Welte <[email protected]>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Max <[email protected]>
Gerrit-Reviewer: Pau Espin Pedrol <[email protected]>

Reply via email to