Attention is currently required from: fixeria.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/33911
to look at the new patch set (#3).
Change subject: gsm_08_08: define GSM0808_SCT_EXT (separately)
......................................................................
gsm_08_08: define GSM0808_SCT_EXT (separately)
As per 3GPP TS 48.008, section 3.2.2.103, the "Codec Type" field may
contain either a certain 3GPP Speech Codec Type directly (4 bit value),
or the so called "Codec Extension" = 0xFh, in which case the real Codec
Type follows in the next octet as "Extended Codec Type".
CSD is such an example, the encoding is defined as follows:
8 7 6 5 4 3 2 1
+----+----+----+----+-------------------+
| -- | PI | PT | -- | 0xFh |
+----+----+----+----+-------------------+
| Extended Codec Type (CSData) |
+----+----+-----------------------------+
| R2 | R3 | |
+----+----+-----------------------------+
CSData is coded with 0xFDh or '1111 1101' (0xfd).
Let's have the "Codec Extension" value clearly defined in the header
file, but intentionally separate from the other GSM0808_SCT_* values.
Change-Id: Iafaa25070684d2ba400c75fa33e803651a5ce857
Related: OS#6110, OS#4393, OS#4394
---
M include/osmocom/gsm/protocol/gsm_08_08.h
M src/gsm/gsm0808.c
M src/gsm/gsm0808_utils.c
3 files changed, 41 insertions(+), 3 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/11/33911/3
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33911
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Iafaa25070684d2ba400c75fa33e803651a5ce857
Gerrit-Change-Number: 33911
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: fixeria.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/33911
to look at the new patch set (#2).
Change subject: gsm_08_08: add missing GSM0808_SCT_EXT
......................................................................
gsm_08_08: add missing GSM0808_SCT_EXT
As per 3GPP TS 48.008, section 3.2.2.103, the "Codec Type" field may
contain either a certain 3GPP Speech Codec Type directly (4 bit value),
or the so called "Codec Extension" = 0xFh, in which case the real Codec
Type follows in the next octet as "Extended Codec Type".
CSD is such an example, the encoding is defined as follows:
8 7 6 5 4 3 2 1
+----+----+----+----+-------------------+
| -- | PI | PT | -- | 0xFh |
+----+----+----+----+-------------------+
| Extended Codec Type (CSData) |
+----+----+-----------------------------+
| R2 | R3 | |
+----+----+-----------------------------+
CSData is coded with 0xFDh or '1111 1101' (0xfd).
We already have GSM0808_SCT_CSD, add the missing GSM0808_SCT_EXT.
Change-Id: Iafaa25070684d2ba400c75fa33e803651a5ce857
Related: OS#6110, OS#4393, OS#4394
---
M include/osmocom/gsm/protocol/gsm_08_08.h
M src/gsm/gsm0808.c
2 files changed, 32 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/11/33911/2
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33911
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Iafaa25070684d2ba400c75fa33e803651a5ce857
Gerrit-Change-Number: 33911
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/33911 )
Change subject: gsm_08_08: add missing GSM0808_SCT_EXT
......................................................................
gsm_08_08: add missing GSM0808_SCT_EXT
As per 3GPP TS 48.008, section 3.2.2.103, the "Codec Type" field may
contain either a certain 3GPP Speech Codec Type directly (4 bit value),
or the so called "Codec Extension" = 0xFh, in which case the real Codec
Type follows in the next octet as "Extended Codec Type".
CSD is such an example, the encoding is defined as follows:
8 7 6 5 4 3 2 1
+----+----+----+----+-------------------+
| -- | PI | PT | -- | 0xFh |
+----+----+----+----+-------------------+
| Extended Codec Type (CSData) |
+----+----+-----------------------------+
| R2 | R3 | |
+----+----+-----------------------------+
CSData is coded with 0xFDh or '1111 1101' (0xfd).
We already have GSM0808_SCT_CSD, add the missing GSM0808_SCT_EXT.
Change-Id: Iafaa25070684d2ba400c75fa33e803651a5ce857
Related: OS#6110, OS#4393, OS#4394
---
M include/osmocom/gsm/protocol/gsm_08_08.h
1 file changed, 32 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/11/33911/1
diff --git a/include/osmocom/gsm/protocol/gsm_08_08.h b/include/osmocom/gsm/protocol/gsm_08_08.h
index 967e4fe..05cfa26 100644
--- a/include/osmocom/gsm/protocol/gsm_08_08.h
+++ b/include/osmocom/gsm/protocol/gsm_08_08.h
@@ -554,6 +554,8 @@
GSM0808_SCT_HR3 = 0x4, /*!< HR_AMR */
GSM0808_SCT_HR4 = 0xd, /*!< OHR AMR-WB */
GSM0808_SCT_HR6 = 0xb, /*!< OHR AMR */
+ GSM0808_SCT_HR6 = 0xb, /*!< OHR AMR */
+ GSM0808_SCT_EXT = 0xf, /*!< Codec Extension (the real Codec Type follows) */
GSM0808_SCT_CSD = 0xfd, /*!< CSData (see also TS 26.103) */
};
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33911
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Iafaa25070684d2ba400c75fa33e803651a5ce857
Gerrit-Change-Number: 33911
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newchange
Attention is currently required from: laforge, dexter.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33898 )
Change subject: S1AP_Emulation: improve accessibility of unit-data
......................................................................
Patch Set 4:
(1 comment)
File library/S1AP_Emulation.ttcn:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33898/comment/10da5150_3d78…
PS4, Line 389: return S1apExpectTableProc[i].vc_conn;
you are assuming only 1 vc_conn may register a given procedure code. Instead you should send message to all matching ones.
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33898
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I041b45b247e365b0d4ee8645c07dc5f26007af82
Gerrit-Change-Number: 33898
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 24 Jul 2023 18:46:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: arehbein.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/33892 )
Change subject: bsc (WIP): Make socket queue max. length configurable
......................................................................
Patch Set 2:
(4 comments)
Patchset:
PS2:
I think the problem is that you are storing the config value directly in the wqueue, and then when initializing the wqueue you may erase it? Then add an extra field which is updated by VTY and use that when initializing the wqueue.
File src/osmo-bsc/bsc_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/33892/comment/6b5794f4_a314410c
PS1, Line 2487: if (fd >= 0 && ((ofd = osmo_fd_get_by_fd(fd)) != NULL)) {
> This is to check whether or not the socket and hence has been (properly) opened via `pcu_sock_init`, […]
why do you care about the socket being opened here?
if you want to check for pcu-state presence, then simply: if(net->pcu_state) { ...}
https://gerrit.osmocom.org/c/osmo-bsc/+/33892/comment/9b834f0c_34581e20
PS1, Line 2491: }
> That's what this does; osmo write queue code (the code in `osmo_wqueue_bfd_cb()`) will use the addre […]
ah I thought there may be an API so I didn't see it sorry.
Actually, there should be an API, since it should internally free the extra messages already enequeued if the new max_len is smaller.
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/33892/comment/0d190f8a_33730028
PS1, Line 956: net->pcu_sock_fd = rc;
> it enables the check commented on [above](https://gerrit.osmocom. […]
That check is not needed imho.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/33892
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ic5f19f4613bccaf582997a4d02b689adee083a0b
Gerrit-Change-Number: 33892
Gerrit-PatchSet: 2
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 24 Jul 2023 16:39:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/33891 )
Change subject: osmo-bsc (WIP): Have PCU socket connection use osmo_wqueue
......................................................................
Patch Set 1:
(2 comments)
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/33891/comment/960559c1_5a623ca6
PS1, Line 845: return -1;
> aren't you leaking msgb here? Or is it freed by the caller?
done by `osmo_wqueue_bfd_cb()`
https://gerrit.osmocom.org/c/osmo-bsc/+/33891/comment/fa21e8dd_d6e43e18
PS1, Line 853: return -1;
> aren't you leaking msgb here? Or is it freed by the caller?
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/33891
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ifd9741045a87338e17eec3492590a5de9c308cb5
Gerrit-Change-Number: 33891
Gerrit-PatchSet: 1
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 24 Jul 2023 16:22:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/33892 )
Change subject: bsc (WIP): Make socket queue max. length configurable
......................................................................
Patch Set 1:
(4 comments)
File include/osmocom/bsc/pcu_if.h:
https://gerrit.osmocom.org/c/osmo-bsc/+/33892/comment/5a9e10ac_b8308d02
PS1, Line 12: #define BSC_CFG_PCU_SOCK_WQUEUE_LEN_MAX_MAX 2147483646
> I don't really think this is really needed, just use the value directly.
Oh yeah, this is something left over from previous iterations of this patch where I used this value in another place as well
File src/osmo-bsc/bsc_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/33892/comment/6c4f1134_55a7bb15
PS1, Line 2487: if (fd >= 0 && ((ofd = osmo_fd_get_by_fd(fd)) != NULL)) {
> why do you need this? I don't see the point.
This is to check whether or not the socket and hence has been (properly) opened via `pcu_sock_init`, in which case `net->pcu_state` should exist.
If this is not the case, then the update in line 2487 will suffice.
https://gerrit.osmocom.org/c/osmo-bsc/+/33892/comment/5e880d1d_900d529c
PS1, Line 2491: }
> btw, if the queue is already created you should update the max len there.
That's what this does; osmo write queue code (the code in `osmo_wqueue_bfd_cb()`) will use the address of `pcu_state->upqueue.bfd` (the pointer passed on `osmo_fd_setup()` in `pcu_sock_init()` along with `osmo_wqueue_bfd_cb()` as callback) to come back to `&pcu_state->upqueue` via `container_of()`
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/33892/comment/367c0aea_c00188dc
PS1, Line 956: net->pcu_sock_fd = rc;
> this looks really unrelated.
it enables the check commented on [above](https://gerrit.osmocom.org/c/osmo-bsc/+/33892/comments/406fedbe_14fc…
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/33892
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ic5f19f4613bccaf582997a4d02b689adee083a0b
Gerrit-Change-Number: 33892
Gerrit-PatchSet: 1
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 24 Jul 2023 16:16:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment