Change in osmo-bts[master]: osmo-bts-trx: prioritize FACCH in s/tx_tch_common()/tch_dl_dequeue()/s

fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/27804 ) Change subject: osmo-bts-trx: prioritize FACCH in s/tx_tch_common()/tch_dl_dequeue()/s ...................................................................... osmo-bts-trx: prioritize FACCH in s/tx_tch_common()/tch_dl_dequeue()/s Unlike SACCH, FACCH has no dedicated slots on the multiframe layout. It's multiplexed together with TCH (speech or data) frames basically by replacing (stealing) them. This is common for both TCH/F and TCH/H, with the only difference that FACCH/H steals two TCH frames (not just one) due to a longer interleaving period. Let's implement the multiplexing in the common function, which is used to dequeue to be transmitted frames - this slightly reduces code duplication. Use a new name, so that it's clear what it does. Change-Id: I9822b1a17185d5487f0f6d3ed0203e806c053d7d Related: SYS#5919, OS#4823 --- M src/osmo-bts-trx/sched_lchan_tchf.c M src/osmo-bts-trx/sched_lchan_tchh.c 2 files changed, 41 insertions(+), 41 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, approved osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/osmo-bts-trx/sched_lchan_tchf.c b/src/osmo-bts-trx/sched_lchan_tchf.c index fb19dd2..8fee874 100644 --- a/src/osmo-bts-trx/sched_lchan_tchf.c +++ b/src/osmo-bts-trx/sched_lchan_tchf.c @@ -322,8 +322,7 @@ /* common section for generation of TCH bursts (TCH/H and TCH/F). * FIXME: this function is over-complicated, refactor / get rid of it. */ -void tx_tch_common(struct l1sched_ts *l1ts, struct trx_dl_burst_req *br, - struct msgb **_msg_tch, struct msgb **_msg_facch) +struct msgb *tch_dl_dequeue(struct l1sched_ts *l1ts, struct trx_dl_burst_req *br) { struct msgb *msg1, *msg2, *msg_tch = NULL, *msg_facch = NULL; struct l1sched_chan_state *chan_state = &l1ts->chan_state[br->chan]; @@ -365,7 +364,8 @@ LOGL1SB(DL1P, LOGL_ERROR, l1ts, br, "Failed to encode AMR_BAD frame (rc=%d), " "not sending BFI\n", len); - return; + len = 0; + break; } memset(tch_data + 2, 0, len - 2); break; @@ -420,8 +420,19 @@ msg_facch = NULL; } + /* prioritize FACCH over speech */ + if (msg_facch) { + /* Unlike SACCH, FACCH has no dedicated slots on the multiframe layout. + * It's multiplexed together with TCH (speech or data) frames basically + * by replacing (stealing) them. This is common for both TCH/F and + * TCH/H, with the only difference that FACCH/H steals two TCH frames + * (not just one) due to a longer interleaving period. */ + msgb_free(msg_tch); + return msg_facch; + } + /* check validity of message, get AMR ft and cmr */ - if (!msg_facch && msg_tch) { + if (msg_tch) { int len; uint8_t cmr_codec; int cmr, ft, i; @@ -501,22 +512,19 @@ /* free message */ msgb_free(msg_tch); msg_tch = NULL; - goto send_frame; } } -send_frame: - *_msg_tch = msg_tch; - *_msg_facch = msg_facch; + return msg_tch; } /* obtain a to-be-transmitted TCH/F (Full Traffic Channel) burst */ int tx_tchf_fn(struct l1sched_ts *l1ts, struct trx_dl_burst_req *br) { - struct msgb *msg_tch = NULL, *msg_facch = NULL; struct l1sched_chan_state *chan_state = &l1ts->chan_state[br->chan]; uint8_t tch_mode = chan_state->tch_mode; ubit_t *burst, **bursts_p = &chan_state->dl_bursts; + struct msgb *msg; /* send burst, if we already got a frame */ if (br->bid > 0) { @@ -525,8 +533,6 @@ goto send_burst; } - tx_tch_common(l1ts, br, &msg_tch, &msg_facch); - /* BURST BYPASS */ /* allocate burst memory, if not already, @@ -540,8 +546,11 @@ memset(*bursts_p + 464, 0, 464); } + /* dequeue a message to be transmitted */ + msg = tch_dl_dequeue(l1ts, br); + /* no message at all, send a dummy L2 frame on FACCH */ - if (!msg_tch && !msg_facch) { + if (msg == NULL) { static const uint8_t dummy[GSM_MACBLOCK_LEN] = { 0x03, 0x03, 0x01, /* TODO: use randomized padding */ 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, @@ -553,28 +562,24 @@ goto send_burst; } - /* encode bursts (prioritize FACCH) */ - if (msg_facch) { - gsm0503_tch_fr_encode(*bursts_p, msg_facch->l2h, msgb_l2len(msg_facch), - 1); + /* populate the buffer with bursts */ + if (msgb_l2len(msg) == GSM_MACBLOCK_LEN) { + gsm0503_tch_fr_encode(*bursts_p, msg->l2h, msgb_l2len(msg), 1); chan_state->dl_facch_bursts = 8; } else if (tch_mode == GSM48_CMODE_SPEECH_AMR) /* the first FN 4,13,21 defines that CMI is included in frame, * the first FN 0,8,17 defines that CMR is included in frame. */ - gsm0503_tch_afs_encode(*bursts_p, msg_tch->l2h + 2, - msgb_l2len(msg_tch) - 2, !dl_amr_fn_is_cmi(br->fn), + gsm0503_tch_afs_encode(*bursts_p, msg->l2h + 2, + msgb_l2len(msg) - 2, !dl_amr_fn_is_cmi(br->fn), chan_state->codec, chan_state->codecs, chan_state->dl_ft, chan_state->dl_cmr); else - gsm0503_tch_fr_encode(*bursts_p, msg_tch->l2h, msgb_l2len(msg_tch), 1); + gsm0503_tch_fr_encode(*bursts_p, msg->l2h, msgb_l2len(msg), 1); /* free message */ - if (msg_tch) - msgb_free(msg_tch); - if (msg_facch) - msgb_free(msg_facch); + msgb_free(msg); send_burst: /* compose burst */ diff --git a/src/osmo-bts-trx/sched_lchan_tchh.c b/src/osmo-bts-trx/sched_lchan_tchh.c index 2281fb7..dcfbe3e 100644 --- a/src/osmo-bts-trx/sched_lchan_tchh.c +++ b/src/osmo-bts-trx/sched_lchan_tchh.c @@ -353,17 +353,15 @@ /* common section for generation of TCH bursts (TCH/H and TCH/F). * FIXME: this function is over-complicated, refactor / get rid of it. */ -extern void tx_tch_common(struct l1sched_ts *l1ts, - const struct trx_dl_burst_req *br, - struct msgb **_msg_tch, struct msgb **_msg_facch); +extern struct msgb *tch_dl_dequeue(struct l1sched_ts *l1ts, const struct trx_dl_burst_req *br); /* obtain a to-be-transmitted TCH/H (Half Traffic Channel) burst */ int tx_tchh_fn(struct l1sched_ts *l1ts, struct trx_dl_burst_req *br) { - struct msgb *msg_tch = NULL, *msg_facch = NULL; struct l1sched_chan_state *chan_state = &l1ts->chan_state[br->chan]; uint8_t tch_mode = chan_state->tch_mode; ubit_t *burst, **bursts_p = &chan_state->dl_bursts; + struct msgb *msg; /* send burst, if we already got a frame */ if (br->bid > 0) { @@ -372,9 +370,6 @@ goto send_burst; } - /* get TCH and/or FACCH */ - tx_tch_common(l1ts, br, &msg_tch, &msg_facch); - /* BURST BYPASS */ /* allocate burst memory, if not already, @@ -393,8 +388,11 @@ } } + /* dequeue a message to be transmitted */ + msg = tch_dl_dequeue(l1ts, br); + /* no message at all, send a dummy L2 frame on FACCH */ - if (!msg_tch && !msg_facch && !chan_state->dl_ongoing_facch) { + if (msg == NULL && !chan_state->dl_ongoing_facch) { static const uint8_t dummy[GSM_MACBLOCK_LEN] = { 0x03, 0x03, 0x01, /* TODO: use randomized padding */ 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, @@ -406,9 +404,9 @@ goto send_burst; } - /* encode bursts (prioritize FACCH) */ - if (msg_facch) { - gsm0503_tch_hr_encode(*bursts_p, msg_facch->l2h, msgb_l2len(msg_facch)); + /* populate the buffer with bursts */ + if (msgb_l2len(msg) == GSM_MACBLOCK_LEN) { + gsm0503_tch_hr_encode(*bursts_p, msg->l2h, msgb_l2len(msg)); chan_state->dl_ongoing_facch = 1; /* first of two TCH frames */ chan_state->dl_facch_bursts = 6; } else if (chan_state->dl_ongoing_facch) /* second of two TCH frames */ @@ -417,19 +415,16 @@ /* the first FN 4,13,21 or 5,14,22 defines that CMI is included * in frame, the first FN 0,8,17 or 1,9,18 defines that CMR is * included in frame. */ - gsm0503_tch_ahs_encode(*bursts_p, msg_tch->l2h + 2, - msgb_l2len(msg_tch) - 2, !dl_amr_fn_is_cmi(br->fn), + gsm0503_tch_ahs_encode(*bursts_p, msg->l2h + 2, + msgb_l2len(msg) - 2, !dl_amr_fn_is_cmi(br->fn), chan_state->codec, chan_state->codecs, chan_state->dl_ft, chan_state->dl_cmr); else - gsm0503_tch_hr_encode(*bursts_p, msg_tch->l2h, msgb_l2len(msg_tch)); + gsm0503_tch_hr_encode(*bursts_p, msg->l2h, msgb_l2len(msg)); /* free message */ - if (msg_tch) - msgb_free(msg_tch); - if (msg_facch) - msgb_free(msg_facch); + msgb_free(msg); send_burst: /* compose burst */ -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/27804 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I9822b1a17185d5487f0f6d3ed0203e806c053d7d Gerrit-Change-Number: 27804 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(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-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged