Changes includes uplink PAN handling for Downlink tbf and bitmap handling to update Tentative ack or nacked --- src/decoding.cpp | 40 +++++++++++++++++++++ src/tbf.cpp | 2 ++ src/tbf.h | 1 + src/tbf_ul.cpp | 92 +++++++++++++++++++++++++++++++++++++++++++++-- tests/edge/EdgeTest.cpp | 68 +++++++++++++++++++++++++++++++++++ 5 files changed, 201 insertions(+), 2 deletions(-)
diff --git a/src/decoding.cpp b/src/decoding.cpp index 649f68d..d3d0d89 100644 --- a/src/decoding.cpp +++ b/src/decoding.cpp @@ -423,6 +423,19 @@ int Decoding::decode_ul_data_fanr_type1(struct gprs_rlc_ul_header_generic *rlc, rlc->block_info[1].data_len = data_len; rlc->data_offs_bytes[0] = 7; rlc->data_offs_bytes[1] = 7 + data_len + 1; + if ( rlc->pani == 1) { + const struct egprs_ssn_based_pan_header *egprs1_fanr; + egprs1_fanr = static_cast<struct egprs_ssn_based_pan_header*> + ((void *)&data[rlc->block_info[1].data_len + rlc->data_offs_bytes[1]]); + rlc->pan_info.bow = egprs1_fanr->bow; + + rlc->pan_info.short_ssn_a = egprs1_fanr->short_ssn_a; + rlc->pan_info.short_ssn_rb = egprs1_fanr->short_ssn_rb; + + rlc->pan_info.rb_a = egprs1_fanr->rb_a; + rlc->pan_info.rb_b = egprs1_fanr->rb_b; + rlc->pan_info.tfi = (egprs1_fanr->tfi_a << 0) | (egprs1_fanr->tfi_b << 4); + } return 0; }
@@ -502,6 +515,19 @@ int Decoding::decode_ul_data_fanr_type2(struct gprs_rlc_ul_header_generic *rlc, rlc->block_info[0].data_len = data_len; rlc->data_offs_bytes[0] = 6;
+ if ( rlc->pani == 1) { + const struct egprs_ssn_based_pan_header *egprs2_fanr; + egprs2_fanr = static_cast<struct egprs_ssn_based_pan_header*> + ((void *)&data[rlc->block_info[0].data_len + rlc->data_offs_bytes[0]]); + rlc->pan_info.bow = egprs2_fanr->bow; + + rlc->pan_info.short_ssn_a = egprs2_fanr->short_ssn_a; + rlc->pan_info.short_ssn_rb = egprs2_fanr->short_ssn_rb; + + rlc->pan_info.rb_a = egprs2_fanr->rb_a; + rlc->pan_info.rb_b = egprs2_fanr->rb_b; + rlc->pan_info.tfi = (egprs2_fanr->tfi_a << 0) | (egprs2_fanr->tfi_b << 4); + } if(*cs == GprsCodingScheme::MCS6){ if(rlc->cps == 2 || rlc->cps == 3 ){ rlc->block_info[0].data_len -= pad_bytes; @@ -588,6 +614,20 @@ int Decoding::decode_ul_data_fanr_type3(struct gprs_rlc_ul_header_generic *rlc, rlc->block_info[0].data_len = data_len; rlc->data_offs_bytes[0] = 5;
+ if ( rlc->pani == 1) { + const struct egprs_ssn_based_pan_header *egprs3_fanr; + egprs3_fanr = static_cast<struct egprs_ssn_based_pan_header*> + ((void *)&data[rlc->block_info[0].data_len + rlc->data_offs_bytes[0]]); + + rlc->pan_info.bow = egprs3_fanr->bow; + + rlc->pan_info.short_ssn_a = egprs3_fanr->short_ssn_a; + rlc->pan_info.short_ssn_rb = egprs3_fanr->short_ssn_rb; + + rlc->pan_info.rb_a = egprs3_fanr->rb_a; + rlc->pan_info.rb_b = egprs3_fanr->rb_b; + rlc->pan_info.tfi = (egprs3_fanr->tfi_a << 0) | (egprs3_fanr->tfi_b << 4); + } if(rlc->block_info[0].spb == 2){ if(*cs == GprsCodingScheme::MCS3){ if(rlc->cps == 6 || rlc->cps == 7 || rlc->cps == 8){ diff --git a/src/tbf.cpp b/src/tbf.cpp index 05f5c82..559e565 100644 --- a/src/tbf.cpp +++ b/src/tbf.cpp @@ -53,6 +53,8 @@ uint16_t windowsizebyTS[9] = { 1024 // 8 TS };
+uint16_t log2ws[9] = {6,7,8,8,9,9,9,9,10}; + gprs_rlcmac_tbf::Meas::Meas() : rssi_sum(0), rssi_num(0) diff --git a/src/tbf.h b/src/tbf.h index a6878d7..ba0dce1 100644 --- a/src/tbf.h +++ b/src/tbf.h @@ -504,6 +504,7 @@ struct gprs_rlcmac_ul_tbf : public gprs_rlcmac_tbf { uint8_t m_usf[8]; /* list USFs per PDCH (timeslot) */ uint8_t m_contention_resolution_done; /* set after done */ uint8_t m_final_ack_sent; /* set if we sent final ack */ + int uplink_pan_handling(const gprs_rlc_ul_header_generic *rlc);
protected: void maybe_schedule_uplink_acknack(const gprs_rlc_ul_header_generic *rlc); diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp index e89ac14..e56dccf 100644 --- a/src/tbf_ul.cpp +++ b/src/tbf_ul.cpp @@ -41,7 +41,8 @@ extern "C" { #define SEND_ACK_AFTER_FRAMES 20
extern void *tall_pcu_ctx; - +extern uint16_t log2ws[]; +extern uint16_t windowsizebyTS[];
/* * Store received block data in LLC message(s) and forward to SGSN @@ -392,9 +393,96 @@ int gprs_rlcmac_ul_tbf::rcv_data_block_acknowledged( /* If TLLI is included or if we received half of the window, we send * an ack/nack */ maybe_schedule_uplink_acknack(rlc); - + /*Uplink PAN info handling for downlink data, Need to update Ack/Nack + based on recieved bitmap */ + if (rlc->pani == 1) { + uplink_pan_handling(rlc); + } else { + LOGP(DRLCMACUL, LOGL_ERROR, "No PAN info present \n"); + } return 0; +}
+int gprs_rlcmac_ul_tbf::uplink_pan_handling( + const gprs_rlc_ul_header_generic *rlc) +{ + bool bit; + uint16_t bsn; + int i = 0, j; + int8_t rb_length, bits_left, shortSSNsize, mask, rb_mask; + int ssn = 0, ssn_base_va, ssn_base_vs; + + gprs_rlcmac_dl_tbf *tbf; + tbf = gprs_rlcmac_ul_tbf::ms()->dl_tbf(); + if (!tbf) { + LOGP(DRLCMACUL, LOGL_ERROR, "DL DATA unknown TFI=%d\n", + rlc->pan_info.tfi); + return 0; + } + if (tbf->tfi() != rlc->pan_info.tfi) { + LOGP(DRLCMACUL, LOGL_ERROR, "Received TFI %d and DL TBF Tfi %d are not\n" + "matching \n", tbf->tfi(), rlc->pan_info.tfi); + return 0; + } + for (j = 0; j < 9; j++) { + if (tbf->m_window.ws() == windowsizebyTS[j]) { + break; + } + } + shortSSNsize = log2ws[j] + 1; + mask = (1 << (shortSSNsize -7)) -1 ; + //Short SSN + int16_t short_ssn = ((rlc->pan_info.short_ssn_a) | + ((rlc->pan_info.short_ssn_rb << 8) & mask )); + //RB + rb_mask = (1 << (4 - (shortSSNsize -7))) -1 ; + + int16_t rb = ((rlc->pan_info.rb_b << (8 - (shortSSNsize -7))) | + (rlc->pan_info.rb_a << (4 - (shortSSNsize -7))) | + ((rlc->pan_info.short_ssn_rb >> (shortSSNsize -7)) & rb_mask )); + + //Get SSN from short SSN + ssn_base_va = (tbf->m_window.v_a() & (((1 << (11-shortSSNsize))-1) << shortSSNsize)) ; + ssn_base_vs = (tbf->m_window.v_s() & (((1 << (11-shortSSNsize))-1) << shortSSNsize)) ; + + if ( ssn_base_va == ssn_base_vs ) { + ssn = MOD2048((ssn_base_va|short_ssn)); + } + else { + /*Algorithm to be modified for the case V(A) is less than 2048 and V(S) is greater than + 2048 and RB comes in between these two */ + //TODO different cases to be handled + return 0; + } + /* if Beginning of window is set 1, Mark Acked between SSN-2 ( V(Q)-1) to V(A)*/ + if (rlc->pan_info.bow) { + for (int i = tbf->m_window.v_a(); i <= (ssn -2); i++ ) { + uint16_t bsn = tbf->m_window.mod_sns(i); + tbf->m_window.m_v_b.mark_tentative_acked(bsn); + } + } + + rb_length = 8 + (4 - (shortSSNsize -7)); + + //Mark TENTATIVE_ACK + rb = rb << (16 - rb_length); + bits_left = rb_length; + + while (bits_left != 0) { + bsn = tbf->m_window.mod_sns(ssn + i); + if (tbf->m_window.mod_sns(tbf->m_window.v_s() - bsn) >= tbf->m_window.distance()) + break; + bit = !!(rb & 0x8000); + if (bit) { + tbf->m_window.m_v_b.mark_tentative_acked(bsn); + } else { + tbf->m_window.m_v_b.mark_nacked(bsn); + } + rb = rb << 1; + bits_left--; + i++; + } + return 1; }
void gprs_rlcmac_ul_tbf::maybe_schedule_uplink_acknack( diff --git a/tests/edge/EdgeTest.cpp b/tests/edge/EdgeTest.cpp index 7177832..924728f 100644 --- a/tests/edge/EdgeTest.cpp +++ b/tests/edge/EdgeTest.cpp @@ -1641,7 +1641,74 @@ static gprs_rlcmac_ul_tbf *establish_ul_tbf_two_phase_cv0(BTS *the_bts, return ul_tbf; }
+void fanr_handling(gprs_rlcmac_ul_tbf *ul_tbf, BTS *the_bts, uint32_t *fn, uint8_t ts_no, + GprsMs *ms , uint32_t tlli, uint16_t qta) +{ + uint8_t data_msg[200] = {0}; + int tfi = 0; + struct gprs_rlcmac_pdch *pdch; + uint32_t rach_fn = *fn - 51; + uint32_t sba_fn = *fn + 52; + uint8_t trx_no = 0; + + tfi = ul_tbf->tfi(); + struct pcu_l1_meas meas; + meas.set_rssi(31); + + ul_tbf->enable_fanr(); + /* send fake data */ + data_msg[0] = 0x00 | 0xf << 2; + data_msg[1] = uint8_t(0 | (tfi << 1)); + data_msg[2] = uint8_t(0);/* BSN:7, E:1 */ + data_msg[3] =uint8_t(1 << 6); /* BSN:7, E:1 */ + data_msg[4] = 0x2b; /* E: 1 */ + data_msg[5] = 0x2b; /* E: 1 */ + data_msg[6] = 0x2b; /* E: 1 */ + data_msg[7] = 0x2b; /* E: 1 */ + data_msg[8] = 0x2b; /* E: 1 */ + data_msg[9] = 0x2b; /* E: 1 */ + data_msg[10] = 0x2b; /* E: 1 */ + data_msg[11] = 0x2b; /* E: 1 */ + data_msg[12] = 0x2b; /* E: 1 */ + data_msg[13] = 0x2b; /* E: 1 */ + data_msg[14] = 0x2b; /* E: 1 */ + data_msg[15] = 0x2b; /* E: 1 */ + data_msg[16] = 0x2b; /* E: 1 */ + data_msg[17] = 0x2b; /* E: 1 */ + data_msg[18] = 0x2b; /* E: 1 */ + data_msg[19] = 0x2b; /* E: 1 */ + data_msg[20] = 0x2b; /* E: 1 */ + data_msg[21] = 0x2b; /* E: 1 */ + data_msg[22] = 0x2b; /* E: 1 */ + data_msg[23] = 0x2b; /* E: 1 */ + data_msg[24] = 0x2b; /* E: 1 */ + data_msg[25] = 0x2b; /* E: 1 */ + data_msg[26] = 0x2b; + data_msg[27] = 0x02; /* Fanr SSN & BOW */ + data_msg[28] = 0xff; /* RB & SSN */ + data_msg[29] = 0x0f; /* TFI & RB */ + data_msg[30] = 0x00; /* TFI */ + data_msg[31] = 0x00; + + pdch = &the_bts->bts_data()->trx[trx_no].pdch[ts_no]; + pdch->rcv_block(&data_msg[0], 31, *fn, &meas); + + ms = the_bts->ms_by_tlli(tlli); + OSMO_ASSERT(ms != NULL); + OSMO_ASSERT(ms->ta() == qta/4); + OSMO_ASSERT(ms->ul_tbf() == ul_tbf); + + + /* send fake data */ + data_msg[0] = 0x00 | 0xf << 2; + data_msg[1] = uint8_t(0 | (tfi << 1)); + data_msg[2] = uint8_t(0);/* BSN:7, E:1 */ + data_msg[3] =uint8_t(1 << 6); /* BSN:7, E:1 */ + data_msg[4] = uint8_t(1); /* E: 1 */
+ pdch = &the_bts->bts_data()->trx[trx_no].pdch[ts_no]; + pdch->rcv_block(&data_msg[0], 37, *fn, &meas); +} void test_tbf_two_phase() { BTS the_bts; @@ -1669,6 +1736,7 @@ void test_tbf_two_phase()
/* Send Packet Downlink Assignment to MS */ request_dl_rlc_block(ul_tbf, &fn); + fanr_handling (ul_tbf, &the_bts, &fn, ts_no, ms, tlli, qta); printf("=== end %s ===\n", __func__); }
On 10 Mar 2016, at 15:20, sangamesh sajjan sangamesh.sajjan@radisys.com wrote:
Hi,
Changes includes uplink PAN handling for Downlink tbf and bitmap handling to update Tentative ack or nacked
we have been waiting for the second version of it. There are too many coding style violations so it doesn't make sense.
src/decoding.cpp | 40 +++++++++++++++++++++ src/tbf.cpp | 2 ++ src/tbf.h | 1 + src/tbf_ul.cpp | 92 +++++++++++++++++++++++++++++++++++++++++++++-- tests/edge/EdgeTest.cpp | 68 +++++++++++++++++++++++++++++++++++ 5 files changed, 201 insertions(+), 2 deletions(-)
diff --git a/src/decoding.cpp b/src/decoding.cpp index 649f68d..d3d0d89 100644 --- a/src/decoding.cpp +++ b/src/decoding.cpp @@ -423,6 +423,19 @@ int Decoding::decode_ul_data_fanr_type1(struct gprs_rlc_ul_header_generic *rlc, rlc->block_info[1].data_len = data_len; rlc->data_offs_bytes[0] = 7; rlc->data_offs_bytes[1] = 7 + data_len + 1;
- if ( rlc->pani == 1) {
const struct egprs_ssn_based_pan_header *egprs1_fanr;egprs1_fanr = static_cast<struct egprs_ssn_based_pan_header*>((void *)&data[rlc->block_info[1].data_len + rlc->data_offs_bytes[1]]);
why do you need to combine old C style cast with C++ one?
+uint16_t log2ws[9] = {6,7,8,8,9,9,9,9,10};
why is it define in tbf.cpp and not used there? ws == window size?
gprs_rlcmac_tbf::Meas::Meas() : rssi_sum(0), rssi_num(0) diff --git a/src/tbf.h b/src/tbf.h index a6878d7..ba0dce1 100644 --- a/src/tbf.h +++ b/src/tbf.h @@ -504,6 +504,7 @@ struct gprs_rlcmac_ul_tbf : public gprs_rlcmac_tbf { uint8_t m_usf[8]; /* list USFs per PDCH (timeslot) */ uint8_t m_contention_resolution_done; /* set after done */ uint8_t m_final_ack_sent; /* set if we sent final ack */
- int uplink_pan_handling(const gprs_rlc_ul_header_generic *rlc);
What should it do? What should this method do? I can not say based on the name.
+int gprs_rlcmac_ul_tbf::uplink_pan_handling(
const gprs_rlc_ul_header_generic *rlc)+{
Code is read a lot more frequently than it is written. For being able to unit test and also for allowing people to read, methods should be small. I think you will find a nice way to split the different concerns and make it more straight forward.
- return 1;
}
void gprs_rlcmac_ul_tbf::maybe_schedule_uplink_acknack( diff --git a/tests/edge/EdgeTest.cpp b/tests/edge/EdgeTest.cpp index 7177832..924728f 100644 --- a/tests/edge/EdgeTest.cpp +++ b/tests/edge/EdgeTest.cpp @@ -1641,7 +1641,74 @@ static gprs_rlcmac_ul_tbf *establish_ul_tbf_two_phase_cv0(BTS *the_bts, return ul_tbf; }
+void fanr_handling(gprs_rlcmac_ul_tbf *ul_tbf, BTS *the_bts, uint32_t *fn, uint8_t ts_no,
GprsMs *ms , uint32_t tlli, uint16_t qta)+{
- ms = the_bts->ms_by_tlli(tlli);
- OSMO_ASSERT(ms != NULL);
- OSMO_ASSERT(ms->ta() == qta/4);
- OSMO_ASSERT(ms->ul_tbf() == ul_tbf);
these are the only asserts in the testcase and they do not concern to anything related to FANR. Could you please elaborate about it?
holger
Hello,
Thank you for the comments, Currently I'm working on merging of compression algorithm to master branch. I will resubmit the patch by considering all your comments after completion of current activity.
Thanks, Sangamesh
-----Original Message----- From: Holger Freyther [mailto:holger@freyther.de] Sent: Wednesday, March 16, 2016 7:18 PM To: Sangamesh Sajjan Sangamesh.Sajjan@radisys.com Cc: osmocom-net-gprs@lists.osmocom.org Subject: Re: [PATCH] Uplink PAN handling for FANR feature
On 10 Mar 2016, at 15:20, sangamesh sajjan sangamesh.sajjan@radisys.com wrote:
Hi,
Changes includes uplink PAN handling for Downlink tbf and bitmap handling to update Tentative ack or nacked
we have been waiting for the second version of it. There are too many coding style violations so it doesn't make sense.
src/decoding.cpp | 40 +++++++++++++++++++++ src/tbf.cpp | 2 ++ src/tbf.h | 1 + src/tbf_ul.cpp | 92 +++++++++++++++++++++++++++++++++++++++++++++-- tests/edge/EdgeTest.cpp | 68 +++++++++++++++++++++++++++++++++++ 5 files changed, 201 insertions(+), 2 deletions(-)
diff --git a/src/decoding.cpp b/src/decoding.cpp index 649f68d..d3d0d89 100644 --- a/src/decoding.cpp +++ b/src/decoding.cpp @@ -423,6 +423,19 @@ int Decoding::decode_ul_data_fanr_type1(struct gprs_rlc_ul_header_generic *rlc, rlc->block_info[1].data_len = data_len; rlc->data_offs_bytes[0] = 7; rlc->data_offs_bytes[1] = 7 + data_len + 1;
- if ( rlc->pani == 1) {
const struct egprs_ssn_based_pan_header *egprs1_fanr;egprs1_fanr = static_cast<struct egprs_ssn_based_pan_header*>((void *)&data[rlc->block_info[1].data_len ++rlc->data_offs_bytes[1]]);
why do you need to combine old C style cast with C++ one?
+uint16_t log2ws[9] = {6,7,8,8,9,9,9,9,10};
why is it define in tbf.cpp and not used there? ws == window size?
gprs_rlcmac_tbf::Meas::Meas() : rssi_sum(0), rssi_num(0) diff --git a/src/tbf.h b/src/tbf.h index a6878d7..ba0dce1 100644 --- a/src/tbf.h +++ b/src/tbf.h @@ -504,6 +504,7 @@ struct gprs_rlcmac_ul_tbf : public gprs_rlcmac_tbf { uint8_t m_usf[8]; /* list USFs per PDCH (timeslot) */ uint8_t m_contention_resolution_done; /* set after done */ uint8_t m_final_ack_sent; /* set if we sent final ack */
- int uplink_pan_handling(const gprs_rlc_ul_header_generic *rlc);
What should it do? What should this method do? I can not say based on the name.
+int gprs_rlcmac_ul_tbf::uplink_pan_handling(
const gprs_rlc_ul_header_generic *rlc) {
Code is read a lot more frequently than it is written. For being able to unit test and also for allowing people to read, methods should be small. I think you will find a nice way to split the different concerns and make it more straight forward.
- return 1;
}
void gprs_rlcmac_ul_tbf::maybe_schedule_uplink_acknack( diff --git a/tests/edge/EdgeTest.cpp b/tests/edge/EdgeTest.cpp index 7177832..924728f 100644 --- a/tests/edge/EdgeTest.cpp +++ b/tests/edge/EdgeTest.cpp @@ -1641,7 +1641,74 @@ static gprs_rlcmac_ul_tbf *establish_ul_tbf_two_phase_cv0(BTS *the_bts, return ul_tbf; }
+void fanr_handling(gprs_rlcmac_ul_tbf *ul_tbf, BTS *the_bts, uint32_t *fn, uint8_t ts_no,
GprsMs *ms , uint32_t tlli, uint16_t qta) {ms = the_bts->ms_by_tlli(tlli);
OSMO_ASSERT(ms != NULL);
OSMO_ASSERT(ms->ta() == qta/4);
OSMO_ASSERT(ms->ul_tbf() == ul_tbf);
these are the only asserts in the testcase and they do not concern to anything related to FANR. Could you please elaborate about it?
holger
osmocom-net-gprs@lists.osmocom.org