dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/31269 ) Change subject: mgcp_e1: be more frugal withe E1 line resources ...................................................................... mgcp_e1: be more frugal withe E1 line resources At the moment we open an E1 timeslot when needed, but we never close it even when it is not needed anymore. This may block other entities from using it. Lets add some logic to detect whether the E1 timeslot is still needed and make sure that it is closed when it is no longer needed. Change-Id: Ie6a32abbc5cd984f6d72a384e3b47c1b82ce7058 Related: OS#5198 --- M include/osmocom/mgcp/mgcp_e1.h M include/osmocom/mgcp/mgcp_trunk.h M src/libosmo-mgcp/mgcp_e1.c M src/libosmo-mgcp/mgcp_endp.c 4 files changed, 75 insertions(+), 19 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/69/31269/1 diff --git a/include/osmocom/mgcp/mgcp_e1.h b/include/osmocom/mgcp/mgcp_e1.h index f0cf3ec..fbd594c 100644 --- a/include/osmocom/mgcp/mgcp_e1.h +++ b/include/osmocom/mgcp/mgcp_e1.h @@ -23,5 +23,5 @@ int mgcp_e1_endp_equip(struct mgcp_endpoint *endp, uint8_t ts, uint8_t ss, uint8_t offs); void mgcp_e1_endp_update(struct mgcp_endpoint *endp); -void mgcp_e1_endp_release(struct mgcp_endpoint *endp); +void mgcp_e1_endp_release(struct mgcp_endpoint *endp, uint8_t ts); int mgcp_e1_send_rtp(struct mgcp_endpoint *endp, struct mgcp_rtp_codec *codec, struct msgb *msg); diff --git a/include/osmocom/mgcp/mgcp_trunk.h b/include/osmocom/mgcp/mgcp_trunk.h index 3f14f97..a8257bb 100644 --- a/include/osmocom/mgcp/mgcp_trunk.h +++ b/include/osmocom/mgcp/mgcp_trunk.h @@ -66,7 +66,7 @@ /* E1 specific */ struct { unsigned int vty_line_nr; - bool ts_in_use[NUM_E1_TS-1]; + uint8_t ts_in_use[NUM_E1_TS-1]; struct osmo_i460_timeslot i460_ts[NUM_E1_TS-1]; /* Note: on an E1 line TS 0 is devoted to framing and * alignment and therefore only NUM_E1_TS-1 timeslots diff --git a/src/libosmo-mgcp/mgcp_e1.c b/src/libosmo-mgcp/mgcp_e1.c index 7f30c1d..51b5caf 100644 --- a/src/libosmo-mgcp/mgcp_e1.c +++ b/src/libosmo-mgcp/mgcp_e1.c @@ -365,13 +365,11 @@ msgb_free(msg); } -static int e1_init(struct mgcp_trunk *trunk, uint8_t ts_nr) +static int e1_open(struct mgcp_trunk *trunk, uint8_t ts_nr) { - /*! Each timeslot needs only to be configured once. The Timeslot then - * stays open and permanently receives data. It is then up to the - * I.460 demultiplexer to add/remove subchannels as needed. It is - * allowed to call this function multiple times since we check if the - * timeslot is already configured. */ + /*! One E1 timeslot may serve multiple I.460 subslot. The timeslot is opened as soon as an I.460 subslot is + * opened and will stay open until the last I.460 subslot is closed (see e1_close belos). This function must + * be called any time a new I.460 subslot is opened in order to maintain constancy of the ts_in_use counter. */ struct e1inp_line *e1_line; int rc; @@ -379,12 +377,14 @@ OSMO_ASSERT(ts_nr > 0 || ts_nr < NUM_E1_TS); cfg = trunk->cfg; - if (trunk->e1.ts_in_use[ts_nr - 1]) { - LOGPTRUNK(trunk, DE1, LOGL_INFO, "E1 timeslot %u already set up, skipping...\n", ts_nr); + if (trunk->e1.ts_in_use[ts_nr - 1] > 0) { + LOGPTRUNK(trunk, DE1, LOGL_INFO, "E1 timeslot %u already set up and in use by %u subslot(s), using it as it is...\n", + ts_nr, trunk->e1.ts_in_use[ts_nr - 1]); + trunk->e1.ts_in_use[ts_nr - 1]++; return 0; } - /* Get E1 line */ + /* Find E1 line */ e1_line = e1inp_line_find(trunk->e1.vty_line_nr); if (!e1_line) { LOGPTRUNK(trunk, DE1, LOGL_ERROR, "no such E1 line %u - check VTY config!\n", @@ -406,7 +406,53 @@ } LOGPTRUNK(trunk, DE1, LOGL_INFO, "E1 timeslot %u set up successfully.\n", ts_nr); - trunk->e1.ts_in_use[ts_nr - 1] = true; + trunk->e1.ts_in_use[ts_nr - 1]++; + OSMO_ASSERT(trunk->e1.ts_in_use[ts_nr - 1] == 1); + + return 0; +} + +static int e1_close(struct mgcp_trunk *trunk, uint8_t ts_nr) +{ + /* See also comment above (e1_open). This function must be called any time an I.460 subslot is closed */ + + struct e1inp_line *e1_line; + int rc; + + OSMO_ASSERT(ts_nr > 0 || ts_nr < NUM_E1_TS); + cfg = trunk->cfg; + + if (trunk->e1.ts_in_use[ts_nr - 1] > 1) { + trunk->e1.ts_in_use[ts_nr - 1]--; + LOGPTRUNK(trunk, DE1, LOGL_INFO, "E1 timeslot %u still in use by %u other subslot(s), leaving it open...\n", + ts_nr, trunk->e1.ts_in_use[ts_nr - 1]); + return 0; + } else if (trunk->e1.ts_in_use[ts_nr - 1] == 0) { + /* This should not be as it means we close the timeslot too often. */ + LOGPTRUNK(trunk, DE1, LOGL_INFO, "E1 timeslot %u already closed, leaving it as it is...\n", ts_nr); + return 0; + } + + /* Find E1 line */ + e1_line = e1inp_line_find(trunk->e1.vty_line_nr); + if (!e1_line) { + LOGPTRUNK(trunk, DE1, LOGL_ERROR, "no such E1 line %u - check VTY config!\n", + trunk->e1.vty_line_nr); + return -EINVAL; + } + + /* Release E1 timeslot */ + e1_line->ts[ts_nr - 1].type = E1INP_TS_TYPE_NONE; + rc = e1inp_line_update(e1_line); + if (rc < 0) { + LOGPTRUNK(trunk, DE1, LOGL_ERROR, "failed to update E1 timeslot %u.\n", ts_nr); + return -EINVAL; + } + + + LOGPTRUNK(trunk, DE1, LOGL_INFO, "E1 timeslot %u closed.\n", ts_nr); + trunk->e1.ts_in_use[ts_nr - 1]--; + OSMO_ASSERT(trunk->e1.ts_in_use[ts_nr - 1] == 0); return 0; } @@ -496,7 +542,7 @@ } } -/*! Equip E1 endpoint with I.460 mux resources. +/*! Equip E1 endpoint with I.460 mux and E1 timeslot resources. * \param[in] endp endpoint to equip * \param[in] ts E1 timeslot number. * \param[in] ss E1 subslot number. @@ -517,7 +563,7 @@ endp->e1.last_amr_ft = AMR_4_75; /* Set up E1 line / timeslot */ - rc = e1_init(endp->trunk, ts); + rc = e1_open(endp->trunk, ts); if (rc != 0) return -EINVAL; @@ -604,9 +650,15 @@ } /*! Remove E1 resources from endpoint - * \param[in] endp endpoint to release. */ -void mgcp_e1_endp_release(struct mgcp_endpoint *endp) + * \param[in] endp endpoint to release. + * \param[in] ts E1 timeslot number. */ +void mgcp_e1_endp_release(struct mgcp_endpoint *endp, uint8_t ts) { + /* Gurad against multiple calls. In case we do see a subchannel anymore we can safely assume that all work is + * done. */ + if (!(endp->e1.schan || endp->e1.trau_rtp_st || endp->e1.trau_sync_fi)) + return; + LOGPENDP(endp, DE1, LOGL_DEBUG, "removing I.460 subchannel and sync...\n"); if (endp->e1.schan) @@ -615,8 +667,10 @@ talloc_free(endp->e1.trau_rtp_st); if (endp->e1.trau_sync_fi) osmo_fsm_inst_term(endp->e1.trau_sync_fi, OSMO_FSM_TERM_REGULAR, NULL); - memset(&endp->e1, 0, sizeof(endp->e1)); + + /* Close E1 timeslot */ + e1_close(endp->trunk, ts); } /*! Accept RTP message buffer with RTP data and enqueue voice data for E1 transmit. diff --git a/src/libosmo-mgcp/mgcp_endp.c b/src/libosmo-mgcp/mgcp_endp.c index 03e5330..2bf15e0 100644 --- a/src/libosmo-mgcp/mgcp_endp.c +++ b/src/libosmo-mgcp/mgcp_endp.c @@ -671,7 +671,9 @@ talloc_free(endp->local_options.codec); endp->local_options.codec = NULL; - if (endp->trunk->trunk_type == MGCP_TRUNK_E1) - mgcp_e1_endp_release(endp); + if (endp->trunk->trunk_type == MGCP_TRUNK_E1) { + uint8_t ts = e1_ts_nr_from_epname(endp->name); + mgcp_e1_endp_release(endp, ts); + } } -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/31269 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ie6a32abbc5cd984f6d72a384e3b47c1b82ce7058 Gerrit-Change-Number: 31269 Gerrit-PatchSet: 1 Gerrit-Owner: dexter &lt;pmaier(a)sysmocom.de&gt; Gerrit-MessageType: newchange