laforge has submitted this change. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/31618 )

Change subject: pcu_sock: handle multiple BTSs with one BSC co-located PCU (in 
theory)
......................................................................

pcu_sock: handle multiple BTSs with one BSC co-located PCU (in theory)

The current PCU implementation has never been tested with multiple BTS
attached to it. This is due to the fact that it has been used
exclusively in an BTS co-located setup where naturally only one BTS is
present. The PCU sock protocol supports multiple BTSs in theory and we
should handle this correctly.

Related: OS#5198
Change-Id: I0b42c2c130106f6ffca2dd08d079e1a7bda41f0b
---
M include/osmocom/bsc/bts.h
M include/osmocom/bsc/gsm_data.h
M include/osmocom/bsc/pcu_if.h
M src/osmo-bsc/bsc_vty.c
M src/osmo-bsc/bts_vty.c
M src/osmo-bsc/pcu_sock.c
M src/osmo-bsc/timeslot_fsm.c
7 files changed, 143 insertions(+), 98 deletions(-)

Approvals:
  Jenkins Builder: Verified
  pespin: Looks good to me, but someone else must approve
  laforge: Looks good to me, approved




diff --git a/include/osmocom/bsc/bts.h b/include/osmocom/bsc/bts.h
index ccca186..20ed089 100644
--- a/include/osmocom/bsc/bts.h
+++ b/include/osmocom/bsc/bts.h
@@ -600,10 +600,6 @@
        /* osmux config: */
        enum osmux_usage use_osmux;

-       /* PCU socket state */
-       char *pcu_sock_path;
-       struct pcu_sock_state *pcu_state;
-
        struct rate_ctr_group *bts_ctrs;
        struct osmo_stat_item_group *bts_statg;

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index 25e634a..7779449 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -1003,6 +1003,10 @@

        struct chan_counts chan_counts;
        struct all_allocated all_allocated;
+
+       /* PCU socket state */
+       char *pcu_sock_path;
+       struct pcu_sock_state *pcu_state;
 };

 struct gsm_audio_support {
diff --git a/include/osmocom/bsc/pcu_if.h b/include/osmocom/bsc/pcu_if.h
index 2a2afa8..34349dc 100644
--- a/include/osmocom/bsc/pcu_if.h
+++ b/include/osmocom/bsc/pcu_if.h
@@ -8,14 +8,14 @@
 #define PCUIF_HDR_SIZE (sizeof(struct gsm_pcu_if) - sizeof(((struct gsm_pcu_if 
*)0)->u))

 struct pcu_sock_state {
-       struct gsm_network *net;
+       struct gsm_network *net;        /* backpointer to GSM network */
        struct osmo_fd listen_bfd;      /* fd for listen socket */
        struct osmo_fd conn_bfd;        /* fd for connection to lcr */
        struct llist_head upqueue;      /* queue for sending messages */
 };

 /* Check if BTS has a PCU connection */
-bool pcu_connected(struct gsm_bts *bts);
+bool pcu_connected(struct gsm_network *net);

 /* PCU relevant information has changed; Inform PCU (if connected) */
 void pcu_info_update(struct gsm_bts *bts);
@@ -32,9 +32,9 @@
 int pcu_tx_imm_ass_sent(struct gsm_bts *bts, uint32_t tlli);

 /* Open connection to PCU */
-int pcu_sock_init(const char *path, struct gsm_bts *bts);
+int pcu_sock_init(struct gsm_network *net);

 /* Close connection to PCU */
-void pcu_sock_exit(struct gsm_bts *bts);
+void pcu_sock_exit(struct gsm_network *net);

 #endif /* _PCU_IF_H */
diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c
index 467301b..f1bd7c4 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -67,6 +67,7 @@
 #include <osmocom/bsc/bssmap_reset.h>
 #include <osmocom/bsc/bsc_msc_data.h>
 #include <osmocom/bsc/lchan.h>
+#include <osmocom/bsc/pcu_if.h>

 #include <inttypes.h>

@@ -218,6 +219,9 @@
                vty_out(vty, "  Last RF Lock Command: %s%s",
                        net->rf_ctrl->last_rf_lock_ctrl_command,
                        VTY_NEWLINE);
+
+       if (net->pcu_sock_path)
+               vty_out(vty, "  PCU Socket Path: %s%s", net->pcu_sock_path, 
VTY_NEWLINE);
 }

 DEFUN(bsc_show_net, bsc_show_net_cmd, "show network",
@@ -406,6 +410,9 @@
                vty_out(vty, "%s", VTY_NEWLINE);
        }

+       if (gsmnet->pcu_sock_path)
+               vty_out(vty, "  pcu-socket %s%s", gsmnet->pcu_sock_path, 
VTY_NEWLINE);
+
        neighbor_ident_vty_write_network(vty, " ");
        mgcp_client_pool_config_write(vty, " ");

@@ -2460,6 +2467,42 @@
        return CMD_SUCCESS;
 }

+DEFUN_ATTR(cfg_net_pcu_sock,
+          cfg_net_pcu_sock_cmd,
+          "pcu-socket PATH",
+          "PCU Socket Path for using OsmoPCU co-located with BSC\n"
+          "Path in the file system for the unix-domain PCU socket\n",
+          CMD_ATTR_IMMEDIATE)
+{
+       struct gsm_network *net = gsmnet_from_vty(vty);
+       int rc;
+
+       osmo_talloc_replace_string(net, &net->pcu_sock_path, argv[0]);
+       pcu_sock_exit(net);
+       rc = pcu_sock_init(net);
+       if (rc < 0) {
+               vty_out(vty, "%% Error creating PCU socket `%s'%s",
+                       net->pcu_sock_path, VTY_NEWLINE);
+               return CMD_WARNING;
+       }
+
+       return CMD_SUCCESS;
+}
+
+DEFUN_ATTR(cfg_net_no_pcu_sock,
+          cfg_net_no_pcu_sock_cmd,
+          "no pcu-socket",
+          NO_STR "Disable BSC co-located PCU\n",
+          CMD_ATTR_IMMEDIATE)
+{
+       struct gsm_network *net = gsmnet_from_vty(vty);
+
+       pcu_sock_exit(net);
+       talloc_free(net->pcu_sock_path);
+       net->pcu_sock_path = NULL;
+       return CMD_SUCCESS;
+}
+
 static struct bsc_msc_data *bsc_msc_data(struct vty *vty)
 {
        return vty->index;
@@ -3534,6 +3577,8 @@
        install_element(GSMNET_NODE, &cfg_net_meas_feed_scenario_cmd);
        install_element(GSMNET_NODE, &cfg_net_timer_cmd);
        install_element(GSMNET_NODE, &cfg_net_allow_unusable_timeslots_cmd);
+       install_element(GSMNET_NODE, &cfg_net_pcu_sock_cmd);
+       install_element(GSMNET_NODE, &cfg_net_no_pcu_sock_cmd);

        /* Timer configuration commands (generic osmo_tdef API) */
        osmo_tdef_vty_groups_init(GSMNET_NODE, bsc_tdef_group);
diff --git a/src/osmo-bsc/bts_vty.c b/src/osmo-bsc/bts_vty.c
index 08166ca..e02c2d5 100644
--- a/src/osmo-bsc/bts_vty.c
+++ b/src/osmo-bsc/bts_vty.c
@@ -2191,28 +2191,6 @@
        return CMD_SUCCESS;
 }

