laforge submitted this change.

View Change


Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved
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(-)

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 change 33156. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I4538a8fe6b29f8d6eca33ad27d4a9852e3a3e86c
Gerrit-Change-Number: 33156
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge@osmocom.org>
Gerrit-Reviewer: osmith <osmith@sysmocom.de>
Gerrit-Reviewer: pespin <pespin@sysmocom.de>
Gerrit-MessageType: merged