16 Jul
2014
16 Jul
'14
7:04 p.m.
In the lookup functions make sure that we are actually returning tbfs
with the expected direction.
Ticket: SYS#389
Sponsored-by: On-Waves ehf
---
src/bts.cpp | 22 ++++++++++++++++------
src/bts.h | 3 ++-
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/src/bts.cpp b/src/bts.cpp
index 08baee0..f614dab 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -234,12 +234,14 @@ gprs_rlcmac_tbf *BTS::tbf_by_tlli(uint32_t tlli, enum
gprs_rlcmac_tbf_direction
struct gprs_rlcmac_tbf *tbf;
if (dir == GPRS_RLCMAC_UL_TBF) {
llist_for_each_entry(tbf, &m_bts.ul_tbfs, list) {
+ OSMO_ASSERT(tbf->direction == dir);
if (tbf->state_is_not(GPRS_RLCMAC_RELEASING)
&& tbf->tlli() == tlli && tbf->is_tlli_valid())
return tbf;
}
} else {
llist_for_each_entry(tbf, &m_bts.dl_tbfs, list) {
+ OSMO_ASSERT(tbf->direction == dir);
if (tbf->state_is_not(GPRS_RLCMAC_RELEASING)
&& tbf->tlli() == tlli)
return tbf;
@@ -258,8 +260,10 @@ gprs_rlcmac_tbf *BTS::dl_tbf_by_poll_fn(uint32_t fn, uint8_t trx,
uint8_t ts)
if (tbf->state_is_not(GPRS_RLCMAC_RELEASING)
&& tbf->poll_state == GPRS_RLCMAC_POLL_SCHED
&& tbf->poll_fn == fn && tbf->trx->trx_no == trx
- && tbf->control_ts == ts)
+ && tbf->control_ts == ts) {
+ OSMO_ASSERT(tbf->direction == GPRS_RLCMAC_DL_TBF);
return tbf;
+ }
}
return NULL;
}
@@ -273,8 +277,10 @@ gprs_rlcmac_tbf *BTS::ul_tbf_by_poll_fn(uint32_t fn, uint8_t trx,
uint8_t ts)
if (tbf->state_is_not(GPRS_RLCMAC_RELEASING)
&& tbf->poll_state == GPRS_RLCMAC_POLL_SCHED
&& tbf->poll_fn == fn && tbf->trx->trx_no == trx
- && tbf->control_ts == ts)
+ && tbf->control_ts == ts) {
+ OSMO_ASSERT(tbf->direction == GPRS_RLCMAC_UL_TBF);
return tbf;
+ }
}
return NULL;
}
@@ -307,8 +313,10 @@ gprs_rlcmac_tbf *BTS::tbf_by_tfi(uint8_t tfi, uint8_t trx,
if (!tbf)
return NULL;
- if (tbf->state_is_not(GPRS_RLCMAC_RELEASING))
+ if (tbf->state_is_not(GPRS_RLCMAC_RELEASING)) {
+ OSMO_ASSERT(tbf->direction == dir);
return tbf;
+ }
return NULL;
}
@@ -1023,11 +1031,13 @@ int gprs_rlcmac_pdch::rcv_block(uint8_t *data, uint8_t len,
uint32_t fn, int8_t
return rc;
}
-gprs_rlcmac_tbf *gprs_rlcmac_pdch::tbf_from_list_by_tfi(struct llist_head *tbf_list,
uint8_t tfi)
+gprs_rlcmac_tbf *gprs_rlcmac_pdch::tbf_from_list_by_tfi(struct llist_head *tbf_list,
uint8_t tfi,
+ enum gprs_rlcmac_tbf_direction dir)
{
gprs_rlcmac_tbf *tbf;
llist_for_each_entry(tbf, tbf_list, list) {
+ OSMO_ASSERT(tbf->direction == dir);
if (tbf->tfi() != tfi)
continue;
if (!tbf->pdch[ts_no])
@@ -1039,10 +1049,10 @@ gprs_rlcmac_tbf *gprs_rlcmac_pdch::tbf_from_list_by_tfi(struct
llist_head *tbf_l
gprs_rlcmac_tbf *gprs_rlcmac_pdch::ul_tbf_by_tfi(uint8_t tfi)
{
- return tbf_from_list_by_tfi(&bts_data()->ul_tbfs, tfi);
+ return tbf_from_list_by_tfi(&bts_data()->ul_tbfs, tfi, GPRS_RLCMAC_UL_TBF);
}
gprs_rlcmac_tbf *gprs_rlcmac_pdch::dl_tbf_by_tfi(uint8_t tfi)
{
- return tbf_from_list_by_tfi(&bts_data()->dl_tbfs, tfi);
+ return tbf_from_list_by_tfi(&bts_data()->dl_tbfs, tfi, GPRS_RLCMAC_DL_TBF);
}
diff --git a/src/bts.h b/src/bts.h
index 621bcba..a0b808e 100644
--- a/src/bts.h
+++ b/src/bts.h
@@ -87,7 +87,8 @@ private:
void rcv_control_dl_ack_nack(Packet_Downlink_Ack_Nack_t *, uint32_t fn);
void rcv_resource_request(Packet_Resource_Request_t *t, uint32_t fn);
void rcv_measurement_report(Packet_Measurement_Report_t *t, uint32_t fn);
- gprs_rlcmac_tbf *tbf_from_list_by_tfi(struct llist_head *tbf_list, uint8_t tfi);
+ gprs_rlcmac_tbf *tbf_from_list_by_tfi(struct llist_head *tbf_list, uint8_t tfi,
+ enum gprs_rlcmac_tbf_direction dir);
#endif
};
--
1.8.4.2
16 Jul
16 Jul
7:04 p.m.
New subject: [osmo-pcu 2/5] tbf: Add ul and dl TBF types and allocate them in tbf_alloc()
Ticket: SYS#389
Sponsored-by: On-Waves ehf
---
src/tbf.cpp | 6 +++++-
src/tbf.h | 6 ++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/tbf.cpp b/src/tbf.cpp
index f913f97..39549f1 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -495,7 +495,11 @@ struct gprs_rlcmac_tbf *tbf_alloc(struct gprs_rlcmac_bts *bts,
if (trx >= 8 || tfi >= 32)
return NULL;
- tbf = talloc_zero(tall_pcu_ctx, struct gprs_rlcmac_tbf);
+ if (dir == GPRS_RLCMAC_UL_TBF)
+ tbf = talloc_zero(tall_pcu_ctx, struct gprs_rlcmac_ul_tbf);
+ else
+ tbf = talloc_zero(tall_pcu_ctx, struct gprs_rlcmac_dl_tbf);
+
if (!tbf)
return NULL;
diff --git a/src/tbf.h b/src/tbf.h
index 80e2068..8464e19 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -326,6 +326,12 @@ inline time_t gprs_rlcmac_tbf::created_ts() const
return m_created_ts;
}
+struct gprs_rlcmac_dl_tbf : public gprs_rlcmac_tbf {
+};
+
+struct gprs_rlcmac_ul_tbf : public gprs_rlcmac_tbf {
+};
+
#endif
#ifdef __cplusplus
--
1.8.4.2
7:04 p.m.
New subject: [osmo-pcu 3/5] bts: Return the special type for {ul, dl}_tbf_by_* functions
Start returning the special type instead of the base gprs_rlcmac_tbf
when retrieving a TBF.
Ticket: SYS#389
Sponsored-by: On-Waves ehf
---
src/bts.cpp | 36 ++++++++++++++++++------------------
src/bts.h | 16 ++++++++--------
2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/src/bts.cpp b/src/bts.cpp
index f614dab..68d72f2 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -217,15 +217,15 @@ int BTS::add_paging(uint8_t chan_needed, uint8_t *identity_lv)
}
/* search for active downlink tbf */
-gprs_rlcmac_tbf *BTS::dl_tbf_by_tlli(uint32_t tlli)
+gprs_rlcmac_dl_tbf *BTS::dl_tbf_by_tlli(uint32_t tlli)
{
- return tbf_by_tlli(tlli, GPRS_RLCMAC_DL_TBF);
+ return (gprs_rlcmac_dl_tbf *)tbf_by_tlli(tlli, GPRS_RLCMAC_DL_TBF);
}
/* search for active uplink tbf */
-gprs_rlcmac_tbf *BTS::ul_tbf_by_tlli(uint32_t tlli)
+gprs_rlcmac_ul_tbf *BTS::ul_tbf_by_tlli(uint32_t tlli)
{
- return tbf_by_tlli(tlli, GPRS_RLCMAC_UL_TBF);
+ return (gprs_rlcmac_ul_tbf *)tbf_by_tlli(tlli, GPRS_RLCMAC_UL_TBF);
}
/* search for active downlink or uplink tbf */
@@ -250,9 +250,9 @@ gprs_rlcmac_tbf *BTS::tbf_by_tlli(uint32_t tlli, enum
gprs_rlcmac_tbf_direction
return NULL;
}
-gprs_rlcmac_tbf *BTS::dl_tbf_by_poll_fn(uint32_t fn, uint8_t trx, uint8_t ts)
+gprs_rlcmac_dl_tbf *BTS::dl_tbf_by_poll_fn(uint32_t fn, uint8_t trx, uint8_t ts)
{
- struct gprs_rlcmac_tbf *tbf;
+ struct gprs_rlcmac_dl_tbf *tbf;
/* only one TBF can poll on specific TS/FN, because scheduler can only
* schedule one downlink control block (with polling) at a FN per TS */
@@ -262,14 +262,14 @@ gprs_rlcmac_tbf *BTS::dl_tbf_by_poll_fn(uint32_t fn, uint8_t trx,
uint8_t ts)
&& tbf->poll_fn == fn && tbf->trx->trx_no == trx
&& tbf->control_ts == ts) {
OSMO_ASSERT(tbf->direction == GPRS_RLCMAC_DL_TBF);
- return tbf;
+ return (gprs_rlcmac_dl_tbf *)tbf;
}
}
return NULL;
}
-gprs_rlcmac_tbf *BTS::ul_tbf_by_poll_fn(uint32_t fn, uint8_t trx, uint8_t ts)
+gprs_rlcmac_ul_tbf *BTS::ul_tbf_by_poll_fn(uint32_t fn, uint8_t trx, uint8_t ts)
{
- struct gprs_rlcmac_tbf *tbf;
+ struct gprs_rlcmac_ul_tbf *tbf;
/* only one TBF can poll on specific TS/FN, because scheduler can only
* schedule one downlink control block (with polling) at a FN per TS */
@@ -279,22 +279,22 @@ gprs_rlcmac_tbf *BTS::ul_tbf_by_poll_fn(uint32_t fn, uint8_t trx,
uint8_t ts)
&& tbf->poll_fn == fn && tbf->trx->trx_no == trx
&& tbf->control_ts == ts) {
OSMO_ASSERT(tbf->direction == GPRS_RLCMAC_UL_TBF);
- return tbf;
+ return (gprs_rlcmac_ul_tbf *)tbf;
}
}
return NULL;
}
/* lookup downlink TBF Entity (by TFI) */
-gprs_rlcmac_tbf *BTS::dl_tbf_by_tfi(uint8_t tfi, uint8_t trx)
+gprs_rlcmac_dl_tbf *BTS::dl_tbf_by_tfi(uint8_t tfi, uint8_t trx)
{
- return tbf_by_tfi(tfi, trx, GPRS_RLCMAC_DL_TBF);
+ return (gprs_rlcmac_dl_tbf *)tbf_by_tfi(tfi, trx, GPRS_RLCMAC_DL_TBF);
}
/* lookup uplink TBF Entity (by TFI) */
-gprs_rlcmac_tbf *BTS::ul_tbf_by_tfi(uint8_t tfi, uint8_t trx)
+gprs_rlcmac_ul_tbf *BTS::ul_tbf_by_tfi(uint8_t tfi, uint8_t trx)
{
- return tbf_by_tfi(tfi, trx, GPRS_RLCMAC_UL_TBF);
+ return (gprs_rlcmac_ul_tbf *)tbf_by_tfi(tfi, trx, GPRS_RLCMAC_UL_TBF);
}
/* lookup TBF Entity (by TFI) */
@@ -1047,12 +1047,12 @@ gprs_rlcmac_tbf *gprs_rlcmac_pdch::tbf_from_list_by_tfi(struct
llist_head *tbf_l
return NULL;
}
-gprs_rlcmac_tbf *gprs_rlcmac_pdch::ul_tbf_by_tfi(uint8_t tfi)
+gprs_rlcmac_ul_tbf *gprs_rlcmac_pdch::ul_tbf_by_tfi(uint8_t tfi)
{
- return tbf_from_list_by_tfi(&bts_data()->ul_tbfs, tfi, GPRS_RLCMAC_UL_TBF);
+ return (gprs_rlcmac_ul_tbf *)tbf_from_list_by_tfi(&bts_data()->ul_tbfs, tfi,
GPRS_RLCMAC_UL_TBF);
}
-gprs_rlcmac_tbf *gprs_rlcmac_pdch::dl_tbf_by_tfi(uint8_t tfi)
+gprs_rlcmac_dl_tbf *gprs_rlcmac_pdch::dl_tbf_by_tfi(uint8_t tfi)
{
- return tbf_from_list_by_tfi(&bts_data()->dl_tbfs, tfi, GPRS_RLCMAC_DL_TBF);
+ return (gprs_rlcmac_dl_tbf *)tbf_from_list_by_tfi(&bts_data()->dl_tbfs, tfi,
GPRS_RLCMAC_DL_TBF);
}
diff --git a/src/bts.h b/src/bts.h
index a0b808e..8f99942 100644
--- a/src/bts.h
+++ b/src/bts.h
@@ -62,8 +62,8 @@ struct gprs_rlcmac_pdch {
BTS *bts() const;
uint8_t trx_no() const;
- struct gprs_rlcmac_tbf *ul_tbf_by_tfi(uint8_t tfi);
- struct gprs_rlcmac_tbf *dl_tbf_by_tfi(uint8_t tfi);
+ struct gprs_rlcmac_ul_tbf *ul_tbf_by_tfi(uint8_t tfi);
+ struct gprs_rlcmac_dl_tbf *dl_tbf_by_tfi(uint8_t tfi);
#endif
uint8_t m_is_enabled; /* TS is enabled */
@@ -193,12 +193,12 @@ public:
/** add paging to paging queue(s) */
int add_paging(uint8_t chan_needed, uint8_t *identity_lv);
- gprs_rlcmac_tbf *dl_tbf_by_tlli(uint32_t tlli);
- gprs_rlcmac_tbf *ul_tbf_by_tlli(uint32_t tlli);
- gprs_rlcmac_tbf *dl_tbf_by_poll_fn(uint32_t fn, uint8_t trx, uint8_t ts);
- gprs_rlcmac_tbf *ul_tbf_by_poll_fn(uint32_t fn, uint8_t trx, uint8_t ts);
- gprs_rlcmac_tbf *dl_tbf_by_tfi(uint8_t tfi, uint8_t trx);
- gprs_rlcmac_tbf *ul_tbf_by_tfi(uint8_t tfi, uint8_t trx);
+ gprs_rlcmac_dl_tbf *dl_tbf_by_tlli(uint32_t tlli);
+ gprs_rlcmac_ul_tbf *ul_tbf_by_tlli(uint32_t tlli);
+ gprs_rlcmac_dl_tbf *dl_tbf_by_poll_fn(uint32_t fn, uint8_t trx, uint8_t ts);
+ gprs_rlcmac_ul_tbf *ul_tbf_by_poll_fn(uint32_t fn, uint8_t trx, uint8_t ts);
+ gprs_rlcmac_dl_tbf *dl_tbf_by_tfi(uint8_t tfi, uint8_t trx);
+ gprs_rlcmac_ul_tbf *ul_tbf_by_tfi(uint8_t tfi, uint8_t trx);
int tfi_find_free(enum gprs_rlcmac_tbf_direction dir, uint8_t *_trx, int8_t use_trx);
--
1.8.4.2
7:04 p.m.
New subject: [osmo-pcu 4/5] bts, tbf: Change the TBF return type of functions to the ul/dl version
Many functions only ever deal with or return a UL or a DL TBF.
Explicitly change the type to reflect which TBF is used where.
Ticket: SYS#389
Sponsored-by: On-Waves ehf
---
src/bts.cpp | 38 +++++++++++++++++++-------------------
src/tbf.cpp | 6 +++---
src/tbf.h | 2 +-
3 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/src/bts.cpp b/src/bts.cpp
index 68d72f2..8489431 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -417,7 +417,7 @@ int BTS::rcv_imm_ass_cnf(const uint8_t *data, uint32_t fn)
int BTS::rcv_rach(uint8_t ra, uint32_t Fn, int16_t qta)
{
- struct gprs_rlcmac_tbf *tbf;
+ struct gprs_rlcmac_ul_tbf *tbf;
uint8_t trx_no, ts_no = 0;
int8_t tfi; /* must be signed */
uint8_t sb = 0;
@@ -461,7 +461,7 @@ int BTS::rcv_rach(uint8_t ra, uint32_t Fn, int16_t qta)
return -EBUSY;
}
/* set class to 0, since we don't know the multislot class yet */
- tbf = tbf_alloc(&m_bts, NULL, GPRS_RLCMAC_UL_TBF, tfi, trx_no, 0, 1);
+ tbf = (gprs_rlcmac_ul_tbf *)tbf_alloc(&m_bts, NULL, GPRS_RLCMAC_UL_TBF, tfi,
trx_no, 0, 1);
if (!tbf) {
LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH resource\n");
/* FIXME: send reject */
@@ -692,7 +692,7 @@ void gprs_rlcmac_pdch::add_paging(struct gprs_rlcmac_paging *pag)
*/
int gprs_rlcmac_pdch::rcv_data_block_acknowledged(uint8_t *data, uint8_t len, int8_t
rssi)
{
- struct gprs_rlcmac_tbf *tbf;
+ struct gprs_rlcmac_ul_tbf *tbf;
struct rlc_ul_header *rh = (struct rlc_ul_header *)data;
switch (len) {
@@ -867,24 +867,24 @@ void
gprs_rlcmac_pdch::rcv_control_dl_ack_nack(Packet_Downlink_Ack_Nack_t *ack_n
void gprs_rlcmac_pdch::rcv_resource_request(Packet_Resource_Request_t *request, uint32_t
fn)
{
- struct gprs_rlcmac_tbf *tbf;
+ struct gprs_rlcmac_ul_tbf *ul_tbf;
+ struct gprs_rlcmac_dl_tbf *dl_tbf;
struct gprs_rlcmac_sba *sba;
int rc;
if (request->ID.UnionType) {
uint32_t tlli = request->ID.u.TLLI;
uint8_t ms_class = 0;
- struct gprs_rlcmac_tbf *dl_tbf;
uint8_t ta;
- tbf = bts()->ul_tbf_by_tlli(tlli);
- if (tbf) {
+ ul_tbf = bts()->ul_tbf_by_tlli(tlli);
+ if (ul_tbf) {
LOGP(DRLCMACUL, LOGL_NOTICE, "Got RACH from "
"TLLI=0x%08x while %s still "
"exists. Killing pending DL TBF\n",
- tlli, tbf_name(tbf));
- tbf_free(tbf);
- tbf = NULL;
+ tlli, tbf_name(ul_tbf));
+ tbf_free(ul_tbf);
+ ul_tbf = NULL;
}
if ((dl_tbf = bts()->dl_tbf_by_tlli(tlli))) {
@@ -918,34 +918,34 @@ void
gprs_rlcmac_pdch::rcv_resource_request(Packet_Resource_Request_t *request,
ms_class =
Decoding::get_ms_class_by_capability(&request->MS_Radio_Access_capability);
if (!ms_class)
LOGP(DRLCMAC, LOGL_NOTICE, "MS does not give us a class.\n");
- tbf = tbf_alloc_ul(bts_data(), trx_no(), ms_class, tlli, ta, NULL);
- if (!tbf)
+ ul_tbf = tbf_alloc_ul(bts_data(), trx_no(), ms_class, tlli, ta, NULL);
+ if (!ul_tbf)
return;
/* set control ts to current MS's TS, until assignment complete */
LOGP(DRLCMAC, LOGL_DEBUG, "Change control TS to %d until assinment is
complete.\n", ts_no);
- tbf->control_ts = ts_no;
+ ul_tbf->control_ts = ts_no;
/* schedule uplink assignment */
- tbf->ul_ass_state = GPRS_RLCMAC_UL_ASS_SEND_ASS;
+ ul_tbf->ul_ass_state = GPRS_RLCMAC_UL_ASS_SEND_ASS;
return;
}
if (request->ID.u.Global_TFI.UnionType) {
int8_t tfi = request->ID.u.Global_TFI.u.DOWNLINK_TFI;
- tbf = bts()->dl_tbf_by_tfi(tfi, trx_no());
- if (!tbf) {
+ dl_tbf = bts()->dl_tbf_by_tfi(tfi, trx_no());
+ if (!dl_tbf) {
LOGP(DRLCMAC, LOGL_NOTICE, "PACKET RESSOURCE REQ unknown downlink TFI=%d\n",
tfi);
return;
}
} else {
int8_t tfi = request->ID.u.Global_TFI.u.UPLINK_TFI;
- tbf = bts()->ul_tbf_by_tfi(tfi, trx_no());
- if (!tbf) {
+ ul_tbf = bts()->ul_tbf_by_tfi(tfi, trx_no());
+ if (!ul_tbf) {
LOGP(DRLCMAC, LOGL_NOTICE, "PACKET RESSOURCE REQ unknown uplink TFI=%d\n",
tfi);
return;
}
}
- LOGP(DRLCMAC, LOGL_ERROR, "RX: [PCU <- BTS] %s FIXME: Packet resource
request\n", tbf_name(tbf));
+ LOGP(DRLCMAC, LOGL_ERROR, "RX: [PCU <- BTS] %s FIXME: Packet resource
request\n", tbf_name(ul_tbf));
}
void gprs_rlcmac_pdch::rcv_measurement_report(Packet_Measurement_Report_t *report,
uint32_t fn)
diff --git a/src/tbf.cpp b/src/tbf.cpp
index 39549f1..15301a1 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -214,12 +214,12 @@ int gprs_rlcmac_tbf::handle(struct gprs_rlcmac_bts *bts,
return tbf_new_dl_assignment(bts, imsi, tlli, ms_class, data, len);
}
-struct gprs_rlcmac_tbf *tbf_alloc_ul(struct gprs_rlcmac_bts *bts,
+gprs_rlcmac_ul_tbf *tbf_alloc_ul(struct gprs_rlcmac_bts *bts,
int8_t use_trx, uint8_t ms_class,
uint32_t tlli, uint8_t ta, struct gprs_rlcmac_tbf *dl_tbf)
{
uint8_t trx;
- struct gprs_rlcmac_tbf *tbf;
+ struct gprs_rlcmac_ul_tbf *tbf;
int8_t tfi; /* must be signed */
#warning "Copy and paste with tbf_new_dl_assignment"
@@ -231,7 +231,7 @@ struct gprs_rlcmac_tbf *tbf_alloc_ul(struct gprs_rlcmac_bts *bts,
return NULL;
}
/* use multislot class of downlink TBF */
- tbf = tbf_alloc(bts, dl_tbf, GPRS_RLCMAC_UL_TBF, tfi, trx, ms_class, 0);
+ tbf = (gprs_rlcmac_ul_tbf *)tbf_alloc(bts, dl_tbf, GPRS_RLCMAC_UL_TBF, tfi, trx,
ms_class, 0);
if (!tbf) {
LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH resource\n");
/* FIXME: send reject */
diff --git a/src/tbf.h b/src/tbf.h
index 8464e19..d650021 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -259,7 +259,7 @@ protected:
};
-struct gprs_rlcmac_tbf *tbf_alloc_ul(struct gprs_rlcmac_bts *bts,
+struct gprs_rlcmac_ul_tbf *tbf_alloc_ul(struct gprs_rlcmac_bts *bts,
int8_t use_trx, uint8_t ms_class,
uint32_t tlli, uint8_t ta, struct gprs_rlcmac_tbf *dl_tbf);
--
1.8.4.2
17 Jul
17 Jul
8:52 a.m.
New subject: [osmo-pcu 4/5] bts, tbf: Change the TBF return type of functions to the ul/dl version
On Wed, Jul 16, 2014 at 07:04:31PM +0200, Daniel Willmann wrote:
void
gprs_rlcmac_pdch::rcv_resource_request(Packet_Resource_Request_t *request, uint32_t fn)
{
- struct gprs_rlcmac_tbf *tbf;
+ struct gprs_rlcmac_ul_tbf *ul_tbf;
+ struct gprs_rlcmac_dl_tbf *dl_tbf;
Limit the dl_tbf (and ul_tbf) in scope. The dl_tbf one part of the code
is dealing with is not the one the other is using. We should not accidently
leak a tbf into another scope just because this method is huge.
In general cppcheck's style warnings complain about this kind of thing
8:55 a.m.
New subject: [osmo-pcu 4/5] bts, tbf: Change the TBF return type of functions to the ul/dl version
On Wed, Jul 16, 2014 at 07:04:31PM +0200, Daniel Willmann wrote:
- LOGP(DRLCMAC, LOGL_ERROR, "RX: [PCU <- BTS]
%s FIXME: Packet resource request\n", tbf_name(tbf));
+ LOGP(DRLCMAC, LOGL_ERROR, "RX: [PCU <- BTS] %s FIXME: Packet resource
request\n", tbf_name(ul_tbf));
And the compiler just re-inforced me with a warning:
home/ich/install/openbsc/include/osmocom/core/logging.h: In member function 'void
gprs_rlcmac_pdch::rcv_resource_request(Packet_Resource_Request_t*, uint32_t)':
/home/ich/install/openbsc/include/osmocom/core/logging.h:42:53: warning: 'ul_tbf'
may be used uninitialized in this function [-Wmaybe-uninitialized]
logp2(ss, level, __FILE__, __LINE__, 0, fmt, ##args)
After the last if/else either dl_tbf or ul_tbf will be assigned. Maybe just
move the warning into the if/else and remove the declaration of the dl_tbf
ul_tbf.
16 Jul
16 Jul
7:04 p.m.
New subject: [osmo-pcu 5/5] bts, tbf: Split alloc_tbf function into separate UL and DL versions
UL and DL tbfs are used in very separate parts and are not the same
thing so split the alloc function and use the UL/DL version throughout
the code.
Ticket: SYS#389
Sponsored-by: On-Waves ehf
---
src/bts.cpp | 2 +-
src/tbf.cpp | 102 +++++++++++++++++++++++++++++++++-------------
src/tbf.h | 9 +++-
tests/alloc/AllocTest.cpp | 31 +++++++++-----
tests/tbf/TbfTest.cpp | 8 ++--
5 files changed, 106 insertions(+), 46 deletions(-)
diff --git a/src/bts.cpp b/src/bts.cpp
index 8489431..5bf139d 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -461,7 +461,7 @@ int BTS::rcv_rach(uint8_t ra, uint32_t Fn, int16_t qta)
return -EBUSY;
}
/* set class to 0, since we don't know the multislot class yet */
- tbf = (gprs_rlcmac_ul_tbf *)tbf_alloc(&m_bts, NULL, GPRS_RLCMAC_UL_TBF, tfi,
trx_no, 0, 1);
+ tbf = tbf_alloc_ul_tbf(&m_bts, NULL, tfi, trx_no, 0, 1);
if (!tbf) {
LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH resource\n");
/* FIXME: send reject */
diff --git a/src/tbf.cpp b/src/tbf.cpp
index 15301a1..1201642 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -165,7 +165,7 @@ static int tbf_new_dl_assignment(struct gprs_rlcmac_bts *bts,
return -EBUSY;
}
/* set number of downlink slots according to multislot class */
- tbf = tbf_alloc(bts, tbf, GPRS_RLCMAC_DL_TBF, tfi, trx, ms_class, ss);
+ tbf = tbf_alloc_dl_tbf(bts, tbf, tfi, trx, ms_class, ss);
if (!tbf) {
LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH resource\n");
/* FIXME: send reject */
@@ -231,7 +231,7 @@ gprs_rlcmac_ul_tbf *tbf_alloc_ul(struct gprs_rlcmac_bts *bts,
return NULL;
}
/* use multislot class of downlink TBF */
- tbf = (gprs_rlcmac_ul_tbf *)tbf_alloc(bts, dl_tbf, GPRS_RLCMAC_UL_TBF, tfi, trx,
ms_class, 0);
+ tbf = tbf_alloc_ul_tbf(bts, dl_tbf, tfi, trx, ms_class, 0);
if (!tbf) {
LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH resource\n");
/* FIXME: send reject */
@@ -479,33 +479,20 @@ void gprs_rlcmac_tbf::poll_timeout()
LOGP(DRLCMAC, LOGL_ERROR, "- Poll Timeout, but no event!\n");
}
-struct gprs_rlcmac_tbf *tbf_alloc(struct gprs_rlcmac_bts *bts,
- struct gprs_rlcmac_tbf *old_tbf, enum gprs_rlcmac_tbf_direction dir,
- uint8_t tfi, uint8_t trx,
+static int setup_tbf(struct gprs_rlcmac_tbf *tbf, struct gprs_rlcmac_bts *bts,
+ struct gprs_rlcmac_tbf *old_tbf, uint8_t tfi, uint8_t trx,
uint8_t ms_class, uint8_t single_slot)
{
- struct gprs_rlcmac_tbf *tbf;
int rc;
- LOGP(DRLCMAC, LOGL_DEBUG, "********** TBF starts here **********\n");
- LOGP(DRLCMAC, LOGL_INFO, "Allocating %s TBF: TFI=%d TRX=%d "
- "MS_CLASS=%d\n", (dir == GPRS_RLCMAC_UL_TBF) ? "UL" :
"DL",
- tfi, trx, ms_class);
-
if (trx >= 8 || tfi >= 32)
- return NULL;
-
- if (dir == GPRS_RLCMAC_UL_TBF)
- tbf = talloc_zero(tall_pcu_ctx, struct gprs_rlcmac_ul_tbf);
- else
- tbf = talloc_zero(tall_pcu_ctx, struct gprs_rlcmac_dl_tbf);
+ return -1;
if (!tbf)
- return NULL;
+ return -1;
tbf->m_created_ts = time(NULL);
tbf->bts = bts->bts;
- tbf->direction = dir;
tbf->m_tfi = tfi;
tbf->trx = &bts->trx[trx];
tbf->ms_class = ms_class;
@@ -514,16 +501,14 @@ struct gprs_rlcmac_tbf *tbf_alloc(struct gprs_rlcmac_bts *bts,
single_slot);
/* if no resource */
if (rc < 0) {
- talloc_free(tbf);
- return NULL;
+ return -1;
}
/* assign control ts */
tbf->control_ts = 0xff;
rc = tbf_assign_control_ts(tbf);
/* if no resource */
if (rc < 0) {
- talloc_free(tbf);
- return NULL;
+ return -1;
}
/* set timestamp */
@@ -532,14 +517,73 @@ struct gprs_rlcmac_tbf *tbf_alloc(struct gprs_rlcmac_bts *bts,
gettimeofday(&tbf->meas.dl_loss_tv, NULL);
tbf->m_llc.init();
- if (dir == GPRS_RLCMAC_UL_TBF) {
- llist_add(&tbf->list, &bts->ul_tbfs);
- tbf->bts->tbf_ul_created();
- } else {
- llist_add(&tbf->list, &bts->dl_tbfs);
- tbf->bts->tbf_dl_created();
+ return 0;
+}
+
+
+struct gprs_rlcmac_ul_tbf *tbf_alloc_ul_tbf(struct gprs_rlcmac_bts *bts,
+ struct gprs_rlcmac_tbf *old_tbf, uint8_t tfi, uint8_t trx,
+ uint8_t ms_class, uint8_t single_slot)
+{
+ struct gprs_rlcmac_ul_tbf *tbf;
+ int rc;
+
+ LOGP(DRLCMAC, LOGL_DEBUG, "********** TBF starts here **********\n");
+ LOGP(DRLCMAC, LOGL_INFO, "Allocating %s TBF: TFI=%d TRX=%d "
+ "MS_CLASS=%d\n", "UL", tfi, trx, ms_class);
+
+ if (trx >= 8 || tfi >= 32)
+ return NULL;
+
+ tbf = talloc_zero(tall_pcu_ctx, struct gprs_rlcmac_ul_tbf);
+
+ if (!tbf)
+ return NULL;
+
+ tbf->direction = GPRS_RLCMAC_UL_TBF;
+ rc = setup_tbf(tbf, bts, old_tbf, tfi, trx, ms_class, single_slot);
+ /* if no resource */
+ if (rc < 0) {
+ talloc_free(tbf);
+ return NULL;
+ }
+
+ llist_add(&tbf->list, &bts->ul_tbfs);
+ tbf->bts->tbf_ul_created();
+
+ return tbf;
+}
+
+struct gprs_rlcmac_dl_tbf *tbf_alloc_dl_tbf(struct gprs_rlcmac_bts *bts,
+ struct gprs_rlcmac_tbf *old_tbf, uint8_t tfi, uint8_t trx,
+ uint8_t ms_class, uint8_t single_slot)
+{
+ struct gprs_rlcmac_dl_tbf *tbf;
+ int rc;
+
+ LOGP(DRLCMAC, LOGL_DEBUG, "********** TBF starts here **********\n");
+ LOGP(DRLCMAC, LOGL_INFO, "Allocating %s TBF: TFI=%d TRX=%d "
+ "MS_CLASS=%d\n", "DL", tfi, trx, ms_class);
+
+ if (trx >= 8 || tfi >= 32)
+ return NULL;
+
+ tbf = talloc_zero(tall_pcu_ctx, struct gprs_rlcmac_dl_tbf);
+
+ if (!tbf)
+ return NULL;
+
+ tbf->direction = GPRS_RLCMAC_DL_TBF;
+ rc = setup_tbf(tbf, bts, old_tbf, tfi, trx, ms_class, single_slot);
+ /* if no resource */
+ if (rc < 0) {
+ talloc_free(tbf);
+ return NULL;
}
+ llist_add(&tbf->list, &bts->dl_tbfs);
+ tbf->bts->tbf_dl_created();
+
return tbf;
}
diff --git a/src/tbf.h b/src/tbf.h
index d650021..4d20987 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -263,9 +263,14 @@ struct gprs_rlcmac_ul_tbf *tbf_alloc_ul(struct gprs_rlcmac_bts *bts,
int8_t use_trx, uint8_t ms_class,
uint32_t tlli, uint8_t ta, struct gprs_rlcmac_tbf *dl_tbf);
-struct gprs_rlcmac_tbf *tbf_alloc(struct gprs_rlcmac_bts *bts,
+struct gprs_rlcmac_ul_tbf *tbf_alloc_ul_tbf(struct gprs_rlcmac_bts *bts,
struct gprs_rlcmac_tbf *old_tbf,
- enum gprs_rlcmac_tbf_direction dir, uint8_t tfi, uint8_t trx,
+ uint8_t tfi, uint8_t trx,
+ uint8_t ms_class, uint8_t single_slot);
+
+struct gprs_rlcmac_dl_tbf *tbf_alloc_dl_tbf(struct gprs_rlcmac_bts *bts,
+ struct gprs_rlcmac_tbf *old_tbf,
+ uint8_t tfi, uint8_t trx,
uint8_t ms_class, uint8_t single_slot);
void tbf_free(struct gprs_rlcmac_tbf *tbf);
diff --git a/tests/alloc/AllocTest.cpp b/tests/alloc/AllocTest.cpp
index 80e8c34..830ca90 100644
--- a/tests/alloc/AllocTest.cpp
+++ b/tests/alloc/AllocTest.cpp
@@ -36,6 +36,17 @@ extern "C" {
void *tall_pcu_ctx;
int16_t spoof_mnc = 0, spoof_mcc = 0;
+static gprs_rlcmac_tbf *tbf_alloc(struct gprs_rlcmac_bts *bts,
+ struct gprs_rlcmac_tbf *old_tbf, gprs_rlcmac_tbf_direction dir,
+ uint8_t tfi, uint8_t trx,
+ uint8_t ms_class, uint8_t single_slot)
+{
+ if (dir == GPRS_RLCMAC_UL_TBF)
+ return tbf_alloc_ul_tbf(bts, old_tbf, tfi, trx, ms_class, single_slot);
+ else
+ return tbf_alloc_dl_tbf(bts, old_tbf, tfi, trx, ms_class, single_slot);
+}
+
static void test_alloc_a(gprs_rlcmac_tbf_direction dir, const int count)
{
int tfi;
@@ -136,14 +147,14 @@ static void test_alloc_b(int ms_class)
tfi = the_bts.tfi_find_free(GPRS_RLCMAC_UL_TBF, &trx_no, -1);
OSMO_ASSERT(tfi >= 0);
- ul_tbf = tbf_alloc(bts, NULL, GPRS_RLCMAC_UL_TBF, tfi, trx_no, ms_class, 1);
+ ul_tbf = tbf_alloc_ul_tbf(bts, NULL, tfi, trx_no, ms_class, 1);
OSMO_ASSERT(ul_tbf);
dump_assignment(ul_tbf, "UL");
/* assume final ack has not been sent */
tfi = the_bts.tfi_find_free(GPRS_RLCMAC_UL_TBF, &trx_no, -1);
OSMO_ASSERT(tfi >= 0);
- dl_tbf = tbf_alloc(bts, ul_tbf, GPRS_RLCMAC_DL_TBF, tfi, trx_no, ms_class, 0);
+ dl_tbf = tbf_alloc_dl_tbf(bts, ul_tbf, tfi, trx_no, ms_class, 0);
OSMO_ASSERT(dl_tbf);
dump_assignment(dl_tbf, "DL");
@@ -177,7 +188,7 @@ static void test_alloc_b(int ms_class)
tfi = the_bts.tfi_find_free(GPRS_RLCMAC_UL_TBF, &trx_no, -1);
OSMO_ASSERT(tfi >= 0);
- dl_tbf = tbf_alloc(bts, NULL, GPRS_RLCMAC_DL_TBF, tfi, trx_no, ms_class, 1);
+ dl_tbf = tbf_alloc_dl_tbf(bts, NULL, tfi, trx_no, ms_class, 1);
dl_tbf->m_tlli = 0x23;
dl_tbf->m_tlli_valid = true;
OSMO_ASSERT(dl_tbf);
@@ -185,7 +196,7 @@ static void test_alloc_b(int ms_class)
tfi = the_bts.tfi_find_free(GPRS_RLCMAC_UL_TBF, &trx_no, -1);
OSMO_ASSERT(tfi >= 0);
- ul_tbf = tbf_alloc(bts, dl_tbf, GPRS_RLCMAC_UL_TBF, tfi, trx_no, ms_class, 0);
+ ul_tbf = tbf_alloc_ul_tbf(bts, dl_tbf, tfi, trx_no, ms_class, 0);
ul_tbf->m_tlli = 0x23;
ul_tbf->m_tlli_valid = true;
ul_tbf->dir.ul.contention_resolution_done = 1;
@@ -226,14 +237,14 @@ static void test_alloc_b(int ms_class)
tfi = the_bts.tfi_find_free(GPRS_RLCMAC_UL_TBF, &trx_no, -1);
OSMO_ASSERT(tfi >= 0);
- ul_tbf = tbf_alloc(bts, NULL, GPRS_RLCMAC_UL_TBF, tfi, trx_no, ms_class, 0);
+ ul_tbf = tbf_alloc_ul_tbf(bts, NULL, tfi, trx_no, ms_class, 0);
OSMO_ASSERT(ul_tbf);
dump_assignment(ul_tbf, "UL");
/* assume final ack has not been sent */
tfi = the_bts.tfi_find_free(GPRS_RLCMAC_UL_TBF, &trx_no, -1);
OSMO_ASSERT(tfi >= 0);
- dl_tbf = tbf_alloc(bts, ul_tbf, GPRS_RLCMAC_DL_TBF, tfi, trx_no, ms_class, 0);
+ dl_tbf = tbf_alloc_dl_tbf(bts, ul_tbf, tfi, trx_no, ms_class, 0);
OSMO_ASSERT(dl_tbf);
dump_assignment(dl_tbf, "DL");
@@ -290,13 +301,13 @@ static void test_alloc_b(bool ts0, bool ts1, bool ts2, bool ts3,
bool ts4, bool
tfi = the_bts.tfi_find_free(GPRS_RLCMAC_UL_TBF, &trx_no, -1);
OSMO_ASSERT(tfi >= 0);
- ul_tbf = tbf_alloc(bts, NULL, GPRS_RLCMAC_UL_TBF, tfi, trx_no, ms_class, 1);
+ ul_tbf = tbf_alloc_ul_tbf(bts, NULL, tfi, trx_no, ms_class, 1);
OSMO_ASSERT(ul_tbf);
/* assume final ack has not been sent */
tfi = the_bts.tfi_find_free(GPRS_RLCMAC_UL_TBF, &trx_no, -1);
OSMO_ASSERT(tfi >= 0);
- dl_tbf = tbf_alloc(bts, ul_tbf, GPRS_RLCMAC_DL_TBF, tfi, trx_no, ms_class, 0);
+ dl_tbf = tbf_alloc_dl_tbf(bts, ul_tbf, tfi, trx_no, ms_class, 0);
OSMO_ASSERT(dl_tbf);
/* verify that both are on the same ts */
@@ -333,14 +344,14 @@ static void test_alloc_b(bool ts0, bool ts1, bool ts2, bool ts3,
bool ts4, bool
tfi = the_bts.tfi_find_free(GPRS_RLCMAC_UL_TBF, &trx_no, -1);
OSMO_ASSERT(tfi >= 0);
- dl_tbf = tbf_alloc(bts, NULL, GPRS_RLCMAC_DL_TBF, tfi, trx_no, ms_class, 1);
+ dl_tbf = tbf_alloc_dl_tbf(bts, NULL, tfi, trx_no, ms_class, 1);
OSMO_ASSERT(dl_tbf);
dl_tbf->m_tlli = 0x23;
dl_tbf->m_tlli_valid = true;
tfi = the_bts.tfi_find_free(GPRS_RLCMAC_UL_TBF, &trx_no, -1);
OSMO_ASSERT(tfi >= 0);
- ul_tbf = tbf_alloc(bts, dl_tbf, GPRS_RLCMAC_UL_TBF, tfi, trx_no, ms_class, 0);
+ ul_tbf = tbf_alloc_ul_tbf(bts, dl_tbf, tfi, trx_no, ms_class, 0);
OSMO_ASSERT(ul_tbf);
ul_tbf->m_tlli = 0x23;
ul_tbf->m_tlli_valid = true;
diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp
index 38975f9..decf4d9 100644
--- a/tests/tbf/TbfTest.cpp
+++ b/tests/tbf/TbfTest.cpp
@@ -44,16 +44,16 @@ static void test_tbf_tlli_update()
/*
* Make a uplink and downlink allocation
*/
- gprs_rlcmac_tbf *dl_tbf = tbf_alloc(the_bts.bts_data(),
- NULL, GPRS_RLCMAC_DL_TBF, 0,
+ gprs_rlcmac_tbf *dl_tbf = tbf_alloc_dl_tbf(the_bts.bts_data(),
+ NULL, 0,
0, 0, 0);
dl_tbf->update_tlli(0x2342);
dl_tbf->tlli_mark_valid();
dl_tbf->ta = 4;
the_bts.timing_advance()->remember(0x2342, dl_tbf->ta);
- gprs_rlcmac_tbf *ul_tbf = tbf_alloc(the_bts.bts_data(),
- ul_tbf, GPRS_RLCMAC_UL_TBF, 0,
+ gprs_rlcmac_tbf *ul_tbf = tbf_alloc_ul_tbf(the_bts.bts_data(),
+ ul_tbf, 0,
0, 0, 0);
ul_tbf->update_tlli(0x2342);
ul_tbf->tlli_mark_valid();
--
1.8.4.2
17 Jul
17 Jul
8:46 a.m.
On Wed, Jul 16, 2014 at 07:04:28PM +0200, Daniel Willmann wrote:
Hi!
as we have to move forward I will merge it. One comment in regard to
the approach taken here.
+gprs_rlcmac_tbf
*gprs_rlcmac_pdch::tbf_from_list_by_tfi(struct llist_head *tbf_list, uint8_t tfi,
+ enum gprs_rlcmac_tbf_direction dir)
{
gprs_rlcmac_tbf *tbf;
llist_for_each_entry(tbf, tbf_list, list) {
+ OSMO_ASSERT(tbf->direction == dir);
+ return
tbf_from_list_by_tfi(&bts_data()->ul_tbfs, tfi, GPRS_RLCMAC_UL_TBF);
+ return tbf_from_list_by_tfi(&bts_data()->dl_tbfs, tfi, GPRS_RLCMAC_DL_TBF);
assert it on _entry_ into the ul_tbfs and dl_tbfs. If we only have one
place where the list is manipulated we can easily pay the price on entry.
7 Aug
7 Aug
4:20 p.m.
Hi,
On Thu, 2014-07-17 at 08:46, Holger Hans Peter Freyther wrote:
On Wed, Jul 16, 2014 at 07:04:28PM +0200, Daniel
Willmann wrote:
as we have to move forward I will merge it. One comment in regard to
the approach taken here.
This is obsolete now anyway as the functions that do llist_add
explicitly allocate a DL/UL TBF.
I have a patch that will remove the ASSERTs completely
Regards,
Daniel Willmann
--
- Daniel Willmann <dwillmann(a)sysmocom.de> http://www.sysmocom.de/
=======================================================================
* sysmocom - systems for mobile communications GmbH
* Schivelbeiner Str. 5
* 10439 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B
* Geschaeftsfuehrer / Managing Directors: Holger Freyther, Harald Welte
+gprs_rlcmac_tbf
*gprs_rlcmac_pdch::tbf_from_list_by_tfi(struct llist_head *tbf_list, uint8_t tfi,
+ enum gprs_rlcmac_tbf_direction dir)
{
gprs_rlcmac_tbf *tbf;
llist_for_each_entry(tbf, tbf_list, list) {
+ OSMO_ASSERT(tbf->direction == dir);
+ return
tbf_from_list_by_tfi(&bts_data()->ul_tbfs, tfi, GPRS_RLCMAC_UL_TBF);
+ return tbf_from_list_by_tfi(&bts_data()->dl_tbfs, tfi, GPRS_RLCMAC_DL_TBF);
assert it on _entry_ into the ul_tbfs and dl_tbfs. If we only have one
place where the list is manipulated we can easily pay the price on entry.
3558
days inactive
3580
days old
8 comments
2 participants
participants (2)
-
Daniel Willmann -
Holger Hans Peter Freyther