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


Change subject: Clarify type and values of sccp.conn_id
......................................................................

Clarify type and values of sccp.conn_id

Currently, the conn_id is allocated in a range 0..0xffffff by
bsc_sccp_inst_next_conn_id(), and -1 means it is unset.

This means allocation expects "int" to be at least 32 bits integer,
in order to fit 24 bits for 0..0xffffff plus the -1.
Hence, let's define the variable as uint32_t, as already done in
libosmo-sccp. Use last value 0xFFFFFFFF ((uint32_t)-1) and avoid playing
with the value being unsigned sometimes and signed only for "unset"
value.

The value is actually already handled as unsigned (printed with %u) in
most places.

Change-Id: If019bcbc1e28929fe8d981fef9103835fc6ead2e
---
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/bsc_sccp.c
M src/osmo-bsc/bsc_subscr_conn_fsm.c
M src/osmo-bsc/lb.c
M src/osmo-bsc/osmo_bsc_sigtran.c
5 files changed, 42 insertions(+), 19 deletions(-)



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

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index 533be93..1d6a266 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -329,7 +329,7 @@
                struct bsc_msc_data *msc;

                /* Sigtran connection ID */
-               int conn_id;
+               uint32_t conn_id; /* 0xFFFFFFFF (-1) unset, 0-0x00FFFFFF (24 
bit) set */
                enum subscr_sccp_state state;
        } sccp;

@@ -379,7 +379,7 @@

                /* Lb interface to the SMLC: BSSMAP-LE/SCCP connection 
associated with this subscriber */
                struct {
-                       int conn_id;
+                       uint32_t conn_id; /* 0xFFFFFFFF unset, 0-0x00FFFFFF (24 
bit) set */
                        enum subscr_sccp_state state;
                } lb;
        } lcs;
@@ -1022,7 +1022,7 @@
 enum gsm48_rr_cause bsc_gsm48_rr_cause_from_gsm0808_cause(enum gsm0808_cause 
c);
 enum gsm48_rr_cause bsc_gsm48_rr_cause_from_rsl_cause(uint8_t c);

