Change in osmo-pcu[master]: tbf_dl: Clarify requirements for DL ACK/NACK

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

pespin gerrit-no-reply at lists.osmocom.org
Mon Aug 23 16:28:30 UTC 2021


pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/25105 )

Change subject: tbf_dl: Clarify requirements for DL ACK/NACK
......................................................................

tbf_dl: Clarify requirements for DL ACK/NACK

Method is renamed since it clearly relates to getting DL ACK/NACK, no
CTRL ACK.

use same methods in both scheduler and internal use since they are
expectd to be run in the same code path by the scheduler. This way we
make sure the same conditions apply and it's clearer when looking at
the code.

Change-Id: Ib0e9b9547f5292b95064bab2dc182fdf659f0518
---
M src/gprs_rlcmac_sched.cpp
M src/tbf_dl.cpp
M src/tbf_dl.h
3 files changed, 11 insertions(+), 12 deletions(-)

Approvals:
  Jenkins Builder: Verified
  pespin: Looks good to me, approved
  osmith: Looks good to me, but someone else must approve



diff --git a/src/gprs_rlcmac_sched.cpp b/src/gprs_rlcmac_sched.cpp
index d73e55d..ed81981 100644
--- a/src/gprs_rlcmac_sched.cpp
+++ b/src/gprs_rlcmac_sched.cpp
@@ -239,7 +239,7 @@
 	int age_thresh1 = msecs_to_frames(200);
 	int age_thresh2 = msecs_to_frames(OSMO_MIN(msecs_t3190/2, dl_tbf_idle_msec));
 
-	if (tbf->is_control_ts(ts) && tbf->need_control_ts())
+	if (tbf->is_control_ts(ts) && tbf->need_poll_for_dl_ack_nack())
 		return DL_PRIO_CONTROL;
 
 	if (tbf->is_control_ts(ts) && age > age_thresh2 && age_thresh2 > 0)
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index bed7a08..34c5630 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -779,7 +779,6 @@
 	uint8_t *msg_data;
 	struct msgb *dl_msg;
 	unsigned msg_len;
-	bool need_poll;
 	/* TODO: support MCS-7 - MCS-9, where data_block_idx can be 1 */
 	uint8_t data_block_idx = 0;
 	unsigned int rrbp;
@@ -940,23 +939,20 @@
 	if (m_last_dl_poll_fn < 0)
 		m_last_dl_poll_fn = fn;
 
-	need_poll = state_fsm.state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK);
-
-	/* poll after POLL_ACK_AFTER_FRAMES frames, or when final block is tx.
-	 */
-	if (m_tx_counter >= POLL_ACK_AFTER_FRAMES || m_dl_ack_requested ||
-			need_poll) {
+	/* poll after POLL_ACK_AFTER_FRAMES frames, or when final block is tx or
+	 * when last polled DL ACK/NACK was lost. */
+	if (need_poll_for_dl_ack_nack()) {
 		if (m_dl_ack_requested) {
 			LOGPTBFDL(this, LOGL_DEBUG,
 				  "Scheduling Ack/Nack polling, because it was requested explicitly "
 				  "(e.g. first final block sent).\n");
-		} else if (need_poll) {
+		} else if (state_fsm.state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK)) {
 			LOGPTBFDL(this, LOGL_DEBUG,
 				  "Scheduling Ack/Nack polling, because polling timed out.\n");
 		} else {
 			LOGPTBFDL(this, LOGL_DEBUG,
 				  "Scheduling Ack/Nack polling, because %d blocks sent.\n",
-				POLL_ACK_AFTER_FRAMES);
+				  POLL_ACK_AFTER_FRAMES);
 		}
 
 		rc = check_polling(fn, ts, &new_poll_fn, &rrbp);
@@ -1211,8 +1207,11 @@
 	m_dl_ack_requested = true;
 }
 
-bool gprs_rlcmac_dl_tbf::need_control_ts() const
+/* Does this DL TBF require to poll the MS for DL ACK/NACK? */
+bool gprs_rlcmac_dl_tbf::need_poll_for_dl_ack_nack() const
 {
+	/* poll after POLL_ACK_AFTER_FRAMES frames, or when final block is tx or
+	 * when last polled DL ACK/NACK was lost. */
 	return state_fsm.state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK) ||
 		m_tx_counter >= POLL_ACK_AFTER_FRAMES ||
 		m_dl_ack_requested;
diff --git a/src/tbf_dl.h b/src/tbf_dl.h
index ad1469a..27b6a2c 100644
--- a/src/tbf_dl.h
+++ b/src/tbf_dl.h
@@ -53,7 +53,7 @@
 	void trigger_ass(struct gprs_rlcmac_tbf *old_tbf);
 
 	void request_dl_ack();
-	bool need_control_ts() const;
+	bool need_poll_for_dl_ack_nack() const;
 	bool have_data() const;
 	int frames_since_last_poll(unsigned fn) const;
 	int frames_since_last_drain(unsigned fn) const;

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/25105
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: Ib0e9b9547f5292b95064bab2dc182fdf659f0518
Gerrit-Change-Number: 25105
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210823/b9cfc05a/attachment.htm>


More information about the gerrit-log mailing list