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

[M] Change in osmo-bts[master]: bts-{sysmo,oc2g,lc15}: Fix RTP of AMR SID_FIRST_P1
by laforge 28 Mar '23

28 Mar '23

laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32088 ) Change subject: bts-{sysmo,oc2g,lc15}: Fix RTP of AMR SID_FIRST_P1 ...................................................................... Patch Set 4: Verified+1 Code-Review+2 (1 comment) Patchset: PS4: verified in a pcap file, it now does what it's supposed to do. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32088 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I90479efcc002d497648a71e73847af54e6208358 Gerrit-Change-Number: 32088 Gerrit-PatchSet: 4 Gerrit-Owner: laforge <laforge(a)osmocom.org> 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-Comment-Date: Tue, 28 Mar 2023 15:23:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: common/vty: Print AMR MultiRate Configuration in "show lchan"
by fixeria 28 Mar '23

28 Mar '23

Attention is currently required from: laforge. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32091 ) Change subject: common/vty: Print AMR MultiRate Configuration in "show lchan" ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32091 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I630af8c3835c2fcbea325c07db269d25e4e18f62 Gerrit-Change-Number: 32091 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Tue, 28 Mar 2023 14:01:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in gapk[master]: fmt_rtp_amr,fmt_rtp_efr: replace damaged packets with silence
by matanp 28 Mar '23

28 Mar '23

Attention is currently required from: pespin. matanp has posted comments on this change. ( https://gerrit.osmocom.org/c/gapk/+/32094 ) Change subject: fmt_rtp_amr,fmt_rtp_efr: replace damaged packets with silence ...................................................................... Patch Set 3: (3 comments) File src/fmt_rtp_amr.c: https://gerrit.osmocom.org/c/gapk/+/32094/comment/5e4eeea0_d1879047 PS2, Line 30: static const uint8_t SILENCE[] = {0x80, 0x44, 0x00, 0x00, 0x00, 0x00, 0x00}; > can you add a some comment with some spec reference as from where this hexstring comes from? Ack File src/fmt_rtp_efr.c: https://gerrit.osmocom.org/c/gapk/+/32094/comment/81820468_713af606 PS2, Line 33: static const uint8_t DAMAGED_PACKET[EFR_LEN] = { > Same here. Ack https://gerrit.osmocom.org/c/gapk/+/32094/comment/2f0f3bfc_a952cfe2 PS2, Line 70: if (!memcmp(DAMAGED_PACKET, src, EFR_LEN)) > is EFR_LEN garanteed to be == src_len here? Yes, the if statement in line 65 checks it -- To view, visit https://gerrit.osmocom.org/c/gapk/+/32094 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: gapk Gerrit-Branch: master Gerrit-Change-Id: I7245aa0bc0955cc8b94d5401a15e694f50498093 Gerrit-Change-Number: 32094 Gerrit-PatchSet: 3 Gerrit-Owner: matanp <matan1008(a)gmail.com> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Mar 2023 13:57:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in libosmo-gprs[master]: gmm: Several fixes to GMMSM prim alloc functions
by pespin 28 Mar '23

28 Mar '23

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-gprs/+/32101 to look at the new patch set (#2). Change subject: gmm: Several fixes to GMMSM prim alloc functions ...................................................................... gmm: Several fixes to GMMSM prim alloc functions Change-Id: Ie49cb448806101a24c58a85f7073e13baccada5b --- M include/osmocom/gprs/gmm/gmm_prim.h M src/gmm/gmm_prim.c 2 files changed, 17 insertions(+), 8 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-gprs refs/changes/01/32101/2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/32101 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: Ie49cb448806101a24c58a85f7073e13baccada5b Gerrit-Change-Number: 32101 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

1 0

[S] Change in gapk[master]: fmt_rtp_amr,fmt_rtp_efr: replace damaged packets with silence
by pespin 28 Mar '23

28 Mar '23

Attention is currently required from: matanp. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/gapk/+/32094 ) Change subject: fmt_rtp_amr,fmt_rtp_efr: replace damaged packets with silence ...................................................................... Patch Set 3: (1 comment) File src/fmt_rtp_efr.c: https://gerrit.osmocom.org/c/gapk/+/32094/comment/ec56af19_a7992598 PS2, Line 70: if (!memcmp(DAMAGED_PACKET, src, EFR_LEN)) > is EFR_LEN garanteed to be == src_len here? Did you verify this already? maybe add an OSMO_ASSERT(src_len == EFR_LEN)? -- To view, visit https://gerrit.osmocom.org/c/gapk/+/32094 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: gapk Gerrit-Branch: master Gerrit-Change-Id: I7245aa0bc0955cc8b94d5401a15e694f50498093 Gerrit-Change-Number: 32094 Gerrit-PatchSet: 3 Gerrit-Owner: matanp <matan1008(a)gmail.com> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: matanp <matan1008(a)gmail.com> Gerrit-Comment-Date: Tue, 28 Mar 2023 13:46:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[L] Change in osmo-bts[master]: Replace explicit gsm_lchan_name() calls with LOGPLCHAN
by Jenkins Builder 28 Mar '23

28 Mar '23

Attention is currently required from: laforge. Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32093 ) Change subject: Replace explicit gsm_lchan_name() calls with LOGPLCHAN ...................................................................... Patch Set 2: (4 comments) File src/common/rsl.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-5518): https://gerrit.osmocom.org/c/osmo-bts/+/32093/comment/3096b769_87c64101 PS2, Line 3298: gsm_pchan_name(ts->pchan), pdch_act? "PDCH" : "TCH/F", ts->flags); spaces required around that '?' (ctx:VxW) File src/osmo-bts-lc15/oml.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-5518): https://gerrit.osmocom.org/c/osmo-bts/+/32093/comment/50b391c0_84c50973 PS2, Line 937: LOGPLCHAN(lchan, DL1C, LOGL_INFO, "%s tch_mode=0x%02x\n", __FUNCTION__, lchan->tch_mode); __func__ should be used instead of gcc specific __FUNCTION__ File src/osmo-bts-oc2g/oml.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-5518): https://gerrit.osmocom.org/c/osmo-bts/+/32093/comment/38cfaac0_3ca25dd9 PS2, Line 952: LOGPLCHAN(lchan, DL1C, LOGL_INFO, "%s tch_mode=0x%02x\n", __FUNCTION__, lchan->tch_mode); __func__ should be used instead of gcc specific __FUNCTION__ File src/osmo-bts-sysmo/oml.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-5518): https://gerrit.osmocom.org/c/osmo-bts/+/32093/comment/fd999cc7_96d926cd PS2, Line 945: __FUNCTION__, lchan->tch_mode); __func__ should be used instead of gcc specific __FUNCTION__ -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32093 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: If4f4f555f5ca61dfa624b298805f5375efc0b137 Gerrit-Change-Number: 32093 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Tue, 28 Mar 2023 13:43:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: bts-{sysmo,oc2g,lc15}: Dump logical channel params during MPH-ACTIVAT...
by laforge 28 Mar '23

28 Mar '23

