libosmo-netif[master]: osmux: Re-write osmux_snprintf
Patch Set 3: That sounds good. Split it in two patches I would suggest. Thanks! -- To view, visit https://gerrit.osmocom.org/3537 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I695771d099833842db37a415b636035d17f1bba7 Gerrit-PatchSet: 3 Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-HasComments: No
[MERGED] libosmo-netif[master]: src: _snprintf() helper functions nul-terminate buffers, if ...
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: src: _snprintf() helper functions nul-terminate buffers, if
possible
..
src: _snprintf() helper functions nul-terminate buffers, if possible
This patch inconditionally initializes the buffer we get to
nul-terminate it, whenever possible. It's a very simple solution to
catch three overly corner cases:
1) snprintf() returns -1, very much unlikely in practise.
2) msg->len == 0: In such case, I would expect this function is never
called with an empty message, but let's be safe in this case too.
3) If your buffer is empty, it doesn't nul-terminate the buffer.
Change-Id: I97e517f2d98e83894ea707c63489559302ff6bd2
---
M src/osmux.c
M src/rtp.c
2 files changed, 6 insertions(+), 0 deletions(-)
Approvals:
Pau Espin Pedrol: Looks good to me, but someone else must approve
Neels Hofmeyr: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/osmux.c b/src/osmux.c
index b18246f..7290d99 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -902,6 +902,9 @@
int msg_len = msg->len;
int ret;
+ if (size)
+ buf[0] = '\0';
+
while (msg_len > 0) {
if (msg_len < sizeof(struct osmux_hdr)) {
LOGP(DLMIB, LOGL_ERROR,
diff --git a/src/rtp.c b/src/rtp.c
index 56fc37c..48cb9b0 100644
--- a/src/rtp.c
+++ b/src/rtp.c
@@ -200,6 +200,9 @@
uint8_t *payload;
int ret, i;
+ if (size)
+ buf[0] = '\0';
+
rtph = osmo_rtp_get_hdr(msg);
if (rtph == NULL)
return -1;
--
To view, visit https://gerrit.osmocom.org/3830
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I97e517f2d98e83894ea707c63489559302ff6bd2
Gerrit-PatchSet: 3
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de>
[PATCH] openbsc[master]: libmsc: sms_route_mt_sms() may return uninitialized return v...
Hello Harald Welte, Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/3899
to look at the new patch set (#4).
libmsc: sms_route_mt_sms() may return uninitialized return value
If smpp_first is set off and there is a local receiver for this sms,
then return 0. Without this patch, we return 'rc' which is uninitialized
in the scenario that I'm describing above.
Change-Id: I0c0bcd919cc3275d491995d17c6a32bb61c6afe1
---
M openbsc/src/libmsc/gsm_04_11.c
1 file changed, 22 insertions(+), 21 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/99/3899/4
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index 27bffc9..25ef487 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -363,31 +363,32 @@
/* determine gsms->receiver based on dialled number */
gsms->receiver = subscr_get_by_extension(conn->network->subscr_group,
gsms->dst.addr);
- if (!gsms->receiver) {
-#ifdef BUILD_SMPP
- /* Avoid a second look-up */
- if (smpp_first) {
-
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
- return GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
- }
+ if (gsms->receiver)
+ return 0;
- rc = smpp_try_deliver(gsms, conn);
- if (rc == GSM411_RP_CAUSE_MO_NUM_UNASSIGNED) {
-
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
- } else if (rc < 0) {
- LOGP(DLSMS, LOGL_ERROR, "%s: SMS delivery error: %d.",
-subscr_name(conn->subscr), rc);
- rc = GSM411_RP_CAUSE_MO_TEMP_FAIL;
- /* rc will be logged by gsm411_send_rp_error() */
- rate_ctr_inc(>bts->network->msc_ctrs->ctr[
- MSC_CTR_SMS_DELIVER_UNKNOWN_ERROR]);
- }
-#else
- rc = GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
+#ifdef BUILD_SMPP
+ /* Avoid a second look-up */
+ if (smpp_first) {
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
-#endif
+ return GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
}
+ rc = smpp_try_deliver(gsms, conn);
+ if (rc == GSM411_RP_CAUSE_MO_NUM_UNASSIGNED) {
+
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
+ } else if (rc < 0) {
+ LOGP(DLSMS, LOGL_ERROR, "%s: SMS delivery error: %d.",
+subscr_name(conn->subscr), rc);
+ rc = GSM411_RP_CAUSE_MO_TEMP_FAIL;
+ /* rc will be logged by gsm411_send_rp_error() */
+ rate_ctr_inc(>bts->network->msc_ctrs->ctr[
+ MSC_CTR_SMS_DELIVER_UNKNOWN_ERROR]);
+ }
+#else
+ rc = GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
+ rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
+#endif
+
return rc;
}
--
To view, visit https://gerrit.osmocom.org/3899
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0c0bcd919cc3275d491995d17c6a32bb61c6afe1
Gerrit-PatchSet: 4
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: laforge <lafo...@gnumonks.org>
Gerrit-Reviewer: neels <nhofm...@sysmocom.de>
openbsc[master]: libmsc: db_subscriber_alloc_exten() remove infinite loop
Patch Set 2: Ok, let me reformulate my question: Do you ever see this message after this patch? "Out of Trys, no extension for IMSI %s" What I see from the code you're posting is that we now have a loop that is limited by the number of extensions that are available, however, we still use a random function. So I see this as a lottery game, where the less extensions are available, the less chances you have to land on a spare extension, since candidates are selected randomly. -- To view, visit https://gerrit.osmocom.org/3910 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icf0f1e5a7f360bc27592a55890f74a9a12bc9f42 Gerrit-PatchSet: 2 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
openbsc[master]: libmsc: db_subscriber_alloc_exten() remove infinite loop
Patch Set 1:
I see... it's from the VTY itself that hangs with a valid configuration.
Would you send a v2 of this patch that does something like this?
... extension_alloc(...)
{
range = max - min;
extension = min + (rand() % range);
return extension
}
Then:
if extension is busy:
extension++;
if count == 100:
extension = extension_alloc();
So the idea is that we don't call random() again, but try with next door
extensions to see if they are available.
Play around with it to see if it cures the neverending loop there.
--
To view, visit https://gerrit.osmocom.org/3910
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf0f1e5a7f360bc27592a55890f74a9a12bc9f42
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-HasComments: No
openbsc[master]: libmsc: db_subscriber_alloc_exten() remove infinite loop
Patch Set 1:
I see... it's from the VTY itself that hangs with a valid configuration.
Would you send a v2 of this patch that does something like this?
... extension_alloc(...)
{
range = max - min;
extension = min + (rand() % range);
return extension
}
Then:
if extension is busy:
extension++;
if count == 100:
extension = extension_alloc();
So the idea is that we don't call random() again, but try with next door
extensions to see if they are available.
Play around with it to see if it cures the neverending loop there.
--
To view, visit https://gerrit.osmocom.org/3910
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf0f1e5a7f360bc27592a55890f74a9a12bc9f42
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-HasComments: No
openbsc[master]: libmsc: db_subscriber_alloc_exten() remove infinite loop
Patch Set 1: (1 comment) https://gerrit.osmocom.org/#/c/3910/1/openbsc/src/libmsc/db.c File openbsc/src/libmsc/db.c: Line 1419: try = (rand() % (smax - smin + 1) + smin); Why don't we better update this code to use something better than a random function? To keep it simple, something like rand() to land on a space, then keep incrementally searching for an unused extension. If the concern is only the infinite loop and the random function is fine, then just keep a global counter on the number of available extensions, so we can early give up on it. My only concern with the random function is being _unlucky_, I mean, you get extensions that already used several times. But I'm not in touch with a real testbed, so I cannot tell how realistic this is. -- To view, visit https://gerrit.osmocom.org/3910 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icf0f1e5a7f360bc27592a55890f74a9a12bc9f42 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: Yes
[PATCH] libosmo-netif[master]: src: _snprintf() helper functions nul-terminate buffers, if ...
Hello Pau Espin Pedrol, Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/3830
to look at the new patch set (#3).
src: _snprintf() helper functions nul-terminate buffers, if possible
This patch inconditionally initializes the buffer we get to
nul-terminate it, whenever possible. It's a very simple solution to
catch three overly corner cases:
1) snprintf() returns -1, very much unlikely in practise.
2) msg->len == 0: In such case, I would expect this function is never
called with an empty message, but let's be safe in this case too.
3) If your buffer is empty, it doesn't nul-terminate the buffer.
Change-Id: I97e517f2d98e83894ea707c63489559302ff6bd2
---
M src/osmux.c
M src/rtp.c
2 files changed, 6 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/30/3830/3
diff --git a/src/osmux.c b/src/osmux.c
index b18246f..7290d99 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -902,6 +902,9 @@
int msg_len = msg->len;
int ret;
+ if (size)
+ buf[0] = '\0';
+
while (msg_len > 0) {
if (msg_len < sizeof(struct osmux_hdr)) {
LOGP(DLMIB, LOGL_ERROR,
diff --git a/src/rtp.c b/src/rtp.c
index 56fc37c..48cb9b0 100644
--- a/src/rtp.c
+++ b/src/rtp.c
@@ -200,6 +200,9 @@
uint8_t *payload;
int ret, i;
+ if (size)
+ buf[0] = '\0';
+
rtph = osmo_rtp_get_hdr(msg);
if (rtph == NULL)
return -1;
--
To view, visit https://gerrit.osmocom.org/3830
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I97e517f2d98e83894ea707c63489559302ff6bd2
Gerrit-PatchSet: 3
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de>
openbsc[master]: libmsc: Log Rx DELIVER-SM RESP before calling gsm411_send_rp...
Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/3909 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I29270652957f58093be8bf7f2e898b0b4933bd93 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
[PATCH] libosmo-netif[master]: src: _snprintf() helper functions always nul-terminate buffers
Hello Pau Espin Pedrol, Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/3830
to look at the new patch set (#2).
src: _snprintf() helper functions always nul-terminate buffers
This patch inconditionally initializes the buffer we get to
nul-terminated, it's a very simple solution to catch two overly corner
cases:
1) snprintf() returns -1, very much unlikely in practise.
2) msg->len == 0: In such case, I would expect this function is never
called with an empty message, but let's be safe in this case too.
Change-Id: I97e517f2d98e83894ea707c63489559302ff6bd2
---
M src/osmux.c
M src/rtp.c
2 files changed, 4 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/30/3830/2
diff --git a/src/osmux.c b/src/osmux.c
index b18246f..47def14 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -902,6 +902,8 @@
int msg_len = msg->len;
int ret;
+ buf[0] = '\0';
+
while (msg_len > 0) {
if (msg_len < sizeof(struct osmux_hdr)) {
LOGP(DLMIB, LOGL_ERROR,
diff --git a/src/rtp.c b/src/rtp.c
index 56fc37c..38bee3a 100644
--- a/src/rtp.c
+++ b/src/rtp.c
@@ -200,6 +200,8 @@
uint8_t *payload;
int ret, i;
+ buf[0] = '\0';
+
rtph = osmo_rtp_get_hdr(msg);
if (rtph == NULL)
return -1;
--
To view, visit https://gerrit.osmocom.org/3830
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I97e517f2d98e83894ea707c63489559302ff6bd2
Gerrit-PatchSet: 2
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de>
openbsc[master]: libmsc: sms_route_mt_sms() may return uninitialized return v...
Patch Set 2: Hm, Jenkins is complaining on VTY tests again. This patch doesn't update such bits. Any clue on what is wrong? Thanks! -- To view, visit https://gerrit.osmocom.org/3899 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0c0bcd919cc3275d491995d17c6a32bb61c6afe1 Gerrit-PatchSet: 2 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: laforge <lafo...@gnumonks.org> Gerrit-Reviewer: neels <nhofm...@sysmocom.de> Gerrit-HasComments: No
openbsc[master]: libmsc: annotate esme route in the sms object from deliver_t...
Patch Set 4: Verified+1 -- To view, visit https://gerrit.osmocom.org/3900 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib8a8fd9bbb0d3b6aff7931e4bacbea99d000e484 Gerrit-PatchSet: 4 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: neels <nhofm...@sysmocom.de> Gerrit-HasComments: No
[MERGED] openbsc[master]: libmsc: annotate esme route in the sms object from deliver_t...
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: libmsc: annotate esme route in the sms object from
deliver_to_esme()
..
libmsc: annotate esme route in the sms object from deliver_to_esme()
Annotate this esme route, so we can use it to return -EINPROGRESS to
skip sending premature RP-ACK to the mobile station, in case we're
handling sms routes through SMPP.
Now that we have this information in place, we use it wherever possible
to avoid kludgy checks on sms->receiver.
sms_free() already releases references to this object, so we should be
fine with this.
Fixes: 4e5b90a594f9 ("libmsc: remove 'deferred' parameter in
sms_route_mt_sms()")
Change-Id: Ib8a8fd9bbb0d3b6aff7931e4bacbea99d000e484
---
M openbsc/src/libmsc/gsm_04_11.c
M openbsc/src/libmsc/smpp_openbsc.c
2 files changed, 12 insertions(+), 3 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Pablo Neira Ayuso: Verified
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index ddef444..27bffc9 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -519,7 +519,10 @@
rc = sms_route_mt_sms(conn, gsms);
- /* This SMS got routed through SMPP or no receiver exists. */
+ /* This SMS got routed through SMPP. */
+ if (gsms->smpp.esme)
+ return -EINPROGRESS;
+
if (!gsms->receiver)
return rc;
@@ -613,8 +616,10 @@
return gsm411_send_rp_ack(trans, rph->msg_ref);
else if (rc > 0)
return gsm411_send_rp_error(trans, rph->msg_ref, rc);
- else
- return rc;
+ else if (rc == -EINPROGRESS)
+ rc = 0;
+
+ return rc;
}
/* Receive a 04.11 RP-DATA message in accordance with Section 7.3.1.2 */
diff --git a/openbsc/src/libmsc/smpp_openbsc.c
b/openbsc/src/libmsc/smpp_openbsc.c
index af2d1be..3fe2dfd 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -714,6 +714,10 @@
if (ret < 0)
return ret;
+ OSMO_ASSERT(!sms->smpp.esme);
+ smpp_esme_get(esme);
+ sms->smpp.esme = esme;
+
return smpp_cmd_enqueue(esme, conn->subscr, sms,
deliver.sequence_number);
}
--
To view, visit https://gerrit.osmocom.org/3900
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib8a8fd9bbb0d3b6aff7931e4bacbea99d000e484
Gerrit-PatchSet: 4
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: neels <nhofm...@sysmocom.de>
[PATCH] openbsc[master]: libmsc: sms_route_mt_sms() may return uninitialized return v...
Hello Harald Welte, Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/3899
to look at the new patch set (#2).
libmsc: sms_route_mt_sms() may return uninitialized return value
If smpp_first is set off and there is a local receiver for this sms,
then return 0.
Without this patch, we return 'rc' which is uninitialized in the
scenario that I'm describing above.
Change-Id: I0c0bcd919cc3275d491995d17c6a32bb61c6afe1
---
M openbsc/src/libmsc/gsm_04_11.c
1 file changed, 22 insertions(+), 21 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/99/3899/2
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index ddef444..eb0a8ae 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -363,31 +363,32 @@
/* determine gsms->receiver based on dialled number */
gsms->receiver = subscr_get_by_extension(conn->network->subscr_group,
gsms->dst.addr);
- if (!gsms->receiver) {
-#ifdef BUILD_SMPP
- /* Avoid a second look-up */
- if (smpp_first) {
-
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
- return GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
- }
+ if (gsms->receiver)
+ return 0;
- rc = smpp_try_deliver(gsms, conn);
- if (rc == GSM411_RP_CAUSE_MO_NUM_UNASSIGNED) {
-
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
- } else if (rc < 0) {
- LOGP(DLSMS, LOGL_ERROR, "%s: SMS delivery error: %d.",
-subscr_name(conn->subscr), rc);
- rc = GSM411_RP_CAUSE_MO_TEMP_FAIL;
- /* rc will be logged by gsm411_send_rp_error() */
- rate_ctr_inc(>bts->network->msc_ctrs->ctr[
- MSC_CTR_SMS_DELIVER_UNKNOWN_ERROR]);
- }
-#else
- rc = GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
+#ifdef BUILD_SMPP
+ /* Avoid a second look-up */
+ if (smpp_first) {
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
-#endif
+ return GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
}
+ rc = smpp_try_deliver(gsms, conn);
+ if (rc == GSM411_RP_CAUSE_MO_NUM_UNASSIGNED) {
+
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
+ } else if (rc < 0) {
+ LOGP(DLSMS, LOGL_ERROR, "%s: SMS delivery error: %d.",
+subscr_name(conn->subscr), rc);
+ rc = GSM411_RP_CAUSE_MO_TEMP_FAIL;
+ /* rc will be logged by gsm411_send_rp_error() */
+ rate_ctr_inc(>bts->network->msc_ctrs->ctr[
+ MSC_CTR_SMS_DELIVER_UNKNOWN_ERROR]);
+ }
+#else
+ rc = GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
+ rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
+#endif
+
return rc;
}
--
To view, visit https://gerrit.osmocom.org/3899
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0c0bcd919cc3275d491995d17c6a32bb61c6afe1
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: laforge <lafo...@gnumonks.org>
Gerrit-Reviewer: neels <nhofm...@sysmocom.de>
[MERGED] libosmo-netif[master]: osmux: fix buffer management mess in snprintf() calls
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: osmux: fix buffer management mess in snprintf() calls
..
osmux: fix buffer management mess in snprintf() calls
SNPRINTF_BUFFER_SIZE() looks too complex, previous version maintains two
different variables to account for the remaining space in the buffer,
one of them is always decremented based on what snprintf() returns,
which may result in underflow. These variables are swapped - not used
consistently - all over this code.
Replace this macro by a simplified version, with one single parameter to
account for remaining space. This macro also deals with two corner
cases:
1) snprintf() fails, actually never happens in practise, but
documentation indicates it may return -1, so let's catch this case
from here to stick to specs.
2) There is not enough space in the buffer, in that case, keep
increasing offset, so we know how much would have been printed, just
like snprintf() does.
Thanks to Pau Espin for reporting, and Holger for clues on this.
I have run osmux_test and, at quick glance, it looks good.
Change-Id: I5b5d6ec57a02f57c23b1ae86dbd894bad28ea797
---
M src/osmux.c
M src/rtp.c
M tests/osmux/osmux_test.c
3 files changed, 44 insertions(+), 45 deletions(-)
Approvals:
Pau Espin Pedrol: Looks good to me, but someone else must approve
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/osmux.c b/src/osmux.c
index b3c43e2..b18246f 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -846,19 +846,20 @@
h->rtp_ssrc = rtp_ssrc;
}
-#define SNPRINTF_BUFFER_SIZE(ret, size, len, offset) \
- size -= ret;\
- if (ret > len) \
- ret = len; \
+#define SNPRINTF_BUFFER_SIZE(ret, remain, offset) \
+ if (ret < 0)\
+ ret = 0;\
offset += ret; \
- len -= ret;
+ if (ret > remain) \
+ ret = remain; \
+ remain -= ret;
static int osmux_snprintf_header(char *buf, size_t size, struct osmux_hdr
*osmuxh)
{
+ unsigned int remain = size, offset = 0;
int ret;
- int len = size, offset = 0;
- ret = snprintf(buf, len, "OSMUX seq=%03u ccid=%03u "
+ ret = snprintf(buf, remain, "OSMUX seq=%03u ccid=%03u "
"ft=%01u ctr=%01u "
"amr_f=%01u amr_q=%01u "
"amr_ft=%02u amr_cmr=%02u ",
@@ -866,7 +867,7 @@
osmuxh->ft, osmuxh->ctr,
osmuxh->amr_f, osmuxh->amr_q,
osmuxh->amr_ft, osmuxh->amr_cmr);
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ SNPRINTF_BUFFER_SIZE(ret, remain, offset);
return offset;
}
@@ -874,19 +875,19 @@
static int osmux_snprintf_payload(char *buf, size_t size,
const uint8_t *payload, int payload_len)
{
+ unsigned int remain = size, offset = 0;
int ret, i;
- int len = size, offset = 0;
- ret = snprintf(buf+offset, len, "[ ");
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ ret = snprintf(buf + offset, remain, "[ ");
+ SNPRINTF_BUFFER_SIZE(ret, remain, offset);
for (i=0; i<payload_len; i++) {
- ret = snprintf(buf+offset, len, "%02x ", payload[i]);
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ ret = snprintf(buf + offset, remain, "%02x ", payload[i]);
+ SNPRINTF_BUFFER_SIZE(ret, remain, offset);
}
- ret = snprintf(buf+offset, len, "] ");
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ ret = snprintf(buf + offset, remain, "]");
+ SNPRINTF_BUFFER_SIZE(ret, remain, offset);
return offset;
}
@@ -894,11 +895,12 @@
int osmux_snprintf(char *buf, size_t size, struct msgb *msg)
{
- int ret;
- unsigned int offset = 0;
- int msg_len = msg->len, len = size;
- struct osmux_hdr *osmuxh;
+ unsigned int remain = size;
int this_len, msg_off = 0;
+ struct osmux_hdr *osmuxh;
+ unsigned int offset = 0;
+ int msg_len = msg->len;
+ int ret;
while (msg_len > 0) {
if (msg_len < sizeof(struct osmux_hdr)) {
@@ -915,10 +917,8 @@
return -1;
}
- ret = osmux_snprintf_header(buf+offset, size, osmuxh);
- if (ret < 0)
- break;
- SNPRINTF_BUFFER_SIZ
libosmo-netif[master]: osmux: fix buffer management mess in snprintf() calls
Patch Set 3: Thanks for reviewing Pau! BTW, it seems Jenkins is experiencing problems? By reading the console output it looks the build failed for reasons other than this patch. -- To view, visit https://gerrit.osmocom.org/3825 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5b5d6ec57a02f57c23b1ae86dbd894bad28ea797 Gerrit-PatchSet: 3 Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-HasComments: No
openbsc[master]: libmsc: annotate esme route in the sms object from deliver_t...
Patch Set 1: Code-Review+1 (1 comment) https://gerrit.osmocom.org/#/c/3900/1/openbsc/src/libmsc/smpp_openbsc.c File openbsc/src/libmsc/smpp_openbsc.c: Line 718: sms->smpp.esme = esme; > check that we're not overwriting sms->smpp.esme if it is already set? (i.e Please, if you carefully review the codepath you will notice that is always unset... However, you vote this with -1 and this is blocking a real fix, something that Keith confirms that is solving a real problem there. I don't want to add this superfluous check, it's better to agree on semantics, not just add a branch there because maybe someone else make a sloppy use of this in the future... Stop forcing people to do overly defensive programming, just because you are scared. Or develop your real concerns, instead of nacking work from others with poor arguments. Thank you. -- To view, visit https://gerrit.osmocom.org/3900 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib8a8fd9bbb0d3b6aff7931e4bacbea99d000e484 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: neels <nhofm...@sysmocom.de> Gerrit-HasComments: Yes
[PATCH] openbsc[master]: libmsc: annotate esme route in the sms object from deliver_t...
Review at https://gerrit.osmocom.org/3900
libmsc: annotate esme route in the sms object from deliver_to_esme()
Annotate this esme route, so we can use it to return -EINPROGRESS to
skip sending premature RP-ACK to the mobile station, in case we're
handling sms routes through SMPP.
Now that we have this information in place, we use it wherever possible
to avoid kludgy checks on sms->receiver.
sms_free() already releases references to this object, so we should be
fine with this.
Fixes: 4e5b90a594f9 ("libmsc: remove 'deferred' parameter in
sms_route_mt_sms()")
Change-Id: Ib8a8fd9bbb0d3b6aff7931e4bacbea99d000e484
---
M openbsc/src/libmsc/gsm_04_11.c
M openbsc/src/libmsc/smpp_openbsc.c
2 files changed, 11 insertions(+), 3 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/00/3900/1
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index ad788f2..98c362f 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -520,7 +520,10 @@
rc = sms_route_mt_sms(conn, gsms);
- /* This SMS got routed through SMPP or no receiver exists. */
+ /* This SMS got routed through SMPP. */
+ if (gsms->smpp.esme)
+ return -EINPROGRESS;
+
if (!gsms->receiver)
return rc;
@@ -614,8 +617,10 @@
return gsm411_send_rp_ack(trans, rph->msg_ref);
else if (rc > 0)
return gsm411_send_rp_error(trans, rph->msg_ref, rc);
- else
- return rc;
+ else if (rc == -EINPROGRESS)
+ rc = 0;
+
+ return rc;
}
/* Receive a 04.11 RP-DATA message in accordance with Section 7.3.1.2 */
diff --git a/openbsc/src/libmsc/smpp_openbsc.c
b/openbsc/src/libmsc/smpp_openbsc.c
index af2d1be..02b2aba 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -714,6 +714,9 @@
if (ret < 0)
return ret;
+ smpp_esme_get(esme);
+ sms->smpp.esme = esme;
+
return smpp_cmd_enqueue(esme, conn->subscr, sms,
deliver.sequence_number);
}
--
To view, visit https://gerrit.osmocom.org/3900
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib8a8fd9bbb0d3b6aff7931e4bacbea99d000e484
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
openbsc[master]: libmsc: Either route report to ESME or send it, not both
Patch Set 1:
(1 comment)
https://gerrit.osmocom.org/#/c/3885/1/openbsc/src/libmsc/gsm_04_11.c
File openbsc/src/libmsc/gsm_04_11.c:
Line 693: if (rc == 0) {
> Pablo, I'm not sure how you would like to make this test here.
Keith, could you try this patch instead to see if it cures the problem for you?
https://gerrit.osmocom.org/#/c/3899/
--
To view, visit https://gerrit.osmocom.org/3885
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e86a34f5d3087c9c25479192d9a690922113da2
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-HasComments: Yes
openbsc[master]: libmsc: Either route report to ESME or send it, not both
Patch Set 1:
(1 comment)
https://gerrit.osmocom.org/#/c/3885/1/openbsc/src/libmsc/gsm_04_11.c
File openbsc/src/libmsc/gsm_04_11.c:
Line 693: if (rc == 0) {
> The uninitialized return value is a real issue, I just sent a patch to addr
Actually, this can be a real issue from gsm340_rx_tpdu(). Given rc determines
what to do.
--
To view, visit https://gerrit.osmocom.org/3885
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e86a34f5d3087c9c25479192d9a690922113da2
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-HasComments: Yes
openbsc[master]: libmsc: Either route report to ESME or send it, not both
Patch Set 1:
(1 comment)
https://gerrit.osmocom.org/#/c/3885/1/openbsc/src/libmsc/gsm_04_11.c
File openbsc/src/libmsc/gsm_04_11.c:
Line 693: if (rc == 0) {
> Took a brief look at the code which I don't know well, and at it looks such
The uninitialized return value is a real issue, I just sent a patch to address
this. This may result in bogus log messages indication "Failed to send status
report!". I just sent a patch to address this.
Regarding sms_free(sms_report), this is just called at the end of this
function, once this object is not needed at all. Given we don't return after
sms_route_mt_sms() call, I see no problem with it.
--
To view, visit https://gerrit.osmocom.org/3885
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e86a34f5d3087c9c25479192d9a690922113da2
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-HasComments: Yes
[PATCH] openbsc[master]: libmsc: sms_route_mt_sms() may return uninitialized return v...
Review at https://gerrit.osmocom.org/3899
libmsc: sms_route_mt_sms() may return uninitialized return value
If smpp_first is set off and there is a local receiver for this sms,
then return 0.
Without this patch, we return 'rc' which is uninitialized in the
scenario that I'm describing above.
Change-Id: I0c0bcd919cc3275d491995d17c6a32bb61c6afe1
---
M openbsc/src/libmsc/gsm_04_11.c
1 file changed, 22 insertions(+), 21 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/99/3899/1
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index ddef444..ad788f2 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -363,31 +363,32 @@
/* determine gsms->receiver based on dialled number */
gsms->receiver = subscr_get_by_extension(conn->network->subscr_group,
gsms->dst.addr);
- if (!gsms->receiver) {
-#ifdef BUILD_SMPP
- /* Avoid a second look-up */
- if (smpp_first) {
-
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
- return GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
- }
+ if (gsms->receiver)
+ return 0;
- rc = smpp_try_deliver(gsms, conn);
- if (rc == GSM411_RP_CAUSE_MO_NUM_UNASSIGNED) {
-
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
- } else if (rc < 0) {
- LOGP(DLSMS, LOGL_ERROR, "%s: SMS delivery error: %d.",
-subscr_name(conn->subscr), rc);
- rc = GSM411_RP_CAUSE_MO_TEMP_FAIL;
- /* rc will be logged by gsm411_send_rp_error() */
- rate_ctr_inc(>bts->network->msc_ctrs->ctr[
- MSC_CTR_SMS_DELIVER_UNKNOWN_ERROR]);
- }
-#else
- rc = GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
+#ifdef BUILD_SMPP
+ /* Avoid a second look-up */
+ if (smpp_first) {
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
-#endif
+ return GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
}
+ rc = smpp_try_deliver(gsms, conn);
+ if (rc == GSM411_RP_CAUSE_MO_NUM_UNASSIGNED) {
+
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
+ } else if (rc < 0) {
+ LOGP(DLSMS, LOGL_ERROR, "%s: SMS delivery error: %d.",
+subscr_name(conn->subscr), rc);
+ rc = GSM411_RP_CAUSE_MO_TEMP_FAIL;
+ /* rc will be logged by gsm411_send_rp_error() */
+ rate_ctr_inc(>bts->network->msc_ctrs->ctr[
+ MSC_CTR_SMS_DELIVER_UNKNOWN_ERROR]);
+ }
+#else
+ rc = GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
+ rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
+#endif
+
return rc;
}
--
To view, visit https://gerrit.osmocom.org/3899
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0c0bcd919cc3275d491995d17c6a32bb61c6afe1
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
libosmo-netif[master]: src: _snprintf() helper functions always nul-terminate buffers
Patch Set 1: (1 comment) https://gerrit.osmocom.org/#/c/3830/1/src/osmux.c File src/osmux.c: Line 941: /* This _snprintf() variant always nul-terminates the buffer. */ > I still don't get why you say at least some variants of snprintf don't cont Because I got confused. I remember your original patch is trying to ensure that we leave room for the nul-termination. Anyway, bottom line is: I don't we need this patch at all. Given that snprintf() output is always nul-terminated, the last snprintf() call already guarantees that we deliver a nul-terminated buffer. So all this logic this patch adds is entirely superfluous. I'm going to abbandon this patch. -- To view, visit https://gerrit.osmocom.org/3830 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I97e517f2d98e83894ea707c63489559302ff6bd2 Gerrit-PatchSet: 1 Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-HasComments: Yes
[PATCH] libosmo-netif[master]: osmux: fix buffer management mess in snprintf() calls
SNPRINTF_BUFFER_SIZE(ret, remain, offset);
this_len = sizeof(struct osmux_hdr) +
osmux_get_payload_len(osmuxh);
@@ -931,12 +931,10 @@
return -1;
}
- ret = osmux_snprintf_payload(buf+offset, size,
+ ret = osmux_snprintf_payload(buf + offset, remain,
osmux_get_payload(osmuxh),
osmux_get_payload_len(osmuxh));
- if (ret < 0)
- break;
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ SNPRINTF_BUFFER_SIZE(ret, remain, offset);
msg_len -= this_len;
}
diff --git a/src/rtp.c b/src/rtp.c
index 44fc217..56fc37c 100644
--- a/src/rtp.c
+++ b/src/rtp.c
@@ -185,19 +185,20 @@
return msg;
}
-#define SNPRINTF_BUFFER_SIZE(ret, size, len, offset) \
- size += ret;\
- if (ret > len) \
- ret = len; \
+#define SNPRINTF_BUFFER_SIZE(ret, remain, offset) \
+ if (ret < 0)\
+ ret = 0;\
offset += ret; \
- len -= ret;
+ if (ret > remain) \
+ ret = remain; \
+ remain -= ret;
int osmo_rtp_snprintf(char *buf, size_t size, struct msgb *msg)
{
+ unsigned int remain = size, offset = 0;
struct rtp_hdr *rtph;
- int ret, i;
uint8_t *payload;
- unsigned int len = size, offset = 0;
+ int ret, i;
rtph = osmo_rtp_get_hdr(msg);
if (rtph == NULL)
@@ -205,22 +206,22 @@
payload = (uint8_t *)rtph + sizeof(struct rtp_hdr);
- ret = snprintf(buf, len, "RTP ver=%01u ssrc=%u type=%02u "
+ ret = snprintf(buf, remain, "RTP ver=%01u ssrc=%u type=%02u "
"marker=%01u ext=%01u csrc_count=%01u "
"sequence=%u timestamp=%u [", rtph->version,
ntohl(rtph->ssrc), rtph->payload_type,
rtph->marker, rtph->extension,
rtph->csrc_count, ntohs(rtph->sequence),
ntohl(rtph->timestamp));
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ SNPRINTF_BUFFER_SIZE(ret, remain, offset);
for (i=0; ilen - sizeof(struct rtp_hdr); i++) {
- ret = snprintf(buf+offset, len, "%02x ", payload[i]);
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ ret = snprintf(buf + offset, remain, "%02x ", payload[i]);
+ SNPRINTF_BUFFER_SIZE(ret, remain, offset);
}
- ret = snprintf(buf+offset, len, "]");
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ ret = snprintf(buf + offset, remain, "]");
+ SNPRINTF_BUFFER_SIZE(ret, remain, offset);
return offset;
}
diff --git a/tests/osmux/osmux_test.c b/tests/osmux/osmux_test.c
index bf6174b..09b7a9a 100644
--- a/tests/osmux/osmux_test.c
+++ b/tests/osmux/osmux_test.c
@@ -65,8 +65,8 @@
static void tx_cb(struct msgb *msg, void *data)
{
- char buf[4096];
struct rtp_hdr *rtph = (struct rtp_hdr *)msg->data;
+ char buf[4096];
#if OSMUX_TEST_USE_TIMING
struct timeval now, diff;
@@ -102,9 +102,9 @@
static void osmux_deliver(struct msgb *batch_msg, void *data)
{
- char buf[2048];
struct osmux_hdr *osmuxh;
LLIST_HEAD(list);
+ char buf[2048];
osmux_snprintf(buf, sizeof(buf), batch_msg);
fprintf(stderr, "OSMUX message (len=%d) %s\n", batch_msg->len, buf);
@@ -182,11 +182,11 @@
static void osmux_test_loop(int ccid)
{
- struct msgb *msg;
- char buf[1024];
struct rtp_hdr *rtph = (struct rtp_hdr *)rtp_pkt;
- uint16_t seq;
+ struct msgb *msg;
int i, j, k = 0;
+ char buf[1024];
+ uint16_t seq;
for (i = 1; i < 65; i++) {
msg = msgb_alloc(1500, "test");
--
To view, visit https://gerrit.osmocom.org/3825
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5b5d6ec57a02f57c23b1ae86dbd894bad28ea797
Gerrit-PatchSet: 3
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de>
libosmo-netif[master]: osmux: fix buffer management mess in snprintf() calls
Patch Set 2: (1 comment) https://gerrit.osmocom.org/#/c/3825/2//COMMIT_MSG Commit Message: Line 31: As in snprintf(), caller should not assume the buffer is nul-terminated. > I think this is still not true and should be removed. Right, it's sprintf() the one that does not guarantee this. I'm going to submit a new version rewriting commit description. -- To view, visit https://gerrit.osmocom.org/3825 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5b5d6ec57a02f57c23b1ae86dbd894bad28ea797 Gerrit-PatchSet: 2 Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-HasComments: Yes
[MERGED] libosmo-netif[master]: rtp: return offset in osmo_rtp_snprintf()
Pablo Neira Ayuso has submitted this change and it was merged. Change subject: rtp: return offset in osmo_rtp_snprintf() .. rtp: return offset in osmo_rtp_snprintf() Instead of the result of the last snprintf() call. Change-Id: I10066d73387be96a4e1f3349d700405beb138076 --- M src/rtp.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Pau Espin Pedrol: Looks good to me, but someone else must approve Harald Welte: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/rtp.c b/src/rtp.c index 7ac5862..44fc217 100644 --- a/src/rtp.c +++ b/src/rtp.c @@ -222,5 +222,5 @@ ret = snprintf(buf+offset, len, "]"); SNPRINTF_BUFFER_SIZE(ret, size, len, offset); - return ret; + return offset; } -- To view, visit https://gerrit.osmocom.org/3824 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I10066d73387be96a4e1f3349d700405beb138076 Gerrit-PatchSet: 2 Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de>
[PATCH] libosmo-netif[master]: src: _snprintf() helper functions always nul-terminate buffers
Review at https://gerrit.osmocom.org/3830 src: _snprintf() helper functions always nul-terminate buffers Add the \0 for every case, even if this does not match exactly the snprintf semantics. Change-Id: I97e517f2d98e83894ea707c63489559302ff6bd2 --- M src/osmux.c M src/rtp.c 2 files changed, 15 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/30/3830/1 diff --git a/src/osmux.c b/src/osmux.c index b18246f..683c6e9 100644 --- a/src/osmux.c +++ b/src/osmux.c @@ -938,6 +938,13 @@ msg_len -= this_len; } + /* This _snprintf() variant always nul-terminates the buffer. */ + if (!offset) + buf[0] = '\0'; + else if (offset + 1 >= size) + buf[offset] = '\0'; + else + buf[++offset] = '\0'; return offset; } diff --git a/src/rtp.c b/src/rtp.c index 56fc37c..f524f73 100644 --- a/src/rtp.c +++ b/src/rtp.c @@ -223,5 +223,13 @@ ret = snprintf(buf + offset, remain, "]"); SNPRINTF_BUFFER_SIZE(ret, remain, offset); + /* This _snprintf() variant always nul-terminates the buffer. */ + if (!offset) + buf[0] = '\0'; + else if (offset + 1 >= size) + buf[offset] = '\0'; + else + buf[++offset] = '\0'; + return offset; } -- To view, visit https://gerrit.osmocom.org/3830 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I97e517f2d98e83894ea707c63489559302ff6bd2 Gerrit-PatchSet: 1 Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
[PATCH] libosmo-netif[master]: osmux: fix buffer management mess in snprintf() calls
break;
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ ret = osmux_snprintf_header(buf + offset, remain, osmuxh);
+ SNPRINTF_BUFFER_SIZE(ret, remain, offset);
this_len = sizeof(struct osmux_hdr) +
osmux_get_payload_len(osmuxh);
@@ -931,12 +931,10 @@
return -1;
}
- ret = osmux_snprintf_payload(buf+offset, size,
+ ret = osmux_snprintf_payload(buf + offset, remain,
osmux_get_payload(osmuxh),
osmux_get_payload_len(osmuxh));
- if (ret < 0)
- break;
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ SNPRINTF_BUFFER_SIZE(ret, remain, offset);
msg_len -= this_len;
}
diff --git a/src/rtp.c b/src/rtp.c
index 44fc217..56fc37c 100644
--- a/src/rtp.c
+++ b/src/rtp.c
@@ -185,19 +185,20 @@
return msg;
}
-#define SNPRINTF_BUFFER_SIZE(ret, size, len, offset) \
- size += ret;\
- if (ret > len) \
- ret = len; \
+#define SNPRINTF_BUFFER_SIZE(ret, remain, offset) \
+ if (ret < 0)\
+ ret = 0;\
offset += ret; \
- len -= ret;
+ if (ret > remain) \
+ ret = remain; \
+ remain -= ret;
int osmo_rtp_snprintf(char *buf, size_t size, struct msgb *msg)
{
+ unsigned int remain = size, offset = 0;
struct rtp_hdr *rtph;
- int ret, i;
uint8_t *payload;
- unsigned int len = size, offset = 0;
+ int ret, i;
rtph = osmo_rtp_get_hdr(msg);
if (rtph == NULL)
@@ -205,22 +206,22 @@
payload = (uint8_t *)rtph + sizeof(struct rtp_hdr);
- ret = snprintf(buf, len, "RTP ver=%01u ssrc=%u type=%02u "
+ ret = snprintf(buf, remain, "RTP ver=%01u ssrc=%u type=%02u "
"marker=%01u ext=%01u csrc_count=%01u "
"sequence=%u timestamp=%u [", rtph->version,
ntohl(rtph->ssrc), rtph->payload_type,
rtph->marker, rtph->extension,
rtph->csrc_count, ntohs(rtph->sequence),
ntohl(rtph->timestamp));
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ SNPRINTF_BUFFER_SIZE(ret, remain, offset);
for (i=0; ilen - sizeof(struct rtp_hdr); i++) {
- ret = snprintf(buf+offset, len, "%02x ", payload[i]);
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ ret = snprintf(buf + offset, remain, "%02x ", payload[i]);
+ SNPRINTF_BUFFER_SIZE(ret, remain, offset);
}
- ret = snprintf(buf+offset, len, "]");
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ ret = snprintf(buf + offset, remain, "]");
+ SNPRINTF_BUFFER_SIZE(ret, remain, offset);
return offset;
}
diff --git a/tests/osmux/osmux_test.c b/tests/osmux/osmux_test.c
index bf6174b..09b7a9a 100644
--- a/tests/osmux/osmux_test.c
+++ b/tests/osmux/osmux_test.c
@@ -65,8 +65,8 @@
static void tx_cb(struct msgb *msg, void *data)
{
- char buf[4096];
struct rtp_hdr *rtph = (struct rtp_hdr *)msg->data;
+ char buf[4096];
#if OSMUX_TEST_USE_TIMING
struct timeval now, diff;
@@ -102,9 +102,9 @@
static void osmux_deliver(struct msgb *batch_msg, void *data)
{
- char buf[2048];
struct osmux_hdr *osmuxh;
LLIST_HEAD(list);
+ char buf[2048];
osmux_snprintf(buf, sizeof(buf), batch_msg);
fprintf(stderr, "OSMUX message (len=%d) %s\n", batch_msg->len, buf);
@@ -182,11 +182,11 @@
static void osmux_test_loop(int ccid)
{
- struct msgb *msg;
- char buf[1024];
struct rtp_hdr *rtph = (struct rtp_hdr *)rtp_pkt;
- uint16_t seq;
+ struct msgb *msg;
int i, j, k = 0;
+ char buf[1024];
+ uint16_t seq;
for (i = 1; i < 65; i++) {
msg = msgb_alloc(1500, "test");
--
To view, visit https://gerrit.osmocom.org/3825
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5b5d6ec57a02f57c23b1ae86dbd894bad28ea797
Gerrit-PatchSet: 2
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de>
libosmo-netif[master]: osmux: Re-write osmux_snprintf
Patch Set 3: (1 comment) https://gerrit.osmocom.org/#/c/3537/3/src/osmux.c File src/osmux.c: Line 918: SNPRINTF_BUFFER_SIZE(ret, buf_offset, size); > Are you asking about this exact "No room for OSMUX payload" error case? OK, if the problem are truncated packet, then fix _snprintf() function to use msg->len instead of fetching the field from the RTP header. We shouldn't trust what the header announces. -- To view, visit https://gerrit.osmocom.org/3537 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I695771d099833842db37a415b636035d17f1bba7 Gerrit-PatchSet: 3 Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-HasComments: Yes
libosmo-netif[master]: osmux: Re-write osmux_snprintf
Patch Set 3: (2 comments) https://gerrit.osmocom.org/#/c/3537/3/src/osmux.c File src/osmux.c: Line 851: return ret; \ > I strongly object this coding style. Please no hidden return statements in Return hidden is macros is not good. I'm with Neels in this one. Line 918: SNPRINTF_BUFFER_SIZE(ret, buf_offset, size); > How come is the buffer getting full? AMR payload is very very small and we' I'm telling this because I suspect this is papering a problem somewhere else... -- To view, visit https://gerrit.osmocom.org/3537 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I695771d099833842db37a415b636035d17f1bba7 Gerrit-PatchSet: 3 Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-HasComments: Yes
libosmo-netif[master]: osmux: Re-write osmux_snprintf
Patch Set 1: @Pau: You're not the only one getting confused with it, snprintf() is a mess, it's hard to deal with it, hence this macro that retains semantics that aims to simplify things... if you find any better, let me know I'd be happy to reuse it in my code moving forward :-). @Holger: Either way, I don't mind if you don't need the snprintf semantics, this is a _snprintf() function after all, just explaning here. So the intention at least that I can remember was to keep in sync with how snprintf() works. Anyway, feel free to simplify this. Anyway, I just wanted to make sure this patch was really fixing up the real issue. I understand you observe a crash, but not clear to me why the patch is fixing it. Cheers. -- To view, visit https://gerrit.osmocom.org/3537 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I695771d099833842db37a415b636035d17f1bba7 Gerrit-PatchSet: 1 Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-HasComments: No
libosmo-netif[master]: osmux: Re-write osmux_snprintf
Patch Set 1: SOrry, I mean: If you want to retain snprintf semantics, ie. return the number of bytes that would have been written, then you need to keep 'len' and offset, because offset should not ever go over the boundary. But 'len' may indeed go over the boundary, since it expresses this "number of byte that would have been written". Under normal circumstances, this stands true: len == offset. If the buffer is too small, then len > offset. -- To view, visit https://gerrit.osmocom.org/3537 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I695771d099833842db37a415b636035d17f1bba7 Gerrit-PatchSet: 1 Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
libosmo-netif[master]: osmux: Re-write osmux_snprintf
Patch Set 1: size is the original buffer size, number of bytes you can fit into the buffer you provide. len is the number of byte that has been written into the buffer. offset is where you should continue to append more bytes. If you want to retain snprintf semantics, ie. return the number of bytes that would have been written, then you need to keep size and offset, because offset should not ever go over the boundary. Retaining snprintf semantics can be good to log how many bytes are missing in the buffer you provide. If you don't want snprintf semantics, then you can remove len and use offset in any case. -- To view, visit https://gerrit.osmocom.org/3537 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I695771d099833842db37a415b636035d17f1bba7 Gerrit-PatchSet: 1 Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
openbsc[master]: libmsc: Fix wrong handling of user_message_reference parameter
Patch Set 2: Code-Review+1 Thanks a lot for fixing up this Pau. -- To view, visit https://gerrit.osmocom.org/3554 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If748548a4a223e529a1110c89e483b599b406e8b Gerrit-PatchSet: 2 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
libosmocore[master]: gsm0411_utils: GSM03.40 9.2.3.11 SCTS should be local
Patch Set 1: My impression is that noone didn't have the resources/time to have a careful look at this so far... So now you're the GSM03.40 9.2.3.11 SCTS expert ;-). These small details are a bit non-essencial (in terms of network operation) but they can be very time-consuming... I think follow up patches to polish this would be good if times allows there. -- To view, visit https://gerrit.osmocom.org/3551 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4efdb1eaae43aced33961b64d4f14b0040321c10 Gerrit-PatchSet: 1 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
libosmo-netif[master]: osmux: Re-write osmux_snprintf
Patch Set 1: (1 comment) https://gerrit.osmocom.org/#/c/3537/1/src/osmux.c File src/osmux.c: PS1, Line 849: I'm still trying to understand this update, not sure what corner case you're trying to catch. Why this code below not just enough to address this? http://git.netfilter.org/nftables/tree/include/utils.h#n84 -- To view, visit https://gerrit.osmocom.org/3537 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I695771d099833842db37a415b636035d17f1bba7 Gerrit-PatchSet: 1 Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: Yes
libosmocore[master]: gsm0411_utils: GSM03.40 9.2.3.11 SCTS should be local
Patch Set 1: Code-Review+1 This oneliner LGTM. Is this fixing up the issue you were observing with Delivery Reports with wrong dates? -- To view, visit https://gerrit.osmocom.org/3551 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4efdb1eaae43aced33961b64d4f14b0040321c10 Gerrit-PatchSet: 1 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
openbsc[master]: libmsc: Use actual delivery time in delivery reports.
Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/3540 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9056429d40bf02731f004b7833f1de45a0d1add8 Gerrit-PatchSet: 3 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
openbsc[master]: SMS Delivery reports: Use actual delivery time, not time of ...
Patch Set 1: (1 comment) https://gerrit.osmocom.org/#/c/3540/1/openbsc/include/openbsc/gsm_data.h File openbsc/include/openbsc/gsm_data.h: Line 460: time_t created; This creates a "hole" in the structure, I would place it before "bool is_report;" See "pahole" utility for what I mean, it shows the structure layout in binary, including alignments. This is not a big issue here, since neither reducing memory is a real concern nor trying to fit this structure into a cacheline to speed up things (performance) is a major goal I would say... but it's a good thing in general to keep things tidy in this regard. It's just a matter of placing this new field definition before bool is_report; so it doesn't take much time to do this. -- To view, visit https://gerrit.osmocom.org/3540 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9056429d40bf02731f004b7833f1de45a0d1add8 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: Yes
openbsc[master]: SMS Delivery reports: Use actual delivery time, not time of ...
Patch Set 1: Code-Review+1 (3 comments) Apart from the obvious the existing issues, it's great you have tracked down and fixed this little aspect, so this is my +1 :) https://gerrit.osmocom.org/#/c/3540/1//COMMIT_MSG Commit Message: Line 7: SMS Delivery reports: Use actual delivery time, not time of report creation. Probably good to keep using consistent names in patch titles, so I would propose as patch title: "libmsc: Use actual delivery time in delivery reports" Not sure there's a policy in osmocom, but this info is good for grepping in logs from time to time. Anyway, not a big issue. https://gerrit.osmocom.org/#/c/3540/1/openbsc/src/libmsc/db.c File openbsc/src/libmsc/db.c: Line 1708: "SELECT datetime(SMS.created,'localtime') as created, SMS.* " > Hmm, this passes the test on my build box, but I guess I was being lazy her Probably this is breaking openbsc/tests/db/db_test ? https://gerrit.osmocom.org/#/c/3540/1/openbsc/src/libmsc/gsm_04_11.c File openbsc/src/libmsc/gsm_04_11.c: Line 313: gsm340_gen_scts(smsp, sms->created); > Maybe this one should be "now" Not sure what your question is? :-) -- To view, visit https://gerrit.osmocom.org/3540 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9056429d40bf02731f004b7833f1de45a0d1add8 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: Yes
[PATCH] openbsc[master]: libmsc: use SMPP34_DELIVERY_RECEIPT_* in libsmpp34
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/3515
to look at the new patch set (#2).
libmsc: use SMPP34_DELIVERY_RECEIPT_* in libsmpp34
Use new definitions in libsmpp34 to set the registered_delivery field
accordingly, as provided by I5b3afff1b3b77cccd949e0606914c7ac3ba6114c.
Moreover, do not set this header field to zero if status reports are
off, the deliver_t structure has been already zeroed so this not
required.
Change-Id: Ie78e17323796120f576b9c0e1bc5ccc32da8ee12
---
M openbsc/src/libmsc/smpp_openbsc.c
1 file changed, 1 insertion(+), 7 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/15/3515/2
diff --git a/openbsc/src/libmsc/smpp_openbsc.c
b/openbsc/src/libmsc/smpp_openbsc.c
index ddb24b5..f2a036d 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -627,10 +627,6 @@
return NULL;
}
-/* See SMPP 3.4, Section 5.2.17. registered_delivery (1 byte field). */
-#define SMPP34_NO_DELIVERY_RECEIPT 0x0
-#define SMPP34_DELIVERY_RECEIPT_REQ0x1
-
static int deliver_to_esme(struct osmo_esme *esme, struct gsm_sms *sms,
struct gsm_subscriber_connection *conn)
{
@@ -676,9 +672,7 @@
deliver.protocol_id = sms->protocol_id;
deliver.priority_flag = 0;
if (sms->status_rep_req)
- deliver.registered_delivery = SMPP34_DELIVERY_RECEIPT_REQ;
- else
- deliver.registered_delivery = SMPP34_NO_DELIVERY_RECEIPT;
+ deliver.registered_delivery = SMPP34_DELIVERY_RECEIPT_ON;
/* Figure out SMPP DCS from TP-DCS */
dcs = sms->data_coding_scheme;
--
To view, visit https://gerrit.osmocom.org/3515
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie78e17323796120f576b9c0e1bc5ccc32da8ee12
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
openbsc[master]: smpp: Fix compilation warning
Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/3524 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0901ddadb5f72e1585cb1797ac22c8ab95e83146 Gerrit-PatchSet: 2 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
openbsc[master]: smpp: Fix compilation warning
Patch Set 1: (1 comment) https://gerrit.osmocom.org/#/c/3524/1/openbsc/src/libmsc/smpp_openbsc.c File openbsc/src/libmsc/smpp_openbsc.c: Line 100: unsigned int sms_msg_len = 0; /* initialization not needed but compilers cry with -Wmaybe-uninitialized */ This comment on the right is too much... We have 'git annotate' these days... BTW, in the kernel, this is preferred: const uint8_t *sms_msg = NULL; unsigned int sms_msg_len = 0; struct gsm_subscriber *dest; uint16_t msg_ref = 0; struct gsm_sms *sms; struct tlv_t *t; ie. variables are declared from longer line to smaller line, it makes things more readable... Just a couple of nitpicks... Nothing very much relevant here. -- To view, visit https://gerrit.osmocom.org/3524 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0901ddadb5f72e1585cb1797ac22c8ab95e83146 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: Yes
openbsc[master]: LIBMSC: Place correct dst address in status report
Patch Set 2: Code-Review+1 Thanks for fixing up this! -- To view, visit https://gerrit.osmocom.org/3516 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2d4f87ac777465de9bfb5a775a789a2691755ee9 Gerrit-PatchSet: 2 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
[MERGED] libsmpp34[master]: add delivery receipt definitions
Pablo Neira Ayuso has submitted this change and it was merged. Change subject: add delivery receipt definitions .. add delivery receipt definitions The registered_delivery field in SMPP message is a toggle on/off, this patch adds two new definition so we don't have to use magic numbers from the code. Change-Id: I5b3afff1b3b77cccd949e0606914c7ac3ba6114c --- M src/smpp34.h 1 file changed, 4 insertions(+), 0 deletions(-) Approvals: Harald Welte: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/smpp34.h b/src/smpp34.h index 9d51a4a..b66d47f 100644 --- a/src/smpp34.h +++ b/src/smpp34.h @@ -212,4 +212,8 @@ #define SMPP34_UDHI_IND0x40 #define SMPP34_REPLY_PATH 0x80 +/* Sect. 5.2.17 registered_delivery. */ +#define SMPP34_DELIVERY_RECEIPT_OFF0x00 +#define SMPP34_DELIVERY_RECEIPT_ON 0x01 + #endif -- To view, visit https://gerrit.osmocom.org/3513 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5b3afff1b3b77cccd949e0606914c7ac3ba6114c Gerrit-PatchSet: 1 Gerrit-Project: libsmpp34 Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
libsmpp34[master]: add delivery receipt definitions
Patch Set 1: My queue is now empty :-), unless someone needs any more cleanups ;-) -- To view, visit https://gerrit.osmocom.org/3513 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5b3afff1b3b77cccd949e0606914c7ac3ba6114c Gerrit-PatchSet: 1 Gerrit-Project: libsmpp34 Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
openbsc[master]: libmsc: use SMPP34_DELIVERY_RECEIPT_* in libsmpp34
Patch Set 1: Just for the record, this patch depends on: https://gerrit.osmocom.org/#/c/3513/ that's why Jenkins fails to build this. -- To view, visit https://gerrit.osmocom.org/3515 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie78e17323796120f576b9c0e1bc5ccc32da8ee12 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
[PATCH] openbsc[master]: libmsc: use SMPP34_DELIVERY_RECEIPT_* in libsmpp34
Review at https://gerrit.osmocom.org/3515
libmsc: use SMPP34_DELIVERY_RECEIPT_* in libsmpp34
Use new definitions for the registered_delivery field in libsmpp34 with
I5b3afff1b3b77cccd949e0606914c7ac3ba6114c.
Moreover, do not set this header field to zero if status reports are
off, the deliver_t structure has been already zeroed so this not
required.
Change-Id: Ie78e17323796120f576b9c0e1bc5ccc32da8ee12
---
M openbsc/src/libmsc/smpp_openbsc.c
1 file changed, 1 insertion(+), 7 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/15/3515/1
diff --git a/openbsc/src/libmsc/smpp_openbsc.c
b/openbsc/src/libmsc/smpp_openbsc.c
index ddb24b5..f2a036d 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -627,10 +627,6 @@
return NULL;
}
-/* See SMPP 3.4, Section 5.2.17. registered_delivery (1 byte field). */
-#define SMPP34_NO_DELIVERY_RECEIPT 0x0
-#define SMPP34_DELIVERY_RECEIPT_REQ0x1
-
static int deliver_to_esme(struct osmo_esme *esme, struct gsm_sms *sms,
struct gsm_subscriber_connection *conn)
{
@@ -676,9 +672,7 @@
deliver.protocol_id = sms->protocol_id;
deliver.priority_flag = 0;
if (sms->status_rep_req)
- deliver.registered_delivery = SMPP34_DELIVERY_RECEIPT_REQ;
- else
- deliver.registered_delivery = SMPP34_NO_DELIVERY_RECEIPT;
+ deliver.registered_delivery = SMPP34_DELIVERY_RECEIPT_ON;
/* Figure out SMPP DCS from TP-DCS */
dcs = sms->data_coding_scheme;
--
To view, visit https://gerrit.osmocom.org/3515
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie78e17323796120f576b9c0e1bc5ccc32da8ee12
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
[PATCH] libsmpp34[master]: add delivery receipt definitions
Review at https://gerrit.osmocom.org/3513 add delivery receipt definitions The registered_delivery field in SMPP message is a toggle on/off, this patch adds two new definition so we don't have to use magic numbers from the code. Change-Id: I5b3afff1b3b77cccd949e0606914c7ac3ba6114c --- M src/smpp34.h 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libsmpp34 refs/changes/13/3513/1 diff --git a/src/smpp34.h b/src/smpp34.h index 9d51a4a..b66d47f 100644 --- a/src/smpp34.h +++ b/src/smpp34.h @@ -212,4 +212,8 @@ #define SMPP34_UDHI_IND0x40 #define SMPP34_REPLY_PATH 0x80 +/* Sect. 5.2.17 registered_delivery. */ +#define SMPP34_DELIVERY_RECEIPT_OFF0x00 +#define SMPP34_DELIVERY_RECEIPT_ON 0x01 + #endif -- To view, visit https://gerrit.osmocom.org/3513 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5b3afff1b3b77cccd949e0606914c7ac3ba6114c Gerrit-PatchSet: 1 Gerrit-Project: libsmpp34 Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
osmo-msc[master]: 04.08: log protocol discriminators and message types by name
Patch Set 6: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/3350 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ida205d217e304337d816b14fd15e2ee435e7397d Gerrit-PatchSet: 6 Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Owner: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
osmo-gsm-manuals[master]: osmux: Fix description for Dummy frames FT field
Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/3478 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia421655bd1be45101da3db2a0af44fbb3cc111c1 Gerrit-PatchSet: 1 Gerrit-Project: osmo-gsm-manuals Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
openbsc[master]: mgcp_osmux: Remove unused parameter
Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/3483 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icb1e7cb15fe04642578f5292124ebc1eac9c9aa3 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
[PATCH] openbsc[master]: libmsc: use smpp34_tlv_for_each() to avoid suboptimal TLV ha...
VID_user_message_reference); - if (t) - sms->msg_ref = ntohs(t->value.val16); *psms = sms; return ESME_ROK; -- To view, visit https://gerrit.osmocom.org/3470 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I53a65164a6cc4abc6bf57d9a8dc275cf21c90222 Gerrit-PatchSet: 3 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de>
[PATCH] libsmpp34[master]: add esm_class definitions
Hello Harald Welte, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/3473 to look at the new patch set (#2). add esm_class definitions Add special message attributes definitions that are associated with the short message. Thus, we can get rid of magic numbers in our codebase. Change-Id: I91afd8b462b8fd3b2c4c5b54f4eeb7ec5b730b65 --- M src/smpp34.h 1 file changed, 8 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libsmpp34 refs/changes/73/3473/2 diff --git a/src/smpp34.h b/src/smpp34.h index 5fb6395..9d51a4a 100644 --- a/src/smpp34.h +++ b/src/smpp34.h @@ -204,4 +204,12 @@ #define TLVID_its_reply_type 0x1380 /* CDMA */ #define TLVID_its_session_info0x1383 /* CDMA */ +/* As defined by SMPP 3.4, Sect. 5.2.12 Parameter Definition esm_class. */ +#define SMPP34_DATAGRAM_MODE 0x01 +#define SMPP34_MSG_MODE_MASK 0x03 +#define SMPP34_DELIVERY_RECEIPT0x04 +#define SMPP34_DELIVERY_ACK0x08 +#define SMPP34_UDHI_IND0x40 +#define SMPP34_REPLY_PATH 0x80 + #endif -- To view, visit https://gerrit.osmocom.org/3473 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I91afd8b462b8fd3b2c4c5b54f4eeb7ec5b730b65 Gerrit-PatchSet: 2 Gerrit-Project: libsmpp34 Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
[MERGED] libsmpp34[master]: add smpp34_tlv_for_each() helper
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: add smpp34_tlv_for_each() helper
..
add smpp34_tlv_for_each() helper
So we don't need to remember this is an opencoded list of TLVs.
Change-Id: I446929feed049d0411e1629ca263e2bc41f714cc
---
M src/smpp34_structs.h
1 file changed, 3 insertions(+), 0 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/smpp34_structs.h b/src/smpp34_structs.h
index b9effb2..025109d 100644
--- a/src/smpp34_structs.h
+++ b/src/smpp34_structs.h
@@ -86,6 +86,9 @@
OCTET16( inst, octet, size); \
} par;
+#define smpp34_tlv_for_each(pos, head) \
+ for (pos = (head); pos != NULL; pos = pos->next)
+
#define DAD( inst, par, do_dest_address ) dad_t *par;
#define UU2( inst, par, size ) union { \
struct { \
--
To view, visit https://gerrit.osmocom.org/3469
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I446929feed049d0411e1629ca263e2bc41f714cc
Gerrit-PatchSet: 1
Gerrit-Project: libsmpp34
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
openbsc[master]: libmsc: use smpp34_tlv_for_each() to avoid suboptimal TLV ha...
Patch Set 2: Oh, I forgot. You can probably speed up the process if you give me +X to my libsmpp34 patches (if you're familiar with that code). Thanks! -- To view, visit https://gerrit.osmocom.org/3470 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I53a65164a6cc4abc6bf57d9a8dc275cf21c90222 Gerrit-PatchSet: 2 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-HasComments: No
openbsc[master]: libmsc: use smpp34_tlv_for_each() to avoid suboptimal TLV ha...
Patch Set 2: OK, will include this info once patches for libsmpp34 get in. So this also helps re-kick Jenkins building. -- To view, visit https://gerrit.osmocom.org/3470 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I53a65164a6cc4abc6bf57d9a8dc275cf21c90222 Gerrit-PatchSet: 2 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-HasComments: No
openbsc[master]: libmsc: use new smpp34 esm_class definitions
Patch Set 1: This jenkins build will fail too, because it depends on a patch in libsmpp34 that I have just submitted. -- To view, visit https://gerrit.osmocom.org/3474 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c458690da60c8f3637680efbd718f6e8c6feb4c Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
[PATCH] openbsc[master]: libmsc: use new smpp34 esm_class definitions
Review at https://gerrit.osmocom.org/3474
libmsc: use new smpp34 esm_class definitions
Replace magic numbers by esm_class definitions.
Change-Id: I6c458690da60c8f3637680efbd718f6e8c6feb4c
---
M openbsc/src/libmsc/smpp_openbsc.c
1 file changed, 8 insertions(+), 10 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/74/3474/1
diff --git a/openbsc/src/libmsc/smpp_openbsc.c
b/openbsc/src/libmsc/smpp_openbsc.c
index 7e23abd..ddb24b5 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -157,14 +157,13 @@
osmo_strlcpy(sms->src.addr, (char *)submit->source_addr,
sizeof(sms->src.addr));
- /* This is a Delivery Acknowledgment. */
- if (submit->esm_class == 0x08)
+ if (submit->esm_class == SMPP34_DELIVERY_ACK)
sms->is_report = true;
- if (submit->esm_class & 0x40)
+ if (submit->esm_class & SMPP34_UDHI_IND)
sms->ud_hdr_ind = 1;
- if (submit->esm_class & 0x80) {
+ if (submit->esm_class & SMPP34_REPLY_PATH) {
sms->reply_path_req = 1;
#warning Implement reply path
}
@@ -240,7 +239,7 @@
sms->smpp.esme = esme;
sms->protocol_id = submit->protocol_id;
- switch (submit->esm_class & 3) {
+ switch (submit->esm_class & SMPP34_MSG_MODE_MASK) {
case 0: /* default */
case 1: /* datagram */
case 3: /* store-and-forward */
@@ -664,16 +663,15 @@
memcpy(deliver.destination_addr, sms->dst.addr,
sizeof(deliver.destination_addr));
- /* Short message contains a delivery receipt? Sect. 5.2.12. */
if (sms->is_report)
- deliver.esm_class = 0x04;
+ deliver.esm_class = SMPP34_DELIVERY_RECEIPT;
else
- deliver.esm_class = 1; /* datagram mode */
+ deliver.esm_class = SMPP34_DATAGRAM_MODE;
if (sms->ud_hdr_ind)
- deliver.esm_class |= 0x40;
+ deliver.esm_class |= SMPP34_UDHI_IND;
if (sms->reply_path_req)
- deliver.esm_class |= 0x80;
+ deliver.esm_class |= SMPP34_REPLY_PATH;
deliver.protocol_id = sms->protocol_id;
deliver.priority_flag = 0;
--
To view, visit https://gerrit.osmocom.org/3474
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6c458690da60c8f3637680efbd718f6e8c6feb4c
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
openbsc[master]: libmsc: use smpp34_tlv_for_each() to avoid suboptimal TLV ha...
Patch Set 2: Build breaks because this patch depends on a patch in libsmpp34. -- To view, visit https://gerrit.osmocom.org/3470 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I53a65164a6cc4abc6bf57d9a8dc275cf21c90222 Gerrit-PatchSet: 2 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
[PATCH] libsmpp34[master]: add esm_class definitions
Review at https://gerrit.osmocom.org/3473
add esm_class definitions
Add special message attributes definitions that are associated with the
short message. Thus, we can get rid of magic numbers in our codebase.
Change-Id: I91afd8b462b8fd3b2c4c5b54f4eeb7ec5b730b65
---
M src/smpp34.h
1 file changed, 11 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libsmpp34 refs/changes/73/3473/1
diff --git a/src/smpp34.h b/src/smpp34.h
index 5fb6395..b114ba7 100644
--- a/src/smpp34.h
+++ b/src/smpp34.h
@@ -204,4 +204,15 @@
#define TLVID_its_reply_type 0x1380 /* CDMA */
#define TLVID_its_session_info0x1383 /* CDMA */
+/* As defined by SMPP 3.4, Sect. 5.2.12 Parameter Definition esm_class. */
+enum smpp34_esm_class {
+ SMPP34_DATAGRAM_MODE= 0x01,
+ SMPP34_DELIVERY_RECEIPT = 0x04,
+ SMPP34_DELIVERY_ACK = 0x08,
+ SMPP34_UDHI_IND = 0x40,
+ SMPP34_REPLY_PATH = 0x80,
+};
+
+#define SMPP34_MSG_MODE_MASK 0x03
+
#endif
--
To view, visit https://gerrit.osmocom.org/3473
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I91afd8b462b8fd3b2c4c5b54f4eeb7ec5b730b65
Gerrit-PatchSet: 1
Gerrit-Project: libsmpp34
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
openbsc[master]: libmsc: update database to accomodate SMS status-report fields
Patch Set 4: Agreed. So let it sink by when the NITB code becomes officially dead. Thanks! -- To view, visit https://gerrit.osmocom.org/3435 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7276d356d805a83ebeec72b02c8563b7135ea0b6 Gerrit-PatchSet: 4 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
[PATCH] openbsc[master]: libmsc: use smpp34_tlv_for_each() to avoid suboptimal TLV ha...
bscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I53a65164a6cc4abc6bf57d9a8dc275cf21c90222 Gerrit-PatchSet: 2 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder
[PATCH] openbsc[master]: libmsc: use smpp34_tlv_for_each() to avoid suboptimal TLV ha...
return ESME_ROK; -- To view, visit https://gerrit.osmocom.org/3470 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I53a65164a6cc4abc6bf57d9a8dc275cf21c90222 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
openbsc[master]: libmsc: update database to accomodate SMS status-report fields
Patch Set 4: When would it be good time to remove code for backward compatibility? Holger added v3 by 2014-03-07, it's a bit more than 3 years now. -- To view, visit https://gerrit.osmocom.org/3435 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7276d356d805a83ebeec72b02c8563b7135ea0b6 Gerrit-PatchSet: 4 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
[PATCH] libsmpp34[master]: add smpp34_tlv_for_each() helper
Review at https://gerrit.osmocom.org/3469
add smpp34_tlv_for_each() helper
So we don't need to remember this is an opencoded list of TLVs.
Change-Id: I446929feed049d0411e1629ca263e2bc41f714cc
---
M src/smpp34_structs.h
1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libsmpp34 refs/changes/69/3469/1
diff --git a/src/smpp34_structs.h b/src/smpp34_structs.h
index b9effb2..025109d 100644
--- a/src/smpp34_structs.h
+++ b/src/smpp34_structs.h
@@ -86,6 +86,9 @@
OCTET16( inst, octet, size); \
} par;
+#define smpp34_tlv_for_each(pos, head) \
+ for (pos = (head); pos != NULL; pos = pos->next)
+
#define DAD( inst, par, do_dest_address ) dad_t *par;
#define UU2( inst, par, size ) union { \
struct { \
--
To view, visit https://gerrit.osmocom.org/3469
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I446929feed049d0411e1629ca263e2bc41f714cc
Gerrit-PatchSet: 1
Gerrit-Project: libsmpp34
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
[PATCH] openbsc[master]: libmsc: gsm340_gen_oa_sub() may return negative value
Review at https://gerrit.osmocom.org/3461
libmsc: gsm340_gen_oa_sub() may return negative value
gsm340_gen_oa() returns a negative value if the output buffer that the
caller passes is too small, so we have to check the return value of this
function.
Fixes: CID 174178
Fixes: CID 174179
Change-Id: I47215d7d89771730a7f84efa8aeeb187a0911fdb
---
M openbsc/src/libmsc/gsm_04_11.c
1 file changed, 9 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/61/3461/1
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index 73e0f55..8b4ffce 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -213,9 +213,9 @@
{
uint8_t *smsp;
uint8_t oa[12]; /* max len per 03.40 */
- uint8_t oa_len = 0;
uint8_t octet_len;
unsigned int old_msg_len = msg->len;
+ int oa_len;
/* generate first octet with masked bits */
smsp = msgb_put(msg, 1);
@@ -233,6 +233,9 @@
/* generate originator address */
oa_len = gsm340_gen_oa_sub(oa, sizeof(oa), >src);
+ if (oa_len < 0)
+ return -ENOSPC;
+
smsp = msgb_put(msg, oa_len);
memcpy(smsp, oa, oa_len);
@@ -282,9 +285,9 @@
struct gsm_sms *sms)
{
unsigned int old_msg_len = msg->len;
- uint8_t oa_len = 0;
uint8_t oa[12]; /* max len per 03.40 */
uint8_t *smsp;
+ int oa_len;
/* generate first octet with masked bits */
smsp = msgb_put(msg, 1);
@@ -296,8 +299,12 @@
/* TP-MR (message reference) */
smsp = msgb_put(msg, 1);
*smsp = sms->msg_ref;
+
/* generate recipient address */
oa_len = gsm340_gen_oa_sub(oa, sizeof(oa), >dst);
+ if (oa_len < 0)
+ return -ENOSPC;
+
smsp = msgb_put(msg, oa_len);
memcpy(smsp, oa, oa_len);
--
To view, visit https://gerrit.osmocom.org/3461
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I47215d7d89771730a7f84efa8aeeb187a0911fdb
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
[MERGED] openbsc[master]: libmsc: handle delivery ack via SMPP SUBMIT SM / send GSM 03...
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: libmsc: handle delivery ack via SMPP SUBMIT SM / send GSM 03.40
status report
..
libmsc: handle delivery ack via SMPP SUBMIT SM / send GSM 03.40 status report
This patch adds gsm340_sms_send_status_report_tpdu() to build a
status-report. Moreover, set sms->report field if we see a SMPP
SUBMIT_SM with Delivery Acknowledgment esm_class, so this identifies
that this is a delivery report.
MSGSM 03.40 SMSC SMPP 3.4 ESME
| ||
| | SUBMIT-SM|
| |esm_class = Delivery Ack|
| |<---|
| | SUBMIT-SM-RESP |
| |--->|
| ||
| SMS-STATUS-REPORT ||
|<||
| GSM 04.11 RP-ACK||
|>||
| ||
There is a FIXME message in this patch, that I just copied from
gsm340_gen_sms_deliver_tpdu() since TP-MMS is not supported by OpenBSC.
Change-Id: Ib70e534840308ed315f7add440351e649de3f907
---
M openbsc/src/libmsc/gsm_04_11.c
M openbsc/src/libmsc/smpp_openbsc.c
2 files changed, 54 insertions(+), 2 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Pablo Neira Ayuso: Verified
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index 0edbe0b..80c3d77 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -277,6 +277,49 @@
return msg->len - old_msg_len;
}
+/* As defined by GSM 03.40, Section 9.2.2.3. */
+static int gsm340_gen_sms_status_report_tpdu(struct msgb *msg,
+struct gsm_sms *sms)
+{
+ unsigned int old_msg_len = msg->len;
+ uint8_t oa_len = 0;
+ uint8_t oa[12]; /* max len per 03.40 */
+ uint8_t *smsp;
+
+ /* generate first octet with masked bits */
+ smsp = msgb_put(msg, 1);
+ /* TP-MTI (message type indicator) */
+ *smsp = GSM340_SMS_STATUS_REP_SC2MS;
+ /* TP-MMS (more messages to send) */
+ if (0 /* FIXME */)
+ *smsp |= 0x04;
+ /* TP-MR (message reference) */
+ smsp = msgb_put(msg, 1);
+ *smsp = sms->msg_ref;
+ /* generate recipient address */
+ oa_len = gsm340_gen_oa_sub(oa, sizeof(oa), >dst);
+ smsp = msgb_put(msg, oa_len);
+ memcpy(smsp, oa, oa_len);
+
+ /* generate TP-SCTS (Service centre timestamp) */
+ smsp = msgb_put(msg, 7);
+ gsm340_gen_scts(smsp, time(NULL));
+
+ /* generate TP-DT (Discharge time, in TP-SCTS format). */
+ smsp = msgb_put(msg, 7);
+ gsm340_gen_scts(smsp, time(NULL));
+
+ /* TP-ST (status) */
+ smsp = msgb_put(msg, 1);
+ /* From GSM 03.40, Section 9.2.3.15, 0x00 means OK. */
+ *smsp = 0x00;
+
+ LOGP(DLSMS, LOGL_INFO, "sending status report for SMS reference %x\n",
+sms->msg_ref);
+
+ return msg->len - old_msg_len;
+}
+
static int sms_route_mt_sms(struct gsm_subscriber_connection *conn,
struct gsm_sms *gsms)
{
@@ -989,8 +1032,13 @@
/* obtain a pointer for the rp_ud_len, so we can fill it later */
rp_ud_len = (uint8_t *)msgb_put(msg, 1);
- /* generate the 03.40 SMS-DELIVER TPDU */
- rc = gsm340_gen_sms_deliver_tpdu(msg, sms);
+ if (sms->is_report) {
+ /* generate the 03.40 SMS-STATUS-REPORT TPDU */
+ rc = gsm340_gen_sms_status_report_tpdu(msg, sms);
+ } else {
+ /* generate the 03.40 SMS-DELIVER TPDU */
+ rc = gsm340_gen_sms_deliver_tpdu(msg, sms);
+ }
if (rc < 0) {
send_signal(S_SMS_UNKNOWN_ERROR, trans, sms, 0);
sms_free(sms);
diff --git a/openbsc/src/libmsc/smpp_openbsc.c
b/openbsc/src/libmsc/smpp_openbsc.c
index c0aa89b..85de040 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -143,6 +143,10 @@
osmo_strlcpy(sms->src.addr, (char *)submit->source_addr,
sizeof(sms->src.addr));
+ /* This is a Delivery Acknowledgment. */
+ if (submit->esm_class == 0x08)
+ sms->is_report = true;
+
if (submit->esm_class & 0x40)
sms->ud_hdr_ind = 1;
--
To view, visit https:
[MERGED] openbsc[master]: libmsc: support GSM 03.40 status report for nitb
Pablo Neira Ayuso has submitted this change and it was merged. Change subject: libmsc: support GSM 03.40 status report for nitb .. libmsc: support GSM 03.40 status report for nitb This patch adds support for GSM 03.40 in nitb mode. MS GSM 03.40 SMSC || | SMS-SUBMIT | |--->| | GSM 04.11 RP-ACK | |<---| | SMS-DELIVER| |<---| | GSM 04.11 RP-ACK | |--->| | SMS-STATUS-REPORT | |<---| | GSM 04.11 RP-ACK | |--->| || Change-Id: I5cc7bb4ebadde0940f44d10c3df34707b0615160 --- M openbsc/src/libmsc/gsm_04_11.c 1 file changed, 5 insertions(+), 0 deletions(-) Approvals: Keith Whyte: Verified Harald Welte: Looks good to me, approved diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c index 80c3d77..73e0f55 100644 --- a/openbsc/src/libmsc/gsm_04_11.c +++ b/openbsc/src/libmsc/gsm_04_11.c @@ -682,6 +682,11 @@ LOGP(DLSMS, LOGL_ERROR, "Failed to send status report! err=%d\n", rc); } + + /* No route via SMPP, send the GSM 03.40 status-report now. */ + if (gsms->receiver) + gsm340_rx_sms_submit(sms_report); + LOGP(DLSMS, LOGL_NOTICE, "Status report has been sent\n"); sms_free(sms_report); -- To view, visit https://gerrit.osmocom.org/3438 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5cc7bb4ebadde0940f44d10c3df34707b0615160 Gerrit-PatchSet: 4 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
openbsc[master]: libmsc: handle delivery ack via SMPP SUBMIT SM / send GSM 03...
Patch Set 4: Verified+1 -- To view, visit https://gerrit.osmocom.org/3437 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib70e534840308ed315f7add440351e649de3f907 Gerrit-PatchSet: 4 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
[MERGED] openbsc[master]: utils: smpp_mirror: bounce Delivery Receipts as Delivery Ack...
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: utils: smpp_mirror: bounce Delivery Receipts as Delivery
Acknowledgments
..
utils: smpp_mirror: bounce Delivery Receipts as Delivery Acknowledgments
Simple patch to test the new status-report support code, remove previous
code before Delivery Acknowledgement support was in place. Use
LOGL_DEBUG for logging messages here as suggested by Neels and Harald.
Change-Id: I877e228d8e174430f700631edbf9955972da7892
---
M openbsc/src/utils/smpp_mirror.c
1 file changed, 8 insertions(+), 9 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Pablo Neira Ayuso: Verified
diff --git a/openbsc/src/utils/smpp_mirror.c b/openbsc/src/utils/smpp_mirror.c
index 88545de..c570505 100644
--- a/openbsc/src/utils/smpp_mirror.c
+++ b/openbsc/src/utils/smpp_mirror.c
@@ -123,14 +123,6 @@
PACK_AND_SEND(esme, _r);
- /* This is a delivery receipt, temporarily munch it until we teach
-* openbsc what to do with this.
-*/
- if (deliver.esm_class == 0x04) {
- LOGP(DSMPP, LOGL_NOTICE, "%s\n", deliver.short_message);
- return 0;
- }
-
memset(, 0, sizeof(submit));
submit.command_id = SUBMIT_SM;
submit.command_status = ESME_ROK;
@@ -148,7 +140,14 @@
OSMO_MIN(sizeof(submit.source_addr),
sizeof(deliver.destination_addr)));
- submit.esm_class = deliver.esm_class;
+ /* Mirror delivery receipts as a delivery acknowledgements. */
+ if (deliver.esm_class == 0x04) {
+ LOGP(DSMPP, LOGL_DEBUG, "%s\n", deliver.short_message);
+ submit.esm_class = 0x08;
+ } else {
+ submit.esm_class = deliver.esm_class;
+ }
+
submit.registered_delivery = deliver.registered_delivery;
submit.protocol_id = deliver.protocol_id;
submit.priority_flag = deliver.priority_flag;
--
To view, visit https://gerrit.osmocom.org/3436
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I877e228d8e174430f700631edbf9955972da7892
Gerrit-PatchSet: 4
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Holger Freyther <hol...@freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
openbsc[master]: utils: smpp_mirror: bounce Delivery Receipts as Delivery Ack...
Patch Set 4: Verified+1 -- To view, visit https://gerrit.osmocom.org/3436 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I877e228d8e174430f700631edbf9955972da7892 Gerrit-PatchSet: 4 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
openbsc[master]: libmsc: update database to accomodate SMS status-report fields
Patch Set 4: Verified+1 (1 comment) https://gerrit.osmocom.org/#/c/3435/1/openbsc/src/libmsc/db.c File openbsc/src/libmsc/db.c: Line 375: /* Just like v3, but there is a new message reference field for status reports, > "just like v5"? Right, just like v3 actually. -- To view, visit https://gerrit.osmocom.org/3435 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7276d356d805a83ebeec72b02c8563b7135ea0b6 Gerrit-PatchSet: 4 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: Yes
[MERGED] openbsc[master]: libmsc: update database to accomodate SMS status-report fields
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: libmsc: update database to accomodate SMS status-report fields
..
libmsc: update database to accomodate SMS status-report fields
SMPP DELIVER_SM messages with esm_class = Delivery Receipt need to send
this message reference (that the mobile phone allocates) to the ESME.
Thus, the ESME propagates it via SUBMIT_SM with esm_class = Delivery
Acknoledgment so that the SMSC sends the GSM 03.40 status-report to the
origin including this. Given this field is useful for status-reports, we
need to store it in the HLR database.
Moreover, we need a new field that specifies if the entry represents a
SMS status-report, to do the right handling from the gsm411_send_sms() -
such new handling comes in a follow up patch entitled "libmsc: handle
delivery ack via SMPP SUBMIT SM / send GSM 03.40 status report".
This patch includes the migration routines to the new database schema
revision 5, it's quite a bit of dbi boilerplate code - copied-pasted and
adapted.
Change-Id: I7276d356d805a83ebeec72b02c8563b7135ea0b6
---
M openbsc/src/libmsc/db.c
M openbsc/tests/db/db_test.err
2 files changed, 163 insertions(+), 6 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Pablo Neira Ayuso: Verified
diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c
index 5fe2a3c..9945dca 100644
--- a/openbsc/src/libmsc/db.c
+++ b/openbsc/src/libmsc/db.c
@@ -50,7 +50,7 @@
static char *db_dirname = NULL;
static dbi_conn conn;
-#define SCHEMA_REVISION "4"
+#define SCHEMA_REVISION "5"
enum {
SCHEMA_META,
@@ -124,6 +124,8 @@
"valid_until TIMESTAMP, "
"reply_path_req INTEGER NOT NULL, "
"status_rep_req INTEGER NOT NULL, "
+ "is_report INTEGER NOT NULL, "
+ "msg_ref INTEGER NOT NULL, "
"protocol_id INTEGER NOT NULL, "
"data_coding_scheme INTEGER NOT NULL, "
"ud_hdr_ind INTEGER NOT NULL, "
@@ -370,6 +372,152 @@
return -EINVAL;
}
+/* Just like v3, but there is a new message reference field for status reports,
+ * that is set to zero for existing entries since there is no way we can infer
+ * this.
+ */
+static struct gsm_sms *sms_from_result_v4(dbi_result result)
+{
+ struct gsm_sms *sms = sms_alloc();
+ const unsigned char *user_data;
+ const char *text, *addr;
+
+ if (!sms)
+ return NULL;
+
+ sms->id = dbi_result_get_ulonglong(result, "id");
+
+ sms->reply_path_req = dbi_result_get_ulonglong(result,
"reply_path_req");
+ sms->status_rep_req = dbi_result_get_ulonglong(result,
"status_rep_req");
+ sms->ud_hdr_ind = dbi_result_get_ulonglong(result, "ud_hdr_ind");
+ sms->protocol_id = dbi_result_get_ulonglong(result, "protocol_id");
+ sms->data_coding_scheme = dbi_result_get_ulonglong(result,
+ "data_coding_scheme");
+
+ addr = dbi_result_get_string(result, "src_addr");
+ osmo_strlcpy(sms->src.addr, addr, sizeof(sms->src.addr));
+ sms->src.ton = dbi_result_get_ulonglong(result, "src_ton");
+ sms->src.npi = dbi_result_get_ulonglong(result, "src_npi");
+
+ addr = dbi_result_get_string(result, "dest_addr");
+ osmo_strlcpy(sms->dst.addr, addr, sizeof(sms->dst.addr));
+ sms->dst.ton = dbi_result_get_ulonglong(result, "dest_ton");
+ sms->dst.npi = dbi_result_get_ulonglong(result, "dest_npi");
+
+ sms->user_data_len = dbi_result_get_field_length(result, "user_data");
+ user_data = dbi_result_get_binary(result, "user_data");
+ if (sms->user_data_len > sizeof(sms->user_data))
+ sms->user_data_len = (uint8_t) sizeof(sms->user_data);
+ memcpy(sms->user_data, user_data, sms->user_data_len);
+
+ text = dbi_result_get_string(result, "text");
+ if (text)
+ osmo_strlcpy(sms->text, text, sizeof(sms->text));
+ return sms;
+}
+
+static int update_db_revision_4(void)
+{
+ dbi_result result;
+ struct gsm_sms *sms;
+
+ LOGP(DDB, LOGL_NOTICE, "Going to migrate from revision 4\n");
+
+ result = dbi_conn_query(conn, "BEGIN EXCLUSIVE TRANSACTION");
+ if (!result) {
+ LOGP(DDB, LOGL_ERROR,
+ "Failed to begin transaction (upgrade from rev 4)\n");
+ return -EINVAL;
+ }
+ dbi_result_free(result);
+
+ /* Rename old SMS table to be able create a new one */
+ result =
[MERGED] openbsc[master]: utils: smpp_mirror: temporarily munch SMPP delivery receipts
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: utils: smpp_mirror: temporarily munch SMPP delivery receipts
..
utils: smpp_mirror: temporarily munch SMPP delivery receipts
Just munch and log SMPP delivery receipts by now, don't mirror this, it
is going to break things in openbsc.
Follow up patch removes this and mirrors this SMPP message as a
SUBMIT_SM with esm_class = Delivery Acknowledgement.
Change-Id: I78e93bc4034679e238c8642ccf6a0e844b1d6d8b
---
M openbsc/src/utils/smpp_mirror.c
1 file changed, 8 insertions(+), 0 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/openbsc/src/utils/smpp_mirror.c b/openbsc/src/utils/smpp_mirror.c
index ec28f0a..edb40b5 100644
--- a/openbsc/src/utils/smpp_mirror.c
+++ b/openbsc/src/utils/smpp_mirror.c
@@ -112,6 +112,14 @@
PACK_AND_SEND(esme, _r);
+ /* This is a delivery receipt, temporarily munch it until we teach
+* openbsc what to do with this.
+*/
+ if (deliver.esm_class == 0x04) {
+ LOGP(DSMPP, LOGL_NOTICE, "%s\n", deliver.short_message);
+ return 0;
+ }
+
memset(, 0, sizeof(submit));
submit.command_id = SUBMIT_SM;
submit.command_status = ESME_ROK;
--
To view, visit https://gerrit.osmocom.org/3432
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I78e93bc4034679e238c8642ccf6a0e844b1d6d8b
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Holger Freyther <hol...@freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
[MERGED] openbsc[master]: utils: smpp_mirror: set registered_delivery field in SMPP SU...
Pablo Neira Ayuso has submitted this change and it was merged. Change subject: utils: smpp_mirror: set registered_delivery field in SMPP SUBMIT_SM .. utils: smpp_mirror: set registered_delivery field in SMPP SUBMIT_SM To test delivery reports using this utility. Change-Id: I0e477407531fdd4d906e53c9b5a48a79a239966f --- M openbsc/src/utils/smpp_mirror.c 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: Harald Welte: Looks good to me, approved Jenkins Builder: Verified diff --git a/openbsc/src/utils/smpp_mirror.c b/openbsc/src/utils/smpp_mirror.c index 95df5f2..ec28f0a 100644 --- a/openbsc/src/utils/smpp_mirror.c +++ b/openbsc/src/utils/smpp_mirror.c @@ -130,6 +130,7 @@ sizeof(deliver.destination_addr))); submit.esm_class = deliver.esm_class; + submit.registered_delivery = deliver.registered_delivery; submit.protocol_id = deliver.protocol_id; submit.priority_flag = deliver.priority_flag; memcpy(submit.schedule_delivery_time, deliver.schedule_delivery_time, -- To view, visit https://gerrit.osmocom.org/3431 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I0e477407531fdd4d906e53c9b5a48a79a239966f Gerrit-PatchSet: 2 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
[MERGED] openbsc[master]: libmsc: add support for SMPP delivery receipts
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: libmsc: add support for SMPP delivery receipts
..
libmsc: add support for SMPP delivery receipts
If the mobile phone requests a status report via SMS, send a DELIVER_SM
with esm_class = Delivery Receipt to ESME to indicate that the SMS has
been already delivered to its destination.
MSGSM 03.40 SMSC SMPP 3.4 ESME
| ||
| SMS-DELIVER ||
|<||
| GSM 04.11 RP-ACK||
|>||
| | DELIVER-SM |
| | esm_class = Delivery Receipt |
| |--->|
| | DELIVER-SM-RESP|
| |<---|
| ||
This patch implements "Appendix B. Delivery Receipt Format" as specified
in the SMPP 3.4 specs. This string is conveyed in the SMS message as
data, and it is only meaningful to the ESME, for logging purposes. The
"submit date" and "done date" are not yet set, and other fields are just
sent with dummy values, so they are left to be finished as future work.
The new SMPP TLV tag TLVID_user_message_reference is added to the SMPP
messages inconditionally now since this information is required by
delivery-reports to associate the status-report with the original SMS.
Change-Id: Ic1a9023074bfa938099377980b6aff9b262fab2a
---
M openbsc/include/openbsc/gsm_data.h
M openbsc/src/libmsc/gsm_04_11.c
M openbsc/src/libmsc/smpp_openbsc.c
M openbsc/src/libmsc/smpp_smsc.h
4 files changed, 76 insertions(+), 1 deletion(-)
Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/openbsc/include/openbsc/gsm_data.h
b/openbsc/include/openbsc/gsm_data.h
index 37a341c..6829d22 100644
--- a/openbsc/include/openbsc/gsm_data.h
+++ b/openbsc/include/openbsc/gsm_data.h
@@ -456,6 +456,7 @@
} smpp;
unsigned long validity_minutes;
+ bool is_report;
uint8_t reply_path_req;
uint8_t status_rep_req;
uint8_t ud_hdr_ind;
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index 1aed60e..0edbe0b 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -593,6 +593,57 @@
rpud_len, rp_ud);
}
+static struct gsm_sms *sms_report_alloc(struct gsm_sms *sms)
+{
+ struct gsm_sms *sms_report;
+ int len;
+
+ sms_report = sms_alloc();
+ OSMO_ASSERT(sms_report);
+
+ sms_report->msg_ref = sms->msg_ref;
+ sms_report->protocol_id = sms->protocol_id;
+ sms_report->data_coding_scheme = GSM338_DCS__8BIT_DATA;
+
+ /* Invert address to send status report back to origin. */
+ sms_report->src = sms->dst;
+ sms_report->dst = sms->src;
+
+ /* As specified by Appendix B. Delivery Receipt Format.
+* TODO: Many fields in this string are just set with dummy values,
+* revisit this.
+*/
+ len = snprintf((char *)sms_report->user_data,
+ sizeof(sms_report->user_data),
+ "id:%.08llu sub:000 dlvrd:000 submit date:YYMMDDhhmm
done date:YYMMDDhhmm stat:DELIVRD err:000 text:%.20s",
+ sms->id, sms->user_data);
+ sms_report->user_data_len = len;
+ LOGP(DLSMS, LOGL_NOTICE, "%s\n", sms_report->user_data);
+
+ /* This represents a sms report. */
+ sms_report->is_report = true;
+
+ return sms_report;
+}
+
+static void sms_status_report(struct gsm_sms *gsms,
+ struct gsm_subscriber_connection *conn)
+{
+ struct gsm_sms *sms_report;
+ int rc;
+
+ sms_report = sms_report_alloc(gsms);
+
+ rc = sms_route_mt_sms(conn, sms_report);
+ if (rc < 0) {
+ LOGP(DLSMS, LOGL_ERROR,
+"Failed to send status report! err=%d\n", rc);
+ }
+ LOGP(DLSMS, LOGL_NOTICE, "Status report has been sent\n");
+
+ sms_free(sms_report);
+}
+
/* Receive a 04.11 RP-ACK message (response to RP-DATA from us) */
static int gsm411_rx_rp_ack(struct msgb *msg, struct gsm_trans *trans,
struct gsm411_rp_hdr *rph)
@@ -614,6 +665,9 @@
send_signal(S_SMS_DELIVERED, trans, sms, 0);
+ if (sms->status_rep_req)
+
[MERGED] openbsc[master]: libmsc: report status report request flag from SMPP SUBMIT_SM
Pablo Neira Ayuso has submitted this change and it was merged. Change subject: libmsc: report status report request flag from SMPP SUBMIT_SM .. libmsc: report status report request flag from SMPP SUBMIT_SM Restore the sms status report request flag from SUBMIT_SM. Change-Id: Iac05252253f8933a3875b4904599b7a225191a4b --- M openbsc/src/libmsc/smpp_openbsc.c 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: Harald Welte: Looks good to me, approved Jenkins Builder: Verified Pablo Neira Ayuso: Verified diff --git a/openbsc/src/libmsc/smpp_openbsc.c b/openbsc/src/libmsc/smpp_openbsc.c index cf78676..a803763 100644 --- a/openbsc/src/libmsc/smpp_openbsc.c +++ b/openbsc/src/libmsc/smpp_openbsc.c @@ -129,6 +129,7 @@ sms = sms_alloc(); sms->source = SMS_SOURCE_SMPP; sms->smpp.sequence_nr = submit->sequence_number; + sms->status_rep_req = submit->registered_delivery; /* fill in the destination address */ sms->receiver = dest; -- To view, visit https://gerrit.osmocom.org/3429 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iac05252253f8933a3875b4904599b7a225191a4b Gerrit-PatchSet: 3 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
[MERGED] openbsc[master]: libmsc: missing bit shift in status report flag when stored ...
Pablo Neira Ayuso has submitted this change and it was merged. Change subject: libmsc: missing bit shift in status report flag when stored in sms object .. libmsc: missing bit shift in status report flag when stored in sms object So we just store 0 or 1 depending on what the mobile phone requests. Change-Id: Idb7d5594219c0e458ccb561383a59604bc1a4201 --- M openbsc/src/libmsc/gsm_04_11.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Harald Welte: Looks good to me, approved Jenkins Builder: Verified diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c index 7294153..1aed60e 100644 --- a/openbsc/src/libmsc/gsm_04_11.c +++ b/openbsc/src/libmsc/gsm_04_11.c @@ -366,7 +366,7 @@ /* invert those fields where 0 means active/present */ sms_mti = *smsp & 0x03; sms_vpf = (*smsp & 0x18) >> 3; - gsms->status_rep_req = (*smsp & 0x20); + gsms->status_rep_req = (*smsp & 0x20) >> 5; gsms->ud_hdr_ind = (*smsp & 0x40); /* * Not evaluating MMS (More Messages to Send) because the -- To view, visit https://gerrit.osmocom.org/3430 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Idb7d5594219c0e458ccb561383a59604bc1a4201 Gerrit-PatchSet: 2 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
[MERGED] openbsc[master]: libmsc: set registered_delivery field in SMPP 3.4 DELIVER_SM...
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: libmsc: set registered_delivery field in SMPP 3.4 DELIVER_SM
messages
..
libmsc: set registered_delivery field in SMPP 3.4 DELIVER_SM messages
Propagate the status report request field to the SMPP message through
the registered_delivery field, so the ESME knows that the mobile phone
is asking for explicit delivery acknowledgment is required. See SMPP 3.4
specs section 5.2.17.
Change-Id: I59af60fa89cd10ae973c5e122789e3e03e3728ee
---
M openbsc/src/libmsc/smpp_openbsc.c
1 file changed, 8 insertions(+), 1 deletion(-)
Approvals:
Neels Hofmeyr: Looks good to me, but someone else must approve
Harald Welte: Looks good to me, approved; Verified
diff --git a/openbsc/src/libmsc/smpp_openbsc.c
b/openbsc/src/libmsc/smpp_openbsc.c
index 6b89df2..cf78676 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -602,6 +602,10 @@
return NULL;
}
+/* See SMPP 3.4, Section 5.2.17. registered_delivery (1 byte field). */
+#define SMPP34_NO_DELIVERY_RECEIPT 0x0
+#define SMPP34_DELIVERY_RECEIPT_REQ0x1
+
static int deliver_to_esme(struct osmo_esme *esme, struct gsm_sms *sms,
struct gsm_subscriber_connection *conn)
{
@@ -642,7 +646,10 @@
deliver.protocol_id = sms->protocol_id;
deliver.priority_flag = 0;
- deliver.registered_delivery = 0;
+ if (sms->status_rep_req)
+ deliver.registered_delivery = SMPP34_DELIVERY_RECEIPT_REQ;
+ else
+ deliver.registered_delivery = SMPP34_NO_DELIVERY_RECEIPT;
/* Figure out SMPP DCS from TP-DCS */
dcs = sms->data_coding_scheme;
--
To view, visit https://gerrit.osmocom.org/3428
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I59af60fa89cd10ae973c5e122789e3e03e3728ee
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Holger Freyther <hol...@freyther.de>
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de>
[PATCH] openbsc[master]: libmsc: update database to accomodate SMS status-report fields
sms->ud_hdr_ind, q_udata, q_text, q_daddr, sms->dst.ton, sms->dst.npi, q_saddr, sms->src.ton, sms->src.npi); @@ -1489,6 +1643,8 @@ /* FIXME: those should all be get_uchar, but sqlite3 is braindead */ sms->reply_path_req = dbi_result_get_ulonglong(result, "reply_path_req"); sms->status_rep_req = dbi_result_get_ulonglong(result, "status_rep_req"); + sms->is_report = dbi_result_get_ulonglong(result, "is_report"); + sms->msg_ref = dbi_result_get_ulonglong(result, "msg_ref"); sms->ud_hdr_ind = dbi_result_get_ulonglong(result, "ud_hdr_ind"); sms->protocol_id = dbi_result_get_ulonglong(result, "protocol_id"); sms->data_coding_scheme = dbi_result_get_ulonglong(result, diff --git a/openbsc/tests/db/db_test.err b/openbsc/tests/db/db_test.err index fa9a54c..27e5703 100644 --- a/openbsc/tests/db/db_test.err +++ b/openbsc/tests/db/db_test.err @@ -1,2 +1,3 @@ Going to migrate from revision 3 +[0;mGoing to migrate from revision 4 [0;m \ No newline at end of file -- To view, visit https://gerrit.osmocom.org/3435 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7276d356d805a83ebeec72b02c8563b7135ea0b6 Gerrit-PatchSet: 4 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
openbsc[master]: libmsc: report status report request flag from SMPP SUBMIT_SM
Patch Set 3: Verified+1 -- To view, visit https://gerrit.osmocom.org/3429 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iac05252253f8933a3875b4904599b7a225191a4b Gerrit-PatchSet: 3 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
[PATCH] openbsc[master]: libmsc: update database to accomodate SMS status-report fields
q_udata, q_text, q_daddr, sms->dst.ton, sms->dst.npi, q_saddr, sms->src.ton, sms->src.npi); @@ -1489,6 +1643,8 @@ /* FIXME: those should all be get_uchar, but sqlite3 is braindead */ sms->reply_path_req = dbi_result_get_ulonglong(result, "reply_path_req"); sms->status_rep_req = dbi_result_get_ulonglong(result, "status_rep_req"); + sms->is_report = dbi_result_get_ulonglong(result, "is_report"); + sms->msg_ref = dbi_result_get_ulonglong(result, "msg_ref"); sms->ud_hdr_ind = dbi_result_get_ulonglong(result, "ud_hdr_ind"); sms->protocol_id = dbi_result_get_ulonglong(result, "protocol_id"); sms->data_coding_scheme = dbi_result_get_ulonglong(result, -- To view, visit https://gerrit.osmocom.org/3435 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7276d356d805a83ebeec72b02c8563b7135ea0b6 Gerrit-PatchSet: 3 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
[PATCH] openbsc[master]: utils: smpp_mirror: bounce Delivery Receipts as Delivery Ack...
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/3436
to look at the new patch set (#3).
utils: smpp_mirror: bounce Delivery Receipts as Delivery Acknowledgments
Simple patch to test the new status-report support code, remove previous
code before Delivery Acknowledgement support was in place. Use
LOGL_DEBUG for logging messages here as suggested by Neels and Harald.
Change-Id: I877e228d8e174430f700631edbf9955972da7892
---
M openbsc/src/utils/smpp_mirror.c
1 file changed, 8 insertions(+), 9 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/36/3436/3
diff --git a/openbsc/src/utils/smpp_mirror.c b/openbsc/src/utils/smpp_mirror.c
index 88545de..c570505 100644
--- a/openbsc/src/utils/smpp_mirror.c
+++ b/openbsc/src/utils/smpp_mirror.c
@@ -123,14 +123,6 @@
PACK_AND_SEND(esme, _r);
- /* This is a delivery receipt, temporarily munch it until we teach
-* openbsc what to do with this.
-*/
- if (deliver.esm_class == 0x04) {
- LOGP(DSMPP, LOGL_NOTICE, "%s\n", deliver.short_message);
- return 0;
- }
-
memset(, 0, sizeof(submit));
submit.command_id = SUBMIT_SM;
submit.command_status = ESME_ROK;
@@ -148,7 +140,14 @@
OSMO_MIN(sizeof(submit.source_addr),
sizeof(deliver.destination_addr)));
- submit.esm_class = deliver.esm_class;
+ /* Mirror delivery receipts as a delivery acknowledgements. */
+ if (deliver.esm_class == 0x04) {
+ LOGP(DSMPP, LOGL_DEBUG, "%s\n", deliver.short_message);
+ submit.esm_class = 0x08;
+ } else {
+ submit.esm_class = deliver.esm_class;
+ }
+
submit.registered_delivery = deliver.registered_delivery;
submit.protocol_id = deliver.protocol_id;
submit.priority_flag = deliver.priority_flag;
--
To view, visit https://gerrit.osmocom.org/3436
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I877e228d8e174430f700631edbf9955972da7892
Gerrit-PatchSet: 3
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Holger Freyther <hol...@freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
[PATCH] openbsc[master]: libmsc: support GSM 03.40 status report for nitb
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/3438 to look at the new patch set (#3). libmsc: support GSM 03.40 status report for nitb This patch adds support for GSM 03.40 in nitb mode. MS GSM 03.40 SMSC || | SMS-SUBMIT | |--->| | GSM 04.11 RP-ACK | |<---| | SMS-DELIVER| |<---| | GSM 04.11 RP-ACK | |--->| | SMS-STATUS-REPORT | |<---| | GSM 04.11 RP-ACK | |--->| || Change-Id: I5cc7bb4ebadde0940f44d10c3df34707b0615160 --- M openbsc/src/libmsc/gsm_04_11.c 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/38/3438/3 diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c index 80c3d77..73e0f55 100644 --- a/openbsc/src/libmsc/gsm_04_11.c +++ b/openbsc/src/libmsc/gsm_04_11.c @@ -682,6 +682,11 @@ LOGP(DLSMS, LOGL_ERROR, "Failed to send status report! err=%d\n", rc); } + + /* No route via SMPP, send the GSM 03.40 status-report now. */ + if (gsms->receiver) + gsm340_rx_sms_submit(sms_report); + LOGP(DLSMS, LOGL_NOTICE, "Status report has been sent\n"); sms_free(sms_report); -- To view, visit https://gerrit.osmocom.org/3438 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5cc7bb4ebadde0940f44d10c3df34707b0615160 Gerrit-PatchSet: 3 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
[PATCH] openbsc[master]: libmsc: add support for SMPP delivery receipts
ee(sms);
trans->sms.sms = NULL;
diff --git a/openbsc/src/libmsc/smpp_openbsc.c
b/openbsc/src/libmsc/smpp_openbsc.c
index a803763..c0aa89b 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -200,6 +200,10 @@
sms->user_data_len = sms_msg_len;
}
+ t = find_tlv(submit->tlv, TLVID_user_message_reference);
+ if (t)
+ sms->msg_ref = ntohs(t->value.val16);
+
*psms = sms;
return ESME_ROK;
}
@@ -514,6 +518,9 @@
struct gsm_subscriber_connection *conn;
struct gsm_trans *trans;
+ if (cmd->is_report)
+ goto out;
+
conn = connection_for_subscr(cmd->subscr);
if (!conn) {
LOGP(DSMPP, LOGL_ERROR, "No connection to subscriber
anymore\n");
@@ -537,6 +544,9 @@
struct gsm_subscriber_connection *conn;
struct gsm_trans *trans;
int gsm411_cause;
+
+ if (cmd->is_report)
+ goto out;
conn = connection_for_subscr(cmd->subscr);
if (!conn) {
@@ -575,6 +585,7 @@
return -1;
cmd->sequence_nr= sequence_number;
+ cmd->is_report = sms->is_report;
cmd->gsm411_msg_ref = sms->gsm411.msg_ref;
cmd->gsm411_trans_id= sms->gsm411.transaction_id;
cmd->subscr = subscr_get(subscr);
@@ -639,7 +650,12 @@
memcpy(deliver.destination_addr, sms->dst.addr,
sizeof(deliver.destination_addr));
- deliver.esm_class = 1;/* datagram mode */
+ /* Short message contains a delivery receipt? Sect. 5.2.12. */
+ if (sms->is_report)
+ deliver.esm_class = 0x04;
+ else
+ deliver.esm_class = 1; /* datagram mode */
+
if (sms->ud_hdr_ind)
deliver.esm_class |= 0x40;
if (sms->reply_path_req)
@@ -685,6 +701,9 @@
if (esme->acl && esme->acl->osmocom_ext && conn->lchan)
append_osmo_tlvs(, conn->lchan);
+ append_tlv_u16(, TLVID_user_message_reference,
+ sms->msg_ref);
+
ret = smpp_tx_deliver(esme, );
if (ret < 0)
return ret;
diff --git a/openbsc/src/libmsc/smpp_smsc.h b/openbsc/src/libmsc/smpp_smsc.h
index 468d3b4..257383f 100644
--- a/openbsc/src/libmsc/smpp_smsc.h
+++ b/openbsc/src/libmsc/smpp_smsc.h
@@ -92,6 +92,7 @@
uint32_tsequence_nr;
uint32_tgsm411_msg_ref;
uint8_t gsm411_trans_id;
+ boolis_report;
struct osmo_timer_list response_timer;
};
--
To view, visit https://gerrit.osmocom.org/3434
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic1a9023074bfa938099377980b6aff9b262fab2a
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Holger Freyther <hol...@freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
[PATCH] openbsc[master]: libmsc: handle delivery ack via SMPP SUBMIT SM / send GSM 03...
rit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib70e534840308ed315f7add440351e649de3f907 Gerrit-PatchSet: 3 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
openbsc[master]: libmsc: set registered_delivery field in SMPP 3.4 DELIVER_SM...
Patch Set 1: If I push the "submit" button here, gerrit (on a black screen) fails with: Code Review - Error Cannot merge 6f615241665a8c47500855e2d608cab4467e6443 Missing dependency -- To view, visit https://gerrit.osmocom.org/3428 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59af60fa89cd10ae973c5e122789e3e03e3728ee Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
[MERGED] openbsc[master]: libmsc: move gsm340_rx_sms_submit() to sms_route_mt_sms()
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: libmsc: move gsm340_rx_sms_submit() to sms_route_mt_sms()
..
libmsc: move gsm340_rx_sms_submit() to sms_route_mt_sms()
Move the sms message-type-identifier (mti) handling away from the
routing logic. This patch allows us to reuse the sms_route_mt_sms()
function in a follow up patch for sms reports send through SMPP
DELIVER_SM with esm_class = Delivery Receipt whose Change-Id is
Ic1a9023074bfa938099377980b6aff9b262fab2a.
Change-Id: I3f3d30e0762b91e2099243b0be1a4b67cbb5e9c0
---
M openbsc/src/libmsc/gsm_04_11.c
1 file changed, 22 insertions(+), 19 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index d60de08..7294153 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -278,7 +278,7 @@
}
static int sms_route_mt_sms(struct gsm_subscriber_connection *conn,
- struct gsm_sms *gsms, uint8_t sms_mti)
+ struct gsm_sms *gsms)
{
int rc;
@@ -336,23 +336,6 @@
rc = GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
#endif
- return rc;
- }
-
- switch (sms_mti) {
- case GSM340_SMS_SUBMIT_MS2SC:
- /* MS is submitting a SMS */
- rc = gsm340_rx_sms_submit(gsms);
- break;
- case GSM340_SMS_COMMAND_MS2SC:
- case GSM340_SMS_DELIVER_REP_MS2SC:
- LOGP(DLSMS, LOGL_NOTICE, "Unimplemented MTI 0x%02x\n", sms_mti);
- rc = GSM411_RP_CAUSE_IE_NOTEXIST;
- break;
- default:
- LOGP(DLSMS, LOGL_NOTICE, "Undefined MTI 0x%02x\n", sms_mti);
- rc = GSM411_RP_CAUSE_IE_NOTEXIST;
- break;
}
return rc;
@@ -484,7 +467,27 @@
/* FIXME: This looks very wrong */
send_signal(0, NULL, gsms, 0);
- rc = sms_route_mt_sms(conn, gsms, sms_mti);
+ rc = sms_route_mt_sms(conn, gsms);
+
+ /* This SMS got routed through SMPP or no receiver exists. */
+ if (!gsms->receiver)
+ return rc;
+
+ switch (sms_mti) {
+ case GSM340_SMS_SUBMIT_MS2SC:
+ /* MS is submitting a SMS */
+ rc = gsm340_rx_sms_submit(gsms);
+ break;
+ case GSM340_SMS_COMMAND_MS2SC:
+ case GSM340_SMS_DELIVER_REP_MS2SC:
+ LOGP(DLSMS, LOGL_NOTICE, "Unimplemented MTI 0x%02x\n", sms_mti);
+ rc = GSM411_RP_CAUSE_IE_NOTEXIST;
+ break;
+ default:
+ LOGP(DLSMS, LOGL_NOTICE, "Undefined MTI 0x%02x\n", sms_mti);
+ rc = GSM411_RP_CAUSE_IE_NOTEXIST;
+ break;
+ }
out:
sms_free(gsms);
--
To view, visit https://gerrit.osmocom.org/3427
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3f3d30e0762b91e2099243b0be1a4b67cbb5e9c0
Gerrit-PatchSet: 2
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Holger Freyther <hol...@freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
[MERGED] openbsc[master]: libmsc: remove 'deferred' parameter in sms_route_mt_sms()
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: libmsc: remove 'deferred' parameter in sms_route_mt_sms()
..
libmsc: remove 'deferred' parameter in sms_route_mt_sms()
No need to cache the sms object, just cache what we need into the
smpp_cmd structure. This simplifies what that I introduced in
93ffbd0029d1 ("libmsc: send RP-ACK to MS after ESME sends SMPP
DELIVER-SM-RESP").
Change-Id: Iba5f864f9bb963baff95969e306b1b7cff00c1e3
---
M openbsc/src/libmsc/gsm_04_11.c
M openbsc/src/libmsc/smpp_openbsc.c
M openbsc/src/libmsc/smpp_smsc.h
3 files changed, 24 insertions(+), 30 deletions(-)
Approvals:
Neels Hofmeyr: Looks good to me, but someone else must approve
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index 7df5e64..d60de08 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -278,8 +278,7 @@
}
static int sms_route_mt_sms(struct gsm_subscriber_connection *conn,
- struct gsm_sms *gsms, uint8_t sms_mti,
- bool *deferred)
+ struct gsm_sms *gsms, uint8_t sms_mti)
{
int rc;
@@ -293,7 +292,7 @@
* delivery of the SMS.
*/
if (smpp_first) {
- rc = smpp_try_deliver(gsms, conn, deferred);
+ rc = smpp_try_deliver(gsms, conn);
if (rc == GSM411_RP_CAUSE_MO_NUM_UNASSIGNED)
/* unknown subscriber, try local */
goto try_local;
@@ -322,7 +321,7 @@
return GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
}
- rc = smpp_try_deliver(gsms, conn, deferred);
+ rc = smpp_try_deliver(gsms, conn);
if (rc == GSM411_RP_CAUSE_MO_NUM_UNASSIGNED) {
rate_ctr_inc(>network->msc_ctrs->ctr[MSC_CTR_SMS_NO_RECEIVER]);
} else if (rc < 0) {
@@ -363,7 +362,7 @@
/* process an incoming TPDU (called from RP-DATA)
* return value > 0: RP CAUSE for ERROR; < 0: silent error; 0 = success */
static int gsm340_rx_tpdu(struct gsm_trans *trans, struct msgb *msg,
- uint32_t gsm411_msg_ref, bool *deferred)
+ uint32_t gsm411_msg_ref)
{
struct gsm_subscriber_connection *conn = trans->conn;
uint8_t *smsp = msgb_sms(msg);
@@ -485,10 +484,9 @@
/* FIXME: This looks very wrong */
send_signal(0, NULL, gsms, 0);
- rc = sms_route_mt_sms(conn, gsms, sms_mti, deferred);
+ rc = sms_route_mt_sms(conn, gsms, sms_mti);
out:
- if (!deferred)
- sms_free(gsms);
+ sms_free(gsms);
return rc;
}
@@ -541,7 +539,6 @@
uint8_t dst_len, uint8_t *dst,
uint8_t tpdu_len, uint8_t *tpdu)
{
- bool deferred = false;
int rc = 0;
if (src_len && src)
@@ -558,8 +555,8 @@
DEBUGP(DLSMS, "DST(%u,%s)\n", dst_len, osmo_hexdump(dst, dst_len));
- rc = gsm340_rx_tpdu(trans, msg, rph->msg_ref, );
- if (rc == 0 && !deferred)
+ rc = gsm340_rx_tpdu(trans, msg, rph->msg_ref);
+ if (rc == 0)
return gsm411_send_rp_ack(trans, rph->msg_ref);
else if (rc > 0)
return gsm411_send_rp_error(trans, rph->msg_ref, rc);
diff --git a/openbsc/src/libmsc/smpp_openbsc.c
b/openbsc/src/libmsc/smpp_openbsc.c
index c9379ec..6b89df2 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -497,7 +497,6 @@
osmo_timer_del(>response_timer);
llist_del(>list);
subscr_put(cmd->subscr);
- sms_free(cmd->sms);
talloc_free(cmd);
}
@@ -520,15 +519,14 @@
goto out;
}
- trans = trans_find_by_id(conn, GSM48_PDISC_SMS,
-cmd->sms->gsm411.transaction_id);
+ trans = trans_find_by_id(conn, GSM48_PDISC_SMS, cmd->gsm411_trans_id);
if (!trans) {
LOGP(DSMPP, LOGL_ERROR, "GSM transaction %u is gone\n",
-cmd->sms->gsm411.transaction_id);
+cmd->gsm411_trans_id);
goto out;
}
- gsm411_send_rp_ack(trans, cmd->sms->gsm411.msg_ref);
+ gsm411_send_rp_ack(trans, cmd->gsm411_msg_ref);
out:
smpp_cmd_free(cmd);
}
@@ -545,18 +543,17 @@
goto out;
}
- trans = trans_find_by_id(conn, GSM48_PDISC_SMS,
-cmd->sms->gsm411.transaction_id);
+ trans = trans_find_by_id(conn, GSM48_PDISC_SMS, cmd->gsm411_trans_id);
if (!trans) {
LOGP(DSMPP, LOGL_ERROR, "GSM transaction %u
[MERGED] openbsc[master]: libmsc: remove duplicate lines in deliver_to_esme()
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: libmsc: remove duplicate lines in deliver_to_esme()
..
libmsc: remove duplicate lines in deliver_to_esme()
This code is accidentally doing the same thing twice, remove it.
Change-Id: I68087a850399e22951d2407e4d8a09c671a775c9
---
M openbsc/src/libmsc/smpp_openbsc.c
1 file changed, 0 insertions(+), 2 deletions(-)
Approvals:
Neels Hofmeyr: Looks good to me, approved
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 36ad0a2..c9379ec 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -676,8 +676,6 @@
} else {
deliver.sm_length = sms->user_data_len;
memcpy(deliver.short_message, sms->user_data,
deliver.sm_length);
- deliver.sm_length = sms->user_data_len;
- memcpy(deliver.short_message, sms->user_data,
deliver.sm_length);
}
if (esme->acl && esme->acl->osmocom_ext && conn->lchan)
--
To view, visit https://gerrit.osmocom.org/3425
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I68087a850399e22951d2407e4d8a09c671a775c9
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Holger Freyther <hol...@freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
[MERGED] openbsc[master]: gsm_04_11: get rid of unused parameter in sms_route_mt_sms()
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: gsm_04_11: get rid of unused parameter in sms_route_mt_sms()
..
gsm_04_11: get rid of unused parameter in sms_route_mt_sms()
This parameter is unused, remove it.
Change-Id: I797abce3f91447e8f397c7cf726db7425479fe0e
---
M openbsc/src/libmsc/gsm_04_11.c
1 file changed, 6 insertions(+), 5 deletions(-)
Approvals:
Neels Hofmeyr: Looks good to me, but someone else must approve
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index acf425a..da4460c 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -187,7 +187,7 @@
return gsm411_smc_send(>sms.smc_inst, msg_type, msg);
}
-static int gsm340_rx_sms_submit(struct msgb *msg, struct gsm_sms *gsms)
+static int gsm340_rx_sms_submit(struct gsm_sms *gsms)
{
if (db_sms_store(gsms) != 0) {
LOGP(DLSMS, LOGL_ERROR, "Failed to store SMS in Database\n");
@@ -277,8 +277,9 @@
return msg->len - old_msg_len;
}
-int sms_route_mt_sms(struct gsm_subscriber_connection *conn, struct msgb *msg,
-struct gsm_sms *gsms, uint8_t sms_mti, bool *deferred)
+static int sms_route_mt_sms(struct gsm_subscriber_connection *conn,
+ struct gsm_sms *gsms, uint8_t sms_mti,
+ bool *deferred)
{
int rc;
@@ -342,7 +343,7 @@
switch (sms_mti) {
case GSM340_SMS_SUBMIT_MS2SC:
/* MS is submitting a SMS */
- rc = gsm340_rx_sms_submit(msg, gsms);
+ rc = gsm340_rx_sms_submit(gsms);
break;
case GSM340_SMS_COMMAND_MS2SC:
case GSM340_SMS_DELIVER_REP_MS2SC:
@@ -487,7 +488,7 @@
/* FIXME: This looks very wrong */
send_signal(0, NULL, gsms, 0);
- rc = sms_route_mt_sms(conn, msg, gsms, sms_mti, deferred);
+ rc = sms_route_mt_sms(conn, gsms, sms_mti, deferred);
out:
if (!deferred)
sms_free(gsms);
--
To view, visit https://gerrit.osmocom.org/3422
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I797abce3f91447e8f397c7cf726db7425479fe0e
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Holger Freyther <hol...@freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
[MERGED] openbsc[master]: libmsc: remove dead code in sms_route_mt_sms()
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: libmsc: remove dead code in sms_route_mt_sms()
..
libmsc: remove dead code in sms_route_mt_sms()
The following branch:
if (!rc && !gsms->receiver)
rc = GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
at the end of sms_route_mt_sms() always evaluates false.
Just a bit before, in such function, we have this:
if (!gsms->receiver) {
...
#ifdef BUILD_SMPP
...
#else
...
#endif
return rc;
}
So, if there is no receiver, we just stop running code and return the RP
cause via the rc variable. Same applies to the smpp_first check under
the BUILD_SMPP ifdef (that I have removed in this snippet to keep this
commit message small).
Change-Id: Ic3502b5b169bc7a73a67fd6ff53d8b6c0dc045c8
---
M openbsc/src/libmsc/gsm_04_11.c
1 file changed, 0 insertions(+), 3 deletions(-)
Approvals:
Neels Hofmeyr: Looks good to me, approved
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index da4460c..7df5e64 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -356,9 +356,6 @@
break;
}
- if (!rc && !gsms->receiver)
- rc = GSM411_RP_CAUSE_MO_NUM_UNASSIGNED;
-
return rc;
}
--
To view, visit https://gerrit.osmocom.org/3424
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic3502b5b169bc7a73a67fd6ff53d8b6c0dc045c8
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Holger Freyther <hol...@freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
[MERGED] openbsc[master]: libmsc: do not leak pending SMPP command object on error path
Pablo Neira Ayuso has submitted this change and it was merged.
Change subject: libmsc: do not leak pending SMPP command object on error path
..
libmsc: do not leak pending SMPP command object on error path
Make sure the SMPP command object is released on errors.
Change-Id: I474584425d23fb379a9d71b33e29ac0e24f01e61
---
M openbsc/src/libmsc/smpp_openbsc.c
1 file changed, 6 insertions(+), 5 deletions(-)
Approvals:
Neels Hofmeyr: Looks good to me, approved
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 f7d1441..36ad0a2 100644
--- a/openbsc/src/libmsc/smpp_openbsc.c
+++ b/openbsc/src/libmsc/smpp_openbsc.c
@@ -517,7 +517,7 @@
conn = connection_for_subscr(cmd->subscr);
if (!conn) {
LOGP(DSMPP, LOGL_ERROR, "No connection to subscriber
anymore\n");
- return;
+ goto out;
}
trans = trans_find_by_id(conn, GSM48_PDISC_SMS,
@@ -525,10 +525,11 @@
if (!trans) {
LOGP(DSMPP, LOGL_ERROR, "GSM transaction %u is gone\n",
cmd->sms->gsm411.transaction_id);
- return;
+ goto out;
}
gsm411_send_rp_ack(trans, cmd->sms->gsm411.msg_ref);
+out:
smpp_cmd_free(cmd);
}
@@ -541,7 +542,7 @@
conn = connection_for_subscr(cmd->subscr);
if (!conn) {
LOGP(DSMPP, LOGL_ERROR, "No connection to subscriber
anymore\n");
- return;
+ goto out;
}
trans = trans_find_by_id(conn, GSM48_PDISC_SMS,
@@ -549,14 +550,14 @@
if (!trans) {
LOGP(DSMPP, LOGL_ERROR, "GSM transaction %u is gone\n",
cmd->sms->gsm411.transaction_id);
- return;
+ goto out;
}
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);
-
+out:
smpp_cmd_free(cmd);
}
--
To view, visit https://gerrit.osmocom.org/3423
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I474584425d23fb379a9d71b33e29ac0e24f01e61
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Holger Freyther <hol...@freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
openbsc[master]: gsm_04_11: get rid of unused parameter in sms_route_mt_sms()
Patch Set 1: I need to call gsm340_rx_sms_submit() in the last patch of this series, to add support for status-report in nitb mode. In such scenario, I have no real msgb object, since the struct gsm_sms object is allocated from the SMPP SUBMIT_SM esm_class = Delivery Acknoledgement. If this patch is kept back, I will have to pass a NULL pointer as parameter here when calling gsm340_rx_sms_submit() from the new sms_status_report() path. -- To view, visit https://gerrit.osmocom.org/3422 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I797abce3f91447e8f397c7cf726db7425479fe0e Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
[PATCH] openbsc[master]: libmsc: support GSM 03.40 status report for nitb
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/3438 to look at the new patch set (#2). libmsc: support GSM 03.40 status report for nitb This patch adds support for GSM 03.40 in nitb mode. MS GSM 03.40 SMSC || | SMS-SUBMIT | |--->| | GSM 04.11 RP-ACK | |<---| | SMS-DELIVER| |<---| | GSM 04.11 RP-ACK | |--->| | SMS-STATUS-REPORT | |<---| | GSM 04.11 RP-ACK | |--->| || Change-Id: I5cc7bb4ebadde0940f44d10c3df34707b0615160 --- M openbsc/src/libmsc/gsm_04_11.c 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/38/3438/2 diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c index a2e002e..fa00907 100644 --- a/openbsc/src/libmsc/gsm_04_11.c +++ b/openbsc/src/libmsc/gsm_04_11.c @@ -685,6 +685,11 @@ LOGP(DLSMS, LOGL_ERROR, "Failed to send status report! err=%d\n", rc); } + + /* No route via SMPP, send the GSM 03.40 status-report now. */ + if (gsms->receiver) + gsm340_rx_sms_submit(sms_report); + LOGP(DLSMS, LOGL_NOTICE, "Status report has been sent\n"); sms_free(sms_report); -- To view, visit https://gerrit.osmocom.org/3438 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5cc7bb4ebadde0940f44d10c3df34707b0615160 Gerrit-PatchSet: 2 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
[PATCH] openbsc[master]: libmsc: update database to accomodate SMS status-report fields
q_daddr, sms->dst.ton, sms->dst.npi, q_saddr, sms->src.ton, sms->src.npi); @@ -1489,6 +1643,8 @@ /* FIXME: those should all be get_uchar, but sqlite3 is braindead */ sms->reply_path_req = dbi_result_get_ulonglong(result, "reply_path_req"); sms->status_rep_req = dbi_result_get_ulonglong(result, "status_rep_req"); + sms->report = dbi_result_get_ulonglong(result, "report"); + sms->msg_ref = dbi_result_get_ulonglong(result, "msg_ref"); sms->ud_hdr_ind = dbi_result_get_ulonglong(result, "ud_hdr_ind"); sms->protocol_id = dbi_result_get_ulonglong(result, "protocol_id"); sms->data_coding_scheme = dbi_result_get_ulonglong(result, -- To view, visit https://gerrit.osmocom.org/3435 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7276d356d805a83ebeec72b02c8563b7135ea0b6 Gerrit-PatchSet: 2 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
openbsc[master]: utils: smpp_mirror: reflect message reference TLV
Patch Set 1: (1 comment) https://gerrit.osmocom.org/#/c/3433/1/openbsc/src/utils/smpp_mirror.c File openbsc/src/utils/smpp_mirror.c: Line 98: static struct tlv_t *find_tlv(struct tlv_t *head, uint16_t tag) > this is kind of on its own... could it use libosmocore gsm/tlv.h instead? B I think this belongs to the libsmpp library, since these are SMPP TLVs. There is more code in the openBSC tree that is related to SMPP that could be placed there too. -- To view, visit https://gerrit.osmocom.org/3433 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1b0abaa7e06ffe1bd2242c70813d8b70e9fa954f Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: Yes
openbsc[master]: libmsc: add support for SMPP delivery receipts
Patch Set 1: (5 comments) https://gerrit.osmocom.org/#/c/3434/1/openbsc/src/libmsc/gsm_04_11.c File openbsc/src/libmsc/gsm_04_11.c: Line 637: if (!sms_report) > log error, or maybe OSMO_ASSERT(sms_report)? Is there anything the user can do after this error? Why bother? I mean, you only hit this is there is not memory left... So likely there is a leak. OSMO_ASSERT() would stop openBSC from running, is that getting this any better? This sounds like calling BUG() in the kernel. So what do we get from spamming the user with an error that *only* happens if we have no memory left? https://gerrit.osmocom.org/#/c/3434/1/openbsc/src/libmsc/smpp_openbsc.c File openbsc/src/libmsc/smpp_openbsc.c: Line 521: /* We got a DELIVER_SM response for this is status report, this was > (hard to understand, fix typo/punctuation?) This is a reply to a DELIVER_SM with esm_class = Delivery Receipt, we're just having a conversation that was initiated by the SMSC. So we don't need to send any RP-ACK to the mobile, as we do with DELIVER_SM for a plan SMS. Line 659: deliver.esm_class = 0x04; > (would be nice to have constants instead of magic numbers ... but not relat I agree, this applies all over the place, but that would require a bit of work. Line 709: sms->msg_ref); > user message reference was always missing? maybe the fact that it is added Yes, message reference was always missing because there was no status-reports ;-). We can just add a branch here to skip this if saving bytes on the network are really a concern at the cost of slowing down the CPU path. https://gerrit.osmocom.org/#/c/3434/1/openbsc/src/libmsc/smpp_smsc.h File openbsc/src/libmsc/smpp_smsc.h: Line 95:boolreport; > (can be understood as "you should report" or "this is a report" ... would b This just annotates that the SMPP command that we send represents a deliver-report. -- To view, visit https://gerrit.osmocom.org/3434 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic1a9023074bfa938099377980b6aff9b262fab2a Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: Yes
openbsc[master]: gsm_04_11: get rid of unused parameter in sms_route_mt_sms()
Patch Set 1: This is DEAD code, just consuming more CPU cycles on something we don't need. We can restore this once you need it ;-) Thanks! -- To view, visit https://gerrit.osmocom.org/3422 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I797abce3f91447e8f397c7cf726db7425479fe0e Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Holger Freyther <hol...@freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org> Gerrit-HasComments: No
