14 Jan
2014
14 Jan
'14
12:58 p.m.
hi,
while re-basing the osmo-bts-trx code with master, i saw that the ABIS
code has been replaced by libosmo-abis. libosmo-abis is not capable of
handling multiple RSL connections, so the multi-TRX support is broke
now. the attached patch will add multiple RSL connection support to
libosmo-abis.
regards,
andreas
14 Jan
14 Jan
5:55 p.m.
New subject: [PATCH] Support of multiple RSL connections for ABIS/ipaccess (BTS side)
On Tue, Jan 14, 2014 at 12:58:11PM +0100, Andreas Eversberg wrote:
Good Evening Andreas,
The code was tested with osmobts-trx.
What does that it mean? What was the result of the test? What was
the content of the test. "I tested it" is a sentence without any
content. ;)
The user of e1inp_ipa_bts_rsl_connect() (which is
osmo-bts) must be
upgraded after applying the patch.
okay. thanks for the patch.
- dev->site_id, dev->bts_id,
dev->trx_id);
+ dev->site_id, dev->bts_id, trx_nr);
We have an inconsistency problem here. From a design point of view it is
bad to have dev->trx_id and a way to pass in the trx_nr. We can't remove
dev->trx_id as OpenBSC is using it. Why can't we have more than one
instance of the ipaccess_unit structure? For the sysmoBTS 2050 the mac
addresses might/will be different too.
+ if (link->ofd->priv_nr >=
E1INP_SIGN_RSL)
+ trx_nr = link->ofd->priv_nr - E1INP_SIGN_RSL;
+ e1i_ts =
&link->line->ts[link->ofd->priv_nr-1];
Where does this difference come from?
What are the alternatives to this patch? Can we copy the cfg.ipa fields
into a per RSL line field and set the trx_id there before the first time
we use it?
holger
15 Jan
15 Jan
10:09 a.m.
New subject: [PATCH] Support of multiple RSL connections for ABIS/ipaccess (BTS side)
Holger Hans Peter Freyther wrote:
hi holger,
the priv_nr is set to E1INP_SIGN_OML (1) or E1INP_SIGN_RSL+trx_nr (2..).
in order to retrieve the trx_nr from priv_nr, i substract
E1INP_SIGN_RSL. the priv_nr is also the index for selecting the ts
structure. because we do not have a physical E1 line, we can use ts[0]
also, so '1' is substracted.
What does that it mean? What was the result of the
test? What was
the content of the test. "I tested it" is a sentence without any
content. ;)
i setup a UmTRX with two transceivers and used the jolly/trx_rebased
branch to develop and test this patch. then i made calls on timeslots of
different TRX, to see if the correct RSL connection is used to handle
the channel. RSL connections on both TRX worked.
+ if
(link->ofd->priv_nr >= E1INP_SIGN_RSL)
+ trx_nr = link->ofd->priv_nr - E1INP_SIGN_RSL;
+ e1i_ts = &link->line->ts[link->ofd->priv_nr-1];
Where does this difference come from?
What are the alternatives to this patch? Can we copy
the cfg.ipa fields
into a per RSL line field and set the trx_id there before the first time
we use it?
i looked at the data structure. the ipaccess_unit structure, which is
linked to cfg.ipa.dev could be removed. instead we could provide
individual ipaccess_unit structure per RSL/OML link. when calling
e1inp_ipa_bts_rsl_connect(), we could provide a pointer to the
individual structure. this pointer must be stored inside ipa_client_conn
structure then.
alternatively we could provide an array of ipaccess_unit, instead of a
single structure and attach it to cfg.ipa.dev.
in both cases, the BTS code must care of filling the ipaccess_unit
structures with data for OML and each RSL link.
regards,
andreas
20 Jan
20 Jan
8:33 a.m.
New subject: [PATCH] Support of multiple RSL connections for ABIS/ipaccess (BTS side)
On Wed, Jan 15, 2014 at 10:09:16AM +0100, Andreas Eversberg wrote:
alternatively we could provide an array of
ipaccess_unit, instead of a
single structure and attach it to cfg.ipa.dev.
in both cases, the BTS code must care of filling the ipaccess_unit
structures with data for OML and each RSL link.
please do that.
3:12 p.m.
New subject: [PATCH] Support of multiple RSL connections for ABIS/ipaccess (BTS side)
dear holger,
i change the patch. also i attached the change that is required for
osmo-bts (sysmobts) to work afterwards.
best regards,
andreas
2:28 p.m.
New subject: [PATCH] Support of multiple RSL connections for ABIS/ipaccess (BTS side)
On Mon, Jan 20, 2014 at 03:12:00PM +0100, Andreas Eversberg wrote:
dear holger,
i change the patch. also i attached the change that is required for
osmo-bts (sysmobts) to work afterwards.
hmm, I might not understand what your patch does but the trx_id is
still in more than a single place. Make the ipaccess_unit the primary
holder of the trx_id.
- e1inp_ipa_bts_rsl_connect(line,
"127.0.0.1", IPA_TCP_PORT_RSL);
+ e1inp_ipa_bts_rsl_connect(line, "127.0.0.1", IPA_TCP_PORT_RSL, 0);
- line = abis_open(bts, btsb->bsc_oml_host, "sysmoBTS");
+ line = abis_open(bts, btsb->bsc_oml_host, "sysmoBTS", 1);
Why do you start with trx_id == 1 in the BTS part but in the manual
case with 0?
8:04 p.m.
New subject: [PATCH] Support of multiple RSL connections for ABIS/ipaccess (BTS side)
Holger Hans Peter Freyther wrote:
the abis_open() function gets the number of TRX to be used, not the
trx_id. as shown above, the abis_open shall create the OML link and 1
RSL link for 1 TRX.
the e1inp_ip_bts_rsl_connect() function, as shown above, gets the
parameter 0, which means it should connect the first RSL link, which
uses trx_id 0.
hmm, I might not understand what your patch does but
the trx_id is
still in more than a single place. Make the ipaccess_unit the primary
holder of the trx_id.
in my last patch (for osmo-bts), the trx_id is set in each
entry of an
array of ipaccess_unit structure. the first entry is used for OML link,
trx_id is not used there, so it is set to 0. the second (and subsequent)
array entry is used for RSL. there the trx_id is set to the TRX number,
starting with 0. (sysmobts only uses one TRX.)
>- e1inp_ipa_bts_rsl_connect(line, "127.0.0.1", IPA_TCP_PORT_RSL);
>+ e1inp_ipa_bts_rsl_connect(line, "127.0.0.1", IPA_TCP_PORT_RSL, 0);
>- line = abis_open(bts, btsb->bsc_oml_host, "sysmoBTS");
>+ line = abis_open(bts, btsb->bsc_oml_host, "sysmoBTS", 1);
Why
do you start with trx_id == 1 in the BTS part but in the manual
case with 0?
8 p.m.
New subject: [PATCH] Support of multiple RSL connections for ABIS/ipaccess (BTS side)
On Mon, Jan 20, 2014 at 08:04:22PM +0100, Andreas Eversberg wrote:
Dear Andreas,
my initial comment was that information should be at a single place.
The reason is that due bitrot/misc/etc. can change and one might think
one talks about the same thing when in fact one doesn't.
In the spirit of avoiding broken windows we have to avoid such
inconsistencies when they come up.
in my last patch (for osmo-bts), the trx_id is set in
each entry of
an array of ipaccess_unit structure. the first entry is used for OML
link, trx_id is not used there, so it is set to 0. the second (and
subsequent) array entry is used for RSL. there the trx_id is set to
the TRX number, starting with 0. (sysmobts only uses one TRX.)
>>>- e1inp_ipa_bts_rsl_connect(line, "127.0.0.1", IPA_TCP_PORT_RSL);
>>>+ e1inp_ipa_bts_rsl_connect(line, "127.0.0.1", IPA_TCP_PORT_RSL,
0);
>>>- line = abis_open(bts, btsb->bsc_oml_host, "sysmoBTS");
>>>+ line = abis_open(bts, btsb->bsc_oml_host, "sysmoBTS", 1);
>Why do you start with trx_id == 1 in the BTS part but in the manual
>case with 0?
the abis_open() function gets the number of TRX to be
used, not the
trx_id. as shown above, the abis_open shall create the OML link and
1 RSL link for 1 TRX.
the e1inp_ip_bts_rsl_connect() function, as shown above, gets the
parameter 0, which means it should connect the first RSL link, which
uses trx_id 0.
With your patch there is:
* trx->nr
* trx_id in the parameters
* trx_id in the ipaccess_unit
So how can we reduce the amount of duplication? trx->nr will not go
away. Can we change e1inp_ipa_bts_rsl_connect to already work on a
sign_link (that normally would be created more late, probably not)?
any ideas?
holger
21 Jan
21 Jan
11:19 a.m.
New subject: [PATCH] Support of multiple RSL connections for ABIS/ipaccess (BTS side)
Holger Hans Peter Freyther wrote:
th your patch there is:
* trx->nr
* trx_id in the parameters
* trx_id in the ipaccess_unit
So how can we reduce the amount of duplication? trx->nr will not go
away. Can we change e1inp_ipa_bts_rsl_connect to already work on a
sign_link (that normally would be created more late, probably not)?
dear holger,
i looked at the code again. my thoughts are:
the trx_id in the parameters (that is e1inp_ipa_bts_rsl_connect) should
be renamed to "index". that is what the parameter is used for: the index
of the RSL link instance. we could provide sign_link instead of some
index, but inside e1inp_ipa_bts_rsl_connect we call
ipa_client_conn_create, which also requires an index. (this index is
stored at priv_nr of the osmo_fd, to resolve ABIS link when receiving data.)
the trx_id in the ipaccess_unit is something that is replied to the BSC,
to identify the TRX. there should be no relation between the value of
trx_id and the actual RSL link instance. this makes sense, if multiple
devices provide different TRX for the same BTS. (e.g. first device
provides two TRX with trx_id 0 and 1, and second device provides two
additional TRX with trx_id 2 and 3. both devices have only two RSL link
instances.)
best regards,
andreass
10:54 a.m.
New subject: [PATCH] Support of multiple RSL connections for ABIS/ipaccess (BTS side)
On Tue, Jan 21, 2014 at 11:19:17AM +0100, Andreas Eversberg wrote:
dear andreas,
i looked at the code again. my thoughts are:
the trx_id in the parameters (that is e1inp_ipa_bts_rsl_connect)
should be renamed to "index". that is what the parameter is used
for: the index of the RSL link instance. we could provide sign_link
instead of some index, but inside e1inp_ipa_bts_rsl_connect we call
ipa_client_conn_create, which also requires an index. (this index is
stored at priv_nr of the osmo_fd, to resolve ABIS link when
receiving data.)
thanks a lot. I went through the code as well and I think the first
version you sent is better than the second one. On the first version
what I am missing is some documentation on how we segment the TS
array for OML/RSL and a range check for the trx/index. Do you think
you could add this to the first patch?
thanks
holger
4:17 p.m.
New subject: [PATCH] Support of multiple RSL connections for ABIS/ipaccess (BTS side)
Holger Hans Peter Freyther wrote:
thanks a lot. I went through the code as well and I
think the first
version you sent is better than the second one. On the first version
what I am missing is some documentation on how we segment the TS
array for OML/RSL and a range check for the trx/index. Do you think
you could add this to the first patch?
dear holger,
see attachment.
best regards,
andreas
4169
days inactive
4176
days old
10 comments
2 participants
participants (2)
-
Andreas Eversberg -
Holger Hans Peter Freyther