Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/9650


Change subject: fix dyn TS init: properly identify BTS on OML OPSTART ACK
......................................................................

fix dyn TS init: properly identify BTS on OML OPSTART ACK

Commit "dyn ts, bts_ipaccess_nanobts.c: init PDCH on Chan OPSTART ACK"
bf7099262adf0f27e71a08387747c5cb0d459360 
Icf6e25ff068e8a2600562d52726ead65e864ec02
introduced signal S_NM_OPSTART_ACK and passed the FOM header to identify the BTS
by. But the FOM header's BTS number is zero on each Abis/IP link, and the BTS
and TRX are actually identified by msgb->dst == e1inp_sign_link, member trx. So
the initial implementation associated *all* Channel OPSTART ACKs with BTS 0.

Pass the entire msgb as S_NM_OPSTART_ACK signal argument, implement a
abis_nm_get_ts() to retrieve the proper timeslot and use that during timeslot
init.

Related: OS#3351 OS#3205
Change-Id: I45ce5c24cb62d00f350df1af1be6c11104d74193
---
M include/osmocom/bsc/abis_nm.h
M include/osmocom/bsc/signal.h
M src/osmo-bsc/abis_nm.c
M src/osmo-bsc/bts_ipaccess_nanobts.c
4 files changed, 37 insertions(+), 27 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/50/9650/1

diff --git a/include/osmocom/bsc/abis_nm.h b/include/osmocom/bsc/abis_nm.h
index 9586ded..ef9e4f5 100644
--- a/include/osmocom/bsc/abis_nm.h
+++ b/include/osmocom/bsc/abis_nm.h
@@ -171,4 +171,6 @@
 /* Helper functions for updating attributes */
 int abis_nm_update_max_power_red(struct gsm_bts_trx *trx);

+struct gsm_bts_trx_ts *abis_nm_get_ts(const struct msgb *oml_msg);
+
 #endif /* _NM_H */
diff --git a/include/osmocom/bsc/signal.h b/include/osmocom/bsc/signal.h
index 1dde267..1b351aa 100644
--- a/include/osmocom/bsc/signal.h
+++ b/include/osmocom/bsc/signal.h
@@ -71,7 +71,7 @@
        S_NM_STATECHG_OPER,     /* Operational State changed*/
        S_NM_STATECHG_ADM,      /* Administrative State changed */
        S_NM_OM2K_CONF_RES,     /* OM2K Configuration Result */
-       S_NM_OPSTART_ACK,       /* Received OPSTART ACK, arg is struct 
abis_om_fom_hdr* */
+       S_NM_OPSTART_ACK,       /* Received OPSTART ACK, arg is struct msgb 
*oml_msg */
 };

 /* SS_LCHAN signals */
diff --git a/src/osmo-bsc/abis_nm.c b/src/osmo-bsc/abis_nm.c
index 9dc1f62..cb1cc56 100644
--- a/src/osmo-bsc/abis_nm.c
+++ b/src/osmo-bsc/abis_nm.c
@@ -671,11 +671,36 @@
        return 0;
 }

+/* From a received OML message, determine the matching struct gsm_bts_trx_ts 
instance.
+ * Note that the BTS-TRX-TS numbers received in the FOM header do not 
correspond to the local bts->nr and
+ * bts->trx->nr. Rather, the trx is identified by the e1inp_sign_link* found 
in msg->dst, and the TS is
+ * then obtained by the FOM header's TS number. */
+struct gsm_bts_trx_ts *abis_nm_get_ts(const struct msgb *oml_msg)
+{
+       struct abis_om_fom_hdr *foh = msgb_l3(oml_msg);
+       struct e1inp_sign_link *sign_link = oml_msg->dst;
+       struct gsm_bts_trx *trx = sign_link->trx;
+       uint8_t ts_nr = foh->obj_inst.ts_nr;
+       if (!trx) {
+               LOGP(DNM, LOGL_ERROR, "%s Channel OPSTART ACK for sign_link 
without trx\n",
+                    abis_nm_dump_foh(foh));
+               return NULL;
+       }
+       if (ts_nr >= ARRAY_SIZE(trx->ts)) {
+               LOGP(DNM, LOGL_ERROR, "bts%u-trx%u %s Channel OPSTART ACK for 
non-existent TS\n",
+                    trx->bts->nr, trx->nr, abis_nm_dump_foh(foh));
+               return NULL;
+       }
+       return &trx->ts[ts_nr];
+}
+
 static int abis_nm_rx_opstart_ack(struct msgb *mb)
 {
        struct abis_om_fom_hdr *foh = msgb_l3(mb);
-       DEBUGPFOH(DNM, foh, "Opstart ACK\n");
-       osmo_signal_dispatch(SS_NM, S_NM_OPSTART_ACK, foh);
+       struct e1inp_sign_link *sign_link = mb->dst;
+       struct gsm_bts_trx *trx = sign_link->trx;
+       DEBUGPFOH(DNM, foh, "bts=%u trx=%u Opstart ACK\n", trx->bts->nr, 
trx->nr);
+       osmo_signal_dispatch(SS_NM, S_NM_OPSTART_ACK, mb);
        return 0;
 }

diff --git a/src/osmo-bsc/bts_ipaccess_nanobts.c 
b/src/osmo-bsc/bts_ipaccess_nanobts.c
index 843f264..0e23955 100644
--- a/src/osmo-bsc/bts_ipaccess_nanobts.c
+++ b/src/osmo-bsc/bts_ipaccess_nanobts.c
@@ -298,40 +298,23 @@
        return 0;
 }

-static struct gsm_bts_trx_ts *gsm_bts_trx_ts(struct gsm_network *net,
-                                            int bts_nr, int trx_nr, int ts_nr)
-{
-       struct gsm_bts *bts;
-       struct gsm_bts_trx *trx;
-       bts = gsm_bts_num(net, bts_nr);
-       if (!bts)
-               return NULL;
-       trx = gsm_bts_trx_by_nr(bts, trx_nr);
-       if (!trx)
-               return NULL;
-       if (ts_nr < 0 || ts_nr > ARRAY_SIZE(trx->ts))
-               return NULL;
-       return &trx->ts[ts_nr];
-}
-
-static void nm_rx_opstart_ack_chan(struct abis_om_fom_hdr *foh)
+static void nm_rx_opstart_ack_chan(struct msgb *oml_msg)
 {
        struct gsm_bts_trx_ts *ts;
-       ts = gsm_bts_trx_ts(bsc_gsmnet, foh->obj_inst.bts_nr, 
foh->obj_inst.trx_nr, foh->obj_inst.ts_nr);
-       if (!ts) {
-               LOGP(DNM, LOGL_ERROR, "%s Channel OPSTART ACK for non-existent 
TS\n",
-                    abis_nm_dump_foh(foh));
+       ts = abis_nm_get_ts(oml_msg);
+       if (!ts)
+               /* error already logged in abis_nm_get_ts() */
                return;
-       }
 
        gsm_ts_check_init(ts);
 }

-static void nm_rx_opstart_ack(struct abis_om_fom_hdr *foh)
+static void nm_rx_opstart_ack(struct msgb *oml_msg)
 {
+       struct abis_om_fom_hdr *foh = msgb_l3(oml_msg);
        switch (foh->obj_class) {
        case NM_OC_CHANNEL:
-               nm_rx_opstart_ack_chan(foh);
+               nm_rx_opstart_ack_chan(oml_msg);
                break;
        default:
                break;

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I45ce5c24cb62d00f350df1af1be6c11104d74193
Gerrit-Change-Number: 9650
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr <[email protected]>

Reply via email to