Attention is currently required from: fixeria, pespin.
Hello Jenkins Builder, fixeria, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-pcu/+/35151?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: gprs_rlcmac_sched: rewrite logic around idle block skip
......................................................................
gprs_rlcmac_sched: rewrite logic around idle block skip
The logic that decides whether to generate idle / dummy blocks directly
at the PCU or leave the work to the TRX implementation of that specific
BTS has a confusing appeareance.
(This is a follow up patch to change
I7a08d8cc670fa14f7206ffffdbc22351f3668a17)
Related: OS#6191
Change-Id: Iadb62748b18605bf158169b317f880352bc0a5a6
---
M src/gprs_rlcmac_sched.cpp
1 file changed, 31 insertions(+), 5 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/51/35151/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35151?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: Iadb62748b18605bf158169b317f880352bc0a5a6
Gerrit-Change-Number: 35151
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: neels, osmith.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35097?usp=email )
Change subject: vty: disallow combination of early-IA and frequency hopping
......................................................................
Patch Set 2:
(1 comment)
File src/osmo-bsc/bts_trx_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/35097/comment/7cfd114b_94b51cc7
PS2, Line 352: ts->hopping.enabled = enabled;
> I would have expected the error + return before actually enabling it. […]
Ack
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35097?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I8d375e5155be7b53034d5c0be5566d2f33af5db0
Gerrit-Change-Number: 35097
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: osmith <osmith(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Nov 2023 15:22:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: dexter, fixeria.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35151?usp=email )
Change subject: gprs_rlcmac_sched: rewrite logic around idle block skip
......................................................................
Patch Set 1:
(2 comments)
Patchset:
PS1:
btw, I think this code change can also be applied to the other place where you removed the #ifdef in the previous commit.
File src/gprs_rlcmac_sched.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/35151/comment/c2b472ef_35de08e1
PS1, Line 486: * way we help BTS energy saving (on TRX!=C0) by sending nothing
> I don't get why this is energy saving. The dummy block is sent in any case. […]
IIRC osmo-bts-trx does something slightly different if it detects no block is to be sent because no one is listening, like decreasing the Tx power during that time. If MS are listening on that PDCH, then we still need to transmit at regular Tx power. @vyanitskiy@sysmocom.de probably remembers better.
In any case, I'd rather leave the comment where it was, immedatelly after the parent "else" block, since it refers to the whole case where an RLCMAC Dummy Block would be sent.
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35151?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: Iadb62748b18605bf158169b317f880352bc0a5a6
Gerrit-Change-Number: 35151
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Nov 2023 15:06:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: comment
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: I7a08d8cc670fa14f7206ffffdbc22351f3668a17
Gerrit-Change-Number: 35096
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Nov 2023 14:44:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35151?usp=email )
Change subject: gprs_rlcmac_sched: rewrite logic around idle block skip
......................................................................
Patch Set 1:
(1 comment)
File src/gprs_rlcmac_sched.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/35151/comment/724b3af7_7e2b6a19
PS1, Line 486: * way we help BTS energy saving (on TRX!=C0) by sending nothing
I don't get why this is energy saving. The dummy block is sent in any case. A dummy block should consume as much energy as a block that has actual data in it.
So all we do here is looking up if there are TBF. If there are none => msg = NULL => TRX will create and send a dummy frame by itself)
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35151?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: Iadb62748b18605bf158169b317f880352bc0a5a6
Gerrit-Change-Number: 35151
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: Jenkins Builder
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Nov 2023 14:43:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35151?usp=email )
Change subject: gprs_rlcmac_sched: rewrite logic around idle block skip
......................................................................
gprs_rlcmac_sched: rewrite logic around idle block skip
The logic that decides whether to generate idle / dummy blocks directly
at the PCU or leave the work to the TRX implementation of that specific
BTS has a confusing appeareance.
(This is a follow up patch to change
I7a08d8cc670fa14f7206ffffdbc22351f3668a17)
Related: OS#6191
Change-Id: Iadb62748b18605bf158169b317f880352bc0a5a6
---
M src/gprs_rlcmac_sched.cpp
1 file changed, 36 insertions(+), 15 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/51/35151/1
diff --git a/src/gprs_rlcmac_sched.cpp b/src/gprs_rlcmac_sched.cpp
index 12b9e50..a56fcb3 100644
--- a/src/gprs_rlcmac_sched.cpp
+++ b/src/gprs_rlcmac_sched.cpp
@@ -474,22 +474,26 @@
}
/* Prio 3: send dummy control message if need to poll or USF */
else {
- /* If there's no TBF attached to this PDCH, we can submit an empty
- * data_req since there's nothing to transmit nor to poll/USF. This
- * way we help BTS energy saving (on TRX!=C0) by sending nothing
- * instead of a dummy block. The early return is done here and
- * not at the start of the function because the condition below
- * (num_tbfs==0) may not be enough, because temporary dummy TBFs
- * created to send Imm Ass Rej (see handle_tbf_reject()) don't
- * have a TFI assigned and hence are not attached to the PDCH
- * TS, so they don't show up in the count below.
- */
- const unsigned num_tbfs = pdch->num_tbfs(GPRS_RLCMAC_DL_TBF)
- + pdch->num_tbfs(GPRS_RLCMAC_UL_TBF);
- bool skip_idle = (num_tbfs == 0);
+ bool skip_idle;
- if (bts->gen_idle_blocks)
- skip_idle = skip_idle && trx != 0;
+ if (bts->gen_idle_blocks) {
+ /* Always generate all idle blocks since the TRX of this BTS does
+ * not generate idle blocks by itself. */
+ skip_idle = false;
+ } else {/* Only skip if no one is listening on this PDCH:
+ * If there's no TBF attached to this PDCH, we can submit an empty
+ * data_req since there's nothing to transmit nor to poll/USF. This
+ * way we help BTS energy saving (on TRX!=C0) by sending nothing
+ * instead of a dummy block. The early return is done here and
+ * not at the start of the function because the condition below
+ * (num_tbfs==0) may not be enough, because temporary dummy TBFs
+ * created to send Imm Ass Rej (see handle_tbf_reject()) don't
+ * have a TFI assigned and hence are not attached to the PDCH
+ * TS, so they don't show up in the count below. */
+ const unsigned num_tbfs = pdch->num_tbfs(GPRS_RLCMAC_DL_TBF)
+ + pdch->num_tbfs(GPRS_RLCMAC_UL_TBF);
+ skip_idle = (num_tbfs == 0);
+ }
if (!skip_idle && (msg = sched_dummy())) {
/* increase counter */
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35151?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: Iadb62748b18605bf158169b317f880352bc0a5a6
Gerrit-Change-Number: 35151
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: newchange
Attention is currently required from: daniel.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35078?usp=email )
Change subject: osmo_io: Factor out and use common send function from backend
......................................................................
Patch Set 1:
(1 comment)
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/35078/comment/e07d3106_06f58829
PS1, Line 360:
rc == 0 case seems to be missing here?
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35078?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I6da2653d32aedd0e7872be0cf90a841b56462e59
Gerrit-Change-Number: 35078
Gerrit-PatchSet: 1
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Nov 2023 13:41:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
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: I7a08d8cc670fa14f7206ffffdbc22351f3668a17
Gerrit-Change-Number: 35096
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Nov 2023 13:38:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment