[M] Change in osmo-bts[master]: osmo-bts-trx: fix recent regression in Tx lchan handlers

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33156 ) Change subject: osmo-bts-trx: fix recent regression in Tx lchan handlers ...................................................................... osmo-bts-trx: fix recent regression in Tx lchan handlers In my recent patch a0770250, among with the new burst buffer allocation/release strategy, I introduced a regression: /* send burst, if we already got a frame */ - if (br->bid > 0) { - if (!*bursts_p) - return -ENODEV; + if (br->bid > 0) goto send_burst; - } We used to allocate the burst buffers in Rx/Tx lchan handlers, and release them in case of an error, e.g. when no block is available for transmission. In the case of Tx burst buffers, the state of Tx burst buffer was additionally used to check if we have a valid Tx block for transmission, as can be seen in the code snippet above. As a side effect of my patch, osmo-bts-trx now keeps transmitting 3 out of 4 bursts (br->bid > 0) of the last valid block, until the next valid Tx block is available for transmission. This problem was not affecting the CS domain, where it's expected to have a more or less constant pressure of Tx blocks. However it did show up in the PS domain, where in the absence of active TBFs the PCU may omit DL blocks. Add a new field 'dl_mask' to struct l1sched_chan_state, similar to the existing 'ul_mask', and use it to reconstruct the removed logic. Change-Id: I4538a8fe6b29f8d6eca33ad27d4a9852e3a3e86c Fixes: a0770250 "osmo-bts-trx: alloc/free burst buffers in trx_sched_set_lchan()" --- M include/osmo-bts/scheduler.h M src/osmo-bts-trx/sched_lchan_pdtch.c M src/osmo-bts-trx/sched_lchan_tchf.c M src/osmo-bts-trx/sched_lchan_tchh.c M src/osmo-bts-trx/sched_lchan_xcch.c 5 files changed, 81 insertions(+), 7 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved diff --git a/include/osmo-bts/scheduler.h b/include/osmo-bts/scheduler.h index 01e8e2b..4d76b9b 100644 --- a/include/osmo-bts/scheduler.h +++ b/include/osmo-bts/scheduler.h @@ -90,6 +90,7 @@ bool active; /* Channel is active */ ubit_t *dl_bursts; /* burst buffer for TX */ enum trx_mod_type dl_mod_type; /* Downlink modulation type */ + uint8_t dl_mask; /* mask of transmitted bursts */ sbit_t *ul_bursts; /* burst buffer for RX */ sbit_t *ul_bursts_prev;/* previous burst buffer for RX (repeated SACCH) */ uint32_t ul_first_fn; /* fn of first burst */ diff --git a/src/osmo-bts-trx/sched_lchan_pdtch.c b/src/osmo-bts-trx/sched_lchan_pdtch.c index 1701aaf..3c993f0 100644 --- a/src/osmo-bts-trx/sched_lchan_pdtch.c +++ b/src/osmo-bts-trx/sched_lchan_pdtch.c @@ -144,13 +144,20 @@ int tx_pdtch_fn(struct l1sched_ts *l1ts, struct trx_dl_burst_req *br) { struct msgb *msg = NULL; /* make GCC happy */ - ubit_t *burst, *bursts_p = l1ts->chan_state[br->chan].dl_bursts; - enum trx_mod_type *mod = &l1ts->chan_state[br->chan].dl_mod_type; + struct l1sched_chan_state *chan_state = &l1ts->chan_state[br->chan]; + ubit_t *burst, *bursts_p = chan_state->dl_bursts; + enum trx_mod_type *mod = &chan_state->dl_mod_type; + uint8_t *mask = &chan_state->dl_mask; int rc = 0; /* send burst, if we already got a frame */ - if (br->bid > 0) + if (br->bid > 0) { + if ((*mask & 0x01) != 0x01) + return -ENOMSG; goto send_burst; + } + + *mask = *mask << 4; /* get mac block from queue */ msg = _sched_dequeue_prim(l1ts, br); @@ -206,6 +213,8 @@ br->burst_len = GSM_BURST_LEN; } + *mask |= (1 << br->bid); + LOGL1SB(DL1P, LOGL_DEBUG, l1ts, br, "Transmitting burst=%u.\n", br->bid); return 0; diff --git a/src/osmo-bts-trx/sched_lchan_tchf.c b/src/osmo-bts-trx/sched_lchan_tchf.c index 0188d85..cadb771 100644 --- a/src/osmo-bts-trx/sched_lchan_tchf.c +++ b/src/osmo-bts-trx/sched_lchan_tchf.c @@ -450,11 +450,17 @@ 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; + uint8_t *mask = &chan_state->dl_mask; struct msgb *msg; /* send burst, if we already got a frame */ - if (br->bid > 0) + if (br->bid > 0) { + if ((*mask & 0x01) != 0x01) + return -ENOMSG; goto send_burst; + } + + *mask = *mask << 4; /* BURST BYPASS */ @@ -533,6 +539,8 @@ br->flags |= TRX_BR_F_FACCH; } + *mask |= (1 << br->bid); + LOGL1SB(DL1P, LOGL_DEBUG, l1ts, br, "Transmitting burst=%u.\n", br->bid); return 0; diff --git a/src/osmo-bts-trx/sched_lchan_tchh.c b/src/osmo-bts-trx/sched_lchan_tchh.c index 2f1194a..7714296 100644 --- a/src/osmo-bts-trx/sched_lchan_tchh.c +++ b/src/osmo-bts-trx/sched_lchan_tchh.c @@ -351,11 +351,17 @@ 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; + uint8_t *mask = &chan_state->dl_mask; struct msgb *msg; /* send burst, if we already got a frame */ - if (br->bid > 0) + if (br->bid > 0) { + if ((*mask & 0x01) != 0x01) + return -ENOMSG; goto send_burst; + } + + *mask = *mask << 2; /* BURST BYPASS */ @@ -453,6 +459,8 @@ br->flags |= TRX_BR_F_FACCH; } + *mask |= (1 << br->bid); + LOGL1SB(DL1P, LOGL_DEBUG, l1ts, br, "Transmitting burst=%u.\n", br->bid); return 0; diff --git a/src/osmo-bts-trx/sched_lchan_xcch.c b/src/osmo-bts-trx/sched_lchan_xcch.c index 1945d32..52d80d0 100644 --- a/src/osmo-bts-trx/sched_lchan_xcch.c +++ b/src/osmo-bts-trx/sched_lchan_xcch.c @@ -156,11 +156,18 @@ int tx_data_fn(struct l1sched_ts *l1ts, struct trx_dl_burst_req *br) { struct msgb *msg = NULL; /* make GCC happy */ - ubit_t *burst, *bursts_p = l1ts->chan_state[br->chan].dl_bursts; + struct l1sched_chan_state *chan_state = &l1ts->chan_state[br->chan]; + ubit_t *burst, *bursts_p = chan_state->dl_bursts; + uint8_t *mask = &chan_state->dl_mask; /* send burst, if we already got a frame */ - if (br->bid > 0) + if (br->bid > 0) { + if ((*mask & 0x01) != 0x01) + return -ENOMSG; goto send_burst; + } + + *mask = *mask << 4; /* get mac block from queue */ msg = _sched_dequeue_prim(l1ts, br); @@ -195,6 +202,8 @@ br->burst_len = GSM_BURST_LEN; + *mask |= (1 << br->bid); + LOGL1SB(DL1P, LOGL_DEBUG, l1ts, br, "Transmitting burst=%u.\n", br->bid); return 0; -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33156 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I4538a8fe6b29f8d6eca33ad27d4a9852e3a3e86c Gerrit-Change-Number: 33156 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder 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