pespin has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/41649?usp=email )


Change subject: bts-trx: Convert trx clk socket to iofd
......................................................................

bts-trx: Convert trx clk socket to iofd

Since now the Tx side is driven by the event loop, we can use (and
should) use OSMO_SOCK_F_NONBLOCK on the socket, avoiding potential
blocking of the entire process.

Related: OS#1579
Change-Id: Ia33028a657e7d5dee1a4994b8d6ba33ddea892ec
---
M include/osmo-bts/phy_link.h
M src/osmo-bts-trx/trx_if.c
M src/osmo-bts-trx/trx_vty.c
3 files changed, 49 insertions(+), 26 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/49/41649/1

diff --git a/include/osmo-bts/phy_link.h b/include/osmo-bts/phy_link.h
index 862ed48..719c329 100644
--- a/include/osmo-bts/phy_link.h
+++ b/include/osmo-bts/phy_link.h
@@ -3,6 +3,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 #include <osmocom/core/linuxlist.h>
+#include <osmocom/core/osmo_io.h>

 #include <osmo-bts/scheduler.h>
 #include <osmo-bts/bts_trx.h>
@@ -44,7 +45,7 @@
                        char *remote_ip;
                        uint16_t base_port_local;
                        uint16_t base_port_remote;
-                       struct osmo_fd trx_ofd_clk;
+                       struct osmo_io_fd *trx_clk_iofd;
                        uint32_t clock_advance;
                        uint32_t rts_advance;
                        bool use_legacy_setbsic;
diff --git a/src/osmo-bts-trx/trx_if.c b/src/osmo-bts-trx/trx_if.c
index b6b20e9..91961f4 100644
--- a/src/osmo-bts-trx/trx_if.c
+++ b/src/osmo-bts-trx/trx_if.c
@@ -105,34 +105,35 @@
  */

 /* get clock from clock socket */
