[M] Change in osmo-pcu[master]: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email ) Change subject: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle .. Patch Set 2: (1 comment) File src/gprs_rlcmac_sched.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/35096/comment/15e16302_097cbebb PS2, Line 492: skip_idle = skip_idle && trx != 0; > I think now that we don't have the #ifdef, we can really simplify this to: > […] I have created a follow up patch now, so we can discuss there how we simplify this: https://gerrit.osmocom.org/c/osmo-pcu/+/35151 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I7a08d8cc670fa14f7206dbc22351f3668a17 Gerrit-Change-Number: 35096 Gerrit-PatchSet: 2 Gerrit-Owner: dexter Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Comment-Date: Tue, 28 Nov 2023 14:44:21 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmo-pcu[master]: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle
Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email ) Change subject: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle .. Patch Set 2: (1 comment) File src/gprs_rlcmac_sched.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/35096/comment/bc33e489_dccdf6cf PS2, Line 492: skip_idle = skip_idle && trx != 0; I think now that we don't have the #ifdef, we can really simplify this to: bool skip_dummy; if (bts->gen_dummy_blk) { skip_dummy = false; } else { const unsigned num_tbfs = pdch->num_tbfs(GPRS_RLCMAC_DL_TBF) + pdch->num_tbfs(GPRS_RLCMAC_UL_TBF); skip_dummy = (num_tbfs == 0); } This can again be simplified to: bool skip_dummy = bts->gen_dummy_blk; if (skip_dummy) { /* Only skip if no one is listening on this PDCH: */ const unsigned num_tbfs = pdch->num_tbfs(GPRS_RLCMAC_DL_TBF) + pdch->num_tbfs(GPRS_RLCMAC_UL_TBF); skip_dummy = (num_tbfs == 0); } -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I7a08d8cc670fa14f7206dbc22351f3668a17 Gerrit-Change-Number: 35096 Gerrit-PatchSet: 2 Gerrit-Owner: dexter Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: dexter Gerrit-Comment-Date: Tue, 28 Nov 2023 13:38:50 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in osmo-pcu[master]: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email ) Change subject: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle .. gprs_rlcmac_sched: fix condition for generating dummy blocks on idle When a PDCH is idle, then the gaps are filled with dummy blocks. OsmoPCU supports generating the dummy blocks locally, so that a continous stream of PDCH blocks is sent to L1. However, some BTS models (the OsmoTRX based models in particular) are able to generate the idle blocks locally. In this case the PCU should leave the genration of the dummy blocks to the BTS in order to save processing time and load on the PCUIF interface. In gprs_rlcmac_sched we already have a flag to skip idle frames in case we do not use the so called "direct phy access". A similar mechanism also exists in pcu_l1_if.cpp in function pcu_rx_rts_req_ptcch(). Unfortunately this check is not implemented correctly. The flag gets set when the ENABLE_DIRECT_PHY define constant is set. However, this does not say anything about whether the BTS model supports the generation of idle blocks or not. The define constant is intended to be used to disable direct phy related code in on platforms where no direct phy code is used or cannot be used. We must instead check the BTS model (bts->bts_model) in order to decide whether this particular BTS type requires the generation of dummy blocks or not. Related: OS#6191 Change-Id: I7a08d8cc670fa14f7206dbc22351f3668a17 --- M src/bts.h M src/gprs_rlcmac_sched.cpp M src/pcu_l1_if.cpp 3 files changed, 63 insertions(+), 9 deletions(-) Approvals: daniel: Looks good to me, but someone else must approve Jenkins Builder: Verified fixeria: Looks good to me, approved diff --git a/src/bts.h b/src/bts.h index 1d88cb5..86c161b 100644 --- a/src/bts.h +++ b/src/bts.h @@ -278,6 +278,11 @@ /* BTS hardware model, see pcuif_proto.h */ uint8_t bts_model; + + /* When the PDCH is idle, the timeslot is expected to transmit dummy blocks. Some BTS models will generate +* those dummy blocks automatically when no data is transmitted. In contrast, other BTS models may require a +* continuous stream of PDCH blocks that already has the gaps filled with dummy blocks. */ + bool gen_idle_blocks; }; struct paging_req_cs { diff --git a/src/gprs_rlcmac_sched.cpp b/src/gprs_rlcmac_sched.cpp index 314fd58..12b9e50 100644 --- a/src/gprs_rlcmac_sched.cpp +++ b/src/gprs_rlcmac_sched.cpp @@ -487,11 +487,10 @@ const unsigned num_tbfs = pdch->num_tbfs(GPRS_RLCMAC_DL_TBF) + pdch->num_tbfs(GPRS_RLCMAC_UL_TBF); bool skip_idle = (num_tbfs == 0); -#ifdef ENABLE_DIRECT_PHY - /* In DIRECT_PHY mode we want to always submit something to L1 in -* TRX0, since BTS is not preparing dummy bursts on idle TS for us */ - skip_idle = skip_idle && trx != 0; -#endif + + if (bts->gen_idle_blocks) + skip_idle = skip_idle && trx != 0; + if (!skip_idle && (msg = sched_dummy())) { /* increase counter */ gsmtap_cat = PCU_GSMTAP_C_DL_DUMMY; diff --git a/src/pcu_l1_if.cpp b/src/pcu_l1_if.cpp index e391829..07082d3 100644 --- a/src/pcu_l1_if.cpp +++ b/src/pcu_l1_if.cpp @@ -551,11 +551,10 @@ const unsigned num_tbfs = pdch->num_tbfs(GPRS_RLCMAC_DL_TBF) + pdch->num_tbfs(GPRS_RLCMAC_UL_TBF); bool skip_idle = (num_tbfs == 0); -#ifdef ENABLE_DIRECT_PHY - /* In DIRECT_PHY mode we want to always submit something to L1 in -* TRX0, since BTS is not preparing dummy bursts on idle TS for us: */ + + if (bts->gen_idle_blocks) skip_idle = skip_idle && trx != 0; -#endif + if (skip_idle) { pcu_l1if_tx_ptcch(bts, trx, ts, bts->trx[trx].arfcn, fn, block_nr, NULL, 0); @@ -735,6 +734,27 @@ { 0, NULL } }; +static bool decide_gen_idle_blocks(struct gprs_rlcmac_bts *bts) +{ + switch (bts->bts_model) { + case PCU_IF_BTS_MODEL_UNSPEC: + case PCU_IF_BTS_MODEL_LC15: + case PCU_IF_BTS_MODEL_OC2G: + case PCU_IF_BTS_MODEL_OCTPHY: + case PCU_IF_BTS_MODEL_SYSMO: + case PCU_IF_BTS_MODEL_RBS: + /* The BTS models above do not generate dummy blocks by themselves, so OsmoPCU must fill the idle gaps in the +* stream of generated PDCH blocks with dummy blocks. */ + return true; + case PCU_IF_BTS_MODEL_TRX: + /* The BTS models above generate dummy blocks by themselves, so OsmoBTS will only generate PDCH bloks that +* actually contain data. On idle, no blocks are generated. */ + return false; + default: +
[M] Change in osmo-pcu[master]: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle
Attention is currently required from: dexter. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email ) Change subject: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle .. Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I7a08d8cc670fa14f7206dbc22351f3668a17 Gerrit-Change-Number: 35096 Gerrit-PatchSet: 2 Gerrit-Owner: dexter Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-CC: laforge Gerrit-Attention: dexter Gerrit-Comment-Date: Fri, 24 Nov 2023 20:36:47 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-pcu[master]: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle
Attention is currently required from: dexter, fixeria. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email ) Change subject: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I7a08d8cc670fa14f7206dbc22351f3668a17 Gerrit-Change-Number: 35096 Gerrit-PatchSet: 2 Gerrit-Owner: dexter Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-CC: laforge Gerrit-Attention: fixeria Gerrit-Attention: dexter Gerrit-Comment-Date: Fri, 24 Nov 2023 13:28:14 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-pcu[master]: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle
Attention is currently required from: daniel, fixeria. Hello Jenkins Builder, daniel, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by daniel, Code-Review+1 by fixeria, Verified-1 by Jenkins Builder Change subject: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle .. gprs_rlcmac_sched: fix condition for generating dummy blocks on idle When a PDCH is idle, then the gaps are filled with dummy blocks. OsmoPCU supports generating the dummy blocks locally, so that a continous stream of PDCH blocks is sent to L1. However, some BTS models (the OsmoTRX based models in particular) are able to generate the idle blocks locally. In this case the PCU should leave the genration of the dummy blocks to the BTS in order to save processing time and load on the PCUIF interface. In gprs_rlcmac_sched we already have a flag to skip idle frames in case we do not use the so called "direct phy access". A similar mechanism also exists in pcu_l1_if.cpp in function pcu_rx_rts_req_ptcch(). Unfortunately this check is not implemented correctly. The flag gets set when the ENABLE_DIRECT_PHY define constant is set. However, this does not say anything about whether the BTS model supports the generation of idle blocks or not. The define constant is intended to be used to disable direct phy related code in on platforms where no direct phy code is used or cannot be used. We must instead check the BTS model (bts->bts_model) in order to decide whether this particular BTS type requires the generation of dummy blocks or not. Related: OS#6191 Change-Id: I7a08d8cc670fa14f7206dbc22351f3668a17 --- M src/bts.h M src/gprs_rlcmac_sched.cpp M src/pcu_l1_if.cpp 3 files changed, 63 insertions(+), 9 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/96/35096/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I7a08d8cc670fa14f7206dbc22351f3668a17 Gerrit-Change-Number: 35096 Gerrit-PatchSet: 2 Gerrit-Owner: dexter Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-CC: laforge Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[M] Change in osmo-pcu[master]: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle
Attention is currently required from: daniel, fixeria. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email ) Change subject: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle .. Patch Set 2: (1 comment) File src/bts.h: https://gerrit.osmocom.org/c/osmo-pcu/+/35096/comment/3a6d1146_ed5d3e24 PS1, Line 282: idele > idle Done -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I7a08d8cc670fa14f7206dbc22351f3668a17 Gerrit-Change-Number: 35096 Gerrit-PatchSet: 2 Gerrit-Owner: dexter Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-CC: laforge Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Fri, 24 Nov 2023 13:19:34 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[M] Change in osmo-pcu[master]: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle
Attention is currently required from: dexter. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email ) Change subject: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I7a08d8cc670fa14f7206dbc22351f3668a17 Gerrit-Change-Number: 35096 Gerrit-PatchSet: 1 Gerrit-Owner: dexter Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-CC: laforge Gerrit-Attention: dexter Gerrit-Comment-Date: Fri, 24 Nov 2023 10:17:56 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-pcu[master]: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle
Attention is currently required from: dexter. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email ) Change subject: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle .. Patch Set 1: Code-Review+1 (1 comment) File src/bts.h: https://gerrit.osmocom.org/c/osmo-pcu/+/35096/comment/b6be7124_a8a58beb PS1, Line 282: idele idle -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I7a08d8cc670fa14f7206dbc22351f3668a17 Gerrit-Change-Number: 35096 Gerrit-PatchSet: 1 Gerrit-Owner: dexter Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-CC: laforge Gerrit-Attention: dexter Gerrit-Comment-Date: Fri, 24 Nov 2023 07:29:43 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-pcu[master]: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email ) Change subject: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle .. Patch Set 1: (1 comment) File src/bts.h: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-12577): https://gerrit.osmocom.org/c/osmo-pcu/+/35096/comment/a5197d21_76a7346e PS1, Line 284: * continous stream of PDCH blocks that already has the gaps filled with dummy blocks. */ 'continous' may be misspelled - perhaps 'continuous'? -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I7a08d8cc670fa14f7206dbc22351f3668a17 Gerrit-Change-Number: 35096 Gerrit-PatchSet: 1 Gerrit-Owner: dexter Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Wed, 22 Nov 2023 16:29:57 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in osmo-pcu[master]: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle
dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35096?usp=email ) Change subject: gprs_rlcmac_sched: fix condition for generating dummy blocks on idle .. gprs_rlcmac_sched: fix condition for generating dummy blocks on idle When a PDCH is idle, then the gaps are filled with dummy blocks. OsmoPCU supports generating the dummy blocks locally, so that a continous stream of PDCH blocks is sent to L1. However, some BTS models (the OsmoTRX based models in particular) are able to generate the idle blocks locally. In this case the PCU should leave the genration of the dummy blocks to the BTS in order to save processing time and load on the PCUIF interface. In gprs_rlcmac_sched we already have a flag to skip idle frames in case we do not use the so called "direct phy access". A similar mechanism also exists in pcu_l1_if.cpp in function pcu_rx_rts_req_ptcch(). Unfortunately this check is not implemented correctly. The flag gets set when the ENABLE_DIRECT_PHY define constant is set. However, this does not say anything about whether the BTS model supports the generation of idle blocks or not. The define constant is intended to be used to disable direct phy related code in on platforms where no direct phy code is used or cannot be used. We must instead check the BTS model (bts->bts_model) in order to decide whether this particular BTS type requires the generation of dummy blocks or not. Related: OS#6191 Change-Id: I7a08d8cc670fa14f7206dbc22351f3668a17 --- M src/bts.h M src/gprs_rlcmac_sched.cpp M src/pcu_l1_if.cpp 3 files changed, 63 insertions(+), 9 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/96/35096/1 diff --git a/src/bts.h b/src/bts.h index 1d88cb5..6958881 100644 --- a/src/bts.h +++ b/src/bts.h @@ -278,6 +278,11 @@ /* BTS hardware model, see pcuif_proto.h */ uint8_t bts_model; + + /* When the PDCH is idele, the timeslot is expected to transmit dummy blocks. Some BTS models will generate +* those dummy blocks automatically when no data is transmitted. In contrast, other BTS models may require a +* continous stream of PDCH blocks that already has the gaps filled with dummy blocks. */ + bool gen_idle_blocks; }; struct paging_req_cs { diff --git a/src/gprs_rlcmac_sched.cpp b/src/gprs_rlcmac_sched.cpp index 314fd58..12b9e50 100644 --- a/src/gprs_rlcmac_sched.cpp +++ b/src/gprs_rlcmac_sched.cpp @@ -487,11 +487,10 @@ const unsigned num_tbfs = pdch->num_tbfs(GPRS_RLCMAC_DL_TBF) + pdch->num_tbfs(GPRS_RLCMAC_UL_TBF); bool skip_idle = (num_tbfs == 0); -#ifdef ENABLE_DIRECT_PHY - /* In DIRECT_PHY mode we want to always submit something to L1 in -* TRX0, since BTS is not preparing dummy bursts on idle TS for us */ - skip_idle = skip_idle && trx != 0; -#endif + + if (bts->gen_idle_blocks) + skip_idle = skip_idle && trx != 0; + if (!skip_idle && (msg = sched_dummy())) { /* increase counter */ gsmtap_cat = PCU_GSMTAP_C_DL_DUMMY; diff --git a/src/pcu_l1_if.cpp b/src/pcu_l1_if.cpp index e391829..07082d3 100644 --- a/src/pcu_l1_if.cpp +++ b/src/pcu_l1_if.cpp @@ -551,11 +551,10 @@ const unsigned num_tbfs = pdch->num_tbfs(GPRS_RLCMAC_DL_TBF) + pdch->num_tbfs(GPRS_RLCMAC_UL_TBF); bool skip_idle = (num_tbfs == 0); -#ifdef ENABLE_DIRECT_PHY - /* In DIRECT_PHY mode we want to always submit something to L1 in -* TRX0, since BTS is not preparing dummy bursts on idle TS for us: */ + + if (bts->gen_idle_blocks) skip_idle = skip_idle && trx != 0; -#endif + if (skip_idle) { pcu_l1if_tx_ptcch(bts, trx, ts, bts->trx[trx].arfcn, fn, block_nr, NULL, 0); @@ -735,6 +734,27 @@ { 0, NULL } }; +static bool decide_gen_idle_blocks(struct gprs_rlcmac_bts *bts) +{ + switch (bts->bts_model) { + case PCU_IF_BTS_MODEL_UNSPEC: + case PCU_IF_BTS_MODEL_LC15: + case PCU_IF_BTS_MODEL_OC2G: + case PCU_IF_BTS_MODEL_OCTPHY: + case PCU_IF_BTS_MODEL_SYSMO: + case PCU_IF_BTS_MODEL_RBS: + /* The BTS models above do not generate dummy blocks by themselves, so OsmoPCU must fill the idle gaps in the +* stream of generated PDCH blocks with dummy blocks. */ + return true; + case PCU_IF_BTS_MODEL_TRX: + /* The BTS models above generate dummy blocks by themselves, so OsmoBTS will only generate PDCH bloks that +* actually contain data. On idle, no blocks are generated. */ + return false; + default: + return false; + } +} + static int