March 2023 - gerrit-log - lists.osmocom.org

[M] Change in osmocom-bb[master]: layer23: get rid of lapdm_channel_init() warning
by pespin 13 Mar '23

13 Mar '23

Attention is currently required from: msuraev. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/31664 ) Change subject: layer23: get rid of lapdm_channel_init() warning ...................................................................... Patch Set 4: (1 comment) File src/host/layer23/src/mobile/app_mobile.c: https://gerrit.osmocom.org/c/osmocom-bb/+/31664/comment/612712c2_8613842f PS4, Line 336: ms = osmocom_ms_alloc2(l23_ctx, t200_ms_dcch, t200_ms_acch, GSM_LCHAN_SDCCH, name); This API can be used here. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/31664 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: If3be53a9f20e0f9e7cfb9ccd04ed54a3dcde44b8 Gerrit-Change-Number: 31664 Gerrit-PatchSet: 4 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Mar 2023 15:11:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: GSMTAP: allow configuring src IP for log messages
by pespin 13 Mar '23

13 Mar '23

Attention is currently required from: osmith, neels, fixeria, msuraev. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31513 ) Change subject: GSMTAP: allow configuring src IP for log messages ...................................................................... Patch Set 13: (1 comment) File utils/osmo-stat-dummy/osmo-stat-dummy.cfg: https://gerrit.osmocom.org/c/libosmocore/+/31513/comment/c53bb4df_ab384589 PS4, Line 1: log gsmtap 127.0.0.69 127.6.9.1 > 1 - yes, 127.0.0.1/8 is an IPv4 address? […] Yes. The purpose of that config file is to have an initial file with best defaults people who want to run that program can use to run it. And the best default there is to let the kernel decide based on routing tables configured in the system. It's purpose is not "illustrating use" of some random feature you are adding. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31513 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I9000269ce5b3dce1e757271b7c42e77b68d38f25 Gerrit-Change-Number: 31513 Gerrit-PatchSet: 13 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Mar 2023 15:10:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: GSMTAP: allow configuring src IP for log messages
by pespin 13 Mar '23

13 Mar '23

Attention is currently required from: osmith, neels, fixeria, msuraev. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31513 ) Change subject: GSMTAP: allow configuring src IP for log messages ...................................................................... Patch Set 13: (1 comment) File src/core/logging_gsmtap.c: https://gerrit.osmocom.org/c/libosmocore/+/31513/comment/78af4744_d4da2d91 PS12, Line 186: target->tgt_gsmtap.src_addr = talloc_strdup(target, src); > Exactly the same reasoning can be applied to hostname and ident - nothing is changed in this regard […] Yes, the actionable suggestion is that you verify everything is fine since your are submitting this code. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31513 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I9000269ce5b3dce1e757271b7c42e77b68d38f25 Gerrit-Change-Number: 31513 Gerrit-PatchSet: 13 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Mar 2023 15:08:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmocom-bb[master]: layer23: get rid of lapdm_channel_init() warning
by msuraev 13 Mar '23

13 Mar '23

Attention is currently required from: pespin. msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/31664 ) Change subject: layer23: get rid of lapdm_channel_init() warning ...................................................................... Patch Set 4: (1 comment) File src/host/layer23/src/common/ms.c: https://gerrit.osmocom.org/c/osmocom-bb/+/31664/comment/ea6343f7_803ed68c PS2, Line 50: lapdm_channel_init3(&ms->lapdm_channel, LAPDM_MODE_MS, t200_ms_dcch, t200_ms_acch, chan_t, name); > if these are to be set to some other value then they will most probably be set by the VTY after the […] I don't see a point in adding API which we're not going to use right away. If/when we actually need to set those parameters from vty - than we could add it. Until then it looks like adding some code for the sake of it - looks pointless to me. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/31664 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: If3be53a9f20e0f9e7cfb9ccd04ed54a3dcde44b8 Gerrit-Change-Number: 31664 Gerrit-PatchSet: 4 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Mar 2023 14:57:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmocom-bb[master]: layer23: get rid of lapdm_channel_init() warning
by msuraev 13 Mar '23

13 Mar '23

Attention is currently required from: pespin. msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/31664 ) Change subject: layer23: get rid of lapdm_channel_init() warning ...................................................................... Patch Set 3: (1 comment) File src/host/layer23/include/osmocom/bb/common/ms.h: https://gerrit.osmocom.org/c/osmocom-bb/+/31664/comment/01cf4760_e45ceb01 PS2, Line 106: enum gsm_chan_t chan_t, const char *name); > using chan_t for the name of the variable is confusing, it looks like you are passing a type. […] Done -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/31664 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: If3be53a9f20e0f9e7cfb9ccd04ed54a3dcde44b8 Gerrit-Change-Number: 31664 Gerrit-PatchSet: 3 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Mar 2023 14:51:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmocom-bb[master]: layer23: get rid of lapdm_channel_init() warning
by msuraev 13 Mar '23

13 Mar '23

Attention is currently required from: msuraev. Hello Jenkins Builder, laforge, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmocom-bb/+/31664 to look at the new patch set (#4). Change subject: layer23: get rid of lapdm_channel_init() warning ...................................................................... layer23: get rid of lapdm_channel_init() warning Change-Id: If3be53a9f20e0f9e7cfb9ccd04ed54a3dcde44b8 --- M src/host/layer23/include/osmocom/bb/common/ms.h M src/host/layer23/src/common/ms.c M src/host/layer23/src/mobile/app_mobile.c 3 files changed, 33 insertions(+), 17 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/64/31664/4 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/31664 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: If3be53a9f20e0f9e7cfb9ccd04ed54a3dcde44b8 Gerrit-Change-Number: 31664 Gerrit-PatchSet: 4 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in libosmocore[master]: GSMTAP: allow configuring src IP for log messages
by msuraev 13 Mar '23

13 Mar '23

Attention is currently required from: osmith, neels, pespin, fixeria. msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31513 ) Change subject: GSMTAP: allow configuring src IP for log messages ...................................................................... Patch Set 12: (4 comments) File src/core/logging_gsmtap.c: https://gerrit.osmocom.org/c/libosmocore/+/31513/comment/3f7e0f0b_df54b0b4 PS12, Line 132: gsmtap_source_free(target->tgt_gsmtap.gsmtap_inst); > You could try comparing if new ip if same as the old one and then do nothing. […] Done https://gerrit.osmocom.org/c/libosmocore/+/31513/comment/91ba331e_575878e4 PS12, Line 139: if (ip) > if without {} and else with them. Fix. Done https://gerrit.osmocom.org/c/libosmocore/+/31513/comment/58d640f9_189e6c6a PS12, Line 186: target->tgt_gsmtap.src_addr = talloc_strdup(target, src); > I hope arget->tgt_gsmtap.src_addr is NULL here and we are not leaking. Exactly the same reasoning can be applied to hostname and ident - nothing is changed in this regard by adding one more parameter. So if there's a chance of leaking src_addr than we're also leaking hostname and ident as well. Do you have some actionable suggestion? File utils/osmo-stat-dummy/osmo-stat-dummy.cfg: https://gerrit.osmocom.org/c/libosmocore/+/31513/comment/3e7eff23_f398789e PS4, Line 1: log gsmtap 127.0.0.69 127.6.9.1 > 1- 127.0.0.1/8 always exist on linux for IPv4 (not even the case for IPv6 iirc). […] 1 - yes, 127.0.0.1/8 is an IPv4 address? 2 - sorry, I'm completely at loss as to what's your actual point I'm adding new parameter to logging config and I'm illustrating its use in a config file in the same commit. Do you have some concrete suggestion on what should be done differently? -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31513 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I9000269ce5b3dce1e757271b7c42e77b68d38f25 Gerrit-Change-Number: 31513 Gerrit-PatchSet: 12 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Mar 2023 14:43:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: GSMTAP: allow configuring src IP for log messages
by msuraev 13 Mar '23

13 Mar '23

Attention is currently required from: osmith, neels, fixeria, msuraev. Hello osmith, Jenkins Builder, laforge, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/31513 to look at the new patch set (#13). Change subject: GSMTAP: allow configuring src IP for log messages ...................................................................... GSMTAP: allow configuring src IP for log messages Change-Id: I9000269ce5b3dce1e757271b7c42e77b68d38f25 --- M include/osmocom/core/logging.h M src/core/libosmocore.map M src/core/logging_gsmtap.c M src/vty/logging_vty.c M tests/logging/logging_vty_test.vty M utils/osmo-stat-dummy/osmo-stat-dummy.cfg 6 files changed, 138 insertions(+), 31 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/13/31513/13 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31513 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I9000269ce5b3dce1e757271b7c42e77b68d38f25 Gerrit-Change-Number: 31513 Gerrit-PatchSet: 13 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmocom-bb[master]: layer23: fix parsing of command line options
by msuraev 13 Mar '23

13 Mar '23

Attention is currently required from: laforge, fixeria. msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/31863 ) Change subject: layer23: fix parsing of command line options ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/31863 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I77ca11c14561fa3fcb9add60ccea5b0b847a20c4 Gerrit-Change-Number: 31863 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Mar 2023 13:47:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-bsc[master]: Move bsc_conn_by_bsub() and make it static
by pespin 13 Mar '23

13 Mar '23

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bsc/+/31867 to look at the new patch set (#2). Change subject: Move bsc_conn_by_bsub() and make it static ...................................................................... Move bsc_conn_by_bsub() and make it static The function was currently in osmo_bsc_sigtran.c but was never used there, and it really doesn't have any relation to that file. Let's move it to the only place where it's used so far, and mark it as static. Change-Id: I8a8cef45aa378421e0ac328d3b29b9d194698a55 --- M include/osmocom/bsc/osmo_bsc_sigtran.h M src/osmo-bsc/gsm_08_08.c M src/osmo-bsc/osmo_bsc_sigtran.c 3 files changed, 27 insertions(+), 14 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/67/31867/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31867 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I8a8cef45aa378421e0ac328d3b29b9d194698a55 Gerrit-Change-Number: 31867 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

1 0