Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-05-05 Thread Harald Welte
Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13259 )

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..

add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

osmo-bsc so far omits the AoIP Transport Layer Address from its Handover
Request Acknowledge message, which breaks inter-BSC Handover for AoIP.
Allow fixing that.

One quirk I really don't like about this: I would prefer to directly use struct
sockaddr_storage as a member of the struct gsm0808_handover_request_ack. Even
though struct sockaddr_storage appears in various function signatures, the
gsm0808.c actually also gets built on embedded systems that lack arpa/inet.h
(for me indicated by the ARM build job on jenkins). Compiling gsm0808.c works
only because the actual coding of struct sockaddr_storage is implemented in
gsm0808_util.c, which (apparently) does not get built on embedded and hence,
even though there are undefined references to e.g.
gsm0808_enc_aoip_trasp_addr() it works.

Related: I4a5acdb2d4a0b947cc0c62067a67be88a3d467ff (osmo-bsc)
Change-Id: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
---
M include/osmocom/gsm/gsm0808.h
M src/gsm/gsm0808.c
M src/gsm/libosmogsm.map
3 files changed, 60 insertions(+), 12 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/include/osmocom/gsm/gsm0808.h b/include/osmocom/gsm/gsm0808.h
index 5b05dbc..93195b5 100644
--- a/include/osmocom/gsm/gsm0808.h
+++ b/include/osmocom/gsm/gsm0808.h
@@ -194,9 +194,29 @@
 };
 struct msgb *gsm0808_create_handover_request(const struct 
gsm0808_handover_request *params);

