Attention is currently required from: fixeria.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/31354 )
Change subject: layer23: Fix cmdline args not applied
......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmocom-bb/+/31354/comment/fa22e9e0_8660ea24
PS2, Line 10: chicken-and-egg problem
> It's weird that we ended up having this problem. The usual approach would be: […]
It's as weird as everything else due to this layer23 app concept which adds another layer of indirection and which I'm really hating after having spent so much time in it 😄
The point of having an MS already existing when parsing cmdline opts is the same as for the VTY: it allows setting stuff on the object directly.
The main problem here is that there's a init() function, which I'd like to get called *before* any other call to the layer23app API, so that the app has time to initialize in runtime whatever it wishes. That also includes l23_app_info() which is used during cmd line parsing.
If you move cmdline parsing before l23_app_init(), you call l23_app_info() before l23_app_init() which looks wrong to me. But you still need to allocate the MS in some apps at l23_app_init() time because the MS object needs to be available during VTY config parsing.
So as I was saying, chicken-and-egg problem.
In here, we either:
* [A] Allow layer23 calling apps' l23_app_info() *before* a call to l23_app_init(), which then allows calling handle_options() *before* l23_app_init().
* [B] calling handle_options() *after* l23_app_init(), which means we need to apply some cmdline options to already created objects, VTY style (what I submitted in this patch)
In any case imho those command lines should be deprecated at some point, and VTY commands be used instead for all apps now that we support VTY in the layer23 common code.
I think you seem to prefer option [A], while I prefer option [B].
File src/host/layer23/src/common/main.c:
https://gerrit.osmocom.org/c/osmocom-bb/+/31354/comment/c6c2a2d2_d11c0965
PS2, Line 160: llist_for_each_entry
> At the moment only one MS can be using the L1 PHY (accessed via the layer2_socket_path), so it makes […]
It's up to the app to allocate as many structs as it wants. This is not starting the layer2.
https://gerrit.osmocom.org/c/osmocom-bb/+/31354/comment/77aaa73c_5d63622d
PS2, Line 165: llist_for_each_entry
> Same here. A single SAP socket cannot be used by several MS simultaneously. […]
Same as above.
--
To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/31354
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Change-Id: I285a4908401659b69dac513dd1a4b07facb88c51
Gerrit-Change-Number: 31354
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 17 Feb 2023 15:59:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: laforge.
Hello Jenkins Builder, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/31357
to look at the new patch set (#3).
Change subject: libosmogb.pc.in: Fix missing dependency on libosmogsm
......................................................................
libosmogb.pc.in: Fix missing dependency on libosmogsm
Change-Id: I5efa04d1c5cabc65b42de624b26fdbf9ebe746b1
---
M libosmogb.pc.in
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/57/31357/3
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31357
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I5efa04d1c5cabc65b42de624b26fdbf9ebe746b1
Gerrit-Change-Number: 31357
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/31354 )
Change subject: layer23: Fix cmdline args not applied
......................................................................
Patch Set 2: Code-Review-1
(3 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmocom-bb/+/31354/comment/9e6ae9b0_434f4b19
PS2, Line 10: chicken-and-egg problem
It's weird that we ended up having this problem. The usual approach would be:
* parse command line options and store params to struct app_data,
* allocate one MS with params from struct app_data,
* parse the given VTY config file,
* apply parameters from the config file,
* allocate additional MS if needed,
I am not saying this approach is perfect, but the fact that MS already exists when parsing command line options somehow feels wrong. Moreover, applying identical values to all MS is meaningless in most cases (see comments).
File src/host/layer23/src/common/main.c:
https://gerrit.osmocom.org/c/osmocom-bb/+/31354/comment/6809aea9_471ff526
PS2, Line 160: llist_for_each_entry
At the moment only one MS can be using the L1 PHY (accessed via the layer2_socket_path), so it makes a little sense to assign all MS the same socket path. Additional MS instances may only be allocated via the VTY, where you have per-MS socket path.
https://gerrit.osmocom.org/c/osmocom-bb/+/31354/comment/9d2c5f95_ae35eb8e
PS2, Line 165: llist_for_each_entry
Same here. A single SAP socket cannot be used by several MS simultaneously.
We have per-MS SAP socket path in the VTY.
--
To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/31354
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Change-Id: I285a4908401659b69dac513dd1a4b07facb88c51
Gerrit-Change-Number: 31354
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 17 Feb 2023 14:42:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/31351 )
Change subject: rlcmac: Handle SI13 from L1CTL
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
This patch depends on libosmocore https://gerrit.osmocom.org/c/libosmocore/+/31356 in order to have rpm builds fixed in libosmocore.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/31351
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: Ifaddf0e6e5cad7ea25672804c83c254579874813
Gerrit-Change-Number: 31351
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Comment-Date: Fri, 17 Feb 2023 14:38:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: pespin.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmo-gprs/+/31352
to look at the new patch set (#2).
Change subject: WIP: rlcmac: Implement N3104
......................................................................
WIP: rlcmac: Implement N3104
TODO: Add unit test similar to test_ul_tbf_t3166_timeout but without
advancing wall clock, sending lots of RTS
Change-Id: Ia8c35aad7a537ab76447187847f8cee8c379352c
---
M include/osmocom/gprs/rlcmac/tbf_ul.h
M include/osmocom/gprs/rlcmac/tbf_ul_fsm.h
M src/rlcmac/tbf_ul.c
M src/rlcmac/tbf_ul_fsm.c
4 files changed, 29 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-gprs refs/changes/52/31352/2
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/31352
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: Ia8c35aad7a537ab76447187847f8cee8c379352c
Gerrit-Change-Number: 31352
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: pespin.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmo-gprs/+/31351
to look at the new patch set (#2).
Change subject: rlcmac: Handle SI13 from L1CTL
......................................................................
rlcmac: Handle SI13 from L1CTL
Change-Id: Ifaddf0e6e5cad7ea25672804c83c254579874813
---
M include/osmocom/gprs/rlcmac/rlcmac_private.h
M src/rlcmac/rlcmac.c
M src/rlcmac/rlcmac_prim.c
3 files changed, 35 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-gprs refs/changes/51/31351/2
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/31351
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: Ifaddf0e6e5cad7ea25672804c83c254579874813
Gerrit-Change-Number: 31351
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset