18 Dec
2009
18 Dec
'09
6:28 p.m.
Here's a small series with some minor fixes I found when looking at multiple
SMS in a single RR connection.
For the MO case, it actually now works as described in the spec in the
no-error case. But what can happen is that after the second
CM SERVICE REQUEST, we may never receive the last CP-ACK of the previous
transaction (and thus leak a transaction). The spec says than when receiving
a CP-DATA after a CM SERVICE REQUEST we should consider we also implicitely
received the last CP-ACK of the previous transaction ... that's not
implemented yet.
For the MT case, it currently makes several RR connection which is quite
bad ... to be fixed.
Sylvain
18 Dec
18 Dec
6:28 p.m.
New subject: [PATCH 1/7] [bsc_init] Send OPSTART if SITE_MANAGER reports off-line
From: Sylvain Munaut <tnt(a)246tNt.com>
The exact sequence the states the BTS goes through is slightly
different for one of the nanoBTS 139 I have and it needs this
to start.
Signed-off-by: Sylvain Munaut <tnt(a)246tNt.com>
---
openbsc/src/bsc_init.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/openbsc/src/bsc_init.c b/openbsc/src/bsc_init.c
index ce3d0b4..b72624a 100644
--- a/openbsc/src/bsc_init.c
+++ b/openbsc/src/bsc_init.c
@@ -356,8 +356,10 @@ int nm_state_event(enum nm_evt evt, u_int8_t obj_class, void *obj,
switch (obj_class) {
case NM_OC_SITE_MANAGER:
bts = container_of(obj, struct gsm_bts, site_mgr);
- if (new_state->operational == 2 &&
- new_state->availability == NM_AVSTATE_OK)
+ if ((new_state->operational == 2 &&
+ new_state->availability == NM_AVSTATE_OK) ||
+ (new_state->operational == 1 &&
+ new_state->availability == NM_AVSTATE_OFF_LINE))
abis_nm_opstart(bts, obj_class, 0xff, 0xff, 0xff);
break;
case NM_OC_BTS:
--
1.6.5.1
6:28 p.m.
New subject: [PATCH 2/7] [gsm_04_11] Fix typos in debug message / comments
From: Sylvain Munaut <tnt(a)246tNt.com>
Signed-off-by: Sylvain Munaut <tnt(a)246tNt.com>
---
openbsc/src/gsm_04_11.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/openbsc/src/gsm_04_11.c b/openbsc/src/gsm_04_11.c
index 4930e5c..d256f7e 100644
--- a/openbsc/src/gsm_04_11.c
+++ b/openbsc/src/gsm_04_11.c
@@ -173,7 +173,7 @@ static int gsm411_cp_sendmsg(struct msgb *msg, struct gsm_trans
*trans,
DEBUGP(DSMS, "TX: CP-ACK ");
break;
case GSM411_MT_CP_ERROR:
- DEBUGP(DSMS, "TX: CP-ACK ");
+ DEBUGP(DSMS, "TX: CP-ERROR ");
break;
}
@@ -958,7 +958,7 @@ int gsm0411_rcv_sms(struct msgb *msg, u_int8_t link_id)
bsc_del_timer(&trans->sms.cp_timer);
if (!trans->sms.is_mt) {
- /* FIXME: we have sont one CP-DATA, which was now
+ /* FIXME: we have sent one CP-DATA, which was now
* acknowledged. Check if we want to transfer more,
* i.e. multi-part message */
trans->sms.cp_state = GSM411_CPS_IDLE;
--
1.6.5.1
6:28 p.m.
New subject: [PATCH 3/7] [gsm_04_11] Fix transaction_id displayed in debug message
From: Sylvain Munaut <tnt(a)246tNt.com>
In SMS debug messages, we always display the transaction ID
as if we were 'sending' the message.
Signed-off-by: Sylvain Munaut <tnt(a)246tNt.com>
---
openbsc/src/gsm_04_11.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/openbsc/src/gsm_04_11.c b/openbsc/src/gsm_04_11.c
index d256f7e..84139e2 100644
--- a/openbsc/src/gsm_04_11.c
+++ b/openbsc/src/gsm_04_11.c
@@ -906,7 +906,7 @@ int gsm0411_rcv_sms(struct msgb *msg, u_int8_t link_id)
return -EIO;
/* FIXME: send some error message */
- DEBUGP(DSMS, "trans_id=%x ", gh->proto_discr >> 4);
+ DEBUGP(DSMS, "trans_id=%x ", transaction_id);
trans = trans_find_by_id(lchan->subscr, GSM48_PDISC_SMS,
transaction_id);
if (!trans) {
--
1.6.5.1
6:28 p.m.
New subject: [PATCH 4/7] [gsm_04_11] Use 'new' rather than 'unknown' when starting transaction
From: Sylvain Munaut <tnt(a)246tNt.com>
'unknown' has a negative connotation for a case that's totally
normal so refer to it as 'new' so it doesn't sound like a problem.
Signed-off-by: Sylvain Munaut <tnt(a)246tNt.com>
---
openbsc/src/gsm_04_11.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/openbsc/src/gsm_04_11.c b/openbsc/src/gsm_04_11.c
index 84139e2..31fcabf 100644
--- a/openbsc/src/gsm_04_11.c
+++ b/openbsc/src/gsm_04_11.c
@@ -910,7 +910,7 @@ int gsm0411_rcv_sms(struct msgb *msg, u_int8_t link_id)
trans = trans_find_by_id(lchan->subscr, GSM48_PDISC_SMS,
transaction_id);
if (!trans) {
- DEBUGPC(DSMS, "(unknown) ");
+ DEBUGPC(DSMS, "(new) ");
trans = trans_alloc(lchan->subscr, GSM48_PDISC_SMS,
transaction_id, new_callref++);
if (!trans) {
--
1.6.5.1
6:28 p.m.
New subject: [PATCH 5/7] [gsm_04_08] Fix subscr ref leak for multi CM SERV REQ
From: Sylvain Munaut <tnt(a)246tNt.com>
Multiple CM SERVICE REQUEST can happen on a single RR
connection, in this case, since the subscr reference is
tracked through lchan->subscr and will only be put'd once
on lchan_free, we need to make sure we don't get several
reference ....
Signed-off-by: Sylvain Munaut <tnt(a)246tNt.com>
---
openbsc/src/gsm_04_08.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/openbsc/src/gsm_04_08.c b/openbsc/src/gsm_04_08.c
index 5fea6bf..f87b335 100644
--- a/openbsc/src/gsm_04_08.c
+++ b/openbsc/src/gsm_04_08.c
@@ -1343,7 +1343,9 @@ static int gsm48_rx_mm_serv_req(struct msgb *msg)
if (!msg->lchan->subscr)
msg->lchan->subscr = subscr;
- else if (msg->lchan->subscr != subscr) {
+ else if (msg->lchan->subscr == subscr)
+ subscr_put(subscr); /* lchan already has a ref, don't need another one */
+ else {
DEBUGP(DMM, "<- CM Channel already owned by someone else?\n");
subscr_put(subscr);
}
--
1.6.5.1
6:28 p.m.
New subject: [PATCH 6/7] [transaction] Never allocate transaction ID 7
From: Sylvain Munaut <tnt(a)246tNt.com>
According to GSM 04.07 11.2.3.1.3 , TID 7 is "reserved
for future extensions".
Signed-off-by: Sylvain Munaut <tnt(a)246tNt.com>
---
openbsc/src/transaction.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/openbsc/src/transaction.c b/openbsc/src/transaction.c
index 9f1bbf3..1d58333 100644
--- a/openbsc/src/transaction.c
+++ b/openbsc/src/transaction.c
@@ -133,7 +133,7 @@ int trans_assign_trans_id(struct gsm_subscriber *subscr,
used_tid_bitmask |= (1 << trans->transaction_id);
}
- for (i = 0; i <= 7; i++) {
+ for (i = 0; i < 7; i++) {
if ((used_tid_bitmask & (1 << (i | ti_flag))) == 0)
return i | ti_flag;
}
--
1.6.5.1
6:28 p.m.
New subject: [PATCH 7/7] [gsm_04_11] Replace hardcoded transaction_id by real allocation
From: Sylvain Munaut <tnt(a)246tNt.com>
Signed-off-by: Sylvain Munaut <tnt(a)246tNt.com>
---
openbsc/src/gsm_04_11.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/openbsc/src/gsm_04_11.c b/openbsc/src/gsm_04_11.c
index 31fcabf..9e85f9b 100644
--- a/openbsc/src/gsm_04_11.c
+++ b/openbsc/src/gsm_04_11.c
@@ -1005,7 +1005,11 @@ int gsm411_send_sms_lchan(struct gsm_lchan *lchan, struct gsm_sms
*sms)
u_int8_t transaction_id;
int rc;
- transaction_id = 4; /* FIXME: we always use 4 for now */
+ transaction_id = trans_assign_trans_id(lchan->subscr, GSM48_PDISC_SMS, 0);
+ if (transaction_id == -1) {
+ DEBUGP(DSMS, "No available transaction ids\n");
+ return -EBUSY;
+ }
msg->lchan = lchan;
--
1.6.5.1
22 Dec
22 Dec
1:46 p.m.
New subject: [PATCH 7/7] [gsm_04_11] Replace hardcoded transaction_id by real allocation
thanks, applied.
--
- Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
19 Dec
19 Dec
3:28 a.m.
On Friday 18 December 2009 18:28:05 Sylvain Munaut wrote:
Here's a small series with some minor fixes I
found when looking at
multiple SMS in a single RR connection.
For the MO case, it actually now works as described in the spec in the
no-error case. But what can happen is that after the second
CM SERVICE REQUEST, we may never receive the last CP-ACK of the previous
transaction (and thus leak a transaction). The spec says than when
receiving a CP-DATA after a CM SERVICE REQUEST we should consider we also
implicitely received the last CP-ACK of the previous transaction ...
that's not implemented yet.
For the MT case, it currently makes several RR connection which is quite
bad ... to be fixed.
Great,
I have applied 2-6, for 1 and 7 I would like to let Harald review it (they
seem fine but I can't judge the impacts).
z.
5243
days inactive
5244
days old
9 comments
3 participants
participants (3)
-
Harald Welte -
Holger Freyther -
Sylvain Munaut