Good morning,
I wanted to get inter-BSC handover complete before today, and worked like a maniac for the last two weeks.
I'm looking forward to your comments on the osmo-bsc rework, which started so small and rippled through the entire code base like a flash flood. I hope you will find review of it not too daunting, it fully qualifies as a code bomb.
It is refactoring no less than: - dynamic timeslots, - lchan states from allocation through release, - assignment, - handover - handling of MGW endpoints, and - the conn FSM. - gsm_network->T123 timer handling, - the way a BTS's neighbors are configured, - handover and assignment rate counters, - and whatnot. - adding five new FSM implementations to osmo-bsc, - and actually adding inter-BSC Handover, both sides (almost).
Of course I couldn't wrap things up completely, but the good news is that the only problem left now is that I can't get a ttcn3 test written for inter-bsc MT handover [1]. Consider the MT side broken yet, though it should be small fixes.
So the refactoring is done and all ttcn3-bsc-tests pass. Yes, *all*!
I will now submit the patches to gerrit; some though could want to go to libosmocore, and for some other "neat" inventions of mine I'm watching the comment space in curiosity.
I did try to separate some changes into individual commits, but the final chunk is too inter-related to pull it apart without spending huge amounts of time to make osmo-bsc work for each intermediate state.
I feel both like saying "sorry" and "you're welcome" about this at the same time. Sorry about the size of the single patch; but a lot of code got extremely easier to read and follow, and a lot safer too, into a nicer osmo-bsc future.
~N
[1] I have a marginal test added for inter-bsc MO, but the inter-bsc MT gets me. The problem there is that it is the first time a conn is initiated by the MSC side. I can get that to work by using BSSAP on that Test_CT. It goes well up to the point where osmo-bsc sends MGCP CRCX and expects a response. Now I want to use the MSC_ConnHdlr to manage MGCP, but I can't for the life of me figure out how to be able to do both at the same time. I tried for very long to resolve "BSC_Tests.ttcn:2755.13-2756.31: error: Message type `@BSSAP_CodecPort.BSSAP_N_CONNECT_req' is not present on the outgoing list of port type `@BSSMAP_Emulation.BSSAP_Conn_PT'" but it feels like a brick wall... It's the final thing before I can say: I'm done, take a look at inter-BSC HO.
Hi Neels,
sorry for the slow response, I've been down with fever and minding the bed during most parts of last week.
I've only started to review your patchset, but let me provide some answer:
On Mon, Jun 18, 2018 at 07:50:56AM +0200, Neels Hofmeyr wrote:
I have a marginal test added for inter-bsc MO, but the inter-bsc MT gets me. The problem there is that it is the first time a conn is initiated by the MSC side.
I can get that to work by using BSSAP on that Test_CT.
that's not such an elegant choice, as it means we will never be able to run multuple of those in parallel. The point of the various *_Emulation.ttcn layers is to (de)multiplex per subscriber/connection/lchan/... and to be able to handle all parts related to *one* particular subscriber/connection in a component, while at the same time having any number of other components handling other concurrent connections/subscribers.
It goes well up to the point where osmo-bsc sends MGCP CRCX and expects a response. Now I want to use the MSC_ConnHdlr to manage MGCP, but I can't for the life of me figure out how to be able to do both at the same time.
The only clean solutions is to do everything from within a *_ConnHdlr.
The logical flow of events should be: * start BSSMAP_Emulation etc. using "f_init(N, true)" * start a ConnHdlr component, from there ** send a BSSAP_Conn_Req to the BSSMAP_Emulation, which consists of the SCCP called / calling party address and the BSSAP to send in the first message.
I tried for very long to resolve "BSC_Tests.ttcn:2755.13-2756.31: error: Message type `@BSSAP_CodecPort.BSSAP_N_CONNECT_req' is not present on the outgoing list of port type `@BSSMAP_Emulation.BSSAP_Conn_PT'" but it feels like a brick wall...
This specific error message tells you that the BSSAP_N_CONNECT_req type is not listed in the singature of the BSSAP_Conn_PT. Let's look at its definition:
/* port between individual per-connection components and this dispatcher */ type port BSSAP_Conn_PT message { /* BSSAP or direct DTAP messages from/to clients */ inout PDU_BSSAP, PDU_DTAP_MO, PDU_DTAP_MT, /* misc indications / requests between SCCP and client */ BSSAP_Conn_Prim, /* Client requests us to create SCCP Connection */ BSSAP_Conn_Req, /* MGCP, only used for IPA SCCPlite (MGCP in IPA mux) */ MgcpCommand, MgcpResponse; } with { extension "internal" };
and this definition clearly doesn't list the related type in its signature. So you're sending something to the port which it doesn't support.
After completing my initial review, some more follow-up here.
I think the general strategy to move ahead here is: * address review comments. Sorry there are so many. A lot of them are surprisingly coding style related, where there are many deviations from existing coding style. * make sure that test results with old and new code are identical. ** for tests that don't pass, figure out if it's a regression or if the test expectation is/was wrong. * do some manual testing [not sure if you "only" relied on test suites or also did manual testing with actual BTS / phones? * run the branch in osmo-gsm-tester, also expect identical behavior between master and the inter-bsc-ho branch
*before* we actually do the merge, we should tag a version on the last version before the merge, and do a release. This way we make it easy for people to use the older / more trusted codebase and compare against the new code in master.
Thanks again for all your efforts!
On Mon, Jun 25, 2018 at 03:57:54PM +0200, Harald Welte wrote:
I tried for very long to resolve "BSC_Tests.ttcn:2755.13-2756.31: error: Message type `@BSSAP_CodecPort.BSSAP_N_CONNECT_req' is not present on the outgoing list of port type `@BSSMAP_Emulation.BSSAP_Conn_PT'" but it feels like a brick wall...
This specific error message tells you that the BSSAP_N_CONNECT_req type is not listed in the singature of the BSSAP_Conn_PT. Let's look at its definition:
/* port between individual per-connection components and this dispatcher */ type port BSSAP_Conn_PT message { /* BSSAP or direct DTAP messages from/to clients */ inout PDU_BSSAP, PDU_DTAP_MO, PDU_DTAP_MT, /* misc indications / requests between SCCP and client */ BSSAP_Conn_Prim, /* Client requests us to create SCCP Connection */ BSSAP_Conn_Req, /* MGCP, only used for IPA SCCPlite (MGCP in IPA mux) */ MgcpCommand, MgcpResponse; } with { extension "internal" };
and this definition clearly doesn't list the related type in its signature. So you're sending something to the port which it doesn't support.
Exactly, and I tried to add it, but couldn't figure out how to make it work. I don't remember the details anymore, but I felt very dumb...
In short, can you help me out by making this part compile?
private function f_tc_ho_into_this_bsc(charstring id) runs on MSC_ConnHdlr { BSSAP.send(ts_BSSAP_CONNECT_req(g_bssap.sccp_addr_peer, g_bssap.sccp_addr_own, 2342, ts_BSSMAP_HandoverRequest())); BSSAP.receive(tr_BSSAP_CONNECT_cfm(2342, omit)); }
(the ts_BSSMAP_HandoverRequest definition is in https://gerrit.osmocom.org/#/c/osmo-ttcn3-hacks/+/9674/4/library/BSSMAP_Temp... )
That would be great.
~N
Hi Neels,
On Mon, Jul 02, 2018 at 03:49:08PM +0200, Neels Hofmeyr wrote:
In short, can you help me out by making this part compile?
No, sorry ;)
Your solution is to send "BSSAP_Conn_Req" through the BSSAP_Conn_PT, not ts_BSSAP_CONNET_req. The latter is a primitive of the BSSAP_CodecPort, and not the BSSAP_Conn_PT.
-
Harald Welte -
Neels Hofmeyr