Change in osmo-pcu[master]: Add new PDCH UL Controller, drop SBAllocator class
pespin has submitted this change. (
https://gerrit.osmocom.org/c/osmo-pcu/+/23291 )
Change subject: Add new PDCH UL Controller, drop SBAllocator class
..
Add new PDCH UL Controller, drop SBAllocator class
Right now we handle different types of UL allocations in different
classes like PollAllocator and SBAllocator, and they usually don't take
into account the other one in most cases. Furthermore, those objects are
usually per-BTS object, instead of per PDCH object.
This is a first step towards having a unified per-PDCH controller which
takes care of controlling what is scheduled and hence expected on the
uplink. Each PDCH has a UL Controller which keeps track of all reserved
uplink frame, be it SB, RRBP poll or USF assigned, all under the same
API.
As a first step, only the SBA part is fully implemented and used (being
it the easiest part to replace); TBF poll+usf will come in follow-up
patches later on. As a result, the SBAllocator per-BTS class dissappears
but some of its code is refactored/reused to provide more features to the
gprs_rlcmac_sba object, which is also further integrated into the new UL
Controller.
Related: OS#5020
Change-Id: I84b24beea4a1aa2c1528f41435f77bd16df2b947
---
M src/Makefile.am
M src/bts.cpp
M src/bts.h
M src/gprs_rlcmac_sched.cpp
M src/pcu_l1_if.cpp
M src/pcu_utils.h
M src/pdch.cpp
M src/pdch.h
A src/pdch_ul_controller.c
A src/pdch_ul_controller.h
M src/poll_controller.cpp
A src/sba.c
D src/sba.cpp
M src/sba.h
M src/tbf.cpp
M tests/tbf/TbfTest.cpp
M tests/tbf/TbfTest.err
17 files changed, 443 insertions(+), 225 deletions(-)
Approvals:
Jenkins Builder: Verified
osmith: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
diff --git a/src/Makefile.am b/src/Makefile.am
index eefbea1..ab9aeeb 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -62,9 +62,10 @@
tbf_dl.cpp \
bts.cpp \
pdch.cpp \
+ pdch_ul_controller.c \
poll_controller.cpp \
encoding.cpp \
- sba.cpp \
+ sba.c \
decoding.cpp \
llc.cpp \
rlc.cpp \
@@ -100,6 +101,7 @@
tbf_dl.h \
bts.h \
pdch.h \
+ pdch_ul_controller.h \
poll_controller.h \
encoding.h \
sba.h \
diff --git a/src/bts.cpp b/src/bts.cpp
index 1d3f690..a7d475c 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -212,7 +212,6 @@
* m_ms_store's destructor */
bts->ms_store->cleanup();
delete bts->ms_store;
- delete bts->sba;
delete bts->pollController;
if (bts->ratectrs) {
@@ -246,7 +245,6 @@
bts->nr = bts_nr;
bts->pollController = new PollController(*bts);
- bts->sba = new SBAController(*bts);
bts->ms_store = new GprsMsStorage(bts);
bts->cur_fn = 0;
@@ -819,10 +817,43 @@
return 0;
}
+struct gprs_rlcmac_sba *bts_alloc_sba(struct gprs_rlcmac_bts *bts, uint8_t ta)
+{
+ struct gprs_rlcmac_pdch *pdch;
+ struct gprs_rlcmac_sba *sba = NULL;
+ int8_t trx, ts;
+
+ if (!gsm48_ta_is_valid(ta))
+ return NULL;
+
+ for (trx = 0; trx < 8; trx++) {
+ for (ts = 7; ts >= 0; ts--) {
+ pdch = >trx[trx].pdch[ts];
+ if (!pdch->is_enabled())
+ continue;
+ break;
+ }
+ if (ts >= 0)
+ break;
+ }
+ if (trx == 8) {
+ LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH available.\n");
+ return NULL;
+ }
+
+ sba = sba_alloc(bts, pdch, ta);
+ if (!sba)
+ return NULL;
+
+ bts_do_rate_ctr_inc(bts, CTR_SBA_ALLOCATED);
+ return sba;
+}
+
int bts_rcv_rach(struct gprs_rlcmac_bts *bts, const struct rach_ind_params
*rip)
{
struct chan_req_params chan_req = { 0 };
struct gprs_rlcmac_ul_tbf *tbf = NULL;
+ struct gprs_rlcmac_sba *sba;
uint8_t trx_no, ts_no;
uint32_t sb_fn = 0;
uint8_t usf = 7;
@@ -864,14 +895,18 @@
/* Should we allocate a single block or an Uplink TBF? */
if (chan_req.single_block) {
- rc = bts_sba(bts)->alloc(_no, _no, _fn, ta);
- if (rc < 0) {
+ sba = bts_alloc_sba(bts, ta);
+ if (!sba) {
LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH resource for "
-"single block allocation: rc=%d\n", rc);
+"single block allocation\n");
+ rc = -EBUSY;
/* Send RR Immediate Assignment Reject */
goto send_imm_ass_rej;
}
+ trx_no = sba->pdch->trx_no();
+ ts_no = sba->pdch->ts_no;
+ sb_fn = sba->fn;
tsc = bts->trx[trx_no].pdch[ts_no].tsc;
LOGP(DRLCMAC,
Change in osmo-pcu[master]: Add new PDCH UL Controller, drop SBAllocator class
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/23291 ) Change subject: Add new PDCH UL Controller, drop SBAllocator class .. Patch Set 9: Code-Review+2 (1 comment) https://gerrit.osmocom.org/c/osmo-pcu/+/23291/8/src/pdch_ul_controller.c File src/pdch_ul_controller.c: https://gerrit.osmocom.org/c/osmo-pcu/+/23291/8/src/pdch_ul_controller.c@29 PS8, Line 29: ? > Why the question mark, is this an unresolved TODO? This is to safe my ass for now. Once everything is proven to work reliable and we know usual behavior, then we can fine-tune further :D -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/23291 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I84b24beea4a1aa2c1528f41435f77bd16df2b947 Gerrit-Change-Number: 23291 Gerrit-PatchSet: 9 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: lynxis lazus Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Comment-Date: Tue, 16 Mar 2021 10:02:28 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: osmith Gerrit-MessageType: comment
Change in osmo-pcu[master]: Add new PDCH UL Controller, drop SBAllocator class
osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/23291 ) Change subject: Add new PDCH UL Controller, drop SBAllocator class .. Patch Set 9: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/23291 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I84b24beea4a1aa2c1528f41435f77bd16df2b947 Gerrit-Change-Number: 23291 Gerrit-PatchSet: 9 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: lynxis lazus Gerrit-Reviewer: osmith Gerrit-Comment-Date: Tue, 16 Mar 2021 08:25:46 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-pcu[master]: Add new PDCH UL Controller, drop SBAllocator class
Hello osmith, Jenkins Builder, laforge, fixeria, daniel, lynxis lazus, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-pcu/+/23291 to look at the new patch set (#9). Change subject: Add new PDCH UL Controller, drop SBAllocator class .. Add new PDCH UL Controller, drop SBAllocator class Right now we handle different types of UL allocations in different classes like PollAllocator and SBAllocator, and they usually don't take into account the other one in most cases. Furthermore, those objects are usually per-BTS object, instead of per PDCH object. This is a first step towards having a unified per-PDCH controller which takes care of controlling what is scheduled and hence expected on the uplink. Each PDCH has a UL Controller which keeps track of all reserved uplink frame, be it SB, RRBP poll or USF assigned, all under the same API. As a first step, only the SBA part is fully implemented and used (being it the easiest part to replace); TBF poll+usf will come in follow-up patches later on. As a result, the SBAllocator per-BTS class dissappears but some of its code is refactored/reused to provide more features to the gprs_rlcmac_sba object, which is also further integrated into the new UL Controller. Related: OS#5020 Change-Id: I84b24beea4a1aa2c1528f41435f77bd16df2b947 --- M src/Makefile.am M src/bts.cpp M src/bts.h M src/gprs_rlcmac_sched.cpp M src/pcu_l1_if.cpp M src/pcu_utils.h M src/pdch.cpp M src/pdch.h A src/pdch_ul_controller.c A src/pdch_ul_controller.h M src/poll_controller.cpp A src/sba.c D src/sba.cpp M src/sba.h M src/tbf.cpp M tests/tbf/TbfTest.cpp M tests/tbf/TbfTest.err 17 files changed, 443 insertions(+), 225 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/91/23291/9 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/23291 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I84b24beea4a1aa2c1528f41435f77bd16df2b947 Gerrit-Change-Number: 23291 Gerrit-PatchSet: 9 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: lynxis lazus Gerrit-Reviewer: osmith Gerrit-MessageType: newpatchset
Change in osmo-pcu[master]: Add new PDCH UL Controller, drop SBAllocator class
osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/23291 ) Change subject: Add new PDCH UL Controller, drop SBAllocator class .. Patch Set 8: Code-Review+1 (2 comments) https://gerrit.osmocom.org/c/osmo-pcu/+/23291/8/src/pdch_ul_controller.c File src/pdch_ul_controller.c: https://gerrit.osmocom.org/c/osmo-pcu/+/23291/8/src/pdch_ul_controller.c@29 PS8, Line 29: ? Why the question mark, is this an unresolved TODO? https://gerrit.osmocom.org/c/osmo-pcu/+/23291/8/src/pdch_ul_controller.c@96 PS8, Line 96: new (with "new" as variable name, gerrit syntax highlighting is confused. maybe use another one?) -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/23291 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I84b24beea4a1aa2c1528f41435f77bd16df2b947 Gerrit-Change-Number: 23291 Gerrit-PatchSet: 8 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: lynxis lazus Gerrit-Reviewer: osmith Gerrit-Comment-Date: Mon, 15 Mar 2021 16:17:53 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-pcu[master]: Add new PDCH UL Controller, drop SBAllocator class
Hello Jenkins Builder, laforge, fixeria, lynxis lazus, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-pcu/+/23291 to look at the new patch set (#8). Change subject: Add new PDCH UL Controller, drop SBAllocator class .. Add new PDCH UL Controller, drop SBAllocator class Right now we handle different types of UL allocations in different classes like PollAllocator and SBAllocator, and they usually don't take into account the other one in most cases. Furthermore, those objects are usually per-BTS object, instead of per PDCH object. This is a first step towards having a unified per-PDCH controller which takes care of controlling what is scheduled and hence expected on the uplink. Each PDCH has a UL Controller which keeps track of all reserved uplink frame, be it SB, RRBP poll or USF assigned, all under the same API. As a first step, only the SBA part is fully implemented and used (being it the easiest part to replace); TBF poll+usf will come in follow-up patches later on. As a result, the SBAllocator per-BTS class dissappears but some of its code is refactored/reused to provide more features to the gprs_rlcmac_sba object, which is also further integrated into the new UL Controller. Related: OS#5020 Change-Id: I84b24beea4a1aa2c1528f41435f77bd16df2b947 --- M src/Makefile.am M src/bts.cpp M src/bts.h M src/gprs_rlcmac_sched.cpp M src/pcu_l1_if.cpp M src/pcu_utils.h M src/pdch.cpp M src/pdch.h A src/pdch_ul_controller.c A src/pdch_ul_controller.h M src/poll_controller.cpp A src/sba.c D src/sba.cpp M src/sba.h M src/tbf.cpp M tests/tbf/TbfTest.cpp M tests/tbf/TbfTest.err 17 files changed, 443 insertions(+), 225 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/91/23291/8 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/23291 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I84b24beea4a1aa2c1528f41435f77bd16df2b947 Gerrit-Change-Number: 23291 Gerrit-PatchSet: 8 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: lynxis lazus Gerrit-MessageType: newpatchset
Change in osmo-pcu[master]: Add new PDCH UL Controller, drop SBAllocator class
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/23291 ) Change subject: Add new PDCH UL Controller, drop SBAllocator class .. Patch Set 7: (5 comments) https://gerrit.osmocom.org/c/osmo-pcu/+/23291/7/src/pdch_ul_controller.c File src/pdch_ul_controller.c: https://gerrit.osmocom.org/c/osmo-pcu/+/23291/7/src/pdch_ul_controller.c@39 PS7, Line 39: /* > #if 0 I can actually remove this. https://gerrit.osmocom.org/c/osmo-pcu/+/23291/7/src/pdch_ul_controller.c@55 PS7, Line 55: iterate over rb-tree and free whatever > Doesn't talloc do this for us once you free() the ulc? Yes, I can drop this. https://gerrit.osmocom.org/c/osmo-pcu/+/23291/7/src/pdch_ul_controller.c@79 PS7, Line 79: this > I would avoid using a keyword as a symbol name. I know it's not C++ code, but > still. Ack https://gerrit.osmocom.org/c/osmo-pcu/+/23291/7/src/pdch_ul_controller.c@102 PS7, Line 102: /* It's OK if we don't find one, this func is used by RTS to see if we need to schedule one */ > Without FIXME / TODO, this looks like an early development leftover? […] I'll remove the commented log. https://gerrit.osmocom.org/c/osmo-pcu/+/23291/7/src/pdch_ul_controller.c@129 PS7, Line 129: this > Again keywords... It's confusing to see them used as symbol names in a mixed > C / C++ project. Ack -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/23291 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I84b24beea4a1aa2c1528f41435f77bd16df2b947 Gerrit-Change-Number: 23291 Gerrit-PatchSet: 7 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: lynxis lazus Gerrit-Comment-Date: Fri, 12 Mar 2021 13:49:47 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
Change in osmo-pcu[master]: Add new PDCH UL Controller, drop SBAllocator class
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/23291 ) Change subject: Add new PDCH UL Controller, drop SBAllocator class .. Patch Set 7: Code-Review+1 (5 comments) https://gerrit.osmocom.org/c/osmo-pcu/+/23291/7/src/pdch_ul_controller.c File src/pdch_ul_controller.c: https://gerrit.osmocom.org/c/osmo-pcu/+/23291/7/src/pdch_ul_controller.c@39 PS7, Line 39: /* #if 0 https://gerrit.osmocom.org/c/osmo-pcu/+/23291/7/src/pdch_ul_controller.c@55 PS7, Line 55: iterate over rb-tree and free whatever Doesn't talloc do this for us once you free() the ulc? https://gerrit.osmocom.org/c/osmo-pcu/+/23291/7/src/pdch_ul_controller.c@79 PS7, Line 79: this I would avoid using a keyword as a symbol name. I know it's not C++ code, but still. https://gerrit.osmocom.org/c/osmo-pcu/+/23291/7/src/pdch_ul_controller.c@102 PS7, Line 102: /* It's OK if we don't find one, this func is used by RTS to see if we need to schedule one */ Without FIXME / TODO, this looks like an early development leftover? If the logging message is still useful, then let's uncomment and use LOGL_DEBUG. https://gerrit.osmocom.org/c/osmo-pcu/+/23291/7/src/pdch_ul_controller.c@129 PS7, Line 129: this Again keywords... It's confusing to see them used as symbol names in a mixed C / C++ project. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/23291 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I84b24beea4a1aa2c1528f41435f77bd16df2b947 Gerrit-Change-Number: 23291 Gerrit-PatchSet: 7 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: lynxis lazus Gerrit-Comment-Date: Fri, 12 Mar 2021 13:28:12 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-pcu[master]: Add new PDCH UL Controller, drop SBAllocator class
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/23291 ) Change subject: Add new PDCH UL Controller, drop SBAllocator class .. Patch Set 7: This change is ready for review. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/23291 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I84b24beea4a1aa2c1528f41435f77bd16df2b947 Gerrit-Change-Number: 23291 Gerrit-PatchSet: 7 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: lynxis lazus Gerrit-Comment-Date: Fri, 12 Mar 2021 11:51:03 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
