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

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

by dexter

Attention is currently required from: neels. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/32218 ) Change subject: mgcp_codec: fix codec decision ...................................................................... Patch Set 9: (7 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/86779cf9_6563eada PS8, Line 12: againt > agent? Done Patchset: PS8: > I'm having difficulties reviewing: I need to manually figure out whether mgcp_codec_decide() has cha […] Done File src/libosmo-mgcp/mgcp_codec.c: https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/3d48ff3e_6e75854a PS8, Line 386: codecs_assigned = rtp_end->codecs_assigned; : OSMO_ASS > (lets put these two lines inside the 'if { ... […] Done https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/d77ee675_e934961c PS8, Line 386: codecs_assigned = rtp_end->codecs_assigned; : OSMO_ASS > (lets put these two lines inside the 'if { ... […] Done https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/853f3cf1_5fef3d64 PS8, Line 400: e > (add a comma "in case of foo, barize") Done https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/11618c55_aaf10a76 PS8, Line 401: * a tentative decision is made. > i fail to understand: […] 1) The decision is made for both directions and each side keeps the result in end.codec. The main idea is to make a conscious codec decision (which still may be changed though) once and then convert (if necessary) all packets according to this decision. This also means that we can check if the packets we receive match our decision and toss the ones we do not expect. Also more importantly we also always know into what format/codec we have to convert before we emit the packets. 2) "connection" does not refer so much for RTP, refers to the "conn" we create and manage here. Destination endpoint is also wrong since both "connections" live on one endpoint. RFC3435 also uses the term "connection". 3) When the connections are created, they are created one after another, so when the first connection is created it will have no partner or destination connection. But still we must make an initial codec choice. This is in particular important when the first connection is created in loopback mode. When the second connection is created, then the mgcp_codec_decide is called from the other side again and the full informed codec choice can be made. This may also mean that the codec changes, but it will always be a change that is valid within what both sides have signalled via SDP. So the tentative codec choice it is kind of a corner case but has a very practical reason. File src/libosmo-mgcp/mgcp_network.c: https://gerrit.osmocom.org/c/osmo-mgw/+/32218/comment/910c8bad_f2815f08 PS8, Line 511: rtp_hdr->payload_type = (uint8_t) conn_dst->end.codec->payload_type; > this looks wrong. There should not be one specific payload type on an endpoint. […] I think there must be a consciously chosen payload type for each connection (endpoint is the entity that contains the two connections). I think that this is necessary so that we are able to ignore unexpected packets. Lets imagine that on side A a call agent assigns amr-bwe/PT1, and amr-oa/PT2 for compatibility reasons. On side B only amr-bew/PT3 is assigned. Now side A sends the same audio stream with amr-bew/PT1 and amr-oa/PT2. If we do not know what to expect we would "mix" both audio stream to amr-bew/PT3. This means we have to make a decision and we should make it based on the codecs that both sides have assigned and then follow it. We also cannot make the decision for every packet that arrives since we would end up in the "mix"-effect described below. It is also better to make the decision not for each packets to safe some CPU cycles. Since the decision is already made when the connection is created we can just look up the payload type in end.codec->payload_type. (payload conversion is done in other functions) What might be necessary in the future though are separate codec decisions in case we want to transfer video as well. Then we might need an end.codec_video->payload_type as well. But thats out of scope for the moment as there are no plans to support video telephony yet. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32218 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I6c3291f825488e5d8ce136aeb18450156794aeb5 Gerrit-Change-Number: 32218 Gerrit-PatchSet: 9 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Wed, 26 Apr 2023 15:20:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 2 months

1
0
0 0

[S] Change in osmo-mgw[master]: mgcp_network: do not deliver RTP packets with unpatched PT

by dexter

Attention is currently required from: neels. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/32290 ) Change subject: mgcp_network: do not deliver RTP packets with unpatched PT ...................................................................... Patch Set 4: (2 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-mgw/+/32290/comment/11fa5b13_7187e1f3 PS3, Line 9: call agent > by call agent, do you mean like BSC and MSC? So far I see call agent as meaning a PBX like freeswitc […] As far as I understand a call agent is any entity that controls an MGW, it can be a BSC, MSC or a PBX. File src/libosmo-mgcp/mgcp_network.c: https://gerrit.osmocom.org/c/osmo-mgw/+/32290/comment/eb90af28_86187357 PS3, Line 1168: DEBUG > I guess when we drop packets that would be LOGL_ERROR; only problem is with spamming 20 LOGL_ERROR p […] I think in this case LOGL_NOTICE it is not a too big problem. mgcp_patch_pt() only fails on short RTP packets or if the codec decision is not made properly. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32290 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I013a24c1e0f853557257368cfab9192d4611aafa Gerrit-Change-Number: 32290 Gerrit-PatchSet: 4 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Wed, 26 Apr 2023 15:20:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 2 months

1
0
0 0

[L] Change in osmo-ttcn3-hacks[master]: MGCP_Test: support multiple codecs

by dexter

Attention is currently required from: pespin. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32153 ) Change subject: MGCP_Test: support multiple codecs ...................................................................... Patch Set 9: (1 comment) File library/RTP_Emulation.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32153/comment/9adbfcbb_8297… PS5, Line 406: if (full_match > 0) { > correct Done -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32153 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: I8422313fccad1bfcee52c933f643068bebdaf2d5 Gerrit-Change-Number: 32153 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-Comment-Date: Wed, 26 Apr 2023 12:58:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 2 months

1
0
0 0

[S] Change in osmo-ttcn3-hacks[master]: MGCP_Test: fix typo

by dexter

dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32292 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: MGCP_Test: fix typo ...................................................................... MGCP_Test: fix typo Change-Id: I1c860acb60267a9b4d8049bf231c49f9c915923b --- M mgw/MGCP_Test.ttcn 1 file changed, 10 insertions(+), 1 deletion(-) Approvals: pespin: Looks good to me, approved Jenkins Builder: Verified diff --git a/mgw/MGCP_Test.ttcn b/mgw/MGCP_Test.ttcn index dfbe263..a0c577a 100644 --- a/mgw/MGCP_Test.ttcn +++ b/mgw/MGCP_Test.ttcn @@ -2321,7 +2321,7 @@ } /* create two local RTP emulations; create two connections on MGW EP, see if - * exchanged data is converted between AMR octet-aligned and bandwith + * exchanged data is converted between AMR octet-aligned and bandwidth * efficient-mode */ function f_TC_amr_x_x_rtp_conversion(octetstring pl0, octetstring pl1, charstring fmtp0, charstring fmtp1) runs on dummy_CT { var RtpFlowData flow[2]; -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32292 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: I1c860acb60267a9b4d8049bf231c49f9c915923b Gerrit-Change-Number: 32292 Gerrit-PatchSet: 5 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 2 months

1
0
0 0

[M] Change in osmo-bsc[master]: ctrl: Add penalty time control

by dexter

Attention is currently required from: matanp. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/32498 ) Change subject: ctrl: Add penalty time control ...................................................................... Patch Set 3: (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-bsc/+/32498/comment/55fabfc2_f2979877 PS1, Line 7: ctrl: Add penalty time control Maybe add some context here what "penalty time" is for - apparently its it is some SI value. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32498 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Idfdd54dec72fb5f52eee22df018161d75b8c48c8 Gerrit-Change-Number: 32498 Gerrit-PatchSet: 3 Gerrit-Owner: matanp <matan1008(a)gmail.com> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: dexter <pmaier(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: matanp <matan1008(a)gmail.com> Gerrit-Comment-Date: Wed, 26 Apr 2023 12:34:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 2 months

1
0
0 0

[S] Change in osmocom-bb[master]: layer23: modem: Store P-TMSI allocated by the network

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/32496 ) Change subject: layer23: modem: Store P-TMSI allocated by the network ...................................................................... layer23: modem: Store P-TMSI allocated by the network Change-Id: Ide686230336d68153db59e76dd97b7e7c6f500d8 --- M src/host/layer23/src/modem/gmm.c 1 file changed, 21 insertions(+), 0 deletions(-) Approvals: fixeria: Looks good to me, but someone else must approve osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/src/host/layer23/src/modem/gmm.c b/src/host/layer23/src/modem/gmm.c index 6d1f518..0ad3035 100644 --- a/src/host/layer23/src/modem/gmm.c +++ b/src/host/layer23/src/modem/gmm.c @@ -30,6 +30,7 @@ #include <osmocom/core/linuxlist.h> #include <osmocom/core/talloc.h> #include <osmocom/core/tun.h> +#include <osmocom/gsm/protocol/gsm_04_08_gprs.h> #include <osmocom/gprs/llc/llc.h> #include <osmocom/gprs/llc/llc_prim.h> @@ -47,12 +48,23 @@ static int modem_gmm_prim_up_cb(struct osmo_gprs_gmm_prim *gmm_prim, void *user_data) { const char *pdu_name = osmo_gprs_gmm_prim_name(gmm_prim); + struct osmocom_ms *ms = user_data; int rc = 0; switch (gmm_prim->oph.sap) { case OSMO_GPRS_GMM_SAP_GMMREG: switch (OSMO_PRIM_HDR(&gmm_prim->oph)) { case OSMO_PRIM(OSMO_GPRS_GMM_GMMREG_ATTACH, PRIM_OP_CONFIRM): + if (gmm_prim->gmmreg.attach_cnf.accepted) { + LOGP(DGMM, LOGL_NOTICE, "%s(): Rx %s: Attach success P-TMSI=0x%08x\n", + __func__, pdu_name, gmm_prim->gmmreg.attach_cnf.acc.allocated_ptmsi); + ms->subscr.ptmsi = gmm_prim->gmmreg.attach_cnf.acc.allocated_ptmsi; + } else { + uint8_t cause = gmm_prim->gmmreg.attach_cnf.rej.cause; + LOGP(DGMM, LOGL_ERROR, "%s(): Rx %s: Attach rejected, cause=%u (%s)\n", + __func__, pdu_name, cause, get_value_string(gsm48_gmm_cause_names, cause)); + } + break; case OSMO_PRIM(OSMO_GPRS_GMM_GMMREG_DETACH, PRIM_OP_CONFIRM): case OSMO_PRIM(OSMO_GPRS_GMM_GMMREG_DETACH, PRIM_OP_INDICATION): default: -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/32496 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ide686230336d68153db59e76dd97b7e7c6f500d8 Gerrit-Change-Number: 32496 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 2 months

1
0
0 0

[S] Change in osmocom-bb[master]: layer23: modem: Store P-TMSI allocated by the network

by pespin

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/32496 ) Change subject: layer23: modem: Store P-TMSI allocated by the network ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/32496 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ide686230336d68153db59e76dd97b7e7c6f500d8 Gerrit-Change-Number: 32496 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 26 Apr 2023 12:26:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 2 months

1
0
0 0

[S] Change in libosmo-abis[master]: e1_input: add new driver callback function: line_create

by pespin

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/32400 ) Change subject: e1_input: add new driver callback function: line_create ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32400 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I404fa23e9b8a952be84e9716889c0dbbbc665d22 Gerrit-Change-Number: 32400 Gerrit-PatchSet: 4 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: Wed, 26 Apr 2023 12:25:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 2 months

1
0
0 0

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

by pespin

Attention is currently required from: laforge, keith, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/32374 ) Change subject: e1d: reconnect to osmo-e1d after connection loss ...................................................................... Patch Set 6: (1 comment) File src/input/e1d.c: https://gerrit.osmocom.org/c/libosmo-abis/+/32374/comment/dd39fe4c_b037c13c PS4, Line 123: if (bfd->list.next && bfd->list.next != LLIST_POISON1) > I always go through the whole patchset, fix all comments and then I push. […] No, it doesn't, because osmo_fd_is_registered() can be costly, and in most ocassions you know if the fd is registered or not. This check of bfd->list.next smells like something not thought enough and that can be simplified. Can't you guarantee that if fd >= 0 then it is for sure registered and hence can be unconditionally urnegistered in this code path? -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32374 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: Iaf4d42c2f009b1d7666e319fabdeb2598aa0b338 Gerrit-Change-Number: 32374 Gerrit-PatchSet: 6 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: keith <keith(a)rhizomatica.org> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-CC: tnt <tnt(a)246tNt.com> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: keith <keith(a)rhizomatica.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 26 Apr 2023 12:24:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 2 months

