Attention is currently required from: falconia, fixeria, pespin.
Hello Jenkins Builder, fixeria, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/38555?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Code-Review+1 by pespin, Code-Review+2 by fixeria, Verified+1 by Jenkins Builder
The change is no longer submittable: Code-Review and Verified are unsatisfied now.
Change subject: CSD RTP: verify alignment of V.110 frames
......................................................................
CSD RTP: verify alignment of V.110 frames
Since the beginning of CSD implementation, OsmoBTS has operated
with an implicit (unstated) policy that V.110 frames must be
perfectly aligned within received clearmode RTP packets - a requirement
which is NOT set anywhere in TS 48.103 or any of the other specs
it references. This design policy is sensible from the standpoint
of implementation complexity (both OsmoBTS and osmo_trau2rtp emit
such perfectly aligned packets; if someone is building a gateway
between an Osmocom GSM network and ISDN-style external networks,
that gateway can act as the aligner), but it should be explicit
rather than implicit.
Check V.110 frame alignment in received clearmode RTP packets,
and reject packets that fail this alignment check.
Change-Id: Icd704dc7fa02e60074efc8a29ad7e42ebdf63783
---
M src/common/csd_v110.c
1 file changed, 20 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/55/38555/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38555?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Icd704dc7fa02e60074efc8a29ad7e42ebdf63783
Gerrit-Change-Number: 38555
Gerrit-PatchSet: 3
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Attention is currently required from: fixeria, pespin.
Hello Jenkins Builder, fixeria,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/38553?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Code-Review+1 by fixeria, Verified+1 by Jenkins Builder
Change subject: CSD: implement half-rate modes correctly
......................................................................
CSD: implement half-rate modes correctly
TCH/H4.8 and TCH/H2.4 modes are different from all other TCH in that
the channel coder runs (statistically) every 40 ms instead of the usual
20 ms. However, the standard RTP interface of TS 48.103 still requires
20 ms packets for all CSD modes; furtherfore, this requirement is not
just a matter of 3GPP spec being obstinent, but is highly useful for
interoperability, both between FR and HR and between OsmoBTS and E1 BTS.
The handling of CSD HR was totally broken in this regard: wrong RA2
packing mode, RTP clock model was violated, and even an internal
mismatch with l1sap sending TCH.req prims at twice the rate
at which the PHY expects them.
With this patch CSD HR handling becomes correct for TCH/H2.4 and for
TCH/H4.8 transparent; fully correct handling for TCH/H4.8 NT will
appear in a follow-up patch.
The packet/frame rate mismatch (40 ms Rx/Tx times while each RTP packet
carries 20 ms worth of data) is reconciled by emitting two RTP packets
directly back-to-back for UL, and pulling two RTP packets at a time
from the Rx jitter buffer at the needed times for DL.
Note: due to bug OS#6604, TTCN3 tests for CSD half-rate transparent modes
will start failing once this patch is merged; to make them pass again,
TTCN3 test code will need to be reworked to fix that bug.
Related: OS#6577
Change-Id: Ib35e910df263743cd243f51fb0bd6551ddfcf4c5
---
M include/osmo-bts/csd_v110.h
M src/common/csd_v110.c
M src/common/l1sap.c
M src/osmo-bts-trx/sched_lchan_tchh.c
M tests/csd/csd_test.c
M tests/csd/csd_test.err
6 files changed, 260 insertions(+), 112 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/53/38553/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38553?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ib35e910df263743cd243f51fb0bd6551ddfcf4c5
Gerrit-Change-Number: 38553
Gerrit-PatchSet: 3
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Attention is currently required from: falconia, pespin.
Hello Jenkins Builder, fixeria, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/38556?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
The change is no longer submittable: Verified is unsatisfied now.
Change subject: csd_v110_rtp_decode: preserve E2 & E3 bits for RLP alignment
......................................................................
csd_v110_rtp_decode: preserve E2 & E3 bits for RLP alignment
Modify CSD RTP input path code so incoming bits E2 & E3 from V.110
frames reach the TCH-RTS.ind handler in l1sap. These bits will be
used in the next patch to ensure proper alignment of DL RLP frames
in non-transparent CSD modes.
Related: OS#6579
Change-Id: I43b97caa6030b9401779998ca5dddc4cfe636e2f
---
M include/osmo-bts/csd_v110.h
M include/osmo-bts/msg_utils.h
M src/common/csd_v110.c
M src/common/l1sap.c
M tests/csd/csd_test.c
5 files changed, 17 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/56/38556/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38556?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I43b97caa6030b9401779998ca5dddc4cfe636e2f
Gerrit-Change-Number: 38556
Gerrit-PatchSet: 3
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Attention is currently required from: pespin.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/38557?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: CSD NT modes: transmit properly aligned RLP frames on DL
......................................................................
CSD NT modes: transmit properly aligned RLP frames on DL
There are two levels of alignment inside clearmode RTP packets
carrying CSData per TS 48.103 section 5.6:
1) Alignment of 2 or 4 V.110 (T) or pseudo-V.110 (NT) frames within
one RTP packet of 160 octets of an imaginary ISDN B channel;
2) For NT modes only, alignment of 4 pseudo-V.110 frames to form
a single 240-bit RLP frame.
Per previous patch, alignment 1 is to be treated as mandatory for
RTP transport inside an Osmocom network. Alignment 2 _could_ be
made mandatory for TCH/F9.6 NT, but the same is not possible for
TCH/[FH]4.8 NT: in the best case of half-alignment, alternating
RTP packets will carry alternating halves of RLP frames.
Implemented solution: allow arbitrary state of alignment 2
(aligned or misaligned) in the incoming RTP stream for all CSD NT
modes, and perform the necessary alignment internally.
This approach is consistent with the world of E1 BTS: a TRAU in
data mode is responsible for alignment 1 (with 20 ms TRAU frames
taking the place of our clearmode RTP packets), but only the BTS can
perform alignment 2, as the TRAU is agnostic to T vs NT distinction.
Related: OS#6579
Change-Id: Idaebfce6da13b23ba265a197502712d83991873e
---
M include/osmo-bts/csd_rlp.h
M include/osmo-bts/lchan.h
M src/common/csd_rlp.c
M src/common/l1sap.c
4 files changed, 301 insertions(+), 29 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/57/38557/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38557?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Idaebfce6da13b23ba265a197502712d83991873e
Gerrit-Change-Number: 38557
Gerrit-PatchSet: 3
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Attention is currently required from: fixeria, pespin.
falconia has posted comments on this change by falconia. ( https://gerrit.osmocom.org/c/osmo-bts/+/38553?usp=email )
Change subject: CSD: implement half-rate modes correctly
......................................................................
Patch Set 2:
(4 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/9cc638f0_5f775bca?usp… :
PS2, Line 26: by emitting two RTP packets
: directly back-to-back for UL, and pulling two RTP packets at a time
: from the Rx jitter buffer at the needed times for DL.
> I understand your idea now. […]
You are correct: if we were implementing a new E1 BTS, there would be no other way to handle the present issue except by buffering the bits and emitting them at the TDM-controlled rate. However, E1 BTS do similar buffering at all times, not just in this one corner case. Consider what happens with all other TCH types besides CSD-HR, i.e., consider CSD-FR and all speech modes. For those "traditional" TCH, osmo-bts (all models, not just trx) emits an RTP packet statistically every 20 ms - but it is not perfect 20 ms, it has TDMA jitter, with the actual time deltas between emitted RTP packets being TDMA*4 or TDMA*5 per multiframe structure. E1 BTS are different in this regard: their internal buffering fully conceals TDMA jitter from the external TDM connection, and the timing of TRAU frames on Abis is perfect 20 ms for each frame.
This aspect of osmo-bts (the fact that we normally expose our TDMA jitter in RTP output) is the main reason why I felt it would make no sense to try to replicate the exact behavior of E1 BTS here. Suppose we wanted to artificially delay the second half of CSD-HR frames by 20 ms to make our RTP output more E1-like - would it be a delay of exactly 20 ms per Linux system clock? Or would be 4 TDMA frames? Or 5 TDMA frames? Exactly which FNs would produce an artificial delay of 4 vs 5 frames? There won't be an answer in the specs as we talking about purely artificial delay here. So why delay at all, why not simply emit both packets right away...
File src/common/l1sap.c:
https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/f0babc20_9d4822b7?usp… :
PS2, Line 1539: 2
> `ARRAY_SIZE(input_msg)` here and below, please.
Will do.
https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/938ed2ec_7f9c834d?usp… :
PS2, Line 1556: else
> I would put a comment that it's an IDLE frame (all bits set to 1).
Will do.
https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/3e3f65cd_38cd9886?usp… :
PS2, Line 2098: switch (lchan->tch_mode) {
> As a further improvement, I suggest sharing `csd_v110_lchan_desc[]`, so that it can be used here (`d […]
Not sure if I agree - let me handle all other feedback first, then I'll get back to this one.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38553?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: Ib35e910df263743cd243f51fb0bd6551ddfcf4c5
Gerrit-Change-Number: 38553
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Sun, 27 Oct 2024 22:06:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Attention is currently required from: falconia, pespin.
fixeria has posted comments on this change by falconia. ( https://gerrit.osmocom.org/c/osmo-bts/+/38555?usp=email )
Change subject: CSD RTP: verify alignment of V.110 frames
......................................................................
Patch Set 2: Code-Review+1
(2 comments)
File src/common/csd_v110.c:
https://gerrit.osmocom.org/c/osmo-bts/+/38555/comment/7be057c4_ad8d113b?usp… :
PS2, Line 158: for (i = 0; i < 8; i++) {
We can avoid if-statement in loops to improve performance (in theory):
```
ubit_t bit0 = 0;
ubit_t bit1 = 1;
for (i = 0; i < 8; i++)
bit0 |= ra_bits[i];
for (i = 1; i < 10; i++)
bit1 &= ra_bits[i * 8];
return (bit0 == 0) && (bit1 == 1);
```
https://gerrit.osmocom.org/c/osmo-bts/+/38555/comment/3e949862_619f828a?usp… :
PS2, Line 204: return -EINVAL;
> We do have a rate_ctr: when `csd_v110_rtp_decode()` returns negative, the code in `l1sap_rtp_rx_cb() […]
Acknowledged
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38555?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: Icd704dc7fa02e60074efc8a29ad7e42ebdf63783
Gerrit-Change-Number: 38555
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Sun, 27 Oct 2024 21:34:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: falconia <falcon(a)freecalypso.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Attention is currently required from: falconia, pespin.
fixeria has posted comments on this change by falconia. ( https://gerrit.osmocom.org/c/osmo-bts/+/38554?usp=email )
Change subject: csd_v110: set E2 bit correctly for TCH/[FH]4.8 NT
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
File src/common/l1sap.c:
https://gerrit.osmocom.org/c/osmo-bts/+/38554/comment/6e559ef9_2a6b1630?usp… :
PS2, Line 2066: static const uint8_t tchf48_nt_e2_map[26] = {
> again where does this 26 come from
Obviously from 3GPP specs. mentioned above in comment...
It's further explained in the first patch of this patchset.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38554?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: I485af5e01ea87c1721a298a486cd344a17884200
Gerrit-Change-Number: 38554
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Sun, 27 Oct 2024 21:26:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Attention is currently required from: falconia, pespin.
fixeria has posted comments on this change by falconia. ( https://gerrit.osmocom.org/c/osmo-bts/+/38553?usp=email )
Change subject: CSD: implement half-rate modes correctly
......................................................................
Patch Set 2: Code-Review+1
(5 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/f55d153e_392cfca0?usp… :
PS2, Line 26: by emitting two RTP packets
: directly back-to-back for UL, and pulling two RTP packets at a time
: from the Rx jitter buffer at the needed times for DL.
I understand your idea now. Just curious, how would you do that if osmo-bts were an E1 BTS (not Abis-over-IP)? I guess the only option would be to buffer V.110 frames, since you cannot do the same trick for a TDM interface.
File src/common/l1sap.c:
https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/7e6234ce_4292240f?usp… :
PS2, Line 1493: /* TDMA frame number of burst 'a' % 26 is the table index.
> This bit of code is not newly written with this patch - I simply moved it from osmo-bts-trx to the c […]
I agree with Mychaela here. It's a well known constant that is used quite often here and there.
https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/5aa500ad_532f6eb5?usp… :
PS2, Line 1539: 2
`ARRAY_SIZE(input_msg)` here and below, please.
https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/d0770a5a_b08f40ee?usp… :
PS2, Line 1556: else
I would put a comment that it's an IDLE frame (all bits set to 1).
https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/8879c982_8934bcf2?usp… :
PS2, Line 2098: switch (lchan->tch_mode) {
As a further improvement, I suggest sharing `csd_v110_lchan_desc[]`, so that it can be used here (`desc->num_blocks * desc->num_bits`). I have a WIP patch moving that lookup table to libosmocore: https://gerrit.osmocom.org/c/libosmocore/+/35914.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38553?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: Ib35e910df263743cd243f51fb0bd6551ddfcf4c5
Gerrit-Change-Number: 38553
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Sun, 27 Oct 2024 21:22:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: falconia <falcon(a)freecalypso.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>