- gerrit-log - lists.osmocom.org

[S] Change in osmo-bts[master]: csd_v110: properly set bit E2 for TCH/F4.8 NT
by falconia 27 Sep '24

27 Sep '24

Attention is currently required from: fixeria. falconia has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/osmo-bts/+/38301?usp=email ) Change subject: csd_v110: properly set bit E2 for TCH/F4.8 NT ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: > Hm, then Harald's code in `gsmtap_csd_rlp_process()` should also be wrong. Yes, it is also wrong! > Yes, such a flag can be calculated from current TDMA Fn then. See here: https://gerrit.osmocom.org/c/osmo-bts/+/38304 I haven't touched the gsmtap code yet, mostly because I don't understand it. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38301?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: If8307a9ce0fdc6da45157149ccef7b840ff9d9b3 Gerrit-Change-Number: 38301 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Fri, 27 Sep 2024 10:21:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: falconia <falcon(a)freecalypso.org> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[S] Change in osmo-bts[master]: csd_v110: properly set bit E2 for TCH/F4.8 NT
by fixeria 27 Sep '24

27 Sep '24

Attention is currently required from: falconia. fixeria has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/osmo-bts/+/38301?usp=email ) Change subject: csd_v110: properly set bit E2 for TCH/F4.8 NT ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: > Unfortunately this approach cannot work because bit E7 simply does not exist in NT mode: [... Hm, then Harald's code in `gsmtap_csd_rlp_process()` should also be wrong. > The only correct solution is to extend the internal API to csd_v110_rtp_encode() and pass a flag that indicates alignment with TDMA multiframe structure. Yes, such a flag can be calculated from current TDMA Fn then. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38301?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: If8307a9ce0fdc6da45157149ccef7b840ff9d9b3 Gerrit-Change-Number: 38301 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: falconia <falcon(a)freecalypso.org> Gerrit-Comment-Date: Fri, 27 Sep 2024 07:09:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: falconia <falcon(a)freecalypso.org>

1 0

[S] Change in osmo-bts[master]: csd_v110: set E2 bit correctly for TCH/F4.8 NT
by falconia 27 Sep '24

27 Sep '24

falconia has posted comments on this change by falconia. ( https://gerrit.osmocom.org/c/osmo-bts/+/38304?usp=email ) Change subject: csd_v110: set E2 bit correctly for TCH/F4.8 NT ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Disclaimer: I currently have no way to test any CSD functionality or anything else that requires osmo-bts-trx; this patch has been implemented "from theory only". However, the current code is known to be wrong (E2 is always 0), whereas the new code stands at least a chance of being correct. In any case, I would appreciate it if someone can test it. If someone can run a test setup and send me pcap of the RTP output, I can analyze that pcap myself to verify correctness of E-bits. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38304?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: I1e0ad58aad3361b3a2b0a47113bd065587636730 Gerrit-Change-Number: 38304 Gerrit-PatchSet: 1 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Fri, 27 Sep 2024 03:24:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in osmo-bts[master]: csd_v110: set E2 bit correctly for TCH/F4.8 NT
by falconia 27 Sep '24

27 Sep '24

falconia has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/38304?usp=email ) Change subject: csd_v110: set E2 bit correctly for TCH/F4.8 NT ...................................................................... csd_v110: set E2 bit correctly for TCH/F4.8 NT In all classic NT modes below 14.5 kbit/s, V.110 frame bits E2 and E3 are repurposed to indicate the alignment whereby a single 240-bit RLP frame is composed of 4 pseudo-V.110 frames in switching transport. (TS 48.020 section 15.1.) In the case of TCH/F9.6, setting both E2 and E3 is easy because all 4 pseudo-V.110 frames go out in the same clearmode RTP packet as a result of a single channel decoding operation. However, in the case of TCH/F4.8 there are two separate channel decoding operations producing two separate RTP packets 20 ms apart. Furthermore, GSM 05.03 section 3.4.1 specifies which TDMA frame positions hold which half of the RLP frame - therefore, the correct emission of bit E2 needs to be based on the TDMA frame number at which the block was received. Related: OS#6578 Change-Id: I1e0ad58aad3361b3a2b0a47113bd065587636730 --- M include/osmo-bts/csd_v110.h M src/common/csd_v110.c M src/common/l1sap.c M tests/csd/csd_test.c 4 files changed, 23 insertions(+), 8 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/04/38304/1 diff --git a/include/osmo-bts/csd_v110.h b/include/osmo-bts/csd_v110.h index f6be0ae..b461724 100644 --- a/include/osmo-bts/csd_v110.h +++ b/include/osmo-bts/csd_v110.h @@ -18,6 +18,6 @@ extern const struct csd_v110_lchan_desc csd_v110_lchan_desc[256]; int csd_v110_rtp_encode(const struct gsm_lchan *lchan, uint8_t *rtp, - const uint8_t *data, size_t data_len); + const uint8_t *data, size_t data_len, uint32_t fn); int csd_v110_rtp_decode(const struct gsm_lchan *lchan, uint8_t *data, const uint8_t *rtp, size_t rtp_len); diff --git a/src/common/csd_v110.c b/src/common/csd_v110.c index d6c00b0..d8a8596 100644 --- a/src/common/csd_v110.c +++ b/src/common/csd_v110.c @@ -73,8 +73,17 @@ #endif }; +/* In the case of TCH/F4.8 NT, we have to set bit E2 based on the TDMA + * frame number at which we received the block in question. See + * GSM 05.03 section 3.4.1 and the mapping tables of GSM 05.02. */ +static const uint8_t tchf48_nt_e2_map[26] = { + [4] = 1, /* B1 position */ + [13] = 1, /* B3 position */ + [21] = 1, /* B5 position */ +}; + int csd_v110_rtp_encode(const struct gsm_lchan *lchan, uint8_t *rtp, - const uint8_t *data, size_t data_len) + const uint8_t *data, size_t data_len, uint32_t fn) { const struct csd_v110_frame_desc *desc; ubit_t ra_bits[80 * 4]; @@ -109,9 +118,15 @@ /* E1 .. E3 must set by out-of-band knowledge */ if (lchan->csd_mode == LCHAN_CSD_M_NT) { /* non-transparent: as per 3GPP TS 48.020, Table 7 */ - df.e_bits[0] = 0; /* E1: as per 15.1.2, shall be set to 0 (for BSS-MSC) */ - df.e_bits[1] = (i >> 1) & 0x01; /* E2: 0 for Q1/Q2, 1 for Q3/Q4 */ - df.e_bits[2] = (i >> 0) & 0x01; /* E3: 0 for Q1/Q3, 1 for Q2/Q4 */ + /* E1: as per 15.1.2, shall be set to 0 (for BSS-MSC) */ + df.e_bits[0] = 0; + /* E2: 0 for Q1/Q2, 1 for Q3/Q4 */ + if (desc->num_blocks == 4) + df.e_bits[1] = (i >> 1) & 0x01; + else + df.e_bits[1] = tchf48_nt_e2_map[fn % 26]; + /* E3: 0 for Q1/Q3, 1 for Q2/Q4 */ + df.e_bits[2] = (i >> 0) & 0x01; } else { /* transparent: as per 3GPP TS 44.021, Figure 4 */ df.e_bits[0] = e1e2e3_map[lchan->csd_mode][0]; /* E1 */ diff --git a/src/common/l1sap.c b/src/common/l1sap.c index 5a900f8..68920a4 100644 --- a/src/common/l1sap.c +++ b/src/common/l1sap.c @@ -1937,7 +1937,7 @@ gsmtap_csd_rlp_process(lchan, true, tch_ind, data, data_len); - rc = csd_v110_rtp_encode(lchan, &rtp_pl[0], data, data_len); + rc = csd_v110_rtp_encode(lchan, &rtp_pl[0], data, data_len, tch_ind->fn); if (rc < 0) return; diff --git a/tests/csd/csd_test.c b/tests/csd/csd_test.c index 13a6419..235d5f5 100644 --- a/tests/csd/csd_test.c +++ b/tests/csd/csd_test.c @@ -115,7 +115,7 @@ data_enc[i] = i & 0x01; /* encode an RTP frame and print it */ - rc = csd_v110_rtp_encode(&lchan, &rtp[0], &data_enc[0], bit_num); + rc = csd_v110_rtp_encode(&lchan, &rtp[0], &data_enc[0], bit_num, 0); fprintf(stderr, "[i] csd_v110_rtp_encode() returns %d\n", rc); if (rc != RFC4040_RTP_PLEN) return; @@ -139,7 +139,7 @@ fprintf(stderr, "[i] Testing '%s' (IDLE)\n", tc->name); /* encode an idle RTP frame and print it */ - rc = csd_v110_rtp_encode(&lchan, &rtp[0], &data_enc[0], 0); + rc = csd_v110_rtp_encode(&lchan, &rtp[0], &data_enc[0], 0, 0); fprintf(stderr, "[i] csd_v110_rtp_encode() returns %d\n", rc); if (rc != RFC4040_RTP_PLEN) return; -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38304?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: I1e0ad58aad3361b3a2b0a47113bd065587636730 Gerrit-Change-Number: 38304 Gerrit-PatchSet: 1 Gerrit-Owner: falconia <falcon(a)freecalypso.org>

1 0

[M] Change in libosmo-abis[master]: libosmotrau: add CSD RA2 functions
by falconia 27 Sep '24

27 Sep '24

Attention is currently required from: dexter, fixeria, laforge, pespin. falconia has posted comments on this change by falconia. ( https://gerrit.osmocom.org/c/libosmo-abis/+/38290?usp=email ) Change subject: libosmotrau: add CSD RA2 functions ...................................................................... Patch Set 2: (2 comments) File include/osmocom/trau/csd_ra2.h: https://gerrit.osmocom.org/c/libosmo-abis/+/38290/comment/1bf10950_25d9444b… : PS1, Line 58: > here he nbytes indicates the number of _output_ bytes. See the other comment. https://gerrit.osmocom.org/c/libosmo-abis/+/38290/comment/a63b900b_ead988b7… : PS1, Line 66: ubit_t > so here the nbytes indicats the number of _input_ bytes. I find that a bit confusing. […] The suggested change has been made in the new patch version. -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/38290?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I7b98b958651b3fc1a814d119d1b8644c91f98676 Gerrit-Change-Number: 38290 Gerrit-PatchSet: 2 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 26 Sep 2024 23:01:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org>

1 0

[M] Change in libosmo-abis[master]: libosmotrau: add CSD RA2 functions
by falconia 27 Sep '24

27 Sep '24

Attention is currently required from: dexter, falconia, fixeria, laforge, pespin. Hello Jenkins Builder, dexter, fixeria, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-abis/+/38290?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by laforge, Code-Review+1 by pespin, Verified+1 by Jenkins Builder Change subject: libosmotrau: add CSD RA2 functions ...................................................................... libosmotrau: add CSD RA2 functions Rate adaption function RA2 for GSM CSD calls is defined in 3GPP TS 48.020 chapter 13. While it is nothing but one special case of I.460 submultiplexing and we do have i460_mux infrastructure in Osmocom, that fully-general I.460 infra is too heaviweight for CSD encoding and decoding functions that need simple, single-input and single-output RA2 packing and unpacking functions. Right now ad hoc RA2 functions are used in the trau_rtp_conv module and in OsmoBTS csd_v110; same functions will also be needed for the upcoming RAA' unit test addition. Add proper RA2 packing and unpacking functions that can be used in all of these places. Change-Id: I7b98b958651b3fc1a814d119d1b8644c91f98676 --- M include/Makefile.am A include/osmocom/trau/csd_ra2.h M src/Makefile.am A src/trau/csd_ra2.c 4 files changed, 140 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/90/38290/2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/38290?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I7b98b958651b3fc1a814d119d1b8644c91f98676 Gerrit-Change-Number: 38290 Gerrit-PatchSet: 2 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: falconia <falcon(a)freecalypso.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de>

1 0

[M] Change in pysim[master]: utils: move enc_msisdn and dec_msisdn to legacy/utils.py
by laforge 26 Sep '24

26 Sep '24

Attention is currently required from: dexter. laforge has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/38118?usp=email ) Change subject: utils: move enc_msisdn and dec_msisdn to legacy/utils.py ...................................................................... Patch Set 11: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/38118?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I19ec8ba14551ec282fc0cc12ae2f6d528bdfc527 Gerrit-Change-Number: 38118 Gerrit-PatchSet: 11 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 26 Sep 2024 21:36:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in pysim[master]: ts_51_011: replace encoding of EF.MSISDN with construct model
by laforge 26 Sep '24

26 Sep '24

Attention is currently required from: dexter. laforge has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/38117?usp=email ) Change subject: ts_51_011: replace encoding of EF.MSISDN with construct model ...................................................................... Patch Set 10: (1 comment) File pySim/ts_51_011.py: https://gerrit.osmocom.org/c/pysim/+/38117/comment/10dc5393_c09c3c99?usp=em… : PS10, Line 191: _test_de_encode = [ plese also add some test vectors for the letacy case with `{'msisdn'...` so we know that doesn't break. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/38117?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I647f5c63f7f87902a86c0c5d8e92fdc7f4350a5a Gerrit-Change-Number: 38117 Gerrit-PatchSet: 10 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 26 Sep 2024 21:35:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in pysim[master]: cosmetic: use **kwargs instead of **_kwargs
by laforge 26 Sep '24

