[M] Change in osmocom-bb[master]: trxcon/l1sched: emit DATA.cnf early (on bid=0)

fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/33749 ) Change subject: trxcon/l1sched: emit DATA.cnf early (on bid=0) ...................................................................... trxcon/l1sched: emit DATA.cnf early (on bid=0) trxcon's scheduler is currently emitting DATA.cnf whenever the last burst of a DATA.req has been transmitted. This sounds logical, but makes the implementation quite complex. It's even harder to implement sending of DATA.cnf properly for CSD specific channel modes, which are to be implemented in a follow-up patch. The DATA.cnf prims trigger sending of L1CTL DATA.cnf/TRAFFIC.cnf, which are interpreted as Ready-to-Send by the upper layers (layer23). Additionally DATA.cnf prims trigger sending of GSMTAP PDUs containing the respective Uplink frames. This patch changes the l1sched logic, so that a DATA.cnf primitive is emitted whenever the respective DATA.req is dequeued and encoded using the lchan specific channel coding function. This simplifies the code a lot and prepares for the upcoming CSD support. As a bonus, this patch fixes an inconsistency between TDMA FNs reported in Uplink and Downlink GSMTAP PDUs. Now we're indicating the first Fn in both cases, so Uplink is consistent with Downlink. Change-Id: Ie09a24cd950a93edd871a9fbc5b47ec96c24cceb Related: OS#4396, OS#1572 --- M src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h M src/host/trxcon/include/osmocom/bb/l1sched/prim.h M src/host/trxcon/src/sched_lchan_common.c M src/host/trxcon/src/sched_lchan_pdtch.c M src/host/trxcon/src/sched_lchan_rach.c M src/host/trxcon/src/sched_lchan_tchf.c M src/host/trxcon/src/sched_lchan_tchh.c M src/host/trxcon/src/sched_lchan_xcch.c M src/host/trxcon/src/sched_prim.c M src/host/trxcon/src/sched_trx.c 10 files changed, 100 insertions(+), 102 deletions(-) Approvals: fixeria: Looks good to me, approved pespin: Looks good to me, but someone else must approve osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h b/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h index 5385c00..98282c1 100644 --- a/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h +++ b/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h @@ -223,8 +223,6 @@ /*! Queue of Tx primitives */ struct llist_head tx_prims; - /*! Tx primitive being sent */ - struct msgb *prim; /*! Mode for TCH channels (see GSM48_CMODE_*) */ uint8_t tch_mode; diff --git a/src/host/trxcon/include/osmocom/bb/l1sched/prim.h b/src/host/trxcon/include/osmocom/bb/l1sched/prim.h index af3ac3b..a9187c2 100644 --- a/src/host/trxcon/include/osmocom/bb/l1sched/prim.h +++ b/src/host/trxcon/include/osmocom/bb/l1sched/prim.h @@ -116,16 +116,17 @@ struct msgb *l1sched_prim_alloc(enum l1sched_prim_type type, enum osmo_prim_operation op); -bool l1sched_lchan_amr_prim_is_valid(struct l1sched_lchan_state *lchan, bool is_cmr); +bool l1sched_lchan_amr_prim_is_valid(struct l1sched_lchan_state *lchan, + struct msgb *msg, bool is_cmr); struct msgb *l1sched_lchan_prim_dequeue_sacch(struct l1sched_lchan_state *lchan); struct msgb *l1sched_lchan_prim_dequeue_tch(struct l1sched_lchan_state *lchan, bool facch); struct msgb *l1sched_lchan_prim_dummy_lapdm(const struct l1sched_lchan_state *lchan); -void l1sched_lchan_prim_drop(struct l1sched_lchan_state *lchan); int l1sched_lchan_emit_data_ind(struct l1sched_lchan_state *lchan, const uint8_t *data, size_t data_len, int n_errors, int n_bits_total, bool traffic); -int l1sched_lchan_emit_data_cnf(struct l1sched_lchan_state *lchan, uint32_t fn); +int l1sched_lchan_emit_data_cnf(struct l1sched_lchan_state *lchan, + struct msgb *msg, uint32_t fn); int l1sched_prim_from_user(struct l1sched_state *sched, struct msgb *msg); int l1sched_prim_to_user(struct l1sched_state *sched, struct msgb *msg); diff --git a/src/host/trxcon/src/sched_lchan_common.c b/src/host/trxcon/src/sched_lchan_common.c index bf31831..2308297 100644 --- a/src/host/trxcon/src/sched_lchan_common.c +++ b/src/host/trxcon/src/sched_lchan_common.c @@ -93,17 +93,18 @@ return buf; } -bool l1sched_lchan_amr_prim_is_valid(struct l1sched_lchan_state *lchan, bool is_cmr) +bool l1sched_lchan_amr_prim_is_valid(struct l1sched_lchan_state *lchan, + struct msgb *msg, bool is_cmr) { enum osmo_amr_type ft_codec; uint8_t cmr_codec; int ft, cmr, len; - len = osmo_amr_rtp_dec(msgb_l2(lchan->prim), msgb_l2len(lchan->prim), + len = osmo_amr_rtp_dec(msgb_l2(msg), msgb_l2len(msg), &cmr_codec, NULL, &ft_codec, NULL, NULL); if (len < 0) { LOGP_LCHAND(lchan, LOGL_ERROR, "Cannot send invalid AMR payload (%u): %s\n", - msgb_l2len(lchan->prim), msgb_hexdump_l2(lchan->prim)); + msgb_l2len(msg), msgb_hexdump_l2(msg)); return false; } ft = -1; diff --git a/src/host/trxcon/src/sched_lchan_pdtch.c b/src/host/trxcon/src/sched_lchan_pdtch.c index 03d43ce..001a2d9 100644 --- a/src/host/trxcon/src/sched_lchan_pdtch.c +++ b/src/host/trxcon/src/sched_lchan_pdtch.c @@ -140,19 +140,22 @@ *mask = *mask << 4; - lchan->prim = prim_dequeue_pdtch(lchan, br->fn); - if (lchan->prim == NULL) + struct msgb *msg = prim_dequeue_pdtch(lchan, br->fn); + if (msg == NULL) return -ENOENT; /* Encode payload */ - rc = gsm0503_pdtch_encode(bursts_p, msgb_l2(lchan->prim), msgb_l2len(lchan->prim)); + rc = gsm0503_pdtch_encode(bursts_p, msgb_l2(msg), msgb_l2len(msg)); if (rc < 0) { LOGP_LCHAND(lchan, LOGL_ERROR, "Failed to encode L2 payload (len=%u): %s\n", - msgb_l2len(lchan->prim), msgb_hexdump_l2(lchan->prim)); - l1sched_lchan_prim_drop(lchan); + msgb_l2len(msg), msgb_hexdump_l2(msg)); + msgb_free(msg); return -EINVAL; } + /* Confirm data / traffic sending (pass ownership of the msgb/prim) */ + l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); + send_burst: /* Determine which burst should be sent */ burst = bursts_p + br->bid * 116; @@ -173,11 +176,5 @@ LOGP_LCHAND(lchan, LOGL_DEBUG, "Scheduled at fn=%u burst=%u\n", br->fn, br->bid); - /* If we have sent the last (4/4) burst */ - if ((*mask & 0x0f) == 0x0f) { - /* Confirm data / traffic sending (pass ownership of the prim) */ - l1sched_lchan_emit_data_cnf(lchan, br->fn); - } - return 0; } diff --git a/src/host/trxcon/src/sched_lchan_rach.c b/src/host/trxcon/src/sched_lchan_rach.c index 8df7fa2..905f1d5 100644 --- a/src/host/trxcon/src/sched_lchan_rach.c +++ b/src/host/trxcon/src/sched_lchan_rach.c @@ -73,28 +73,27 @@ struct l1sched_burst_req *br) { const uint8_t bsic = lchan->ts->sched->bsic; - struct l1sched_prim *prim; uint8_t *burst_ptr = br->burst; uint8_t payload[36]; int i, rc; - if (lchan->prim == NULL) { - lchan->prim = msgb_dequeue(&lchan->tx_prims); - if (lchan->prim == NULL) - return 0; - } - prim = l1sched_prim_from_msgb(lchan->prim); + if (llist_empty(&lchan->tx_prims)) + return 0; + + struct msgb *msg = llist_first_entry(&lchan->tx_prims, struct msgb, list); + struct l1sched_prim *prim = l1sched_prim_from_msgb(msg); /* Delay sending according to offset value */ if (prim->rach_req.offset-- > 0) return 0; + llist_del(&msg->list); /* Check requested synch. sequence */ if (prim->rach_req.synch_seq >= RACH_SYNCH_SEQ_NUM) { LOGP_LCHAND(lchan, LOGL_ERROR, "Unknown RACH synch. sequence=0x%02x\n", prim->rach_req.synch_seq); - l1sched_lchan_prim_drop(lchan); + msgb_free(msg); return -ENOTSUP; } @@ -106,7 +105,7 @@ "Could not encode %s-bit RACH burst (ra=%u bsic=%u)\n", prim->rach_req.is_11bit ? "11" : "8", prim->rach_req.ra, bsic); - l1sched_lchan_prim_drop(lchan); + msgb_free(msg); return rc; } @@ -130,8 +129,8 @@ prim->rach_req.is_11bit ? "11" : "8", get_value_string(rach_synch_seq_names, prim->rach_req.synch_seq), br->fn); - /* Confirm RACH request (pass ownership of the prim) */ - l1sched_lchan_emit_data_cnf(lchan, br->fn); + /* Confirm RACH request (pass ownership of the msgb/prim) */ + l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); return 0; } diff --git a/src/host/trxcon/src/sched_lchan_tchf.c b/src/host/trxcon/src/sched_lchan_tchf.c index 0206b61..12bea57 100644 --- a/src/host/trxcon/src/sched_lchan_tchf.c +++ b/src/host/trxcon/src/sched_lchan_tchf.c @@ -229,39 +229,39 @@ memset(&bursts_p[464], 0, 464); *mask = *mask << 4; - lchan->prim = prim_dequeue_tchf(lchan); + struct msgb *msg = prim_dequeue_tchf(lchan); /* populate the buffer with bursts */ switch (lchan->tch_mode) { case GSM48_CMODE_SIGN: - if (lchan->prim == NULL) - lchan->prim = l1sched_lchan_prim_dummy_lapdm(lchan); + if (msg == NULL) + msg = l1sched_lchan_prim_dummy_lapdm(lchan); /* fall-through */ case GSM48_CMODE_SPEECH_V1: case GSM48_CMODE_SPEECH_EFR: - if (lchan->prim == NULL) { + if (msg == NULL) { /* transmit a dummy speech block with inverted CRC3 */ gsm0503_tch_fr_encode(bursts_p, NULL, 0, 1); goto send_burst; } rc = gsm0503_tch_fr_encode(bursts_p, - msgb_l2(lchan->prim), - msgb_l2len(lchan->prim), 1); + msgb_l2(msg), + msgb_l2len(msg), 1); break; case GSM48_CMODE_SPEECH_AMR: { bool amr_fn_is_cmr = !sched_tchf_ul_amr_cmi_map[br->fn % 26]; - const uint8_t *data = lchan->prim ? msgb_l2(lchan->prim) : NULL; - size_t data_len = lchan->prim ? msgb_l2len(lchan->prim) : 0; + const uint8_t *data = msg ? msgb_l2(msg) : NULL; + size_t data_len = msg ? msgb_l2len(msg) : 0; - if (lchan->prim == NULL) { + if (msg == NULL) { /* TODO: It's not clear what to do for TCH/AFS. * TODO: Send dummy FACCH maybe? */ goto send_burst; /* send something */ } if (data_len != GSM_MACBLOCK_LEN) { /* TCH/AFS: speech */ - if (!l1sched_lchan_amr_prim_is_valid(lchan, amr_fn_is_cmr)) + if (!l1sched_lchan_amr_prim_is_valid(lchan, msg, amr_fn_is_cmr)) goto free_bad_msg; /* pull the AMR header - sizeof(struct amr_hdr) */ data_len -= 2; @@ -286,12 +286,15 @@ if (rc) { LOGP_LCHAND(lchan, LOGL_ERROR, "Failed to encode L2 payload (len=%u): %s\n", - msgb_l2len(lchan->prim), msgb_hexdump_l2(lchan->prim)); + msgb_l2len(msg), msgb_hexdump_l2(msg)); free_bad_msg: - l1sched_lchan_prim_drop(lchan); + msgb_free(msg); return -EINVAL; } + /* Confirm data / traffic sending (pass ownership of the msgb/prim) */ + l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); + send_burst: /* Determine which burst should be sent */ burst = bursts_p + br->bid * 116; @@ -312,11 +315,5 @@ LOGP_LCHAND(lchan, LOGL_DEBUG, "Scheduled fn=%u burst=%u\n", br->fn, br->bid); - /* If we have sent the last (4/4) burst */ - if ((*mask & 0x0f) == 0x0f) { - /* Confirm data / traffic sending (pass ownership of the prim) */ - l1sched_lchan_emit_data_cnf(lchan, br->fn); - } - return 0; } diff --git a/src/host/trxcon/src/sched_lchan_tchh.c b/src/host/trxcon/src/sched_lchan_tchh.c index 3b329cb..38aef34 100644 --- a/src/host/trxcon/src/sched_lchan_tchh.c +++ b/src/host/trxcon/src/sched_lchan_tchh.c @@ -381,40 +381,40 @@ goto send_burst; } - lchan->prim = prim_dequeue_tchh(lchan, br->fn); + struct msgb *msg = prim_dequeue_tchh(lchan, br->fn); /* populate the buffer with bursts */ switch (lchan->tch_mode) { case GSM48_CMODE_SIGN: if (!l1sched_tchh_facch_start(lchan->type, br->fn, 1)) goto send_burst; /* XXX: should not happen */ - if (lchan->prim == NULL) - lchan->prim = l1sched_lchan_prim_dummy_lapdm(lchan); + if (msg == NULL) + msg = l1sched_lchan_prim_dummy_lapdm(lchan); /* fall-through */ case GSM48_CMODE_SPEECH_V1: - if (lchan->prim == NULL) { + if (msg == NULL) { /* transmit a dummy speech block with inverted CRC3 */ gsm0503_tch_hr_encode(bursts_p, NULL, 0); goto send_burst; } rc = gsm0503_tch_hr_encode(bursts_p, - msgb_l2(lchan->prim), - msgb_l2len(lchan->prim)); + msgb_l2(msg), + msgb_l2len(msg)); break; case GSM48_CMODE_SPEECH_AMR: { bool amr_fn_is_cmr = !sched_tchh_ul_amr_cmi_map[br->fn % 26]; - const uint8_t *data = lchan->prim ? msgb_l2(lchan->prim) : NULL; - size_t data_len = lchan->prim ? msgb_l2len(lchan->prim) : 0; + const uint8_t *data = msg ? msgb_l2(msg) : NULL; + size_t data_len = msg ? msgb_l2len(msg) : 0; - if (lchan->prim == NULL) { + if (msg == NULL) { /* TODO: It's not clear what to do for TCH/AHS. * TODO: Send dummy FACCH maybe? */ goto send_burst; /* send garbage */ } if (data_len != GSM_MACBLOCK_LEN) { /* TCH/AHS: speech */ - if (!l1sched_lchan_amr_prim_is_valid(lchan, amr_fn_is_cmr)) + if (!l1sched_lchan_amr_prim_is_valid(lchan, msg, amr_fn_is_cmr)) goto free_bad_msg; /* pull the AMR header - sizeof(struct amr_hdr) */ data_len -= 2; @@ -439,15 +439,18 @@ if (rc) { LOGP_LCHAND(lchan, LOGL_ERROR, "Failed to encode L2 payload (len=%u): %s\n", - msgb_l2len(lchan->prim), msgb_hexdump_l2(lchan->prim)); + msgb_l2len(msg), msgb_hexdump_l2(msg)); free_bad_msg: - l1sched_lchan_prim_drop(lchan); + msgb_free(msg); return -EINVAL; } - if (msgb_l2len(lchan->prim) == GSM_MACBLOCK_LEN) + if (msgb_l2len(msg) == GSM_MACBLOCK_LEN) lchan->ul_facch_blocks = 6; + /* Confirm data / traffic sending (pass ownership of the msgb/prim) */ + l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); + send_burst: /* Determine which burst should be sent */ burst = bursts_p + br->bid * 116; @@ -472,13 +475,5 @@ if (lchan->ul_facch_blocks) lchan->ul_facch_blocks--; - if ((*mask & 0x0f) == 0x0f) { - /* Confirm data / traffic sending (pass ownership of the prim) */ - if (!lchan->ul_facch_blocks) - l1sched_lchan_emit_data_cnf(lchan, br->fn); - else /* do not confirm dropped prims */ - l1sched_lchan_prim_drop(lchan); - } - return 0; } diff --git a/src/host/trxcon/src/sched_lchan_xcch.c b/src/host/trxcon/src/sched_lchan_xcch.c index e263836..b3863d5 100644 --- a/src/host/trxcon/src/sched_lchan_xcch.c +++ b/src/host/trxcon/src/sched_lchan_xcch.c @@ -139,20 +139,23 @@ *mask = *mask << 4; - lchan->prim = prim_dequeue_xcch(lchan); - if (lchan->prim == NULL) - lchan->prim = l1sched_lchan_prim_dummy_lapdm(lchan); - OSMO_ASSERT(lchan->prim != NULL); + struct msgb *msg = prim_dequeue_xcch(lchan); + if (msg == NULL) + msg = l1sched_lchan_prim_dummy_lapdm(lchan); + OSMO_ASSERT(msg != NULL); /* Encode payload */ - rc = gsm0503_xcch_encode(bursts_p, msgb_l2(lchan->prim)); + rc = gsm0503_xcch_encode(bursts_p, msgb_l2(msg)); if (rc) { LOGP_LCHAND(lchan, LOGL_ERROR, "Failed to encode L2 payload (len=%u): %s\n", - msgb_l2len(lchan->prim), msgb_hexdump_l2(lchan->prim)); - l1sched_lchan_prim_drop(lchan); + msgb_l2len(msg), msgb_hexdump_l2(msg)); + msgb_free(msg); return -EINVAL; } + /* Confirm data sending (pass ownership of the msgb/prim) */ + l1sched_lchan_emit_data_cnf(lchan, msg, br->fn); + send_burst: /* Determine which burst should be sent */ burst = bursts_p + br->bid * 116; @@ -173,11 +176,5 @@ LOGP_LCHAND(lchan, LOGL_DEBUG, "Scheduled fn=%u burst=%u\n", br->fn, br->bid); - /* If we have sent the last (4/4) burst */ - if ((*mask & 0x0f) == 0x0f) { - /* Confirm data sending (pass ownership of the prim) */ - l1sched_lchan_emit_data_cnf(lchan, br->fn); - } - return 0; } diff --git a/src/host/trxcon/src/sched_prim.c b/src/host/trxcon/src/sched_prim.c index 3211560..2597d84 100644 --- a/src/host/trxcon/src/sched_prim.c +++ b/src/host/trxcon/src/sched_prim.c @@ -249,17 +249,6 @@ } /** - * Drops the current primitive of specified logical channel - * - * @param lchan a logical channel to drop prim from - */ -void l1sched_lchan_prim_drop(struct l1sched_lchan_state *lchan) -{ - msgb_free(lchan->prim); - lchan->prim = NULL; -} - -/** * Allocate a DATA.req with dummy LAPDm func=UI frame for the given logical channel. * To be used when no suitable DATA.req is present in the Tx queue. * @@ -343,15 +332,12 @@ return l1sched_prim_to_user(lchan->ts->sched, msg); } -int l1sched_lchan_emit_data_cnf(struct l1sched_lchan_state *lchan, uint32_t fn) +int l1sched_lchan_emit_data_cnf(struct l1sched_lchan_state *lchan, + struct msgb *msg, uint32_t fn) { struct l1sched_prim *prim; - struct msgb *msg; - /* take ownership of the prim */ - if ((msg = lchan->prim) == NULL) - return -ENODEV; - lchan->prim = NULL; + OSMO_ASSERT(msg != NULL); /* convert from DATA.req to DATA.cnf */ prim = l1sched_prim_from_msgb(msg); diff --git a/src/host/trxcon/src/sched_trx.c b/src/host/trxcon/src/sched_trx.c index cac3a21..d07a579 100644 --- a/src/host/trxcon/src/sched_trx.c +++ b/src/host/trxcon/src/sched_trx.c @@ -524,9 +524,6 @@ lchan->rx_bursts = NULL; lchan->tx_bursts = NULL; - /* Forget the current prim */ - l1sched_lchan_prim_drop(lchan); - /* Flush the queue of pending Tx prims */ while ((msg = msgb_dequeue(&lchan->tx_prims)) != NULL) msgb_free(msg); -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33749 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ie09a24cd950a93edd871a9fbc5b47ec96c24cceb Gerrit-Change-Number: 33749 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: osmith &lt;osmith(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: merged