March 2023 - gerrit-log - lists.osmocom.org

[M] Change in libosmocore[master]: GSMTAP: allow configuring src IP for log messages
by msuraev 13 Mar '23

13 Mar '23

Attention is currently required from: osmith, neels, pespin, fixeria. msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31513 ) Change subject: GSMTAP: allow configuring src IP for log messages ...................................................................... Patch Set 13: (2 comments) File src/core/logging_gsmtap.c: https://gerrit.osmocom.org/c/libosmocore/+/31513/comment/a53b04e1_cdd8449d PS12, Line 186: target->tgt_gsmtap.src_addr = talloc_strdup(target, src); > Yes, the actionable suggestion is that you verify everything is fine since your are submitting this […] See - https://gerrit.osmocom.org/c/libosmocore/+/31868 File utils/osmo-stat-dummy/osmo-stat-dummy.cfg: https://gerrit.osmocom.org/c/libosmocore/+/31513/comment/1fc4d995_15ffd122 PS4, Line 1: log gsmtap 127.0.0.69 127.6.9.1 > Yes. […] The purpose is described in the README - it's to test statsd and rate counters. Having any kind of logging setup does not affect it in any way. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31513 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I9000269ce5b3dce1e757271b7c42e77b68d38f25 Gerrit-Change-Number: 31513 Gerrit-PatchSet: 13 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Mar 2023 15:44:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: logging: print talloc report on exit from vty test
by msuraev 13 Mar '23

13 Mar '23

msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31868 ) Change subject: logging: print talloc report on exit from vty test ...................................................................... Set Ready For Review -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31868 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I5739bceb90d36164fd4cbf21242bbe26bd1e7075 Gerrit-Change-Number: 31868 Gerrit-PatchSet: 3 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Mon, 13 Mar 2023 15:39:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in osmo-bsc[master]: bsc_subscriber: Drop unused function bsc_subscr_find_by_mi()
by pespin 13 Mar '23

13 Mar '23

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31869 ) Change subject: bsc_subscriber: Drop unused function bsc_subscr_find_by_mi() ...................................................................... bsc_subscriber: Drop unused function bsc_subscr_find_by_mi() Change-Id: I15a51fa8a5c30fd92b34ebb366d0a805f8902cd0 --- M include/osmocom/bsc/bsc_subscriber.h M src/osmo-bsc/bsc_subscriber.c 2 files changed, 9 insertions(+), 19 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/69/31869/1 diff --git a/include/osmocom/bsc/bsc_subscriber.h b/include/osmocom/bsc/bsc_subscriber.h index 2beb998..da2115f 100644 --- a/include/osmocom/bsc/bsc_subscriber.h +++ b/include/osmocom/bsc/bsc_subscriber.h @@ -49,8 +49,6 @@ struct bsc_subscr *bsc_subscr_find_by_tmsi(struct llist_head *list, uint32_t tmsi, const char *use_token); -struct bsc_subscr *bsc_subscr_find_by_mi(struct llist_head *list, const struct osmo_mobile_identity *mi, - const char *use_token); void bsc_subscr_set_imsi(struct bsc_subscr *bsub, const char *imsi); void bsc_subscr_set_imei(struct bsc_subscr *bsub, const char *imei); diff --git a/src/osmo-bsc/bsc_subscriber.c b/src/osmo-bsc/bsc_subscriber.c index 9f4e82b..31dfc05 100644 --- a/src/osmo-bsc/bsc_subscriber.c +++ b/src/osmo-bsc/bsc_subscriber.c @@ -139,23 +139,6 @@ return NULL; } -struct bsc_subscr *bsc_subscr_find_by_mi(struct llist_head *list, const struct osmo_mobile_identity *mi, - const char *use_token) -{ - if (!mi) - return NULL; - switch (mi->type) { - case GSM_MI_TYPE_IMSI: - return bsc_subscr_find_by_imsi(list, mi->imsi, use_token); - case GSM_MI_TYPE_IMEI: - return bsc_subscr_find_by_imei(list, mi->imei, use_token); - case GSM_MI_TYPE_TMSI: - return bsc_subscr_find_by_tmsi(list, mi->tmsi, use_token); - default: - return NULL; - } -} - void bsc_subscr_set_imsi(struct bsc_subscr *bsub, const char *imsi) { if (!bsub) -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31869 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I15a51fa8a5c30fd92b34ebb366d0a805f8902cd0 Gerrit-Change-Number: 31869 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in osmo-bsc[master]: bscc_sccp: Avoid allocating conn_id 0x00FFFFFF
by pespin 13 Mar '23

13 Mar '23

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 <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in osmo-bsc[master]: get_bsc_conn_by_conn_id(): Properly match sccp_instance
by pespin 13 Mar '23

13 Mar '23

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31798 ) Change subject: get_bsc_conn_by_conn_id(): Properly match sccp_instance ...................................................................... get_bsc_conn_by_conn_id(): Properly match sccp_instance Function bsc_sccp_inst_next_conn_id() allocating conn_id creates address spaces based on sccp_instance, aka conn_id values can be reused given the sccp_instance (MSC) is different. Hence, when looking up a bsc_conn based on a conn_id, it must also match the sccp_instance, otherwise a bsc_conn from another MSC could be returned. Change-Id: I80a54bdec3973917e88483a62bfc2e968b8c0490 --- M src/osmo-bsc/osmo_bsc_sigtran.c 1 file changed, 28 insertions(+), 7 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/src/osmo-bsc/osmo_bsc_sigtran.c b/src/osmo-bsc/osmo_bsc_sigtran.c index 748b3f0..1d4fe82 100644 --- a/src/osmo-bsc/osmo_bsc_sigtran.c +++ b/src/osmo-bsc/osmo_bsc_sigtran.c @@ -46,14 +46,18 @@ #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(uint32_t conn_id) +static struct gsm_subscriber_connection *get_bsc_conn_by_conn_id(const struct osmo_sccp_user *scu, uint32_t conn_id) { conn_id &= SCCP_CONN_ID_MAX; struct gsm_subscriber_connection *conn; + const struct osmo_sccp_instance *sccp = osmo_sccp_get_sccp(scu); llist_for_each_entry(conn, &bsc_gsmnet->subscr_conns, entry) { - if (conn->sccp.conn_id == conn_id) - return conn; + if (conn->sccp.msc && conn->sccp.msc->a.sccp != sccp) + continue; + if (conn->sccp.conn_id != conn_id) + continue; + return conn; } return NULL; @@ -167,7 +171,7 @@ struct gsm_subscriber_connection *conn; int rc = 0; - conn = get_bsc_conn_by_conn_id(scu_prim->u.connect.conn_id); + conn = get_bsc_conn_by_conn_id(scu, scu_prim->u.connect.conn_id); if (conn) { LOGP(DMSC, LOGL_NOTICE, "(calling_addr=%s conn_id=%u) N-CONNECT.ind with already used conn_id, ignoring\n", @@ -231,7 +235,7 @@ /* Handle outbound connection confirmation */ DEBUGP(DMSC, "N-CONNECT.cnf(%u, %s)\n", scu_prim->u.connect.conn_id, osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg))); - conn = get_bsc_conn_by_conn_id(scu_prim->u.connect.conn_id); + conn = get_bsc_conn_by_conn_id(scu, scu_prim->u.connect.conn_id); if (conn) { osmo_fsm_inst_dispatch(conn->fi, GSCON_EV_A_CONN_CFM, scu_prim); conn->sccp.state = SUBSCR_SCCP_ST_CONNECTED; @@ -250,7 +254,7 @@ osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg))); /* Incoming data is a sign of a vital connection */ - conn = get_bsc_conn_by_conn_id(scu_prim->u.data.conn_id); + conn = get_bsc_conn_by_conn_id(scu, scu_prim->u.data.conn_id); if (conn) { a_reset_conn_success(conn->sccp.msc); handle_data_from_msc(conn, oph->msg); @@ -262,7 +266,7 @@ osmo_hexdump(msgb_l2(oph->msg), msgb_l2len(oph->msg)), scu_prim->u.disconnect.cause); /* indication of disconnect */ - conn = get_bsc_conn_by_conn_id(scu_prim->u.disconnect.conn_id); + conn = get_bsc_conn_by_conn_id(scu, scu_prim->u.disconnect.conn_id); if (conn) { conn->sccp.state = SUBSCR_SCCP_ST_NONE; if (msgb_l2len(oph->msg) > 0) -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31798 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I80a54bdec3973917e88483a62bfc2e968b8c0490 Gerrit-Change-Number: 31798 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmo-bsc[master]: Clarify type and values of sccp.conn_id
by pespin 13 Mar '23

13 Mar '23

pespin has submitted this change. ( 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, 52 insertions(+), 21 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 533be93..268aa0c 100644 --- a/include/osmocom/bsc/gsm_data.h +++ b/include/osmocom/bsc/gsm_data.h @@ -51,6 +51,9 @@ * with delta = time between expiration of T3124 and receiving HANDOVER FAILURE by the serving BSC. */ #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 + struct mgcp_client_conf; struct mgcp_client; struct gsm0808_cell_id; @@ -328,8 +331,10 @@ /* SCCP connection related */ struct bsc_msc_data *msc; - /* Sigtran connection ID */ - int conn_id; + /* Sigtran connection ID: + * if set: Range (0..SCCP_CONN_ID_MAX) (24 bit) + * if unset: SCCP_CONN_ID_UNSET (-1) if unset */ + uint32_t conn_id; enum subscr_sccp_state state; } sccp; @@ -379,7 +384,10 @@ /* Lb interface to the SMLC: BSSMAP-LE/SCCP connection associated with this subscriber */ struct { - int conn_id; + /* Sigtran connection ID: + * if set: Range (0..SCCP_CONN_ID_MAX) (24 bit) + * if unset: SCCP_CONN_ID_UNSET (-1) if unset */ + uint32_t conn_id; enum subscr_sccp_state state; } lb; } lcs; @@ -1022,7 +1030,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..30d4c4c 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,13 @@ /* 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 < SCCP_CONN_ID_MAX; i++) { struct gsm_subscriber_connection *conn; uint32_t conn_id = next_id; bool conn_id_already_used = false; - next_id = (next_id + 1) & 0xffffff; + + /* Optimized modulo operation using bitwise AND: */ + next_id = (next_id + 1) & SCCP_CONN_ID_MAX; llist_for_each_entry(conn, &bsc_gsmnet->subscr_conns, entry) { if (conn->sccp.msc && conn->sccp.msc->a.sccp == sccp) { @@ -60,5 +62,5 @@ if (!conn_id_already_used) return conn_id; } - return -1; + return SCCP_CONN_ID_UNSET; } diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c b/src/osmo-bsc/bsc_subscr_conn_fsm.c index ebfbe82..e37f030 100644 --- a/src/osmo-bsc/bsc_subscr_conn_fsm.c +++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c @@ -1226,8 +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 = SCCP_CONN_ID_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..c34d88e 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 == SCCP_CONN_ID_UNSET) { 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 88ee01c..748b3f0 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 &= SCCP_CONN_ID_MAX; struct gsm_subscriber_connection *conn; llist_for_each_entry(conn, &bsc_gsmnet->subscr_conns, entry) { @@ -317,13 +317,13 @@ { 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); + OSMO_ASSERT(conn->sccp.conn_id == SCCP_CONN_ID_UNSET); msc = conn->sccp.msc; @@ -333,14 +333,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 == SCCP_CONN_ID_UNSET) { 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: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in osmo-bsc[master]: get_bsc_conn_by_conn_id(): Properly match sccp_instance
by pespin 13 Mar '23

13 Mar '23

Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31798 ) Change subject: get_bsc_conn_by_conn_id(): Properly match sccp_instance ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31798 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I80a54bdec3973917e88483a62bfc2e968b8c0490 Gerrit-Change-Number: 31798 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Mar 2023 15:26:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-bsc[master]: bscc_sccp: Avoid allocating conn_id 0x00FFFFFF
by pespin 13 Mar '23

13 Mar '23

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31797 ) Change subject: bscc_sccp: Avoid allocating conn_id 0x00FFFFFF ...................................................................... Patch Set 4: Code-Review+2 -- 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 <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Mar 2023 15:26:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-bsc[master]: Clarify type and values of sccp.conn_id
by pespin 13 Mar '23

13 Mar '23

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31796 ) Change subject: Clarify type and values of sccp.conn_id ...................................................................... Patch Set 4: Code-Review+2 -- 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: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Mar 2023 15:26:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: select: Optimize osmo_fd_get_by_fd
by pespin 13 Mar '23

13 Mar '23

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31808 ) Change subject: select: Optimize osmo_fd_get_by_fd ...................................................................... select: Optimize osmo_fd_get_by_fd Optimize osmo_fd_get_by_fd() from O(n) to O(k) by means of allocating dynamically growing array. Make use from the fact that the kernel always tries to use the smallest possible unused value when allocating a new fd. Change-Id: I8b71547df8bed84192cb160479fa3debf9b7eade --- M src/core/select.c 1 file changed, 59 insertions(+), 8 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve daniel: Looks good to me, but someone else must approve pespin: Looks good to me, approved osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/core/select.c b/src/core/select.c index 72f794f..f983601 100644 --- a/src/core/select.c +++ b/src/core/select.c @@ -54,6 +54,39 @@ static __thread struct llist_head osmo_fds; /* TLS cannot use LLIST_HEAD() */ static __thread int unregistered_count; +/* Array of struct osmo_fd * (size "max_fd") ordered by "ofd->fd" */ +static __thread struct { + struct osmo_fd **table; + unsigned int size; +} osmo_fd_lookup; + +static void osmo_fd_lookup_table_extend(unsigned int new_max_fd) +{ + unsigned int min_new_size; + unsigned int pw2; + unsigned int new_size; + + /* First calculate the minimally required new size of the array in bytes: */ + min_new_size = (new_max_fd + 1) * sizeof(struct osmo_fd *); + /* Now find the lower power of two of min_new_size (in bytes): */ + pw2 = 32 - __builtin_clz(min_new_size); + /* get next (upper side) power of 2: */ + pw2++; + OSMO_ASSERT(pw2 <= 31); /* Avoid shifting more than 31 bits */ + new_size = 1 << (pw2 + 1); + + /* Let's make it at least 1024B, to avoid reallocating quickly at startup */ + if (new_size < 1024) + new_size = 1024; + if (new_size > osmo_fd_lookup.size) { + uint8_t *ptr = talloc_realloc_size(OTC_GLOBAL, osmo_fd_lookup.table, new_size); + OSMO_ASSERT(ptr); + memset(ptr + osmo_fd_lookup.size, 0, new_size - osmo_fd_lookup.size); + osmo_fd_lookup.table = (struct osmo_fd **)ptr; + osmo_fd_lookup.size = new_size; + } +} + #ifndef FORCE_IO_SELECT struct poll_state { /* array of pollfd */ @@ -142,8 +175,10 @@ return flags; /* Register FD */ - if (fd->fd > maxfd) + if (fd->fd > maxfd) { maxfd = fd->fd; + osmo_fd_lookup_table_extend(maxfd); + } #ifdef OSMO_FD_CHECK if (osmo_fd_is_registered(fd)) { @@ -166,6 +201,7 @@ #endif /* FORCE_IO_SELECT */ llist_add_tail(&fd->list, &osmo_fds); + osmo_fd_lookup.table[fd->fd] = fd; return 0; } @@ -180,6 +216,9 @@ * osmo_fd_is_registered() */ unregistered_count++; llist_del(&fd->list); + OSMO_ASSERT(fd->fd >= 0); + OSMO_ASSERT(fd->fd <= maxfd); + osmo_fd_lookup.table[fd->fd] = NULL; #ifndef FORCE_IO_SELECT g_poll.num_registered--; #endif /* FORCE_IO_SELECT */ @@ -464,19 +503,16 @@ * \returns \ref osmo_fd for \ref fd; NULL in case it doesn't exist */ struct osmo_fd *osmo_fd_get_by_fd(int fd) { - struct osmo_fd *ofd; - - llist_for_each_entry(ofd, &osmo_fds, list) { - if (ofd->fd == fd) - return ofd; - } - return NULL; + if (fd > maxfd) + return NULL; + return osmo_fd_lookup.table[fd]; } /*! initialize the osmocom select abstraction for the current thread */ void osmo_select_init(void) { INIT_LLIST_HEAD(&osmo_fds); + osmo_fd_lookup_table_extend(0); } /* ensure main thread always has pre-initialized osmo_fds */ -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31808 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I8b71547df8bed84192cb160479fa3debf9b7eade Gerrit-Change-Number: 31808 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0