Dear all,
after my futile attempt in September last year to finally merge l1sap (see the 201409-l1sap branch), I'm making one final attempt.
The following branches have been created + pushed:
* 201509-l1sap: The core l1sap change that I intend to merge This is most of the heavy lifting by introducing L1SAP between the sysmobts l1 and the common code. Hasn't been modified much.
* 201509-trx-rebase: osmo-bts-trx support that I intend to merge This is what I can merge without too much review, as it does not touch the common code, except some minor l1sap fixes, and * the AMR multirate change, requiring a matching patch on openbsc.git * bts_model_abis_close() which should be fine * ability to configue multiple TRX from VTY
* 201509-fairwaves-rebase Stuff I cannot merge in its current form * the abis/e1inp multi-trx and reconnect changes require in-depth review and testing. Part of it is marked as HACK even by Andreas. * a so-called 'fix for use after free' that is actually a patch that introduces another copy for every primitive and is only required for the loopback mode * copying gsm_data_common.[ch] into the repo for debian packaging [let's not discuss further work on this before l1sap and trx-rebase are merged]
I recommend everyone with a stake in this to review or test the code in 201509-l1sap and 201509-trx-rebase now. My schedule is to merge at some point next week, unless someone raises some serious objections.
What needs to be done after the merge is to unify the src/common/power_control.c code from Holger originating in master with src/common/loops.c from Andreas originating from the osmo-trx branch.
There is no doubt that the power control and timing advance loops should be part of common, as this is required in virtually any BTS hardware.
However, we can safely merge/unify this after the merging of the above branches.
Regards, Harald
Hi Harald,
On Sun, Sep 6, 2015 at 8:09 AM, Harald Welte laforge@gnumonks.org wrote:
after my futile attempt in September last year to finally merge l1sap (see the 201409-l1sap branch), I'm making one final attempt.
That's incredible. Thank you for making this final step.
- a so-called 'fix for use after free' that is actually a patch that introduces another copy for every primitive and is only required for the loopback mode
I'd appreciate recommendations on how to do this differently. IIRC the code frees messages after the function and the queue was pointing to a freed message. this led to undefined behavior.
There is a companion patch to this to manually activate/deactivate a channel. I'd appreciate recommendations on how to properly implement it as well. Loopback and channel activation functions are very helpful for the L0/L1 development.
I recommend everyone with a stake in this to review or test the code in 201509-l1sap and 201509-trx-rebase now. My schedule is to merge at some point next week, unless someone raises some serious objections.
We're little busy this week, so assume we're ok by default. If we find time for testing and find something - we'll let you know. Worst case we'll fix TRX support after it's merged.
Could you share what kind of testing has been performed, so we can shape our expectations?
What needs to be done after the merge is to unify the src/common/power_control.c code from Holger originating in master with src/common/loops.c from Andreas originating from the osmo-trx branch.
There is no doubt that the power control and timing advance loops should be part of common, as this is required in virtually any BTS hardware.
Indeed.
I can promise to look into this, but it'll be a long time before I can do that. If there is any interest in doing this before then - don't hold your breath.
However, we can safely merge/unify this after the merging of the above branches.
Totally agree.
Hi Alexander,
On Mon, Sep 07, 2015 at 01:08:47AM -0500, Alexander Chemeris wrote:
- a so-called 'fix for use after free' that is actually a patch that introduces another copy for every primitive and is only required for the loopback mode
I'd appreciate recommendations on how to do this differently. IIRC the code frees messages after the function and the queue was pointing to a freed message. this led to undefined behavior.
There is a companion patch to this to manually activate/deactivate a channel. I'd appreciate recommendations on how to properly implement it as well. Loopback and channel activation functions are very helpful for the L0/L1 development.
I don't really have a good response for this, other than to keep it out of master (or maybe even a compile time option). 99.9% of all installations will not have any benefit from the extra memcpy(), so I don't want to make it the standard behavior.
We're little busy this week, so assume we're ok by default. If we find time for testing and find something - we'll let you know. Worst case we'll fix TRX support after it's merged.
agreed.
Could you share what kind of testing has been performed, so we can shape our expectations?
Not much, to be honest. We will test for osmo-bts-sysmo during this week. However, no testing will be done by me regarding the osmo-trx related code.
Regards, Harald
HI Harald,
On Tue, Sep 8, 2015 at 10:13 AM, Harald Welte laforge@gnumonks.org wrote:
On Mon, Sep 07, 2015 at 01:08:47AM -0500, Alexander Chemeris wrote:
- a so-called 'fix for use after free' that is actually a patch that introduces another copy for every primitive and is only required for the loopback mode
I'd appreciate recommendations on how to do this differently. IIRC the code frees messages after the function and the queue was pointing to a freed message. this led to undefined behavior.
There is a companion patch to this to manually activate/deactivate a channel. I'd appreciate recommendations on how to properly implement it as well. Loopback and channel activation functions are very helpful for the L0/L1 development.
I don't really have a good response for this, other than to keep it out of master (or maybe even a compile time option). 99.9% of all installations will not have any benefit from the extra memcpy(), so I don't want to make it the standard behavior.
I took a look into this patch today.
So, first of all - as I expected, it changes behavior only for channels marked as loopback. I.e. it has no effect on production systems.
But nevertheless, I found a cleaner solution and checked it in as two first commits at the 201509-fairwaves-rebase branch. If we return "1" from the function, the message is not being freed and we don't need to create a copy of it. I think it worth merging, as it doesn't degrade anything and is a clean fix.
Could you share what kind of testing has been performed, so we can shape our expectations?
Not much, to be honest. We will test for osmo-bts-sysmo during this week. However, no testing will be done by me regarding the osmo-trx related code.
Ok, noted.
I tried to run the code today, but was not able to get osmo-bts to start at all. It stop with an error: <0011> e1_input.c:326 No such E1 driver 'ipa Have you seen this? I guess this comes from the common part of the code, but I don't have much experience with this part of the code, unfortunately.
Hi Alexander,
On Sat, Sep 12, 2015 at 12:19:34AM -0400, Alexander Chemeris wrote:
I tried to run the code today, but was not able to get osmo-bts to start at all. It stop with an error: <0011> e1_input.c:326 No such E1 driver 'ipa Have you seen this? I guess this comes from the common part of the code, but I don't have much experience with this part of the code, unfortunately.
I have seen this and remember fixing it. I will double-check that this fix is in the repo, maybe I forgot to push or didn't push to the right branch. Sorry for that.
Hi Alexander,
On Sat, Sep 12, 2015 at 12:19:34AM -0400, Alexander Chemeris wrote:
I tried to run the code today, but was not able to get osmo-bts to start at all. It stop with an error: <0011> e1_input.c:326 No such E1 driver 'ipa Have you seen this? I guess this comes from the common part of the code, but I don't have much experience with this part of the code, unfortunately.
Please check commit '2e9854e368d72f7481e14ec712de0baede685efd' which should resolve this. Looking forward to your further test results.
On Sat, Sep 12, 2015 at 5:01 AM, Harald Welte laforge@gnumonks.org wrote:
Hi Alexander,
On Sat, Sep 12, 2015 at 12:19:34AM -0400, Alexander Chemeris wrote:
I tried to run the code today, but was not able to get osmo-bts to start at all. It stop with an error: <0011> e1_input.c:326 No such E1 driver 'ipa Have you seen this? I guess this comes from the common part of the code, but I don't have much experience with this part of the code, unfortunately.
Please check commit '2e9854e368d72f7481e14ec712de0baede685efd' which should resolve this. Looking forward to your further test results.
Thanks a lot! Now it starts, but crashes while reading a config file:
#0 cfg_trx_rxgain (self=<optimized out>, vty=<optimized out>, argc=<optimized out>, argv=<optimized out>) at trx_vty.c:232 (gdb) bt #0 cfg_trx_rxgain (self=<optimized out>, vty=<optimized out>, argc=<optimized out>, argv=<optimized out>) at trx_vty.c:232 #1 0x00007f797e2951c7 in cmd_execute_command_strict (vline=vline@entry=0x17ded20, vty=vty@entry=0x17de940, cmd=cmd@entry=0x0) at command.c:2211 #2 0x00007f797e295246 in config_from_file (vty=vty@entry=0x17de940, fp=fp@entry=0x17de6b0) at command.c:2234 #3 0x00007f797e2980b4 in vty_read_file (priv=0x0, confp=0x17de6b0) at vty.c:1458 #4 vty_read_config_file (file_name=0x1763680 "/etc/osmocom/osmo-bts.cfg", priv=priv@entry=0x0) at vty.c:1775 #5 0x00000000004041c6 in main (argc=5, argv=0x7ffd931ee858) at main.c:333
DEFUN(cfg_trx_rxgain, cfg_trx_rxgain_cmd, "rxgain <0-50>", "Set the receiver gain in dB\n" "Gain in dB\n") { struct gsm_bts_trx *trx = vty->index; struct trx_l1h *l1h = trx_l1h_hdl(trx);
l1h->config.rxgain = atoi(argv[0]); l1h->config.rxgain_valid = 1; ^^^^^^^^^^^^^^^^^^^^^^
I'll look into this later if you don't happen to know the reason for this already.
On Sat, Sep 12, 2015 at 11:12 AM, Alexander Chemeris alexander.chemeris@gmail.com wrote:
On Sat, Sep 12, 2015 at 5:01 AM, Harald Welte laforge@gnumonks.org wrote:
On Sat, Sep 12, 2015 at 12:19:34AM -0400, Alexander Chemeris wrote:
I tried to run the code today, but was not able to get osmo-bts to start at all. It stop with an error: <0011> e1_input.c:326 No such E1 driver 'ipa Have you seen this? I guess this comes from the common part of the code, but I don't have much experience with this part of the code, unfortunately.
Please check commit '2e9854e368d72f7481e14ec712de0baede685efd' which should resolve this. Looking forward to your further test results.
Thanks a lot! Now it starts, but crashes while reading a config file:
Please disregard this. I did a clean rebuild and it starts normally now.
I don't have an UmTRX with me to test if it actually works, we'll get back to this in a couple days.
On Sat, Sep 12, 2015 at 7:57 PM, Alexander Chemeris alexander.chemeris@gmail.com wrote:
On Sat, Sep 12, 2015 at 11:12 AM, Alexander Chemeris alexander.chemeris@gmail.com wrote:
On Sat, Sep 12, 2015 at 5:01 AM, Harald Welte laforge@gnumonks.org wrote:
On Sat, Sep 12, 2015 at 12:19:34AM -0400, Alexander Chemeris wrote:
I tried to run the code today, but was not able to get osmo-bts to start at all. It stop with an error: <0011> e1_input.c:326 No such E1 driver 'ipa Have you seen this? I guess this comes from the common part of the code, but I don't have much experience with this part of the code, unfortunately.
Please check commit '2e9854e368d72f7481e14ec712de0baede685efd' which should resolve this. Looking forward to your further test results.
Thanks a lot! Now it starts, but crashes while reading a config file:
Please disregard this. I did a clean rebuild and it starts normally now.
I don't have an UmTRX with me to test if it actually works, we'll get back to this in a couple days.
I did some quick tests with UmSITE today and calls, SMS and USSD seem to work in basic scenarios. Deeper testing TBD.
On 06 Sep 2015, at 15:09, Harald Welte laforge@gnumonks.org wrote:
Dear all,
- 201509-trx-rebase: osmo-bts-trx support that I intend to merge
This is what I can merge without too much review, as it does not touch the common code, except some minor l1sap fixes, and
- the AMR multirate change, requiring a matching patch on openbsc.git
What needs to be done after the merge is to unify the src/common/power_control.c code from Holger originating in master with src/common/loops.c from Andreas originating from the osmo-trx branch.
I think the AMR part could be generalized as well? I am currently adding the configuration for MR to OpenBSC. I wonder if there is value in being able to separate the thresholds for the BTS and the MS?
On Mon, Sep 7, 2015 at 2:24 AM, Holger Freyther holger@freyther.de wrote:
On 06 Sep 2015, at 15:09, Harald Welte laforge@gnumonks.org wrote:
- 201509-trx-rebase: osmo-bts-trx support that I intend to merge
This is what I can merge without too much review, as it does not touch the common code, except some minor l1sap fixes, and
- the AMR multirate change, requiring a matching patch on openbsc.git
What needs to be done after the merge is to unify the src/common/power_control.c code from Holger originating in master with src/common/loops.c from Andreas originating from the osmo-trx branch.
I think the AMR part could be generalized as well?
Sounds like a good idea.
I am currently adding the configuration for MR to OpenBSC. I wonder if there is value in being able to separate the thresholds for the BTS and the MS?
I don't have much experience with AMR, but having more flexibility looks like a better way than less flexibility.
Dear Harald,
Your work is much apreciated, there are a lot of important features in these branches not yet merged to the master.
I will test it on a B200 with a strong focus on PS domain (probably next week because I am quite busy at the moment).
The new Wiki article about how to operate the B200/B210 with the Osmocom projects is also in the pipe :-)
Regards, Csaba
----- Eredeti üzenet ----- Feladó: "Harald Welte" laforge@gnumonks.org Címzett: openbsc@lists.osmocom.org Elküldött üzenetek: Vasárnap, 2015. Szeptember 6. 15:09:09 Tárgy: L1SAP and TRX rebase, the last one (TM)
Dear all,
after my futile attempt in September last year to finally merge l1sap (see the 201409-l1sap branch), I'm making one final attempt.
The following branches have been created + pushed:
* 201509-l1sap: The core l1sap change that I intend to merge This is most of the heavy lifting by introducing L1SAP between the sysmobts l1 and the common code. Hasn't been modified much.
* 201509-trx-rebase: osmo-bts-trx support that I intend to merge This is what I can merge without too much review, as it does not touch the common code, except some minor l1sap fixes, and * the AMR multirate change, requiring a matching patch on openbsc.git * bts_model_abis_close() which should be fine * ability to configue multiple TRX from VTY
* 201509-fairwaves-rebase Stuff I cannot merge in its current form * the abis/e1inp multi-trx and reconnect changes require in-depth review and testing. Part of it is marked as HACK even by Andreas. * a so-called 'fix for use after free' that is actually a patch that introduces another copy for every primitive and is only required for the loopback mode * copying gsm_data_common.[ch] into the repo for debian packaging [let's not discuss further work on this before l1sap and trx-rebase are merged]
I recommend everyone with a stake in this to review or test the code in 201509-l1sap and 201509-trx-rebase now. My schedule is to merge at some point next week, unless someone raises some serious objections.
What needs to be done after the merge is to unify the src/common/power_control.c code from Holger originating in master with src/common/loops.c from Andreas originating from the osmo-trx branch.
There is no doubt that the power control and timing advance loops should be part of common, as this is required in virtually any BTS hardware.
However, we can safely merge/unify this after the merging of the above branches.
Regards, Harald
Dear all,
On Sun, Sep 06, 2015 at 03:09:09PM +0200, Harald Welte wrote:
after my futile attempt in September last year to finally merge l1sap (see the 201409-l1sap branch), I'm making one final attempt.
201509-l1sap and 201509-trx-rebase have just been merged to master.
Whatever fall-out there may be, we will need to fix it incrementally.
Please note that you also need a current openbsc.git master to make osmo-bts master compile due to the modified AMR related structures in shared header files.
Regards, Harald
That's great. I believe incremental fixes work best. We'll do more testing and submit patches.
Please excuse typos. Written with a touchscreen keyboard.
-- Regards, Alexander Chemeris CEO Fairwaves, Inc. https://fairwaves.co On Sep 22, 2015 17:47, "Harald Welte" laforge@gnumonks.org wrote:
Dear all,
On Sun, Sep 06, 2015 at 03:09:09PM +0200, Harald Welte wrote:
after my futile attempt in September last year to finally merge l1sap (see the 201409-l1sap branch), I'm making one final attempt.
201509-l1sap and 201509-trx-rebase have just been merged to master.
Whatever fall-out there may be, we will need to fix it incrementally.
Please note that you also need a current openbsc.git master to make osmo-bts master compile due to the modified AMR related structures in shared header files.
Regards, Harald --
- Harald Welte laforge@gnumonks.org
============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)
-
Alexander Chemeris -
Harald Welte -
Holger Freyther -
Sipos Csaba