1
0
0 0

[S] Change in osmo-pcu[master]: pcu_l1_if: cosmetic: reformat comment

by dexter

dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/32306 ) Change subject: pcu_l1_if: cosmetic: reformat comment ...................................................................... pcu_l1_if: cosmetic: reformat comment Change-Id: Ib4ae65e1bc118572135fbec02e64dbec35f208b4 --- M src/pcu_l1_if.cpp 1 file changed, 10 insertions(+), 3 deletions(-) Approvals: laforge: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/pcu_l1_if.cpp b/src/pcu_l1_if.cpp index 4aae9c4..bb0a309 100644 --- a/src/pcu_l1_if.cpp +++ b/src/pcu_l1_if.cpp @@ -274,9 +274,7 @@ else memset(data, '0', IMSI_DIGITS_FOR_PAGING); - /* block provided by upper layer comes without first byte (plen), - * prepend it manually: - */ + /* block provided by upper layer comes without first byte (plen), prepend it manually: */ OSMO_ASSERT(sizeof(data) >= IMSI_DIGITS_FOR_PAGING + 1 + block->data_len); data[3] = (plen << 2) | 0x01; bitvec_pack(block, data + IMSI_DIGITS_FOR_PAGING + 1); -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/32306 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ib4ae65e1bc118572135fbec02e64dbec35f208b4 Gerrit-Change-Number: 32306 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 2 months

1
0
0 0

2025

2024

2023

2022

gerrit-log April 2023