-DEFUN_ATTR(cfg_bts_pcu_sock,
-          cfg_bts_pcu_sock_cmd,
-          "pcu-socket PATH",
-          "PCU Socket Path for using OsmoPCU co-located with BSC (legacy 
BTS)\n"
-          "Path in the file system for the unix-domain PCU socket\n",
-          CMD_ATTR_IMMEDIATE)
-{
-       struct gsm_bts *bts = vty->index;
-       int rc;
-
-       osmo_talloc_replace_string(bts, &bts->pcu_sock_path, argv[0]);
-       pcu_sock_exit(bts);
-       rc = pcu_sock_init(bts->pcu_sock_path, bts);
-       if (rc < 0) {
-               vty_out(vty, "%% Error creating PCU socket `%s' for BTS %u%s",
-                       bts->pcu_sock_path, bts->nr, VTY_NEWLINE);
-               return CMD_WARNING;
-       }
-
-       return CMD_SUCCESS;
-}
-
 DEFUN_ATTR(cfg_bts_acc_rotate,
           cfg_bts_acc_rotate_cmd,
           "access-control-class-rotate <0-10>",
@@ -3979,8 +3957,6 @@
                bts->early_classmark_allowed_3g && 
!bts->early_classmark_allowed ?
                " (forbidden by 2G bit)" : "",
                VTY_NEWLINE);
-       if (bts->pcu_sock_path)
-               vty_out(vty, "  PCU Socket Path: %s%s", bts->pcu_sock_path, 
VTY_NEWLINE);
        if (is_ipaccess_bts(bts))
                vty_out(vty, "  Unit ID: %u/%u/0, OML Stream ID 0x%02x%s",
                        bts->ip_access.site_id, bts->ip_access.bts_id,
@@ -4618,8 +4594,6 @@
                        vty_out(vty, "  depends-on-bts %d%s", bts_nr, 
VTY_NEWLINE);
                }
        }
-       if (bts->pcu_sock_path)
-               vty_out(vty, "  pcu-socket %s%s", bts->pcu_sock_path, 
VTY_NEWLINE);

        ho_vty_write_bts(vty, bts);

@@ -4857,7 +4831,6 @@
        install_element(BTS_NODE, &cfg_bts_osmux_cmd);
        install_element(BTS_NODE, &cfg_bts_mgw_pool_target_cmd);
        install_element(BTS_NODE, &cfg_bts_no_mgw_pool_target_cmd);
-       install_element(BTS_NODE, &cfg_bts_pcu_sock_cmd);
        install_element(BTS_NODE, &cfg_bts_acc_rotate_cmd);
        install_element(BTS_NODE, &cfg_bts_acc_rotate_quantum_cmd);
        install_element(BTS_NODE, &cfg_bts_acc_ramping_cmd);
diff --git a/src/osmo-bsc/pcu_sock.c b/src/osmo-bsc/pcu_sock.c
index 165fca4..dba502f 100644
--- a/src/osmo-bsc/pcu_sock.c
+++ b/src/osmo-bsc/pcu_sock.c
@@ -46,7 +46,7 @@
 #include <osmocom/bsc/bts_sm.h>
 #include <osmocom/bsc/timeslot_fsm.h>

-static int pcu_sock_send(struct gsm_bts *bts, struct msgb *msg);
+static int pcu_sock_send(struct gsm_network *net, struct msgb *msg);

 static const char *sapi_string[] = {
        [PCU_IF_SAPI_RACH] =    "RACH",
@@ -59,13 +59,9 @@
        [PCU_IF_SAPI_PCH_DT] =  "PCH_DT",
 };
 
-bool pcu_connected(struct gsm_bts *bts)
+bool pcu_connected(struct gsm_network *net)
 {
-       struct pcu_sock_state *state = bts->pcu_state;
-
-       /* BSC co-located PCU is only supported for Ericsson RBS */
-       if (!is_ericsson_bts(bts))
-               return false;
+       struct pcu_sock_state *state = net->pcu_state;

        if (!state)
                return false;
@@ -305,7 +301,7 @@
                info_ind_fill_trx(&info_ind->trx[trx->nr], trx);
        }

-       return pcu_sock_send(bts, msg);
+       return pcu_sock_send(bts->network, msg);
 }

 static int pcu_tx_e1_ccu_ind(struct gsm_bts *bts)
@@ -348,7 +344,7 @@
                        e1_ccu_ind->e1_ts_ss = ts->e1_link.e1_ts_ss;

                        LOG_TRX(trx, DPCU, LOGL_INFO, "Sending E1 CCU info for 
TS %d\n", e1_ccu_ind->ts_nr);
-                       rc = pcu_sock_send(bts, msg);
+                       rc = pcu_sock_send(bts->network, msg);
                        if (rc < 0)
                                return -EINVAL;
                }
