- gerrit-log - lists.osmocom.org

[S] Change in osmo-ttcn3-hacks[master]: sccp: Improve logging receiving unexpected msg
by pespin 11 Mar '25

11 Mar '25

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/39767?usp=email ) Change subject: sccp: Improve logging receiving unexpected msg ...................................................................... sccp: Improve logging receiving unexpected msg Change-Id: I90c5236882e96fc038fee0d2f4551cfc58767c78 --- M sccp/SCCP_Tests_RAW.ttcn 1 file changed, 11 insertions(+), 6 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/67/39767/1 diff --git a/sccp/SCCP_Tests_RAW.ttcn b/sccp/SCCP_Tests_RAW.ttcn index 5d5b0ab..f7c5357 100644 --- a/sccp/SCCP_Tests_RAW.ttcn +++ b/sccp/SCCP_Tests_RAW.ttcn @@ -14,6 +14,7 @@ import from General_Types all; import from Osmocom_Types all; +import from Misc_Helpers all; import from M3UA_Emulation all; @@ -136,9 +137,9 @@ return exp; } -private function f_exp_sccp(template PDU_SCCP sccp) +private function f_exp_sccp(template (present) PDU_SCCP sccp := ?) runs on SCCP_Test_RAW_CT return SCCP_MTP3_TRANSFERind { - var template SCCP_MTP3_TRANSFERind exp := tr_SCCP_MTP3_TRANSFERind(sccp); + var template (present) SCCP_MTP3_TRANSFERind exp := tr_SCCP_MTP3_TRANSFERind(sccp); var SCCP_MTP3_TRANSFERind rx; timer T := 10.0; T.start; @@ -146,13 +147,17 @@ [] MTP3.receive(exp) -> value rx { return rx; } + [] MTP3.receive(tr_SCCP_MTP3_TRANSFERind(*)) -> value rx { + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + log2str("Unexpected MTP/SCCP received: got ", rx, " vs exp ", exp)); + } [] MTP3.receive { - setverdict(fail, "Unexpected MTP/SCCP received"); - self.stop + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + log2str("Unexpected MTP/SCCP received: got (ASP_MTP3_PAUSE|ASP_MTP3_RESUME|ASP_MTP3_STATUS) vs exp ", exp)); } [] T.timeout { - setverdict(fail, "Timeout waiting for ", exp); - self.stop + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + log2str("Timeout waiting for ", exp)); } } return rx; -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/39767?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I90c5236882e96fc038fee0d2f4551cfc58767c78 Gerrit-Change-Number: 39767 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in libosmo-sigtran[master]: sccp: Use protoClass 0 to transmit SSA
by pespin 11 Mar '25

11 Mar '25

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/39766?usp=email ) Change subject: sccp: Use protoClass 0 to transmit SSA ...................................................................... sccp: Use protoClass 0 to transmit SSA Recent commit added the possibility to set proto Class 0 vs 1 based on Sequence Control parameter present in UNITDATA.req, but forgot to extend it to the special code path used tro transmit SSA. As a result, libosmo-sigtran started transmitting SSA with protoClass 1 instead of previous 0. This is actually not bad, but there's no need to use protocClass1 here since we don't care about other messages being sent together with this one, so protoClass0 is better fit here. Fixes: 0061e8d0bcba3b0ed5ea255588619627d0975380 Change-Id: I906094a14be670e665ed8ffd0383dd8da690cb9a --- M src/sccp_scmg.c 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/66/39766/1 diff --git a/src/sccp_scmg.c b/src/sccp_scmg.c index a8ac766..755d272 100644 --- a/src/sccp_scmg.c +++ b/src/sccp_scmg.c @@ -166,7 +166,7 @@ }; static int sccp_scmg_tx(struct osmo_sccp_user *scu, const struct osmo_sccp_addr *calling_addr, - const struct osmo_sccp_addr *called_addr, + const struct osmo_sccp_addr *called_addr, const uint32_t *seq_ctrl, uint8_t msg_type, uint8_t ssn, uint16_t pc, uint8_t smi, uint8_t *ssc_cong_lvl) { struct msgb *msg = sccp_msgb_alloc(__func__); @@ -179,6 +179,7 @@ param = &prim->u.unitdata; memcpy(&param->calling_addr, calling_addr, sizeof(*calling_addr)); memcpy(&param->called_addr, called_addr, sizeof(*called_addr)); + param->in_sequence_control = seq_ctrl ? *seq_ctrl : OSMO_SCU_UNITDATA_REQ_P_SEQUENCE_CONTROL_NOT_PRESENT; osmo_prim_init(&prim->oph, SCCP_SAP_USER, OSMO_SCU_PRIM_N_UNITDATA, PRIM_OP_REQUEST, msg); /* Fill the actual SCMG message */ @@ -243,7 +244,7 @@ if (0 /* !subsys_available(scu) */) return 0; - return sccp_scmg_tx(scu, called_addr, calling_addr, SCCP_SCMG_MSGT_SSA, + return sccp_scmg_tx(scu, called_addr, calling_addr, NULL, SCCP_SCMG_MSGT_SSA, sst->affected_ssn, sst->affected_pc, 0, NULL); } -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/39766?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: I906094a14be670e665ed8ffd0383dd8da690cb9a Gerrit-Change-Number: 39766 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in libosmocore[master]: iuup: Explicitly discard Iu-UP-DATA.req in Initialization state
by pespin 11 Mar '25

11 Mar '25

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/39753?usp=email ) Change subject: iuup: Explicitly discard Iu-UP-DATA.req in Initialization state ...................................................................... iuup: Explicitly discard Iu-UP-DATA.req in Initialization state Since Initialization procedure is asynchronous (need to wait for IuUP ACK from peer), it may be that the user initializes the IuUP layer and starts sending/forarding IuUP data to it before the initialization finishes. Before this patch, an error would be logged for each such Data request. Let's instead explicitly expect this event and hence avoid logging a generic FSM error log "{Initialisation}: Event IuUP-DATA-req not permitted" and log in INFO level and discard the message. Change-Id: I424be5718cdd12cfd0ce80108a002ce68d3e17aa --- M src/gsm/iuup.c 1 file changed, 8 insertions(+), 0 deletions(-) Approvals: Jenkins Builder: Verified osmith: Looks good to me, approved diff --git a/src/gsm/iuup.c b/src/gsm/iuup.c index 4991213..df303e5 100644 --- a/src/gsm/iuup.c +++ b/src/gsm/iuup.c @@ -714,6 +714,13 @@ /* the only permitted 'config req' type is the request to release the instance */ osmo_fsm_inst_state_chg(fi, IUUP_FSM_ST_NULL, 0, 0); break; + case IUUP_FSM_EVT_IUUP_DATA_REQ: + /* Data coming down from RNL (user) towards TNL (transport). + * Discard since we are still not in Data Transfer Ready State. */ + irp = data; + LOGPFSML(fi, LOGL_INFO, "Iu-UP-DATA.req while still initializing, discarding\n"); + msgb_free(irp->oph.msg); + break; case IUUP_FSM_EVT_INIT: itp = data; if (iuup_rx_initialization(iui, itp)) @@ -825,6 +832,7 @@ }, [IUUP_FSM_ST_INIT] = { .in_event_mask = S(IUUP_FSM_EVT_IUUP_CONFIG_REQ) | + S(IUUP_FSM_EVT_IUUP_DATA_REQ) | S(IUUP_FSM_EVT_INIT) | S(IUUP_FSM_EVT_LAST_INIT_ACK) | S(IUUP_FSM_EVT_INIT_NACK), -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/39753?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I424be5718cdd12cfd0ce80108a002ce68d3e17aa Gerrit-Change-Number: 39753 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in libosmocore[master]: iuup: Explicitly discard Iu-UP-DATA.req in Initialization state
by osmith 11 Mar '25

11 Mar '25

Attention is currently required from: laforge, pespin. osmith has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmocore/+/39753?usp=email ) Change subject: iuup: Explicitly discard Iu-UP-DATA.req in Initialization state ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: my +1 and previous +1 with now spelling fixed -> +2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/39753?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I424be5718cdd12cfd0ce80108a002ce68d3e17aa Gerrit-Change-Number: 39753 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 11 Mar 2025 11:12:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[XS] Change in libosmocore[master]: iuup: Explicitly discard Iu-UP-DATA.req in Initialization state
by osmith 11 Mar '25

11 Mar '25

Attention is currently required from: laforge, pespin. osmith has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmocore/+/39753?usp=email ) Change subject: iuup: Explicitly discard Iu-UP-DATA.req in Initialization state ...................................................................... Patch Set 2: Code-Review+1 (2 comments) Patchset: PS2: nice! Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/39753/comment/b989ba17_43894f37?… : PS1, Line 9: asyncrhonous > Done Done (resolving) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/39753?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I424be5718cdd12cfd0ce80108a002ce68d3e17aa Gerrit-Change-Number: 39753 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 11 Mar 2025 11:11:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>

1 0

[M] Change in osmo-mgw[master]: mgw: Split MDCX read-only validation on conn into its own function
by pespin 11 Mar '25

11 Mar '25

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39739?usp=email ) Change subject: mgw: Split MDCX read-only validation on conn into its own function ...................................................................... mgw: Split MDCX read-only validation on conn into its own function Add a second read-only validation step once the conn to be modified is found in the endpoint. Change-Id: I025ca5352eff9ee35c977c69b71934513646b1f2 --- M src/libosmo-mgcp/mgcp_protocol.c 1 file changed, 40 insertions(+), 14 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve laforge: Looks good to me, but someone else must approve Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 79a647a..6a731a3 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -891,6 +891,37 @@ return 0; } +/* Read-only checks for parsed MDCX request, applied to existing found conn. + * Returns negative MGCP error code on failure, 0 on scucess. */ +static int validate_parsed_mdcx_conn(struct mgcp_request_data *rq, struct mgcp_conn *conn) +{ + struct mgcp_parse_data *pdata = rq->pdata; + struct mgcp_parse_hdr_pars *hpars = &pdata->hpars; + struct mgcp_conn_rtp *conn_rtp = mgcp_conn_get_conn_rtp(conn); + + OSMO_ASSERT(conn_rtp); + + if (mgcp_conn_rtp_is_osmux(conn_rtp)) { + OSMO_ASSERT(conn_rtp->osmux.local_cid_allocated); + if (hpars->remote_osmux_cid == MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET) { + LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: Failed to parse Osmux CID!\n"); + return -500; + } + if (hpars->remote_osmux_cid == MGCP_PARSE_HDR_PARS_OSMUX_CID_WILDCARD) { + LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: wilcard in MDCX is not supported!\n"); + return -500; + } + if (conn_rtp->osmux.remote_cid_present && + hpars->remote_osmux_cid != conn_rtp->osmux.remote_cid) { + LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: changing already allocated CID is not supported!\n"); + return -500; + } + } + + /* Everything fine, continue */ + return 0; +} + /* MDCX command handler, processes the received command */ static struct msgb *handle_modify_con(struct mgcp_request_data *rq) { @@ -953,6 +984,15 @@ return create_err_response(endp, endp, 400, "MDCX", pdata->trans); } + rc = validate_parsed_mdcx_conn(rq, conn); + if (rc < 0) + return create_err_response(rq->trunk, NULL, -rc, "MDCX", pdata->trans); + + /******************************************************************* + * Modify endpoint and conn. + * From here on below we start updating endpoint and modifying conn: + *******************************************************************/ + /* Set local connection options, if present */ if (hpars->lco.present) mgcp_endp_update_lco(endp, &hpars->lco); @@ -983,20 +1023,6 @@ rc = mgcp_conn_iuup_init(conn_rtp); if (mgcp_conn_rtp_is_osmux(conn_rtp)) { - OSMO_ASSERT(conn_rtp->osmux.local_cid_allocated); - if (hpars->remote_osmux_cid < -1) { - LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: Failed to parse Osmux CID!\n"); - goto error3; - } - if (hpars->remote_osmux_cid == -1) { - LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: wilcard in MDCX is not supported!\n"); - goto error3; - } - if (conn_rtp->osmux.remote_cid_present && - hpars->remote_osmux_cid != conn_rtp->osmux.remote_cid) { - LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: changing already allocated CID is not supported!\n"); - goto error3; - } conn_rtp->osmux.remote_cid_present = true; conn_rtp->osmux.remote_cid = hpars->remote_osmux_cid; if (conn_osmux_event_rx_crcx_mdcx(conn_rtp) < 0) { -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39739?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I025ca5352eff9ee35c977c69b71934513646b1f2 Gerrit-Change-Number: 39739 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

[M] Change in osmo-mgw[master]: mgw: Split DLCX read-only validations into its own function
by pespin 11 Mar '25

11 Mar '25

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39740?usp=email ) Change subject: mgw: Split DLCX read-only validations into its own function ...................................................................... mgw: Split DLCX read-only validations into its own function Similar to what's already done in CRCX and MDXC. Change-Id: I3d86f0cbeacc7c0cd69809f898714a76d3b86ed0 --- M src/libosmo-mgcp/mgcp_protocol.c 1 file changed, 50 insertions(+), 28 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve osmith: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 6a731a3..237f2bf 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -1073,6 +1073,49 @@ return create_err_response(endp, endp, error_code, "MDCX", pdata->trans); } +/* Read-only checks for parsed DLCX request, applied to existing found conn. + * Returns negative MGCP error code on failure, 0 on scucess. + * NOTE: rq->endp may be NULL here! */ +static int validate_parsed_dlcx(struct mgcp_request_data *rq) +{ + struct mgcp_parse_data *pdata = rq->pdata; + struct mgcp_parse_hdr_pars *hpars = &pdata->hpars; + struct rate_ctr_group *rate_ctrs = rq->trunk->ratectr.mgcp_dlcx_ctr_group; + int rc; + + if (hpars->callid) { + /* If we have no endpoint, but a call id in the request, then this request cannot be handled */ + if (!rq->endp) { + LOGPTRUNK(rq->trunk, DLMGCP, LOGL_NOTICE, + "cannot handle requests with call-id (C) without endpoint -- abort!"); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM)); + return -539; + } + if (!(rq->endp->x_osmo_ign & MGCP_X_OSMO_IGN_CALLID) && + mgcp_verify_call_id(rq->endp, hpars->callid) != 0) { + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CALLID)); + return -516; + } + } + + if (hpars->connid) { + /* If we have no endpoint, but a connection id in the request, then this request cannot be handled */ + if (!rq->endp) { + LOGPTRUNK(rq->trunk, DLMGCP, LOGL_NOTICE, + "cannot handle requests with conn-id (I) without endpoint -- abort!"); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM)); + return -539; + } + if ((rc = mgcp_verify_ci(rq->endp, hpars->connid)) != 0) { + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CONNID)); + return -rc; + } + } + + /* Everything fine, continue */ + return 0; +} + /* DLCX command handler, processes the received command */ static struct msgb *handle_delete_con(struct mgcp_request_data *rq) { @@ -1130,39 +1173,18 @@ if (rc < 0) return create_err_response(rq->trunk, NULL, -rc, "DLCX", pdata->trans); - if (hpars->callid) { - /* If we have no endpoint, but a call id in the request, then this request cannot be handled */ - if (!endp) { - LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE, - "cannot handle requests with call-id (C) without endpoint -- abort!"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM)); - return create_err_response(rq->trunk, NULL, 539, "DLCX", pdata->trans); - } - if (!(endp->x_osmo_ign & MGCP_X_OSMO_IGN_CALLID) && - mgcp_verify_call_id(endp, hpars->callid) != 0) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CALLID)); - return create_err_response(endp, endp, 516, "DLCX", pdata->trans); - } - } - - if (hpars->connid) { - /* If we have no endpoint, but a connection id in the request, then this request cannot be handled */ - if (!endp) { - LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE, - "cannot handle requests with conn-id (I) without endpoint -- abort!"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM)); - return create_err_response(rq->trunk, NULL, 539, "DLCX", pdata->trans); - } - if ((rc = mgcp_verify_ci(endp, hpars->connid)) != 0) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CONNID)); - return create_err_response(endp, endp, rc, "DLCX", pdata->trans); - } - } + rc = validate_parsed_dlcx(rq); + if (rc < 0) + return create_err_response(rq->trunk, NULL, -rc, "DLCX", pdata->trans); /* The logic does not permit to go past this point without having the * the endp pointer populated. */ OSMO_ASSERT(endp); + /***************************************************************************** + * From here on below we start, delete conn and potentially release endpoint. + *****************************************************************************/ + /* When no connection id is supplied, we will interpret this as a * wildcarded DLCX that refers to the selected endpoint. This means * that we drop all connections on that specific endpoint at once. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39740?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I3d86f0cbeacc7c0cd69809f898714a76d3b86ed0 Gerrit-Change-Number: 39740 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in osmo-mgw[master]: mgw: CRCX: Handle codec information before initializing osmux
by pespin 11 Mar '25

11 Mar '25

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39754?usp=email ) Change subject: mgw: CRCX: Handle codec information before initializing osmux ...................................................................... mgw: CRCX: Handle codec information before initializing osmux Same order as done during MDCX. This way we can also eg. check configured codec is AMR before successfully entering configuring osmux. Change-Id: If291db0c048196e6e0eee2c38e648e5a25438078 --- M src/libosmo-mgcp/mgcp_protocol.c 1 file changed, 10 insertions(+), 8 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve laforge: Looks good to me, but someone else must approve Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 237f2bf..ef82796 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -768,6 +768,15 @@ conn_rtp = mgcp_conn_get_conn_rtp(conn); OSMO_ASSERT(conn_rtp); + /* Handle codec information and decide for a suitable codec */ + rc = handle_codec_info(conn_rtp, rq); + mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn)); + if (rc) { + error_code = rc; + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_CODEC_NEGOTIATION)); + goto error2; + } + /* If X-Osmux (remote CID) was received, alloc next avail CID as local CID */ if (hpars->remote_osmux_cid != MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET) { /* Make sure osmux is setup: */ @@ -785,14 +794,6 @@ } /* else: -1 (wildcard) */ } - /* Handle codec information and decide for a suitable codec */ - rc = handle_codec_info(conn_rtp, rq); - mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn)); - if (rc) { - error_code = rc; - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_CODEC_NEGOTIATION)); - goto error2; - } /* Upgrade the conn type RTP_DEFAULT->RTP_IUUP if needed based on requested codec: */ if (conn_rtp->type == MGCP_RTP_DEFAULT && strcmp(conn_rtp->end.cset.codec->subtype_name, "VND.3GPP.IUFP") == 0) { @@ -1017,6 +1018,7 @@ error_code = rc; goto error3; } + /* Upgrade the conn type RTP_DEFAULT->RTP_IUUP if needed based on requested codec: */ if (conn_rtp->type == MGCP_RTP_DEFAULT && strcmp(conn_rtp->end.cset.codec->subtype_name, "VND.3GPP.IUFP") == 0) -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39754?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: If291db0c048196e6e0eee2c38e648e5a25438078 Gerrit-Change-Number: 39754 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

[M] Change in osmo-mgw[master]: mgw: Drop rtp_processing transcoding stubs
by pespin 11 Mar '25

11 Mar '25

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39755?usp=email ) Change subject: mgw: Drop rtp_processing transcoding stubs ...................................................................... mgw: Drop rtp_processing transcoding stubs This has never been used afaik, and it's only clogging the code and adding extra indirect function pointer calls to hot paths for no good reason. Drop all of it, and re-ad whatever makes sense once someone really wants to implement transcoding. Change-Id: Id16af528d94612e771f94333ad5404e5cd1c3318 --- M include/osmocom/mgcp/mgcp.h M include/osmocom/mgcp/mgcp_network.h M include/osmocom/mgcp/mgcp_ratectr.h M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_ratectr.c 6 files changed, 1 insertion(+), 102 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified pespin: Looks good to me, approved laforge: Looks good to me, but someone else must approve diff --git a/include/osmocom/mgcp/mgcp.h b/include/osmocom/mgcp/mgcp.h index f72807e..edc3cb6 100644 --- a/include/osmocom/mgcp/mgcp.h +++ b/include/osmocom/mgcp/mgcp.h @@ -130,10 +130,6 @@ char source_addr[INET6_ADDRSTRLEN]; char call_agent_addr[INET6_ADDRSTRLEN]; - /* RTP processing */ - mgcp_processing rtp_processing_cb; - mgcp_processing_setup setup_rtp_processing_cb; - struct osmo_wqueue gw_fd; struct mgcp_port_range net_ports; diff --git a/include/osmocom/mgcp/mgcp_network.h b/include/osmocom/mgcp/mgcp_network.h index 8a1fc47..f30b788 100644 --- a/include/osmocom/mgcp/mgcp_network.h +++ b/include/osmocom/mgcp/mgcp_network.h @@ -98,18 +98,6 @@ struct osmo_sockaddr *addr, struct msgb *msg); int mgcp_get_local_addr(char *addr, struct mgcp_conn_rtp *conn); -/* payload processing default functions */ -int mgcp_rtp_processing_default(struct mgcp_endpoint *endp, struct mgcp_rtp_end *dst_end, struct msgb *msg); - -int mgcp_setup_rtp_processing_default(struct mgcp_endpoint *endp, - struct mgcp_conn_rtp *conn_dst, - struct mgcp_conn_rtp *conn_src); - -void mgcp_get_net_downlink_format_default(struct mgcp_endpoint *endp, - const struct mgcp_rtp_codec **codec, - const char **fmtp_extra, - struct mgcp_conn_rtp *conn); - /* internal RTP Annex A counting */ void mgcp_rtp_annex_count(const struct mgcp_endpoint *endp, struct mgcp_rtp_state *state, const uint16_t seq, const int32_t transit, diff --git a/include/osmocom/mgcp/mgcp_ratectr.h b/include/osmocom/mgcp/mgcp_ratectr.h index f566625..1e3365b 100644 --- a/include/osmocom/mgcp/mgcp_ratectr.h +++ b/include/osmocom/mgcp/mgcp_ratectr.h @@ -43,7 +43,6 @@ MGCP_MDCX_FAIL_INVALID_MODE, MGCP_MDCX_FAIL_INVALID_CONN_OPTIONS, MGCP_MDCX_FAIL_NO_REMOTE_CONN_DESC, - MGCP_MDCX_FAIL_START_RTP, MGCP_MDCX_FAIL_AVAIL, }; diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index de837c1..abd095b 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -408,31 +408,6 @@ return ts_error; } -/*! dummy callback to disable transcoding (see also cfg->rtp_processing_cb). - * \param[in] associated endpoint. - * \param[in] destination RTP end. - * \param[in,out] msg message bufffer containing data. Function might change length. - * \returns ignores input parameters, return always 0. */ -int mgcp_rtp_processing_default(struct mgcp_endpoint *endp, - struct mgcp_rtp_end *dst_end, - struct msgb *msg) -{ - return 0; -} - -/*! dummy callback to disable transcoding (see also cfg->setup_rtp_processing_cb). - * \param[in] associated endpoint. - * \param[in] destination RTP connnection. - * \param[in] source RTP connection. - * \returns ignores input parameters, return always 0. */ -int mgcp_setup_rtp_processing_default(struct mgcp_endpoint *endp, - struct mgcp_conn_rtp *conn_dst, - struct mgcp_conn_rtp *conn_src) -{ - LOGPENDP(endp, DRTP, LOGL_DEBUG, "transcoding disabled\n"); - return 0; -} - void mgcp_rtp_annex_count(const struct mgcp_endpoint *endp, struct mgcp_rtp_state *state, const uint16_t seq, const int32_t transit, const uint32_t ssrc, @@ -1187,13 +1162,7 @@ * header is present, we will generate one. */ gen_rtp_header(msg, rtp_end, rtp_state); - /* Run transcoder */ - rc = endp->trunk->cfg->rtp_processing_cb(endp, rtp_end, msg); - if (rc < 0) { - LOGPENDP(endp, DRTP, LOGL_ERROR, "Error %d during transcoding\n", rc); - msgb_free(msg); - return rc; - } + /* TODO: Run transcoder */ if (addr) mgcp_patch_and_count(endp, rtp_state, rtp_end, addr, msg); diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index ef82796..7c1a715 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -96,43 +96,6 @@ static struct msgb *handle_rsip(struct mgcp_request_data *data); static struct msgb *handle_noti_req(struct mgcp_request_data *data); -/* Initalize transcoder */ -static int setup_rtp_processing(struct mgcp_endpoint *endp, - struct mgcp_conn_rtp *conn) -{ - struct mgcp_config *cfg = endp->trunk->cfg; - struct mgcp_conn_rtp *conn_src = NULL; - struct mgcp_conn_rtp *conn_dst = conn; - struct mgcp_conn *_conn; - - switch (conn->type) { - case MGCP_RTP_DEFAULT: - case MGCP_RTP_OSMUX: - case MGCP_RTP_IUUP: - break; - default: - LOGPENDP(endp, DLMGCP, LOGL_NOTICE, - "RTP-setup: Endpoint is not configured as RTP default, stopping here!\n"); - return 0; - } - - if (conn->conn->mode == MGCP_CONN_LOOPBACK) { - LOGPENDP(endp, DLMGCP, LOGL_NOTICE, - "RTP-setup: Endpoint is in loopback mode, stopping here!\n"); - return 0; - } - - /* Find the "sister" connection */ - llist_for_each_entry(_conn, &endp->conns, entry) { - if (_conn->id != conn->conn->id) { - conn_src = mgcp_conn_get_conn_rtp(_conn); - break; - } - } - - return cfg->setup_rtp_processing_cb(endp, conn_dst, conn_src); -} - /* Helper function to allocate some memory for responses and retransmissions */ static struct msgb *mgcp_msgb_alloc(void *ctx) { @@ -811,13 +774,6 @@ goto error2; } - if (setup_rtp_processing(endp, conn_rtp) != 0) { - LOGPCONN(conn, DLMGCP, LOGL_ERROR, - "CRCX: could not start RTP processing!\n"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_START_RTP)); - goto error2; - } - /* Notify Osmux conn that CRCX was received */ if (mgcp_conn_rtp_is_osmux(conn_rtp)) { if (conn_osmux_event_rx_crcx_mdcx(conn_rtp) < 0) { @@ -1050,11 +1006,6 @@ } } - if (setup_rtp_processing(endp, conn_rtp) != 0) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_START_RTP)); - goto error3; - } - /* modify */ LOGPCONN(conn, DLMGCP, LOGL_DEBUG, "MDCX: modified conn:%s\n", mgcp_conn_dump(conn)); @@ -1394,9 +1345,6 @@ cfg->source_port = 2427; osmo_strlcpy(cfg->source_addr, "0.0.0.0", sizeof(cfg->source_addr)); - cfg->rtp_processing_cb = &mgcp_rtp_processing_default; - cfg->setup_rtp_processing_cb = &mgcp_setup_rtp_processing_default; - INIT_LLIST_HEAD(&cfg->trunks); /* Allocate virtual trunk */ diff --git a/src/libosmo-mgcp/mgcp_ratectr.c b/src/libosmo-mgcp/mgcp_ratectr.c index abf3883..1f06117 100644 --- a/src/libosmo-mgcp/mgcp_ratectr.c +++ b/src/libosmo-mgcp/mgcp_ratectr.c @@ -93,7 +93,6 @@ [MGCP_MDCX_FAIL_INVALID_CONN_OPTIONS] = { "mdcx:conn_opt", "connection options invalid." }, [MGCP_MDCX_FAIL_NO_REMOTE_CONN_DESC] = { "mdcx:no_remote_conn_desc", "no opposite end specified for connection." }, - [MGCP_MDCX_FAIL_START_RTP] = { "mdcx:start_rtp_failure", "failure to start RTP processing." }, [MGCP_MDCX_FAIL_AVAIL] = { "mdcx:unavailable", "endpoint unavailable." }, }; -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39755?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Id16af528d94612e771f94333ad5404e5cd1c3318 Gerrit-Change-Number: 39755 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in osmo-mgw[master]: mgw: Rearrange last steps of CRCX and MDCX
by pespin 11 Mar '25