-struct msgb *gsm0808_create_handover_request_ack(const uint8_t *l3_info, 
uint8_t l3_info_len,
-uint8_t chosen_channel, 
uint8_t chosen_encr_alg,
-uint8_t chosen_speech_version);
+struct gsm0808_handover_request_ack {
+   const uint8_t *l3_info;
+   uint8_t l3_info_len;
+
+   bool chosen_channel_present;
+   uint8_t chosen_channel;
+
+   /*! For A5/N set chosen_encr_alg = N+1, e.g. chosen_encr_alg = 1 means 
A5/0 (no encryption), 2 means A5/1, 4
+* means A5/3. Set chosen_encr_alg = 0 to omit the Chosen Encryption 
Algorithm IE. */
+   uint8_t chosen_encr_alg;
+
+   /* chosen_speech_version == 0 omits the IE */
+   enum gsm0808_permitted_speech chosen_speech_version;
+
+   bool speech_codec_chosen_present;
+   struct gsm0808_speech_codec speech_codec_chosen;
+
+   const struct sockaddr_storage *aoip_transport_layer;
+
+   /* more items are defined in the spec and may be added later */
+   bool more_items; /*!< always set this to false */
+};
+struct msgb *gsm0808_create_handover_request_ack2(const struct 
gsm0808_handover_request_ack *params);

 struct gsm0808_handover_command {
const uint8_t *l3_info;
diff --git a/src/gsm/gsm0808.c b/src/gsm/gsm0808.c
index 4406043..3c77c77 100644
--- a/src/gsm/gsm0808.c
+++ b/src/gsm/gsm0808.c
@@ -983,9 +983,7 @@
 /*! Create BSSMAP HANDOVER REQUEST ACKNOWLEDGE message, 3GPP TS 48.008 
3.2.1.10.
  * Sent from the MT BSC back to the MSC when it has allocated an lchan to 
handover to.
  * l3_info is the RR Handover Command that the MO BSC sends to the MS to move 
over. */
-struct msgb *gsm0808_create_handover_request_ack(const uint8_t *l3_info, 
uint8_t l3_info_len,
-uint8_t chosen_channel, 
uint8_t chosen_encr_alg,
-uint8_t chosen_speech_version)
+struct msgb *gsm0808_create_handover_request_ack2(const struct 
gsm0808_handover_request_ack *params)
 {
struct msgb *msg;

@@ -996,13 +994,25 @@
/* Message Type, 3.2.2.1 */
msgb_v_put(msg, BSS_MAP_MSG_HANDOVER_RQST_ACKNOWLEDGE);

-   /* Layer 3 Information, 3.2.2.24 */
-   msgb_tlv_put(msg, GSM0808_IE_LAYER_3_INFORMATION, l3_info_len, l3_info);
+   /* Layer 3 Information, 3.2.2.24 -- it is actually mandatory, but 
rather compose a nonstandard message than
+* segfault or return NULL without a log message. */
+   if (params->l3_info && params->l3_info_len)
+   msgb_tlv_put(msg, GSM0808_IE_LAYER_3_INFORMATION, 
params->l3_info_len, params->l3_info);

-   msgb_tv_put(msg, GSM0808_IE_CHOSEN_CHANNEL, chosen_channel);
-   msgb_tv_put(msg, GSM0808_IE_CHOSEN_ENCR_ALG, chosen_encr_alg);
-   if (chosen_speech_version != 0)
-   msgb_tv_put(msg, GSM0808_IE_SPEECH_VERSION, 
chosen_speech_version);
+   if (params->chosen_channel_present)
+   msgb_tv_put(msg, GSM0808_IE_CHOSEN_CHANNEL, 
params->chosen_channel);
+   if (params->chosen_encr_alg)
+   msgb_tv_put(msg, GSM0808_IE_CHOSEN_ENCR_ALG, 
params->chosen_encr_alg);
+
+   if (params->chosen_speech_version != 0)
+   

Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-05-05 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13259 )

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..


Patch Set 12: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 12
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Sun, 05 May 2019 16:25:27 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-05-03 Thread Neels Hofmeyr
Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/13259

to look at the new patch set (#12).

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..

add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

osmo-bsc so far omits the AoIP Transport Layer Address from its Handover
Request Acknowledge message, which breaks inter-BSC Handover for AoIP.
Allow fixing that.

One quirk I really don't like about this: I would prefer to directly use struct
sockaddr_storage as a member of the struct gsm0808_handover_request_ack. Even
though struct sockaddr_storage appears in various function signatures, the
gsm0808.c actually also gets built on embedded systems that lack arpa/inet.h
(for me indicated by the ARM build job on jenkins). Compiling gsm0808.c works
only because the actual coding of struct sockaddr_storage is implemented in
gsm0808_util.c, which (apparently) does not get built on embedded and hence,
even though there are undefined references to e.g.
gsm0808_enc_aoip_trasp_addr() it works.

Related: I4a5acdb2d4a0b947cc0c62067a67be88a3d467ff (osmo-bsc)
Change-Id: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
---
M include/osmocom/gsm/gsm0808.h
M src/gsm/gsm0808.c
M src/gsm/libosmogsm.map
3 files changed, 60 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/59/13259/12
--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 12
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-05-03 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/13259 )

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..


Patch Set 11:

ping


--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 11
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Fri, 03 May 2019 14:06:08 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-04-26 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/13259 )

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..


Patch Set 11:

Given the other patches waiting for this on this branch and all the depending 
patches in other repositories waiting for those, there is a bit of pressure to 
merge this now. If something else should happen with this, it would be good to 
make up our minds sooner rather than later...


--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 11
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Fri, 26 Apr 2019 19:43:54 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-04-11 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/13259 )

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..


Patch Set 9:

(1 comment)

https://gerrit.osmocom.org/#/c/13259/9//COMMIT_MSG
Commit Message:

https://gerrit.osmocom.org/#/c/13259/9//COMMIT_MSG@15
PS9, Line 15: the
: gsm0808.c actually also gets built on embedded systems that lack 
arpa/inet.h
: (for me indicated by the ARM build job on jenkins)
> we can simply not compile 08.08 related functions in the "embedded" case. […]
Yes, I actually did try quite a few things, and in the end decided it's not 
worth it to waste my time on.
Originally I wanted to use a struct sockaddr as a member in struct 
gsm0808_handover_*.
In the meantime I have befriended the idea of passing pointers in those 
structs, so not that urgent.

The core problem was that I cannot use struct sockaddr as a non-opaque struct 
in the header file.
Even though on embedded, the .c file isn't built, the header file is still 
included!
And in the .h file one obviously cannot use '#if (!EMBEDDED)', because in 
/usr/include there is no config.h.

It's not a big deal for me anymore,
but in general the include / compile semantics for gsm0808.h are unsorted/wrong.



--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 9
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Fri, 12 Apr 2019 00:08:30 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-04-11 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13259 )

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..


Patch Set 9:

(1 comment)

https://gerrit.osmocom.org/#/c/13259/9//COMMIT_MSG
Commit Message:

https://gerrit.osmocom.org/#/c/13259/9//COMMIT_MSG@15
PS9, Line 15: the
: gsm0808.c actually also gets built on embedded systems that lack 
arpa/inet.h
: (for me indicated by the ARM build job on jenkins)
we can simply not compile 08.08 related functions in the "embedded" case.  we 
don't want to implement the A interface on bare iron in a microcontroller 
firmware. So you can just use a large "#if (!EMBEDDED)" blockaround those 
parts, or even disable the entire files from Makefile.am in a patch preceding 
this patch?  Or was this approach attempted and didn't work for some reason? In 
the latter case I'm happy to investigate why you couldn't deactivate it.



--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 9
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Thu, 11 Apr 2019 06:08:08 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-04-10 Thread Neels Hofmeyr
Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/13259

to look at the new patch set (#9).

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..

add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

osmo-bsc so far omits the AoIP Transport Layer Address from its Handover
Request Acknowledge message, which breaks inter-BSC Handover for AoIP.
Allow fixing that.

One quirk I really don't like about this: I would prefer to directly use struct
sockaddr_storage as a member of the struct gsm0808_handover_request_ack. Even
though struct sockaddr_storage appears in various function signatures, the
gsm0808.c actually also gets built on embedded systems that lack arpa/inet.h
(for me indicated by the ARM build job on jenkins). Compiling gsm0808.c works
only because the actual coding of struct sockaddr_storage is implemented in
gsm0808_util.c, which (apparently) does not get built on embedded and hence,
even though there are undefined references to e.g.
gsm0808_enc_aoip_trasp_addr() it works.

Related: I4a5acdb2d4a0b947cc0c62067a67be88a3d467ff (osmo-bsc)
Change-Id: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
---
M include/osmocom/gsm/gsm0808.h
M src/gsm/gsm0808.c
M src/gsm/libosmogsm.map
3 files changed, 60 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/59/13259/9
--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 9
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-03-27 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13259 )

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..


Patch Set 8: Code-Review-1

I thin we have to keep this one out of master until the "embedded ARM builds" 
question is resolved.  This is clearly in need of cleanup, and I really don't 
like us to make work around in APIs / runtime code just because some 
compile-time deicisons are not taken correctly.  It may be time to simply 
disable lots of code that we'd never want to run inside a phone (like 
osmocombb) in the arm-non-eabi (EMBEDDED) builds.  I'm not saying neels has to 
do this, I'm just saying it needs to happen.


--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 8
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Wed, 27 Mar 2019 07:41:57 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-03-24 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/13259 )

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..


Patch Set 6:

As I said:

"because the use of sockaddr_storage isn't properly separated from 
implementations that get compiled on embedded and can't use it; embedded builds 
include headers naming struct sockaddr_storage even though the platform doesn't 
support it, and even function implementations using sockaddr_storage get 
compiled there but can't ever be used because they have unresolved references 
to functions handling sockaddr_storage"

I cannot use struct sockaddr_storage in gsm0808.h, because some freaking .c 
files #include it also on ARM. I cannot #ifdef out in gsm0808.h, because 
obviously #include "config.h" doesn't work in a .h file installed in 
$prefix/include/.

So the way headers get included need to be untangled. I don't really want to 
have to do that.

Right now ARM only works because the .h defines an opaque struct 
sockaddr_storage and only uses pointers. Try it and you will see that ARM 
builds on jenkins fail.

I would have liked to use the struct as non-pointer, but it's more work than 
anticipated.


