laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/28272 )
Change subject: iuup: Check for IuUP Initialization retrans
......................................................................
iuup: Check for IuUP Initialization retrans
Since libosmocore.git Change-Id
I5cb740702805693cc7f0a550e2e093f9bfdd507c, the IuUP stack can send INIT
event more than once, it sends one each time an IuUP Initialization
message is received.
This is done since potentially a peer could send an Initialization
message at any time with a different subflow size configuration. So
ideally we should update all osmo-mgw state regarding codecs, and
forward the Init starting the procedure on the other conn of the
endpoint.
However, this scenario is most probably not going to happen right now
and it would be a lot of work to implement and test,
and subsequent INITs we received will almost surely come from
retransmissions of the initial Initialization message, which means
content will not really change.
Hence, it makes sense to simply drop the receive message (the IuUP stack
already takes care of re-ACKing it) and let the endpoint state continue
with its ongoing procedures.
Related: SYS#4705
Change-Id: Ib97bc6f57d265622e24a776b96f0a82c25d33d39
---
M src/libosmo-mgcp/mgcp_iuup.c
1 file changed, 11 insertions(+), 2 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/src/libosmo-mgcp/mgcp_iuup.c b/src/libosmo-mgcp/mgcp_iuup.c
index 120b330..142b002 100644
--- a/src/libosmo-mgcp/mgcp_iuup.c
+++ b/src/libosmo-mgcp/mgcp_iuup.c
@@ -365,13 +365,22 @@
int rc = 0;
struct msgb *msg;
- /* Find RFCI containing NO_DATA: */
- conn_rtp_src->iuup.rfci_id_no_data = _find_rfci_no_data(irp);
+ if (conn_rtp_src->iuup.init_ind) {
+ /* We received more than one IuUP Initialization. It's probably
+ * a retransmission, so simply ignore it (lower layers take care
+ * of ACKing it). */
+ LOGPCONN(conn_rtp_src->conn, DRTP, LOGL_INFO,
+ "Ignoring potential IuUP Initialization retrans\n");
+ return 0;
+ }
msg = msgb_copy_c(conn_rtp_src->conn, irp->oph.msg, "iuup-init-copy");
conn_rtp_src->iuup.init_ind = (struct osmo_iuup_rnl_prim *)msgb_data(msg);
conn_rtp_src->iuup.init_ind->oph.msg = msg;
+ /* Find RFCI containing NO_DATA: */
+ conn_rtp_src->iuup.rfci_id_no_data = _find_rfci_no_data(irp);
+
conn_dst = _find_dst_conn(conn_rtp_src->conn);
/* If not yet there, peer will potentially be IuUP-Initialized later
* when we attempt to bridge audio towards it. See bridge_iuup_to_iuup_peer() */
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/28272
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ib97bc6f57d265622e24a776b96f0a82c25d33d39
Gerrit-Change-Number: 28272
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: merged
laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/28271 )
Change subject: iuup: Fix Handling of subsequent Initialization msgs
......................................................................
iuup: Fix Handling of subsequent Initialization msgs
Once the IuUP FSM moved away from Init state, it stopped handling
Initialization messages received from peers and simply ignored them
starting from that point. As a result, if the first IuUP Init ACK it
sent to the peer was lost, the peer would keep retrying with more IuUP
Init and getting no answer.
In any case, it seems possible and desirable that a peer may send an
IuUP Init at a later point, as pointed out vaguely in 3GPP TS 25.415.
sec 6.5.2.1:
"""
Upon reception of a frame indicating that an Initialisation procedure is
active in the peer Iu UP entity, the Iu UP protocol layer forwards the whole
protocol information contained in the INITIALISATION control frame to the
upper layers. It also stores the RAB sub-Flow Combination set (and thus
replaces a possible previous set) in order to control during the transfer of
user data, that the Iu UP payload is correctly formatted (e.g. RFCI matches
the expected Iu UP frame payload total length). The peer Iu UP entity
receiving the INITIALISATION control frame shall choose a version that it
supports among the proposed versions indicated by the sender for which it
has enough initialisation information.
"""
sec B.2.2 "Initialisation State":
"""
After sending a positive acknowledgement of the last INITIALISATION control
frame, the Iu UP instance enters SMpSDU data transfer ready state. Note that
CN does not know if the initialisation ACK was correctly received by the RNC
(and Initialisation procedure successfully completed) until it receives RAB
assignment response, or use data from the RNC. The CN must therefore be able
to continue receiving INITIALISATION control frames by re-entering the
Initialisation state (from Support Mode Data Transfer Ready State), if the CN
has started to send user data before receiving the indication that
Initialisation was successfully completed
"""
sec B.2.3 "Support Mode Data Transfer Ready State":
"""
In case of handover or relocation, Initialisation procedures may have to be
performed and Iu UP instance may have to enter the initialisation state.
"""
Related: SYS#5995
Change-Id: I5cb740702805693cc7f0a550e2e093f9bfdd507c
---
M src/gsm/iuup.c
M tests/iuup/iuup_test.c
M tests/iuup/iuup_test.ok
3 files changed, 19 insertions(+), 13 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/src/gsm/iuup.c b/src/gsm/iuup.c
index 4c0a948..ca7eb2b 100644
--- a/src/gsm/iuup.c
+++ b/src/gsm/iuup.c
@@ -734,6 +734,7 @@
}
}
+/* 3GPP TS 25.415 B.2.3 "Support Mode Data Transfer Ready State" */
static void iuup_fsm_smpsdu_data(struct osmo_fsm_inst *fi, uint32_t event, void *data)
{
struct osmo_iuup_instance *iui = fi->priv;
@@ -745,6 +746,14 @@
irp = data;
osmo_fsm_inst_state_chg(fi, IUUP_FSM_ST_NULL, 0, 0);
break;
+ case IUUP_FSM_EVT_INIT:
+ /* "In case of handover or relocation, Initialisation procedures
+ * may have to be performed and Iu UP instance may have to enter
+ * the initialisation state." */
+ itp = data;
+ if (!iuup_rx_initialization(iui, itp))
+ osmo_fsm_inst_state_chg(fi, IUUP_FSM_ST_INIT, 0, 0);
+ break;
case IUUP_FSM_EVT_IUUP_DATA_REQ:
/* Data coming down from RNL (user) towards TNL (transport) */
irp = data;
@@ -821,6 +830,7 @@
},
[IUUP_FSM_ST_SMpSDU_DATA_XFER_READY] = {
.in_event_mask = S(IUUP_FSM_EVT_IUUP_CONFIG_REQ) |
+ S(IUUP_FSM_EVT_INIT) |
S(IUUP_FSM_EVT_IUUP_DATA_REQ) |
S(IUUP_FSM_EVT_IUUP_DATA_IND),
.out_state_mask = S(IUUP_FSM_ST_NULL) |
diff --git a/tests/iuup/iuup_test.c b/tests/iuup/iuup_test.c
index 5b55350..e40b9e7 100644
--- a/tests/iuup/iuup_test.c
+++ b/tests/iuup/iuup_test.c
@@ -541,12 +541,10 @@
switch (_passive_init_retrans_user_rx_prim) {
case 0:
- /* FIXME expected case: case 1: */
+ case 1:
OSMO_ASSERT(OSMO_PRIM_HDR(&irp->oph) == OSMO_PRIM(OSMO_IUUP_RNL_STATUS, PRIM_OP_INDICATION));
OSMO_ASSERT(irp->u.status.procedure == IUUP_PROC_INIT);
break;
- /* FIXME current case: */
- case 1:
case 2:
default:
OSMO_ASSERT(OSMO_PRIM_HDR(&irp->oph) == OSMO_PRIM(OSMO_IUUP_RNL_DATA, PRIM_OP_INDICATION));
@@ -603,14 +601,9 @@
tnp->oph.msg->l2h = msgb_put(tnp->oph.msg, sizeof(iuup_initialization));
hdr14 = (struct iuup_pdutype14_hdr *)msgb_l2(tnp->oph.msg);
memcpy(hdr14, iuup_initialization, sizeof(iuup_initialization));
- /* FIXME: unexpected result, should be fixed: */
- OSMO_ASSERT((rc = osmo_iuup_tnl_prim_up(iui, tnp)) < 0); /* INIT not perrmited */
- OSMO_ASSERT(_passive_init_transport_rx_prim == 1); /* We receive an Init ACK */
- OSMO_ASSERT(_passive_init_retrans_user_rx_prim == 1); /* We receive the Status-Init.ind */
- /* FIXME: expected result: */
- //OSMO_ASSERT((rc = osmo_iuup_tnl_prim_up(iui, tnp)) == 0);
- //OSMO_ASSERT(_passive_init_transport_rx_prim == 2); /* We receive another Init ACK */
- //OSMO_ASSERT(_passive_init_retrans_user_rx_prim == 2); /* We receive another Status-Init.ind */
+ OSMO_ASSERT((rc = osmo_iuup_tnl_prim_up(iui, tnp)) == 0);
+ OSMO_ASSERT(_passive_init_transport_rx_prim == 2); /* We receive another Init ACK */
+ OSMO_ASSERT(_passive_init_retrans_user_rx_prim == 2); /* We receive another Status-Init.ind */
/* Send IuUP incoming data to the implementation: */
tnp = osmo_iuup_tnl_prim_alloc(iuup_test_ctx, OSMO_IUUP_TNL_UNITDATA, PRIM_OP_INDICATION, IUUP_MSGB_SIZE);
@@ -619,7 +612,7 @@
memcpy(hdr0, iuup_data, sizeof(iuup_data));
OSMO_ASSERT((rc = osmo_iuup_tnl_prim_up(iui, tnp)) == 0);
/* We receive it in RNL: */
- OSMO_ASSERT(_passive_init_retrans_user_rx_prim == 2);
+ OSMO_ASSERT(_passive_init_retrans_user_rx_prim == 3);
/* Now in opposite direction, RNL->[IuuP]->TNL: */
rnp = osmo_iuup_rnl_prim_alloc(iuup_test_ctx, OSMO_IUUP_RNL_DATA, PRIM_OP_REQUEST, IUUP_MSGB_SIZE);
@@ -629,7 +622,7 @@
rnp->oph.msg->l3h = msgb_put(rnp->oph.msg, sizeof(iuup_data) - 4);
memcpy(rnp->oph.msg->l3h, iuup_data + 4, sizeof(iuup_data) - 4);
OSMO_ASSERT((rc = osmo_iuup_rnl_prim_down(iui, rnp)) == 0);
- OSMO_ASSERT(_passive_init_transport_rx_prim == 2); /* We receive data in TNL */
+ OSMO_ASSERT(_passive_init_transport_rx_prim == 3); /* We receive data in TNL */
osmo_iuup_instance_free(iui);
}
diff --git a/tests/iuup/iuup_test.ok b/tests/iuup/iuup_test.ok
index 93f6954..57baba9 100644
--- a/tests/iuup/iuup_test.ok
+++ b/tests/iuup/iuup_test.ok
@@ -48,6 +48,9 @@
_passive_init_transport_prim_cb()
Transport: DL len=4: e4 00 24 00
_passive_init_retrans_user_prim_cb()
+_passive_init_transport_prim_cb()
+Transport: DL len=4: e4 00 24 00
+_passive_init_retrans_user_prim_cb()
User: UL len=31: 08 55 6d 94 4c 71 a1 a0 81 e7 ea d2 04 24 44 80 00 0e cd 82 b8 11 18 00 00 97 c4 79 4e 77 40
_passive_init_transport_prim_cb()
Transport: DL len=35: 01 00 e3 ff 08 55 6d 94 4c 71 a1 a0 81 e7 ea d2 04 24 44 80 00 0e cd 82 b8 11 18 00 00 97 c4 79 4e 77 40
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28271
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I5cb740702805693cc7f0a550e2e093f9bfdd507c
Gerrit-Change-Number: 28271
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: merged
laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/28269 )
Change subject: iuup: Drop unused events
......................................................................
iuup: Drop unused events
Those events are not realy used there and they are not even present in
the FSM definition. Let's drop them.
Change-Id: I4beec96ac4bdf047ebf144a8b009b297b47cacdc
---
M src/gsm/iuup.c
1 file changed, 0 insertions(+), 6 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/src/gsm/iuup.c b/src/gsm/iuup.c
index bd9e82c..4c0a948 100644
--- a/src/gsm/iuup.c
+++ b/src/gsm/iuup.c
@@ -757,12 +757,6 @@
irp = tnl_to_rnl_data(itp);
iui->user_prim_cb(&irp->oph, iui->user_prim_priv);
break;
- case IUUP_FSM_EVT_IUUP_UNITDATA_REQ:
- case IUUP_FSM_EVT_IUUP_UNITDATA_IND:
- case IUUP_FSM_EVT_SSASAR_UNITDATA_REQ:
- case IUUP_FSM_EVT_SSASAR_UNITDATA_IND:
- /* no state change */
- break;
}
}
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28269
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I4beec96ac4bdf047ebf144a8b009b297b47cacdc
Gerrit-Change-Number: 28269
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: merged
laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/28268 )
Change subject: iuup: Add missing state to bitmask for st SMpSDU_Data_Transfer_Ready
......................................................................
iuup: Add missing state to bitmask for st SMpSDU_Data_Transfer_Ready
The event is expected since the user of the IuUP stack can send a
CONFIGURE.req to transition to state null. The handling was already
there in the function, but the bit was missing in the FSM definition.
Change-Id: I830835a5b8b98f8b91b866f5280f508098c9ae7e
---
M src/gsm/iuup.c
1 file changed, 2 insertions(+), 1 deletion(-)
Approvals:
Jenkins Builder: Verified
daniel: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
diff --git a/src/gsm/iuup.c b/src/gsm/iuup.c
index 2c70307..bd9e82c 100644
--- a/src/gsm/iuup.c
+++ b/src/gsm/iuup.c
@@ -826,7 +826,8 @@
.action = iuup_fsm_init,
},
[IUUP_FSM_ST_SMpSDU_DATA_XFER_READY] = {
- .in_event_mask = S(IUUP_FSM_EVT_IUUP_DATA_REQ) |
+ .in_event_mask = S(IUUP_FSM_EVT_IUUP_CONFIG_REQ) |
+ S(IUUP_FSM_EVT_IUUP_DATA_REQ) |
S(IUUP_FSM_EVT_IUUP_DATA_IND),
.out_state_mask = S(IUUP_FSM_ST_NULL) |
S(IUUP_FSM_ST_INIT),
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28268
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I830835a5b8b98f8b91b866f5280f508098c9ae7e
Gerrit-Change-Number: 28268
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: merged
Attention is currently required from: pespin.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/28268 )
Change subject: iuup: Add missing state to bitmask for st SMpSDU_Data_Transfer_Ready
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28268
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I830835a5b8b98f8b91b866f5280f508098c9ae7e
Gerrit-Change-Number: 28268
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 13 Jun 2022 18:44:35 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment