- gerrit-log - lists.osmocom.org

[M] Change in libosmocore[master]: After writing VTY config, run sync() in a different thread
by fixeria 12 Aug '24

12 Aug '24

Attention is currently required from: jolly. fixeria has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/37798?usp=email ) Change subject: After writing VTY config, run sync() in a different thread ...................................................................... Patch Set 2: (3 comments) Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/37798/comment/aa99c0c4_6eec96d0?… : PS2, Line 10: results typo: result (without 's') https://gerrit.osmocom.org/c/libosmocore/+/37798/comment/f4d2fcb6_3dcf4f8f?… : PS2, Line 11: delays delayed processing causes ... and s/delays/timeouts/? not critical, just to avoid "delay causes delay" ;) https://gerrit.osmocom.org/c/libosmocore/+/37798/comment/39011a8b_9c88fbca?… : PS2, Line 16: The main process will then output the completion : message on the VTY. This is no longer true, we decided to log an INFO instead. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/37798?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3cb2ee68b2e4c730f96522208c4abf00d0f49a44 Gerrit-Change-Number: 37798 Gerrit-PatchSet: 2 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Mon, 12 Aug 2024 17:16:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in ...osmo-python-tests[master]: osmotestconfig: fix return value of write_config()
by fixeria 12 Aug '24

12 Aug '24

Attention is currently required from: jolly, osmith. fixeria has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/python/osmo-python-tests/+/37799?usp=email ) Change subject: osmotestconfig: fix return value of write_config() ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File scripts/osmotestconfig.py: https://gerrit.osmocom.org/c/python/osmo-python-tests/+/37799/comment/9a01e… : PS1, Line 33: if test_config_atest(app_desc, newconfig, write_config, verbose) > 0: For reviewers: the only caller is here. As can be seen, it expects an integer (not tuple). -- To view, visit https://gerrit.osmocom.org/c/python/osmo-python-tests/+/37799?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: python/osmo-python-tests Gerrit-Branch: master Gerrit-Change-Id: I24adfd344937bab1ab641327e59a25bf76e18591 Gerrit-Change-Number: 37799 Gerrit-PatchSet: 1 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Mon, 12 Aug 2024 17:12:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[L] Change in libosmocore[master]: tests: add unit test for osmo_hr_sid_classify()
by falconia 12 Aug '24

12 Aug '24

falconia has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/37797?usp=email ) Change subject: tests: add unit test for osmo_hr_sid_classify() ...................................................................... tests: add unit test for osmo_hr_sid_classify() The bit patterns used to construct this unit test originate from an experiment involving a Calypso GSM MS and a CMU200 test instrument configured to simulate radio conditions ranging from good to bad, as documented here: https://osmocom.org/projects/retro-gsm/wiki/HRv1_error_flags The resulting unit test proves correctness of osmo_hr_sid_classify() implementation not only "against itself" and against theoretical understanding of ETSI reference logic, but also by confirming a match between our computed classification and that produced by TI's DSP. Related: OS#6036 Change-Id: I9944bb7d49b6fe004d4bfc7a3f70cfdb03d62614 --- M tests/Makefile.am A tests/codec/codec_hr_sid_test.c A tests/codec/codec_hr_sid_test.in A tests/codec/codec_hr_sid_test.ok M tests/testsuite.at 5 files changed, 259 insertions(+), 1 deletion(-) Approvals: laforge: Looks good to me, but someone else must approve Jenkins Builder: Verified fixeria: Looks good to me, approved diff --git a/tests/Makefile.am b/tests/Makefile.am index 1e8997c..d458464 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -26,7 +26,8 @@ abis/abis_test endian/endian_test sercomm/sercomm_test \ prbs/prbs_test gsm23003/gsm23003_test \ gsm23236/gsm23236_test \ - codec/codec_ecu_fr_test timer/clk_override_test \ + codec/codec_ecu_fr_test codec/codec_hr_sid_test \ + timer/clk_override_test \ oap/oap_client_test gsm29205/gsm29205_test \ logging/logging_vty_test \ vty/vty_transcript_test \ @@ -265,6 +266,9 @@ codec_codec_ecu_fr_test_SOURCES = codec/codec_ecu_fr_test.c codec_codec_ecu_fr_test_LDADD = $(top_builddir)/src/codec/libosmocodec.la $(LDADD) +codec_codec_hr_sid_test_SOURCES = codec/codec_hr_sid_test.c +codec_codec_hr_sid_test_LDADD = $(top_builddir)/src/codec/libosmocodec.la $(LDADD) + loggingrb_loggingrb_test_SOURCES = loggingrb/loggingrb_test.c loggingrb_loggingrb_test_LDADD = $(LDADD) @@ -438,6 +442,7 @@ loggingrb/logging_test.err strrb/strrb_test.ok \ codec/codec_test.ok \ codec/codec_ecu_fr_test.ok \ + codec/codec_hr_sid_test.ok codec/codec_hr_sid_test.in \ vty/vty_test.ok vty/vty_test.err \ vty/fail_not_de-indented.cfg \ vty/fail_tabs_and_spaces.cfg \ @@ -598,6 +603,8 @@ >$(srcdir)/codec/codec_test.ok codec/codec_ecu_fr_test \ >$(srcdir)/codec/codec_ecu_fr_test.ok + codec/codec_hr_sid_test $(srcdir)/codec/codec_hr_sid_test.in \ + >$(srcdir)/codec/codec_hr_sid_test.ok if ENABLE_GB fr/fr_test \ >$(srcdir)/fr/fr_test.ok diff --git a/tests/codec/codec_hr_sid_test.c b/tests/codec/codec_hr_sid_test.c new file mode 100644 index 0000000..ed4aa9b --- /dev/null +++ b/tests/codec/codec_hr_sid_test.c @@ -0,0 +1,144 @@ +/* + * This program is a test for osmo_hr_sid_classify(). It reads a set of + * TCH/HS Rx bit patterns in TI DSP format (originally captured from a + * Calypso MS under conditions of induced radio errors), converts each + * bit pattern to TS 101 318 format (using same bit reordering function + * as libosmocoding gsm0503 implementation), and feeds each test line + * to osmo_hr_sid_classify(). It then prints the output next to each input. + * + * Author: Mychaela N. Falconia <falcon(a)freecalypso.org>, 2024 - however, + * Mother Mychaela's contributions are NOT subject to copyright. + * No rights reserved, all rights relinquished. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <ctype.h> +#include <stdio.h> +#include <stdint.h> +#include <stdbool.h> +#include <stdlib.h> +#include <string.h> + +#include <osmocom/core/bits.h> +#include <osmocom/core/utils.h> +#include <osmocom/codec/codec.h> + +#define HR_CODEC_BITS (GSM_HR_BYTES * 8) +#define HR_BYTES_TIDSP (GSM_HR_BYTES + 1) + +/* re-arrange according to TS 05.03 Table 3a (receiver) */ +/* function copied from src/coding/gsm0503_coding.c */ +static void tch_hr_d_to_b(ubit_t *b_bits, const ubit_t *d_bits) +{ + int i; + + const uint16_t *map; + + if (!d_bits[93] && !d_bits[94]) + map = gsm620_unvoiced_bitorder; + else + map = gsm620_voiced_bitorder; + + for (i = 0; i < 112; i++) + b_bits[map[i]] = d_bits[i]; +} + +static void process_record(const char *hex_str, bool bci_flag) +{ + uint8_t dsp_rx_bytes[HR_BYTES_TIDSP]; + ubit_t bits_transmission_order[HR_BYTES_TIDSP * 8]; + ubit_t bits_codec_order[HR_CODEC_BITS]; + uint8_t hr_bytes_ts101318[GSM_HR_BYTES]; + bool bfi_flag = false; + enum osmo_gsm631_sid_class sidc; + + osmo_hexparse(hex_str, dsp_rx_bytes, HR_BYTES_TIDSP); + osmo_pbit2ubit(bits_transmission_order, dsp_rx_bytes, + HR_BYTES_TIDSP * 8); + /* TI DSP format has a gap of 4 bits between class 1 and class 2 + * portions - get rid of it. 95 is the number of class 1 bits, + * 17 is the number of class 2 bits. */ + memmove(bits_transmission_order + 95, + bits_transmission_order + 95 + 4, 17); + tch_hr_d_to_b(bits_codec_order, bits_transmission_order); + osmo_ubit2pbit(hr_bytes_ts101318, bits_codec_order, HR_CODEC_BITS); + + sidc = osmo_hr_sid_classify(hr_bytes_ts101318, bci_flag, &bfi_flag); + printf("%s %d ==> %d %d\n", hex_str, (int) bci_flag, + (int) sidc, (int) bfi_flag); +} + +static void process_line(char *linebuf, const char *infname, int lineno) +{ + char *cp = linebuf, *hex_str; + int ndig; + bool bci_flag; + + while (isspace(*cp)) + cp++; + if (*cp == '\0' || *cp == '#') + return; + /* expect string of 30 hex digits */ + hex_str = cp; + for (ndig = 0; ndig < HR_BYTES_TIDSP * 2; ndig++) { + if (!isxdigit(*cp)) + goto inv; + cp++; + } + if (!isspace(*cp)) + goto inv; + *cp++ = '\0'; + while (isspace(*cp)) + cp++; + /* 0 or 1 must follow, giving BCI flag */ + if (*cp == '0') + bci_flag = false; + else if (*cp == '1') + bci_flag = true; + else + goto inv; + cp++; + /* must be end of non-comment line */ + while (isspace(*cp)) + cp++; + if (*cp != '\0' && *cp != '#') + goto inv; + + process_record(hex_str, bci_flag); + return; + +inv: fprintf(stderr, "%s line %d: invalid syntax\n", infname, lineno); + exit(1); +} + +int main(int argc, char **argv) +{ + const char *infname; + FILE *inf; + char linebuf[128]; + int lineno; + + if (argc != 2) { + fprintf(stderr, "usage: %s input-file\n", argv[0]); + exit(1); + } + infname = argv[1]; + inf = fopen(infname, "r"); + if (!inf) { + perror(infname); + exit(1); + } + for (lineno = 1; fgets(linebuf, sizeof(linebuf), inf); lineno++) + process_line(linebuf, infname, lineno); + fclose(inf); + exit(0); +} diff --git a/tests/codec/codec_hr_sid_test.in b/tests/codec/codec_hr_sid_test.in new file mode 100644 index 0000000..e8ad966 --- /dev/null +++ b/tests/codec/codec_hr_sid_test.in @@ -0,0 +1,61 @@ +# This file is input for the HR SID classifier test program. All TCH/HS +# Rx bit strings contained in this file have been taken from hr-test1.dl +# or hr-test2.dl, attached to this wiki page: +# +# https://osmocom.org/projects/retro-gsm/wiki/HRv1_error_flags +# +# Each Rx bit string (15 hex bytes) is from TI DSP; the 0 or 1 that follows +# is the BCI flag to be fed to the SID classifier. The original DSP status +# word is noted in the comments; running the unit test program will show +# whether or not our SID classification agrees with that produced by +# TI Calypso DSP. + +# perfect, error-free SID: hr-test2.dl line 171, C010 +FFFFFFFFFFFFFFFFFFFFFFFE1FFFF0 0 + +# mode bits cleared to 0, all other bits still 1s +FFFFFFFFFFFFFFFFFFFFFFF81FFFF0 0 + +# selected error lines +FFFFFFFFFFFFFFFFFFFFFFFE0FFF70 0 # hr-test2.dl line 4226, C010 +FFFFFFFFFFFFFFFFFFFFFFFE13FFF0 1 # hr-test2.dl line 4256, C012 +FFFFFFFFFFFFFFFFFFFFFFFE17FFF0 1 # hr-test2.dl line 4258, C012 +FFFD01FFFFFFFFFFFFFFFFFE0FFFF0 1 # hr-test2.dl line 4598, C00A +FFFFFFFFFFFCC296452940FE1FFEF0 1 # hr-test2.dl line 5141, C00F +FFFFFFFFFFFCC296452940FE1FFEF0 0 # same bits without BCI +FF6E76F40C276FFFFFFFFFFE0FFFF0 1 # hr-test2.dl line 5173, C003 +FF6E76F40C276FFFFFFFFFFE0FFFF0 0 # same bits without BCI +FFFFFFFFFDFFFFFFFFFFFFFE0FFFF0 1 # hr-test2.dl line 5206, C012 +FFFFFFFFFDFFFFFFFFFFFFFE0FFFF0 0 # same bits without BCI +FFFFFFFFFFD0DFFFFFFFFFFE07FEF0 1 # hr-test2.dl line 5261, C00A +FFFFFFFFFFD0DFFFFFFFFFFE07FEF0 0 # same bits without BCI +FFFFFFFFFFFF41EAC9676FFE1F7DF0 1 # hr-test2.dl line 5738, C00F +FFFFFFFFFFFF41EAC9676FFE1F7DF0 0 # same bits without BCI + +FFFFFFFFFFFF847D5B9DBFFE1937B0 1 # hr-test2.dl line 6175, C00F +FFFFFFFFFFFF847D5B9DBFFE1937B0 0 # same bits without BCI +FFFFFFFFFFFFFFFFFFFFFFD01FFEB0 1 # hr-test2.dl line 6188, C017 +FFFFFFFFFFFFFFFFFFFFFFD01FFEB0 0 # same bits without BCI +FDBD7D552CB25FFFFFFFFFFE1DFBB0 1 # hr-test2.dl line 6191, C002 +FDBD7D552CB25FFFFFFFFFFE1DFBB0 0 # same bits without BCI +FFD2F0A52B8FFFFFFFFEDE600FFDF0 1 # hr-test2.dl line 6195, C007 +FFD2F0A52B8FFFFFFFFEDE600FFDF0 0 # same bits without BCI +FFFFFFFFCC7FFFFFFFF4EE601C31D0 1 # hr-test2.dl line 6318, C007 +FFFFFFFFCC7FFFFFFFF4EE601C31D0 0 # same bits without BCI +FFFFFFFFFFFFFFFFFFFFA5901BEFD0 1 # hr-test2.dl line 6545, C00F +FFFFFFFFFFFFFFFFFFFFA5901BEFD0 0 # same bits without BCI +FFFFFFFFEA97FFFFFFE765901FFFB0 1 # hr-test2.dl line 6973, C00F +FFFFFFFFEA97FFFFFFE765901FFFB0 0 # same bits without BCI + +FFFFFFFFF8C8A5E29DA0DFFE07FFF0 1 # hr-test2.dl line 7158, C00F +FFFFFFFFF8C8A5E29DA0DFFE07FFF0 0 # same bits without BCI +FFFD01853B7206E63FFFFFFE1FBFD0 1 # hr-test2.dl line 7175, C003 +FFFD01853B7206E63FFFFFFE1FBFD0 0 # same bits without BCI +E6ACC7FFFF40FFFFFFFFFFFE1FF770 1 # hr-test2.dl line 7195, C00B +E6ACC7FFFF40FFFFFFFFFFFE1FF770 0 # same bits without BCI + +# hr-test1.dl, PRBS without major errors +55A5404BFAED58A3BE33A978092A40 0 # hr-test1.dl line 1051, C000 +CFE44B516ED5D1F54E4615AA101260 0 # hr-test1.dl line 2710, C000 +D7881D40AA0F68106195DCD41568C0 0 # hr-test1.dl line 4306, C000 +D4CFB4961F8F9F11313454560690E0 1 # hr-test1.dl line 4631, C002 diff --git a/tests/codec/codec_hr_sid_test.ok b/tests/codec/codec_hr_sid_test.ok new file mode 100644 index 0000000..3ea1fed --- /dev/null +++ b/tests/codec/codec_hr_sid_test.ok @@ -0,0 +1,40 @@ +FFFFFFFFFFFFFFFFFFFFFFFE1FFFF0 0 ==> 2 0 +FFFFFFFFFFFFFFFFFFFFFFF81FFFF0 0 ==> 1 0 +FFFFFFFFFFFFFFFFFFFFFFFE0FFF70 0 ==> 2 0 +FFFFFFFFFFFFFFFFFFFFFFFE13FFF0 1 ==> 2 0 +FFFFFFFFFFFFFFFFFFFFFFFE17FFF0 1 ==> 2 0 +FFFD01FFFFFFFFFFFFFFFFFE0FFFF0 1 ==> 1 0 +FFFFFFFFFFFCC296452940FE1FFEF0 1 ==> 1 0 +FFFFFFFFFFFCC296452940FE1FFEF0 0 ==> 0 0 +FF6E76F40C276FFFFFFFFFFE0FFFF0 1 ==> 0 1 +FF6E76F40C276FFFFFFFFFFE0FFFF0 0 ==> 0 0 +FFFFFFFFFDFFFFFFFFFFFFFE0FFFF0 1 ==> 2 0 +FFFFFFFFFDFFFFFFFFFFFFFE0FFFF0 0 ==> 2 0 +FFFFFFFFFFD0DFFFFFFFFFFE07FEF0 1 ==> 1 0 +FFFFFFFFFFD0DFFFFFFFFFFE07FEF0 0 ==> 1 0 +FFFFFFFFFFFF41EAC9676FFE1F7DF0 1 ==> 1 0 +FFFFFFFFFFFF41EAC9676FFE1F7DF0 0 ==> 0 0 +FFFFFFFFFFFF847D5B9DBFFE1937B0 1 ==> 1 0 +FFFFFFFFFFFF847D5B9DBFFE1937B0 0 ==> 0 0 +FFFFFFFFFFFFFFFFFFFFFFD01FFEB0 1 ==> 1 0 +FFFFFFFFFFFFFFFFFFFFFFD01FFEB0 0 ==> 1 0 +FDBD7D552CB25FFFFFFFFFFE1DFBB0 1 ==> 0 1 +FDBD7D552CB25FFFFFFFFFFE1DFBB0 0 ==> 0 0 +FFD2F0A52B8FFFFFFFFEDE600FFDF0 1 ==> 0 1 +FFD2F0A52B8FFFFFFFFEDE600FFDF0 0 ==> 0 0 +FFFFFFFFCC7FFFFFFFF4EE601C31D0 1 ==> 0 1 +FFFFFFFFCC7FFFFFFFF4EE601C31D0 0 ==> 0 0 +FFFFFFFFFFFFFFFFFFFFA5901BEFD0 1 ==> 1 0 +FFFFFFFFFFFFFFFFFFFFA5901BEFD0 0 ==> 1 0 +FFFFFFFFEA97FFFFFFE765901FFFB0 1 ==> 1 0 +FFFFFFFFEA97FFFFFFE765901FFFB0 0 ==> 0 0 +FFFFFFFFF8C8A5E29DA0DFFE07FFF0 1 ==> 1 0 +FFFFFFFFF8C8A5E29DA0DFFE07FFF0 0 ==> 0 0 +FFFD01853B7206E63FFFFFFE1FBFD0 1 ==> 0 1 +FFFD01853B7206E63FFFFFFE1FBFD0 0 ==> 0 0 +E6ACC7FFFF40FFFFFFFFFFFE1FF770 1 ==> 1 0 +E6ACC7FFFF40FFFFFFFFFFFE1FF770 0 ==> 0 0 +55A5404BFAED58A3BE33A978092A40 0 ==> 0 0 +CFE44B516ED5D1F54E4615AA101260 0 ==> 0 0 +D7881D40AA0F68106195DCD41568C0 0 ==> 0 0 +D4CFB4961F8F9F11313454560690E0 1 ==> 0 0 diff --git a/tests/testsuite.at b/tests/testsuite.at index 3433fb0..dbf3925 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -218,6 +218,12 @@ AT_CHECK([$abs_top_builddir/tests/codec/codec_ecu_fr_test], [0], [expout], [ignore]) AT_CLEANUP +AT_SETUP([codec_hr_sid]) +AT_KEYWORDS([codec_hr_sid]) +cat $abs_srcdir/codec/codec_hr_sid_test.ok > expout +AT_CHECK([$abs_top_builddir/tests/codec/codec_hr_sid_test $abs_srcdir/codec/codec_hr_sid_test.in], [0], [expout], [ignore]) +AT_CLEANUP + AT_SETUP([fr]) AT_KEYWORDS([fr]) cat $abs_srcdir/fr/fr_test.ok > expout -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/37797?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I9944bb7d49b6fe004d4bfc7a3f70cfdb03d62614 Gerrit-Change-Number: 37797 Gerrit-PatchSet: 2 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org>

1 0

[M] Change in libosmocore[master]: After writing VTY config, run sync() in a different thread
by fixeria 12 Aug '24

12 Aug '24

Attention is currently required from: jolly. fixeria has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/37798?usp=email ) Change subject: After writing VTY config, run sync() in a different thread ...................................................................... Patch Set 2: (2 comments) File src/vty/command.c: https://gerrit.osmocom.org/c/libosmocore/+/37798/comment/12468d57_8711ac6a?… : PS2, Line 3413: sync(); : sync_complete(&sync_fd, 0); : common path for all failing checks, maybe move it down below to the bottom and use `goto`? https://gerrit.osmocom.org/c/libosmocore/+/37798/comment/9035dec6_77e28dbe?… : PS2, Line 3439: LOGP(DLGLOBAL, LOGL_ERROR, "Failed to create thread, syncing without thread.\n"); missing `osmo_fd_unregister()` here? -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/37798?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3cb2ee68b2e4c730f96522208c4abf00d0f49a44 Gerrit-Change-Number: 37798 Gerrit-PatchSet: 2 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Mon, 12 Aug 2024 16:33:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in libosmocore[master]: After writing VTY config, run sync() in a different thread
by jolly 12 Aug '24

12 Aug '24

Attention is currently required from: jolly. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/37798?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified-1 by Jenkins Builder Change subject: After writing VTY config, run sync() in a different thread ...................................................................... After writing VTY config, run sync() in a different thread Running sync() after writing VTY config causes the process to stop for a fraction of a second. This may results in delayed processing that causes buffer underruns, overflows and delays. After writing VTY configuration to the config file, a new thread is created. This thread runs the sync() command. Once it returns, the thread will signal through a pipe to the main thread that the sync process is complete. The main process will then output the completion message on the VTY. Related: OS#6438 Change-Id: I3cb2ee68b2e4c730f96522208c4abf00d0f49a44 --- M src/vty/command.c 1 file changed, 74 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/98/37798/2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/37798?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3cb2ee68b2e4c730f96522208c4abf00d0f49a44 Gerrit-Change-Number: 37798 Gerrit-PatchSet: 2 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: jolly <andreas(a)eversberg.eu>

1 0

[XS] Change in ...osmo-python-tests[master]: osmotestconfig: fix return value of write_config()
by jolly 12 Aug '24

12 Aug '24

jolly has uploaded this change for review. ( https://gerrit.osmocom.org/c/python/osmo-python-tests/+/37799?usp=email ) Change subject: osmotestconfig: fix return value of write_config() ...................................................................... osmotestconfig: fix return value of write_config() This patch fixes the following exception, if the received string does not match. The calling function just expects an integer. ---- TypeError: '>' not supported between instances of 'tuple' and 'int' ---- Change-Id: I24adfd344937bab1ab641327e59a25bf76e18591 --- M scripts/osmotestconfig.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/python/osmo-python-tests refs/changes/99/37799/1 diff --git a/scripts/osmotestconfig.py b/scripts/osmotestconfig.py index aa926f1..c56849d 100755 --- a/scripts/osmotestconfig.py +++ b/scripts/osmotestconfig.py @@ -86,7 +86,7 @@ new_config = vty.enabled_command("write") if not new_config.startswith("Configuration saved to "): print(new_config) - return 1, [new_config] + return 1 return 0 -- To view, visit https://gerrit.osmocom.org/c/python/osmo-python-tests/+/37799?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: python/osmo-python-tests Gerrit-Branch: master Gerrit-Change-Id: I24adfd344937bab1ab641327e59a25bf76e18591 Gerrit-Change-Number: 37799 Gerrit-PatchSet: 1 Gerrit-Owner: jolly <andreas(a)eversberg.eu>

1 0

[M] Change in libosmocore[master]: After writing VTY config, run sync() in a different thread
by Jenkins Builder 12 Aug '24

12 Aug '24

Jenkins Builder has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/37798?usp=email ) Change subject: After writing VTY config, run sync() in a different thread ...................................................................... Patch Set 1: (1 comment) File src/vty/command.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-17678): https://gerrit.osmocom.org/c/libosmocore/+/37798/comment/efaaf00c_adb5fbe2?… : PS1, Line 3402: static void *sync_thread(void* arg) "foo* bar" should be "foo *bar" -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/37798?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3cb2ee68b2e4c730f96522208c4abf00d0f49a44 Gerrit-Change-Number: 37798 Gerrit-PatchSet: 1 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Mon, 12 Aug 2024 14:28:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in libosmocore[master]: After writing VTY config, run sync() in a different thread
by jolly 12 Aug '24

12 Aug '24

jolly has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/37798?usp=email ) Change subject: After writing VTY config, run sync() in a different thread ...................................................................... After writing VTY config, run sync() in a different thread Running sync() after writing VTY config causes the process to stop for a fraction of a second. This may results in delayed processing that causes buffer underruns, overflows and delays. After writing VTY configuration to the config file, a new thread is created. This thread runs the sync() command. Once it returns, the thread will signal through a pipe to the main thread that the sync process is complete. The main process will then output the completion message on the VTY. Related: OS#6438 Change-Id: I3cb2ee68b2e4c730f96522208c4abf00d0f49a44 --- M src/vty/command.c 1 file changed, 87 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/98/37798/1 diff --git a/src/vty/command.c b/src/vty/command.c index 1719690..a6c5d94 100644 --- a/src/vty/command.c +++ b/src/vty/command.c @@ -39,6 +39,7 @@ #include <time.h> #include <limits.h> #include <signal.h> +#include <pthread.h> #include <sys/time.h> #include <sys/stat.h> #include <sys/types.h> @@ -51,6 +52,7 @@ #include <osmocom/core/talloc.h> #include <osmocom/core/timer.h> #include <osmocom/core/utils.h> +#include <osmocom/core/select.h> #ifndef MAXPATHLEN #define MAXPATHLEN 4096 @@ -3372,7 +3374,85 @@ return CMD_SUCCESS; } -static int write_config_file(const char *config_file, char **outpath) +static struct osmo_fd sync_fd; +static int sync_pipe_fd[2]; + +static int sync_complete(struct osmo_fd *fd, unsigned int what) +{ + struct vty *vty = fd->data; + + if (sync_pipe_fd[0]) + close(sync_pipe_fd[0]); + if (sync_pipe_fd[1]) + close(sync_pipe_fd[1]); + if (osmo_fd_is_registered(fd)) + osmo_fd_unregister(fd); + + /* Check if pointer to VTY session is still valid. + * If session closes before completion, prevent use-after-free condition. + */ + if (vty && vty_is_active(vty)) { + vty_out(vty, "%s", VTY_NEWLINE); + vty_out(vty, "Configuration saved to %s%s", host.config, VTY_NEWLINE); + } + + return 0; +} + +static void *sync_thread(void* arg) +{ + sync(); + write(sync_pipe_fd[1], "", 1); + + return NULL; +} + +/* Run sync thread and setup pipe to signal completion. */ +static void do_sync(struct vty *vty) +{ + int rc; + pthread_t thread; + + sync_pipe_fd[0] = 0; + sync_pipe_fd[1] = 0; + + if (osmo_fd_is_registered(&sync_fd)) { + LOGP(DLGLOBAL, LOGL_INFO, "There is another ongoing sync, syncing without thread.\n"); + sync(); + sync_complete(&sync_fd, 0); + return; + } + + rc = pipe(sync_pipe_fd); + if (rc < 0) { + LOGP(DLGLOBAL, LOGL_INFO, "Failed to create pipe, syncing without thread.\n"); + sync(); + sync_complete(&sync_fd, 0); + return; + } + + sync_fd.data = vty; + sync_fd.fd = sync_pipe_fd[0]; + sync_fd.when = OSMO_FD_READ; + sync_fd.cb = sync_complete; + rc = osmo_fd_register(&sync_fd); + if (rc < 0) { + LOGP(DLGLOBAL, LOGL_INFO, "Failed to register fd, syncing without thread.\n"); + sync(); + sync_complete(&sync_fd, 0); + return; + } + + rc = pthread_create(&thread, NULL, sync_thread, NULL); + if (rc < 0) { + LOGP(DLGLOBAL, LOGL_INFO, "Failed to create thread, syncing without thread.\n"); + sync(); + sync_complete(&sync_fd, 0); + return; + } +} + +static int write_config_file(struct vty *vty, const char *config_file, char **outpath) { unsigned int i; int fd; @@ -3451,12 +3531,12 @@ unlink(config_file_tmp); return -3; } - sync(); if (unlink(config_file) != 0) { *outpath = talloc_strdup(tall_vty_cmd_ctx, config_file); talloc_free(config_file_sav); talloc_free(config_file_tmp); unlink(config_file_tmp); + do_sync(NULL); return -4; } } @@ -3465,10 +3545,11 @@ talloc_free(config_file_sav); talloc_free(config_file_tmp); unlink(config_file_tmp); + do_sync(NULL); return -5; } unlink(config_file_tmp); - sync(); + do_sync(vty); talloc_free(config_file_sav); talloc_free(config_file_tmp); @@ -3511,7 +3592,7 @@ return CMD_WARNING; } - rc = write_config_file(host.config, &failed_file); + rc = write_config_file(vty, host.config, &failed_file); switch (rc) { case -1: vty_out(vty, "Can't open configuration file %s.%s", @@ -3544,7 +3625,6 @@ rc = CMD_WARNING; break; default: - vty_out(vty, "Configuration saved to %s%s", host.config, VTY_NEWLINE); rc = CMD_SUCCESS; break; } @@ -4377,7 +4457,7 @@ char *failed_file; int rc; - rc = write_config_file(filename, &failed_file); + rc = write_config_file(NULL, filename, &failed_file); talloc_free(failed_file); return rc; } @@ -4398,7 +4478,7 @@ if (host.config == NULL) return -7; - rc = write_config_file(host.config, &failed_file); + rc = write_config_file(NULL, host.config, &failed_file); talloc_free(failed_file); return rc; } -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/37798?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3cb2ee68b2e4c730f96522208c4abf00d0f49a44 Gerrit-Change-Number: 37798 Gerrit-PatchSet: 1 Gerrit-Owner: jolly <andreas(a)eversberg.eu>

1 0

[M] Change in pysim[master]: pySim-shell: improve fsdump
by dexter 12 Aug '24

12 Aug '24

Attention is currently required from: laforge. dexter has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/37786?usp=email ) Change subject: pySim-shell: improve fsdump ...................................................................... Patch Set 2: (1 comment) Commit Message: https://gerrit.osmocom.org/c/pysim/+/37786/comment/fcfce20c_6b74011a?usp=em… : PS1, Line 9: Th > somehow this sentence doesn't make sense. At least I can't parse it, please rephrase. Done -- To view, visit https://gerrit.osmocom.org/c/pysim/+/37786?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I4d2ef7b383025a5bbf122f18ecd51b7d73aaba14 Gerrit-Change-Number: 37786 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Mon, 12 Aug 2024 10:45:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org>

1 0

[M] Change in pysim[master]: pySim-shell: improve fsdump
by dexter 12 Aug '24

12 Aug '24

Attention is currently required from: dexter, laforge. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/pysim/+/37786?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by laforge Change subject: pySim-shell: improve fsdump ...................................................................... pySim-shell: improve fsdump In the previous patch we have improved the export command. Since the implementation of the fsdump command is very similar to the implementation of the export command we can now apply the same improvements to the fsdump command as well. Change-Id: I4d2ef7b383025a5bbf122f18ecd51b7d73aaba14 Related: OS#6092 --- M pySim-shell.py 1 file changed, 63 insertions(+), 101 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/86/37786/2 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/37786?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I4d2ef7b383025a5bbf122f18ecd51b7d73aaba14 Gerrit-Change-Number: 37786 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de>

1 0

2026

2025

2024

2023

2022

gerrit-log