--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 6
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-CC: Harald Welte 
Gerrit-Comment-Date: Sun, 24 Mar 2019 06:01:34 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-03-19 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13259 )

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..


Patch Set 6:

I'm sorry, but I don't really understand the problem.

We will *never* nee d A interface support functions of GSM 08.08 inside the 
firmware of a microcontroller where we don't have Linux/POSIX APIs.

So why would we bother with considering bare-iron ARM when working on this kind 
of API?  Why not simply #ifdef it out?


--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 6
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-CC: Harald Welte 
Gerrit-Comment-Date: Tue, 19 Mar 2019 13:27:26 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-03-14 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/13259 )

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..


Patch Set 6:

This quirk explained in the commit log is a minor detail but it bugs me. If we 
merge this once, we will always keep this sockaddr_storage as pointer instead 
of as member chiseled in stone.

The long term perspective could have been to also provide these structs like 
struct gsm0808_handover_request_ack as the output of decoding in libosmocore, 
instead of just as input to encoding. So far the situation is that libosmocore 
can only *encode* BSSAP, but *decoding* is done separately in each program. 
i.e., osmo-bsc implements TLV parsing of some BSSAP messages, while osmo-msc 
implements TLV parsing for others.

Now, if at some point I would add a gsm0808_decode_handover_request_ack() 
function, I would like to provide struct gsm0808_handover_request_ack as 
out-argument, and I would also like to then have the sockaddr_storage as direct 
member. Otherwise the caller would need to first populate with a pointer before 
calling. All possible of course, but rather quirky IMO.

This just because the use of sockaddr_storage isn't properly separated from 
implementations that get compiled on embedded and can't use it; embedded builds 
include headers naming struct sockaddr_storage even though the platform doesn't 
support it, and even function implementations using sockaddr_storage get 
compiled there but can't ever be used because they have unresolved references 
to functions handling sockaddr_storage ... meh

If we reshuffled the files to separate sockaddr_storage from embedded properly, 
we could include netinet/in.h directly in gsm0808.h and just use struct 
sockaddr_* as members.

A similar quirk shows up in that osmo_ip_port patch, I have to carefully 
navigate around what includes what.

In the regression tests, it is even weirder. There is an "if ENABLE_MSGFILE" in 
testsuite.at, but an 'if' like that doesn't seem to have any effect at all. So 
we can't actually exclude individual tests on embedded. How does that work, we 
just don't run any tests there apparently?

What do you guys think, should I try to separate sockaddr_storage from embedded 
builds before merging (or releasing) this, or is this a can of worms and 
incompatibilities that will suck me into realms where I don't want to go?


--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 6
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Fri, 15 Mar 2019 05:05:23 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-03-14 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/13259

to look at the new patch set (#6).

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..

add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

osmo-bsc so far omits the AoIP Transport Layer Address from its Handover
Request Acknowledge message, which breaks inter-BSC Handover for AoIP.
Allow fixing that.

One quirk I really don't like about this: I would prefer to directly use struct
sockaddr_storage as a member of the struct gsm0808_handover_request_ack. Even
though struct sockaddr_storage appears in various function signatures, the
gsm0808.c actually also gets built on embedded systems that lack arpa/inet.h
(for me indicated by the ARM build job on jenkins). Compiling gsm0808.c works
only because the actual coding of struct sockaddr_storage is implemented in
gsm0808_util.c, which (apparently) does not get built on embedded and hence,
even though there are undefined references to e.g.
gsm0808_enc_aoip_trasp_addr() it works.

Related: I4a5acdb2d4a0b947cc0c62067a67be88a3d467ff (osmo-bsc)
Change-Id: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
---
M include/osmocom/gsm/gsm0808.h
M src/gsm/gsm0808.c
M src/gsm/libosmogsm.map
3 files changed, 53 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/59/13259/6
--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 6
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-03-14 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/13259

to look at the new patch set (#5).

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..

add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

osmo-bsc so far omits the AoIP Transport Layer Address from its Handover
Request Acknowledge message, which breaks inter-BSC Handover for AoIP.
Allow fixing that.

Related: I4a5acdb2d4a0b947cc0c62067a67be88a3d467ff (osmo-bsc)
Change-Id: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
---
M include/osmocom/gsm/gsm0808.h
M src/gsm/gsm0808.c
M src/gsm/libosmogsm.map
3 files changed, 53 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/59/13259/5
--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 5
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-03-14 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/13259

to look at the new patch set (#4).

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..

add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

osmo-bsc so far omits the AoIP Transport Layer Address from its Handover
Request Acknowledge message, which breaks inter-BSC Handover for AoIP.
Allow fixing that.

Related: I4a5acdb2d4a0b947cc0c62067a67be88a3d467ff (osmo-bsc)
Change-Id: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
---
M include/osmocom/gsm/gsm0808.h
M include/osmocom/gsm/gsm_utils.h
M src/gsm/gsm0808.c
M src/gsm/libosmogsm.map
4 files changed, 56 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/59/13259/4
--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 4
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-03-14 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/13259

to look at the new patch set (#3).

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..

add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

osmo-bsc so far omits the AoIP Transport Layer Address from its Handover
Request Acknowledge message, which breaks inter-BSC Handover for AoIP.
Allow fixing that.

Related: I4a5acdb2d4a0b947cc0c62067a67be88a3d467ff (osmo-bsc)
Change-Id: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
---
M include/osmocom/gsm/gsm0808.h
M src/gsm/gsm0808.c
M src/gsm/libosmogsm.map
3 files changed, 56 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/59/13259/3
--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-03-14 Thread Neels Hofmeyr
Hello Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/13259

to look at the new patch set (#2).

Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..

add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

osmo-bsc so far omits the AoIP Transport Layer Address from its Handover
Request Acknowledge message, which breaks inter-BSC Handover for AoIP.
Allow fixing that.

Related: I4a5acdb2d4a0b947cc0c62067a67be88a3d467ff (osmo-bsc)
Change-Id: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
---
M include/osmocom/gsm/gsm0808.h
M src/gsm/gsm0808.c
M src/gsm/libosmogsm.map
3 files changed, 54 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/59/13259/2
--
To view, visit https://gerrit.osmocom.org/13259
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: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
Gerrit-Change-Number: 13259
Gerrit-PatchSet: 2
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder (102)


Change in libosmocore[master]: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

2019-03-14 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13259


Change subject: add gsm0808_create_handover_request_ack2 to add AoIP RTP addr
..

add gsm0808_create_handover_request_ack2 to add AoIP RTP addr

Change-Id: Ia71542ea37d4fd2c9fb9b40357db7aeb111ec576
---
M include/osmocom/gsm/gsm0808.h
M src/gsm/gsm0808.c
M src/gsm/libosmogsm.map
3 files changed, 54 insertions(+), 9 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/59/13259/1

diff --git a/include/osmocom/gsm/gsm0808.h b/include/osmocom/gsm/gsm0808.h
index a1345c3..b31a698 100644
--- a/include/osmocom/gsm/gsm0808.h
+++ b/include/osmocom/gsm/gsm0808.h
@@ -143,6 +143,26 @@
 };
 struct msgb *gsm0808_create_handover_required(const struct 
gsm0808_handover_required *params);

+struct gsm0808_create_handover_request_ack {
+   const uint8_t *l3_info;
+   uint8_t l3_info_len;
+
+   bool chosen_channel_present;
+   uint8_t chosen_channel;
+
+   /*! For A5/N set chosen_encr_alg = N+1, e.g. chosen_encr_alg = 1 means 
A5/0 (no encryption), 2 means A5/1, 4
+* means A5/3. Set chosen_encr_alg = 0 to omit the Chosen Encryption 
Algorithm IE. */
+   uint8_t chosen_encr_alg;
+
+   /* chosen_speech_version == 0 omits the IE */
+   uint8_t chosen_speech_version;
+
+   struct sockaddr_storage aoip_transport_layer;
+
+   /* more items are defined in the spec and may be added later */
+   bool more_items; /*!< always set this to false */
+};
+struct msgb *gsm0808_create_handover_request_ack2(const struct 
gsm0808_create_handover_request_ack *params);
 struct msgb *gsm0808_create_handover_request_ack(const uint8_t *l3_info, 
uint8_t l3_info_len,
 uint8_t chosen_channel, 
uint8_t chosen_encr_alg,
 uint8_t chosen_speech_version);
diff --git a/src/gsm/gsm0808.c b/src/gsm/gsm0808.c
index 4873076..ad51b5f 100644
--- a/src/gsm/gsm0808.c
+++ b/src/gsm/gsm0808.c
@@ -860,9 +860,7 @@
 /*! Create BSSMAP HANDOVER REQUEST ACKNOWLEDGE message, 3GPP TS 48.008 
3.2.1.10.
  * Sent from the MT BSC back to the MSC when it has allocated an lchan to 
handover to.
  * l3_info is the RR Handover Command that the MO BSC sends to the MS to move 
over. */
-struct msgb *gsm0808_create_handover_request_ack(const uint8_t *l3_info, 
uint8_t l3_info_len,
-uint8_t chosen_channel, 
uint8_t chosen_encr_alg,
-uint8_t chosen_speech_version)
+struct msgb *gsm0808_create_handover_request_ack2(const struct 
gsm0808_create_handover_request_ack *params)
 {
struct msgb *msg;

@@ -873,13 +871,22 @@
/* Message Type, 3.2.2.1 */
msgb_v_put(msg, BSS_MAP_MSG_HANDOVER_RQST_ACKNOWLEDGE);

-   /* Layer 3 Information, 3.2.2.24 */
-   msgb_tlv_put(msg, GSM0808_IE_LAYER_3_INFORMATION, l3_info_len, l3_info);
+   /* Layer 3 Information, 3.2.2.24 -- it is actually mandatory, but 
rather compose a nonstandard message than
+* segfault or return NULL without a log message. */
+   if (params->l3_info && params->l3_info_len)
+   msgb_tlv_put(msg, GSM0808_IE_LAYER_3_INFORMATION, 
params->l3_info_len, params->l3_info);

-   msgb_tv_put(msg, GSM0808_IE_CHOSEN_CHANNEL, chosen_channel);
-   msgb_tv_put(msg, GSM0808_IE_CHOSEN_ENCR_ALG, chosen_encr_alg);
-   if (chosen_speech_version != 0)
-   msgb_tv_put(msg, GSM0808_IE_SPEECH_VERSION, 
chosen_speech_version);
+   if (params->chosen_channel_present)
+   msgb_tv_put(msg, GSM0808_IE_CHOSEN_CHANNEL, 
params->chosen_channel);
+   if (params->chosen_encr_alg)
+   msgb_tv_put(msg, GSM0808_IE_CHOSEN_ENCR_ALG, 
params->chosen_encr_alg);
+
+   if (params->chosen_speech_version != 0)
+   msgb_tv_put(msg, GSM0808_IE_SPEECH_VERSION, 
params->chosen_speech_version);
+
+   if (params->aoip_transport_layer.ss_family == AF_INET
+   || params->aoip_transport_layer.ss_family == AF_INET6)
+   gsm0808_enc_aoip_trasp_addr(msg, >aoip_transport_layer);

/* prepend header with final length */
msg->l3h = msgb_tv_push(msg, BSSAP_MSG_BSS_MANAGEMENT, 
msgb_length(msg));
@@ -887,6 +894,23 @@
return msg;
 }

+/*! Same as gsm0808_create_handover_request_ack2() but with less parameters.
+ * In particular, this lacks the AoIP Transport Layer address. */
+struct msgb *gsm0808_create_handover_request_ack(const uint8_t *l3_info, 
uint8_t l3_info_len,
+uint8_t chosen_channel, 
uint8_t chosen_encr_alg,
+uint8_t chosen_speech_version)
+{
+   struct gsm0808_create_handover_request_ack params = {
+   .l3_info = l3_info,
+   .l3_info_len = l3_info_len,
+