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

[L] Change in libosmo-netif[master]: stream tests: Eliminate timestamps from output
by pespin 01 Sep '23

01 Sep '23

Attention is currently required from: arehbein, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/34224 ) Change subject: stream tests: Eliminate timestamps from output ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > 1- I still didn't get to look at this issue and test this patch myself, something I wanted to avoid […] I submitted the srv_destroy fix from daniel here for review (changed order of calls bit): https://gerrit.osmocom.org/c/libosmo-netif/+/34289 stream_test: Avoid leaking osmo_stream_srv_link Then, I applied https://gerrit.osmocom.org/c/libosmo-netif/+/33198/19 on top of this other patch I just mentioned above, run it locally and I don't see the timestamps changing. So, arrived to this point: Please if you need any specific help with something specific, just tell me what can I help with. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/34224 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I7faed932927d4f6e328a28c7f30a647a7272e89c Gerrit-Change-Number: 34224 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Fri, 01 Sep 2023 17:04:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in libosmo-netif[master]: stream_test: Avoid leaking osmo_stream_srv_link
by pespin 01 Sep '23

01 Sep '23

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/34289 ) Change subject: stream_test: Avoid leaking osmo_stream_srv_link ...................................................................... stream_test: Avoid leaking osmo_stream_srv_link Change-Id: I43b7327893c220447492d2c36c87b187a414cbdb --- M tests/stream/stream_test.c 1 file changed, 11 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/89/34289/1 diff --git a/tests/stream/stream_test.c b/tests/stream/stream_test.c index 20afc55..026070c 100644 --- a/tests/stream/stream_test.c +++ b/tests/stream/stream_test.c @@ -679,5 +679,7 @@ test_segm_ipa_stream_srv_run(tall_test, host, port, srv); printf("Stream tests completed\n"); + + osmo_stream_srv_link_destroy(srv); return EXIT_SUCCESS; } -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/34289 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I43b7327893c220447492d2c36c87b187a414cbdb Gerrit-Change-Number: 34289 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[M] Change in osmo-bsc[master]: Includes: Remove enum/include from libosmocore instead
by pespin 01 Sep '23

01 Sep '23

Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31304 ) Change subject: Includes: Remove enum/include from libosmocore instead ...................................................................... Patch Set 7: (1 comment) File include/osmocom/bsc/gsm_data.h: https://gerrit.osmocom.org/c/osmo-bsc/+/31304/comment/fe3a8687_384cc4ad PS6, Line 657: uint16_t parameter[_NUM_NM_RLC_PAR_OFFSET]; > I added the suffix, because this makes clear (self-documenting code and such) that their values aren […] This all needs to be reverted. As explained in the libocmocore patch, this is all app-specific stuff and makes no sense trying to share it. In fact, it probably makes sense to drop the array alltogether and use a struct with proper fields instead. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31304 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I2883aee12b501a717d5acecd5638882724336e72 Gerrit-Change-Number: 31304 Gerrit-PatchSet: 7 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Comment-Date: Fri, 01 Sep 2023 16:52:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: gm_12_21.h: Add cross-component parameter defaults (NSE timing)
by pespin 01 Sep '23

01 Sep '23

Attention is currently required from: arehbein, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31877 ) Change subject: gm_12_21.h: Add cross-component parameter defaults (NSE timing) ...................................................................... Patch Set 2: (1 comment) File include/osmocom/gsm/protocol/gsm_12_21.h: https://gerrit.osmocom.org/c/libosmocore/+/31877/comment/c09f18ec_c9981c29 PS2, Line 294: enum abis_nm_par_defaults { > What's the drawback of using an enum here? I already explained in the comment previous to the one where you ask it. I'll extend: This header contains information about Abis protocol and implementation, from TS 12.21. All these offsets that you are defining here, are not specified in TS 12.21 nor have much to do with it. If you look at where are these offsets are used, (eg, take rlc_cfg_default) you will see that those are only used to *internally* store values inside each app process, and that this "array" is never transmitted over the wire. So it's only a given implementation decision of several apps to store the different values that way. Actually, imho, those apps would need to be changed to drop those RLC_T3142* enums they have and simply have an struct with the different fields. It would be clearer for everyone, and would simplify the code. What you could add in a related library include, is a packed struct send over the wire containing the value of NM_ATT_IPACC_RLC_CFG. You can find its contents in nanobts_gen_set_cell_attr() in osmo-bsc.git: """ /* all timers in seconds, unless otherwise stated */ buf[0] = 20; /* T3142 */ buf[1] = 5; /* T3169 */ buf[2] = 5; /* T3191 */ buf[3] = 160; /* T3193 (units of 10ms) */ buf[4] = 5; /* T3195 */ buf[5] = 10; /* N3101 */ buf[6] = 4; /* N3103 */ buf[7] = 8; /* N3105 */ buf[8] = 15; /* RLC CV countdown */ msgb_tl16v_put(msgb, NM_ATT_IPACC_RLC_CFG, 9, buf); """ But again, this is sorta out to the scope in OS#5335, which is about to add "Add VTY commands to configure T3.. and N3.. timers counters for gprs" So in summary, don't attempt to move timer/counter specific context out of each app into a lib, it won't be the good way to work in this case. If at all, we can look if it may make sense later, but first let's clean up everything on each app. > Should we add a centralised gsm_data.h to libosmocore that keeps shared values? As a conculision from the text I wrote above: NO. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31877 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I51d1fd8b596523ae2ac8fb6a186ce7a702334c27 Gerrit-Change-Number: 31877 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Fri, 01 Sep 2023 16:49:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-ttcn3-hacks[master]: hlr: implement str2apn() to convert an apn as string into octetstring
by pespin 01 Sep '23

