<p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/25535">View Change</a></p><p>2 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/25535/1/src/gprs_rlcmac_sched.cpp">File src/gprs_rlcmac_sched.cpp:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/25535/1/src/gprs_rlcmac_sched.cpp@520">Patch Set #1, Line 520:</a> <code style="font-family:monospace,monospace">        if (tx_is_egprs && pdch->has_gprs_only_tbf_attached()) {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Is it safe to skip this block?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">It is safe in practice I'd say. Ideally indeed one would set fn_without_cs14 to 0 as soon as there's no TBF to transmit, since anyway that means there's no GPRS TBF which needs receiving synchronization. With this patch, we leave the value set to last value since we don't pass over here. However that's not a big issue, since for this counter to be incremented there's need for 2 TBFs to be active: 1 EGPRS and 1 GPRS.<br>Hence, at some point before that scenario we'll go through here with 1 EGPRS TBF *OR* 1 GPRS TBF, which will end in "tx_is_egprs && pdch->has_gprs_only_tbf_attached()" being false, hence the counter is properly initialized beofre really being of need.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I agree though that it's more clear to set it always, so I'll move it after the "tx_pdtch" goto tag.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/25535/1/src/gprs_rlcmac_sched.cpp@535">Patch Set #1, Line 535:</a> <code style="font-family:monospace,monospace">gprs_bssgp_update_frames_sent</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Is it safe to skip this call?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I would say it's not really an issue since there's no TBFs anyway. In any case the leak rate counting is quite broken afaict. I'll move it after the tag too anyway.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-pcu/+/25535">change 25535</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-pcu/+/25535"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-pcu </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ife718eeed2af011479c03099ea109518f04567bc </div>
<div style="display:none"> Gerrit-Change-Number: 25535 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 27 Sep 2021 08:58:06 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>