Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/8055 )

Change subject: a_reset: cleanup + remove dead code
......................................................................

a_reset: cleanup + remove dead code

The function a_reset_free() is not used anywhere at the code. The
reason for this is that a BSC instance is never cleared once it
is started up. Also the timer number is not according to the spec.

- Remove a_reset_free()
- Fix timer identification number (T4)
- use fi->priv to hold context info
- Fix sourcecode formatting

Change-Id: I72095d52304c520e383755eee6c889bce492cbd4
Related: OS#3102
---
M include/osmocom/bsc/a_reset.h
M include/osmocom/bsc/bsc_msc_data.h
M src/libbsc/a_reset.c
M src/libbsc/bsc_subscr_conn_fsm.c
M src/osmo-bsc/osmo_bsc_api.c
M src/osmo-bsc/osmo_bsc_bssap.c
M src/osmo-bsc/osmo_bsc_sigtran.c
7 files changed, 77 insertions(+), 105 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/include/osmocom/bsc/a_reset.h b/include/osmocom/bsc/a_reset.h
index c01a8b0..6b6ce81 100644
--- a/include/osmocom/bsc/a_reset.h
+++ b/include/osmocom/bsc/a_reset.h
@@ -20,41 +20,17 @@

 #pragma once

-
-
-/* Reset context data (callbacks, state machine etc...) */
-struct a_reset_ctx {
-
-       /* FSM instance, which handles the reset procedure */
-       struct osmo_fsm_inst *fsm;
-
-       /* Connection failure counter. When this counter
-        * reaches a certain threshold, the reset procedure
-        * will be triggered */
-       int conn_loss_counter;
-
-       /* Callback function to be called when a connection
-        * failure is detected and a rest must occur */
-       void (*cb)(void *priv);
-
-       /* Privated data for the callback function */
-       void *priv;
-};
-
 /* Create and start state machine which handles the reset/reset-ack procedure 
*/
-struct a_reset_ctx *a_reset_alloc(const void *ctx, const char *name, void *cb, 
void *priv);
-
-/* Tear down state machine */
-void a_reset_free(struct a_reset_ctx *reset);
+struct osmo_fsm_inst *a_reset_alloc(void *ctx, const char *name, void *cb, 
void *priv);

 /* Confirm that we sucessfully received a reset acknowlege message */
-void a_reset_ack_confirm(struct a_reset_ctx *reset);
+void a_reset_ack_confirm(struct osmo_fsm_inst *reset_fsm);

 /* Report a failed connection */
-void a_reset_conn_fail(struct a_reset_ctx *reset);
+void a_reset_conn_fail(struct osmo_fsm_inst *reset_fsm);

 /* Report a successful connection */
-void a_reset_conn_success(struct a_reset_ctx *reset);
+void a_reset_conn_success(struct osmo_fsm_inst *reset_fsm);

 /* Check if we have a connection to a specified msc */
-bool a_reset_conn_ready(struct a_reset_ctx *reset);
+bool a_reset_conn_ready(struct osmo_fsm_inst *reset_fsm);
diff --git a/include/osmocom/bsc/bsc_msc_data.h 
b/include/osmocom/bsc/bsc_msc_data.h
index bedf412..345724d 100644
--- a/include/osmocom/bsc/bsc_msc_data.h
+++ b/include/osmocom/bsc/bsc_msc_data.h
@@ -124,7 +124,9 @@
                struct osmo_sccp_addr msc_addr;
                char *msc_addr_name;

-               struct a_reset_ctx *reset;
+               /* Pointer to the osmo-fsm that controls the
+                * BSSMAP RESET procedure */
+               struct osmo_fsm_inst *reset_fsm;
        } a;
 };

diff --git a/src/libbsc/a_reset.c b/src/libbsc/a_reset.c
index 1fccc5f..b8f8c8c 100644
--- a/src/libbsc/a_reset.c
+++ b/src/libbsc/a_reset.c
@@ -29,16 +29,31 @@
 #include <osmocom/bsc/bsc_msc_data.h>
 #include <osmocom/bsc/osmo_bsc_sigtran.h>

-#define RESET_RESEND_INTERVAL 2        /* sec */
-#define RESET_RESEND_TIMER_NO 1234     /* FIXME: dig out the real timer number 
*/
+#define RESET_RESEND_INTERVAL 2                /* sec */
+#define RESET_RESEND_TIMER_NO 4                /* See also 3GPP TS 48.008 
Chapter 3.1.4.1.3.1 */
 #define BAD_CONNECTION_THRESOLD 3      /* connection failures */

-enum fsm_states {
+/* Reset context data (callbacks, state machine etc...) */
+struct reset_ctx {
+       /* Connection failure counter. When this counter
+        * reaches a certain threshold, the reset procedure
+        * will be triggered */
+       int conn_loss_counter;
+
+       /* Callback function to be called when a connection
+        * failure is detected and a rest must occur */
+       void (*cb)(void *priv);
+
+       /* Privated data for the callback function */
+       void *priv;
+};
+
+enum reset_fsm_states {
        ST_DISC,                /* Disconnected from remote end */
        ST_CONN,                /* We have a confirmed connection */
 };

-enum fsm_evt {
+enum reset_fsm_evt {
        EV_RESET_ACK,           /* got reset acknowlegement from remote end */
        EV_N_DISCONNECT,        /* lost a connection */
        EV_N_CONNECT,           /* made a successful connection */
@@ -54,31 +69,30 @@
 /* Disconnected state */
 static void fsm_disc_cb(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
-       struct a_reset_ctx *reset = (struct a_reset_ctx *)data;
-       OSMO_ASSERT(reset);
-       OSMO_ASSERT(reset->fsm);
-       LOGPFSML(reset->fsm, LOGL_NOTICE, "SIGTRAN connection succeded.\n");
+       struct reset_ctx *reset_ctx = (struct reset_ctx *)fi->priv;
+       OSMO_ASSERT(reset_ctx);
+       LOGPFSML(fi, LOGL_NOTICE, "SIGTRAN connection succeded.\n");

-       reset->conn_loss_counter = 0;
+       reset_ctx->conn_loss_counter = 0;
        osmo_fsm_inst_state_chg(fi, ST_CONN, 0, 0);
 }

 /* Connected state */
 static void fsm_conn_cb(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
-       struct a_reset_ctx *reset = (struct a_reset_ctx *)data;
-       OSMO_ASSERT(reset);
+       struct reset_ctx *reset_ctx = (struct reset_ctx *)fi->priv;
+       OSMO_ASSERT(reset_ctx);

        switch (event) {
        case EV_N_DISCONNECT:
-               if (reset->conn_loss_counter >= BAD_CONNECTION_THRESOLD) {
-                       LOGPFSML(reset->fsm, LOGL_NOTICE, "SIGTRAN connection 
down, reconnecting...\n");
+               if (reset_ctx->conn_loss_counter >= BAD_CONNECTION_THRESOLD) {
+                       LOGPFSML(fi, LOGL_NOTICE, "SIGTRAN connection down, 
reconnecting...\n");
                        osmo_fsm_inst_state_chg(fi, ST_DISC, 
RESET_RESEND_INTERVAL, RESET_RESEND_TIMER_NO);
                } else
-                       reset->conn_loss_counter++;
+                       reset_ctx->conn_loss_counter++;
                break;
        case EV_N_CONNECT:
-               reset->conn_loss_counter = 0;
+               reset_ctx->conn_loss_counter = 0;
                break;
        }
 }
@@ -86,18 +100,18 @@
 /* Timer callback to retransmit the reset signal */
 static int fsm_reset_ack_timeout_cb(struct osmo_fsm_inst *fi)
 {
-       struct a_reset_ctx *reset = (struct a_reset_ctx *)fi->priv;
-       OSMO_ASSERT(reset->fsm);
+       struct reset_ctx *reset_ctx = (struct reset_ctx *)fi->priv;
+       OSMO_ASSERT(reset_ctx);

-       LOGPFSML(reset->fsm, LOGL_NOTICE, "(re)sending BSSMAP RESET 
message...\n");
+       LOGPFSML(fi, LOGL_NOTICE, "(re)sending BSSMAP RESET message...\n");

-       reset->cb(reset->priv);
+       reset_ctx->cb(reset_ctx->priv);

        osmo_fsm_inst_state_chg(fi, ST_DISC, RESET_RESEND_INTERVAL, 
RESET_RESEND_TIMER_NO);
        return 0;
 }

-static struct osmo_fsm_state fsm_states[] = {
+static struct osmo_fsm_state reset_fsm_states[] = {
        [ST_DISC] = {
                     .in_event_mask = (1 << EV_RESET_ACK),
                     .out_state_mask = (1 << ST_DISC) | (1 << ST_CONN),
@@ -115,96 +129,76 @@
 /* State machine definition */
 static struct osmo_fsm fsm = {
        .name = "A-RESET",
-       .states = fsm_states,
-       .num_states = ARRAY_SIZE(fsm_states),
+       .states = reset_fsm_states,
+       .num_states = ARRAY_SIZE(reset_fsm_states),
        .log_subsys = DMSC,
        .timer_cb = fsm_reset_ack_timeout_cb,
        .event_names = fsm_event_names,
 };

 /* Create and start state machine which handles the reset/reset-ack procedure 
*/
-struct a_reset_ctx *a_reset_alloc(const void *ctx, const char *name, void *cb, 
void *priv)
+struct osmo_fsm_inst *a_reset_alloc(void *ctx, const char *name, void *cb, 
void *priv)
 {
        OSMO_ASSERT(name);

-       struct a_reset_ctx *reset;
+       struct reset_ctx *reset_ctx;
+       struct osmo_fsm_inst *reset_fsm;

        /* Register the fsm description (if not already done) */
        if (osmo_fsm_find_by_name(fsm.name) != &fsm)
                osmo_fsm_register(&fsm);

        /* Allocate and configure a new fsm instance */
-       reset = talloc_zero(ctx, struct a_reset_ctx);
-       OSMO_ASSERT(reset);
-       reset->priv = priv;
-       reset->cb = cb;
-       reset->conn_loss_counter = 0;
-       reset->fsm = osmo_fsm_inst_alloc(&fsm, NULL, NULL, LOGL_DEBUG, name);
-       OSMO_ASSERT(reset->fsm);
-       reset->fsm->priv = reset;
+       reset_ctx = talloc_zero(ctx, struct reset_ctx);
+       OSMO_ASSERT(reset_ctx);
+       reset_ctx->priv = priv;
+       reset_ctx->cb = cb;
+       reset_ctx->conn_loss_counter = 0;
+       reset_fsm = osmo_fsm_inst_alloc(&fsm, ctx, reset_ctx, LOGL_DEBUG, name);
+       OSMO_ASSERT(reset_fsm);

        /* kick off reset-ack sending mechanism */
-       osmo_fsm_inst_state_chg(reset->fsm, ST_DISC, RESET_RESEND_INTERVAL, 
RESET_RESEND_TIMER_NO);
+       osmo_fsm_inst_state_chg(reset_fsm, ST_DISC, RESET_RESEND_INTERVAL, 
RESET_RESEND_TIMER_NO);

-       return reset;
-}
-
-/* Tear down state machine */
-void a_reset_free(struct a_reset_ctx *reset)
-{
-       OSMO_ASSERT(reset);
-       OSMO_ASSERT(reset->fsm);
-
-       osmo_fsm_inst_free(reset->fsm);
-       reset->fsm = NULL;
-
-       memset(reset, 0, sizeof(*reset));
-       talloc_free(reset);
+       return reset_fsm;
 }

 /* Confirm that we sucessfully received a reset acknowlege message */
-void a_reset_ack_confirm(struct a_reset_ctx *reset)
+void a_reset_ack_confirm(struct osmo_fsm_inst *reset_fsm)
 {
-       OSMO_ASSERT(reset);
-       OSMO_ASSERT(reset->fsm);
-
-       osmo_fsm_inst_dispatch(reset->fsm, EV_RESET_ACK, reset);
+       OSMO_ASSERT(reset_fsm);
+       osmo_fsm_inst_dispatch(reset_fsm, EV_RESET_ACK, NULL);
 }

 /* Report a failed connection */
-void a_reset_conn_fail(struct a_reset_ctx *reset)
+void a_reset_conn_fail(struct osmo_fsm_inst *reset_fsm)
 {
        /* If no reset context is supplied, just drop the info */
-       if (!reset)
+       if (!reset_fsm)
                return;

-       OSMO_ASSERT(reset->fsm);
-
-       osmo_fsm_inst_dispatch(reset->fsm, EV_N_DISCONNECT, reset);
+       osmo_fsm_inst_dispatch(reset_fsm, EV_N_DISCONNECT, NULL);
 }

 /* Report a successful connection */
-void a_reset_conn_success(struct a_reset_ctx *reset)
+void a_reset_conn_success(struct osmo_fsm_inst *reset_fsm)
 {
        /* If no reset context is supplied, just drop the info */
-       if (!reset)
+       if (!reset_fsm)
                return;

-       OSMO_ASSERT(reset->fsm);
-
-       osmo_fsm_inst_dispatch(reset->fsm, EV_N_CONNECT, reset);
+       osmo_fsm_inst_dispatch(reset_fsm, EV_N_CONNECT, NULL);
 }

 /* Check if we have a connection to a specified msc */
-bool a_reset_conn_ready(struct a_reset_ctx *reset)
+bool a_reset_conn_ready(struct osmo_fsm_inst *reset_fsm)
 {
        /* If no reset context is supplied, we assume that
         * the connection can't be ready! */
-       if (!reset)
+       if (!reset_fsm)
                return false;

-       OSMO_ASSERT(reset->fsm);
-       if (reset->fsm->state == ST_CONN)
+       if (reset_fsm->state == ST_CONN)
                return true;

        return false;
diff --git a/src/libbsc/bsc_subscr_conn_fsm.c b/src/libbsc/bsc_subscr_conn_fsm.c
index 7f53f1b..3e0ccc0 100644
--- a/src/libbsc/bsc_subscr_conn_fsm.c
+++ b/src/libbsc/bsc_subscr_conn_fsm.c
@@ -1075,7 +1075,7 @@
                 * could indicate a bad SCCP connection. We now inform the the
                 * FSM that controls the BSSMAP reset about the event. Maybe
                 * a BSSMAP reset is necessary. */
-               a_reset_conn_fail(conn->sccp.msc->a.reset);
+               a_reset_conn_fail(conn->sccp.msc->a.reset_fsm);

                /* Since we could not reach the MSC, we give up and terminate
                 * the FSM instance now (N-DISCONNET.req is sent in
diff --git a/src/osmo-bsc/osmo_bsc_api.c b/src/osmo-bsc/osmo_bsc_api.c
index 239bb54..8c16bde 100644
--- a/src/osmo-bsc/osmo_bsc_api.c
+++ b/src/osmo-bsc/osmo_bsc_api.c
@@ -44,7 +44,7 @@
                return false;

        /* Reset procedure not (yet) executed */
-       if (a_reset_conn_ready(conn->sccp.msc->a.reset) == false)
+       if (a_reset_conn_ready(conn->sccp.msc->a.reset_fsm) == false)
                return false;

        return true;
diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c
index bfa4091..24dbc95 100644
--- a/src/osmo-bsc/osmo_bsc_bssap.c
+++ b/src/osmo-bsc/osmo_bsc_bssap.c
@@ -208,7 +208,7 @@

        /* Inform the FSM that controls the RESET/RESET-ACK procedure
         * that we have successfully received the reset-ack message */
-       a_reset_ack_confirm(msc->a.reset);
+       a_reset_ack_confirm(msc->a.reset_fsm);

        return 0;
 }
diff --git a/src/osmo-bsc/osmo_bsc_sigtran.c b/src/osmo-bsc/osmo_bsc_sigtran.c
index 1a31a7c..e3d4829 100644
--- a/src/osmo-bsc/osmo_bsc_sigtran.c
+++ b/src/osmo-bsc/osmo_bsc_sigtran.c
@@ -205,7 +205,7 @@
                /* Incoming data is a sign of a vital connection */
                conn = get_bsc_conn_by_conn_id(scu_prim->u.data.conn_id);
                if (conn) {
-                       a_reset_conn_success(conn->sccp.msc->a.reset);
+                       a_reset_conn_success(conn->sccp.msc->a.reset_fsm);
                        handle_data_from_msc(conn, oph->msg);
                }
                break;
@@ -249,7 +249,7 @@
        LOGP(DMSC, LOGL_NOTICE, "Initializing resources for new SIGTRAN 
connection to MSC: %s...\n",
             osmo_sccp_addr_name(ss7, &msc->a.msc_addr));

-       if (a_reset_conn_ready(msc->a.reset) == false) {
+       if (a_reset_conn_ready(msc->a.reset_fsm) == false) {
                LOGP(DMSC, LOGL_ERROR, "MSC is not connected. Dropping.\n");
                return BSC_CON_REJECT_NO_LINK;
        }
@@ -279,7 +279,7 @@

        msc = conn->sccp.msc;

-       if (a_reset_conn_ready(msc->a.reset) == false) {
+       if (a_reset_conn_ready(msc->a.reset_fsm) == false) {
                LOGP(DMSC, LOGL_ERROR, "MSC is not connected. Dropping.\n");
                return -EINVAL;
        }
@@ -333,7 +333,7 @@
        } else
                LOGP(DMSC, LOGL_ERROR, "Tx MSC (message too short)\n");

-       if (a_reset_conn_ready(msc->a.reset) == false) {
+       if (a_reset_conn_ready(msc->a.reset_fsm) == false) {
                LOGP(DMSC, LOGL_ERROR, "MSC is not connected. Dropping.\n");
                return -EINVAL;
        }
@@ -487,8 +487,8 @@
                        return -EINVAL;

                /* Start MSC-Reset procedure */
-               msc->a.reset = a_reset_alloc(msc, msc_name, 
osmo_bsc_sigtran_reset_cb, msc);
-               if (!msc->a.reset)
+               msc->a.reset_fsm = a_reset_alloc(msc, msc_name, 
osmo_bsc_sigtran_reset_cb, msc);
+               if (!msc->a.reset_fsm)
                        return -EINVAL;

                /* If we have detected that the SS7 configuration of the MSC we 
have just initalized

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I72095d52304c520e383755eee6c889bce492cbd4
Gerrit-Change-Number: 8055
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <pma...@sysmocom.de>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>

Reply via email to