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

[S] Change in libosmo-abis[master]: e1d: get rid of strange file descriptor registered check

by dexter

Attention is currently required from: tnt, pespin. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/32569 ) Change subject: e1d: get rid of strange file descriptor registered check ...................................................................... Patch Set 1: (1 comment) File src/input/e1d.c: https://gerrit.osmocom.org/c/libosmo-abis/+/32569/comment/388a121e_6a89d65f PS1, Line 420: continue; > here you already have a path where the bfd becomes unregistered but keeps >= 0. […] Hmm. I think in this case lets better call osmo_fd_registered()? the line update does not happen very frequently. -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32569 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I92c426271dc5455427471030fcf0749566e4c2ee Gerrit-Change-Number: 32569 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: tnt <tnt(a)246tNt.com> Gerrit-Attention: tnt <tnt(a)246tNt.com> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 02 May 2023 14:47:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[L] Change in osmo-mgw[master]: mgcp_codec: fix codec decision

by dexter

Attention is currently required from: neels, pespin. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/32218 ) Change subject: mgcp_codec: fix codec decision ...................................................................... Patch Set 11: (2 comments) File src/libosmo-mgcp/mgcp_codec.c: https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/71c22f40_fb53a4a9 PS8, Line 401: * a tentative decision is made. > 1) ack for dexter. […] 3) The follow up patch makes this more also more precise. When no no codec is chosen for the destination, then then mgcp_patch_pt() would fail and the packet would be tossed (follow up patch). File src/libosmo-mgcp/mgcp_network.c: https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/fde237f0_67e01d94 PS8, Line 511: rtp_hdr->payload_type = (uint8_t) conn_dst->end.codec->payload_type; > ACK, AFAIU the PayloadType is assigned to a connection, not to an endpoint. Yes that is correct. (The PayloadType is assigned to a codec and that codec is assigned to a conn) -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32218 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I6c3291f825488e5d8ce136aeb18450156794aeb5 Gerrit-Change-Number: 32218 Gerrit-PatchSet: 11 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 02 May 2023 14:38:10 +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: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in osmo-ttcn3-hacks[master]: gbproxy: Reduce timeout to fix flaky test

by daniel

daniel has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32570 ) Change subject: gbproxy: Reduce timeout to fix flaky test ...................................................................... gbproxy: Reduce timeout to fix flaky test The individual timeouts in TC_BVC_bringup_conflicting add up to almost 15.0s (the Tguard timeout) which causes the test to sometimes fail. Decrease the time waiting for all BVCs to be unblocked from 10s to 5s, in reality this should be plenty time. Change-Id: I620ce90f9e6b54cc94b4d36ac123f43d8d809f47 --- M gbproxy/GBProxy_Tests.ttcn 1 file changed, 15 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/70/32570/1 diff --git a/gbproxy/GBProxy_Tests.ttcn b/gbproxy/GBProxy_Tests.ttcn index ac81580..e5bec7d 100644 --- a/gbproxy/GBProxy_Tests.ttcn +++ b/gbproxy/GBProxy_Tests.ttcn @@ -1114,7 +1114,7 @@ } /* wait until all BVC are unblocked on both sides */ - timer T := 10.0; + timer T := 5.0; T.start; alt { [] SGSN_MGMT.receive(BssgpStatusIndication:{*, ?, ?}) { -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32570 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: I620ce90f9e6b54cc94b4d36ac123f43d8d809f47 Gerrit-Change-Number: 32570 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

[S] Change in libosmo-abis[master]: e1d: get rid of strange file descriptor registered check

by pespin

Attention is currently required from: tnt, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/32569 ) Change subject: e1d: get rid of strange file descriptor registered check ...................................................................... Patch Set 1: (1 comment) File src/input/e1d.c: https://gerrit.osmocom.org/c/libosmo-abis/+/32569/comment/4edaf764_0623e2e8 PS1, Line 420: continue; here you already have a path where the bfd becomes unregistered but keeps >= 0. I bet I may be able to find more of them if I look further below :) -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32569 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I92c426271dc5455427471030fcf0749566e4c2ee Gerrit-Change-Number: 32569 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: tnt <tnt(a)246tNt.com> Gerrit-CC: Jenkins Builder Gerrit-Attention: tnt <tnt(a)246tNt.com> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 02 May 2023 14:06:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[L] Change in libosmo-abis[master]: e1d: reconnect to osmo-e1d after connection loss

by dexter

Attention is currently required from: laforge, pespin, keith. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/32374 ) Change subject: e1d: reconnect to osmo-e1d after connection loss ...................................................................... Patch Set 8: (1 comment) File src/input/e1d.c: https://gerrit.osmocom.org/c/libosmo-abis/+/32374/comment/41bf4a60_f54d4ca8 PS4, Line 123: if (bfd->list.next && bfd->list.next != LLIST_POISON1) > No, it doesn't, because osmo_fd_is_registered() can be costly, and in most ocassions you know if the […] Indeed osmo_fd_is_registered() is a bit costly due to the list iteration. You are also right about the fd >= 0 check. This makes sense. Thanks. -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32374 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: Iaf4d42c2f009b1d7666e319fabdeb2598aa0b338 Gerrit-Change-Number: 32374 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: keith <keith(a)rhizomatica.org> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-CC: tnt <tnt(a)246tNt.com> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: keith <keith(a)rhizomatica.org> Gerrit-Comment-Date: Tue, 02 May 2023 14:02:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in libosmo-abis[master]: e1d: get rid of strange file descriptor registered check

by dexter

dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-abis/+/32569 ) Change subject: e1d: get rid of strange file descriptor registered check ...................................................................... e1d: get rid of strange file descriptor registered check When the line update happens the list.next member of the file descriptor is checked for NULL and != LLIST_POISON1. This directly tampers with the llist API and might be problematic. Also we can tell by the file descriptor number if the file descriptor is registered or not. An open file descriptor is always registered. Change-Id: I92c426271dc5455427471030fcf0749566e4c2ee Related: OS#5983 --- M src/input/e1d.c 1 file changed, 18 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/69/32569/1 diff --git a/src/input/e1d.c b/src/input/e1d.c index e1a1526..f1b9ac0 100644 --- a/src/input/e1d.c +++ b/src/input/e1d.c @@ -410,8 +410,8 @@ struct e1inp_ts *e1i_ts = &line->ts[idx]; struct osmo_fd *bfd = &e1i_ts->driver.e1d.fd; - /* unregister FD if it was already registered */ - if (bfd->list.next && bfd->list.next != LLIST_POISON1) + /* unregister FD if it was already registered/in use */ + if (bfd->fd >= 0) osmo_fd_unregister(bfd); if (e1i_ts->type != E1INP_TS_TYPE_NONE && ts >= num_ts_info) { -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32569 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I92c426271dc5455427471030fcf0749566e4c2ee Gerrit-Change-Number: 32569 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

[L] Change in libosmo-abis[master]: e1d: reconnect to osmo-e1d after connection loss

by dexter

Attention is currently required from: laforge, pespin, keith. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-abis/+/32374 to look at the new patch set (#8). Change subject: e1d: reconnect to osmo-e1d after connection loss ...................................................................... e1d: reconnect to osmo-e1d after connection loss When osmo-e1d crashes while a line is in use the client process may experience not only a lost connection, it may also hang up in an endless loop that would also spam the logfile. The reason for this is that the e1d driver in libosmo-abis lacks mechanisms to detect when the connection to osmo-e1d gets lost. When osmo-e1d goes down the effects should be similar to those of a normal connection loss of an e1 line (cable pulled). So let's add an FSM that takes care of the recovery of the connection when it is lost. Also make sure that no havoc happens when the connection gets lost. Related: OS#5983 Change-Id: Iaf4d42c2f009b1d7666e319fabdeb2598aa0b338 --- M src/input/e1d.c 1 file changed, 250 insertions(+), 13 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/74/32374/8 -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32374 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: Iaf4d42c2f009b1d7666e319fabdeb2598aa0b338 Gerrit-Change-Number: 32374 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: keith <keith(a)rhizomatica.org> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-CC: tnt <tnt(a)246tNt.com> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: keith <keith(a)rhizomatica.org> Gerrit-MessageType: newpatchset

2 years, 1 month

1
0
0 0

[S] Change in libosmocore[master]: tlv: Show bug in decoded tlv_parsed for type TLV_TYPE_SINGLE_TV

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32566 ) Change subject: tlv: Show bug in decoded tlv_parsed for type TLV_TYPE_SINGLE_TV ...................................................................... tlv: Show bug in decoded tlv_parsed for type TLV_TYPE_SINGLE_TV A commit was merged recently attempting to fix decoding of TLV_TYPE_SINGLE_TV. It did mostly a good job, but missed updating the o_tag pointer used to fill in the structures. This new unit test showcases the mentioned problem. A follow-up patch will fix the bug. Change-Id: Ia17c84059a413f80c2bcf194034ebac586ecf7e1 --- M tests/tlv/tlv_test.c 1 file changed, 42 insertions(+), 0 deletions(-) Approvals: Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve daniel: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/tests/tlv/tlv_test.c b/tests/tlv/tlv_test.c index f9137ad..aaa86a3 100644 --- a/tests/tlv/tlv_test.c +++ b/tests/tlv/tlv_test.c @@ -454,6 +454,32 @@ } } +static void test_tlv_type_single_tv(void) +{ + #define SAMPLE_SINGLE_TV_IE 0x08 + const struct tlv_definition att_tlvdef = { + .def = { + [SAMPLE_SINGLE_TV_IE] = { TLV_TYPE_SINGLE_TV, 0 }, + }, + }; + struct tlv_parsed tp; + int rc; + uint8_t exp_val = 0x03; + uint8_t buf[] = { (SAMPLE_SINGLE_TV_IE << 4) | (exp_val & 0x0f) }; + const uint8_t *val; + + rc = tlv_parse(&tp, &att_tlvdef, buf, sizeof(buf), 0, 0); + OSMO_ASSERT(rc == 1); + OSMO_ASSERT(!TLVP_PRESENT(&tp, SAMPLE_SINGLE_TV_IE)); //FIXME! + val = TLVP_VAL(&tp, SAMPLE_SINGLE_TV_IE); + OSMO_ASSERT(!val); //FIXME! +#if 0 + OSMO_ASSERT(val == &buf[0]); + OSMO_ASSERT(*val == buf[0]); + OSMO_ASSERT((*val & 0x0f) == exp_val); +#endif +} + int main(int argc, char **argv) { //osmo_init_logging2(ctx, &info); @@ -463,6 +489,7 @@ test_tlv_encoder(); test_tlv_parser_bounds(); test_tlv_lens(); + test_tlv_type_single_tv(); printf("Done.\n"); return EXIT_SUCCESS; -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32566 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia17c84059a413f80c2bcf194034ebac586ecf7e1 Gerrit-Change-Number: 32566 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 1 month

1
0
0 0

[S] Change in libosmocore[master]: Fix 'Fix parsing of TLV_TYPE_SINGLE_TV'

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32549 ) Change subject: Fix 'Fix parsing of TLV_TYPE_SINGLE_TV' ...................................................................... Fix 'Fix parsing of TLV_TYPE_SINGLE_TV' A commit was merged recently attempting to fix decoding of TLV_TYPE_SINGLE_TV. It did mostly a good job, but missed updating the o_tag pointer used to fill in the structures. This commit fixes that specific part missing. Fixes: 559a6ee68359dab691a483573982e6f8c6439ae2 Change-Id: Id619459c17976b77cd2c7e4179123bb06807285c --- M src/gsm/tlv_parser.c M tests/tlv/tlv_test.c 2 files changed, 28 insertions(+), 7 deletions(-) Approvals: Jenkins Builder: Verified daniel: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/src/gsm/tlv_parser.c b/src/gsm/tlv_parser.c index e47b94f..8dd460d 100644 --- a/src/gsm/tlv_parser.c +++ b/src/gsm/tlv_parser.c @@ -225,7 +225,11 @@ * \param[in] def structure defining the valid TLV tags / configurations * \param[in] buf the input data buffer to be parsed * \param[in] buf_len length of the input data buffer - * \returns number of bytes consumed by the TLV entry / IE parsed; negative in case of error + * \returns number of bytes consumed by the TLV entry / IE parsed; negative in case of error. + * + * In IEs of type TLV_TYPE_SINGLE_TV, the data pointer \ref o_val will point to the + * byte shared by both the Tag and te Value, hence the tag is to be trimmed + * by the caller. */ int tlv_parse_one(uint8_t *o_tag, uint16_t *o_len, const uint8_t **o_val, const struct tlv_definition *def, @@ -241,9 +245,13 @@ *o_tag = tag; /* single octet TV IE */ - if (def->def[tag >> 4].type == TLV_TYPE_SINGLE_TV + if (def->def[tag >> 4].type == TLV_TYPE_SINGLE_TV) { + *o_tag = tag >> 4; + *o_val = buf; + *o_len = 1; + return 1; + } else if ((tag > 0x0f) && (def->def[tag & 0xf0].type == TLV_TYPE_SINGLE_TV)) { /* backward compat for old IEs with half-octet tag defined as 0xN0: */ - || ((tag > 0x0f) && (def->def[tag & 0xf0].type == TLV_TYPE_SINGLE_TV))) { *o_tag = tag & 0xf0; *o_val = buf; *o_len = 1; diff --git a/tests/tlv/tlv_test.c b/tests/tlv/tlv_test.c index aaa86a3..8e8bd60 100644 --- a/tests/tlv/tlv_test.c +++ b/tests/tlv/tlv_test.c @@ -470,14 +470,12 @@ rc = tlv_parse(&tp, &att_tlvdef, buf, sizeof(buf), 0, 0); OSMO_ASSERT(rc == 1); - OSMO_ASSERT(!TLVP_PRESENT(&tp, SAMPLE_SINGLE_TV_IE)); //FIXME! + OSMO_ASSERT(TLVP_PRESENT(&tp, SAMPLE_SINGLE_TV_IE)); val = TLVP_VAL(&tp, SAMPLE_SINGLE_TV_IE); - OSMO_ASSERT(!val); //FIXME! -#if 0 + OSMO_ASSERT(val); OSMO_ASSERT(val == &buf[0]); OSMO_ASSERT(*val == buf[0]); OSMO_ASSERT((*val & 0x0f) == exp_val); -#endif } int main(int argc, char **argv) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32549 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id619459c17976b77cd2c7e4179123bb06807285c Gerrit-Change-Number: 32549 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-MessageType: merged

2 years, 1 month

1
0
0 0

[S] Change in libosmocore[master]: Fix 'Fix parsing of TLV_TYPE_SINGLE_TV'

by pespin

Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32549 ) Change subject: Fix 'Fix parsing of TLV_TYPE_SINGLE_TV' ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32549 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id619459c17976b77cd2c7e4179123bb06807285c Gerrit-Change-Number: 32549 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Tue, 02 May 2023 13:51:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

2025

2024

2023

2022

gerrit-log May 2023