November 2022 - gerrit-log - lists.osmocom.org

Change in osmo-pcu[master]: pdch: Initial support Handling PktResReq with ID_TYPE=UL/DL_TFI

by pespin

pespin has uploaded a new patch set (#2). ( https://gerrit.osmocom.org/c/osmo-pcu/+/30293 ) Change subject: pdch: Initial support Handling PktResReq with ID_TYPE=UL/DL_TFI ...................................................................... pdch: Initial support Handling PktResReq with ID_TYPE=UL/DL_TFI This patch refactors rcv_resource_request() in several ways: * Move the ID_TYPE=DL/UL_TFI handling at the start of the function, so that the function is split in 2 sections: First section gathers a GprsMS object from the ID_TYPE in the PktResReq. Second section handles the packet for the GprsMS based on the expectd ULC slot. * Initial handling of PktResReq when transmitted by the MS on the UL-TBF as an answer to USF. This case is basically the one where the MS wishes to change some parameters of the currently active UL-TBF. In order to do so, for now simply delete the current TBF and re-create a new one to triger the PktUlAss which is expected by the MS. This behavior is not entirely correct since in this case the MS is expected to keep using actively the old TBF until the PktUlAss is received, so in this case ideally we should be keeping the TBF object and simply upgrading it and using itself to trigger a PktUlAss in its ul_tbf->ul_ass_fsm. Doing this however requires far more work, so it can be done later as an incremental step fix. The current behavior is alreday better than the previous one, since the MS has been tested to be PKT_CTRL_ACKing the PKT_UL_ASS and continuing to use the new TBF. Related: OS#4947 Change-Id: Ie6b1b438d26cd977f88ddb4eff6b3041e0739d92 --- M src/pdch.cpp 1 file changed, 167 insertions(+), 142 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/93/30293/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/30293 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ie6b1b438d26cd977f88ddb4eff6b3041e0739d92 Gerrit-Change-Number: 30293 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-CC: Jenkins Builder Gerrit-MessageType: newpatchset

2 years, 7 months

1
0
0 0

Change in osmo-pcu[master]: pdch: Initial support Handling PktResReq with ID_TYPE=UL/DL_TFI

by Jenkins Builder

Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/30293 ) Change subject: pdch: Initial support Handling PktResReq with ID_TYPE=UL/DL_TFI ...................................................................... Patch Set 1: (1 comment) File src/pdch.cpp: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-1291): https://gerrit.osmocom.org/c/osmo-pcu/+/30293/comment/9e6ff8f3_9290ead3 PS1, Line 726: * This is requried by spec, and MS are know to continue using the TBF (due to delay in between DL and 'requried' may be misspelled - perhaps 'required'? -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/30293 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ie6b1b438d26cd977f88ddb4eff6b3041e0739d92 Gerrit-Change-Number: 30293 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Thu, 24 Nov 2022 15:22:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 7 months

1
0
0 0

Change in osmo-pcu[master]: pdch: Initial support Handling PktResReq with ID_TYPE=UL/DL_TFI

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcu/+/30293 ) Change subject: pdch: Initial support Handling PktResReq with ID_TYPE=UL/DL_TFI ...................................................................... pdch: Initial support Handling PktResReq with ID_TYPE=UL/DL_TFI This patch refactors rcv_resource_request() in several ways: * Move the ID_TYPE=DL/UL_TFI handling at the start of the function, so that the function is split in 2 sections: First section gathers a GprsMS object from the ID_TYPE in the PktResReq. Second section handles the packet for the GprsMS based on the expectd ULC slot. * Initial handling of PktResReq when transmitted by the MS on the UL-TBF as an answer to USF. This case is basically the one where the MS wishes to change some parameters of the currently active UL-TBF. In order to do so, for now simply delete the current TBF and re-create a new one to triger the PktUlAss which is expected by the MS. This behavior is not entirely correct since in this case the MS is expected to keep using actively the old TBF until the PktUlAss is received, so in this case ideally we should be keeping the TBF object and simply upgrading it and using itself to trigger a PktUlAss in its ul_tbf->ul_ass_fsm. Doing this however requires far more work, so it can be done later as an incremental step fix. The current behavior is alreday better than the previous one, since the MS has been tested to be PKT_CTRL_ACKing the PKT_UL_ASS and continuing to use the new TBF. Related: OS#4947 Change-Id: Ie6b1b438d26cd977f88ddb4eff6b3041e0739d92 --- M src/pdch.cpp 1 file changed, 169 insertions(+), 140 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/93/30293/1 diff --git a/src/pdch.cpp b/src/pdch.cpp index 0988af6..90917f2 100644 --- a/src/pdch.cpp +++ b/src/pdch.cpp @@ -644,10 +644,16 @@ void gprs_rlcmac_pdch::rcv_resource_request(Packet_Resource_Request_t *request, uint32_t fn, struct pcu_l1_meas *meas) { - struct gprs_rlcmac_sba *sba; int rc; struct gprs_rlcmac_bts *bts = trx->bts; struct pdch_ulc_node *item; + char buf[128]; + struct GprsMs *ms = NULL; + struct gprs_rlcmac_sba *sba; + struct gprs_rlcmac_dl_tbf *dl_tbf = NULL; + struct gprs_rlcmac_ul_tbf *ul_tbf = NULL; + struct gprs_rlcmac_ul_tbf *new_ul_tbf = NULL; + if (!(item = pdch_ulc_get_node(ulc, fn))) { LOGPDCH(this, DRLCMAC, LOGL_NOTICE, "FN=%u PKT RESOURCE REQ: " @@ -655,144 +661,15 @@ return; } - if (request->ID.UnionType) { - struct gprs_rlcmac_ul_tbf *ul_tbf = NULL; - struct gprs_rlcmac_dl_tbf *dl_tbf = NULL; + /* First gather MS from TLLI/DL_TFI/UL_TFI:*/ + if (request->ID.UnionType) { /* ID_TYPE = TLLI */ uint32_t tlli = request->ID.u.TLLI; - - GprsMs *ms = bts_ms_by_tlli(bts, tlli, GSM_RESERVED_TMSI); + ms = bts_ms_by_tlli(bts, tlli, GSM_RESERVED_TMSI); if (!ms) { ms = bts_alloc_ms(bts, 0, 0); /* ms class updated later */ ms_set_tlli(ms, tlli); } - - /* Keep the ms, even if it gets idle temporarily */ - ms_ref(ms); - - switch (item->type) { - case PDCH_ULC_NODE_TBF_USF: - /* Is it actually valid for an MS to send a PKT Res Req during USF? */ - ul_tbf = item->tbf_usf.ul_tbf; - LOGPDCH(this, DRLCMAC, LOGL_NOTICE, "FN=%u PKT RESOURCE REQ: " - "Unexpectedly received, waiting USF of %s\n", - fn, tbf_name(item->tbf_usf.ul_tbf)); - /* Ignore it, let common path expire related ULC entry */ - goto return_unref; - case PDCH_ULC_NODE_SBA: - sba = item->sba.sba; - LOGPDCH(this, DRLCMAC, LOGL_DEBUG, "FN=%u PKT RESOURCE REQ: " - "MS requests UL TBF throguh SBA\n", fn); - ms_set_ta(ms, sba->ta); - sba_free(sba); - /* If MS identified by TLLI sent us a PktResReq through SBA, it means it came - * from CCCH, so it's for sure not using previous UL - * TBF; drop it if it still exits on our end: */ - if ((ul_tbf = ms_ul_tbf(ms))) { - /* Get rid of previous finished UL TBF before providing a new one */ - LOGPTBFUL(ul_tbf, LOGL_NOTICE, - "Got PACKET RESOURCE REQ while TBF not finished, killing pending UL TBF\n"); - tbf_free(ul_tbf); - ul_tbf = NULL; - } - /* Similarly, it is for sure not using any DL-TBF. We - * still may have some because we were trying to assign a - * DL-TBF over CCCH when the MS proactively requested for a - * UL-TBF. In that case we'll need to re-assigna new DL-TBF through PACCH when contention resolution is done: */ - if ((dl_tbf = ms_dl_tbf(ms))) { - /* Get rid of previous finished UL TBF before providing a new one */ - LOGPTBFDL(dl_tbf, LOGL_NOTICE, - "Got PACKET RESOURCE REQ while DL-TBF pending, killing it\n"); - tbf_free(dl_tbf); - dl_tbf = NULL; - } - break; - case PDCH_ULC_NODE_TBF_POLL: - if (item->tbf_poll.poll_tbf->direction != GPRS_RLCMAC_UL_TBF) { - LOGPDCH(this, DRLCMAC, LOGL_NOTICE, "FN=%u PKT RESOURCE REQ: " - "Unexpectedly received for DL TBF %s\n", fn, - tbf_name(item->tbf_poll.poll_tbf)); - /* let common path expire the poll */ - goto return_unref; - } - ul_tbf = tbf_as_ul_tbf(item->tbf_poll.poll_tbf); - if (item->tbf_poll.reason != PDCH_ULC_POLL_UL_ACK) { - LOGPDCH(this, DRLCMAC, LOGL_NOTICE, "FN=%u PKT RESOURCE REQ: " - "Unexpectedly received, waiting for poll reason %d\n", - fn, item->tbf_poll.reason); - /* let common path expire the poll */ - goto return_unref; - } - if (ul_tbf != ms_ul_tbf(ms)) { - LOGPDCH(this, DRLCMAC, LOGL_NOTICE, "FN=%u PKT RESOURCE REQ: " - "Unexpected TLLI 0x%08x received vs exp 0x%08x\n", - fn, tlli, ul_tbf->tlli()); - /* let common path expire the poll */ - goto return_unref; - } - /* 3GPP TS 44.060 $ 9.3.3.3 */ - LOGPTBFUL(ul_tbf, LOGL_DEBUG, "FN=%u PKT RESOURCE REQ: " - "MS requests reuse of finished UL TBF in RRBP " - "block of final UL ACK/NACK\n", fn); - ul_tbf->n_reset(N3103); - pdch_ulc_release_node(ulc, item); - rc = osmo_fsm_inst_dispatch(ul_tbf->state_fi, TBF_EV_FINAL_UL_ACK_CONFIRMED, (void*)true); - if (rc) { - /* FSM failed handling, get rid of previous finished UL TBF before providing a new one */ - LOGPTBFUL(ul_tbf, LOGL_NOTICE, - "Got PACKET RESOURCE REQ while TBF not finished, killing pending UL TBF\n"); - tbf_free(ul_tbf); - } /* else: ul_tbf has been freed by state_fsm */ - ul_tbf = NULL; - break; - default: - OSMO_ASSERT(0); - } - - /* Here ul_tbf is NULL: - * - SBA case: no previous TBF) and in - * - POLL case: PktResReq is a final ACk confirmation and ul_tbf was freed - */ - - if (request->Exist_MS_Radio_Access_capability2) { - uint8_t ms_class, egprs_ms_class; - ms_class = get_ms_class_by_capability(&request->MS_Radio_Access_capability2); - egprs_ms_class = get_egprs_ms_class_by_capability(&request->MS_Radio_Access_capability2); - if (ms_class) - ms_set_ms_class(ms, ms_class); - if (egprs_ms_class) - ms_set_egprs_ms_class(ms, egprs_ms_class); - } - - /* get measurements */ - get_meas(meas, request); - ms_update_l1_meas(ms, meas); - - ul_tbf = ms_new_ul_tbf_assigned_pacch(ms, trx_no()); - if (!ul_tbf) { - handle_tbf_reject(bts, ms, trx_no(), ts_no); - goto return_unref; - } - - /* Set control TS to the TS where this PktResReq was received, - * which in practice happens to be the control_ts from the - * previous UL-TBF or SBA. When CTRL ACK is received as RRBP of the Pkt - * UL Ass scheduled below, then TBF_EV_ASSIGN_ACK_PACCH will be - * sent to tbf_fsm which will call tbf_assign_control_ts(), - * effectively setting back control_ts to - * tbf->initial_common_ts. */ - LOGPTBF(ul_tbf, LOGL_INFO, "change control TS %d -> %d until assignment is complete.\n", - ul_tbf->control_ts, ts_no); - - ul_tbf->control_ts = ts_no; - /* schedule uplink assignment */ - osmo_fsm_inst_dispatch(ul_tbf->ul_ass_fsm.fi, TBF_UL_ASS_EV_SCHED_ASS, NULL); -return_unref: - ms_unref(ms); - return; - } - - if (request->ID.u.Global_TFI.UnionType) { - struct gprs_rlcmac_dl_tbf *dl_tbf; + } else if (request->ID.u.Global_TFI.UnionType) { /* ID_TYPE = DL_TFI */ int8_t tfi = request->ID.u.Global_TFI.u.DOWNLINK_TFI; dl_tbf = bts_dl_tbf_by_tfi(bts, tfi, trx_no(), ts_no); if (!dl_tbf) { @@ -800,12 +677,12 @@ return; } LOGPTBFDL(dl_tbf, LOGL_ERROR, - "RX: [PCU <- BTS] FIXME: Packet resource request\n"); - + "RX: [PCU <- BTS] FIXME: Packet resource request (DL_TBF)\n"); /* Reset N3101 counter: */ dl_tbf->n_reset(N3101); - } else { - struct gprs_rlcmac_ul_tbf *ul_tbf; + ms = tbf_ms(dl_tbf_as_tbf(dl_tbf)); + dl_tbf = NULL; + } else { /* ID_TYPE = UL_TFI */ int8_t tfi = request->ID.u.Global_TFI.u.UPLINK_TFI; ul_tbf = bts_ul_tbf_by_tfi(bts, tfi, trx_no(), ts_no); if (!ul_tbf) { @@ -813,11 +690,163 @@ return; } LOGPTBFUL(ul_tbf, LOGL_ERROR, - "RX: [PCU <- BTS] FIXME: Packet resource request\n"); - + "RX: [PCU <- BTS] FIXME: Packet resource request (UL_TBF)\n"); /* Reset N3101 counter: */ ul_tbf->n_reset(N3101); + ms = tbf_ms(ul_tbf_as_tbf(ul_tbf)); + ul_tbf = NULL; } + + + /* Here ms is available (non-null). Keep the ms, even if it gets idle + * temporarily, in order to avoid it being freed if we free any of its + * resources (TBF). */ + OSMO_ASSERT(ms); + ms_ref(ms); + + + switch (item->type) { + case PDCH_ULC_NODE_TBF_USF: + /* TS 44.060 to 8.1.1.1.2: An MS can send a PKT Res Req during USF, for instance to change its Radio Priority. */ + ul_tbf = item->tbf_usf.ul_tbf; + if (tbf_ms(ul_tbf_as_tbf(ul_tbf)) != ms) { + LOGPDCH(this, DRLCMAC, LOGL_NOTICE, "FN=%u PKT RESOURCE REQ: " + "Received from unexpected %s vs exp %s\n", fn, + ms_name_buf(ms, buf, sizeof(buf)), ms_name(tbf_ms(ul_tbf_as_tbf(ul_tbf)))); + /* let common path expire the poll */ + goto return_unref; + } + LOGPDCH(this, DRLCMAC, LOGL_NOTICE, "FN=%u PKT RESOURCE REQ: " + "MS requests UL TBF upgrade through USF of %s\n", + fn, tbf_name(item->tbf_usf.ul_tbf)); + pdch_ulc_release_node(ulc, item); + /* FIXME OS#4947: Currenty we free the UL TBF and create a new one below in order to trigger the Pkt UL Ass. + * We should instead keep the same UL TBF working and upgrading it (modify ul_tbf->ul_ass_fsm.fi to + * handle the PktUlAss + PktCtrlAck). + * This is requried by spec, and MS are know to continue using the TBF (due to delay in between DL and + * UL scheduling). + * TS 44.060 to 8.1.1.1.2: + * "If the mobile station or the network does not support multiple TBF procedures, then after the + * transmission of the PACKET RESOURCE REQUEST message with the reason for changing PFI, the radio + * priority or peak throughput class of an assigned uplink TBF the mobile station continue to use the + * currently assigned uplink TBF assuming that the requested radio priority or peak throughput class is + * already assigned to that TBF." + */ + tbf_free(ul_tbf); + ul_tbf = NULL; + break; + case PDCH_ULC_NODE_SBA: + sba = item->sba.sba; + LOGPDCH(this, DRLCMAC, LOGL_DEBUG, "FN=%u PKT RESOURCE REQ: " + "MS requests UL TBF throguh SBA\n", fn); + ms_set_ta(ms, sba->ta); + sba_free(sba); + /* If MS identified by TLLI sent us a PktResReq through SBA, it means it came + * from CCCH, so it's for sure not using previous UL + * TBF; drop it if it still exits on our end: */ + if ((ul_tbf = ms_ul_tbf(ms))) { + /* Get rid of previous finished UL TBF before providing a new one */ + LOGPTBFUL(ul_tbf, LOGL_NOTICE, + "Got PACKET RESOURCE REQ while TBF not finished, killing pending UL TBF\n"); + tbf_free(ul_tbf); + ul_tbf = NULL; + } + /* Similarly, it is for sure not using any DL-TBF. We + * still may have some because we were trying to assign a + * DL-TBF over CCCH when the MS proactively requested for a + * UL-TBF. In that case we'll need to re-assigna new DL-TBF through PACCH when contention resolution is done: */ + if ((dl_tbf = ms_dl_tbf(ms))) { + /* Get rid of previous finished UL TBF before providing a new one */ + LOGPTBFDL(dl_tbf, LOGL_NOTICE, + "Got PACKET RESOURCE REQ while DL-TBF pending, killing it\n"); + tbf_free(dl_tbf); + dl_tbf = NULL; + } + break; + case PDCH_ULC_NODE_TBF_POLL: + if (item->tbf_poll.poll_tbf->direction != GPRS_RLCMAC_UL_TBF) { + LOGPDCH(this, DRLCMAC, LOGL_NOTICE, "FN=%u PKT RESOURCE REQ: " + "Unexpectedly received for DL TBF %s\n", fn, + tbf_name(item->tbf_poll.poll_tbf)); + /* let common path expire the poll */ + goto return_unref; + } + ul_tbf = tbf_as_ul_tbf(item->tbf_poll.poll_tbf); + if (item->tbf_poll.reason != PDCH_ULC_POLL_UL_ACK) { + LOGPDCH(this, DRLCMAC, LOGL_NOTICE, "FN=%u PKT RESOURCE REQ: " + "Unexpectedly received, waiting for poll reason %d\n", + fn, item->tbf_poll.reason); + /* let common path expire the poll */ + goto return_unref; + } + if (tbf_ms(ul_tbf_as_tbf(ul_tbf)) != ms) { + LOGPDCH(this, DRLCMAC, LOGL_NOTICE, "FN=%u PKT RESOURCE REQ: " + "Received from unexpected %s vs exp %s\n", fn, + ms_name_buf(ms, buf, sizeof(buf)), ms_name(tbf_ms(ul_tbf_as_tbf(ul_tbf)))); + /* let common path expire the poll */ + goto return_unref; + } + /* 3GPP TS 44.060 $ 9.3.3.3 */ + LOGPTBFUL(ul_tbf, LOGL_DEBUG, "FN=%u PKT RESOURCE REQ: " + "MS requests reuse of finished UL TBF in RRBP " + "block of final UL ACK/NACK\n", fn); + ul_tbf->n_reset(N3103); + pdch_ulc_release_node(ulc, item); + rc = osmo_fsm_inst_dispatch(ul_tbf->state_fi, TBF_EV_FINAL_UL_ACK_CONFIRMED, (void*)true); + if (rc) { + /* FSM failed handling, get rid of previous finished UL TBF before providing a new one */ + LOGPTBFUL(ul_tbf, LOGL_NOTICE, + "Got PACKET RESOURCE REQ while TBF not finished, killing pending UL TBF\n"); + tbf_free(ul_tbf); + } /* else: ul_tbf has been freed by state_fsm */ + ul_tbf = NULL; + break; + default: + OSMO_ASSERT(0); + } + + /* Here ul_tbf is NULL: + * - USF case: Previous TBF is explicitly freed (FIXME: this is incorrect, old TBF should be kept in this case) + * - SBA case: no previous TBF + * - POLL case: PktResReq is a final ACk confirmation and ul_tbf was freed + */ + + if (request->Exist_MS_Radio_Access_capability2) { + uint8_t ms_class, egprs_ms_class; + ms_class = get_ms_class_by_capability(&request->MS_Radio_Access_capability2); + egprs_ms_class = get_egprs_ms_class_by_capability(&request->MS_Radio_Access_capability2); + if (ms_class) + ms_set_ms_class(ms, ms_class); + if (egprs_ms_class) + ms_set_egprs_ms_class(ms, egprs_ms_class); + } + + /* get measurements */ + get_meas(meas, request); + ms_update_l1_meas(ms, meas); + + new_ul_tbf = ms_new_ul_tbf_assigned_pacch(ms, trx_no()); + if (!new_ul_tbf) { + handle_tbf_reject(bts, ms, trx_no(), ts_no); + goto return_unref; + } + + /* Set control TS to the TS where this PktResReq was received, + * which in practice happens to be the control_ts from the + * previous UL-TBF or SBA. When CTRL ACK is received as RRBP of the Pkt + * UL Ass scheduled below, then TBF_EV_ASSIGN_ACK_PACCH will be + * sent to tbf_fsm which will call tbf_assign_control_ts(), + * effectively setting back control_ts to tbf->initial_common_ts. + */ + LOGPTBF(new_ul_tbf, LOGL_INFO, "change control TS %d -> %d until assignment is complete.\n", + new_ul_tbf->control_ts, ts_no); + + new_ul_tbf->control_ts = ts_no; + /* schedule uplink assignment */ + osmo_fsm_inst_dispatch(new_ul_tbf->ul_ass_fsm.fi, TBF_UL_ASS_EV_SCHED_ASS, NULL); +return_unref: + ms_unref(ms); + return; } void gprs_rlcmac_pdch::rcv_measurement_report(Packet_Measurement_Report_t *report, uint32_t fn) -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/30293 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ie6b1b438d26cd977f88ddb4eff6b3041e0739d92 Gerrit-Change-Number: 30293 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 7 months

1
0
0 0

Change in osmo-ttcn3-hacks[master]: {sgsn,gbproxy}: do not link non-existent TCCConversion.hh

by osmith

Attention is currently required from: fixeria. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/30286 ) Change subject: {sgsn,gbproxy}: do not link non-existent TCCConversion.hh ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/30286 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: If1a3b0bac7438aa49faa905c8879d096248edfb7 Gerrit-Change-Number: 30286 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 24 Nov 2022 12:27:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 7 months

1
0
0 0

Change in osmo-ttcn3-hacks[master]: library/Osmocom_Gb_Types: fix wrong IEI in ts_BSSGP_LLC_PDU

by osmith

Attention is currently required from: fixeria. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/30285 ) Change subject: library/Osmocom_Gb_Types: fix wrong IEI in ts_BSSGP_LLC_PDU ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/30285 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: I219119a4e10e7a8e2ff40aabc1e526568c06f50a Gerrit-Change-Number: 30285 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 24 Nov 2022 12:27:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 7 months

1
0
0 0

Change in libosmo-netif[master]: osmux_output: Refactor init code to avoid calling deprecated APIs int...

by osmith

Attention is currently required from: laforge, pespin, fixeria. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/30292 ) Change subject: osmux_output: Refactor init code to avoid calling deprecated APIs internally ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/30292 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Ie8e168740c0421edd96013620256aab0306dc6c5 Gerrit-Change-Number: 30292 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-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 24 Nov 2022 12:25:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 7 months

