[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 Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Holger Freyther Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Pablo Neira Ayuso
openbsc[master]: libmsc: support GSM 03.40 status report for nitb
Patch Set 4: > Patch Set 4: Verified+1 Not sure why I am permitted to Verify this... :-/ -- To view, visit https://gerrit.osmocom.org/3438 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5cc7bb4ebadde0940f44d10c3df34707b0615160 Gerrit-PatchSet: 4 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Holger Freyther Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Pablo Neira Ayuso Gerrit-HasComments: No
openbsc[master]: libmsc: support GSM 03.40 status report for nitb
Patch Set 4: Verified+1 -- To view, visit https://gerrit.osmocom.org/3438 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5cc7bb4ebadde0940f44d10c3df34707b0615160 Gerrit-PatchSet: 4 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Holger Freyther Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Pablo Neira Ayuso Gerrit-HasComments: No
openbsc[master]: libmsc: support GSM 03.40 status report for nitb
Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/3438 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5cc7bb4ebadde0940f44d10c3df34707b0615160 Gerrit-PatchSet: 4 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Holger Freyther Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Pablo Neira Ayuso 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 (#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 Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Holger Freyther Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Pablo Neira Ayuso
[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 Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Holger Freyther Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Keith Whyte Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Pablo Neira Ayuso
openbsc[master]: libmsc: support GSM 03.40 status report for nitb
Patch Set 1:
(1 comment)
https://gerrit.osmocom.org/#/c/3438/1/openbsc/src/libmsc/gsm_04_11.c
File openbsc/src/libmsc/gsm_04_11.c:
Line 692: gsm340_rx_sms_submit(sms_report);
> code cosmetics: I see from the old sms_route_mt_sms() that it looks correct
I'm going to try to address all your concerns in one go.
With SMPP support, we're getting this from the SMPP SUBMIT_SM, so there is no
mti here. In nitb mode, the sms_report object is created from the original sms
that we get from the mobile station. I don't think the old switch(mti) belong
sms_route_mt_sms().
I don't see where the name "sms_route_mt_sms()" suggests it has been already
sent :-), we can probably rename this to simply sms_route() given that no mti
handling happens anymore.
I agree the log could be moved after the gsms->receiver branch, it would be
more intuitive, yes. Anyway, this code runs inconditionally, so from the
logging perspective, this would be printed in the right.
Regarding the gsm->receiver indicator, the SMSC handles both SMPP and nitb
modes. With SMPP there is no receiver, we really don't know since it's the ESME
the one routing all SMS. We can probably wrap this in some simple:
static inline bool sms_has_route(struct gsm_sms *sms)
{
return gsm->receiver;
}
Thanks for your fast review!
--
To view, visit https://gerrit.osmocom.org/3438
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cc7bb4ebadde0940f44d10c3df34707b0615160
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Holger Freyther
Gerrit-Reviewer: Keith Whyte
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Reviewer: Pablo Neira Ayuso
Gerrit-HasComments: Yes
openbsc[master]: libmsc: support GSM 03.40 status report for nitb
Patch Set 1: (1 comment) https://gerrit.osmocom.org/#/c/3438/1/openbsc/src/libmsc/gsm_04_11.c File openbsc/src/libmsc/gsm_04_11.c: Line 692: gsm340_rx_sms_submit(sms_report); code cosmetics: I see from the old sms_route_mt_sms() that it looks correct (besides no switch(sms_mti) here which I hope you're sure is correct), but the code flow seems a bit weird now. The name "sms_route_mt_sms()" suggests that it has already been sent, which above log message also states. Yet we send it only later. I'm thinking the log should be moved below after sending concluded. Also the indicator whether it was routed or still needs to be sent seems a bit odd, could be made more obvious and/or clearly documented at sms_route_mt_sms() API doc? -- To view, visit https://gerrit.osmocom.org/3438 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5cc7bb4ebadde0940f44d10c3df34707b0615160 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Holger Freyther Gerrit-Reviewer: Keith Whyte Gerrit-Reviewer: Neels Hofmeyr Gerrit-HasComments: Yes
[PATCH] openbsc[master]: libmsc: support GSM 03.40 status report for nitb
Review at https://gerrit.osmocom.org/3438 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, 4 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/38/3438/1 diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c index a2e002e..13d7d1c 100644 --- a/openbsc/src/libmsc/gsm_04_11.c +++ b/openbsc/src/libmsc/gsm_04_11.c @@ -687,6 +687,10 @@ } LOGP(DLSMS, LOGL_NOTICE, "Status report has been sent\n"); + /* No route via SMPP, send the GSM 03.40 status-report now. */ + if (gsms->receiver) + gsm340_rx_sms_submit(sms_report); + sms_free(sms_report); } -- To view, visit https://gerrit.osmocom.org/3438 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5cc7bb4ebadde0940f44d10c3df34707b0615160 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Pablo Neira Ayuso