@@ -360,7 +356,7 @@
 /* Allow test to overwrite it */
 __attribute__((weak)) void pcu_info_update(struct gsm_bts *bts)
 {
-       if (pcu_connected(bts)) {
+       if (pcu_connected(bts->network)) {
                if (bsc_co_located_pcu(bts)) {
                        /* In cases where the CCU is connected via an E1 line, 
we transmit the connection parameters for the
                         * PDCH before we announce the other BTS related 
parameters. */
@@ -403,7 +399,7 @@
                memcpy(data_ind->data, data, len);
        data_ind->len = len;

-       return pcu_sock_send(bts, msg);
+       return pcu_sock_send(bts->network, msg);
 }

 /* Forward rach indication to PCU */
@@ -415,7 +411,7 @@
        struct gsm_pcu_if_rach_ind *rach_ind;

        /* Bail if no PCU is connected */
-       if (!pcu_connected(bts)) {
+       if (!pcu_connected(bts->network)) {
                LOG_BTS(bts, DRSL, LOGL_ERROR, "CHAN RQD(GPRS) but PCU not 
connected!\n");
                return -ENODEV;
        }
@@ -436,7 +432,7 @@
        rach_ind->is_11bit = is_11bit;
        rach_ind->burst_type = burst_type;

-       return pcu_sock_send(bts, msg);
+       return pcu_sock_send(bts->network, msg);
 }

 /* Confirm the sending of an immediate assignment to the pcu */
@@ -457,7 +453,7 @@
        data_cnf_dt->sapi = PCU_IF_SAPI_PCH;
        data_cnf_dt->tlli = tlli;

-       return pcu_sock_send(bts, msg);
+       return pcu_sock_send(bts->network, msg);
 }

 /* we need to decode the raw RR paging message (see PCU code
@@ -693,8 +689,9 @@
        int rc = 0;
        struct gsm_bts *bts;

-       /* FIXME: allow multiple BTS */
-       bts = llist_entry(net->bts_list.next, struct gsm_bts, list);
+       bts = gsm_bts_num(net, pcu_prim->bts_nr);
+       if (!bts)
+               return -EINVAL;

        switch (msg_type) {
        case PCU_IF_MSG_DATA_REQ:
@@ -719,9 +716,9 @@
  * PCU socket interface
  */

-static int pcu_sock_send(struct gsm_bts *bts, struct msgb *msg)
+static int pcu_sock_send(struct gsm_network *net, struct msgb *msg)
 {
-       struct pcu_sock_state *state = bts->pcu_state;
+       struct pcu_sock_state *state = net->pcu_state;
        struct osmo_fd *conn_bfd;
        struct gsm_pcu_if *pcu_prim = (struct gsm_pcu_if *) msg->data;

@@ -746,25 +743,11 @@
        return 0;
 }

-static void pcu_sock_close(struct pcu_sock_state *state)
+static void pdch_deact_bts(struct gsm_bts *bts)
 {
-       struct osmo_fd *bfd = &state->conn_bfd;
-       struct gsm_bts *bts;
        struct gsm_bts_trx *trx;
        int j;

-       /* FIXME: allow multiple BTS */
-       bts = llist_entry(state->net->bts_list.next, struct gsm_bts, list);
-
-       LOGP(DPCU, LOGL_NOTICE, "PCU socket has LOST connection\n");
-
-       osmo_fd_unregister(bfd);
-       close(bfd->fd);
-       bfd->fd = -1;
-
-       /* re-enable the generation of ACCEPT for new connections */
-       osmo_fd_read_enable(&state->listen_bfd);
-
 #if 0
        /* remove si13, ... */
        bts->si_valid &= ~(1 << SYSINFO_TYPE_13);
@@ -783,6 +766,27 @@
                        }
                }
        }
+}
+
+static void pcu_sock_close(struct pcu_sock_state *state)
+{
+       struct osmo_fd *bfd = &state->conn_bfd;
+       struct gsm_bts *bts;
+
+       LOGP(DPCU, LOGL_NOTICE, "PCU socket has LOST connection\n");
+
+       osmo_fd_unregister(bfd);
+       close(bfd->fd);
+       bfd->fd = -1;
+
+       /* re-enable the generation of ACCEPT for new connections */
+       osmo_fd_read_enable(&state->listen_bfd);
+
+       /* Disable all PDCHs on all BTSs that are served by the PCU */
+       llist_for_each_entry(bts, &state->net->bts_list, list) {
+               if (bsc_co_located_pcu(bts))
+                       pdch_deact_bts(bts);
+       }

        /* flush the queue */
        while (!llist_empty(&state->upqueue)) {
@@ -900,6 +904,24 @@
        return rc;
 }

+static void pdch_act_bts(struct gsm_bts *bts)
+{
+       struct gsm_bts_trx *trx;
+       int j;
+
+       /* activate PDCH */
+       llist_for_each_entry(trx, &bts->trx_list, list) {
+               for (j = 0; j < ARRAY_SIZE(trx->ts); j++) {
+                       struct gsm_bts_trx_ts *ts = &trx->ts[j];
+                       /* (See comment in pdch_deact_bts above) */
+                       if (ts->mo.nm_state.operational == NM_OPSTATE_ENABLED
+                           && ts->pchan_on_init == GSM_PCHAN_OSMO_DYN) {
+                               ts_pdch_act(ts);
+                       }
+               }
+       }
+}
+
 /* accept connection coming from PCU */
 static int pcu_sock_accept(struct osmo_fd *bfd, unsigned int flags)
 {
@@ -907,23 +929,18 @@
        struct osmo_fd *conn_bfd = &state->conn_bfd;
        struct sockaddr_un un_addr;
        struct gsm_bts *bts;
-       struct gsm_bts_trx *trx;
-       int j;
        socklen_t len;
        int fd;

-       /* FIXME: allow multiple BTS */
-       bts = llist_entry(state->net->bts_list.next, struct gsm_bts, list);
-
        len = sizeof(un_addr);
        fd = accept(bfd->fd, (struct sockaddr *)&un_addr, &len);
        if (fd < 0) {
-               LOG_BTS(bts, DPCU, LOGL_ERROR, "Failed to accept a new 
connection\n");
+               LOGP(DPCU, LOGL_ERROR, "Failed to accept a new connection\n");
                return -1;
        }

        if (conn_bfd->fd >= 0) {
-               LOG_BTS(bts, DPCU, LOGL_NOTICE, "PCU connects but we already 
have another active connection ?!?\n");
+               LOGP(DPCU, LOGL_NOTICE, "PCU connects but we already have 
another active connection ?!?\n");
                /* We already have one PCU connected, this is all we support */
                osmo_fd_read_disable(&state->listen_bfd);
                close(fd);
@@ -933,31 +950,25 @@
        osmo_fd_setup(conn_bfd, fd, OSMO_FD_READ, pcu_sock_cb, state, 0);

        if (osmo_fd_register(conn_bfd) != 0) {
-               LOG_BTS(bts, DPCU, LOGL_ERROR, "Failed to register new 
connection fd\n");
+               LOGP(DPCU, LOGL_ERROR, "Failed to register new connection 
fd\n");
                close(conn_bfd->fd);
                conn_bfd->fd = -1;
                return -1;
        }

-       LOG_BTS(bts, DPCU, LOGL_NOTICE, "PCU socket connected to external 
PCU\n");
+       LOGP(DPCU, LOGL_NOTICE, "PCU socket connected to external PCU\n");

-       /* activate PDCH */
-       llist_for_each_entry(trx, &bts->trx_list, list) {
-               for (j = 0; j < ARRAY_SIZE(trx->ts); j++) {
-                       struct gsm_bts_trx_ts *ts = &trx->ts[j];
-                       /* (See comment in pcu_sock_close above) */
-                       if (ts->mo.nm_state.operational == NM_OPSTATE_ENABLED
-                           && ts->pchan_on_init == GSM_PCHAN_OSMO_DYN) {
-                               ts_pdch_act(ts);
-                       }
-               }
+       /* Activate all PDCHs on all BTSs that are served by the PCU */
+       llist_for_each_entry(bts, &state->net->bts_list, list) {
+               if (bsc_co_located_pcu(bts))
+                       pdch_act_bts(bts);
        }

        return 0;
 }

 /* Open connection to PCU */
-int pcu_sock_init(const char *path, struct gsm_bts *bts)
+int pcu_sock_init(struct gsm_network *net)
 {
        struct pcu_sock_state *state;
        struct osmo_fd *bfd;
@@ -968,14 +979,14 @@
                return -ENOMEM;

        INIT_LLIST_HEAD(&state->upqueue);
-       state->net = bts->network;
+       state->net = net;
        state->conn_bfd.fd = -1;

        bfd = &state->listen_bfd;

-       rc = osmo_sock_unix_init(SOCK_SEQPACKET, 0, path, OSMO_SOCK_F_BIND);
+       rc = osmo_sock_unix_init(SOCK_SEQPACKET, 0, net->pcu_sock_path, 
OSMO_SOCK_F_BIND);
        if (rc < 0) {
-               LOG_BTS(bts, DPCU, LOGL_ERROR, "Could not create unix socket: 
%s\n",
+               LOGP(DPCU, LOGL_ERROR, "Could not create unix socket: %s\n",
                        strerror(errno));
                talloc_free(state);
                return -1;
@@ -985,23 +996,23 @@

        rc = osmo_fd_register(bfd);
        if (rc < 0) {
-               LOG_BTS(bts, DPCU, LOGL_ERROR, "Could not register listen fd: 
%d\n",
+               LOGP(DPCU, LOGL_ERROR, "Could not register listen fd: %d\n",
                        rc);
                close(bfd->fd);
                talloc_free(state);
                return rc;
        }

-       LOG_BTS(bts, DPCU, LOGL_INFO, "Started listening on PCU socket: %s\n", 
path);
+       LOGP(DPCU, LOGL_INFO, "Started listening on PCU socket: %s\n", 
net->pcu_sock_path);

-       bts->pcu_state = state;
+       net->pcu_state = state;
        return 0;
 }

 /* Close connection to PCU */
-void pcu_sock_exit(struct gsm_bts *bts)
+void pcu_sock_exit(struct gsm_network *net)
 {
-       struct pcu_sock_state *state = bts->pcu_state;
+       struct pcu_sock_state *state = net->pcu_state;
        struct osmo_fd *bfd, *conn_bfd;

        if (!state)
@@ -1014,5 +1025,5 @@
        osmo_fd_unregister(bfd);
        close(bfd->fd);
        talloc_free(state);
-       bts->pcu_state = NULL;
+       net->pcu_state = NULL;
 }
diff --git a/src/osmo-bsc/timeslot_fsm.c b/src/osmo-bsc/timeslot_fsm.c
index 790ad77..9618f92 100644
--- a/src/osmo-bsc/timeslot_fsm.c
+++ b/src/osmo-bsc/timeslot_fsm.c
@@ -341,7 +341,7 @@
                return;
        }

-       if (bsc_co_located_pcu(bts) && !pcu_connected(bts)) {
+       if (bsc_co_located_pcu(bts) && !pcu_connected(bts->network)) {
                LOG_TS(ts, LOGL_DEBUG, "PCU not connected: not activating 
PDCH.\n");
                return;
        }

--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31618
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I0b42c2c130106f6ffca2dd08d079e1a7bda41f0b
Gerrit-Change-Number: 31618
Gerrit-PatchSet: 13
Gerrit-Owner: dexter <pma...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: pespin <pes...@sysmocom.de>
Gerrit-CC: fixeria <vyanits...@sysmocom.de>
Gerrit-CC: neels <nhofm...@sysmocom.de>
Gerrit-MessageType: merged

Reply via email to