From 2a9ba66922b6292a7ad90038ea1d6077213aeab0 Mon Sep 17 00:00:00 2001 From: Philipp Maier Date: Thu, 9 Feb 2023 13:39:31 +0100 Subject: [PATCH] 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. Depends: libosmo-abis.git I073cfaba0d5073447842f22665e213135ea3f635 Change-Id: Ie6a32abbc5cd984f6d72a384e3b47c1b82ce7058 Related: OS#5198 --- include/osmocom/mgcp/mgcp_e1.h | 2 +- include/osmocom/mgcp/mgcp_trunk.h | 2 +- src/libosmo-mgcp/mgcp_e1.c | 87 +++++++++++++++++++++++++------ src/libosmo-mgcp/mgcp_endp.c | 6 ++- 4 files changed, 78 insertions(+), 19 deletions(-) diff --git a/include/osmocom/mgcp/mgcp_e1.h b/include/osmocom/mgcp/mgcp_e1.h index f0cf3ec3a..fbd594ca9 100644 --- a/include/osmocom/mgcp/mgcp_e1.h +++ b/include/osmocom/mgcp/mgcp_e1.h @@ -23,5 +23,5 @@ static const uint8_t e1_offsets[] = { 0, 0, 4, 0, 2, 4, 6, 0, 1, 2, 3, 4, 5, 6, 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 3f14f97ac..33c3d5b03 100644 --- a/include/osmocom/mgcp/mgcp_trunk.h +++ b/include/osmocom/mgcp/mgcp_trunk.h @@ -66,7 +66,7 @@ struct mgcp_trunk { /* E1 specific */ struct { unsigned int vty_line_nr; - bool ts_in_use[NUM_E1_TS-1]; + uint8_t ts_usecount[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 24f297f6e..c1aeadd20 100644 --- a/src/libosmo-mgcp/mgcp_e1.c +++ b/src/libosmo-mgcp/mgcp_e1.c @@ -365,13 +365,11 @@ static void e1_recv_cb(struct e1inp_ts *ts, struct msgb *msg) 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 subslots. 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 below). This function must + * be called any time a new I.460 subslot is opened in order to maintain constancy of the ts_usecount counter. */ struct e1inp_line *e1_line; int rc; @@ -379,12 +377,14 @@ static int e1_init(struct mgcp_trunk *trunk, uint8_t ts_nr) 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_usecount[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_usecount[ts_nr - 1]); + trunk->e1.ts_usecount[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,56 @@ static int e1_init(struct mgcp_trunk *trunk, uint8_t ts_nr) } 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_usecount[ts_nr - 1]++; + OSMO_ASSERT(trunk->e1.ts_usecount[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_usecount[ts_nr - 1] > 1) { + trunk->e1.ts_usecount[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_usecount[ts_nr - 1]); + return 0; + } else if (trunk->e1.ts_usecount[ts_nr - 1] == 0) { + /* This should not be as it means we close the timeslot too often. */ + LOGPTRUNK(trunk, DE1, LOGL_ERROR, "E1 timeslot %u already closed, leaving it as it is...\n", ts_nr); + return -EINVAL; + } + + /* 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 */ + rc = e1inp_ts_config_none(&e1_line->ts[ts_nr - 1], e1_line); + if (rc < 0) { + LOGPTRUNK(trunk, DE1, LOGL_ERROR, "failed to disable E1 timeslot %u.\n", ts_nr); + return -EINVAL; + } + rc = e1inp_line_update(e1_line); + if (rc < 0) { + LOGPTRUNK(trunk, DE1, LOGL_ERROR, "failed to update E1 line %u.\n", trunk->e1.vty_line_nr); + return -EINVAL; + } + + LOGPTRUNK(trunk, DE1, LOGL_INFO, "E1 timeslot %u closed.\n", ts_nr); + trunk->e1.ts_usecount[ts_nr - 1]--; + OSMO_ASSERT(trunk->e1.ts_usecount[ts_nr - 1] == 0); return 0; } @@ -496,7 +545,7 @@ static bool tf_type_is_amr(enum osmo_trau_frame_type ft) } } -/*! 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 +566,7 @@ int mgcp_e1_endp_equip(struct mgcp_endpoint *endp, uint8_t ts, uint8_t ss, uint8 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 +653,15 @@ void mgcp_e1_endp_update(struct mgcp_endpoint *endp) } /*! 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) { + /* Guard against multiple calls. In case we don't 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 +670,10 @@ void mgcp_e1_endp_release(struct mgcp_endpoint *endp) 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 03e533021..2bf15e0cf 100644 --- a/src/libosmo-mgcp/mgcp_endp.c +++ b/src/libosmo-mgcp/mgcp_endp.c @@ -671,7 +671,9 @@ void mgcp_endp_release(struct mgcp_endpoint *endp) 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); + } }