pespin submitted this change.
nm: Apply BTS/TRX/TS OML Attributes through NM FSMs
This way we have further control on how to handle the SetAttr meessages
received. For instance, NACK them if the NM object FSMs are not at the expected
correct state.
The originating msgs are now kept owned and freed by the OML layer
(oml.c), and the NM FSMs only uses them and create new OML msgb when
answering with ACK/NACK.
Related: OS#5992
Change-Id: Id68868e25bbf96227ab6459fcd3c9181852ed28e
---
M include/osmo-bts/nm_common_fsm.h
M include/osmo-bts/oml.h
M src/common/nm_bb_transc_fsm.c
M src/common/nm_bts_fsm.c
M src/common/nm_bts_sm_fsm.c
M src/common/nm_channel_fsm.c
M src/common/nm_common_fsm.c
M src/common/nm_radio_carrier_fsm.c
M src/common/oml.c
M src/osmo-bts-lc15/oml.c
M src/osmo-bts-oc2g/oml.c
M src/osmo-bts-octphy/l1_oml.c
M src/osmo-bts-omldummy/bts_model.c
M src/osmo-bts-sysmo/oml.c
M src/osmo-bts-trx/l1_if.c
M src/osmo-bts-virtual/bts_model.c
16 files changed, 111 insertions(+), 28 deletions(-)
diff --git a/include/osmo-bts/nm_common_fsm.h b/include/osmo-bts/nm_common_fsm.h
index 1f0accc..2363779 100644
--- a/include/osmo-bts/nm_common_fsm.h
+++ b/include/osmo-bts/nm_common_fsm.h
@@ -30,6 +30,7 @@
/* Common */
enum nm_fsm_events {
NM_EV_SW_ACT,
+ NM_EV_RX_SETATTR, /* data: struct nm_fsm_ev_setattr_data */
NM_EV_SETATTR_ACK, /* data: struct nm_fsm_ev_setattr_data */
NM_EV_SETATTR_NACK, /* data: struct nm_fsm_ev_setattr_data */
NM_EV_OPSTART_ACK,
@@ -50,8 +51,9 @@
extern const struct value_string nm_fsm_event_names[];
struct nm_fsm_ev_setattr_data {
- struct msgb *msg; /* msgb ownership is transferred to FSM */
- int cause;
+ struct msgb *msg;
+ struct tlv_parsed *tp;
+ int cause; /* set in NM_EV_SETATTR_(N)ACK */
};
diff --git a/include/osmo-bts/oml.h b/include/osmo-bts/oml.h
index 90c9077..c59f6ba 100644
--- a/include/osmo-bts/oml.h
+++ b/include/osmo-bts/oml.h
@@ -63,6 +63,7 @@
int oml_mo_tx_sw_act_rep(const struct gsm_abis_mo *mo);
int oml_fom_ack_nack(struct msgb *old_msg, uint8_t cause);
+int oml_fom_ack_nack_copy_msg(const struct msgb *old_msg, uint8_t cause);
int oml_mo_fom_ack_nack(const struct gsm_abis_mo *mo, uint8_t orig_msg_type,
uint8_t cause);
diff --git a/src/common/nm_bb_transc_fsm.c b/src/common/nm_bb_transc_fsm.c
index ca78256..82ef89d 100644
--- a/src/common/nm_bb_transc_fsm.c
+++ b/src/common/nm_bb_transc_fsm.c
@@ -128,7 +128,7 @@
case NM_EV_SETATTR_NACK:
setattr_data = (struct nm_fsm_ev_setattr_data *)data;
bb_transc->mo.setattr_success = setattr_data->cause == 0;
- oml_fom_ack_nack(setattr_data->msg, setattr_data->cause);
+ oml_fom_ack_nack_copy_msg(setattr_data->msg, setattr_data->cause);
break;
case NM_EV_OPSTART_ACK:
bb_transc->mo.opstart_success = true;
diff --git a/src/common/nm_bts_fsm.c b/src/common/nm_bts_fsm.c
index ea3a6c3..8ff57fe 100644
--- a/src/common/nm_bts_fsm.c
+++ b/src/common/nm_bts_fsm.c
@@ -111,13 +111,20 @@
{
struct gsm_bts *bts = (struct gsm_bts *)fi->priv;
struct nm_fsm_ev_setattr_data *setattr_data;
+ int rc;
switch (event) {
+ case NM_EV_RX_SETATTR:
+ setattr_data = (struct nm_fsm_ev_setattr_data *)data;
+ rc = bts_model_apply_oml(bts, setattr_data->msg, setattr_data->tp,
+ NM_OC_BTS, bts);
+ (void)rc;
+ break;
case NM_EV_SETATTR_ACK:
case NM_EV_SETATTR_NACK:
setattr_data = (struct nm_fsm_ev_setattr_data *)data;
bts->mo.setattr_success = setattr_data->cause == 0;
- oml_fom_ack_nack(setattr_data->msg, setattr_data->cause);
+ oml_fom_ack_nack_copy_msg(setattr_data->msg, setattr_data->cause);
break;
case NM_EV_OPSTART_ACK:
bts->mo.opstart_success = true;
@@ -178,6 +185,7 @@
},
[NM_BTS_ST_OP_DISABLED_OFFLINE] = {
.in_event_mask =
+ X(NM_EV_RX_SETATTR) |
X(NM_EV_SETATTR_ACK) |
X(NM_EV_SETATTR_NACK) |
X(NM_EV_OPSTART_ACK) |
diff --git a/src/common/nm_bts_sm_fsm.c b/src/common/nm_bts_sm_fsm.c
index 2d43315..66c4077 100644
--- a/src/common/nm_bts_sm_fsm.c
+++ b/src/common/nm_bts_sm_fsm.c
@@ -92,7 +92,7 @@
case NM_EV_SETATTR_NACK:
setattr_data = (struct nm_fsm_ev_setattr_data *)data;
site_mgr->mo.setattr_success = setattr_data->cause == 0;
- oml_fom_ack_nack(setattr_data->msg, setattr_data->cause);
+ oml_fom_ack_nack_copy_msg(setattr_data->msg, setattr_data->cause);
break;
case NM_EV_OPSTART_ACK:
site_mgr->mo.opstart_success = true;
diff --git a/src/common/nm_channel_fsm.c b/src/common/nm_channel_fsm.c
index 503ddfb..557d5b9 100644
--- a/src/common/nm_channel_fsm.c
+++ b/src/common/nm_channel_fsm.c
@@ -94,13 +94,20 @@
{
struct gsm_bts_trx_ts *ts = (struct gsm_bts_trx_ts *)fi->priv;
struct nm_fsm_ev_setattr_data *setattr_data;
+ int rc;
switch (event) {
+ case NM_EV_RX_SETATTR:
+ setattr_data = (struct nm_fsm_ev_setattr_data *)data;
+ rc = bts_model_apply_oml(ts->trx->bts, setattr_data->msg, setattr_data->tp,
+ NM_OC_CHANNEL, ts);
+ (void)rc;
+ break;
case NM_EV_SETATTR_ACK:
case NM_EV_SETATTR_NACK:
setattr_data = (struct nm_fsm_ev_setattr_data *)data;
ts->mo.setattr_success = setattr_data->cause == 0;
- oml_fom_ack_nack(setattr_data->msg, setattr_data->cause);
+ oml_fom_ack_nack_copy_msg(setattr_data->msg, setattr_data->cause);
break;
case NM_EV_OPSTART_ACK:
LOGPFSML(fi, LOGL_NOTICE, "BSC trying to activate TS while still in avail=dependency. "
@@ -138,13 +145,20 @@
{
struct gsm_bts_trx_ts *ts = (struct gsm_bts_trx_ts *)fi->priv;
struct nm_fsm_ev_setattr_data *setattr_data;
+ int rc;
switch (event) {
+ case NM_EV_RX_SETATTR:
+ setattr_data = (struct nm_fsm_ev_setattr_data *)data;
+ rc = bts_model_apply_oml(ts->trx->bts, setattr_data->msg, setattr_data->tp,
+ NM_OC_CHANNEL, ts);
+ (void)rc;
+ break;
case NM_EV_SETATTR_ACK:
case NM_EV_SETATTR_NACK:
setattr_data = (struct nm_fsm_ev_setattr_data *)data;
ts->mo.setattr_success = setattr_data->cause == 0;
- oml_fom_ack_nack(setattr_data->msg, setattr_data->cause);
+ oml_fom_ack_nack_copy_msg(setattr_data->msg, setattr_data->cause);
break;
case NM_EV_OPSTART_ACK:
ts->mo.opstart_success = true;
@@ -220,6 +234,7 @@
},
[NM_CHAN_ST_OP_DISABLED_DEPENDENCY] = {
.in_event_mask =
+ X(NM_EV_RX_SETATTR) |
X(NM_EV_SETATTR_ACK) |
X(NM_EV_SETATTR_NACK) |
X(NM_EV_OPSTART_ACK) | /* backward compatibility, buggy BSC */
@@ -238,6 +253,7 @@
},
[NM_CHAN_ST_OP_DISABLED_OFFLINE] = {
.in_event_mask =
+ X(NM_EV_RX_SETATTR) |
X(NM_EV_SETATTR_ACK) |
X(NM_EV_SETATTR_NACK) |
X(NM_EV_OPSTART_ACK) |
diff --git a/src/common/nm_common_fsm.c b/src/common/nm_common_fsm.c
index be11bef..6a8d3d1 100644
--- a/src/common/nm_common_fsm.c
+++ b/src/common/nm_common_fsm.c
@@ -25,6 +25,7 @@
const struct value_string nm_fsm_event_names[] = {
{ NM_EV_SW_ACT, "SW_ACT" },
+ { NM_EV_SETATTR_ACK, "RX_SETATTR" },
{ NM_EV_SETATTR_ACK, "SETATTR_ACK" },
{ NM_EV_SETATTR_NACK, "SETATTR_NACK" },
{ NM_EV_OPSTART_ACK, "OPSTART_ACK" },
diff --git a/src/common/nm_radio_carrier_fsm.c b/src/common/nm_radio_carrier_fsm.c
index 88930dd..587847f 100644
--- a/src/common/nm_radio_carrier_fsm.c
+++ b/src/common/nm_radio_carrier_fsm.c
@@ -103,13 +103,20 @@
struct nm_fsm_ev_setattr_data *setattr_data;
bool phy_state_connected;
bool rsl_link_connected;
+ int rc;
switch (event) {
+ case NM_EV_RX_SETATTR:
+ setattr_data = (struct nm_fsm_ev_setattr_data *)data;
+ rc = bts_model_apply_oml(trx->bts, setattr_data->msg, setattr_data->tp,
+ NM_OC_RADIO_CARRIER, trx);
+ (void)rc;
+ break;
case NM_EV_SETATTR_ACK:
case NM_EV_SETATTR_NACK:
setattr_data = (struct nm_fsm_ev_setattr_data *)data;
trx->mo.setattr_success = setattr_data->cause == 0;
- oml_fom_ack_nack(setattr_data->msg, setattr_data->cause);
+ oml_fom_ack_nack_copy_msg(setattr_data->msg, setattr_data->cause);
break;
case NM_EV_OPSTART_ACK:
trx->mo.opstart_success = true;
@@ -219,6 +226,7 @@
},
[NM_RCARRIER_ST_OP_DISABLED_OFFLINE] = {
.in_event_mask =
+ X(NM_EV_RX_SETATTR) |
X(NM_EV_SETATTR_ACK) |
X(NM_EV_SETATTR_NACK) |
X(NM_EV_OPSTART_ACK) |
diff --git a/src/common/oml.c b/src/common/oml.c
index 5474fb1..2d81054 100644
--- a/src/common/oml.c
+++ b/src/common/oml.c
@@ -48,6 +48,7 @@
#include <osmo-bts/bts.h>
#include <osmo-bts/signal.h>
#include <osmo-bts/phy_link.h>
+#include <osmo-bts/nm_common_fsm.h>
#define LOGPFOH(ss, lvl, foh, fmt, args ...) LOGP(ss, lvl, "%s: " fmt, abis_nm_dump_foh(foh), ## args)
#define DEBUGPFOH(ss, foh, fmt, args ...) LOGPFOH(ss, LOGL_DEBUG, foh, fmt, ## args)
@@ -472,6 +473,15 @@
return 1;
}
+/* Copy msg before calling oml_fom_ack_nack(), which takes its ownership */
+int oml_fom_ack_nack_copy_msg(const struct msgb *old_msg, uint8_t cause)
+{
+ struct msgb *msg = msgb_copy(old_msg, "OML-ack_nack");
+ msg->trx = old_msg->trx;
+ oml_fom_ack_nack(msg, cause);
+ return 0;
+}
+
/*
* Formatted O&M messages
*/
@@ -549,6 +559,7 @@
struct tlv_parsed tp, *tp_merged;
int rc, i;
const uint8_t *payload;
+ struct nm_fsm_ev_setattr_data ev_data;
DEBUGPFOH(DOML, foh, "Rx SET BTS ATTR\n");
@@ -742,8 +753,15 @@
}
}
- /* call into BTS driver to apply new attributes to hardware */
- return bts_model_apply_oml(bts, msg, tp_merged, NM_OC_BTS, bts);
+ ev_data = (struct nm_fsm_ev_setattr_data){
+ .msg = msg,
+ .tp = tp_merged,
+ };
+
+ rc = osmo_fsm_inst_dispatch(bts->mo.fi, NM_EV_RX_SETATTR, &ev_data);
+ if (rc < 0)
+ return oml_fom_ack_nack(msg, NM_NACK_CANT_PERFORM);
+ return rc;
}
/* 8.6.2 Set Radio Attributes has been received */
@@ -752,6 +770,7 @@
struct abis_om_fom_hdr *foh = msgb_l3(msg);
struct tlv_parsed tp, *tp_merged;
int rc;
+ struct nm_fsm_ev_setattr_data ev_data;
DEBUGPFOH(DOML, foh, "Rx SET RADIO CARRIER ATTR\n");
@@ -830,8 +849,17 @@
trx->arfcn = arfcn;
}
#endif
- /* call into BTS driver to apply new attributes to hardware */
- return bts_model_apply_oml(trx->bts, msg, tp_merged, NM_OC_RADIO_CARRIER, trx);
+
+ ev_data = (struct nm_fsm_ev_setattr_data){
+ .msg = msg,
+ .tp = tp_merged,
+ };
+
+ rc = osmo_fsm_inst_dispatch(trx->mo.fi, NM_EV_RX_SETATTR, &ev_data);
+ if (rc < 0)
+ return oml_fom_ack_nack(msg, NM_NACK_CANT_PERFORM);
+ return rc;
+
}
static int handle_chan_comb(struct gsm_bts_trx_ts *ts, const uint8_t comb)
@@ -927,6 +955,7 @@
struct gsm_bts *bts = ts->trx->bts;
struct tlv_parsed tp, *tp_merged;
int rc, i;
+ struct nm_fsm_ev_setattr_data ev_data;
DEBUGPFOH(DOML, foh, "Rx SET CHAN ATTR\n");
@@ -1035,8 +1064,15 @@
ts->hopping.hsn, ts->hopping.maio, ts->hopping.arfcn_num);
LOGPC(DOML, LOGL_INFO, ")\n");
- /* call into BTS driver to apply new attributes to hardware */
- return bts_model_apply_oml(bts, msg, tp_merged, NM_OC_CHANNEL, ts);
+ ev_data = (struct nm_fsm_ev_setattr_data){
+ .msg = msg,
+ .tp = tp_merged,
+ };
+
+ rc = osmo_fsm_inst_dispatch(ts->mo.fi, NM_EV_RX_SETATTR, &ev_data);
+ if (rc < 0)
+ return oml_fom_ack_nack(msg, NM_NACK_CANT_PERFORM);
+ return rc;
}
/* 8.9.2 Opstart has been received */
diff --git a/src/osmo-bts-lc15/oml.c b/src/osmo-bts-lc15/oml.c
index 1842008..9502765 100644
--- a/src/osmo-bts-lc15/oml.c
+++ b/src/osmo-bts-lc15/oml.c
@@ -1859,8 +1859,7 @@
rc = osmo_fsm_inst_dispatch(mo->fi,
ev_data.cause == 0 ? NM_EV_SETATTR_ACK : NM_EV_SETATTR_NACK,
&ev_data);
- /* msgb ownership is transferred to FSM if it received ev: */
- return rc == 0 ? 1 : 0;
+ return rc;
}
/* callback from OML */
diff --git a/src/osmo-bts-oc2g/oml.c b/src/osmo-bts-oc2g/oml.c
index 98c2fbc..40c1c85 100644
--- a/src/osmo-bts-oc2g/oml.c
+++ b/src/osmo-bts-oc2g/oml.c
@@ -1864,8 +1864,7 @@
rc = osmo_fsm_inst_dispatch(mo->fi,
ev_data.cause == 0 ? NM_EV_SETATTR_ACK : NM_EV_SETATTR_NACK,
&ev_data);
- /* msgb ownsership is transferred to FSM if it received ev: */
- return rc == 0 ? 1 : 0;
+ return rc;
}
/* callback from OML */
diff --git a/src/osmo-bts-octphy/l1_oml.c b/src/osmo-bts-octphy/l1_oml.c
index 01e3d56..8ffd1ac 100644
--- a/src/osmo-bts-octphy/l1_oml.c
+++ b/src/osmo-bts-octphy/l1_oml.c
@@ -1765,8 +1765,7 @@
rc = osmo_fsm_inst_dispatch(mo->fi,
ev_data.cause == 0 ? NM_EV_SETATTR_ACK : NM_EV_SETATTR_NACK,
&ev_data);
- /* msgb ownsership is transferred to FSM if it received ev: */
- return rc == 0 ? 1 : 0;
+ return rc;
}
diff --git a/src/osmo-bts-omldummy/bts_model.c b/src/osmo-bts-omldummy/bts_model.c
index 7fb58f7..af358b6 100644
--- a/src/osmo-bts-omldummy/bts_model.c
+++ b/src/osmo-bts-omldummy/bts_model.c
@@ -119,8 +119,7 @@
rc = osmo_fsm_inst_dispatch(mo->fi,
ev_data.cause == 0 ? NM_EV_SETATTR_ACK : NM_EV_SETATTR_NACK,
&ev_data);
- /* msgb ownsership is transferred to FSM if it received ev: */
- return rc == 0 ? 1 : 0;
+ return rc;
}
/* MO: TS 12.21 Managed Object */
diff --git a/src/osmo-bts-sysmo/oml.c b/src/osmo-bts-sysmo/oml.c
index 67e1275..12b50bf 100644
--- a/src/osmo-bts-sysmo/oml.c
+++ b/src/osmo-bts-sysmo/oml.c
@@ -1741,8 +1741,7 @@
rc = osmo_fsm_inst_dispatch(mo->fi,
ev_data.cause == 0 ? NM_EV_SETATTR_ACK : NM_EV_SETATTR_NACK,
&ev_data);
- /* msgb ownsership is transferred to FSM if it received ev: */
- return rc == 0 ? 1 : 0;
+ return rc;
}
/* callback from OML */
diff --git a/src/osmo-bts-trx/l1_if.c b/src/osmo-bts-trx/l1_if.c
index 4989bcc..1834cf5 100644
--- a/src/osmo-bts-trx/l1_if.c
+++ b/src/osmo-bts-trx/l1_if.c
@@ -574,8 +574,7 @@
rc = osmo_fsm_inst_dispatch(mo->fi,
ev_data.cause == 0 ? NM_EV_SETATTR_ACK : NM_EV_SETATTR_NACK,
&ev_data);
- /* msgb ownsership is transferred to FSM if it received ev: */
- return rc == 0 ? 1 : 0;
+ return rc;
}
/* callback from OML */
diff --git a/src/osmo-bts-virtual/bts_model.c b/src/osmo-bts-virtual/bts_model.c
index 57e5304..477ffb3 100644
--- a/src/osmo-bts-virtual/bts_model.c
+++ b/src/osmo-bts-virtual/bts_model.c
@@ -154,8 +154,7 @@
rc = osmo_fsm_inst_dispatch(mo->fi,
ev_data.cause == 0 ? NM_EV_SETATTR_ACK : NM_EV_SETATTR_NACK,
&ev_data);
- /* msgb ownsership is transferred to FSM if it received ev: */
- return rc == 0 ? 1 : 0;
+ return rc;
}
/* MO: TS 12.21 Managed Object */
To view, visit change 32194. To unsubscribe, or for help writing mail filters, visit settings.