Attention is currently required from: laforge. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/32092 to look at the new patch set (#2). Change subject: bts-{sysmo,oc2g,lc15}: Dump logical channel params during MPH-ACTIVATE.req ...................................................................... bts-{sysmo,oc2g,lc15}: Dump logical channel params during MPH-ACTIVATE.req So far we only printed it during later modification. Let's print it also during initial activation of a logical channel. Change-Id: I6982a52905e4719e2e9c40630252ffef2ff9fbed --- M src/osmo-bts-lc15/oml.c M src/osmo-bts-oc2g/oml.c M src/osmo-bts-sysmo/oml.c 3 files changed, 21 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/92/32092/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32092 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I6982a52905e4719e2e9c40630252ffef2ff9fbed Gerrit-Change-Number: 32092 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in osmo-bts[master]: bts-{sysmo,oc2g,lc15}: Fix RTP of AMR SID_FIRST_P1
by laforge 28 Mar '23

28 Mar '23

Hello Jenkins Builder, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/32088 to look at the new patch set (#4). Change subject: bts-{sysmo,oc2g,lc15}: Fix RTP of AMR SID_FIRST_P1 ...................................................................... bts-{sysmo,oc2g,lc15}: Fix RTP of AMR SID_FIRST_P1 When we receive a SID_FIRST_P1 frame from the PHY (during AMR/HR DTXu), we must generate a SID frame on the RTP side. The existing code * ignored that the Amr_SidFirstP1 PHYIF message actually contains the RTP payload * manually generated the same content using osmo_amr_rtp_enc() * forgot to prefix that with the AMR CMR+TOC * in the end, sent a completely broken (too short) AMR SID frame over RTP Let's fix this by simply using the 7-byte RTP payload with CMR+TOC that the PHY is providing to us. Change-Id: I90479efcc002d497648a71e73847af54e6208358 Related: OS#5944 --- M src/osmo-bts-lc15/tch.c M src/osmo-bts-oc2g/tch.c M src/osmo-bts-sysmo/tch.c 3 files changed, 29 insertions(+), 24 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/88/32088/4 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32088 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I90479efcc002d497648a71e73847af54e6208358 Gerrit-Change-Number: 32088 Gerrit-PatchSet: 4 Gerrit-Owner: laforge <laforge(a)osmocom.org> 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-MessageType: newpatchset

1 0

[S] Change in osmo-bts[master]: common/vty: Print AMR MultiRate Configuration in "show lchan"
by laforge 28 Mar '23

28 Mar '23

Attention is currently required from: laforge. Hello Jenkins Builder, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/32091 to look at the new patch set (#2). Change subject: common/vty: Print AMR MultiRate Configuration in "show lchan" ...................................................................... common/vty: Print AMR MultiRate Configuration in "show lchan" It can be useful to introspect the current active AMR mode set. Related: OS#5944 Change-Id: I630af8c3835c2fcbea325c07db269d25e4e18f62 --- M src/common/vty.c M src/osmo-bts-virtual/Makefile.am 2 files changed, 28 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/91/32091/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32091 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I630af8c3835c2fcbea325c07db269d25e4e18f62 Gerrit-Change-Number: 32091 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-bts[master]: lc15: fix compiler warning about wrong indent
by laforge 28 Mar '23

28 Mar '23

laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/32103 ) Change subject: lc15: fix compiler warning about wrong indent ...................................................................... lc15: fix compiler warning about wrong indent l1_if.c: In function ‘activate_rf_compl_cb’: l1_if.c:1280:17: error: this ‘if’ clause does not guard... [-Werror=misleading-indentation] 1280 | if (bts_lc15->led_ctrl_mode == LC15_LED_CONTROL_BTS) | ^~ In file included from ../../include/osmo-bts/dtx_dl_amr_fsm.h:3, from ../../include/osmo-bts/msg_utils.h:8, from l1_if.c:55: l1_if.c:1282:25: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’ 1282 | osmo_fsm_inst_dispatch(trx->mo.fi, NM_EV_DISABLE, NULL); | ^~~~~~~~~~~~~~~~~~~~~~ Change-Id: I00ae3faf0f85fecf6e15e71dff071165725e547c --- M src/osmo-bts-lc15/l1_if.c 1 file changed, 22 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/03/32103/1 diff --git a/src/osmo-bts-lc15/l1_if.c b/src/osmo-bts-lc15/l1_if.c index 2955440..7af3be6 100644 --- a/src/osmo-bts-lc15/l1_if.c +++ b/src/osmo-bts-lc15/l1_if.c @@ -1279,8 +1279,8 @@ } else { if (bts_lc15->led_ctrl_mode == LC15_LED_CONTROL_BTS) bts_update_status(BTS_STATUS_RF_ACTIVE, 0); - osmo_fsm_inst_dispatch(trx->mo.fi, NM_EV_DISABLE, NULL); - osmo_fsm_inst_dispatch(trx->bb_transc.mo.fi, NM_EV_DISABLE, NULL); + osmo_fsm_inst_dispatch(trx->mo.fi, NM_EV_DISABLE, NULL); + osmo_fsm_inst_dispatch(trx->bb_transc.mo.fi, NM_EV_DISABLE, NULL); } msgb_free(resp); -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32103 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I00ae3faf0f85fecf6e15e71dff071165725e547c Gerrit-Change-Number: 32103 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newchange

1 0