26 Sep '24

Attention is currently required from: dexter. laforge has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/38300?usp=email ) Change subject: cosmetic: use **kwargs instead of **_kwargs ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/38300?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I98857cc774185e55a604eb4fbfbf62ed4bd6ded7 Gerrit-Change-Number: 38300 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 26 Sep 2024 21:34:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in pysim[master]: filesystem: pass total_len to construct of when encoding file contents
by laforge 26 Sep '24

26 Sep '24

Attention is currently required from: dexter. laforge has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/38195?usp=email ) Change subject: filesystem: pass total_len to construct of when encoding file contents ...................................................................... Patch Set 6: Code-Review-1 (5 comments) Patchset: PS6: I think what we see from the several last few iterations of this patchset is that there is a lack of testing. I argue the code of patchset 5 and 6 are broken - yet we do not detect this in unit tests. Please add some tests that exercise those code paths. File pySim/filesystem.py: https://gerrit.osmocom.org/c/pysim/+/38195/comment/be057514_7059e409?usp=em… : PS6, Line 746: def __get_size(self, total_len: Optional[int] = None) -> int: must be `-> Optional[int]` as it also may return `None` https://gerrit.osmocom.org/c/pysim/+/38195/comment/0884379a_54f82d98?usp=em… : PS6, Line 1054: def __get_rec_len(self, total_len: Optional[int] = None) -> int: likewise: `-> Optional[int]` https://gerrit.osmocom.org/c/pysim/+/38195/comment/25bd5ecf_253bd16c?usp=em… : PS6, Line 1274: here we still don't return an Optional[int] but a dict? I think that's inconsistent comparing the __get_size situation above with the one here for record oriented EF? https://gerrit.osmocom.org/c/pysim/+/38195/comment/1cf177e7_df86be34?usp=em… : PS6, Line 1303: and this will again break. Your __get_rec_len() method wil return a dict and you're prefixing that with total_len, so the result is something like `{'total_len': {'total_len': 23}` -- To view, visit https://gerrit.osmocom.org/c/pysim/+/38195?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I1b7a51594fbc5d9fe01132c39354a2fa88d53f9b Gerrit-Change-Number: 38195 Gerrit-PatchSet: 6 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 26 Sep 2024 21:33:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

2026

2025

2024

2023

2022

gerrit-log