25 Nov
2009
25 Nov
'09
8:47 p.m.
Hi Holger!
I'm currently reviewing changeset 2d501ea26a219176b1c556449e45ebd90d4accfb
and just noticed that you introduce a new 'int rf_locked' member to struct
gsm_bts_trx, which I believe is not needed.
We are already tracking the administrative state of all objects in the BTS
and mirror their state.
The specific state you are looking for should be in
gsm_bts_trx.nm_state.administrative
I would appreciate if you could fix this.
--
- 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)
26 Nov
26 Nov
1:44 a.m.
On Wednesday 25 November 2009 20:47:28 Harald Welte wrote:
Hi Holger!
I'm currently reviewing changeset 2d501ea26a219176b1c556449e45ebd90d4accfb
and just noticed that you introduce a new 'int rf_locked' member to struct
gsm_bts_trx, which I believe is not needed.
We are already tracking the administrative state of all objects in the BTS
and mirror their state.
The specific state you are looking for should be in
gsm_bts_trx.nm_state.administrative
I would appreciate if you could fix this.
I will, honestly I was just too lazy to figure out if the state is updated, how
it should be set on startup. I will try to fix it before FOSS.IN.
z.
20 Dec
20 Dec
2:16 p.m.
On Thursday 26 November 2009 01:44:45 Holger Freyther wrote:
Hi,
I did not forget this. Today I was starting to set the nm_state.administrative
of the trx and it is working for the _LOCKED start up. For the _UNLOCKED
startup we seem to copy a new state to the nm_state which includes changing
the administrative back to _LOCKED so the TRX is getting locked during
startup. I tried using hardware watchpoints on the memory address in gdb but
this made things quite slow (in the Xen instance I was testing).
I think I will have something working by tomorrow.
regards
holger
On Wednesday 25 November 2009 20:47:28 Harald Welte
wrote:
I would
appreciate if you could fix this.
I will, honestly I was just too lazy to figure out if the state is updated,
how it should be set on startup. I will try to fix it before FOSS.IN.
21 Dec
21 Dec
10:35 a.m.
On Sunday 20 December 2009 14:16:31 Holger Freyther wrote:
I think I will have something working by tomorrow.
Using administrative will not make us happy. From what I see in the 12.21 it
has two different meanings. In one way it should be controlled by the BSC and
in another way it can be used by the BTS to send information (MMI whatever
that is...)
So when we set the administrative state of the trx to unlocked it will be
changed in abis_nm_rx_statechg_rep to whatever we got from the BTS (mostly
being locked).
One approach would be to never set administrative in abis_nm but then we would
lose the state of the BTS... and we would not update it when we get acks..
So I was introducing administrative_bsc and administrative_bts to the nm_state
struct and keep the bsc state in the bsc one and the bts field in the bts one.
Now I'm kind of struggling with bringing up the BTS (I make it reboot...)
Harald do you have an idea of how it should look like?
11:52 a.m.
On Monday 21 December 2009 10:35:20 Holger Freyther wrote:
On Sunday 20 December 2009 14:16:31 Holger Freyther
wrote:
I think I will have something working by
tomorrow.
Harald do you have an idea of how it should look
like?
Okay, there is a heisenbug somewhere... (when compiled with -O0..)
This is the patch I would like to commit. The biggest change is in the
abis_nm_rx_statechg_rep function to not use memcmp or assignment
of the structs.
The semantic to the administrative state is that it will be assigned
if it is 0 (default by the BSC) and ignored once it is set. It will still
be updated in the ACK messages...
diff --git a/openbsc/include/openbsc/gsm_data.h
b/openbsc/include/openbsc/gsm_data.h
index f3ec9eb..da085fc 100644
--- a/openbsc/include/openbsc/gsm_data.h
+++ b/openbsc/include/openbsc/gsm_data.h
@@ -273,9 +273,6 @@ struct gsm_bts_trx {
} bs11;
};
struct gsm_bts_trx_ts ts[TRX_NR_TS];
-
- /* NM state */
- int rf_locked;
};
enum gsm_bts_type {
diff --git a/openbsc/src/abis_nm.c b/openbsc/src/abis_nm.c
index e7e3bf0..e7def1a 100644
--- a/openbsc/src/abis_nm.c
+++ b/openbsc/src/abis_nm.c
@@ -812,12 +812,17 @@ static int abis_nm_rx_statechg_rep(struct msgb *mb)
}
DEBUGPC(DNM, "\n");
- if (memcmp(&new_state, nm_state, sizeof(new_state))) {
+ if ((new_state.administrative != 0 && nm_state->administrative == 0) ||
+ new_state.operational != nm_state->operational ||
+ new_state.availability != nm_state->availability) {
/* Update the operational state of a given object in our in-memory data
* structures and send an event to the higher layer */
void *obj = objclass2obj(bts, foh->obj_class, &foh->obj_inst);
rc = nm_state_event(EVT_STATECHG_OPER, foh->obj_class, obj, nm_state,
&new_state);
- *nm_state = new_state;
+ nm_state->operational = new_state.operational;
+ nm_state->availability = new_state.availability;
+ if (nm_state->administrative == 0)
+ nm_state->administrative = new_state.administrative;
}
#if 0
if (op_state == 1) {
@@ -2781,7 +2786,7 @@ void gsm_trx_lock_rf(struct gsm_bts_trx *trx, int
locked)
{
int new_state = locked ? NM_STATE_LOCKED : NM_STATE_UNLOCKED;
- trx->rf_locked = locked;
+ trx->nm_state.administrative = new_state;
if (!trx->bts || !trx->bts->oml_link)
return;
diff --git a/openbsc/src/bsc_init.c b/openbsc/src/bsc_init.c
index 8450af6..d4e0616 100644
--- a/openbsc/src/bsc_init.c
+++ b/openbsc/src/bsc_init.c
@@ -429,8 +429,7 @@ static int sw_activ_rep(struct msgb *mb)
* This code is here to make sure that on start
* a TRX remains locked.
*/
- int rc_state = trx->rf_locked ?
- NM_STATE_LOCKED : NM_STATE_UNLOCKED;
+ int rc_state = trx->nm_state.administrative;
/* Patch ARFCN into radio attribute */
nanobts_attr_radio[5] &= 0xf0;
nanobts_attr_radio[5] |= trx->arfcn >> 8;
diff --git a/openbsc/src/gsm_data.c b/openbsc/src/gsm_data.c
index 94ed91b..62576bd 100644
--- a/openbsc/src/gsm_data.c
+++ b/openbsc/src/gsm_data.c
@@ -126,6 +126,7 @@ struct gsm_bts_trx *gsm_bts_trx_alloc(struct gsm_bts *bts)
trx->bts = bts;
trx->nr = bts->num_trx++;
+ trx->nm_state.administrative = NM_STATE_UNLOCKED;
for (k = 0; k < TRX_NR_TS; k++) {
struct gsm_bts_trx_ts *ts = &trx->ts[k];
10:10 p.m.
On Mon, Dec 21, 2009 at 10:35:20AM +0100, Holger Freyther wrote:
On Sunday 20 December 2009 14:16:31 Holger Freyther
wrote:
MMI == man machine interface.
I think I will have something working by
tomorrow.
Using administrative will not make us happy. From what I see in the 12.21 it
has two different meanings. In one way it should be controlled by the BSC and
in another way it can be used by the BTS to send information (MMI whatever
that is...) [...]
Harald do you have an idea of how it should look like?
I really don't understand the problem, sorry.
5242
days inactive
5268
days old
5 comments
2 participants
participants (2)
-
Harald Welte -
Holger Freyther