Attention is currently required from: osmith.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/33361 )
Change subject: codec_filter_init: prepare for CSD
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Patchset:
PS1:
You could have moved this together with the other function in the previous commit.
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/33361
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I1270b00464456abc5300fd47e6087a0ba6243d03
Gerrit-Change-Number: 33361
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 19 Jun 2023 17:03:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: osmith.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/33360 )
Change subject: Cosmetic: gsm48_cc_tx_setup: remove TODO comment
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
"Remove comment as ... will be used..." Then why don't you remove the comment in the same commit where yo implement that?
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/33360
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I0539632f464bc44945599bec52dc2a4df2f0115f
Gerrit-Change-Number: 33360
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 19 Jun 2023 17:02:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: osmith.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/33358 )
Change subject: codec_filter_set_ms_from_bc: prepare for CSD
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Patchset:
PS1:
Not sure if it makes sense to create an own file, there's already transaction.c (just mentioning, not strong thoughts since I don't know the code).
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/33358
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: If225f2a299ce6bc9ae35a17d6f591d889f49155e
Gerrit-Change-Number: 33358
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 19 Jun 2023 17:00:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33337 )
Change subject: stream: set name to sockname by default
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
On second thoughts, I'm starting to think this patch should be better changing the sockname information to be separate from the "user-set API 'name'".
Which means, during LOGGCLI it will always print the sockname information (eg pre-stored in cli->sockname[]), and then if the user set a name, also the name.
This makes the user not having to care about filling "name" with sockname if it wants to keep some specific naming plus the sockname info. Also, when exporting to CTRL, FSM, etc. names would be far better/easier to identify.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33337
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I17ef699dab72c1b613708070d22e9f040b0fe069
Gerrit-Change-Number: 33337
Gerrit-PatchSet: 4
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-Comment-Date: Mon, 19 Jun 2023 16:55:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: pespin.
Hello Jenkins Builder, fixeria, daniel,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmo-sccp/+/33335
to look at the new patch set (#7).
Change subject: Set stream_cli/srv name to contain ASP + sockname
......................................................................
Set stream_cli/srv name to contain ASP + sockname
Upon connect/accept, update the name to contain both the ASP and the
sockname of the established socked. This allows easily identifying the
stream based on IP layer as well as upper xUA layer.
Example logs:
"""
stream.c:1193 SRV(::1:2905) accept()ed new link from ::1 to port 2905
stream.c:1672 SRVCONN(asp-dyn-0_(r=::1:43568<->l=::1:2905)) connected read/write (what=0x1)
stream.c:1589 SRVCONN(asp-dyn-0_(r=::1:43568<->l=::1:2905)) message received
"""
"""
stream.c:521 CLICONN(asp0){CONNECTING} connection established
stream.c:552 CLICONN(asp0_(r=::1:2905<->l=::ffff:127.0.0.2:35911)){CONNECTED} connected read
stream.c:401 CLICONN(asp0_(r=::1:2905<->l=::ffff:127.0.0.2:35911)){CONNECTED} message received
"""
Depends: libosmo-netif.git I539a0d29d11348efe702f971965a55cf56db5c59
Change-Id: I05bc5f6c7795f62c1814c1c774287b41ee85a475
---
M TODO-RELEASE
M src/osmo_ss7.c
2 files changed, 38 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/35/33335/7
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/33335
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: I05bc5f6c7795f62c1814c1c774287b41ee85a475
Gerrit-Change-Number: 33335
Gerrit-PatchSet: 7
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33373 )
Change subject: pdch.cpp: Drop impossible code path
......................................................................
pdch.cpp: Drop impossible code path
In that code path, new_tbf is an UPLINK_TBF. Uplink TBFs (tbf_ul_fsm.c)
don't use state TBF_ST_WAIT_RELEASE, hence that condition cannot ever be
true.
Change-Id: I379d4140326a46e94914152fe9b735de852ceda0
---
M src/pdch.cpp
1 file changed, 13 insertions(+), 3 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/73/33373/1
diff --git a/src/pdch.cpp b/src/pdch.cpp
index f871d1d..8a2ad96 100644
--- a/src/pdch.cpp
+++ b/src/pdch.cpp
@@ -401,9 +401,6 @@
"TBF is gone TLLI=0x%08x\n", tlli);
return;
}
- if (tbf->state_is(TBF_ST_WAIT_RELEASE) &&
- tbf->direction == new_tbf->direction)
- tbf_free(tbf);
osmo_fsm_inst_dispatch(new_tbf->state_fi, TBF_EV_ASSIGN_ACK_PACCH, NULL);
/* there might be LLC packets waiting in the queue, but the DL
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33373
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I379d4140326a46e94914152fe9b735de852ceda0
Gerrit-Change-Number: 33373
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33374 )
Change subject: Differentiate between T3192 and T3193
......................................................................
Differentiate between T3192 and T3193
T3192 on the MS side (boadcasted by the network on SI13) is used for the
MS to stay monitoring the PDCH of a DL TBF after sending PKT DL ACK/NACK
for the last DL data block (FinalAck=1).
T3193 is the counterpart in the network, defined as >T3192, which is
used to make sure the TFI is no longer used by the MS and hence can be
reused/re-assigned again.
Hence, we want to differentiate between those 2 timers, since the first
one gives us information on how late can we still tx PktDlAss on PACCH
on that DL TBF, while the later only provides info on when to free the
DL TBF in order to reuse the TFI.
Change-Id: I7421342461bf159d945651037e6fe690f88c6fae
---
M src/gprs_ms.c
M src/pdch.cpp
M src/tbf_dl_fsm.c
M src/tbf_fsm.h
M tests/tbf/TbfTest.err
5 files changed, 111 insertions(+), 23 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/74/33374/1
diff --git a/src/gprs_ms.c b/src/gprs_ms.c
index 0b062c1..dda2f29 100644
--- a/src/gprs_ms.c
+++ b/src/gprs_ms.c
@@ -1368,20 +1368,31 @@
dl_tbf = ms_dl_tbf(ms);
if (dl_tbf) {
- if (tbf_state(dl_tbf_as_tbf_const(dl_tbf)) == TBF_ST_WAIT_RELEASE) {
- LOGPTBFDL(dl_tbf, LOGL_DEBUG, "in WAIT RELEASE state (T3193), so reuse TBF\n");
+ switch (tbf_state(dl_tbf_as_tbf_const(dl_tbf))) {
+ case TBF_ST_WAIT_RELEASE:
+ LOGPTBFDL(dl_tbf, LOGL_DEBUG, "in WAIT RELEASE state (T3192), so reuse TBF\n");
rc = ms_new_dl_tbf_assigned_on_pacch(ms, dl_tbf_as_tbf(dl_tbf));
+ return rc;
+ case TBF_ST_WAIT_REUSE_TFI:
+ /* According to DL TBF state it should be back to CCCH, let's check UL TBF to have more information. */
+ break;
+ case TBF_ST_RELEASING:
+ /* Something went wrong (T3195), delay for later. */
+ default:
+ /* DL TBF in working status (do nothing)*/
+ return 0;
}
- } else {
- /* Check if we can create a DL TBF to start sending the enqueued
- * data. Otherwise it will be triggered later when it is reachable
- * again. */
- if (ms_is_reachable_for_dl_ass(ms)) {
- if (ms_ul_tbf(ms))
- rc = ms_new_dl_tbf_assigned_on_pacch(ms, ul_tbf_as_tbf(ms_ul_tbf(ms)));
- else
- rc = ms_new_dl_tbf_assigned_on_pch(ms);
- }
+
+ }
+
+ /* Check if we can create a DL TBF to start sending the enqueued
+ * data. Otherwise it will be triggered later when it is reachable
+ * again. */
+ if (ms_is_reachable_for_dl_ass(ms)) {
+ if (ms_ul_tbf(ms))
+ rc = ms_new_dl_tbf_assigned_on_pacch(ms, ul_tbf_as_tbf(ms_ul_tbf(ms)));
+ else
+ rc = ms_new_dl_tbf_assigned_on_pch(ms);
}
return rc;
}
diff --git a/src/pdch.cpp b/src/pdch.cpp
index 8a2ad96..7dfd3c2 100644
--- a/src/pdch.cpp
+++ b/src/pdch.cpp
@@ -429,8 +429,9 @@
"TBF is gone TLLI=0x%08x\n", tlli);
return;
}
- if (tbf->state_is(TBF_ST_WAIT_RELEASE) &&
- tbf->direction == new_tbf->direction)
+ if ((tbf->state_is(TBF_ST_WAIT_RELEASE) ||
+ tbf->state_is(TBF_ST_WAIT_REUSE_TFI)) &&
+ tbf->direction == new_tbf->direction)
tbf_free(tbf);
osmo_fsm_inst_dispatch(new_tbf->state_fi, TBF_EV_ASSIGN_ACK_PACCH, NULL);
diff --git a/src/tbf_dl_fsm.c b/src/tbf_dl_fsm.c
index ebcd8b8..2877e21 100644
--- a/src/tbf_dl_fsm.c
+++ b/src/tbf_dl_fsm.c
@@ -34,7 +34,8 @@
[TBF_ST_ASSIGN] = {},
[TBF_ST_FLOW] = {},
[TBF_ST_FINISHED] = {},
- [TBF_ST_WAIT_RELEASE] = { .T = 3193 },
+ [TBF_ST_WAIT_RELEASE] = { .T = 3192 },
+ [TBF_ST_WAIT_REUSE_TFI] = { /* .T = 3193 set manually onenter subtracting T3192 */ },
[TBF_ST_RELEASING] = { .T = 3195 },
};
@@ -243,8 +244,9 @@
break;
case TBF_EV_FINAL_ACK_RECVD:
/* We received Final Ack (DL ACK/NACK) from MS. move to
- * WAIT_RELEASE, we wait there for release or re-use the TBF in
- * case we receive more DL data to tx */
+ * WAIT_RELEASE, where MS stays monitoring PDCH over T3192 span,
+ * where we can use this DL TBF to assign a new one in case we
+ * receive more DL data to Tx */
tbf_dl_fsm_state_chg(fi, TBF_ST_WAIT_RELEASE);
break;
case TBF_EV_MAX_N3105:
@@ -259,6 +261,14 @@
{
struct tbf_dl_fsm_ctx *ctx = (struct tbf_dl_fsm_ctx *)fi->priv;
+ /* T3192 is running on the MS and has also been armed by this FSM now.
+ * During that time, it is possible to reach the MS over PACCH to assign
+ * new DL TBF.
+ * Upon T3192 expiration, FSM wil transition to TBF_ST_WAIT_REUSE_TFI
+ * for some more time (T3193 - T3192) imtil internally freeing the TBF
+ * object, at which time the resources can be reused.
+ */
+
mod_ass_type(ctx, GPRS_RLCMAC_FLAG_CCCH, false);
}
@@ -266,10 +276,41 @@
{
switch (event) {
case TBF_EV_FINAL_ACK_RECVD:
- /* ignore, duplicate ACK, we already know about since we are in WAIT_RELEASE */
+ /* ignore, duplicate ACK, we already know about since we left ST_FINISHED */
break;
- case TBF_EV_MAX_N3105:
- tbf_dl_fsm_state_chg(fi, TBF_ST_RELEASING);
+ default:
+ OSMO_ASSERT(0);
+ }
+}
+
+static void st_wait_reuse_tfi_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state)
+{
+ struct tbf_dl_fsm_ctx *ctx = (struct tbf_dl_fsm_ctx *)fi->priv;
+ struct GprsMs *ms = tbf_ms(ctx->tbf);
+ unsigned long t3192, t3193, res;
+
+ /* T3192 has expired, so the MS is not listening on that PACCH anymore.
+ * Still, wait until remaining of T3193 expiration (>T3192) to internally
+ * free the TBF, at which point the TFI and other allocated resources
+ * will be freed and can then be reused.
+ */
+
+ t3192 = osmo_tdef_get(ms->bts->T_defs_bts, 3192, OSMO_TDEF_MS, -1);
+ t3193 = osmo_tdef_get(ms->bts->T_defs_bts, 3193, OSMO_TDEF_MS, -1);
+ /* As per spec T3193 shall be greater than T3192, but let's be safe against wrong configs: */
+ res = (t3193 >= t3192) ? (t3193 - t3192) : 0;
+ fi->T = 3193;
+ LOGPTBF(ctx->tbf, LOGL_DEBUG, "Waiting %lu sec. %lu microsec (T3193 - T3192) [REUSE TFI]\n",
+ res / 1000, (res % 1000) * 1000);
+ osmo_timer_schedule(&fi->timer, res / 1000, (res % 1000) * 1000);
+}
+
+static void st_wait_reuse_tfi(struct osmo_fsm_inst *fi, uint32_t event, void *data)
+{
+ /* Simply wait for T3193 timeout, it will tbf_free() */
+ switch (event) {
+ case TBF_EV_FINAL_ACK_RECVD:
+ /* ignore, duplicate ACK, we already know about since we left ST_FINISHED */
break;
default:
OSMO_ASSERT(0);
@@ -334,6 +375,9 @@
case -2002:
handle_timeout_X2002(fi);
break;
+ case 3192:
+ tbf_dl_fsm_state_chg(fi, TBF_ST_WAIT_REUSE_TFI);
+ break;
case -2001:
LOGPTBFDL(ctx->dl_tbf, LOGL_NOTICE, "releasing due to PACCH assignment timeout.\n");
/* fall-through */
@@ -401,11 +445,21 @@
X(TBF_EV_FINAL_ACK_RECVD) |
X(TBF_EV_MAX_N3105),
.out_state_mask =
+ X(TBF_ST_WAIT_REUSE_TFI) |
X(TBF_ST_RELEASING),
.name = "WAIT_RELEASE",
.action = st_wait_release,
.onenter = st_wait_release_on_enter,
},
+ [TBF_ST_WAIT_REUSE_TFI] = {
+ .in_event_mask =
+ X(TBF_EV_FINAL_ACK_RECVD),
+ .out_state_mask =
+ X(TBF_ST_RELEASING),
+ .name = "WAIT_REUSE_TFI",
+ .action = st_wait_reuse_tfi,
+ .onenter = st_wait_reuse_tfi_on_enter,
+ },
[TBF_ST_RELEASING] = {
.in_event_mask =
X(TBF_EV_DL_ACKNACK_MISS),
diff --git a/src/tbf_fsm.h b/src/tbf_fsm.h
index 999b3c0..fd6ffa6 100644
--- a/src/tbf_fsm.h
+++ b/src/tbf_fsm.h
@@ -52,7 +52,8 @@
TBF_ST_ASSIGN, /* wait for downlink assignment */
TBF_ST_FLOW, /* RLC/MAC flow, resource needed */
TBF_ST_FINISHED, /* flow finished, wait for release */
- TBF_ST_WAIT_RELEASE,/* DL TBF: wait for release or restart */
+ TBF_ST_WAIT_RELEASE, /* DL TBF: wait for release or restart */
+ TBF_ST_WAIT_REUSE_TFI, /* DL TBF: wait to reuse TFI after last PKT ACK/NACK */
TBF_ST_RELEASING, /* releasing, wait to free TFI/USF */
};
diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err
index aac7f4f..4aa594b 100644
--- a/tests/tbf/TbfTest.err
+++ b/tests/tbf/TbfTest.err
@@ -295,7 +295,7 @@
DL_TBF(DL:TFI-0-0-0:G){FINISHED}: state_chg to WAIT_RELEASE
TBF(DL:TFI-0-0-0:G:TLLI-0xffeeddcc){WAIT_RELEASE} unset ass. type CCCH [prev CCCH:1, PACCH:0]
MS(TLLI-0xffeeddcc:TA-0:MSCLS-45-0:DL) appending 200 bytes to DL LLC queue
-TBF(DL:TFI-0-0-0:G:TLLI-0xffeeddcc){WAIT_RELEASE} in WAIT RELEASE state (T3193), so reuse TBF
+TBF(DL:TFI-0-0-0:G:TLLI-0xffeeddcc){WAIT_RELEASE} in WAIT RELEASE state (T3192), so reuse TBF
[DL] algo A <multi> (suggested TRX: 0): Alloc start
- Skipping TS 0, because not enabled
- Skipping TS 1, because not enabled
@@ -573,7 +573,7 @@
DL_TBF(DL:TFI-0-0-0:G){FINISHED}: state_chg to WAIT_RELEASE
TBF(DL:TFI-0-0-0:G:TLLI-0xffeeddcc){WAIT_RELEASE} unset ass. type CCCH [prev CCCH:1, PACCH:0]
MS(TLLI-0xffeeddcc:TA-0:MSCLS-45-0:DL) appending 200 bytes to DL LLC queue
-TBF(DL:TFI-0-0-0:G:TLLI-0xffeeddcc){WAIT_RELEASE} in WAIT RELEASE state (T3193), so reuse TBF
+TBF(DL:TFI-0-0-0:G:TLLI-0xffeeddcc){WAIT_RELEASE} in WAIT RELEASE state (T3192), so reuse TBF
[DL] algo A <multi> (suggested TRX: 0): Alloc start
- Skipping TS 0, because not enabled
- Skipping TS 1, because not enabled
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33374
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I7421342461bf159d945651037e6fe690f88c6fae
Gerrit-Change-Number: 33374
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange