laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/38420?usp=email )
Change subject: rsl: rsl_handle_chan_mod_ie(): set lchan->csd_mode for NT CSD
......................................................................
rsl: rsl_handle_chan_mod_ie(): set lchan->csd_mode for NT CSD
In commit 66eae187, I skipped assigning LCHAN_CSD_M_NT (0) to
lchan->csd_mode for non-transparent channel modes. I assumed that
the entire gsm_lchan struct was zero-initialized during activation
or release procedures, but this assumption was incorrect.
In fact, some lchan fields are initialized during activation, while
others are initialized during release. Specifically, lchan->csd_mode
is zero-initialized when the process starts, and then updated only
for transparent mode requests. For non-transparent mode requests,
this field is not updated, meaning it retains its previous value.
This bug caused incorrect E1/E2/E3 bit settings and missing GSMTAP
RLP output for channels that were previously used for transparent
CSD calls. This patch is fixing it.
Change-Id: I793ab4dc25fa852eade6f7a3b67ae961ceb7a093
Fixes: 66eae187 "rsl: rsl_handle_chan_mod_ie(): set lchan->csd_mode"
Related: OS#1572, OS#6579
---
M src/common/rsl.c
1 file changed, 11 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/src/common/rsl.c b/src/common/rsl.c
index cc802c8..20581d4 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -205,37 +205,48 @@
/* If octet 4 indicates non-transparent data */
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_14k5):
lchan->tch_mode = GSM48_CMODE_DATA_14k5;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_12k0):
lchan->tch_mode = GSM48_CMODE_DATA_12k0;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_6k0):
lchan->tch_mode = GSM48_CMODE_DATA_6k0;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_43k5):
lchan->tch_mode = GSM48_CMODE_DATA_43k5;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_28k8):
/* 28.8 kbit/s services, 29.0 kbit/s radio interface rate */
lchan->tch_mode = GSM48_CMODE_DATA_29k0;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NTA_43k5_14k5):
lchan->tch_mode = GSM48_CMODE_DATA_43k5_14k5;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NTA_29k0_14k5):
lchan->tch_mode = GSM48_CMODE_DATA_29k0_14k5;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NTA_43k5_29k0):
lchan->tch_mode = GSM48_CMODE_DATA_43k5_29k0;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NTA_14k5_43k5):
lchan->tch_mode = GSM48_CMODE_DATA_14k5_43k5;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NTA_14k5_29k0):
lchan->tch_mode = GSM48_CMODE_DATA_14k5_29k0;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NTA_29k0_43k5):
lchan->tch_mode = GSM48_CMODE_DATA_29k0_43k5;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
/* If octet 4 indicates transparent data */
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38420?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I793ab4dc25fa852eade6f7a3b67ae961ceb7a093
Gerrit-Change-Number: 38420
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Attention is currently required from: fixeria.
laforge has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/osmo-bts/+/38420?usp=email )
Change subject: rsl: rsl_handle_chan_mod_ie(): set lchan->csd_mode for NT CSD
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38420?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I793ab4dc25fa852eade6f7a3b67ae961ceb7a093
Gerrit-Change-Number: 38420
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Sun, 13 Oct 2024 12:50:09 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/38420?usp=email )
Change subject: rsl: rsl_handle_chan_mod_ie(): set lchan->csd_mode for NT CSD
......................................................................
rsl: rsl_handle_chan_mod_ie(): set lchan->csd_mode for NT CSD
In commit 66eae187, I skipped assigning LCHAN_CSD_M_NT (0) to
lchan->csd_mode for non-transparent channel modes. I assumed that
the entire gsm_lchan struct was zero-initialized during activation
or release procedures, but this assumption was incorrect.
In fact, some lchan fields are initialized during activation, while
others are initialized during release. Specifically, lchan->csd_mode
is zero-initialized when the process starts, and then updated only
for transparent mode requests. For non-transparent mode requests,
this field is not updated, meaning it retains its previous value.
This bug caused incorrect E1/E2/E3 bit settings and missing GSMTAP
RLP output for channels that were previously used for transparent
CSD calls. This patch is fixing it.
Change-Id: I793ab4dc25fa852eade6f7a3b67ae961ceb7a093
Fixes: 66eae187 "rsl: rsl_handle_chan_mod_ie(): set lchan->csd_mode"
Related: OS#1572, OS#6579
---
M src/common/rsl.c
1 file changed, 11 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/20/38420/1
diff --git a/src/common/rsl.c b/src/common/rsl.c
index cc802c8..20581d4 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -205,37 +205,48 @@
/* If octet 4 indicates non-transparent data */
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_14k5):
lchan->tch_mode = GSM48_CMODE_DATA_14k5;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_12k0):
lchan->tch_mode = GSM48_CMODE_DATA_12k0;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_6k0):
lchan->tch_mode = GSM48_CMODE_DATA_6k0;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_43k5):
lchan->tch_mode = GSM48_CMODE_DATA_43k5;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NT_28k8):
/* 28.8 kbit/s services, 29.0 kbit/s radio interface rate */
lchan->tch_mode = GSM48_CMODE_DATA_29k0;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NTA_43k5_14k5):
lchan->tch_mode = GSM48_CMODE_DATA_43k5_14k5;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NTA_29k0_14k5):
lchan->tch_mode = GSM48_CMODE_DATA_29k0_14k5;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NTA_43k5_29k0):
lchan->tch_mode = GSM48_CMODE_DATA_43k5_29k0;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NTA_14k5_43k5):
lchan->tch_mode = GSM48_CMODE_DATA_14k5_43k5;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NTA_14k5_29k0):
lchan->tch_mode = GSM48_CMODE_DATA_14k5_29k0;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
case RSL_CMODE(RSL_CMOD_SPD_DATA, RSL_CMOD_CSD_NTA_29k0_43k5):
lchan->tch_mode = GSM48_CMODE_DATA_29k0_43k5;
+ lchan->csd_mode = LCHAN_CSD_M_NT;
break;
/* If octet 4 indicates transparent data */
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38420?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I793ab4dc25fa852eade6f7a3b67ae961ceb7a093
Gerrit-Change-Number: 38420
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Attention is currently required from: fixeria, osmith, pespin.
laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375?usp=email )
Change subject: ss7 vty: Fix insert order of routes with higher priority
......................................................................
Patch Set 5:
(1 comment)
File tests/vty/osmo_stp_route_prio.vty:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375/comment/4ae0ddab_5f17d… :
PS2, Line 87: 3.2.1/14 0 as3 ? ? ?
> @laforge@gnumonks.org I'm fine with that, I can do that in a follow-up patch. Something related to discuss though: We are currently storing the value in uint32_t value in strcut osmo_ss7_route cfg.
> We could make that one a uint8_t, but that may break ABI compatibility.I'll check because anyway since we added secondary_pc we are already breaking ABI since last version.
I wouldn't bother. It doesn't hurt to store that value as uint32_t at all.
> TBH I think we should break ABI once and move as many as possible of those structs into private headers. Same goes for most of the APIs too.
> This would make it far easier improving the library, since anyway when adding features will keep otherwise breaking ABI again and again.
That's a separate discussion, unrelated.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I6e7b5e3f06ae2c9468da58c55d2f42cbcd777218
Gerrit-Change-Number: 38375
Gerrit-PatchSet: 5
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Sat, 12 Oct 2024 19:48:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Attention is currently required from: fixeria, laforge, osmith.
pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375?usp=email )
Change subject: ss7 vty: Fix insert order of routes with higher priority
......................................................................
Patch Set 5:
(1 comment)
File tests/vty/osmo_stp_route_prio.vty:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375/comment/e00eef81_a38ad… :
PS2, Line 87: 3.2.1/14 0 as3 ? ? ?
> I agree, we should also use 5 as default. […]
@laforge@gnumonks.org I'm fine with that, I can do that in a follow-up patch. Something related to discuss though: We are currently storing the value in uint32_t value in strcut osmo_ss7_route cfg.
We could make that one a uint8_t, but that may break ABI compatibility.I'll check because anyway since we added secondary_pc we are already breaking ABI since last version.
TBH I think we should break ABI once and move as many as possible of those structs into private headers. Same goes for most of the APIs too.
This would make it far easier improving the library, since anyway when adding features will keep otherwise breaking ABI again and again.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/38375?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I6e7b5e3f06ae2c9468da58c55d2f42cbcd777218
Gerrit-Change-Number: 38375
Gerrit-PatchSet: 5
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Sat, 12 Oct 2024 18:54:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Attention is currently required from: pespin.
laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/upf-benchmark/+/38354?usp=email )
Change subject: Rename osmo-pfcp-tool to osmo-upf-genload
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> I was thinking more in the sense of "generate load", but I have no strong opinion, I can change it.
we already have rtp-load-gen (osmo-mgw) gtp-load-gen (separate git repo), so it might make sense to stay with one approach.
--
To view, visit https://gerrit.osmocom.org/c/upf-benchmark/+/38354?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: upf-benchmark
Gerrit-Branch: master
Gerrit-Change-Id: Ie36379b5dcf4ab86276dfda9e25410fe41db1d93
Gerrit-Change-Number: 38354
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Sat, 12 Oct 2024 17:15:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>