openbsc[master]: libmsc: Map SMPP command status to GSM 04.11 cause
Patch Set 1: Code-Review-1
(8 comments)
some too detailed review of (mostly minor) cosmetics...
https://gerrit.osmocom.org/#/c/2589/1/openbsc/src/libmsc/smpp_openbsc.c
File openbsc/src/libmsc/smpp_openbsc.c:
Line 464: #define SMPP_TO_GSM411_MAX 4
You're defining this value but not using it the array definition... (s.b.)
Line 466: struct x {
(no x here please)
Line 469: } smpp_to_gsm411_err_array[4] = {
(continued from above)
...here. In fact though we usually define the array and later use the
ARRAY_SIZE() macro to iterate its items. You can/should actually omit the 4
completely, as in
struct {
...
} my_name[] = { {item}, {item} ... };
for (i = 0; i < ARRAY_SIZE(my_name); i++) {...}
Line 477: .smpp_status_code = ESME_RSYSERR,
could omit the explicit names, just { ESME_X, GSM411_X }, ...
Line 495: int *gsm411_cause)
(if you're using tab and space mixed-indent, then it would be nice to line up
'uint32_t' with 'int')
Line 498: for (i = 0; i < SMPP_TO_GSM411_MAX; i++) {
(here)
Line 502: }
(wrong indent)
I guess we'd normally go for an early continue instead:
{
if (A != B)
continue;
do stuff
return 0;
}
Line 571: }
(No braces needed around single line.)
(No real need for 'rc', could just do:
if (smpp_to_gsm411_err(..) < 0)
...
)
--
To view, visit https://gerrit.osmocom.org/2589
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I61fb2d9ef4f2d2eabdc49b53d9966ad328d15e51
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Keith Whyte
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Reviewer: Pablo Neira Ayuso
Gerrit-HasComments: Yes
[MERGED] openbsc[master]: fix subscriber random extension allocation range
Neels Hofmeyr has submitted this change and it was merged.
Change subject: fix subscriber random extension allocation range
..
fix subscriber random extension allocation range
The VTY config allows above 32bit range extensions, but
db_subscriber_alloc_exten() was unable to generate extensions outside of 32bit.
Add VTY regression test and fix the problem by using proper 64bit types.
Related: OS#2253
Change-Id: I9afe6a8833004ecd2f3f936b2d5aa4de8e7dbcb0
---
M openbsc/src/libmsc/db.c
M openbsc/tests/vty_test_runner.py
2 files changed, 15 insertions(+), 4 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c
index 9fa6415..5fe2a3c 100644
--- a/openbsc/src/libmsc/db.c
+++ b/openbsc/src/libmsc/db.c
@@ -1260,13 +1260,13 @@
uint64_t smax)
{
dbi_result result = NULL;
- uint32_t try;
+ uint64_t try;
for (;;) {
try = (rand() % (smax - smin + 1) + smin);
result = dbi_conn_queryf(conn,
"SELECT * FROM Subscriber "
- "WHERE extension = %i",
+ "WHERE extension = %"PRIu64,
try
);
if (!result) {
@@ -1284,8 +1284,8 @@
}
dbi_result_free(result);
}
- sprintf(subscriber->extension, "%i", try);
- DEBUGP(DDB, "Allocated extension %i for IMSI %s.\n", try,
subscriber->imsi);
+ sprintf(subscriber->extension, "%"PRIu64, try);
+ DEBUGP(DDB, "Allocated extension %"PRIu64 " for IMSI %s.\n", try,
subscriber->imsi);
return db_sync_subscriber(subscriber);
}
/*
diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py
index b886911..92775d5 100644
--- a/openbsc/tests/vty_test_runner.py
+++ b/openbsc/tests/vty_test_runner.py
@@ -485,6 +485,17 @@
self.assert_(res.find("subscriber-create-on-demand random 98
99"))
self.vty.command("end")
+res = self.vty.command('subscriber create imsi ' + imsi)
+print(res)
+self.assert_(res.find("IMSI: " + imsi) > 0)
+self.assert_(res.find("98") > 0 or res.find("99") > 0)
+self.assert_(res.find("Extension: ") > 0)
+
+res = self.vty.command('subscriber imsi ' + imsi + ' delete')
+self.assert_("" == res)
+
+res = self.vty.command('show subscriber imsi '+imsi)
+self.assert_(('% No subscriber found for imsi ' + imsi) == res)
def testSubscriberSettings(self):
--
To view, visit https://gerrit.osmocom.org/2584
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9afe6a8833004ecd2f3f936b2d5aa4de8e7dbcb0
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr
[MERGED] openbsc[master]: fix VTY parsing: subscriber-create-on-demand random
Neels Hofmeyr has submitted this change and it was merged.
Change subject: fix VTY parsing: subscriber-create-on-demand random
..
fix VTY parsing: subscriber-create-on-demand random
Fix parsing of the 'subscriber-create-on-demand random' VTY: atoi() is not
enough to include the specified range of 1-99.
Use atoll() instead to ensure a large enough number space also on 32bit
systems.
(Note: for me, atoll() truncates at 32 bit when is not included.)
Add a VTY regression test for this.
Related: OS#2253
Change-Id: I353e04481ec567adca383d6b51ba8fb865eed73e
---
M openbsc/src/libmsc/vty_interface_layer3.c
M openbsc/tests/vty_test_runner.py
2 files changed, 12 insertions(+), 1 deletion(-)
Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/openbsc/src/libmsc/vty_interface_layer3.c
b/openbsc/src/libmsc/vty_interface_layer3.c
index f631bcc..e503291 100644
--- a/openbsc/src/libmsc/vty_interface_layer3.c
+++ b/openbsc/src/libmsc/vty_interface_layer3.c
@@ -1079,7 +1079,7 @@
"Minimum for subscriber extension\n""Maximum for subscriber extension\n")
{
struct gsm_network *gsmnet = gsmnet_from_vty(vty);
- uint64_t mi = atoi(argv[0]), ma = atoi(argv[1]);
+ uint64_t mi = atoll(argv[0]), ma = atoll(argv[1]);
gsmnet->auto_create_subscr = true;
gsmnet->auto_assign_exten = true;
if (mi >= ma) {
diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py
index 305c956..b886911 100644
--- a/openbsc/tests/vty_test_runner.py
+++ b/openbsc/tests/vty_test_runner.py
@@ -476,6 +476,17 @@
res = self.vty.command('show subscriber imsi '+imsi)
self.assert_(('% No subscriber found for imsi ' + imsi) == res)
+# range
+self.vty.command("end")
+self.vty.command("configure terminal")
+self.vty.command("nitb")
+self.assertTrue(self.vty.verify("subscriber-create-on-demand random
98 99", ['']))
+res = self.vty.command("show running-config")
+self.assert_(res.find("subscriber-create-on-demand random 98
99"))
+self.vty.command("end")
+
+
+
def testSubscriberSettings(self):
self.vty.enable()
--
To view, visit https://gerrit.osmocom.org/2583
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I353e04481ec567adca383d6b51ba8fb865eed73e
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr
[PATCH] openbsc[master]: libmsc: Map SMPP command status to GSM 04.11 cause
Review at https://gerrit.osmocom.org/2589
libmsc: Map SMPP command status to GSM 04.11 cause
Send SMS RP ERROR with a failure cause that relates to
the status returned by the ESME in the deliver_sm_resp.
Actual mapping array is limited as most phones I tested
don't seem to care about the failure cause anyway,
although some will display a different notification for
GSM411_RP_CAUSE_MO_NUM_UNASSIGNED
Change-Id: I61fb2d9ef4f2d2eabdc49b53d9966ad328d15e51
---
M openbsc/src/libmsc/smpp_openbsc.c
M openbsc/src/libmsc/smpp_smsc.c
M openbsc/src/libmsc/smpp_smsc.h
3 files changed, 55 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/89/2589/1
diff --git a/openbsc/src/libmsc/smpp_openbsc.c
b/openbsc/src/libmsc/smpp_openbsc.c
index 8111d92..2b6aec6 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -461,6 +461,49 @@
}
}
+#define SMPP_TO_GSM411_MAX 4
+
+struct x {
+ uint32_t smpp_status_code;
+ uint8_t gsm411_cause;
+} smpp_to_gsm411_err_array[4] = {
+
+ /* Seems like most phones don't care about the failure cause,
+* although some will display a different notification for
+* GSM411_RP_CAUSE_MO_NUM_UNASSIGNED
+*/
+
+ {
+ .smpp_status_code = ESME_RSYSERR,
+ .gsm411_cause = GSM411_RP_CAUSE_MO_DEST_OUT_OF_ORDER,
+ },
+ {
+ .smpp_status_code = ESME_RINVDSTADR,
+ .gsm411_cause = GSM411_RP_CAUSE_MO_NUM_UNASSIGNED,
+ },
+ {
+ .smpp_status_code = ESME_RMSGQFUL,
+ .gsm411_cause = GSM411_RP_CAUSE_MO_CONGESTION,
+ },
+ {
+ .smpp_status_code = ESME_RINVSRCADR,
+ .gsm411_cause = GSM411_RP_CAUSE_MO_SMS_REJECTED,
+ },
+};
+
+static int smpp_to_gsm411_err(uint32_t smpp_status_code,
+ int *gsm411_cause)
+{
+ int i;
+ for (i = 0; i < SMPP_TO_GSM411_MAX; i++) {
+ if (smpp_to_gsm411_err_array[i].smpp_status_code ==
smpp_status_code) {
+ *gsm411_cause =
smpp_to_gsm411_err_array[i].gsm411_cause;
+ return 0;
+ }
+ }
+ return -1;
+}
+
static void smpp_cmd_free(struct osmo_smpp_cmd *cmd)
{
osmo_timer_del(&cmd->response_timer);
@@ -501,10 +544,12 @@
smpp_cmd_free(cmd);
}
-void smpp_cmd_err(struct osmo_smpp_cmd *cmd)
+void smpp_cmd_err(struct osmo_smpp_cmd *cmd, uint32_t status)
{
struct gsm_subscriber_connection *conn;
struct gsm_trans *trans;
+ int gsm411_cause;
+ int rc;
conn = connection_for_subscr(cmd->subscr);
if (!conn) {
@@ -520,14 +565,19 @@
return;
}
+ rc = smpp_to_gsm411_err(status, &gsm411_cause);
+ if (rc < 0) {
+ gsm411_cause = GSM411_RP_CAUSE_MO_NET_OUT_OF_ORDER;
+ }
+
gsm411_send_rp_error(trans, cmd->sms->gsm411.msg_ref,
-GSM411_RP_CAUSE_MO_NET_OUT_OF_ORDER);
+gsm411_cause);
smpp_cmd_free(cmd);
}
static void smpp_deliver_sm_cb(void *data)
{
- smpp_cmd_err(data);
+ smpp_cmd_err(data, ESME_RSYSERR);
}
static int smpp_cmd_enqueue(struct osmo_esme *esme,
diff --git a/openbsc/src/libmsc/smpp_smsc.c b/openbsc/src/libmsc/smpp_smsc.c
index bd25918..48a1192 100644
--- a/openbsc/src/libmsc/smpp_smsc.c
+++ b/openbsc/src/libmsc/smpp_smsc.c
@@ -681,11 +681,10 @@
return -1;
}
- /* Map SMPP command status to GSM 04.11 cause? */
if (deliver_r.command_status == ESME_ROK)
smpp_cmd_ack(cmd);
else
- smpp_cmd_err(cmd);
+ smpp_cmd_err(cmd, deliver_r.command_status);
LOGP(DSMPP, LOGL_INFO, "[%s] Rx DELIVER-SM RESP (%s)\n",
esme->system_id, get_value_string(smpp_status_strs,
diff --git a/openbsc/src/libmsc/smpp_smsc.h b/openbsc/src/libmsc/smpp_smsc.h
index b95a1f5..d8e82e4 100644
--- a/openbsc/src/libmsc/smpp_smsc.h
+++ b/openbsc/src/libmsc/smpp_smsc.h
@@ -97,7 +97,7 @@
struct osmo_smpp_cmd *smpp_cmd_find_by_seqnum(struct osmo_esme *esme,
uint32_t sequence_number);
void smpp_cmd_ack(struct osmo_smpp_cmd *cmd);
-void smpp_cmd_err(struct osmo_smpp_cmd *cmd);
+void smpp_cmd_err(struct osmo_smpp_cmd *cmd, uint32_t status);
void smpp_cmd_flush_pending(struct osmo_esme *esme);
struct smsc {
--
To view, visit https://gerrit.osmocom.org/2589
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I61fb2d9ef4f2d2eabdc49b53d9966ad328d15e51
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Keith Whyte
openbsc[master]: fix VTY parsing: subscriber-create-on-demand random
Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/2583 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I353e04481ec567adca383d6b51ba8fb865eed73e Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-HasComments: No
openbsc[master]: fix subscriber random extension allocation range
Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/2584 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9afe6a8833004ecd2f3f936b2d5aa4de8e7dbcb0 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-HasComments: No
