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
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
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
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
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
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
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
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