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

[S] Change in osmo-ttcn3-hacks[master]: pgw: Avoid access to unbound global variable

by laforge

Attention is currently required from: pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/31592 ) Change subject: pgw: Avoid access to unbound global variable ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/31592 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: I9731b755ddc7bb3ebd56bdec550961cc36f2e2c6 Gerrit-Change-Number: 31592 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 18:18:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[S] Change in libosmocore[master]: osmo_wqueue_init(): Fix parameter type

by laforge

Attention is currently required from: arehbein. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31595 ) Change subject: osmo_wqueue_init(): Fix parameter type ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: In general I agree, but I'm worried about potential fall-out in applications. It really is an API Change, and if somebody compiles their software with strict integer type checks, compilation might now fail after the patch applied. So we have the option of a) breaking the API, but think we can get away with it (we might) b) introduce a new wqueue_init2() function (is it worth the effort) c) doing nothing, keep it as-is. After all, there's no use case of a queue with more than INT32_MAX entries, and wqueue_init() could and should check that no negative values are passed in by the caller. I personally would go for "c" -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31595 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I6c9295750e1755bf1d80f3a3b32efc2aefb11a57 Gerrit-Change-Number: 31595 Gerrit-PatchSet: 1 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-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 18:18:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[M] Change in osmo-mgw[master]: mgcp_sdp: add fmtp string to define HR GSM RTP format

by laforge

Attention is currently required from: neels, dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/31214 ) Change subject: mgcp_sdp: add fmtp string to define HR GSM RTP format ...................................................................... Patch Set 10: (1 comment) File include/osmocom/mgcp/mgcp_common.h: https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/f7441827_05b888af PS10, Line 71: enum mgcp_gsm_hr_fmt gsm_hr_format; > https://osmocom. […] Ack -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/31214 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Idde8da27fd335dc03b8fbd9e0fedc1491b77e9e4 Gerrit-Change-Number: 31214 Gerrit-PatchSet: 10 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 18:13:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[M] Change in osmo-bts[master]: common: Have PCU socket connection use osmo_wqueue

by laforge

Attention is currently required from: arehbein, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31533 ) Change subject: common: Have PCU socket connection use osmo_wqueue ...................................................................... Patch Set 3: (2 comments) File include/osmo-bts/pcuif_proto.h: https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/776f5791_bde18613 PS2, Line 8: #define PCU_SOCK_QLENGTH_MAX_DEFAULT 10 > Yes because that path is used by both ends to connect. […] Ack File src/common/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/4fc3d05d_9de3f278 PS3, Line 993: LOGP(DLGLOBAL, LOGL_NOTICE, "PCU not reacting (more thatn %u messages waiting). Closing connection\n", the code above is using DPCU everywhere, so I don't see a need to use a non-PCU-specific log subsystem here. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 Gerrit-Change-Number: 31533 Gerrit-PatchSet: 3 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-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 18:12:02 +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 year, 1 month

1
0
0 0

[S] Change in osmo-bsc[master]: pcuif_proto: increment version number

by laforge

Attention is currently required from: dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31597 ) Change subject: pcuif_proto: increment version number ...................................................................... Patch Set 1: Verified-1 Code-Review+1 (1 comment) Patchset: PS1: WARNING: we have to make sure to merge this version increment simultaneously in TTCN3 code, osmo-pcu, osmo-bsc and osmo-bts. Hence the V-1 to prevent accidential merge. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31597 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I8315bd67c7f3eb0d7ee71b64cd4dff889a84fcf1 Gerrit-Change-Number: 31597 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 18:10:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[M] Change in osmo-bsc[master]: pcu_sock: improve logging

by laforge

Attention is currently required from: dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31579 ) Change subject: pcu_sock: improve logging ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31579 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I4a3ff23ffccf803b1a97e3f2604d3b422c6d6afd Gerrit-Change-Number: 31579 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 18:09:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[M] Change in osmo-bsc[master]: pcu_sock: use struct to transfer IMMEDIATE ASSIGNMENT for PCH

by laforge

Attention is currently required from: dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31578 ) Change subject: pcu_sock: use struct to transfer IMMEDIATE ASSIGNMENT for PCH ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31578 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Id6acbd243adf26169e5e8319dd66bb68dd6a3c22 Gerrit-Change-Number: 31578 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 18:08:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[S] Change in osmo-bsc[master]: abis_rsl: assert maximum length

by laforge

Attention is currently required from: dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31577 ) Change subject: abis_rsl: assert maximum length ...................................................................... Patch Set 3: -Code-Review -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31577 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I9417b35fb8c0517f2555e17059bf8ac60fa59791 Gerrit-Change-Number: 31577 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 18:08:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[S] Change in osmo-bsc[master]: abis_rsl: assert maximum length

by laforge

Attention is currently required from: dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31577 ) Change subject: abis_rsl: assert maximum length ...................................................................... Patch Set 3: (1 comment) File src/osmo-bsc/abis_rsl.c: https://gerrit.osmocom.org/c/osmo-bsc/+/31577/comment/33323c21_2a72466b PS3, Line 948: val we have to be a bit carefult with assert's to avoid ending up in the sukchan-style of software development, where any (even unexpeted) protocol message can create a denial of service against the program. I didn't check who calls this function, but it looks like it's triggered from the RSL input path.. In general, ASSERT should be used to protect us against programming mistakes in this program (BSC). But not against unexpected messages received from other network elements. In that case we need to do runtime length checking + error handling -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31577 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I9417b35fb8c0517f2555e17059bf8ac60fa59791 Gerrit-Change-Number: 31577 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 18:08:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[S] Change in osmo-bsc[master]: abis_rsl: assert maximum length

by laforge

Attention is currently required from: dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31577 ) Change subject: abis_rsl: assert maximum length ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31577 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I9417b35fb8c0517f2555e17059bf8ac60fa59791 Gerrit-Change-Number: 31577 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 18:06:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[S] Change in osmo-bsc[master]: pcu_sock: print OML alerts from PCU

by laforge

Attention is currently required from: fixeria, dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31499 ) Change subject: pcu_sock: print OML alerts from PCU ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31499 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Id32553556356c2affe32e47ae1c3ae6a514efdce Gerrit-Change-Number: 31499 Gerrit-PatchSet: 5 Gerrit-Owner: dexter <pmaier(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: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 18:05:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[M] Change in osmo-bsc[master]: pcu_sock: activate/deactivate PDCH on pcu reconnect

by laforge

Attention is currently required from: neels, fixeria, dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31497 ) Change subject: pcu_sock: activate/deactivate PDCH on pcu reconnect ...................................................................... Patch Set 5: (3 comments) File src/osmo-bsc/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/1565225a_6f6796c5 PS2, Line 801: printf("l1sap_chan_rel(trx,gsm_lchan2chan_nr(ts->lchan));\n"); > either keep this printf, or if it should be dropped, drop it in a separate patch? Ack https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/968de04c_416d056b PS2, Line 801: && ts->pchan_on_init == GSM_PCHAN_OSMO_DYN > Why does this apply only to dynamic timeslots? I think I understand why, but I'd like to see a code […] Ericsson only has fully-dynamic timeslots, AFAIR. But yes, mentioning that here might make sense for the most common reader of this code who has no clue about ericsson specifics. https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/880ebec0_1f12759b PS2, Line 941: bts = llist_entry(state->net->bts_list.next, struct gsm_bts, list); > rather use llist_first_entry_or_null() […] I actually think it's a serious problem that only one BTS appears to be suppored here. Not only that, but there's an implicit assumption that the first BTS is the Ericsson BTS served by the PCU? In general, a BSC-colocated PCU can of course serve any number of BTSs. Until we address that general problem/shortcoming in our current code, we shouldn't make a blind assumption, but we should make sure that the user can operate this only in a safe configuration. In the worst case, we could check if thre are more than one (ericsson) BTS configured in the BSC, and refuse to operate the PCU socket with an associated error message. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31497 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I9ea0c53a5e68a51c781ef43bae71f947cdb95678 Gerrit-Change-Number: 31497 Gerrit-PatchSet: 5 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 18:05:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[XL] Change in osmo-pcu[master]: support for Ericsson RBS E1 CCU

