pespin has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/32196 )


Change subject: nm: Drop NM_EV_SETATTR_{ACK/NACK}
......................................................................

nm: Drop NM_EV_SETATTR_{ACK/NACK}

Simply return error codes from lower layer implementations, and do
the OML handling in the common NM TS 12.21 FSMs.

As a result, we simplify the logic in the lower layers.

Change-Id: I281c07bb5ad88ee03542f092141cebe036d10aee
---
M include/osmo-bts/nm_common_fsm.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/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
14 files changed, 78 insertions(+), 135 deletions(-)



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

diff --git a/include/osmo-bts/nm_common_fsm.h b/include/osmo-bts/nm_common_fsm.h
index b1b779e..b2aaa89 100644
--- a/include/osmo-bts/nm_common_fsm.h
+++ b/include/osmo-bts/nm_common_fsm.h
@@ -31,8 +31,6 @@
 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,
        NM_EV_OPSTART_NACK,
        NM_EV_SHUTDOWN_START,
@@ -53,7 +51,6 @@
 struct nm_fsm_ev_setattr_data {
        struct msgb *msg; /* msgb ownership is transferred to FSM */
        struct tlv_parsed *tp;
-       int cause; /* set in NM_EV_SETATTR_(N)ACK */
 };


diff --git a/src/common/nm_bb_transc_fsm.c b/src/common/nm_bb_transc_fsm.c
index ca78256..bc79f17 100644
--- a/src/common/nm_bb_transc_fsm.c
+++ b/src/common/nm_bb_transc_fsm.c
@@ -122,13 +122,15 @@
        struct nm_fsm_ev_setattr_data *setattr_data;
        bool phy_state_connected;
        bool rsl_link_connected;
+       int rc;

        switch (event) {
-       case NM_EV_SETATTR_ACK:
-       case NM_EV_SETATTR_NACK:
+       case NM_EV_RX_SETATTR:
                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);
+               rc = bts_model_apply_oml(trx->bts, setattr_data->msg, 
setattr_data->tp,
+                                        NM_OC_BASEB_TRANSC, bb_transc);
+               bb_transc->mo.setattr_success = rc == 0;
+               oml_fom_ack_nack(setattr_data->msg, rc);
                break;
        case NM_EV_OPSTART_ACK:
                bb_transc->mo.opstart_success = true;
@@ -245,8 +247,7 @@
        },
        [NM_BBTRANSC_ST_OP_DISABLED_OFFLINE] = {
                .in_event_mask =
-                       X(NM_EV_SETATTR_ACK) |
-                       X(NM_EV_SETATTR_NACK) |
+                       X(NM_EV_RX_SETATTR) |
                        X(NM_EV_OPSTART_ACK) |
                        X(NM_EV_OPSTART_NACK) |
                        X(NM_EV_RSL_UP) |
diff --git a/src/common/nm_bts_fsm.c b/src/common/nm_bts_fsm.c
index 47b91e4..a51c9a7 100644
--- a/src/common/nm_bts_fsm.c
+++ b/src/common/nm_bts_fsm.c
@@ -118,13 +118,8 @@
                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);
+               bts->mo.setattr_success = rc == 0;
+               oml_fom_ack_nack(setattr_data->msg, rc);
                break;
        case NM_EV_OPSTART_ACK:
                bts->mo.opstart_success = true;
@@ -186,8 +181,6 @@
        [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) |
                        X(NM_EV_OPSTART_NACK),
                .out_state_mask =
diff --git a/src/common/nm_bts_sm_fsm.c b/src/common/nm_bts_sm_fsm.c
index 2d43315..939bc43 100644
--- a/src/common/nm_bts_sm_fsm.c
+++ b/src/common/nm_bts_sm_fsm.c
@@ -86,13 +86,15 @@
 {
        struct gsm_bts_sm *site_mgr = (struct gsm_bts_sm *)fi->priv;
        struct nm_fsm_ev_setattr_data *setattr_data;
+       int rc;

        switch (event) {
-       case NM_EV_SETATTR_ACK:
-       case NM_EV_SETATTR_NACK:
+       case NM_EV_RX_SETATTR:
                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);
+               /* No bts_model_apply_oml() needed yet for site_mgr obj yet: */
+               rc = 0;
+               site_mgr->mo.setattr_success = rc == 0;
+               oml_fom_ack_nack(setattr_data->msg, rc);
                break;
        case NM_EV_OPSTART_ACK:
                site_mgr->mo.opstart_success = true;
@@ -153,8 +155,7 @@
        },
        [NM_BTS_SM_ST_OP_DISABLED_OFFLINE] = {
                .in_event_mask =
-                       X(NM_EV_SETATTR_ACK) |
-                       X(NM_EV_SETATTR_NACK) |
+                       X(NM_EV_RX_SETATTR) |
                        X(NM_EV_OPSTART_ACK) |
                        X(NM_EV_OPSTART_NACK),
                .out_state_mask =
diff --git a/src/common/nm_channel_fsm.c b/src/common/nm_channel_fsm.c
index 3abf941..632d58c 100644
--- a/src/common/nm_channel_fsm.c
+++ b/src/common/nm_channel_fsm.c
@@ -101,13 +101,8 @@
                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);
+               ts->mo.setattr_success = rc == 0;
+               oml_fom_ack_nack(setattr_data->msg, rc);
                break;
        case NM_EV_OPSTART_ACK:
                 LOGPFSML(fi, LOGL_NOTICE, "BSC trying to activate TS while 
still in avail=dependency. "
@@ -151,14 +146,9 @@
        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);
+                                       NM_OC_CHANNEL, ts);
+               ts->mo.setattr_success = rc == 0;
+               oml_fom_ack_nack(setattr_data->msg, rc);
                break;
        case NM_EV_OPSTART_ACK:
                ts->mo.opstart_success = true;
@@ -235,8 +225,6 @@
        [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 */
                        X(NM_EV_OPSTART_NACK) |
                        X(NM_EV_BBTRANSC_ENABLED) |
@@ -254,8 +242,6 @@
        [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) |
                        X(NM_EV_OPSTART_NACK) |
                        X(NM_EV_BBTRANSC_DISABLED) |
diff --git a/src/common/nm_common_fsm.c b/src/common/nm_common_fsm.c
index 6a8d3d1..5298e4d 100644
--- a/src/common/nm_common_fsm.c
+++ b/src/common/nm_common_fsm.c
@@ -25,9 +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_RX_SETATTR, "RX_SETATTR" },
        { NM_EV_OPSTART_ACK, "OPSTART_ACK" },
        { NM_EV_OPSTART_NACK, "OPSTART_NACK" },
        { NM_EV_SHUTDOWN_START, "SHUTDOWN_START" },
diff --git a/src/common/nm_radio_carrier_fsm.c 
b/src/common/nm_radio_carrier_fsm.c
index 4513d6c..0cb4fbb 100644
--- a/src/common/nm_radio_carrier_fsm.c
+++ b/src/common/nm_radio_carrier_fsm.c
@@ -110,13 +110,8 @@
                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);
+               trx->mo.setattr_success = rc == 0;
+               oml_fom_ack_nack(setattr_data->msg, rc);
                break;
        case NM_EV_OPSTART_ACK:
                trx->mo.opstart_success = true;
@@ -227,8 +222,6 @@
        [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) |
                        X(NM_EV_OPSTART_NACK) |
                        X(NM_EV_RSL_UP) |
diff --git a/src/osmo-bts-lc15/oml.c b/src/osmo-bts-lc15/oml.c
index 1842008..9370738 100644
--- a/src/osmo-bts-lc15/oml.c
+++ b/src/osmo-bts-lc15/oml.c
@@ -1805,11 +1805,6 @@
                        struct tlv_parsed *new_attr, int kind, void *obj)
 {
        struct abis_om_fom_hdr *foh = msgb_l3(msg);
-       struct gsm_abis_mo *mo = gsm_objclass2mo(bts, foh->obj_class, 
&foh->obj_inst);
-       struct nm_fsm_ev_setattr_data ev_data = {
-               .msg = msg,
-               .cause = 0,
-       };
        int rc;
        struct gsm_bts_trx *trx;
        struct lc15l1_hdl *fl1h;
@@ -1853,14 +1848,14 @@
                        }
 #endif
                }
+               rc = 0;
+               break;
+       default:
+               rc = 0;
                break;
        }

-       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..4de93b4 100644
--- a/src/osmo-bts-oc2g/oml.c
+++ b/src/osmo-bts-oc2g/oml.c
@@ -1814,11 +1814,6 @@
                        struct tlv_parsed *new_attr, int kind, void *obj)
 {
        struct abis_om_fom_hdr *foh = msgb_l3(msg);
-       struct gsm_abis_mo *mo = gsm_objclass2mo(bts, foh->obj_class, 
&foh->obj_inst);
-       struct nm_fsm_ev_setattr_data ev_data = {
-               .msg = msg,
-               .cause = 0,
-       };
        int rc;
        struct gsm_bts_trx *trx;
        struct oc2gl1_hdl *fl1h;
@@ -1858,14 +1853,14 @@
                                l1if_set_txpower_c0_idle_pwr_red(fl1h, 
fl1h->phy_inst->u.oc2g.tx_c0_idle_pwr_red);
                        }
                }
+               rc = 0;
+               break;
+       default:
+               rc = 0;
                break;
        }

-       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..28281f8 100644
--- a/src/osmo-bts-octphy/l1_oml.c
+++ b/src/osmo-bts-octphy/l1_oml.c
@@ -1746,27 +1746,21 @@
                        struct tlv_parsed *new_attr, int kind, void *obj)
 {
        struct abis_om_fom_hdr *foh = msgb_l3(msg);
-       struct gsm_abis_mo *mo = gsm_objclass2mo(bts, foh->obj_class, 
&foh->obj_inst);
-       struct nm_fsm_ev_setattr_data ev_data = {
-               .msg = msg,
-               .cause = 0,
-       };
        int rc;
        struct gsm_bts_trx *trx;

        switch (foh->msg_type) {
        case NM_MT_SET_RADIO_ATTR:
                trx = obj;
-               /*struct octphy_hdl *fl1h = trx_octphy_hdl(trx); */
                power_ramp_start(trx, get_p_target_mdBm(trx, 0), 0, NULL);
+               rc = 0;
+               break;
+       default:
+               rc = 0;
                break;
        }

-       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..c010508 100644
--- a/src/osmo-bts-omldummy/bts_model.c
+++ b/src/osmo-bts-omldummy/bts_model.c
@@ -97,30 +97,24 @@
                        struct tlv_parsed *new_attr, int kind, void *obj)
 {
        struct abis_om_fom_hdr *foh = msgb_l3(msg);
-       struct gsm_abis_mo *mo = gsm_objclass2mo(bts, foh->obj_class, 
&foh->obj_inst);
-       struct nm_fsm_ev_setattr_data ev_data = {
-               .msg = msg,
-               .cause = 0,
-       };
        int rc;

        switch (foh->msg_type) {
        case NM_MT_SET_BTS_ATTR:
-               ev_data.cause =  vbts_set_bts(obj);
+               rc = vbts_set_bts(obj);
                break;
        case NM_MT_SET_RADIO_ATTR:
-               ev_data.cause = vbts_set_trx(obj);
+               rc = vbts_set_trx(obj);
                break;
        case NM_MT_SET_CHAN_ATTR:
-               ev_data.cause = vbts_set_ts(obj);
+               rc = vbts_set_ts(obj);
+               break;
+       default:
+               rc = 0;
                break;
        }

-       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..1d5b18c 100644
--- a/src/osmo-bts-sysmo/oml.c
+++ b/src/osmo-bts-sysmo/oml.c
@@ -1718,11 +1718,6 @@
                        struct tlv_parsed *new_attr, int kind, void *obj)
 {
        struct abis_om_fom_hdr *foh = msgb_l3(msg);
-       struct gsm_abis_mo *mo = gsm_objclass2mo(bts, foh->obj_class, 
&foh->obj_inst);
-       struct nm_fsm_ev_setattr_data ev_data = {
-               .msg = msg,
-               .cause = 0,
-       };
        int rc;
        struct gsm_bts_trx *trx;
        struct femtol1_hdl *fl1h;
@@ -1736,13 +1731,12 @@
                if (fl1h->hLayer1)
                        power_ramp_start(trx, get_p_target_mdBm(trx, 0), 0, 
NULL);
                break;
+       default:
+               rc = 0;
+               break;
        }

-       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 57a4c41..9e9dd64 100644
--- a/src/osmo-bts-trx/l1_if.c
+++ b/src/osmo-bts-trx/l1_if.c
@@ -552,30 +552,24 @@
                        struct tlv_parsed *new_attr, int kind, void *obj)
 {
        struct abis_om_fom_hdr *foh = msgb_l3(msg);
-       struct gsm_abis_mo *mo = gsm_objclass2mo(bts, foh->obj_class, 
&foh->obj_inst);
-       struct nm_fsm_ev_setattr_data ev_data = {
-               .msg = msg,
-               .cause = 0,
-       };
        int rc;

        switch (foh->msg_type) {
        case NM_MT_SET_BTS_ATTR:
-               ev_data.cause = trx_set_bts(obj, new_attr);
+               rc = trx_set_bts(obj, new_attr);
                break;
        case NM_MT_SET_RADIO_ATTR:
-               ev_data.cause = trx_set_trx(obj);
+               rc = trx_set_trx(obj);
                break;
        case NM_MT_SET_CHAN_ATTR:
-               ev_data.cause = trx_set_ts(obj);
+               rc = trx_set_ts(obj);
+               break;
+       default:
+               rc = 0;
                break;
        }

-       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..bcf66be 100644
--- a/src/osmo-bts-virtual/bts_model.c
+++ b/src/osmo-bts-virtual/bts_model.c
@@ -132,30 +132,24 @@
                        struct tlv_parsed *new_attr, int kind, void *obj)
 {
        struct abis_om_fom_hdr *foh = msgb_l3(msg);
-       struct gsm_abis_mo *mo = gsm_objclass2mo(bts, foh->obj_class, 
&foh->obj_inst);
-       struct nm_fsm_ev_setattr_data ev_data = {
-               .msg = msg,
-               .cause = 0,
-       };
        int rc;

        switch (foh->msg_type) {
        case NM_MT_SET_BTS_ATTR:
-               ev_data.cause = vbts_set_bts(obj);
+               rc = vbts_set_bts(obj);
                break;
        case NM_MT_SET_RADIO_ATTR:
-               ev_data.cause = vbts_set_trx(obj);
+               rc = vbts_set_trx(obj);
                break;
        case NM_MT_SET_CHAN_ATTR:
-               ev_data.cause = vbts_set_ts(obj);
+               rc = vbts_set_ts(obj);
+               break;
+       default:
+               rc = 0;
                break;
        }

-       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 https://gerrit.osmocom.org/c/osmo-bts/+/32196
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

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

Reply via email to