[MERGED] openbsc[master]: libmsc: Map SMPP command status to GSM 04.11 cause

[Keith Whyte]

Keith Whyte has submitted this change and it was merged.

Change subject: libmsc: Map SMPP command status to GSM 04.11 cause
..


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, 41 insertions(+), 7 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/openbsc/src/libmsc/smpp_openbsc.c 
b/openbsc/src/libmsc/smpp_openbsc.c
index 8111d92..f94968a 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -461,6 +461,37 @@
}
 }
 
+struct {
+   uint32_t smpp_status_code;
+   uint8_t gsm411_cause;
+} smpp_to_gsm411_err_array[] = {
+
+   /* 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
+* Some provoke a display of "Try again later"
+* while others a more definitive "Message sending failed"
+*/
+
+   { ESME_RSYSERR, GSM411_RP_CAUSE_MO_DEST_OUT_OF_ORDER},
+   { ESME_RINVDSTADR,  GSM411_RP_CAUSE_MO_NUM_UNASSIGNED   },
+   { ESME_RMSGQFUL,GSM411_RP_CAUSE_MO_CONGESTION   },
+   { ESME_RINVSRCADR,  GSM411_RP_CAUSE_MO_SMS_REJECTED },
+   { ESME_RINVMSGID,   GSM411_RP_CAUSE_INV_TRANS_REF   }
+};
+
+static int smpp_to_gsm411_err(uint32_t smpp_status_code, int *gsm411_cause)
+{
+   int i;
+   for (i = 0; i < ARRAY_SIZE(smpp_to_gsm411_err_array); i++) {
+   if (smpp_to_gsm411_err_array[i].smpp_status_code != 
smpp_status_code)
+   continue;
+   *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(>response_timer);
@@ -501,10 +532,11 @@
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;
 
conn = connection_for_subscr(cmd->subscr);
if (!conn) {
@@ -520,14 +552,17 @@
return;
}
 
-   gsm411_send_rp_error(trans, cmd->sms->gsm411.msg_ref,
-GSM411_RP_CAUSE_MO_NET_OUT_OF_ORDER);
+   if (smpp_to_gsm411_err(status, _cause) < 0)
+   gsm411_cause = GSM411_RP_CAUSE_MO_NET_OUT_OF_ORDER;
+
+   gsm411_send_rp_error(trans, cmd->sms->gsm411.msg_ref, 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: merged
Gerrit-Change-Id: I61fb2d9ef4f2d2eabdc49b53d9966ad328d15e51
Gerrit-PatchSet: 3
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Keith Whyte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte 

openbsc[master]: libmsc: Map SMPP command status to GSM 04.11 cause

[Harald Welte]

Patch Set 2: Code-Review+2

-- 
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: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Keith Whyte 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pablo Neira Ayuso 
Gerrit-HasComments: No

[PATCH] openbsc[master]: libmsc: Map SMPP command status to GSM 04.11 cause

[Keith Whyte]

Hello Neels Hofmeyr, Jenkins Builder, Pablo Neira Ayuso,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/2589

to look at the new patch set (#2).

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, 41 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/89/2589/2

diff --git a/openbsc/src/libmsc/smpp_openbsc.c 
b/openbsc/src/libmsc/smpp_openbsc.c
index 8111d92..f94968a 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -461,6 +461,37 @@
}
 }
 
+struct {
+   uint32_t smpp_status_code;
+   uint8_t gsm411_cause;
+} smpp_to_gsm411_err_array[] = {
+
+   /* 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
+* Some provoke a display of "Try again later"
+* while others a more definitive "Message sending failed"
+*/
+
+   { ESME_RSYSERR, GSM411_RP_CAUSE_MO_DEST_OUT_OF_ORDER},
+   { ESME_RINVDSTADR,  GSM411_RP_CAUSE_MO_NUM_UNASSIGNED   },
+   { ESME_RMSGQFUL,GSM411_RP_CAUSE_MO_CONGESTION   },
+   { ESME_RINVSRCADR,  GSM411_RP_CAUSE_MO_SMS_REJECTED },
+   { ESME_RINVMSGID,   GSM411_RP_CAUSE_INV_TRANS_REF   }
+};
+
+static int smpp_to_gsm411_err(uint32_t smpp_status_code, int *gsm411_cause)
+{
+   int i;
+   for (i = 0; i < ARRAY_SIZE(smpp_to_gsm411_err_array); i++) {
+   if (smpp_to_gsm411_err_array[i].smpp_status_code != 
smpp_status_code)
+   continue;
+   *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(>response_timer);
@@ -501,10 +532,11 @@
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;
 
conn = connection_for_subscr(cmd->subscr);
if (!conn) {
@@ -520,14 +552,17 @@
return;
}
 
-   gsm411_send_rp_error(trans, cmd->sms->gsm411.msg_ref,
-GSM411_RP_CAUSE_MO_NET_OUT_OF_ORDER);
+   if (smpp_to_gsm411_err(status, _cause) < 0)
+   gsm411_cause = GSM411_RP_CAUSE_MO_NET_OUT_OF_ORDER;
+
+   gsm411_send_rp_error(trans, cmd->sms->gsm411.msg_ref, 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: newpatchset
Gerrit-Change-Id: I61fb2d9ef4f2d2eabdc49b53d9966ad328d15e51
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Keith Whyte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte 
Gerrit-Reviewer: Neels Hofmeyr 

openbsc[master]: libmsc: Map SMPP command status to GSM 04.11 cause

[Keith Whyte]

Patch Set 1:

(3 comments)

https://gerrit.osmocom.org/#/c/2589/1/openbsc/src/libmsc/smpp_openbsc.c
File openbsc/src/libmsc/smpp_openbsc.c:

Line 477:   .smpp_status_code = ESME_RSYSERR,
> This is C99 structure initialization... which is actually a good idea since
So, everywhere else I see this in osmo it's like Neels says. 
I am going to submit patch set 2 as he suggests. I guess it's not for me to get 
into a debate about whether or not this C99 struct init should be the way. Is 
that OK?


Line 571:   }
> (No braces needed around single line.)
Done


Line 574:gsm411_cause);
> This fits into the line, so no need to break it.
Done


-- 
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: Keith Whyte 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: Pablo Neira Ayuso 
Gerrit-HasComments: Yes

openbsc[master]: libmsc: Map SMPP command status to GSM 04.11 cause

[Pablo Neira Ayuso]

Patch Set 1: Code-Review-1

(6 comments)

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.)
Right. Keith, if you use ARRAY_SIZE() then we can skip this macro definition.


Line 466: struct x {
> (no x here please)
Keith, just remove this x. Keep it:

struct {
  ...


Line 477:   .smpp_status_code = ESME_RSYSERR,
> could omit the explicit names, just { ESME_X, GSM411_X }, ...
This is C99 structure initialization... which is actually a good idea since it 
makes code more readable and maintainable. Please keep it there.


PS1, Line 481: .
Add indent here, and everywhere inside the curly braces.


Line 502:   }
> (wrong indent)
That's equivalent... Fair enough.


Line 574:gsm411_cause);
This fits into the line, so no need to break it.


-- 
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

[PATCH] openbsc[master]: libmsc: Map SMPP command status to GSM 04.11 cause

[Keith Whyte]

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(>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, _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