by laforge

Attention is currently required from: neels, dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31176 ) Change subject: support for Ericsson RBS E1 CCU ...................................................................... Patch Set 16: (9 comments) File src/ericsson-rbs/er_ccu_descr.h: https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/a5fc4f28_cb3a31c1 PS16, Line 27: uint32_t pseq_ccu; : uint32_t pseq_pcu; : uint32_t last_afn_ul; : uint32_t last_afn_dl; : bool ccu_synced; : enum time_adj_val tav; : bool ul_frame_err; you could potentally group those things into sub-structs? If all of this is sync state, then it might be a good idea. Also, we have a trau_sync_fi above (but that's in the i460 subslot group? so maybe clarify which kind of sync is meant where. Also, I think it would be nice to have comments for each member of the struct (could be on the same line). some are self-explanatory (trx_no, ts_, pdch_connected, ccu_synced, ccu_connected). Some others don't have any meaning to me, like what is "pseq" or "afn". File src/ericsson-rbs/er_ccu_if.c: https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/ee9fb459_a5abb1da PS16, Line 38: LOGP(DE1, level, "%s: E1-l this probably also should use LOGPITS, see comment below? https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/083e780a_ea12a29a PS16, Line 54: static void *tall_ctx = NULL; might make sens to give this a less generic name. like ccu_tall_ctx? https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/c7e6b8a3_18a894e9 PS16, Line 100: static void "e1_send" is very generic. I'd try to be more specific. It is sending one frame/chunk for one timeslot. So both of that could be part of the function to make it clear to the reader what it does. e1_ts_send_one_{frame,chunk} ? https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/3a7be8fa_afcfb32c PS16, Line 121: LOGP(DE1, LOGL_DEBUG, "E1-TX: (line=%u,t we do have a LOGPITS() macro for logging the e1inp_line + timeslot in a standardized manner. Please use in all locations where an e1inp_ts is available. In general, every time you end up writing more than two log lines that print some context like "(line=%u,ts=%u)" it is a very strong indication that you're not using an existing macro for logging context information, or that you should write a new one if it doesn't exist yet. https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/7186f198_8ed02cbd PS16, Line 156: e1inp_rx_ts might be worth mentioning that e1inp_rx_ts is the caller of this call-back. like "our caller e1inp_rx_ts() ..." https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/24ef8a02_cc4b7c19 PS16, Line 233: ccu_descr->trau_sync_fi = osmo_trau_sync_alloc(NULL, "trau-sync", sync_frame_out_cb, sync_pattern, ccu_descr); do we really have no better talloc-parent? Attaching things to the NULL Context is a last resort. https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/9ef85535_b19266a0 PS16, Line 285: e1inp_ts_config_raw why are we not using e1inp_ts_config_i460() here? It seems we're using I.460 but not calling the related ts_config function. https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/661ae5dd_59b959d5 PS16, Line 367: return don't we need to close the e1_timeslot in the error path, if add_i468_subslot fails? -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31176 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I5c0a76667339ca984a12cbd2052f5d9e5b0f9c4d Gerrit-Change-Number: 31176 Gerrit-PatchSet: 16 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 17:59:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[S] Change in osmo-pcu[master]: pcu_l1_if_phy: add new PHY API function to initialize PHY

by laforge

Attention is currently required from: fixeria, dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31574 ) Change subject: pcu_l1_if_phy: add new PHY API function to initialize PHY ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31574 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ibf2a3a058c826f6ee5b740eee72d5be94d460517 Gerrit-Change-Number: 31574 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(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: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 17:44:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[M] Change in osmo-pcu[master]: pcu_l1_if_phy: add new PHY API function to disconnect PDCH

by laforge

Attention is currently required from: pespin, fixeria, dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31573 ) Change subject: pcu_l1_if_phy: add new PHY API function to disconnect PDCH ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31573 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ied88f3289bda87c48f5f9255c4591470633cc805 Gerrit-Change-Number: 31573 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(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-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 17:44:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[M] Change in osmo-pcu[master]: bts: add IMMEDIATE ASSIGNMENT via PCH transmission

by laforge

Attention is currently required from: neels, pespin, fixeria, dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31145 ) Change subject: bts: add IMMEDIATE ASSIGNMENT via PCH transmission ...................................................................... Patch Set 18: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31145 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I2a78651593323e8b9627c39918d949a33497b70f Gerrit-Change-Number: 31145 Gerrit-PatchSet: 18 Gerrit-Owner: dexter <pmaier(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-CC: neels <nhofmeyr(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-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 17:43:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[S] Change in libosmocore[master]: Add osmo_sockaddr_size() to return the size of the variant used

by laforge

Attention is currently required from: pespin, daniel, msuraev. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31172 ) Change subject: Add osmo_sockaddr_size() to return the size of the variant used ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31172 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I952b6bb752441fe019fc18f89bce4bbfbe58994a Gerrit-Change-Number: 31172 Gerrit-PatchSet: 3 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 17:42:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[M] Change in docker-playground[master]: docker kill: wait until containers are stopped

by laforge

Attention is currently required from: osmith. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/docker-playground/+/31598 ) Change subject: docker kill: wait until containers are stopped ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/docker-playground/+/31598 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: docker-playground Gerrit-Branch: master Gerrit-Change-Id: I0242ece96541d8036ebbf8b0f498ebf231db26b5 Gerrit-Change-Number: 31598 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 17:42:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[M] Change in docker-playground[master]: docker kill: wait until containers are stopped

by osmith

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/docker-playground/+/31598 to look at the new patch set (#2). Change subject: docker kill: wait until containers are stopped ...................................................................... docker kill: wait until containers are stopped As "docker kill" / "docker container kill" (alias) doesn't block until the given container stops, make sure to always run "docker wait" afterwards. Closes: OS#5928 Change-Id: I0242ece96541d8036ebbf8b0f498ebf231db26b5 --- M jenkins-common.sh M osmo-cn-latest/run.sh M osmo-ran/split/jenkins-split.sh M scripts/regen_doc.sh M ttcn3-bts-test/jenkins.sh M ttcn3-fr-test/jenkins.sh M ttcn3-hnbgw-test/jenkins.sh M ttcn3-hnodeb-test/jenkins.sh M ttcn3-remsim-test/jenkins.sh 9 files changed, 46 insertions(+), 17 deletions(-) git pull ssh://gerrit.osmocom.org:29418/docker-playground refs/changes/98/31598/2 -- To view, visit https://gerrit.osmocom.org/c/docker-playground/+/31598 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: docker-playground Gerrit-Branch: master Gerrit-Change-Id: I0242ece96541d8036ebbf8b0f498ebf231db26b5 Gerrit-Change-Number: 31598 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

1 year, 1 month

1
0
0 0

[M] Change in docker-playground[master]: docker kill: wait until containers are stopped

by osmith

osmith has uploaded this change for review. ( https://gerrit.osmocom.org/c/docker-playground/+/31598 ) Change subject: docker kill: wait until containers are stopped ...................................................................... docker kill: wait until containers are stopped As "docker kill" / "docker container kill" (alias) doesn't block until the given container stops, make sure to always run "docker wait" afterwards. Closes: OS#5928 Change-Id: I0242ece96541d8036ebbf8b0f498ebf231db26b5 --- M jenkins-common.sh M osmo-cn-latest/run.sh M osmo-ran/split/jenkins-split.sh M scripts/regen_doc.sh M ttcn3-bts-test/jenkins.sh M ttcn3-fr-test/jenkins.sh M ttcn3-hnbgw-test/jenkins.sh M ttcn3-hnodeb-test/jenkins.sh M ttcn3-remsim-test/jenkins.sh 9 files changed, 46 insertions(+), 17 deletions(-) git pull ssh://gerrit.osmocom.org:29418/docker-playground refs/changes/98/31598/1 diff --git a/jenkins-common.sh b/jenkins-common.sh index b09db58..ec91b3f 100644 --- a/jenkins-common.sh +++ b/jenkins-common.sh @@ -190,9 +190,24 @@ done } +# Kill a docker container and ensure it doesn't exist anymore (OS#5928) +docker_kill_wait() { + docker kill "$1" + docker wait "$1" || true +} + #kills all containers attached to network network_clean() { - docker network inspect $NET_NAME | grep Name | cut -d : -f2 | awk -F\" 'NR>1{print $2}' | xargs -rn1 docker kill + local containers="$(docker network inspect $NET_NAME | grep Name | cut -d : -f2 | awk -F\" 'NR>1{print $2}')" + local container + + if [ -z "$containers" ]; then + return + fi + + for container in $containers; do + docker_kill_wait "$container" + done } # Create network and find a free subnet automatically. The global variables diff --git a/osmo-cn-latest/run.sh b/osmo-cn-latest/run.sh index d7bba95..4745ee3 100755 --- a/osmo-cn-latest/run.sh +++ b/osmo-cn-latest/run.sh @@ -28,6 +28,6 @@ echo Stopping containers -docker container kill osmo-cn +docker_kill_wait osmo-cn network_remove diff --git a/osmo-ran/split/jenkins-split.sh b/osmo-ran/split/jenkins-split.sh index 5cecf4a..58c1aea 100755 --- a/osmo-ran/split/jenkins-split.sh +++ b/osmo-ran/split/jenkins-split.sh @@ -69,7 +69,7 @@ kill_containers() { for i in "${docker_names[@]}"; do - docker kill $i + docker_kill_wait $i done } diff --git a/scripts/regen_doc.sh b/scripts/regen_doc.sh index 27c3138..fdc9da2 100755 --- a/scripts/regen_doc.sh +++ b/scripts/regen_doc.sh @@ -51,6 +51,6 @@ -H "172.18.$SUBNET.23" \ -O "$VTYFILE" -docker container kill "${BUILD_TAG}-$NAME-$IMAGE_SUFFIX" +docker_kill_wait "${BUILD_TAG}-$NAME-$IMAGE_SUFFIX" network_remove diff --git a/ttcn3-bts-test/jenkins.sh b/ttcn3-bts-test/jenkins.sh index 0c1cbb0..ed1e095 100755 --- a/ttcn3-bts-test/jenkins.sh +++ b/ttcn3-bts-test/jenkins.sh @@ -168,9 +168,9 @@ # 2) some GPRS tests require virt_phy echo "Changing to virtphy configuration" # switch from osmo-bts-trx + trxcon + faketrx to virtphy + osmo-bts-virtual -docker container kill ${BUILD_TAG}-trxcon -docker container kill ${BUILD_TAG}-fake_trx -docker container kill ${BUILD_TAG}-bts +docker_kill_wait ${BUILD_TAG}-trxcon +docker_kill_wait ${BUILD_TAG}-fake_trx +docker_kill_wait ${BUILD_TAG}-bts cp virtphy/osmo-bts.gen.cfg $VOL_BASE_DIR/bts/ network_replace_subnet_in_configs start_bts virtual 0 @@ -179,10 +179,10 @@ #start_testsuite virtphy # 3) OML tests require us to run without BSC -docker container kill ${BUILD_TAG}-bsc +docker_kill_wait ${BUILD_TAG}-bsc # switch back from virtphy + osmo-bts-virtual to osmo-bts-trx -docker container kill ${BUILD_TAG}-virtphy -docker container kill ${BUILD_TAG}-bts +docker_kill_wait ${BUILD_TAG}-virtphy +docker_kill_wait ${BUILD_TAG}-bts cp oml/osmo-bts.gen.cfg $VOL_BASE_DIR/bts/ network_replace_subnet_in_configs @@ -196,7 +196,7 @@ cp fh/osmo-bsc.gen.cfg $VOL_BASE_DIR/bsc/ cp generic/osmo-bts.gen.cfg $VOL_BASE_DIR/bts/ # restart the BSC/BTS and run the testsuite again -docker container kill ${BUILD_TAG}-bts +docker_kill_wait ${BUILD_TAG}-bts network_replace_subnet_in_configs start_bsc start_bts trx 1 diff --git a/ttcn3-fr-test/jenkins.sh b/ttcn3-fr-test/jenkins.sh index 12e14a5..36bb77b 100755 --- a/ttcn3-fr-test/jenkins.sh +++ b/ttcn3-fr-test/jenkins.sh @@ -10,7 +10,7 @@ clean_up() { # kill the frnet container to avoid "You cannot remove a running container " below in 'rm' - docker kill ${BUILD_TAG}-frnet || true + docker_kill_wait ${BUILD_TAG}-frnet || true # store execution logs for both containers docker logs --timestamps ${BUILD_TAG}-ttcn3-fr-test > $VOL_BASE_DIR/fr-tester/exec.log || true diff --git a/ttcn3-hnbgw-test/jenkins.sh b/ttcn3-hnbgw-test/jenkins.sh index f3c5694..e93c8af 100755 --- a/ttcn3-hnbgw-test/jenkins.sh +++ b/ttcn3-hnbgw-test/jenkins.sh @@ -74,8 +74,8 @@ $REPO_USER/ttcn3-hnbgw-test echo Stopping containers - docker container kill ${BUILD_TAG}-hnbgw - docker container kill ${BUILD_TAG}-stp + docker_kill_wait ${BUILD_TAG}-hnbgw + docker_kill_wait ${BUILD_TAG}-stp } echo Testing without PFCP diff --git a/ttcn3-hnodeb-test/jenkins.sh b/ttcn3-hnodeb-test/jenkins.sh index 21e72c0..3b57fd8 100755 --- a/ttcn3-hnodeb-test/jenkins.sh +++ b/ttcn3-hnodeb-test/jenkins.sh @@ -45,4 +45,4 @@ $REPO_USER/ttcn3-hnodeb-test echo Stopping containers -docker container kill ${BUILD_TAG}-hnodeb +docker_kill_wait ${BUILD_TAG}-hnodeb diff --git a/ttcn3-remsim-test/jenkins.sh b/ttcn3-remsim-test/jenkins.sh index a7325e7..158a352 100755 --- a/ttcn3-remsim-test/jenkins.sh +++ b/ttcn3-remsim-test/jenkins.sh @@ -80,7 +80,7 @@ network_replace_subnet_in_configs start_server start_testsuite -docker container kill ${BUILD_TAG}-server +docker_kill_wait ${BUILD_TAG}-server # 2) bankd test suite echo "Changing to bankd configuration" @@ -89,7 +89,7 @@ network_replace_subnet_in_configs start_bankd start_testsuite -docker container kill ${BUILD_TAG}-bankd +docker_kill_wait ${BUILD_TAG}-bankd # 3) client test suite echo "Changing to client configuration" -- To view, visit https://gerrit.osmocom.org/c/docker-playground/+/31598 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: docker-playground Gerrit-Branch: master Gerrit-Change-Id: I0242ece96541d8036ebbf8b0f498ebf231db26b5 Gerrit-Change-Number: 31598 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 1 month

1
0
0 0

[S] Change in libosmocore[master]: Add osmo_sockaddr_size() to return the size of the variant used

by daniel

Attention is currently required from: pespin, msuraev. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31172 ) Change subject: Add osmo_sockaddr_size() to return the size of the variant used ...................................................................... Patch Set 3: (1 comment) File include/osmocom/core/socket.h: https://gerrit.osmocom.org/c/libosmocore/+/31172/comment/4d3adf3c_a252f0aa PS1, Line 42: { > Ok, maybe add a comment in the code about this when handling this case, because it's really not intu […] Done -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31172 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I952b6bb752441fe019fc18f89bce4bbfbe58994a Gerrit-Change-Number: 31172 Gerrit-PatchSet: 3 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 17:13:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[S] Change in libosmocore[master]: Add osmo_sockaddr_size() to return the size of the variant used

by daniel

Attention is currently required from: daniel, msuraev. Hello Jenkins Builder, pespin, msuraev, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/31172 to look at the new patch set (#3). Change subject: Add osmo_sockaddr_size() to return the size of the variant used ...................................................................... Add osmo_sockaddr_size() to return the size of the variant used Change-Id: I952b6bb752441fe019fc18f89bce4bbfbe58994a --- M include/osmocom/core/socket.h 1 file changed, 32 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/72/31172/3 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31172 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I952b6bb752441fe019fc18f89bce4bbfbe58994a Gerrit-Change-Number: 31172 Gerrit-PatchSet: 3 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 year, 1 month

1
0
0 0

[M] Change in osmo-pcu[master]: bts: add IMMEDIATE ASSIGNMENT via PCH transmission

by dexter

Attention is currently required from: neels, pespin, fixeria. Hello Jenkins Builder, laforge, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-pcu/+/31145 to look at the new patch set (#18). Change subject: bts: add IMMEDIATE ASSIGNMENT via PCH transmission ...................................................................... bts: add IMMEDIATE ASSIGNMENT via PCH transmission In situations where the PCU is co-located to the BSC, the IMMEDIATE ASSIGNMENT for downlink TBFs must be sent via RSL and the BSC also must instruct the BTS to transmit the IMMEDIATE ASSIGNMENT via PCH instead of AGCH. Eventually the BSC must sent a confirmation message (follow-up patch) where the TLLI is used as an identifer. Change-Id: I2a78651593323e8b9627c39918d949a33497b70f Related: OS#5198 --- M include/osmocom/pcu/pcuif_proto.h M src/bts.cpp M src/bts.h M src/pcu_l1_if.cpp M src/pcu_l1_if.h 5 files changed, 80 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/45/31145/18 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31145 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I2a78651593323e8b9627c39918d949a33497b70f Gerrit-Change-Number: 31145 Gerrit-PatchSet: 18 Gerrit-Owner: dexter <pmaier(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-CC: neels <nhofmeyr(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-MessageType: newpatchset

1 year, 1 month

1
0
0 0

[M] Change in osmo-pcu[master]: bts: add IMMEDIATE ASSIGNMENT via PCH transmission

by dexter

Attention is currently required from: neels, pespin, fixeria. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31145 ) Change subject: bts: add IMMEDIATE ASSIGNMENT via PCH transmission ...................................................................... Patch Set 18: (6 comments) File include/osmocom/pcu/pcuif_proto.h: https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/5510a419_bd202104 PS17, Line 48: #define PCU_IF_FLAG_DT (1 << 2)/* use TLLI for confirmation directly */ > I think we agreed in the call this flag was not needed. Thanks for reminding me. I wasn't sure anymore but it certainly makes sense to drop the flag and look at the PCUIIF version number instead. https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/e20bdc85_35de6adb PS17, Line 280: uint8_t pgroup[3]; > This can be a int16_t containing a number 0-999. I think this would then make using the struct at the BSC side more complicated, but I am not entirely sure. The thing is that this value ends up at extract_paging_group and this function does a str_to_imsi(imsi_digit_buf) with the value, which then ends up at libosmocore:gsm0502_calc_paging_group, which gets the IMSI as an uint64_t. If we can feed gsm0502_calc_paging_group() the pgroup value from here directly then we can use an uint16_t. (what makes me wonder a bit is that gsm0502_calc_paging_group() returns an unsigned int but we store the returned paging group in an uint8_t.) https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/851ea54f_2dfab0f6 PS17, Line 294: struct gsm_pcu_if_data_cnf_dt data_cnf_dt; > isn't it a bit weird that we have a cnf_dt but no data_dt? The message in the direction towards the BSC is sent as data_req under the SAPI PCU_IF_SAPI_PCH_DT. For the confirmation we have specific message types. That is the reason why there is no data_dt. File src/bts.h: https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/d48a6ed7_c5eeb3ef PS17, Line 282: /* Use the TLLI directly to handle IMMEDIATE ASSIGNMENT confirmation, otherwise the TLLI is extracted > This is most probably not needed anymore and can be dropped. (see above, we still needed it, but hopefully not very long.) File src/bts.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/63442e9d_6731f25d PS17, Line 1134: pcu_l1if_tx_pch(bts, immediate_assignment, plen, ms_paging_group(tbf_ms(tbf))); > I thought we agreed on dropping the previous message? If we drop it now, then the PCU will become incompatible with osmo-bts. As far as I remember we decided to add a deprecation warning and upgrade osmo-bts later. Its easy to remove the code pathes later so lets not get blocked by the BTS part and keep compatibility for now. File src/pcu_l1_if.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/d78cf3c1_09a75e9d PS17, Line 769: > this can be dropped. I have reworked this part and added the deprecation note. I also have created a ticket for the BTS part now. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31145 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I2a78651593323e8b9627c39918d949a33497b70f Gerrit-Change-Number: 31145 Gerrit-PatchSet: 18 Gerrit-Owner: dexter <pmaier(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-CC: neels <nhofmeyr(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: Tue, 28 Feb 2023 15:47:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 1 month

1
0
0 0

[S] Change in osmo-bsc[master]: pcuif_proto: increment version number

by pespin

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31597 ) Change subject: pcuif_proto: increment version number ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31597 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I8315bd67c7f3eb0d7ee71b64cd4dff889a84fcf1 Gerrit-Change-Number: 31597 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 15:01:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[M] Change in osmo-mgw[master]: mgcp_sdp: add fmtp string to define HR GSM RTP format

by neels

Attention is currently required from: dexter. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/31214 ) Change subject: mgcp_sdp: add fmtp string to define HR GSM RTP format ...................................................................... Patch Set 10: (1 comment) File include/osmocom/mgcp/mgcp_common.h: https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/d64b4f29_c9638175 PS10, Line 71: enum mgcp_gsm_hr_fmt gsm_hr_format; > struct mgcp_codec_param has a fundamental problem: […] https://osmocom.org/issues/5723 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/31214 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Idde8da27fd335dc03b8fbd9e0fedc1491b77e9e4 Gerrit-Change-Number: 31214 Gerrit-PatchSet: 10 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 14:03:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[M] Change in osmo-mgw[master]: mgcp_sdp: add fmtp string to define HR GSM RTP format

by neels

Attention is currently required from: dexter. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/31214 ) Change subject: mgcp_sdp: add fmtp string to define HR GSM RTP format ...................................................................... Patch Set 10: Code-Review-1 (1 comment) File include/osmocom/mgcp/mgcp_common.h: https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/1701ba1a_21068860 PS10, Line 71: enum mgcp_gsm_hr_fmt gsm_hr_format; struct mgcp_codec_param has a fundamental problem: it lives in struct mgcp_msg, i.e. there can *only be one* in an entire SDP message in the osmo_mgcp_client API. However, "a=fmtp:<pt-nr>" are scoped *per payload type nr*, i.e. there should be one for every codec, not only one per SDP message. amr_octet_aligned has this problem, meaning that our MGCP client cannot offer two distinct payload types for AMR octet-aligned and AMR bandwidth-efficient. This recently bit me while working on codec resolution in osmo-msc and osmo-bsc. So far I have to decide for OA or not for *all* AMR codecs represented in our mgcp_client SDP. Let's not proliferate this problem. This probably means we first need to fix this scoping problem before adding new fmtp -- because when you make room for per-pt fmtp, might as well move the OA flag there. (If you instead would want one global flag, it would be a new X-Osmo header; but I think per pt fmtp is the right choice here) -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/31214 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Idde8da27fd335dc03b8fbd9e0fedc1491b77e9e4 Gerrit-Change-Number: 31214 Gerrit-PatchSet: 10 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 14:02:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[M] Change in osmo-bsc[master]: pcu_sock: use struct to transfer IMMEDIATE ASSIGNMENT for PCH

by Jenkins Builder

Attention is currently required from: dexter. Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31578 ) Change subject: pcu_sock: use struct to transfer IMMEDIATE ASSIGNMENT for PCH ...................................................................... Patch Set 3: (1 comment) File src/osmo-bsc/pcu_sock.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-4178): https://gerrit.osmocom.org/c/osmo-bsc/+/31578/comment/2804b4b8_1cc7ba13 PS3, Line 575: pch_dt = (struct gsm_pcu_if_pch_dt*)data_req->data; "(foo*)" should be "(foo *)" -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31578 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Id6acbd243adf26169e5e8319dd66bb68dd6a3c22 Gerrit-Change-Number: 31578 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:58:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[S] Change in osmo-bsc[master]: pcuif_proto: increment version number

by dexter

dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31597 ) Change subject: pcuif_proto: increment version number ...................................................................... pcuif_proto: increment version number The co-located PCU support for Ericsson RBS E1 CCU made it necessary to add new features to the PCU socket interface, so lets increase the version number. Change-Id: I8315bd67c7f3eb0d7ee71b64cd4dff889a84fcf1 Related: OS#5198 --- M include/osmocom/bsc/pcuif_proto.h 1 file changed, 15 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/97/31597/1 diff --git a/include/osmocom/bsc/pcuif_proto.h b/include/osmocom/bsc/pcuif_proto.h index 1d12f50..3265dfa 100644 --- a/include/osmocom/bsc/pcuif_proto.h +++ b/include/osmocom/bsc/pcuif_proto.h @@ -7,7 +7,7 @@ #define PCU_SOCK_DEFAULT "/tmp/pcu_bts" -#define PCU_IF_VERSION 0x0a +#define PCU_IF_VERSION 0x0b #define TXT_MAX_LEN 128 /* msg_type */ -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31597 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I8315bd67c7f3eb0d7ee71b64cd4dff889a84fcf1 Gerrit-Change-Number: 31597 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 2 months

1
0
0 0

[S] Change in osmo-ttcn3-hacks[master]: pgw: stop components early if test fails

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/31596 ) Change subject: pgw: stop components early if test fails ...................................................................... pgw: stop components early if test fails Change-Id: I3465a421a4b2a3fce72a1c54037d8dc2b4e3a0fe --- M pgw/PGW_Tests.ttcn 1 file changed, 27 insertions(+), 10 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/96/31596/1 diff --git a/pgw/PGW_Tests.ttcn b/pgw/PGW_Tests.ttcn index 8a25ebf..adae0a4 100644 --- a/pgw/PGW_Tests.ttcn +++ b/pgw/PGW_Tests.ttcn @@ -632,17 +632,20 @@ process_bctx_create(bctx); } case ('0001'B) { // removed - setverdict(fail, "We don't expect removed bearer contexts yet"); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "We don't expect removed bearer contexts yet"); } } } } [] GTP2.receive(tr_GTP2C_CreateSessionResp(d_teid:=g_teic, cause:=?)) -> value rx { - setverdict(fail, "Unexpected CreateSessionResp(cause=", - rx.gtpcv2_pdu.createSessionResponse.cause.causeValue, ")"); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + log2str("Unexpected CreateSessionResp(cause=", + rx.gtpcv2_pdu.createSessionResponse.cause.causeValue, ")")); } [] GTP2.receive { - setverdict(fail, "Unexpected GTPv2 while waiting for CreateSessionResp"); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "Unexpected GTPv2 while waiting for CreateSessionResp"); } } @@ -673,10 +676,12 @@ setverdict(pass); } [] GTP2.receive(tr_GTP2C_DeleteSessionResp(?, ?)) { - setverdict(fail, "Unexpected DeleteSessionResp"); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "Unexpected DeleteSessionResp"); } [] GTP2.receive { - setverdict(fail, "Unexpected GTPv2 while waiting for DeleteSessionResp"); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "Unexpected GTPv2 while waiting for DeleteSessionResp"); } } @@ -712,7 +717,8 @@ log("Starting a program: ", command); var UECUPS_StartProgramRes res := f_gtp2_start_program(sprog); if (res.result != OK) { - setverdict(fail, "Unable to start program '", command, "'"); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + log2str("Unable to start program '", command, "'")); } return res.pid; } @@ -730,10 +736,12 @@ setverdict(pass); } [] GTP2.receive(UECUPS_ProgramTermInd:?) -> value pti { - setverdict(fail, "Received unexpected ProgramTermInd := ", pti); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + log2str("Received unexpected ProgramTermInd := ", pti)); } [] T.timeout { - setverdict(fail, "timeout waiting for user-plane program termination"); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "timeout waiting for user-plane program termination"); } } } @@ -778,7 +786,7 @@ setverdict(pass); } [] T.timeout { - setverdict(fail, "timeout waiting for Echo Response"); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, "timeout waiting for Echo Response"); } } } -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/31596 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: I3465a421a4b2a3fce72a1c54037d8dc2b4e3a0fe Gerrit-Change-Number: 31596 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 2 months

1
0
0 0

[M] Change in osmo-bsc[master]: pcu_sock: activate/deactivate PDCH on pcu reconnect

by dexter

Attention is currently required from: neels, fixeria, dexter. Hello Jenkins Builder, neels, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bsc/+/31497 to look at the new patch set (#5). Change subject: pcu_sock: activate/deactivate PDCH on pcu reconnect ...................................................................... pcu_sock: activate/deactivate PDCH on pcu reconnect When the PCU is disconnected while the BSC keeps running the PDCH should be closed. Also the PDCH should be reopened when the PCU is reconnected. Change-Id: I9ea0c53a5e68a51c781ef43bae71f947cdb95678 Related: OS#5198 --- M doc/timeslot.msc M include/osmocom/bsc/timeslot_fsm.h M src/osmo-bsc/pcu_sock.c M src/osmo-bsc/timeslot_fsm.c 4 files changed, 112 insertions(+), 18 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/97/31497/5 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31497 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I9ea0c53a5e68a51c781ef43bae71f947cdb95678 Gerrit-Change-Number: 31497 Gerrit-PatchSet: 5 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 year, 2 months

1
0
0 0

[M] Change in osmo-mgw[master]: mgcp_sdp: add fmtp string to define HR GSM RTP format

by pespin

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/31214 ) Change subject: mgcp_sdp: add fmtp string to define HR GSM RTP format ...................................................................... Patch Set 10: (4 comments) File src/libosmo-mgcp/mgcp_network.c: https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/b7f7d957_a0203c85 PS10, Line 705: case GSM_HR_BYTES + sizeof(struct rtp_hdr): if possible order the fields left to right based on the order you encounter them in the pkt. https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/94877e38_ee6f532d PS10, Line 741: bool convert = false; can be dropped. https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/59257e91_d17bf92c PS10, Line 746: convert = true; return true; https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/8fa69cd4_49258296 PS10, Line 750: convert = true; return true; -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/31214 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Idde8da27fd335dc03b8fbd9e0fedc1491b77e9e4 Gerrit-Change-Number: 31214 Gerrit-PatchSet: 10 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:36:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[S] Change in osmo-mgw[master]: mgcp_client: fix sourcecode formatting

by dexter

dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/31406 ) ( 5 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: mgcp_client: fix sourcecode formatting ...................................................................... mgcp_client: fix sourcecode formatting Change-Id: I55f98379ea48ffddc25186c15e4dd40762e818b6 --- M src/libosmo-mgcp-client/mgcp_client.c 1 file changed, 11 insertions(+), 5 deletions(-) Approvals: pespin: Looks good to me, but someone else must approve fixeria: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c index 0434e70..81847ee 100644 --- a/src/libosmo-mgcp-client/mgcp_client.c +++ b/src/libosmo-mgcp-client/mgcp_client.c @@ -1006,11 +1006,8 @@ return epname; } -struct mgcp_response_pending * mgcp_client_pending_add( - struct mgcp_client *mgcp, - mgcp_trans_id_t trans_id, - mgcp_response_cb_t response_cb, - void *priv) +struct mgcp_response_pending *mgcp_client_pending_add(struct mgcp_client *mgcp, mgcp_trans_id_t trans_id, + mgcp_response_cb_t response_cb, void *priv) { struct mgcp_response_pending *pending; -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/31406 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I55f98379ea48ffddc25186c15e4dd40762e818b6 Gerrit-Change-Number: 31406 Gerrit-PatchSet: 7 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 year, 2 months

1
0
0 0

[S] Change in libosmocore[master]: bts_features: Add features for HR formats (TS 101813 vs. RFC5993)

by pespin

Attention is currently required from: neels, fixeria, msuraev, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31415 ) Change subject: bts_features: Add features for HR formats (TS 101813 vs. RFC5993) ...................................................................... Patch Set 8: (1 comment) File include/osmocom/gsm/bts_features.h: https://gerrit.osmocom.org/c/libosmocore/+/31415/comment/dfc81cd8_142911f5 PS7, Line 39: _TX > In a correctly configured setup we should always see the same format in both directions. If you say so... I don't think that's really stated anywhere in the spec, and sounds like a personal expectancy of what a "correctly configured setup" is imho 😊 > I thought the OML feature flags were introduced for things like this Yes, to know that a BTS supports a given feature. Not to indicate which of the several supported is to be used. > BTS model is a PHY based model with TS 101318 support or an osmo-trx based model with RFC 5993 support. So our osmo-bts-sysmo is transmitting different RTP type than our osmo-bts-trx? what a mess! 😮 > The BSC knows that it talks to an osmo-bts, but it does not know if this BTS model My point is only that you still need to store that in osmo-bsc hardcoded somehow for other types of BTS, like nanobts, etc. so you could simply do the same for osmobts (you will need this for older versions of osmo-bts anyway). Maybe that's actually done using this set of features? > BTS model is a PHY based model with TS 101318 support or an osmo-trx based model with RFC 5993 support. So our osmo-bts-sysmo is transmitting different RTP type than our osmo-bts-trx? what a mess! :O > Extending IPACC can be also a way, here we can at least be sure that osmo-bts type BTSs will support the extension but we will have to make sure that older osmo-bts versions will tolerate/ignore the extensions. Yes, have a look if adding the new IE is really a problem for old versions (I think it won't be an issue). If it was, then you could add a new BTS feature stating whether the BTS supports this new IE and then only send it based on it. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31415 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I05e4ed7a85f3a0451de7cd07380503a7ac76d043 Gerrit-Change-Number: 31415 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(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-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:33:25 +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: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[M] Change in osmo-mgw[master]: mgcp_sdp: add fmtp string to define HR GSM RTP format

by dexter

Attention is currently required from: pespin, dexter. Hello Jenkins Builder, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/31214 to look at the new patch set (#10). Change subject: mgcp_sdp: add fmtp string to define HR GSM RTP format ...................................................................... mgcp_sdp: add fmtp string to define HR GSM RTP format There are two different RTP HR GSM formats defined (TS 101.318 and RFC5993). Lets add an fmtp parameter In order to select between the two formats (and convert if necessary) via MGCP/SDP. Unfortunately there is no official fmtp string defined, so we have to define an osmocom specific string. Change-Id: Idde8da27fd335dc03b8fbd9e0fedc1491b77e9e4 Related: OS#5688 --- M include/osmocom/mgcp/mgcp_common.h M src/libosmo-mgcp-client/mgcp_client.c M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_sdp.c 4 files changed, 102 insertions(+), 17 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/14/31214/10 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/31214 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Idde8da27fd335dc03b8fbd9e0fedc1491b77e9e4 Gerrit-Change-Number: 31214 Gerrit-PatchSet: 10 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 year, 2 months

1
0
0 0

[S] Change in libosmocore[master]: osmo_wqueue_enqueue(): Avoid redundant if-check

by pespin

Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31591 ) Change subject: osmo_wqueue_enqueue(): Avoid redundant if-check ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: I'm not sure it's really worth it and it's clearer the way it is now. So I vote for not merging this. If enough think differently then I won't block. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31591 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3ecc1698e836ca40e178a5b84c2946ae0e6bbfc9 Gerrit-Change-Number: 31591 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:23:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[M] Change in osmo-mgw[master]: mgcp_sdp: add fmtp string to define HR GSM RTP format

by Jenkins Builder

Attention is currently required from: pespin, dexter. Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/31214 ) Change subject: mgcp_sdp: add fmtp string to define HR GSM RTP format ...................................................................... Patch Set 9: (4 comments) File src/libosmo-mgcp/mgcp_network.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-4169): https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/00e89359_c2fc46e4 PS9, Line 704: switch(msgb_length(msg)) that open brace { should be on the previous line Robot Comment from checkpatch (run ID jenkins-gerrit-lint-4169): https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/dc1b001a_398a1359 PS9, Line 704: switch(msgb_length(msg)) space required before the open parenthesis '(' Robot Comment from checkpatch (run ID jenkins-gerrit-lint-4169): https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/cb2a49a9_6640a804 PS9, Line 706: case GSM_HR_BYTES + sizeof(struct rtp_hdr): code indent should use tabs where possible Robot Comment from checkpatch (run ID jenkins-gerrit-lint-4169): https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/89d6e140_9512b368 PS9, Line 706: case GSM_HR_BYTES + sizeof(struct rtp_hdr): please, no spaces at the start of a line -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/31214 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Idde8da27fd335dc03b8fbd9e0fedc1491b77e9e4 Gerrit-Change-Number: 31214 Gerrit-PatchSet: 9 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:21:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[M] Change in osmo-bts[master]: common: Have PCU socket connection use osmo_wqueue

by pespin

Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31533 ) Change subject: common: Have PCU socket connection use osmo_wqueue ...................................................................... Patch Set 3: Code-Review-1 (3 comments) File src/common/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/71841978_da3ed4aa PS2, Line 1092: return -1; > It doesn't change the control flow afaik (I looked at `osmo_wqueue_bfd_cb()`). […] osmo_wqueue_bfd_cb: if (rc == -EBADF) goto err_badfd; You have to return -EBADF while handling the socket fd if you close/free it, otherwise osmo_wqueue_bfd_cb() continues using it and potentially calling the write_cb for a fd/mem which is no longer existing which can cause problems. https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/1c4ccd75_92870a87 PS2, Line 1102: osmo_fd_write_disable(&state->upqueue.bfd); > From my understanding/by looking at this code, it looks like `osmo_fd_write_disable()` is meant to p […] that's not to prevent concurrent write-access. It's to manage the main loop poll() to tell the kernel to trigger us when the socket is available for writing again. https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/be99ebb8_52a56719 PS2, Line 1181: osmo_wqueue_init(&state->upqueue, PCU_SOCK_QLENGTH_MAX_DEFAULT); > No it's not possible, with the current code the current length of the wqueue is always incremented/t […] I'm fine with whatever but use WQUEUE instead of the QLENGTH stuff :) -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 Gerrit-Change-Number: 31533 Gerrit-PatchSet: 3 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: Tue, 28 Feb 2023 13:21:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[M] Change in osmo-bts[master]: common: Have PCU socket connection use osmo_wqueue

by pespin

Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31533 ) Change subject: common: Have PCU socket connection use osmo_wqueue ...................................................................... Patch Set 3: (1 comment) File src/common/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/10227697_0c0939c4 PS3, Line 993: LOGP(DLGLOBAL, LOGL_NOTICE, "PCU not reacting (more thatn %u messages waiting). Closing connection\n", typo: than -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 Gerrit-Change-Number: 31533 Gerrit-PatchSet: 3 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: Tue, 28 Feb 2023 13:17:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[S] Change in libosmocore[master]: bts_features: Add features for HR formats (TS 101813 vs. RFC5993)

by dexter

Attention is currently required from: neels, pespin, fixeria, msuraev. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31415 ) Change subject: bts_features: Add features for HR formats (TS 101813 vs. RFC5993) ...................................................................... Patch Set 8: (1 comment) File include/osmocom/gsm/bts_features.h: https://gerrit.osmocom.org/c/libosmocore/+/31415/comment/379b4fa6_dda6ba66 PS7, Line 39: _TX > A couple of extra points here: […] @neels: You are correct. Thats how it works. (In the DSP case the RTP packets are going through l1sap.c as well, but the DSP is generating the payloads.) @pespin: In a correctly configured setup we should always see the same format in both directions. I also can understand the objections against the "_TX_", so I am fine with dropping that again. I thought the OML feature flags were introduced for things like this. I also see no other way to find out which BTS model supports which format. The BSC knows that it talks to an osmo-bts, but it does not know if this BTS model is a PHY based model with TS 101318 support or an osmo-trx based model with RFC 5993 support. Extending IPACC can be also a way, here we can at least be sure that osmo-bts type BTSs will support the extension but we will have to make sure that older osmo-bts versions will tolerate/ignore the extensions. To me this sounds difficult and since the implementation in osmo-mgw is already done I would opt for the OML flag based method. Please let me know what you think. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31415 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I05e4ed7a85f3a0451de7cd07380503a7ac76d043 Gerrit-Change-Number: 31415 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(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-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:16:57 +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: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[M] Change in osmo-bts[master]: common: Have PCU socket connection use osmo_wqueue

by pespin

Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31533 ) Change subject: common: Have PCU socket connection use osmo_wqueue ...................................................................... Patch Set 3: (1 comment) File include/osmo-bts/pcuif_proto.h: https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/80644552_54bd56c5 PS2, Line 8: #define PCU_SOCK_QLENGTH_MAX_DEFAULT 10 > I would say this is rather consistent with https://gerrit.osmocom. […] Yes because that path is used by both ends to connect. It would be like defining a default TCP port here. But this queue length is something really not related to what both ends use to communicate, so it doesn't belong here. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 Gerrit-Change-Number: 31533 Gerrit-PatchSet: 3 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: Tue, 28 Feb 2023 13:16:24 +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 year, 2 months

1
0
0 0

[S] Change in osmo-bsc[master]: bssmap_handle_ass_req_tp_codec_list: tweak log msg

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31550 ) Change subject: bssmap_handle_ass_req_tp_codec_list: tweak log msg ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31550 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Idfab0b7f6e97a6b67d140f967ddfe9b29818586e Gerrit-Change-Number: 31550 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:06:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[M] Change in osmo-bsc[master]: rsl_tx_ipacc_crcx/mdcx: omit speech mode for CSD

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31549 ) Change subject: rsl_tx_ipacc_crcx/mdcx: omit speech mode for CSD ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31549 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I6641b713177276bcf798f08123e1dd2e88ffdce6 Gerrit-Change-Number: 31549 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:06:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[M] Change in osmo-bsc[master]: bssmap_handle_ass_req_ct_data: implement

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31547 ) Change subject: bssmap_handle_ass_req_ct_data: implement ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31547 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I350bea15fd2158eb6edc9bc92f2dca48930736e9 Gerrit-Change-Number: 31547 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:05:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[M] Change in osmo-bsc[master]: codec_pref: split test_codec_support_bts_rate

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31544 ) Change subject: codec_pref: split test_codec_support_bts_rate ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31544 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Iea8a23ef3c66ed556110038fe9f3bc7f6eef3e96 Gerrit-Change-Number: 31544 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:05:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[M] Change in osmo-bsc[master]: bssmap_handle_ass_req_ct_speech: refactor

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31543 ) Change subject: bssmap_handle_ass_req_ct_speech: refactor ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31543 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I6bb16b26d89056dffa50bd8296fefe9315c587ca Gerrit-Change-Number: 31543 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:04:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[M] Change in osmo-bts[master]: common: Have PCU socket connection use osmo_wqueue

by arehbein

Attention is currently required from: pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31533 ) Change subject: common: Have PCU socket connection use osmo_wqueue ...................................................................... Patch Set 2: (7 comments) File include/osmo-bts/pcuif_proto.h: https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/e798b3f5_d70a3eaa PS2, Line 8: #define PCU_SOCK_QLENGTH_MAX_DEFAULT 10 > that's application/peer implementation specific, not really part of the shared protocol and hence sh […] I would say this is rather consistent with https://gerrit.osmocom.org/c/osmo-bts/+/6981 , which put PCU_SOCK_DEFAULT there in the first place. File src/common/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/d906cb23_9a0cfe37 PS2, Line 988: if (osmo_wqueue_enqueue(&state->upqueue, msg) == -ENOSPC) { > Simply check for any error "< 0" Done https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/c90c5a50_b825bf92 PS2, Line 989: LOGP(DLGLOBAL, LOGL_NOTICE, "PCU not reacting (more thatn %d messages waiting). Closing connection\n", > max_length is an int? or an unsigned? Yeah it's a bit confusing because `osmo_wqueue_init()` takes a regular `int` for I suppose historical reasons (I pushed a patch for that), so that's what I remembered last. Format specifier should be fixed now https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/10ff445f_2b2a210e PS2, Line 992: } else { > Usually we do early return on error, then drop this "else" block. Done https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/79fa212c_7c519e51 PS2, Line 1092: return -1; > In heere probably -EBADF needs to be returned so that the wqueue potentially calls the write_cb() wi […] It doesn't change the control flow afaik (I looked at `osmo_wqueue_bfd_cb()`). But I could check on the errno stuff in these callbacks and improve on previously existing code https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/d1139b96_f34845bf PS2, Line 1102: osmo_fd_write_disable(&state->upqueue.bfd); > This is wrong. You should disable the bfd after successful write() only if the queue is empty. From my understanding/by looking at this code, it looks like `osmo_fd_write_disable()` is meant to prevent concurrent fd write-access via osmo-wqueue code; with that in mind it would make sense to disable before write. What has changed? I didn't change the logic of this function. https://gerrit.osmocom.org/c/osmo-bts/+/31533/comment/242b7e4a_e343f70f PS2, Line 1181: osmo_wqueue_init(&state->upqueue, PCU_SOCK_QLENGTH_MAX_DEFAULT); > I would split this into 2 commits if possible: […] No it's not possible, with the current code the current length of the wqueue is always incremented/there's always a length check. I find `PCU_SOCK_WQUEUE_LEN` misleading, because I would say the queue length is a property subject to change, so maybe `PCU_SOCK_WQUEUE_MAX_LEN` (I would also prefer not omitting the `_DEFAULT` suffix). Or maybe `PCU_SOCK_WQ_LEN_MAX_DEFAULT` ? I don't find `WQ` that absurd considering it's used right around `osmo_wqueue`-identifiers. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 Gerrit-Change-Number: 31533 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 12:54:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[M] Change in osmo-bts[master]: common: Have PCU socket connection use osmo_wqueue

by arehbein

Attention is currently required from: arehbein. Hello Jenkins Builder, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/31533 to look at the new patch set (#3). Change subject: common: Have PCU socket connection use osmo_wqueue ...................................................................... common: Have PCU socket connection use osmo_wqueue Fixes memleak in case of connected PCU process being suspended without proper close on socket Related: OS#5774 Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 --- M include/osmo-bts/pcuif_proto.h M src/common/pcu_sock.c 2 files changed, 59 insertions(+), 67 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/33/31533/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 Gerrit-Change-Number: 31533 Gerrit-PatchSet: 3 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-MessageType: newpatchset

1 year, 2 months

1
0
0 0

[S] Change in libosmocore[master]: osmo_wqueue_init(): Fix parameter type

by pespin

Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31595 ) Change subject: osmo_wqueue_init(): Fix parameter type ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31595 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I6c9295750e1755bf1d80f3a3b32efc2aefb11a57 Gerrit-Change-Number: 31595 Gerrit-PatchSet: 1 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: Tue, 28 Feb 2023 12:36:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 2 months

1
0
0 0

[S] Change in libosmocore[master]: osmo_wqueue_init(): Fix parameter type

by arehbein

arehbein has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/31595 ) Change subject: osmo_wqueue_init(): Fix parameter type ...................................................................... osmo_wqueue_init(): Fix parameter type The field `max_length` of `struct osmo_wqueue` is of type `unsigned int`; the function body doesn't show any reason why we should accept `int` as a parameter Change-Id: I6c9295750e1755bf1d80f3a3b32efc2aefb11a57 --- M include/osmocom/core/write_queue.h M src/core/write_queue.c 2 files changed, 13 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/95/31595/1 diff --git a/include/osmocom/core/write_queue.h b/include/osmocom/core/write_queue.h index 6cb0a6b..86f2ca9 100644 --- a/include/osmocom/core/write_queue.h +++ b/include/osmocom/core/write_queue.h @@ -46,7 +46,7 @@ int (*except_cb)(struct osmo_fd *fd); }; -void osmo_wqueue_init(struct osmo_wqueue *queue, int max_length); +void osmo_wqueue_init(struct osmo_wqueue *queue, unsigned int max_length); void osmo_wqueue_clear(struct osmo_wqueue *queue); int osmo_wqueue_enqueue(struct osmo_wqueue *queue, struct msgb *data); int osmo_wqueue_enqueue_quiet(struct osmo_wqueue *queue, struct msgb *data); diff --git a/src/core/write_queue.c b/src/core/write_queue.c index 884cebd..2c5a85b 100644 --- a/src/core/write_queue.c +++ b/src/core/write_queue.c @@ -88,7 +88,7 @@ * \param[in] queue Write queue to operate on * \param[in] max_length Maximum length of write queue */ -void osmo_wqueue_init(struct osmo_wqueue *queue, int max_length) +void osmo_wqueue_init(struct osmo_wqueue *queue, unsigned int max_length) { queue->max_length = max_length; queue->current_length = 0; -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31595 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I6c9295750e1755bf1d80f3a3b32efc2aefb11a57 Gerrit-Change-Number: 31595 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 2 months

1
0
0 0

2024

2023

2022

gerrit-log February 2023