This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Vadim Yanitskiy gerrit-no-reply at lists.osmocom.orgVadim Yanitskiy has submitted this change and it was merged. ( https://gerrit.osmocom.org/12628 ) Change subject: libmsc/gsm_04_11.c: introduce and use gsm411_assign_sm_rp_mr() ...................................................................... libmsc/gsm_04_11.c: introduce and use gsm411_assign_sm_rp_mr() Initially, it was assumed that if there is no active RAN connection, we can just start counting from 0x00, as there are no other SMS related transactions, and transaction itself is allocated using talloc_zero(). Until now it was looking good, but... As soon as we establish RAN connection with subscriber, we already have a transaction with SM-RP-MR 0x00, but conn->next_rp_ref also remains 0x00 - it isn't being increased! It means that we can face a SM-RP-MR conflict (or collision) if another MT SMS would arrive to the MSC (from SMSC over GSUP) when this transaction is still active, i.e. the first SMS is still being sent, because conn->next_rp_ref++ would return 0x00 again. Moreover, there might be already a MO SMS transaction, and using the conn->next_rp_ref counter wouldn't prevent us from having duplicate SM-RP-MR value. Let's get rid of this per-connection counter, and introduce a function instead, that would iterate over existing transactions and look for an unused SM-RP-MR value. This change makes the following test cases pass: - TC_gsup_mt_sms_rp_mr, - TC_gsup_mo_mt_sms_rp_mr. Discovered by: Neels Hofmeyr Related Change-Id: (TTCN) I3a52d44f4abde9b6b471b9108c1cee905884c9bc Related Change-Id: (TTCN) I17cbbaa64d9bce770f985588e93cd3eecd732120 Change-Id: Ife6d954c46b7d8348a4221ab677d0355eb3ee7ac --- M include/osmocom/msc/ran_conn.h M src/libmsc/gsm_04_11.c 2 files changed, 29 insertions(+), 5 deletions(-) Approvals: Harald Welte: Looks good to me, approved Max: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/include/osmocom/msc/ran_conn.h b/include/osmocom/msc/ran_conn.h index bec7201..affebc8 100644 --- a/include/osmocom/msc/ran_conn.h +++ b/include/osmocom/msc/ran_conn.h @@ -102,8 +102,6 @@ /* LU expiration handling */ uint8_t expire_timer_stopped; - /* SMS helpers for libmsc */ - uint8_t next_rp_ref; /* Are we part of a special "silent" call */ int silent_call; diff --git a/src/libmsc/gsm_04_11.c b/src/libmsc/gsm_04_11.c index 2f39b06..1edf2d4 100644 --- a/src/libmsc/gsm_04_11.c +++ b/src/libmsc/gsm_04_11.c @@ -1028,6 +1028,30 @@ return trans; } +/* Assigns an (unused) SM-RP-MR value to a given transaction */ +static int gsm411_assign_sm_rp_mr(struct gsm_trans *trans) +{ + uint8_t mr; + + /* After allocation a given transaction has zero-initialized + * SM-RP-MR value, so trans_find_by_sm_rp_mr() may consider + * 0x00 as used. This is why we "poison" this transaction + * using the highest value. */ + trans->sms.sm_rp_mr = 0xff; + + /* According to 8.2.3, MR is in the range 0 through 255 */ + for (mr = 0x00; mr < 0xff; mr++) { + if (trans_find_by_sm_rp_mr(trans->net, trans->vsub, mr)) + continue; /* this MR is busy, find another one */ + /* An unused value has been found, assign it */ + trans->sms.sm_rp_mr = mr; + return 0; + } + + /* All possible values are busy */ + return -EBUSY; +} + static struct gsm_trans *gsm411_alloc_mt_trans(struct gsm_network *net, struct vlr_subscr *vsub) { @@ -1052,9 +1076,11 @@ if (!trans) return NULL; - if (conn) { - /* Generate unique RP Message Reference */ - trans->sms.sm_rp_mr = conn->next_rp_ref++; + /* Assign a unique SM-RP Message Reference */ + if (gsm411_assign_sm_rp_mr(trans) != 0) { + LOGP(DLSMS, LOGL_ERROR, "Failed to assign SM-RP-MR\n"); + trans_free(trans); + return NULL; } /* Use SAPI 3 (see GSM 04.11, section 2.3) */ -- To view, visit https://gerrit.osmocom.org/12628 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ife6d954c46b7d8348a4221ab677d0355eb3ee7ac Gerrit-Change-Number: 12628 Gerrit-PatchSet: 4 Gerrit-Owner: Vadim Yanitskiy <axilirator at gmail.com> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Max <msuraev at sysmocom.de> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190201/302efecf/attachment.htm>