26 Jun
2013
26 Jun
'13
4:37 p.m.
Dear Andreas, Ivan,
when trying to understand the CPU usage of the PCU without having
any subscribers I noticed the following backtrace:
#0 _int_malloc (av=0x402fe4d4, bytes=52) at malloc.c:3241
#1 0x4023a3b8 in __GI___libc_malloc (bytes=52) at malloc.c:2859
#2 0x4929b6e8 in _talloc_zero () from /usr/lib/libosmocore.so.4
#3 0x49422e44 in vector_init () from /usr/lib/libosmovty.so.0
#4 0x4941f1f8 in install_element () from /usr/lib/libosmovty.so.0
#5 0x492e602c in gprs_ns_vty_init () from /usr/lib/libosmogb.so.2
#6 0x0001b670 in gprs_bssgp_create (local_port=<optimized out>, sgsn_ip=184194049,
sgsn_port=<optimized out>, nsei=<optimized out>, nsvci=2, bvci=2,
mcc=<optimized out>,
mnc=<optimized out>, lac=1, rac=0, cell_id=0) at gprs_bssgp_pcu.cpp:585
#7 0x00016990 in pcu_rx_info_ind (info_ind=0x7) at pcu_l1_if.cpp:431
#8 pcu_rx (msg_type=<optimized out>, pcu_prim=0x3) at pcu_l1_if.cpp:577
#9 0x00017914 in pcu_sock_read (bfd=0x6a3f0) at sysmo_sock.cpp:151
#10 pcu_sock_cb (bfd=0x6a3f0, flags=1) at sysmo_sock.cpp:218
#11 0x49294ff4 in osmo_select_main () from /usr/lib/libosmocore.so.4
#12 0x0000a668 in main (argc=<optimized out>, argv=0xbee40b04) at pcu_main.cpp:219
This means when a info indication is received gprs_bssgp_create will
be called which will re-initialize the VTY commands but more important
re-set vty_nsi inside the VTY code of libosmogb. But it also means that
vty_nsi will be a dangling pointer in case of
1.) gprs_ns_nsip_connect, btsctx_alloc, gprs_ns_nsip_listen fail
2.) Between the sysmobts software exiting and re-starting and bringing
OML back up
3.) More cases that I might not understand.
The dangling pointer will lead to a crash when the VTY of the PCU is
used in such situation/window.
does any of you care to fix that race? I think the easiest would be
to just close/open/bind/listen/connect on the NS once we get a new
config.
holger
--
- Holger Freyther <hfreyther(a)sysmocom.de> http://www.sysmocom.de/
=======================================================================
* sysmocom - systems for mobile communications GmbH
* Schivelbeiner Str. 5
* 10439 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B
* Geschaeftsfuehrer / Managing Directors: Holger Freyther, Harald Welte
28 Jun
28 Jun
4 p.m.
Holger Hans Peter Freyther wrote:
The dangling pointer will lead to a crash when the VTY
of the PCU is
used in such situation/window.
hi holger,
the main problem is that the bssgp/ns instance is created at the time
the bts (and the pcu) receives informations from bsc. the pcu cannot
(yet) instantiate at startup time, the required informations for the
instance are not yet present then. this means that the pcu.cfg cannot
have any ns/nse vty command. i don't see a good solution right now.
maybe it is possible to create only the bssgp/ns instance itself,
without creating, listening, connecting the nsvc. then, if the bts
becomes unavailable, the nsvc is destroyed only, then the nsvc can be
created again on next start of bts.
regards,
andreas
4:52 p.m.
Hi Andreas,
On Fri, Jun 28, 2013 at 04:00:56PM +0200, Andreas Eversberg wrote:
the main problem is that the bssgp/ns instance is
created at the time
the bts (and the pcu) receives informations from bsc. the pcu cannot
(yet) instantiate at startup time, the required informations for the
instance are not yet present then. this means that the pcu.cfg cannot
have any ns/nse vty command. i don't see a good solution right now.
maybe it is possible to create only the bssgp/ns instance itself,
without creating, listening, connecting the nsvc.
The existing libgb API should cater for that. You can very well just
call gprs_ns_instantiate() and gprs_ns_vty_init() at PCU startup time,
but call gprs_ns_nsip_listen() and gprs_ns_nsip_connect() at the time
you receive the configuration from the BTS via the PCU_IF_MSG_INFO_IND.
Wouldn't that solve the problem?
The fact that the VTY would parse statically configured 'nse' sections
in the config file should not be a problem, we can simply assume that
the user does not put any such config sections in the config file for
now. If you're worried about saving them via 'write file / copy
running-config startup-config': this is what the gprs_nsvc.persistent
member is for, isn't it? Any non-persistent nsvc's will not be saved
via the VTY code.
Maybe I'm not understanding the problem yet, sorry.
Regards,
Harald
--
- 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)
29 Jun
29 Jun
9:55 a.m.
On Fri, Jun 28, 2013 at 04:52:28PM +0200, Harald Welte wrote:
The existing libgb API should cater for that. You can very well just
call gprs_ns_instantiate() and gprs_ns_vty_init() at PCU startup time,
but call gprs_ns_nsip_listen() and gprs_ns_nsip_connect() at the time
you receive the configuration from the BTS via the PCU_IF_MSG_INFO_IND.
Wouldn't that solve the problem?
Exactly.
Maybe I'm not understanding the problem yet,
sorry.
the "problem" comes from the fact that in case the "BTS" exits, the
next
time the BTS restarts it might gets different IP/PORT settings from
the BSC. So what was/is missing is a "gprs_ns_nsip_shutdown" which
will close the old socket and the PCU should look into destroying the
NSVC that got allocated for the old remote.
I don't know how to phrase it but I mean it nicely. Both Fairwaves
and sysmocom intend to use the PCU commercially. This means when we
wrote software we should avoid races or at least document them in a
TODO file. Because no matter how small the race, we will eventually
lose it. And the cost of understanding what went wrong is certainly
a lot higher than doing the right thing from the beginning.
I am attaching two diffs as a proposal (not even compile tested), they
should avoid the race condition.
cheers
holger
--
- Holger Freyther <hfreyther(a)sysmocom.de> http://www.sysmocom.de/
=======================================================================
* sysmocom - systems for mobile communications GmbH
* Schivelbeiner Str. 5
* 10439 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B
* Geschaeftsfuehrer / Managing Directors: Holger Freyther, Harald Welte
10:03 a.m.
On Sat, Jun 29, 2013 at 11:55 AM, Holger Hans Peter Freyther
<hfreyther(a)sysmocom.de> wrote:
On Fri, Jun 28, 2013 at 04:52:28PM +0200, Harald Welte
wrote:
I don't know how to phrase it but I mean it nicely. Both Fairwaves
and sysmocom intend to use the PCU commercially. This means when we
wrote software we should avoid races or at least document them in a
TODO file. Because no matter how small the race, we will eventually
lose it. And the cost of understanding what went wrong is certainly
a lot higher than doing the right thing from the beginning.
I should say I 100% share this attitude and I try to promote it among
Fairwaves. There must be no undocumented known limitations/todos/bugs.
--
Regards,
Alexander Chemeris.
CEO, Fairwaves LLC / ООО УмРадио
http://fairwaves.ru
10:34 a.m.
Hi Holger,
On Sat, Jun 29, 2013 at 09:55:12AM +0200, Holger Hans Peter Freyther wrote:
the "problem" comes from the fact that in
case the "BTS" exits, the next
time the BTS restarts it might gets different IP/PORT settings from
the BSC. So what was/is missing is a "gprs_ns_nsip_shutdown" which
will close the old socket and the PCU should look into destroying the
NSVC that got allocated for the old remote.
Ah, ok. I was (erroneously) assuming that the PCU would simply exit at
the time the connection to osmo-bts drops. This way, it re-spawns and
re-initializes nicely when the BTS is sending the next
PCU_IF_MSG_INFO_IND. However, I may be writing too much Erlang code in
recent days, hence my 'let it exit and re-spawn' attitude ;)
I am attaching two diffs as a proposal (not even
compile tested), they
should avoid the race condition.
Your patches look fine to me. The new gprs_ns_close() declaration of
course would have to go in a libosmogb header file.
I can try to test it later today.
Regards,
Harald
--
- 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)
3:24 p.m.
On Sat, Jun 29, 2013 at 10:34:25AM +0200, Harald Welte wrote:
Hi Holger,
Your patches look fine to me. The new gprs_ns_close()
declaration of
course would have to go in a libosmogb header file.
no rush, I just wanted to check how much work it is to do it properly.
I will write a small test for the semantic of the gprs_ns_close function.
cheers
holger
--
- Holger Freyther <hfreyther(a)sysmocom.de> http://www.sysmocom.de/
=======================================================================
* sysmocom - systems for mobile communications GmbH
* Schivelbeiner Str. 5
* 10439 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B
* Geschaeftsfuehrer / Managing Directors: Holger Freyther, Harald Welte
11 Jul
11 Jul
7:19 p.m.
On Sat, Jun 29, 2013 at 10:34:25AM +0200, Harald Welte wrote:
Good Evening,
Ah, ok. I was (erroneously) assuming that the PCU
would simply exit at
the time the connection to osmo-bts drops. This way, it re-spawns and
re-initializes nicely when the BTS is sending the next
PCU_IF_MSG_INFO_IND. However, I may be writing too much Erlang code in
recent days, hence my 'let it exit and re-spawn' attitude ;)
ironically Harald and me were bitten by incomplete bssgp reset after
the exit/restart of a bts/network. sysmocom would prefer if the PCU
would fail/exit instead of an almost working reset. Currently a shell
script and in the future systemd will relaunch the osmo-pcu process.
I can prepare a patch that adds this behavior as VTY option unless
everyone agrees that the process should exit.
cheers
holger
--
- Holger Freyther <hfreyther(a)sysmocom.de> http://www.sysmocom.de/
=======================================================================
* sysmocom - systems for mobile communications GmbH
* Schivelbeiner Str. 5
* 10439 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B
* Geschaeftsfuehrer / Managing Directors: Holger Freyther, Harald Welte
12 Jul
12 Jul
6:21 p.m.
Holger Hans Peter Freyther wrote:
sysmocom would prefer if the PCU
would fail/exit instead of an almost working reset.
i agree to have this exit case
for more stable productive usage. still
it should be an option. also i would like to know what is actually
incomplete and how i can reproduce this bug.
29 Jun
29 Jun
9:58 p.m.
Holger Hans Peter Freyther wrote:
I am attaching two diffs as a proposal (not even
compile tested), they
should avoid the race condition.
i tried to fix this problem today, but i actually had (other) problems
to get PCU running. now it works, so i will test and review the patches
tomorrow.
30 Jun
30 Jun
10:43 a.m.
hi holger,
with some minor changes, your libosmogb-patch works. (see attachment)
i applied and tested the pcu-patch. there are some changes to correctly
keep track of the current state of the NS instance (open or closed). one
major change is that the NS instance is created and destroyed in the
main function. this way it is possible to parse config after NS instance
is created. you will find this patch at jolly/testing branch of pcu git.
in my jolly/testing branch are three more fixes (patches 2..4) that
prevent messy debugging output due to non/wrong initialized debug
categories.
best regards,
andreas
3952
days inactive
3968
days old
osmocom-net-gprs@lists.osmocom.org
10 comments
4 participants
participants (4)
-
Alexander Chemeris -
Andreas Eversberg -
Harald Welte -
Holger Hans Peter Freyther