11 Mar '25

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39756?usp=email ) Change subject: mgw: Rearrange last steps of CRCX and MDCX ...................................................................... mgw: Rearrange last steps of CRCX and MDCX Make code in CRCX and MDCX look closer, since it is really doing the same. Move the dummy ping code after everything has been done correctly, since the ping is just a side effect of the conn being created. Change-Id: I42acda16cb2d59a9b7aae06b7584d4dfc1e91f9e --- M src/libosmo-mgcp/mgcp_protocol.c 1 file changed, 10 insertions(+), 15 deletions(-) Approvals: pespin: Looks good to me, approved laforge: Looks good to me, but someone else must approve Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 7c1a715..e4386b2 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -782,8 +782,11 @@ } } - LOGPCONN(conn, DLMGCP, LOGL_DEBUG, - "CRCX: Creating connection: port: %u\n", conn_rtp->end.local_port); + mgcp_endp_update(endp); + + LOGPCONN(conn, DLMGCP, LOGL_NOTICE, + "CRCX: connection successfully created: %s\n", mgcp_conn_dump(conn)); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_SUCCESS)); /* Send dummy packet, see also comments in mgcp_keepalive_timer_cb() */ OSMO_ASSERT(trunk->keepalive_interval >= MGCP_KEEPALIVE_ONCE); @@ -791,11 +794,6 @@ trunk->keepalive_interval != MGCP_KEEPALIVE_NEVER) send_dummy(endp, conn_rtp); - LOGPCONN(conn, DLMGCP, LOGL_NOTICE, - "CRCX: connection successfully created\n"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_SUCCESS)); - mgcp_endp_update(endp); - /* NOTE: Only in the virtual trunk we allow dynamic endpoint names */ bool add_epname = rq->wildcarded && trunk->trunk_type == MGCP_TRUNK_VIRTUAL; return create_response_with_sdp(endp, conn_rtp, "CRCX", pdata->trans, add_epname, true); @@ -1006,9 +1004,11 @@ } } - /* modify */ - LOGPCONN(conn, DLMGCP, LOGL_DEBUG, - "MDCX: modified conn:%s\n", mgcp_conn_dump(conn)); + mgcp_endp_update(endp); + + LOGPCONN(conn, DLMGCP, LOGL_NOTICE, + "MDCX: connection successfully modified: %s\n", mgcp_conn_dump(conn)); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_SUCCESS)); /* Send dummy packet, see also comments in mgcp_keepalive_timer_cb() */ OSMO_ASSERT(trunk->keepalive_interval >= MGCP_KEEPALIVE_ONCE); @@ -1016,11 +1016,6 @@ trunk->keepalive_interval != MGCP_KEEPALIVE_NEVER) send_dummy(endp, conn_rtp); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_SUCCESS)); - - LOGPCONN(conn, DLMGCP, LOGL_NOTICE, - "MDCX: connection successfully modified\n"); - mgcp_endp_update(endp); return create_response_with_sdp(endp, conn_rtp, "MDCX", pdata->trans, false, false); error3: return create_err_response(endp, endp, error_code, "MDCX", pdata->trans); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39756?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I42acda16cb2d59a9b7aae06b7584d4dfc1e91f9e Gerrit-Change-Number: 39756 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

2026

2025

2024

2023

2022

gerrit-log