-int bsc_sccp_inst_next_conn_id(struct osmo_sccp_instance *sccp);
+uint32_t bsc_sccp_inst_next_conn_id(struct osmo_sccp_instance *sccp);

 /* Interference Measurement Parameters */
 struct gsm_interf_meas_params {
diff --git a/src/osmo-bsc/bsc_sccp.c b/src/osmo-bsc/bsc_sccp.c
index 0cd1dc9..67834a5 100644
--- a/src/osmo-bsc/bsc_sccp.c
+++ b/src/osmo-bsc/bsc_sccp.c
@@ -26,7 +26,7 @@
 #include <osmocom/bsc/lb.h>

 /* We need an unused SCCP conn_id across all SCCP users. */
-int bsc_sccp_inst_next_conn_id(struct osmo_sccp_instance *sccp)
+uint32_t bsc_sccp_inst_next_conn_id(struct osmo_sccp_instance *sccp)
 {
        static uint32_t next_id = 1;
        int i;
@@ -34,11 +34,11 @@
        /* This looks really suboptimal, but in most cases the static next_id 
should indicate exactly the next unused
         * conn_id, and we only iterate all conns once to make super sure that 
it is not already in use. */

-       for (i = 0; i < 0xFFFFFF; i++) {
+       for (i = 0; i < 0x00FFFFFF; i++) {
                struct gsm_subscriber_connection *conn;
                uint32_t conn_id = next_id;
                bool conn_id_already_used = false;
-               next_id = (next_id + 1) & 0xffffff;
+               next_id = (next_id + 1) & 0x00FFFFFF;

                llist_for_each_entry(conn, &bsc_gsmnet->subscr_conns, entry) {
                        if (conn->sccp.msc && conn->sccp.msc->a.sccp == sccp) {
@@ -60,5 +60,5 @@
                if (!conn_id_already_used)
                        return conn_id;
        }
-       return -1;
+       return 0xFFFFFFFF;
 }
diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c 
b/src/osmo-bsc/bsc_subscr_conn_fsm.c
index ebfbe82..14d1575 100644
--- a/src/osmo-bsc/bsc_subscr_conn_fsm.c
+++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c
@@ -1226,7 +1226,7 @@
        conn->network = net;
        INIT_LLIST_HEAD(&conn->dtap_queue);
        INIT_LLIST_HEAD(&conn->hodec2.penalty_timers);
-       conn->sccp.conn_id = -1;
+       conn->sccp.conn_id = 0xFFFFFFFF; /* mark as unset */

        /* Default clear cause (on RR translates to 
GSM48_RR_CAUSE_ABNORMAL_UNSPEC) */
        conn->clear_cause = GSM0808_CAUSE_EQUIPMENT_FAILURE;
diff --git a/src/osmo-bsc/lb.c b/src/osmo-bsc/lb.c
index 6f3cfac..ce28ebc 100644
--- a/src/osmo-bsc/lb.c
+++ b/src/osmo-bsc/lb.c
@@ -32,7 +32,7 @@
 #include <osmocom/bsc/lcs_loc_req.h>
 #include <osmocom/bsc/bssmap_reset.h>

-static struct gsm_subscriber_connection *get_bsc_conn_by_lb_conn_id(int 
conn_id)
+static struct gsm_subscriber_connection *get_bsc_conn_by_lb_conn_id(uint32_t 
conn_id)
 {
        struct gsm_subscriber_connection *conn;

@@ -229,7 +229,7 @@
 static int lb_open_conn(struct gsm_subscriber_connection *conn, struct msgb 
*msg)
 {
        struct osmo_ss7_instance *ss7;
-       int conn_id;
+       uint32_t conn_id;
        int rc;

        OSMO_ASSERT(conn);
@@ -242,14 +242,14 @@
        }

        conn_id = bsc_sccp_inst_next_conn_id(bsc_gsmnet->smlc->sccp);
-       if (conn_id < 0) {
+       if (conn_id == 0xFFFFFFFF) {
                LOGPFSMSL(conn->fi, DLCS, LOGL_ERROR, "Unable to allocate SCCP 
Connection ID for BSSMAP-LE to SMLC\n");
                return -ENOSPC;
        }
        conn->lcs.lb.conn_id = conn_id;
        ss7 = osmo_ss7_instance_find(bsc_gsmnet->smlc->cs7_instance);
        OSMO_ASSERT(ss7);
-       LOGPFSMSL(conn->fi, DLCS, LOGL_INFO, "Opening new SCCP connection 
(id=%i) to SMLC: %s\n", conn_id,
+       LOGPFSMSL(conn->fi, DLCS, LOGL_INFO, "Opening new SCCP connection 
(id=%u) to SMLC: %s\n", conn_id,
                  osmo_sccp_addr_name(ss7, &bsc_gsmnet->smlc->smlc_addr));

        rc = osmo_sccp_tx_conn_req_msg(bsc_gsmnet->smlc->sccp_user, conn_id, 
&bsc_gsmnet->smlc->bsc_addr,
diff --git a/src/osmo-bsc/osmo_bsc_sigtran.c b/src/osmo-bsc/osmo_bsc_sigtran.c
index 04c2e99..d765675 100644
--- a/src/osmo-bsc/osmo_bsc_sigtran.c
+++ b/src/osmo-bsc/osmo_bsc_sigtran.c
@@ -46,9 +46,9 @@
 #define DEFAULT_ASP_REMOTE_IP "localhost"

 /* Helper function to Check if the given connection id is already assigned */
-static struct gsm_subscriber_connection *get_bsc_conn_by_conn_id(int conn_id)
+static struct gsm_subscriber_connection *get_bsc_conn_by_conn_id(uint32_t 
conn_id)
 {
-       conn_id &= 0xFFFFFF;
+       conn_id &= 0x00FFFFFF;
        struct gsm_subscriber_connection *conn;

        llist_for_each_entry(conn, &bsc_gsmnet->subscr_conns, entry) {
@@ -317,13 +317,14 @@
 {
        struct osmo_ss7_instance *ss7;
        struct bsc_msc_data *msc;
-       int conn_id;
+       uint32_t conn_id;
        int rc;

        OSMO_ASSERT(conn);
        OSMO_ASSERT(msg);
        OSMO_ASSERT(conn->sccp.msc);
-       OSMO_ASSERT(conn->sccp.conn_id == -1);
+       /* make sure conn_id is so far unset: */
+       OSMO_ASSERT(conn->sccp.conn_id == 0xFFFFFFFF);

        msc = conn->sccp.msc;

@@ -333,14 +334,14 @@
        }

        conn->sccp.conn_id = conn_id = 
bsc_sccp_inst_next_conn_id(conn->sccp.msc->a.sccp);
-       if (conn->sccp.conn_id < 0) {
+       if (conn->sccp.conn_id == 0xFFFFFFFF) {
                LOGP(DMSC, LOGL_ERROR, "Unable to allocate SCCP Connection 
ID\n");
                return -1;
        }
-       LOGP(DMSC, LOGL_DEBUG, "Allocated new connection id: %d\n", 
conn->sccp.conn_id);
+       LOGP(DMSC, LOGL_DEBUG, "Allocated new connection id: %u\n", 
conn->sccp.conn_id);
        ss7 = osmo_ss7_instance_find(msc->a.cs7_instance);
        OSMO_ASSERT(ss7);
-       LOGP(DMSC, LOGL_INFO, "Opening new SCCP connection (id=%i) to MSC %d: 
%s\n", conn_id,
+       LOGP(DMSC, LOGL_INFO, "Opening new SCCP connection (id=%u) to MSC %d: 
%s\n", conn_id,
             msc->nr, osmo_sccp_addr_name(ss7, &msc->a.msc_addr));

        rc = osmo_sccp_tx_conn_req_msg(msc->a.sccp_user, conn_id, 
&msc->a.bsc_addr,

--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31796
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: If019bcbc1e28929fe8d981fef9103835fc6ead2e
Gerrit-Change-Number: 31796
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <[email protected]>
Gerrit-MessageType: newchange

Reply via email to