01 Sep '23

Attention is currently required from: lynxis lazus. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34288 ) Change subject: hlr: implement str2apn() to convert an apn as string into octetstring ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34288 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I607969cd58110d4d5ff1b828e64cf2b5031868ac Gerrit-Change-Number: 34288 Gerrit-PatchSet: 2 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Fri, 01 Sep 2023 16:17:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-ttcn3-hacks[master]: hlr: implement str2apn() to convert an apn as string into octetstring
by lynxis lazus 01 Sep '23

01 Sep '23

Attention is currently required from: lynxis lazus. Hello Jenkins Builder, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34288 to look at the new patch set (#2). Change subject: hlr: implement str2apn() to convert an apn as string into octetstring ...................................................................... hlr: implement str2apn() to convert an apn as string into octetstring APN are encoded by splitting each domain part by the dot and prefix each element by a 8bit length. E.g. internet -> \x08internet and internet.foo -> \x08internet\x03foo Change-Id: I607969cd58110d4d5ff1b828e64cf2b5031868ac --- M hlr/HLR_Tests.ttcn 1 file changed, 72 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/88/34288/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34288 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I607969cd58110d4d5ff1b828e64cf2b5031868ac Gerrit-Change-Number: 34288 Gerrit-PatchSet: 2 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-MessageType: newpatchset

1 0

[M] Change in osmo-ttcn3-hacks[master]: HLR_Tests: add testcase for multiple APNs in subscriber data
by pespin 01 Sep '23

01 Sep '23

Attention is currently required from: laforge, lynxis lazus. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32510 ) Change subject: HLR_Tests: add testcase for multiple APNs in subscriber data ...................................................................... Patch Set 7: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32510 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I8d0c08272bc239370e800d6014ab9c68087b8989 Gerrit-Change-Number: 32510 Gerrit-PatchSet: 7 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Fri, 01 Sep 2023 16:14:21 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-ttcn3-hacks[master]: hlr: implement str2apn() to convert an apn as string into octetstring
by pespin 01 Sep '23

01 Sep '23

Attention is currently required from: lynxis lazus. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34288 ) Change subject: hlr: implement str2apn() to convert an apn as string into octetstring ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34288 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I607969cd58110d4d5ff1b828e64cf2b5031868ac Gerrit-Change-Number: 34288 Gerrit-PatchSet: 1 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Fri, 01 Sep 2023 16:14:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-ttcn3-hacks[master]: HLR_Tests: add testcase for multiple APNs in subscriber data
by lynxis lazus 01 Sep '23

01 Sep '23

Attention is currently required from: laforge, pespin. lynxis lazus has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32510 ) Change subject: HLR_Tests: add testcase for multiple APNs in subscriber data ...................................................................... Patch Set 7: (1 comment) File hlr/HLR_Tests.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32510/comment/73131189_088d… PS6, Line 1037: tr_GSUP_IE_APN('08'O & char2oct("internet")) > lynxis, can you please get that function implemented? It shouldn't be too hard to iterate over the d […] Implemented by the previous change. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32510 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I8d0c08272bc239370e800d6014ab9c68087b8989 Gerrit-Change-Number: 32510 Gerrit-PatchSet: 7 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 01 Sep 2023 16:13:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: lynxis lazus <lynxis(a)fe80.eu> Gerrit-MessageType: comment

1 0

[M] Change in osmo-ttcn3-hacks[master]: hlr: implement str2apn() to convert an apn as string into octetstring
by Jenkins Builder 01 Sep '23

01 Sep '23

Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34288 ) Change subject: hlr: implement str2apn() to convert an apn as string into octetstring ...................................................................... Patch Set 1: (1 comment) File hlr/HLR_Tests.ttcn: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-10824): https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34288/comment/1413a640_2a44… PS1, Line 1968: setverdict(fail, "Values missmatch"); 'missmatch' may be misspelled - perhaps 'mismatch'? -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34288 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I607969cd58110d4d5ff1b828e64cf2b5031868ac Gerrit-Change-Number: 34288 Gerrit-PatchSet: 1 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Fri, 01 Sep 2023 16:13:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0