Change in libosmocore[master]: LCLS: add gsm0808_create_ass_ext()
Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/11826 ) Change subject: LCLS: add gsm0808_create_ass_ext() .. Patch Set 16: Code-Review-1 (4 comments) https://gerrit.osmocom.org/#/c/11826/10/include/osmocom/gsm/gsm0808.h File include/osmocom/gsm/gsm0808.h: https://gerrit.osmocom.org/#/c/11826/10/include/osmocom/gsm/gsm0808.h@67 PS10, Line 67: struct msgb *gsm0808_create_ass_ext(const struct gsm0808_channel_type *ct, > Why? So far both ...2() and ..._ext() are widely used throughout the code. […] "ext" refers to "extended", while this is not something that the spec calls "extended" in any way. This is just a newer version of our API function, and for that the convention is (or should be) adding numbers. What if at some point some more IEs should be added, or some bug gets fixed? what do we call the next API function then, create_ass_ext_ext()? Use '2'. https://gerrit.osmocom.org/#/c/11826/16/src/gsm/gsm0808.c File src/gsm/gsm0808.c: https://gerrit.osmocom.org/#/c/11826/16/src/gsm/gsm0808.c@424 PS16, Line 424: /*! Create BSSMAP Assignment Request message, 3GPP TS 48.008 §3.2.1.1 especially if adding more comment, the initial summary line must end in a '.' https://gerrit.osmocom.org/#/c/11826/16/src/gsm/gsm0808.c@425 PS16, Line 425: This is identical to gsm0808_create_ass(), but adds KC and LCLS IEs. lacks a '*' https://gerrit.osmocom.org/#/c/11826/16/src/gsm/gsm0808.c@512 PS16, Line 512: /*! Create BSSMAP Assignment Request message, 3GPP TS 48.008 §3.2.1.1 . -- To view, visit https://gerrit.osmocom.org/11826 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id89765df3f8c12f55f73f1d7a9d90c8883eb3bba Gerrit-Change-Number: 11826 Gerrit-PatchSet: 16 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Pau Espin Pedrol Gerrit-CC: Vadim Yanitskiy Gerrit-Comment-Date: Fri, 30 Nov 2018 15:54:22 + Gerrit-HasComments: Yes Gerrit-HasLabels: Yes
Change in libosmocore[master]: LCLS: add gsm0808_create_ass_ext()
Max has posted comments on this change. ( https://gerrit.osmocom.org/11826 ) Change subject: LCLS: add gsm0808_create_ass_ext() .. Patch Set 10: (1 comment) I've tried to address other comments in extended patch series. Let me know if I've missed something. https://gerrit.osmocom.org/#/c/11826/10/include/osmocom/gsm/gsm0808.h File include/osmocom/gsm/gsm0808.h: https://gerrit.osmocom.org/#/c/11826/10/include/osmocom/gsm/gsm0808.h@67 PS10, Line 67: struct msgb *gsm0808_create_ass_ext(const struct gsm0808_channel_type *ct, > please call it gsm0808_create_ass2() instead Why? So far both ...2() and ..._ext() are widely used throughout the code. Did I miss some discussion in ML about it? -- To view, visit https://gerrit.osmocom.org/11826 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id89765df3f8c12f55f73f1d7a9d90c8883eb3bba Gerrit-Change-Number: 11826 Gerrit-PatchSet: 10 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Pau Espin Pedrol Gerrit-CC: Vadim Yanitskiy Gerrit-Comment-Date: Fri, 30 Nov 2018 15:08:56 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in libosmocore[master]: LCLS: add gsm0808_create_ass_ext()
Hello Pau Espin Pedrol, Neels Hofmeyr, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/11826 to look at the new patch set (#14). Change subject: LCLS: add gsm0808_create_ass_ext() .. LCLS: add gsm0808_create_ass_ext() It allows setting additional assignment parameters explicitly. Change-Id: Id89765df3f8c12f55f73f1d7a9d90c8883eb3bba Related: OS#2487 --- M include/osmocom/gsm/gsm0808.h M src/gsm/gsm0808.c M src/gsm/libosmogsm.map M tests/gsm0808/gsm0808_test.c M tests/gsm0808/gsm0808_test.ok 5 files changed, 132 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/26/11826/14 -- To view, visit https://gerrit.osmocom.org/11826 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id89765df3f8c12f55f73f1d7a9d90c8883eb3bba Gerrit-Change-Number: 11826 Gerrit-PatchSet: 14 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Pau Espin Pedrol Gerrit-CC: Vadim Yanitskiy
Change in libosmocore[master]: LCLS: add gsm0808_create_ass_ext()
Max has posted comments on this change. ( https://gerrit.osmocom.org/11826 ) Change subject: LCLS: add gsm0808_create_ass_ext() .. Set Ready For Review -- To view, visit https://gerrit.osmocom.org/11826 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id89765df3f8c12f55f73f1d7a9d90c8883eb3bba Gerrit-Change-Number: 11826 Gerrit-PatchSet: 13 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Pau Espin Pedrol Gerrit-CC: Vadim Yanitskiy Gerrit-Comment-Date: Fri, 30 Nov 2018 09:53:13 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in libosmocore[master]: LCLS: add gsm0808_create_ass_ext()
Max has posted comments on this change. ( https://gerrit.osmocom.org/11826 ) Change subject: LCLS: add gsm0808_create_ass_ext() .. Set Ready For Review -- To view, visit https://gerrit.osmocom.org/11826 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id89765df3f8c12f55f73f1d7a9d90c8883eb3bba Gerrit-Change-Number: 11826 Gerrit-PatchSet: 12 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Pau Espin Pedrol Gerrit-CC: Vadim Yanitskiy Gerrit-Comment-Date: Thu, 29 Nov 2018 22:48:14 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in libosmocore[master]: LCLS: add gsm0808_create_ass_ext()
Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/11826 ) Change subject: LCLS: add gsm0808_create_ass_ext() .. Patch Set 7: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/11826 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id89765df3f8c12f55f73f1d7a9d90c8883eb3bba Gerrit-Change-Number: 11826 Gerrit-PatchSet: 7 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Pau Espin Pedrol Gerrit-Comment-Date: Mon, 19 Nov 2018 12:50:39 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in libosmocore[master]: LCLS: add gsm0808_create_ass_ext()
Max has posted comments on this change. ( https://gerrit.osmocom.org/11826 ) Change subject: LCLS: add gsm0808_create_ass_ext() .. Patch Set 7: > Is it a different message from the existing function? It's the same, I've added spec. ref. to clarify. > Isn't ci inside lcls structure? It's not. Moreover, from my understanding of the spec it's not related to LCLS - I've updated confusing comment. The rest should be fixed in latest revision hence removed WIP mark. -- To view, visit https://gerrit.osmocom.org/11826 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id89765df3f8c12f55f73f1d7a9d90c8883eb3bba Gerrit-Change-Number: 11826 Gerrit-PatchSet: 7 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-CC: Pau Espin Pedrol Gerrit-Comment-Date: Mon, 19 Nov 2018 12:30:32 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in libosmocore[master]: LCLS: add gsm0808_create_ass_ext()
Max has posted comments on this change. ( https://gerrit.osmocom.org/11826 ) Change subject: LCLS: add gsm0808_create_ass_ext() .. Set Ready For Review -- To view, visit https://gerrit.osmocom.org/11826 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id89765df3f8c12f55f73f1d7a9d90c8883eb3bba Gerrit-Change-Number: 11826 Gerrit-PatchSet: 7 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-CC: Pau Espin Pedrol Gerrit-Comment-Date: Mon, 19 Nov 2018 12:25:39 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in libosmocore[master]: LCLS: add gsm0808_create_ass_ext()
Max has uploaded this change for review. ( https://gerrit.osmocom.org/11826
Change subject: LCLS: add gsm0808_create_ass_ext()
..
LCLS: add gsm0808_create_ass_ext()
It allows setting additional assingment parameters explicitly.
Change-Id: Id89765df3f8c12f55f73f1d7a9d90c8883eb3bba
Related: OS#2487
---
M include/osmocom/gsm/gsm0808.h
M src/gsm/gsm0808.c
M src/gsm/libosmogsm.map
M tests/gsm0808/gsm0808_test.c
M tests/gsm0808/gsm0808_test.ok
5 files changed, 135 insertions(+), 9 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/26/11826/1
diff --git a/include/osmocom/gsm/gsm0808.h b/include/osmocom/gsm/gsm0808.h
index cdbb273..b85908f 100644
--- a/include/osmocom/gsm/gsm0808.h
+++ b/include/osmocom/gsm/gsm0808.h
@@ -60,6 +60,12 @@
const struct sockaddr_storage *ss,
const struct gsm0808_speech_codec_list *scl,
const uint32_t *ci);
+struct msgb *gsm0808_create_ass_ext(const struct gsm0808_channel_type *ct,
+ const uint16_t *cic,
+ const struct sockaddr_storage *ss,
+ const struct gsm0808_speech_codec_list *scl,
+ const uint32_t *ci,
+ const uint8_t *kc, const struct osmo_lcls
*lcls);
struct msgb *gsm0808_create_ass_compl(uint8_t rr_cause, uint8_t chosen_channel,
uint8_t encr_alg_id, uint8_t speech_mode,
const struct sockaddr_storage *ss,
diff --git a/src/gsm/gsm0808.c b/src/gsm/gsm0808.c
index e5c3e30..caf37a9 100644
--- a/src/gsm/gsm0808.c
+++ b/src/gsm/gsm0808.c
@@ -400,18 +400,21 @@
return msg;
}
-/*! Create BSSMAP Assignment Request message
+/*! Create BSSMAP Assignment Request message, 3GPP TS 48.008 §3.2.1.1
* \param[in] ct Channel Type
* \param[in] cic Circuit Identity Code (Classic A only)
* \param[in] ss Socket Address of MSC-side RTP socket (AoIP only)
* \param[in] scl Speech Codec List (AoIP only)
- * \param[in] ci Call Identifier (Optional, LCLS)
+ * \param[in] ci Call Identifier (Optional, LCLS), §3.2.2.105
+ * \param[in] kc Kc128 ciphering key (Optional, A5/4), §3.2.2.109
+ * \param[in] lcls Optional LCLS parameters
* \returns callee-allocated msgb with BSSMAP Assignment Request message */
-struct msgb *gsm0808_create_ass(const struct gsm0808_channel_type *ct,
- const uint16_t *cic,
- const struct sockaddr_storage *ss,
- const struct gsm0808_speech_codec_list *scl,
- const uint32_t *ci)
+struct msgb *gsm0808_create_ass_ext(const struct gsm0808_channel_type *ct,
+ const uint16_t *cic,
+ const struct sockaddr_storage *ss,
+ const struct gsm0808_speech_codec_list *scl,
+ const uint32_t *ci,
+ const uint8_t *kc, const struct osmo_lcls
*lcls)
{
/* See also: 3GPP TS 48.008 3.2.1.1 ASSIGNMENT REQUEST */
struct msgb *msg;
@@ -452,8 +455,33 @@
/* AoIP: Call Identifier 3.2.2.105 */
if (ci) {
ci_sw = osmo_htonl(*ci);
- msgb_tv_fixed_put(msg, GSM0808_IE_CALL_ID, sizeof(ci_sw),
- (uint8_t *) & ci_sw);
+ msgb_tv_fixed_put(msg, GSM0808_IE_CALL_ID, sizeof(ci_sw),
(uint8_t *) & ci_sw);
+ }
+
+ if (kc) {
+ msgb_tv_fixed_put(msg, GSM0808_IE_KC_128, 16, kc);
+ }
+
+ if (lcls) {
+
+ /* LCLS: §3.2.2.115 Global Call Reference */
+ if (lcls->gcr) {
+ struct msgb *g = gsm0808_create_gcr(lcls->gcr);
+ msgb_tlv_put(msg, GSM0808_IE_GLOBAL_CALL_REF,
msgb_length(g), msgb_data(g));
+ msgb_free(g);
+ }
+
+ /* LCLS: §3.2.2.116 Configuration */
+ if (lcls->config != GSM0808_LCLS_CFG_NA)
+ msgb_tv_put(msg, GSM0808_IE_LCLS_CONFIG, lcls->config);
+
+ /* LCLS: §3.2.2.117 Connection Status Control */
+ if (lcls->control != GSM0808_LCLS_CSC_NA)
+ msgb_tv_put(msg, GSM0808_IE_LCLS_CONN_STATUS_CTRL,
lcls->control);
+
+ /* LCLS: §3.2.2.118 Correlation-Not-Needed */
+ if (!lcls->corr_needed)
+ msgb_v_put(msg, GSM0808_IE_LCLS_CORR_NOT_NEEDED);
}
/* push the bssmap header */
@@ -463,6 +491,22 @@
return msg;
}
+/*! Create BSSMAP Assignment Request message
+ * \param[in] ct Channel Type
+ * \param[in] cic Circuit Identity Code (Classic A only)
+ * \param[in]
