aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJacob Erlbeck <jerlbeck@sysmocom.de>2015-05-22 15:47:55 +0200
committerJacob Erlbeck <jerlbeck@sysmocom.de>2015-05-28 12:29:43 +0200
commitd3eac2867a84d009fa3c1c7d8da9502f8468fbca (patch)
tree52c91d907249f0eefe6d070af1141273eadb3252
parent1c68abaffab6b7f8472d54b7881c3618d72e00d9 (diff)
tbf: Remove TBF chaining (m_new_tbf and m_old_tbf)
Currently a new TBF is chained to an existing older one, either of the other direction (active or releasing) or of the same direction (releasing). This does not work properly work if and uplink and a downlink TBF are being established at the same time while an old TBF is being released. In that case, one of them is thrown away and the pending procedure is cancelled. The chaining is no longer necessary since the GprsMs objects have been introduced which keep track of the active TBFs. This commit removes the TBF members m_new_tbf and m_old_tbf and the related methods and code paths. Note that a new TBF can replace an older TBF entry of the same direction within an MS object when it is associated with an MS (e.g. by TLLI or because it is assigned via another, already associated TBF). In that case, the old TBF is no longer associated with an MS object. Ticket: #1674 Sponsored-by: On-Waves ehf
-rw-r--r--src/bts.cpp19
-rw-r--r--src/tbf.cpp70
-rw-r--r--src/tbf.h11
-rw-r--r--tests/tbf/TbfTest.cpp15
-rw-r--r--tests/tbf/TbfTest.err6
5 files changed, 25 insertions, 96 deletions
diff --git a/src/bts.cpp b/src/bts.cpp
index f133977..c7daf8e 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -482,7 +482,6 @@ void BTS::trigger_dl_ass(
"PACCH, because %s exists\n", tbf_name(old_tbf));
old_tbf->dl_ass_state = GPRS_RLCMAC_DL_ASS_SEND_ASS;
- old_tbf->set_new_tbf(dl_tbf);
old_tbf->was_releasing = old_tbf->state_is(GPRS_RLCMAC_WAIT_RELEASE);
/* use TA from old TBF */
@@ -498,7 +497,6 @@ void BTS::trigger_dl_ass(
/* change state */
dl_tbf->set_state(GPRS_RLCMAC_ASSIGN);
dl_tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_CCCH);
- dl_tbf->set_new_tbf(dl_tbf);
/* send immediate assignment */
dl_tbf->bts->snd_dl_ass(dl_tbf, 0, dl_tbf->imsi());
dl_tbf->m_wait_confirm = 1;
@@ -713,11 +711,7 @@ void gprs_rlcmac_pdch::rcv_control_ack(Packet_Control_Acknowledgement_t *packet,
return;
}
tbf->update_tlli(tlli);
-
- if (tbf->new_tbf())
- tbf->new_tbf()->update_ms(tlli, GPRS_RLCMAC_UL_TBF);
- else
- tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF);
+ tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF);
LOGP(DRLCMAC, LOGL_DEBUG, "RX: [PCU <- BTS] %s Packet Control Ack\n", tbf_name(tbf));
tbf->poll_state = GPRS_RLCMAC_POLL_NONE;
@@ -742,7 +736,7 @@ void gprs_rlcmac_pdch::rcv_control_ack(Packet_Control_Acknowledgement_t *packet,
tbf->n3105 = 0;
tbf->dl_ass_state = GPRS_RLCMAC_DL_ASS_NONE;
- new_tbf = tbf->new_tbf();
+ new_tbf = tbf->ms() ? tbf->ms()->dl_tbf() : NULL;
if (!new_tbf) {
LOGP(DRLCMAC, LOGL_ERROR, "Got ACK, but DL "
"TBF is gone TLLI=0x%08x\n", tlli);
@@ -770,7 +764,7 @@ void gprs_rlcmac_pdch::rcv_control_ack(Packet_Control_Acknowledgement_t *packet,
tbf->n3105 = 0;
tbf->ul_ass_state = GPRS_RLCMAC_UL_ASS_NONE;
- new_tbf = tbf->new_tbf();
+ new_tbf = tbf->ms() ? tbf->ms()->ul_tbf() : NULL;
if (!new_tbf) {
LOGP(DRLCMAC, LOGL_ERROR, "Got ACK, but UL "
"TBF is gone TLLI=0x%08x\n", tlli);
@@ -837,7 +831,11 @@ void gprs_rlcmac_pdch::rcv_control_dl_ack_nack(Packet_Downlink_Ack_Nack_t *ack_n
if (ack_nack->Exist_Channel_Request_Description) {
LOGP(DRLCMAC, LOGL_DEBUG, "MS requests UL TBF in ack "
"message, so we provide one:\n");
- tbf->set_new_tbf(tbf_alloc_ul(bts_data(), tbf->trx->trx_no, tbf->ms_class, tbf->tlli(), tbf->ta, tbf));
+
+ /* This call will register the new TBF with the MS on success */
+ tbf_alloc_ul(bts_data(), tbf->trx->trx_no, tbf->ms_class,
+ tbf->tlli(), tbf->ta, tbf);
+
/* schedule uplink assignment */
tbf->ul_ass_state = GPRS_RLCMAC_UL_ASS_SEND_ASS;
}
@@ -908,7 +906,6 @@ void gprs_rlcmac_pdch::rcv_resource_request(Packet_Resource_Request_t *request,
if (!ul_tbf)
return;
- ul_tbf->set_new_tbf(ul_tbf);
/* 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);
ul_tbf->control_ts = ts_no;
diff --git a/src/tbf.cpp b/src/tbf.cpp
index 4bafe14..ac0cdf3 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -89,31 +89,6 @@ void gprs_rlcmac_tbf::assign_imsi(const char *imsi_)
m_ms->set_imsi(imsi_);
}
-void gprs_rlcmac_tbf::set_new_tbf(gprs_rlcmac_tbf *tbf)
-{
- if (m_new_tbf) {
- if (m_new_tbf == tbf) {
- LOGP(DRLCMAC, LOGL_INFO,
- "%s reassigning %s to m_new_tbf\n",
- tbf_name(this), tbf_name(tbf));
- return;
- }
- if (m_new_tbf != this) {
- LOGP(DRLCMAC, LOGL_NOTICE,
- "%s m_new_tbf is already assigned to %s, "
- "overwriting the old value with %s\n",
- tbf_name(this), tbf_name(m_new_tbf), tbf_name(tbf));
- }
- /* Detach from other TBF */
- m_new_tbf->m_old_tbf = NULL;
- }
- m_new_tbf = tbf;
- tbf->m_old_tbf = this;
-
- if (!tbf->ms())
- tbf->set_ms(ms());
-}
-
void gprs_rlcmac_tbf::set_ms(GprsMs *ms)
{
if (m_ms == ms)
@@ -227,40 +202,6 @@ void tbf_free(struct gprs_rlcmac_tbf *tbf)
else
tbf->bts->tbf_dl_freed();
- if (tbf->m_old_tbf) {
- if (tbf->m_old_tbf == tbf) {
- /* points to itself, ignore */
- } else if (tbf->m_old_tbf->m_new_tbf == tbf) {
- LOGP(DRLCMAC, LOGL_INFO,
- "%s Old TBF %s still exists, detaching\n",
- tbf_name(tbf), tbf_name(tbf->m_old_tbf));
- tbf->m_old_tbf->m_new_tbf = NULL;
- } else {
- LOGP(DRLCMAC, LOGL_ERROR, "%s Software error: "
- "tbf->m_old_tbf->m_new_tbf != tbf\n",
- tbf_name(tbf));
- }
-
- tbf->m_old_tbf = NULL;
- }
-
- if (tbf->m_new_tbf) {
- if (tbf->m_new_tbf == tbf) {
- /* points to itself, ignore */
- } else if (tbf->m_new_tbf->m_old_tbf == tbf) {
- LOGP(DRLCMAC, LOGL_INFO,
- "%s New TBF %s still exists, detaching\n",
- tbf_name(tbf), tbf_name(tbf->m_new_tbf));
- tbf->m_new_tbf->m_old_tbf = NULL;
- } else {
- LOGP(DRLCMAC, LOGL_ERROR, "%s Software error: "
- "tbf->m_new_tbf->m_old_tbf != tbf\n",
- tbf_name(tbf));
- }
-
- tbf->m_new_tbf = NULL;
- }
-
if (tbf->ms())
tbf->set_ms(NULL);
@@ -668,7 +609,7 @@ int gprs_rlcmac_tbf::rlcmac_diag()
struct msgb *gprs_rlcmac_tbf::create_dl_ass(uint32_t fn)
{
struct msgb *msg;
- struct gprs_rlcmac_dl_tbf *new_dl_tbf;
+ struct gprs_rlcmac_dl_tbf *new_dl_tbf = NULL;
int poll_ass_dl = 1;
if (direction == GPRS_RLCMAC_DL_TBF && control_ts != first_common_ts) {
@@ -706,7 +647,9 @@ struct msgb *gprs_rlcmac_tbf::create_dl_ass(uint32_t fn)
}
}
- new_dl_tbf = static_cast<gprs_rlcmac_dl_tbf *>(m_new_tbf);
+ if (ms())
+ new_dl_tbf = ms()->dl_tbf();
+
if (!new_dl_tbf) {
LOGP(DRLCMACDL, LOGL_ERROR, "We have a schedule for downlink "
"assignment at uplink %s, but there is no downlink "
@@ -758,7 +701,7 @@ struct msgb *gprs_rlcmac_tbf::create_dl_ass(uint32_t fn)
struct msgb *gprs_rlcmac_tbf::create_ul_ass(uint32_t fn)
{
struct msgb *msg;
- struct gprs_rlcmac_ul_tbf *new_tbf;
+ struct gprs_rlcmac_ul_tbf *new_tbf = NULL;
if (poll_state != GPRS_RLCMAC_POLL_NONE) {
LOGP(DRLCMACUL, LOGL_DEBUG, "Polling is already "
@@ -772,7 +715,8 @@ struct msgb *gprs_rlcmac_tbf::create_ul_ass(uint32_t fn)
return NULL;
}
- new_tbf = static_cast<gprs_rlcmac_ul_tbf *>(m_new_tbf);
+ if (ms())
+ new_tbf = ms()->ul_tbf();
if (!new_tbf) {
LOGP(DRLCMACUL, LOGL_ERROR, "We have a schedule for uplink "
"assignment at downlink %s, but there is no uplink "
diff --git a/src/tbf.h b/src/tbf.h
index 0820065..5ea6d4e 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -150,9 +150,6 @@ struct gprs_rlcmac_tbf {
const char *imsi() const;
void assign_imsi(const char *imsi);
- void set_new_tbf(gprs_rlcmac_tbf *tbf);
- gprs_rlcmac_tbf *new_tbf() const;
-
time_t created_ts() const;
/* attempt to make things a bit more fair */
@@ -176,9 +173,6 @@ struct gprs_rlcmac_tbf {
enum gprs_rlcmac_tbf_ul_ass_state ul_ass_state;
enum gprs_rlcmac_tbf_ul_ack_state ul_ack_state;
- gprs_rlcmac_tbf *m_new_tbf;
- gprs_rlcmac_tbf *m_old_tbf; /* reverse pointer for m_new_tbf */
-
enum gprs_rlcmac_tbf_poll_state poll_state;
uint32_t poll_fn; /* frame number to poll */
@@ -297,11 +291,6 @@ inline uint8_t gprs_rlcmac_tbf::tfi() const
return m_tfi;
}
-inline gprs_rlcmac_tbf *gprs_rlcmac_tbf::new_tbf() const
-{
- return m_new_tbf;
-}
-
inline time_t gprs_rlcmac_tbf::created_ts() const
{
return m_created_ts;
diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp
index 83d0a70..84ecb97 100644
--- a/tests/tbf/TbfTest.cpp
+++ b/tests/tbf/TbfTest.cpp
@@ -44,8 +44,6 @@ int16_t spoof_mnc = 0, spoof_mcc = 0;
static void check_tbf(gprs_rlcmac_tbf *tbf)
{
OSMO_ASSERT(tbf);
- OSMO_ASSERT(tbf->m_new_tbf == NULL || tbf->m_new_tbf->m_old_tbf == tbf);
- OSMO_ASSERT(tbf->m_old_tbf == NULL || tbf->m_old_tbf->m_new_tbf == tbf);
}
static void test_tbf_tlli_update()
@@ -143,7 +141,6 @@ static gprs_rlcmac_dl_tbf *create_dl_tbf(BTS *the_bts, uint8_t ms_class,
dl_tbf->dl_ass_state = GPRS_RLCMAC_DL_ASS_SEND_ASS;
dl_tbf->set_state(GPRS_RLCMAC_FLOW);
dl_tbf->m_wait_confirm = 0;
- dl_tbf->set_new_tbf(dl_tbf);
check_tbf(dl_tbf);
*trx_no_ = trx_no;
@@ -177,6 +174,7 @@ static void test_tbf_final_ack(enum test_tbf_final_ack_mode test_mode)
uint32_t fn;
uint8_t block_nr;
uint8_t trx_no;
+ GprsMs *ms;
uint32_t tlli = 0xffeeddcc;
uint8_t rbb[64/8];
@@ -189,6 +187,7 @@ static void test_tbf_final_ack(enum test_tbf_final_ack_mode test_mode)
setup_bts(&the_bts, ts_no);
dl_tbf = create_dl_tbf(&the_bts, ms_class, &trx_no);
dl_tbf->update_ms(tlli, GPRS_RLCMAC_DL_TBF);
+ ms = dl_tbf->ms();
for (i = 0; i < sizeof(llc_data); i++)
llc_data[i] = i%256;
@@ -215,22 +214,25 @@ static void test_tbf_final_ack(enum test_tbf_final_ack_mode test_mode)
/* Clean up and ensure tbfs are in the correct state */
OSMO_ASSERT(dl_tbf->state_is(GPRS_RLCMAC_WAIT_RELEASE));
- new_tbf = dl_tbf->new_tbf();
+ new_tbf = ms->dl_tbf();
check_tbf(new_tbf);
OSMO_ASSERT(new_tbf != dl_tbf);
OSMO_ASSERT(new_tbf->tfi() == 1);
check_tbf(dl_tbf);
dl_tbf->dl_ass_state = GPRS_RLCMAC_DL_ASS_NONE;
if (test_mode == TEST_MODE_REVERSE_FREE) {
+ GprsMs::Guard guard(ms);
tbf_free(new_tbf);
- OSMO_ASSERT(dl_tbf->m_new_tbf != new_tbf);
+ OSMO_ASSERT(ms->dl_tbf() == NULL);
check_tbf(dl_tbf);
tbf_free(dl_tbf);
} else {
+ GprsMs::Guard guard(ms);
tbf_free(dl_tbf);
- OSMO_ASSERT(new_tbf->m_new_tbf != dl_tbf);
+ OSMO_ASSERT(ms->dl_tbf() == new_tbf);
check_tbf(new_tbf);
tbf_free(new_tbf);
+ OSMO_ASSERT(ms->dl_tbf() == NULL);
}
}
@@ -302,7 +304,6 @@ static void test_tbf_delayed_release()
/* Clean up and ensure tbfs are in the correct state */
OSMO_ASSERT(dl_tbf->state_is(GPRS_RLCMAC_WAIT_RELEASE));
- OSMO_ASSERT(dl_tbf->new_tbf() == dl_tbf);
dl_tbf->dl_ass_state = GPRS_RLCMAC_DL_ASS_NONE;
check_tbf(dl_tbf);
tbf_free(dl_tbf);
diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err
index 2d97c9b..03921c5 100644
--- a/tests/tbf/TbfTest.err
+++ b/tests/tbf/TbfTest.err
@@ -83,13 +83,12 @@ TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=NULL) changes state from NULL to ASSIGN
TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=ASSIGN) starting timer 0.
DL packet loss of IMSI= / TLLI=0x00000000: 0%
TBF(TFI=0 TLLI=0x00000000 DIR=DL STATE=WAIT RELEASE) free
-TBF(TFI=0 TLLI=0x00000000 DIR=DL STATE=WAIT RELEASE) New TBF TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=ASSIGN) still exists, detaching
********** TBF ends here **********
TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=ASSIGN) free
TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=ASSIGN) stopping timer 0.
Detaching TBF from MS object, TLLI = 0xffeeddcc, TBF = TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=ASSIGN)
-Destroying MS object, TLLI = 0xffeeddcc
********** TBF ends here **********
+Destroying MS object, TLLI = 0xffeeddcc
Searching for first unallocated TFI: TRX=0 first TS=4
Found TFI=0.
********** TBF starts here **********
@@ -154,13 +153,12 @@ TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=NULL) changes state from NULL to ASSIGN
TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=ASSIGN) starting timer 0.
TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=ASSIGN) free
TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=ASSIGN) stopping timer 0.
-TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=ASSIGN) Old TBF TBF(TFI=0 TLLI=0x00000000 DIR=DL STATE=WAIT RELEASE) still exists, detaching
Detaching TBF from MS object, TLLI = 0xffeeddcc, TBF = TBF(TFI=1 TLLI=0xffeeddcc DIR=DL STATE=ASSIGN)
-Destroying MS object, TLLI = 0xffeeddcc
********** TBF ends here **********
DL packet loss of IMSI= / TLLI=0x00000000: 0%
TBF(TFI=0 TLLI=0x00000000 DIR=DL STATE=WAIT RELEASE) free
********** TBF ends here **********
+Destroying MS object, TLLI = 0xffeeddcc
Searching for first unallocated TFI: TRX=0 first TS=4
Found TFI=0.
********** TBF starts here **********