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


Change subject: Split cnlink FSM code to its own file
......................................................................

Split cnlink FSM code to its own file

As usual, it makes it a lot easier to read an FSM by having all of it
enclosed in one file.

Change-Id: I7e40beb13c2174e01d89915a28395a364709b321
---
M include/osmocom/hnbgw/cnlink.h
M src/osmo-hnbgw/Makefile.am
M src/osmo-hnbgw/cnlink.c
A src/osmo-hnbgw/cnlink_fsm.c
4 files changed, 244 insertions(+), 206 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/62/40262/1

diff --git a/include/osmocom/hnbgw/cnlink.h b/include/osmocom/hnbgw/cnlink.h
index 9495041..3aeed0b 100644
--- a/include/osmocom/hnbgw/cnlink.h
+++ b/include/osmocom/hnbgw/cnlink.h
@@ -13,19 +13,6 @@

 struct hnbgw_cnpool;

-struct hnbgw_cnlink *cnlink_alloc(struct hnbgw_cnpool *cnpool, int nr);
-
-void hnbgw_cnlink_drop_sccp(struct hnbgw_cnlink *cnlink);
-
-bool cnlink_is_conn_ready(const struct hnbgw_cnlink *cnlink);
-void cnlink_rx_reset_cmd(struct hnbgw_cnlink *cnlink);
-void cnlink_rx_reset_ack(struct hnbgw_cnlink *cnlink);
-void cnlink_resend_reset(struct hnbgw_cnlink *cnlink);
-void cnlink_set_disconnected(struct hnbgw_cnlink *cnlink);
-
-const char *cnlink_paging_add_ranap(struct hnbgw_cnlink *cnlink, const 
RANAP_PagingIEs_t *paging_ies);
-struct hnbgw_cnlink *cnlink_find_by_paging_mi(struct hnbgw_cnpool *cnpool, 
const struct osmo_mobile_identity *mi);
-
 enum hnbgw_cnlink_ctr {
        /* TODO: basic counters completely missing
         * ...
@@ -105,6 +92,11 @@
        struct rate_ctr_group *ctrs;
 };

+struct hnbgw_cnlink *cnlink_alloc(struct hnbgw_cnpool *cnpool, int nr);
+void hnbgw_cnlink_drop_sccp(struct hnbgw_cnlink *cnlink);
+int hnbgw_cnlink_tx_ranap_reset(struct hnbgw_cnlink *cnlink);
+int hnbgw_cnlink_tx_ranap_reset_ack(struct hnbgw_cnlink *cnlink);
+
 static inline struct osmo_sccp_instance *cnlink_sccp(const struct hnbgw_cnlink 
*cnlink)
 {
        if (!cnlink)
@@ -116,6 +108,18 @@
        return osmo_ss7_get_sccp(cnlink->hnbgw_sccp_user->ss7);
 }

+/* cnlink_fsm.c related: */
+extern struct osmo_fsm cnlink_fsm;
+bool cnlink_is_conn_ready(const struct hnbgw_cnlink *cnlink);
+void cnlink_rx_reset_cmd(struct hnbgw_cnlink *cnlink);
+void cnlink_rx_reset_ack(struct hnbgw_cnlink *cnlink);
+void cnlink_resend_reset(struct hnbgw_cnlink *cnlink);
+void cnlink_set_disconnected(struct hnbgw_cnlink *cnlink);
+
+/* cnlink_paging.c related: */
+const char *cnlink_paging_add_ranap(struct hnbgw_cnlink *cnlink, const 
RANAP_PagingIEs_t *paging_ies);
+struct hnbgw_cnlink *cnlink_find_by_paging_mi(struct hnbgw_cnpool *cnpool, 
const struct osmo_mobile_identity *mi);
+
 #define LOG_CNLINK(CNLINK, SUBSYS, LEVEL, FMT, ARGS...) \
        LOGP(SUBSYS, LEVEL, "(%s) " FMT, (CNLINK) ? (CNLINK)->name : "null", 
##ARGS)

diff --git a/src/osmo-hnbgw/Makefile.am b/src/osmo-hnbgw/Makefile.am
index c0798a7..c2ff265 100644
--- a/src/osmo-hnbgw/Makefile.am
+++ b/src/osmo-hnbgw/Makefile.am
@@ -46,6 +46,7 @@
        context_map_sccp.c \
        hnbgw_cn.c \
        cnlink.c \
+       cnlink_fsm.c \
        cnlink_paging.c \
        ranap_rab_ass.c \
        mgw_fsm.c \
diff --git a/src/osmo-hnbgw/cnlink.c b/src/osmo-hnbgw/cnlink.c
index 46f53d7..cea2827 100644
--- a/src/osmo-hnbgw/cnlink.c
+++ b/src/osmo-hnbgw/cnlink.c
@@ -34,34 +34,6 @@
 #include <osmocom/hnbgw/tdefs.h>
 #include <osmocom/hnbgw/context_map.h>

-static struct osmo_fsm cnlink_fsm;
-
-enum cnlink_fsm_state {
-       CNLINK_ST_DISC,
-       CNLINK_ST_CONN,
-};
-
-enum cnlink_fsm_event {
-       CNLINK_EV_RX_RESET,
-       CNLINK_EV_RX_RESET_ACK,
-};
-
-static const struct value_string cnlink_fsm_event_names[] = {
-       OSMO_VALUE_STRING(CNLINK_EV_RX_RESET),
-       OSMO_VALUE_STRING(CNLINK_EV_RX_RESET_ACK),
-       {}
-};
-
-static const struct osmo_tdef_state_timeout cnlink_timeouts[32] = {
-       [CNLINK_ST_DISC] = { .T = 4 },
-};
-
-#define cnlink_fsm_state_chg(FI, STATE) \
-       osmo_tdef_fsm_inst_state_chg(FI, STATE, \
-                                    cnlink_timeouts, \
-                                    hnbgw_T_defs, \
-                                    -1)
-
 struct hnbgw_cnlink *cnlink_alloc(struct hnbgw_cnpool *cnpool, int nr)
 {
        struct osmo_fsm_inst *fi;
@@ -93,8 +65,7 @@
        llist_add_tail(&cnlink->entry, &cnpool->cnlinks);
        LOG_CNLINK(cnlink, DCN, LOGL_DEBUG, "allocated\n");

-       /* Immediately (1ms) kick off reset sending mechanism */
-       osmo_fsm_inst_state_chg_ms(fi, CNLINK_ST_DISC, 1, 0);
+       cnlink_resend_reset(cnlink);
        return cnlink;
 }

@@ -123,21 +94,6 @@
        talloc_free(cnlink);
 }

-static void link_up(struct hnbgw_cnlink *cnlink)
-{
-       LOGPFSML(cnlink->fi, LOGL_NOTICE, "link up\n");
-}
-
-static void link_lost(struct hnbgw_cnlink *cnlink)
-{
-       struct hnbgw_context_map *map, *map2;
-
-       LOGPFSML(cnlink->fi, LOGL_NOTICE, "link lost\n");
-
-       llist_for_each_entry_safe(map, map2, &cnlink->map_list, 
hnbgw_cnlink_entry)
-               context_map_cnlink_lost(map);
-}
-
 static int hnbgw_cnlink_tx_sccp_unitdata_req(struct hnbgw_cnlink *cnlink, 
struct msgb *msg)
 {
        CNLINK_CTR_INC(cnlink, CNLINK_CTR_SCCP_N_UNITDATA_REQ);
@@ -146,7 +102,7 @@
                                               msg);
 }

-static void tx_reset(struct hnbgw_cnlink *cnlink)
+int hnbgw_cnlink_tx_ranap_reset(struct hnbgw_cnlink *cnlink)
 {
        struct msgb *msg;
        RANAP_Cause_t cause = {
@@ -158,7 +114,7 @@
        uint8_t plmn_buf[3];

        if (!cnlink)
-               return;
+               return -1;

        /* We need to have chosen an SCCP instance, and the remote SCCP address 
needs to be set.
         * Only check the remote_addr, allowing use.remote_addr_name to be 
NULL: if the user has not set an explicit
@@ -166,7 +122,7 @@
        if (!cnlink->hnbgw_sccp_user
            || !osmo_sccp_check_addr(&cnlink->remote_addr, OSMO_SCCP_ADDR_T_PC 
| OSMO_SCCP_ADDR_T_SSN)) {
                LOG_CNLINK(cnlink, DRANAP, LOGL_DEBUG, "not yet configured, not 
sending RANAP RESET\n");
-               return;
+               return -1;
        }

        LOG_CNLINK(cnlink, DRANAP, LOGL_DEBUG, "Tx RANAP RESET to %s %s\n",
@@ -200,10 +156,10 @@

        msg = ranap_new_msg_reset2(cnlink->pool->domain, &cause, use_grnc_id);
        CNLINK_CTR_INC(cnlink, CNLINK_CTR_RANAP_TX_UDT_RESET);
-       hnbgw_cnlink_tx_sccp_unitdata_req(cnlink, msg);
+       return hnbgw_cnlink_tx_sccp_unitdata_req(cnlink, msg);
 }

-static void tx_reset_ack(struct hnbgw_cnlink *cnlink)
+int hnbgw_cnlink_tx_ranap_reset_ack(struct hnbgw_cnlink *cnlink)
 {
        struct msgb *msg;
        struct osmo_sccp_instance *sccp = cnlink_sccp(cnlink);
@@ -213,7 +169,7 @@

        if (!sccp) {
                LOG_CNLINK(cnlink, DRANAP, LOGL_ERROR, "cannot send RANAP RESET 
ACK: no CN link\n");
-               return;
+               return -1;
        }

        LOG_CNLINK(cnlink, DRANAP, LOGL_NOTICE, "Tx RANAP RESET ACK %s %s --> 
%s\n",
@@ -248,146 +204,5 @@

        msg = ranap_new_msg_reset_ack(cnlink->pool->domain, use_grnc_id);
        CNLINK_CTR_INC(cnlink, CNLINK_CTR_RANAP_TX_UDT_RESET_ACK);
-       hnbgw_cnlink_tx_sccp_unitdata_req(cnlink, msg);
+       return hnbgw_cnlink_tx_sccp_unitdata_req(cnlink, msg);
 }
-
-static void cnlink_disc_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state)
-{
-       struct hnbgw_cnlink *cnlink = (struct hnbgw_cnlink *)fi->priv;
-       if (prev_state == CNLINK_ST_CONN)
-               link_lost(cnlink);
-}
-
-static void cnlink_disc_action(struct osmo_fsm_inst *fi, uint32_t event, void 
*data)
-{
-       struct hnbgw_cnlink *cnlink = (struct hnbgw_cnlink *)fi->priv;
-       switch (event) {
-
-       case CNLINK_EV_RX_RESET:
-               tx_reset_ack(cnlink);
-               cnlink_fsm_state_chg(fi, CNLINK_ST_CONN);
-               break;
-
-       case CNLINK_EV_RX_RESET_ACK:
-               cnlink_fsm_state_chg(fi, CNLINK_ST_CONN);
-               break;
-
-       default:
-               OSMO_ASSERT(false);
-       }
-}
-
-static void cnlink_conn_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state)
-{
-       struct hnbgw_cnlink *cnlink = (struct hnbgw_cnlink *)fi->priv;
-       if (prev_state != CNLINK_ST_CONN)
-               link_up(cnlink);
-}
-
-static void cnlink_conn_action(struct osmo_fsm_inst *fi, uint32_t event, void 
*data)
-{
-       struct hnbgw_cnlink *cnlink = (struct hnbgw_cnlink *)fi->priv;
-
-       switch (event) {
-
-       case CNLINK_EV_RX_RESET:
-               /* We were connected, but the remote side has restarted. */
-               link_lost(cnlink);
-               tx_reset_ack(cnlink);
-               link_up(cnlink);
-               break;
-
-       case CNLINK_EV_RX_RESET_ACK:
-               LOGPFSML(fi, LOGL_INFO, "Link is already up, ignoring RESET 
ACK\n");
-               break;
-
-       default:
-               OSMO_ASSERT(false);
-       }
-}
-
-static int cnlink_fsm_timer_cb(struct osmo_fsm_inst *fi)
-{
-       struct hnbgw_cnlink *cnlink = (struct hnbgw_cnlink *)fi->priv;
-
-       tx_reset(cnlink);
-
-       /* (re-)enter disconnect state to resend RESET after timeout. */
-       cnlink_fsm_state_chg(fi, CNLINK_ST_DISC);
-
-       /* Return 0 to not terminate the fsm */
-       return 0;
-}
-
-#define S(x) (1 << (x))
-
-static struct osmo_fsm_state cnlink_fsm_states[] = {
-       [CNLINK_ST_DISC] = {
-                    .name = "DISCONNECTED",
-                    .in_event_mask = 0
-                            | S(CNLINK_EV_RX_RESET)
-                            | S(CNLINK_EV_RX_RESET_ACK)
-                            ,
-                    .out_state_mask = 0
-                            | S(CNLINK_ST_DISC)
-                            | S(CNLINK_ST_CONN)
-                            ,
-                    .onenter = cnlink_disc_onenter,
-                    .action = cnlink_disc_action,
-                    },
-       [CNLINK_ST_CONN] = {
-                    .name = "CONNECTED",
-                    .in_event_mask = 0
-                            | S(CNLINK_EV_RX_RESET)
-                            | S(CNLINK_EV_RX_RESET_ACK)
-                            ,
-                    .out_state_mask = 0
-                            | S(CNLINK_ST_DISC)
-                            | S(CNLINK_ST_CONN)
-                            ,
-                    .onenter = cnlink_conn_onenter,
-                    .action = cnlink_conn_action,
-                    },
-};
-
-static struct osmo_fsm cnlink_fsm = {
-       .name = "cnlink",
-       .states = cnlink_fsm_states,
-       .num_states = ARRAY_SIZE(cnlink_fsm_states),
-       .log_subsys = DRANAP,
-       .timer_cb = cnlink_fsm_timer_cb,
-       .event_names = cnlink_fsm_event_names,
-};
-
-bool cnlink_is_conn_ready(const struct hnbgw_cnlink *cnlink)
-{
-       return cnlink->fi->state == CNLINK_ST_CONN;
-}
-
-void cnlink_resend_reset(struct hnbgw_cnlink *cnlink)
-{
-       /* Immediately (1ms) kick off reset sending mechanism */
-       osmo_fsm_inst_state_chg_ms(cnlink->fi, CNLINK_ST_DISC, 1, 0);
-}
-
-void cnlink_set_disconnected(struct hnbgw_cnlink *cnlink)
-{
-       /* Go to disconnected state, with the normal RESET timeout to re-send 
RESET. */
-       cnlink_fsm_state_chg(cnlink->fi, CNLINK_ST_DISC);
-}
-
-static __attribute__((constructor)) void cnlink_fsm_init(void)
-{
-       OSMO_ASSERT(osmo_fsm_register(&cnlink_fsm) == 0);
-}
-
-void cnlink_rx_reset_cmd(struct hnbgw_cnlink *cnlink)
-{
-       osmo_fsm_inst_dispatch(cnlink->fi, CNLINK_EV_RX_RESET, NULL);
-}
-
-void cnlink_rx_reset_ack(struct hnbgw_cnlink *cnlink)
-{
-       osmo_fsm_inst_dispatch(cnlink->fi, CNLINK_EV_RX_RESET_ACK, NULL);
-}
-
diff --git a/src/osmo-hnbgw/cnlink_fsm.c b/src/osmo-hnbgw/cnlink_fsm.c
new file mode 100644
index 0000000..acfbae4
--- /dev/null
+++ b/src/osmo-hnbgw/cnlink_fsm.c
@@ -0,0 +1,218 @@
+/* (C) 2023 by sysmocom s.f.m.c. GmbH <i...@sysmocom.de>
+ * All Rights Reserved
+ *
+ * Author: Neels Hofmeyr
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <osmocom/core/fsm.h>
+#include <osmocom/core/tdef.h>
+
+#include <osmocom/gsm/gsm23236.h>
+
+#include <osmocom/sigtran/sccp_helpers.h>
+
+#include <asn1c/asn1helpers.h>
+#include <osmocom/ranap/ranap_ies_defs.h>
+#include <osmocom/ranap/ranap_msg_factory.h>
+
+#include <osmocom/hnbgw/hnbgw.h>
+#include <osmocom/hnbgw/hnbgw_cn.h>
+#include <osmocom/hnbgw/tdefs.h>
+#include <osmocom/hnbgw/context_map.h>
+
+enum cnlink_fsm_state {
+       CNLINK_ST_DISC,
+       CNLINK_ST_CONN,
+};
+
+enum cnlink_fsm_event {
+       CNLINK_EV_RX_RESET,
+       CNLINK_EV_RX_RESET_ACK,
+};
+
+static const struct value_string cnlink_fsm_event_names[] = {
+       OSMO_VALUE_STRING(CNLINK_EV_RX_RESET),
+       OSMO_VALUE_STRING(CNLINK_EV_RX_RESET_ACK),
+       {}
+};
+
+static const struct osmo_tdef_state_timeout cnlink_timeouts[32] = {
+       [CNLINK_ST_DISC] = { .T = 4 },
+};
+
+#define cnlink_fsm_state_chg(FI, STATE) \
+       osmo_tdef_fsm_inst_state_chg(FI, STATE, \
+                                    cnlink_timeouts, \
+                                    hnbgw_T_defs, \
+                                    -1)
+
+static void link_up(struct hnbgw_cnlink *cnlink)
+{
+       LOGPFSML(cnlink->fi, LOGL_NOTICE, "link up\n");
+}
+
+static void link_lost(struct hnbgw_cnlink *cnlink)
+{
+       struct hnbgw_context_map *map, *map2;
+
+       LOGPFSML(cnlink->fi, LOGL_NOTICE, "link lost\n");
+
+       llist_for_each_entry_safe(map, map2, &cnlink->map_list, 
hnbgw_cnlink_entry)
+               context_map_cnlink_lost(map);
+}
+
+
+static void cnlink_disc_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state)
+{
+       struct hnbgw_cnlink *cnlink = (struct hnbgw_cnlink *)fi->priv;
+       if (prev_state == CNLINK_ST_CONN)
+               link_lost(cnlink);
+}
+
+static void cnlink_disc_action(struct osmo_fsm_inst *fi, uint32_t event, void 
*data)
+{
+       struct hnbgw_cnlink *cnlink = (struct hnbgw_cnlink *)fi->priv;
+       switch (event) {
+
+       case CNLINK_EV_RX_RESET:
+               hnbgw_cnlink_tx_ranap_reset_ack(cnlink);
+               cnlink_fsm_state_chg(fi, CNLINK_ST_CONN);
+               break;
+
+       case CNLINK_EV_RX_RESET_ACK:
+               cnlink_fsm_state_chg(fi, CNLINK_ST_CONN);
+               break;
+
+       default:
+               OSMO_ASSERT(false);
+       }
+}
+
+static void cnlink_conn_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state)
+{
+       struct hnbgw_cnlink *cnlink = (struct hnbgw_cnlink *)fi->priv;
+       if (prev_state != CNLINK_ST_CONN)
+               link_up(cnlink);
+}
+
+static void cnlink_conn_action(struct osmo_fsm_inst *fi, uint32_t event, void 
*data)
+{
+       struct hnbgw_cnlink *cnlink = (struct hnbgw_cnlink *)fi->priv;
+
+       switch (event) {
+
+       case CNLINK_EV_RX_RESET:
+               /* We were connected, but the remote side has restarted. */
+               link_lost(cnlink);
+               hnbgw_cnlink_tx_ranap_reset_ack(cnlink);
+               link_up(cnlink);
+               break;
+
+       case CNLINK_EV_RX_RESET_ACK:
+               LOGPFSML(fi, LOGL_INFO, "Link is already up, ignoring RESET 
ACK\n");
+               break;
+
+       default:
+               OSMO_ASSERT(false);
+       }
+}
+
+static int cnlink_fsm_timer_cb(struct osmo_fsm_inst *fi)
+{
+       struct hnbgw_cnlink *cnlink = (struct hnbgw_cnlink *)fi->priv;
+
+       hnbgw_cnlink_tx_ranap_reset(cnlink);
+
+       /* (re-)enter disconnect state to resend RESET after timeout. */
+       cnlink_fsm_state_chg(fi, CNLINK_ST_DISC);
+
+       /* Return 0 to not terminate the fsm */
+       return 0;
+}
+
+#define S(x) (1 << (x))
+
+static struct osmo_fsm_state cnlink_fsm_states[] = {
+       [CNLINK_ST_DISC] = {
+                    .name = "DISCONNECTED",
+                    .in_event_mask = 0
+                            | S(CNLINK_EV_RX_RESET)
+                            | S(CNLINK_EV_RX_RESET_ACK)
+                            ,
+                    .out_state_mask = 0
+                            | S(CNLINK_ST_DISC)
+                            | S(CNLINK_ST_CONN)
+                            ,
+                    .onenter = cnlink_disc_onenter,
+                    .action = cnlink_disc_action,
+                    },
+       [CNLINK_ST_CONN] = {
+                    .name = "CONNECTED",
+                    .in_event_mask = 0
+                            | S(CNLINK_EV_RX_RESET)
+                            | S(CNLINK_EV_RX_RESET_ACK)
+                            ,
+                    .out_state_mask = 0
+                            | S(CNLINK_ST_DISC)
+                            | S(CNLINK_ST_CONN)
+                            ,
+                    .onenter = cnlink_conn_onenter,
+                    .action = cnlink_conn_action,
+                    },
+};
+
+struct osmo_fsm cnlink_fsm = {
+       .name = "cnlink",
+       .states = cnlink_fsm_states,
+       .num_states = ARRAY_SIZE(cnlink_fsm_states),
+       .log_subsys = DRANAP,
+       .timer_cb = cnlink_fsm_timer_cb,
+       .event_names = cnlink_fsm_event_names,
+};
+
+bool cnlink_is_conn_ready(const struct hnbgw_cnlink *cnlink)
+{
+       return cnlink->fi->state == CNLINK_ST_CONN;
+}
+
+void cnlink_resend_reset(struct hnbgw_cnlink *cnlink)
+{
+       /* Immediately (1ms) kick off reset sending mechanism */
+       osmo_fsm_inst_state_chg_ms(cnlink->fi, CNLINK_ST_DISC, 1, 0);
+}
+
+void cnlink_set_disconnected(struct hnbgw_cnlink *cnlink)
+{
+       /* Go to disconnected state, with the normal RESET timeout to re-send 
RESET. */
+       cnlink_fsm_state_chg(cnlink->fi, CNLINK_ST_DISC);
+}
+
+static __attribute__((constructor)) void cnlink_fsm_init(void)
+{
+       OSMO_ASSERT(osmo_fsm_register(&cnlink_fsm) == 0);
+}
+
+void cnlink_rx_reset_cmd(struct hnbgw_cnlink *cnlink)
+{
+       osmo_fsm_inst_dispatch(cnlink->fi, CNLINK_EV_RX_RESET, NULL);
+}
+
+void cnlink_rx_reset_ack(struct hnbgw_cnlink *cnlink)
+{
+       osmo_fsm_inst_dispatch(cnlink->fi, CNLINK_EV_RX_RESET_ACK, NULL);
+}
+

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

Gerrit-MessageType: newchange
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I7e40beb13c2174e01d89915a28395a364709b321
Gerrit-Change-Number: 40262
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pes...@sysmocom.de>

Reply via email to