pespin has submitted this change. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/31797 )

Change subject: bscc_sccp: Avoid allocating conn_id 0x00FFFFFF
......................................................................

bscc_sccp: Avoid allocating conn_id 0x00FFFFFF

The 0x00FFFFFF source local reference is reserved in M3UA/SCCP, hence
avoid allocating a conn_id with that value since later on when reused as
a source local reference it would fail to be encoded.

Change-Id: I5c62bbfa89140d754edccb4404503cb70d5fde89
---
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/bsc_sccp.c
2 files changed, 31 insertions(+), 3 deletions(-)

Approvals:
  fixeria: Looks good to me, but someone else must approve
  daniel: Looks good to me, but someone else must approve
  Jenkins Builder: Verified
  pespin: Looks good to me, approved




diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index 268aa0c..cdf0f43 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -52,7 +52,7 @@
 #define GSM_NY1_DEFAULT ((unsigned long)((GSM_T3124_MAX + 
GSM_NY1_REQ_DELTA)/GSM_T3105_DEFAULT + 1))

 #define SCCP_CONN_ID_UNSET     0xFFFFFFFF
-#define SCCP_CONN_ID_MAX       0x00FFFFFF
+#define SCCP_CONN_ID_MAX       0x00FFFFFE

 struct mgcp_client_conf;
 struct mgcp_client;
diff --git a/src/osmo-bsc/bsc_sccp.c b/src/osmo-bsc/bsc_sccp.c
index 30d4c4c..431e194 100644
--- a/src/osmo-bsc/bsc_sccp.c
+++ b/src/osmo-bsc/bsc_sccp.c
@@ -21,6 +21,8 @@
  *
  */

+#include <osmocom/core/utils.h>
+
 #include <osmocom/bsc/gsm_data.h>
 #include <osmocom/bsc/bsc_msc_data.h>
 #include <osmocom/bsc/lb.h>
@@ -31,6 +33,17 @@
        static uint32_t next_id = 1;
        int i;

+       /* SUA: RFC3868 sec 3.10.4:
+       *    The source reference number is a 4 octet long integer.
+       *    This is allocated by the source SUA instance.
+       * M3UA/SCCP: ITU-T Q.713 sec 3.3:
+       *    The "source local reference" parameter field is a three-octet 
field containing a
+       *    reference number which is generated and used by the local node to 
identify the
+       *    connection section after the connection section is set up.
+       *    The coding "all ones" is reserved for future use.
+       * Hence, let's simply use 24 bit ids to fit all link types (excluding 
0x00ffffff).
+       */
+
        /* 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. */

@@ -39,8 +52,10 @@
                uint32_t conn_id = next_id;
                bool conn_id_already_used = false;

-               /* Optimized modulo operation using bitwise AND: */
-               next_id = (next_id + 1) & SCCP_CONN_ID_MAX;
+               /* Optimized modulo operation (% SCCP_CONN_ID_MAX) using 
bitwise AND plus CMP: */
+               next_id = (next_id + 1) & 0x00FFFFFF;
+               if (OSMO_UNLIKELY(next_id == 0x00FFFFFF))
+                       next_id = 0;

                llist_for_each_entry(conn, &bsc_gsmnet->subscr_conns, entry) {
                        if (conn->sccp.msc && conn->sccp.msc->a.sccp == sccp) {

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I5c62bbfa89140d754edccb4404503cb70d5fde89
Gerrit-Change-Number: 31797
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <[email protected]>
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-MessageType: merged

Reply via email to