1
0
0 0

Change in libosmo-netif[master]: osmux_output: Refactor init code to avoid calling deprecated APIs int...

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/30292 ) Change subject: osmux_output: Refactor init code to avoid calling deprecated APIs internally ...................................................................... osmux_output: Refactor init code to avoid calling deprecated APIs internally Current internal use of APIs marked as deprecated seems to be causing issues when building on debian unstable. Simply rearrange the init code to an internal helper function to avoid code duplication while still keeping the old deprecated APIs working as they used to, without getting deprecation warnings at buildtime. Related: OS#5677 Change-Id: Ie8e168740c0421edd96013620256aab0306dc6c5 --- M src/osmux_output.c 1 file changed, 16 insertions(+), 13 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/92/30292/1 diff --git a/src/osmux_output.c b/src/osmux_output.c index 33d977b..c240578 100644 --- a/src/osmux_output.c +++ b/src/osmux_output.c @@ -294,6 +294,18 @@ return 0; } +/* Placeholder to avoid init code duplication while keeping backward + * compatilbility with deprecated osmux_xfrm_output_init{2}() APIs. */ +static void _osmux_xfrm_output_init(struct osmux_out_handle *h, uint32_t rtp_ssrc, uint8_t rtp_payload_type) +{ + h->rtp_seq = (uint16_t)random(); + h->rtp_timestamp = (uint32_t)random(); + h->rtp_ssrc = rtp_ssrc; + h->rtp_payload_type = rtp_payload_type; + INIT_LLIST_HEAD(&h->list); + osmo_timer_setup(&h->timer, osmux_xfrm_output_trigger, h); +} + /*! \brief Allocate a new osmux out handle * \param[in] ctx talloc context to use when allocating the returned struct * \return Allocated osmux out handle @@ -310,12 +322,7 @@ h = talloc_zero(ctx, struct osmux_out_handle); OSMO_ASSERT(h); - h->rtp_seq = (uint16_t)random(); - h->rtp_timestamp = (uint32_t)random(); - h->rtp_ssrc = (uint32_t)random(); - h->rtp_payload_type = 98; - INIT_LLIST_HEAD(&h->list); - osmo_timer_setup(&h->timer, osmux_xfrm_output_trigger, h); + _osmux_xfrm_output_init(h, (uint32_t)random(), 98); talloc_set_destructor(h, osmux_xfrm_output_talloc_destructor); return h; @@ -325,19 +332,15 @@ void osmux_xfrm_output_init2(struct osmux_out_handle *h, uint32_t rtp_ssrc, uint8_t rtp_payload_type) { memset(h, 0, sizeof(*h)); - h->rtp_seq = (uint16_t)random(); - h->rtp_timestamp = (uint32_t)random(); - h->rtp_ssrc = rtp_ssrc; - h->rtp_payload_type = rtp_payload_type; - INIT_LLIST_HEAD(&h->list); - osmo_timer_setup(&h->timer, osmux_xfrm_output_trigger, h); + _osmux_xfrm_output_init(h, rtp_ssrc, rtp_payload_type); } /* DEPRECATED: Use osmux_xfrm_output_alloc() and osmux_xfrm_output_set_rtp_*() instead */ void osmux_xfrm_output_init(struct osmux_out_handle *h, uint32_t rtp_ssrc) { /* backward compatibility with old users, where 98 was harcoded in osmux_rebuild_rtp() */ - osmux_xfrm_output_init2(h, rtp_ssrc, 98); + memset(h, 0, sizeof(*h)); + _osmux_xfrm_output_init(h, rtp_ssrc, 98); } /*! \brief Set transmission callback to call when a generated RTP packet is to be transmitted -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/30292 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Ie8e168740c0421edd96013620256aab0306dc6c5 Gerrit-Change-Number: 30292 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 7 months

1
0
0 0

Change in osmo-ggsn[master]: gtp: Introduce VTY configurable GTP timer X3

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ggsn/+/30279 ) Change subject: gtp: Introduce VTY configurable GTP timer X3 ...................................................................... gtp: Introduce VTY configurable GTP timer X3 This timer controls the amount of time a resp message transmitted by the local gsn is to be stored in the resp queue. This is used in order to detect duplicate requests received, since GTP states the exact same response should be answered if a duplicate request is received. Prior to this patch, this timer was hardcoded to 60 seconds. This patch actually should be set, in general, to a value equal than (T3-RESPONSE * N3-REQUESTS) values configured at the peer, since that is the maximum period during which the local gsn expects to receive req retransmissions from the peer. Hence, this value must be user configurable to adapt it to the peers connected to the GSN. The 60 seconds hardcoded value is therefore changed to default to our local (T3-RESPONSE * N3-REQUESTS), since the most common scenario for osmo-ggsn/osmo-sgsn is to run it against a peer osmo-sgsn/osmo-ggsn, which will have the same values by default. This way we avoid by default caching response messages for way too long, potentially filling the queue. Related: OS#5485 Change-Id: Ia15c1cfd201d7c43e9a1d6ceb6725ddf392d2c65 --- M gtp/gsn.c M gtp/gsn.h M gtp/gtp.c 3 files changed, 7 insertions(+), 1 deletion(-) Approvals: Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/gtp/gsn.c b/gtp/gsn.c index da30fc9..ce66244 100644 --- a/gtp/gsn.c +++ b/gtp/gsn.c @@ -108,6 +108,9 @@ { .T = GTP_GSN_TIMER_N3_REQUESTS, .default_val = 3, .unit = OSMO_TDEF_CUSTOM, .desc = "Counter N3-REQUESTS holds the maximum number of attempts made by GTP to send a request message" }, + { .T = GTP_GSN_TIMER_T3_HOLD_RESPONSE, .default_val = 5 * 3 /* (GTP_GSN_TIMER_T3_RESPONSE * GTP_GSN_TIMER_N3_REQUESTS) */, .unit = OSMO_TDEF_S, + .desc = "Time a GTP respoonse message is kept cached to re-transmit it when a duplicate request is received. Value is generally equal to (T3-RESPONSE * N3-REQUESTS) set at the peer" + }, {} }; diff --git a/gtp/gsn.h b/gtp/gsn.h index 3b9cb1d..8e47ce3 100644 --- a/gtp/gsn.h +++ b/gtp/gsn.h @@ -67,6 +67,7 @@ enum gtp_gsn_timers { GTP_GSN_TIMER_T3_RESPONSE = 3, GTP_GSN_TIMER_N3_REQUESTS = 1003, + GTP_GSN_TIMER_T3_HOLD_RESPONSE = -3, }; struct gsn_t { diff --git a/gtp/gtp.c b/gtp/gtp.c index f32461c..1ebc6dc 100644 --- a/gtp/gtp.c +++ b/gtp/gtp.c @@ -499,11 +499,13 @@ LOGP(DLGTP, LOGL_ERROR, "Retransmit resp queue is full (seq=%" PRIu16 ")\n", seq); } else { + unsigned int t3_hold_resp; LOGP(DLGTP, LOGL_DEBUG, "Registering seq=%" PRIu16 " in restransmit resp queue\n", seq); + t3_hold_resp = osmo_tdef_get(gsn->tdef, GTP_GSN_TIMER_T3_HOLD_RESPONSE, OSMO_TDEF_S, -1); memcpy(&qmsg->p, packet, sizeof(union gtp_packet)); qmsg->l = len; - qmsg->timeout = time(NULL) + 60; /* When to timeout */ + qmsg->timeout = time(NULL) + t3_hold_resp; /* When to timeout */ qmsg->retrans = 0; /* No retransmissions so far */ qmsg->cbp = NULL; qmsg->type = 0; -- To view, visit https://gerrit.osmocom.org/c/osmo-ggsn/+/30279 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ggsn Gerrit-Branch: master Gerrit-Change-Id: Ia15c1cfd201d7c43e9a1d6ceb6725ddf392d2c65 Gerrit-Change-Number: 30279 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 7 months

1
0
0 0

Change in osmo-ggsn[master]: gtp: Introduce VTY configurable GTP timer X3

by pespin

Attention is currently required from: osmith, neels, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ggsn/+/30279 ) Change subject: gtp: Introduce VTY configurable GTP timer X3 ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ggsn/+/30279 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ggsn Gerrit-Branch: master Gerrit-Change-Id: Ia15c1cfd201d7c43e9a1d6ceb6725ddf392d2c65 Gerrit-Change-Number: 30279 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 24 Nov 2022 11:25:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 7 months

1
0
0 0

Change in osmo-hnbgw[master]: hnbgw: Avoid allocating SCCP conn id >0x00fffffe

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/30283 ) Change subject: hnbgw: Avoid allocating SCCP conn id >0x00fffffe ...................................................................... hnbgw: Avoid allocating SCCP conn id >0x00fffffe This fixes bug in use of M3UA/SCCP after 2**24 connection IDs have been allocated. Related: SYS#6211 Change-Id: I03bad960f65fbff6e467def5bba60fefb328f962 --- M src/osmo-hnbgw/context_map.c 1 file changed, 14 insertions(+), 1 deletion(-) Approvals: fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/osmo-hnbgw/context_map.c b/src/osmo-hnbgw/context_map.c index c069651..7549942 100644 --- a/src/osmo-hnbgw/context_map.c +++ b/src/osmo-hnbgw/context_map.c @@ -58,8 +58,21 @@ uint32_t i; uint32_t id; - for (i = 0; i < 0xffffffff; i++) { + /* SUA: RFC3868 sec 3.10.4: + * The source reference number is a 4 octet long integer. + * This is allocated by the source SUA instance. + * M3UA/SCCP: ITU-T Q.713 sec 3.3: + * The "source local reference" parameter field is a three-octet field containing a + * reference number which is generated and used by the local node to identify the + * connection section after the connection section is set up. + * The coding "all ones" is reserved for future use. + * Hence, let's simply use 24 bit ids to fit all link types (excluding 0x00ffffff). + */ + + for (i = 0; i < 0x00ffffff; i++) { id = cn->next_conn_id++; + if (cn->next_conn_id == 0x00ffffff) + cn->next_conn_id = 0; if (!cn_id_in_use(cn, id)) { *id_out = id; return 1; -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/30283 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I03bad960f65fbff6e467def5bba60fefb328f962 Gerrit-Change-Number: 30283 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 7 months

1
0
0 0

2025

2024

2023

2022

gerrit-log November 2022