-static int trx_clk_read_cb(struct osmo_fd *ofd, unsigned int what)
+static void trx_clk_read_cb(struct osmo_io_fd *iofd, int res, struct msgb *msg)
 {
-       struct phy_link *plink = ofd->data;
+       struct phy_link *plink = osmo_iofd_get_data(iofd);
        struct phy_instance *pinst = phy_instance_by_num(plink, 0);
-       char buf[TRXC_MSG_BUF_SIZE];
-       ssize_t len;
+       char *buf;
        uint32_t fn;

        OSMO_ASSERT(pinst);

-       len = recv(ofd->fd, buf, sizeof(buf) - 1, 0);
-       if (len <= 0) {
-               strerror_r(errno, (char *)buf, sizeof(buf));
+       if (res <= 0) {
+               char errbuf[256];
+               strerror_r(errno, errbuf, sizeof(errbuf));
                LOGPPHI(pinst, DTRX, LOGL_ERROR,
-                       "recv() failed on TRXD with rc=%zd (%s)\n", len, buf);
-               return len;
+                       "recv() failed on TRXD with rc=%d (%s)\n", res, errbuf);
+               goto ret_free_msg;
        }
-       buf[len] = '\0';
+
+       msgb_put_u8(msg, (uint8_t)'\0');
+       buf = (char *)msgb_data(msg);

        if (!!strncmp(buf, "IND CLOCK ", 10)) {
                LOGPPHI(pinst, DTRX, LOGL_NOTICE,
                        "Unknown message on clock port: %s\n", buf);
-               return 0;
+               goto ret_free_msg;
        }

        if (sscanf(buf, "IND CLOCK %u", &fn) != 1) {
                LOGPPHI(pinst, DTRX, LOGL_ERROR, "Unable to parse '%s'\n", buf);
-               return 0;
+               goto ret_free_msg;
        }

        LOGPPHI(pinst, DTRX, LOGL_INFO, "Clock indication: fn=%u\n", fn);
@@ -145,14 +146,20 @@

        if (!plink->u.osmotrx.powered) {
                LOGPPHI(pinst, DTRX, LOGL_NOTICE, "Ignoring CLOCK IND %u, TRX 
not yet powered on\n", fn);
-               return 0;
+               goto ret_free_msg;
        }
        /* inform core TRX clock handling code that a FN has been received */
        trx_sched_clock(pinst->trx->bts, fn);

-       return 0;
+ret_free_msg:
+       msgb_free(msg);
 }

+static void trx_clk_write_cb(struct osmo_io_fd *iofd, int res, struct msgb 
*msg)
+{
+       /* libosmocore before change-id 
I0c071a29e508884bac331ada5e510bbfcf440bbf requires write call-back
+        * even if we don't care about it */
+}

 /*
  * TRX ctrl socket
@@ -1273,6 +1280,11 @@
                return plink->u.osmotrx.base_port_local + (pinst->num << 1) + 
inc;
 }

+static const struct osmo_io_ops trx_clk_ioops = {
+       .read_cb = trx_clk_read_cb,
+       .write_cb = trx_clk_write_cb,
+};
+
 /*! open a TRX interface. creates control + data sockets */
 static int trx_if_open(struct trx_l1h *l1h)
 {
@@ -1339,21 +1351,28 @@
 int bts_model_phy_link_open(struct phy_link *plink)
 {
        struct phy_instance *pinst;
+       char sock_name_buf[OSMO_SOCK_NAME_MAXLEN] = {};
        int rc;

        phy_link_state_set(plink, PHY_LINK_CONNECTING);

        /* open the shared/common clock socket */
-       rc = trx_udp_open(plink, &plink->u.osmotrx.trx_ofd_clk,
-                         plink->u.osmotrx.local_ip,
-                         plink->u.osmotrx.base_port_local,
-                         plink->u.osmotrx.remote_ip,
-                         plink->u.osmotrx.base_port_remote,
-                         trx_clk_read_cb);
+       rc = osmo_sock_init2(AF_UNSPEC, SOCK_DGRAM, IPPROTO_UDP,
+                            plink->u.osmotrx.local_ip,
+                            plink->u.osmotrx.base_port_local,
+                            plink->u.osmotrx.remote_ip,
+                            plink->u.osmotrx.base_port_remote,
+                            OSMO_SOCK_F_BIND | OSMO_SOCK_F_CONNECT | 
OSMO_SOCK_F_NONBLOCK);
        if (rc < 0) {
                phy_link_state_set(plink, PHY_LINK_SHUTDOWN);
                return -1;
        }
+       osmo_sock_get_name_buf(sock_name_buf, OSMO_SOCK_NAME_MAXLEN, rc);
+       plink->u.osmotrx.trx_clk_iofd = osmo_iofd_setup(plink, rc, 
sock_name_buf, OSMO_IO_FD_MODE_READ_WRITE, &trx_clk_ioops, plink);
+       if (!plink->u.osmotrx.trx_clk_iofd)
+               goto cleanup;
+       osmo_iofd_set_alloc_info(plink->u.osmotrx.trx_clk_iofd, 
TRXC_MSG_BUF_SIZE, 0);
+       osmo_iofd_register(plink->u.osmotrx.trx_clk_iofd, -1);

        /* open the individual instances with their ctrl+data sockets */
        llist_for_each_entry(pinst, &plink->instances, list) {
@@ -1373,7 +1392,8 @@
                        pinst->u.osmotrx.hdl = NULL;
                }
        }
-       trx_udp_close(&plink->u.osmotrx.trx_ofd_clk);
+       osmo_iofd_free(plink->u.osmotrx.trx_clk_iofd);
+       plink->u.osmotrx.trx_clk_iofd = NULL;
        return -1;
 }

@@ -1389,7 +1409,8 @@
                }
                trx_phy_inst_close(pinst);
        }
-       trx_udp_close(&plink->u.osmotrx.trx_ofd_clk);
+       osmo_iofd_free(plink->u.osmotrx.trx_clk_iofd);
+       plink->u.osmotrx.trx_clk_iofd = NULL;
        phy_link_state_set(plink, PHY_LINK_SHUTDOWN);
        return 0;
 }
diff --git a/src/osmo-bts-trx/trx_vty.c b/src/osmo-bts-trx/trx_vty.c
index 9056f02..b806dc6 100644
--- a/src/osmo-bts-trx/trx_vty.c
+++ b/src/osmo-bts-trx/trx_vty.c
@@ -62,10 +62,11 @@
        llist_for_each_entry(trx, &g_bts->trx_list, list) {
                struct phy_instance *pinst = trx_phy_instance(trx);
                struct phy_link *plink = pinst->phy_link;
-               char *sname = osmo_sock_get_name(NULL, 
plink->u.osmotrx.trx_ofd_clk.fd);
+               const char *sname = plink->u.osmotrx.trx_clk_iofd ?
+                                   
osmo_iofd_get_name(plink->u.osmotrx.trx_clk_iofd) :
+                                   NULL;
                l1h = pinst->u.osmotrx.hdl;
-               vty_out(vty, "TRX %d %s%s", trx->nr, sname, VTY_NEWLINE);
-               talloc_free(sname);
+               vty_out(vty, "TRX %d %s%s", trx->nr, sname ? sname : "", 
VTY_NEWLINE);
                vty_out(vty, " %s%s",
                        trx_if_powered(l1h) ? "poweron":"poweroff",
                        VTY_NEWLINE);

--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/41649?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: newchange
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ia33028a657e7d5dee1a4994b8d6ba33ddea892ec
Gerrit-Change-Number: 41649
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <[email protected]>

Reply via email to