Change in osmo-pcu[master]: Handle Final UL ACK/NACK Confirmation in tbf_fsm

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
Wed Oct 13 01:32:46 UTC 2021


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

Change subject: Handle Final UL ACK/NACK Confirmation in tbf_fsm
......................................................................

Handle Final UL ACK/NACK Confirmation in tbf_fsm

Pass the event over the FSM for better understading of the entire
lifecycle of the TBF.

Change-Id: If30d881037209d33b2b41ecf8bb8419caf36e367
---
M src/pdch.cpp
M src/tbf_fsm.c
M src/tbf_fsm.h
3 files changed, 23 insertions(+), 11 deletions(-)

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



diff --git a/src/pdch.cpp b/src/pdch.cpp
index a503a55..dbdd47f 100644
--- a/src/pdch.cpp
+++ b/src/pdch.cpp
@@ -355,9 +355,9 @@
 		}
 		pdch_ulc_release_fn(ulc, fn);
 		osmo_fsm_inst_dispatch(ul_tbf->ul_ack_fsm.fi, TBF_UL_ACK_EV_RX_CTRL_ACK, NULL);
-		/* We can free since we only set polling on final UL ACK/NACK */
+		/* We only set polling on final UL ACK/NACK */
 		LOGPTBF(tbf, LOGL_DEBUG, "[UPLINK] END\n");
-		tbf_free(tbf);
+		osmo_fsm_inst_dispatch(ul_tbf->state_fsm.fi, TBF_EV_FINAL_UL_ACK_CONFIRMED, NULL);
 		return;
 
 	case PDCH_ULC_POLL_UL_ASS:
@@ -616,6 +616,7 @@
 void gprs_rlcmac_pdch::rcv_resource_request(Packet_Resource_Request_t *request, uint32_t fn, struct pcu_l1_meas *meas)
 {
 	struct gprs_rlcmac_sba *sba;
+	int rc;
 
 	if (request->ID.UnionType) {
 		struct gprs_rlcmac_ul_tbf *ul_tbf = NULL;
@@ -682,12 +683,23 @@
 				"block of final UL ACK/NACK\n", fn);
 			ul_tbf->n_reset(N3103);
 			pdch_ulc_release_node(ulc, item);
+			rc = osmo_fsm_inst_dispatch(ul_tbf->state_fsm.fi, TBF_EV_FINAL_UL_ACK_CONFIRMED, NULL);
+			if (rc) {
+				/* FSM failed handling, get rid of previous finished UL TBF before providing a new one */
+				LOGPTBFUL(ul_tbf, LOGL_NOTICE,
+					  "Got PACKET RESOURCE REQ while TBF not finished, killing pending UL TBF\n");
+				tbf_free(ul_tbf);
+			} /* else: ul_tbf has been freed by state_fsm */
+			ul_tbf = NULL;
 			break;
 		default:
 			OSMO_ASSERT(0);
 		}
 
-		/* here ul_tbf may be NULL in SBA case (no previous TBF) */
+		/* Here ul_tbf is NULL:
+		 * - SBA case: no previous TBF) and in
+		 * - POLL case: PktResReq is a final ACk confirmation and ul_tbf was freed
+		 */
 
 		if (request->Exist_MS_Radio_Access_capability2) {
 			uint8_t ms_class, egprs_ms_class;
@@ -699,14 +711,6 @@
 				ms_set_egprs_ms_class(ms, egprs_ms_class);
 		}
 
-		/* Get rid of previous finished UL TBF before providing a new one */
-		if (ul_tbf) {
-			if (!ul_tbf->state_is(TBF_ST_FINISHED))
-				LOGPTBFUL(ul_tbf, LOGL_NOTICE,
-					  "Got PACKET RESOURCE REQ while TBF not finished, killing pending UL TBF\n");
-			tbf_free(ul_tbf);
-		}
-
 		ul_tbf = tbf_alloc_ul_pacch(bts(), ms, trx_no(), tlli);
 		if (!ul_tbf) {
 			handle_tbf_reject(bts(), ms, trx_no(), ts_no);
diff --git a/src/tbf_fsm.c b/src/tbf_fsm.c
index 4202a0c..0b6a2a3 100644
--- a/src/tbf_fsm.c
+++ b/src/tbf_fsm.c
@@ -50,6 +50,7 @@
 	{ TBF_EV_LAST_DL_DATA_SENT, "LAST_DL_DATA_SENT" },
 	{ TBF_EV_LAST_UL_DATA_RECVD, "LAST_UL_DATA_RECVD" },
 	{ TBF_EV_FINAL_ACK_RECVD, "FINAL_ACK_RECVD" },
+	{ TBF_EV_FINAL_UL_ACK_CONFIRMED, "FINAL_UL_ACK_CONFIRMED" },
 	{ TBF_EV_MAX_N3101 , "MAX_N3101" },
 	{ TBF_EV_MAX_N3103 , "MAX_N3103" },
 	{ TBF_EV_MAX_N3105 , "MAX_N3105" },
@@ -253,6 +254,11 @@
 		   case we receive more DL data to tx */
 		tbf_fsm_state_chg(fi, TBF_ST_WAIT_RELEASE);
 		break;
+	case TBF_EV_FINAL_UL_ACK_CONFIRMED:
+		/* UL TBF ACKed our transmitted UL ACK/NACK with final Ack
+		 * Indicator set to '1' t. We can free the TBF right away. */
+		tbf_free(ctx->tbf);
+		break;
 	case TBF_EV_MAX_N3103:
 		ctx->T_release = 3169;
 		tbf_fsm_state_chg(fi, TBF_ST_RELEASING);
@@ -432,6 +438,7 @@
 		.in_event_mask =
 			X(TBF_EV_DL_ACKNACK_MISS) |
 			X(TBF_EV_FINAL_ACK_RECVD) |
+			X(TBF_EV_FINAL_UL_ACK_CONFIRMED) |
 			X(TBF_EV_MAX_N3103) |
 			X(TBF_EV_MAX_N3105),
 		.out_state_mask =
diff --git a/src/tbf_fsm.h b/src/tbf_fsm.h
index d6fe41f..d62e091 100644
--- a/src/tbf_fsm.h
+++ b/src/tbf_fsm.h
@@ -36,6 +36,7 @@
 	TBF_EV_LAST_DL_DATA_SENT, /* DL TBF sends RLCMAC block containing last DL avilable data buffered */
 	TBF_EV_LAST_UL_DATA_RECVD, /* UL TBF sends RLCMAC block containing last UL data (cv=0) */
 	TBF_EV_FINAL_ACK_RECVD, /* DL ACK/NACK with FINAL_ACK=1 received from MS */
+	TBF_EV_FINAL_UL_ACK_CONFIRMED, /* UL TBF: MS ACKs (CtrlAck or PktResReq) our UL ACK/NACK w/ FinalAckInd=1 */
 	TBF_EV_MAX_N3101, /* MAX N3101 (max usf timeout) reached (UL TBF) */
 	TBF_EV_MAX_N3103, /* MAX N3103 (max Pkt Ctrl Ack for last UL ACK/NACK timeout) reached (UL TBF) */
 	TBF_EV_MAX_N3105, /* MAX N3105 (max poll timeout) reached (UL/DL TBF) */

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

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: If30d881037209d33b2b41ecf8bb8419caf36e367
Gerrit-Change-Number: 25749
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
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/20211013/a7ee35b7/attachment.htm>


More information about the gerrit-log mailing list