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/7996a975_08dc03f7
PS12, Line 186: target->tgt_gsmtap.src_addr = talloc_strdup(target, src);
> I've manually checked and I've also submitted the changes to facilitate this check as a separate pat […]
Fine if you checked manually then.
"No idea what's wrong with the grep command you've posted" well, this code here relates to logging through gsmtap, and the test you modified in the next patch seems to be doing nothing with gsmtap logging?
--
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 16:38:46 +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: 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 13:
(1 comment)
File src/core/logging_gsmtap.c:
https://gerrit.osmocom.org/c/libosmocore/+/31513/comment/ceb21a93_c62bbce5
PS12, Line 186: target->tgt_gsmtap.src_addr = talloc_strdup(target, src);
> But afaiu the test you are extending there doesn't seem to be using gsmtap at all? […]
I've manually checked and I've also submitted the changes to facilitate this check as a separate patch. No idea what's wrong with the grep command you've posted - you can manually verify it by opening corresponding file from Files tab in web UI. It's supposed to be part of the review process anyway.
--
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: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 13 Mar 2023 16:36:35 +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/libosmocore/+/31868 )
Change subject: logging: print talloc report on exit from vty test
......................................................................
Patch Set 3:
(1 comment)
File tests/logging/logging_vty_test.c:
https://gerrit.osmocom.org/c/libosmocore/+/31868/comment/604722d5_8a043d88
PS3, Line 288: talloc_report(tall_vty_ctx, stderr);
> So I assumed since you didn't change the .ok and . […]
There's neither .err nor .ok associated with the file I've changed - the corresponding test is ran as part of external tests, the report is printed to stderr and ignored as expected. It's also printed just fine during manual tests. So, nope - nothing weird :)
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31868
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I5739bceb90d36164fd4cbf21242bbe26bd1e7075
Gerrit-Change-Number: 31868
Gerrit-PatchSet: 3
Gerrit-Owner: msuraev <msuraev(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 13 Mar 2023 16:32:11 +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: 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/5bd0a965_df689775
PS12, Line 186: target->tgt_gsmtap.src_addr = talloc_strdup(target, src);
> See - https://gerrit.osmocom. […]
But afaiu the test you are extending there doesn't seem to be using gsmtap at all?
$ grep -rIc gsmtap libosmocore/tests/logging
libosmocore/tests/logging/logging_test.ok:0
libosmocore/tests/logging/logging_test.c:0
libosmocore/tests/logging/logging_test.err:0
libosmocore/tests/logging/logging_vty_test.c:0
libosmocore/tests/logging/logging_vty_test.vty:0
In any case, I was basically asking if you can look that you checked and made sure the pointer was not allocated already in any of the functions called before it being assigned here.
--
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 16:17:09 +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: msuraev.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31868 )
Change subject: logging: print talloc report on exit from vty test
......................................................................
Patch Set 3:
(1 comment)
File tests/logging/logging_vty_test.c:
https://gerrit.osmocom.org/c/libosmocore/+/31868/comment/1abe3805_6827de56
PS3, Line 288: talloc_report(tall_vty_ctx, stderr);
So I assumed since you didn't change the .ok and .err file, that nothing is printed and hence nothing is allocated? Sounds weird doesn't it? :/
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31868
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I5739bceb90d36164fd4cbf21242bbe26bd1e7075
Gerrit-Change-Number: 31868
Gerrit-PatchSet: 3
Gerrit-Owner: msuraev <msuraev(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: msuraev <msuraev(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 13 Mar 2023 16:13:21 +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/27980e14_4b714106
PS4, Line 1: log gsmtap 127.0.0.69 127.6.9.1
> The purpose is described in the README - it's to test statsd and rate counters. […]
"Having any kind of logging setup does not affect it in any way."
That's precisely my point. No need in having this setting in there. It does not improve the default config in any way. In my opinion it worses it, as per what users would expect by default when a socket is created.
--
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 16:10:06 +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