[PATCH 1/9] Complete definitions for all speech traffic frames at MNCC interface
The new definitions are: half rate and AMR Change of definition name for bad frame, because it applies to all types of traffic, not only TCH/F. Increase MNCC interface version to 4. --- openbsc/include/openbsc/mncc.h | 6 ++++-- openbsc/src/libmsc/mncc.c | 6 +++++- openbsc/src/libtrau/trau_mux.c | 2 +- openbsc/tests/trau/trau_test.c | 2 +- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/openbsc/include/openbsc/mncc.h b/openbsc/include/openbsc/mncc.h index ffc247b..c61f6b8 100644 --- a/openbsc/include/openbsc/mncc.h +++ b/openbsc/include/openbsc/mncc.h @@ -95,7 +95,9 @@ struct gsm_call { #define GSM_TCHF_FRAME 0x0300 #define GSM_TCHF_FRAME_EFR 0x0301 -#define GSM_TCHF_BAD_FRAME 0x03ff +#define GSM_TCHH_FRAME 0x0302 +#define GSM_TCH_FRAME_AMR 0x0303 +#define GSM_BAD_FRAME 0x03ff #define MNCC_SOCKET_HELLO 0x0400 @@ -161,7 +163,7 @@ struct gsm_data_frame { unsigned char data[0]; }; -#define MNCC_SOCK_VERSION 2 +#define MNCC_SOCK_VERSION 4 struct gsm_mncc_hello { uint32_t msg_type; uint32_t version; diff --git a/openbsc/src/libmsc/mncc.c b/openbsc/src/libmsc/mncc.c index b484772..73db5f0 100644 --- a/openbsc/src/libmsc/mncc.c +++ b/openbsc/src/libmsc/mncc.c @@ -84,7 +84,11 @@ static struct mncc_names { {"MNCC_FRAME_DROP", 0x0202}, {"MNCC_LCHAN_MODIFY", 0x0203}, - {"GSM_TCH_FRAME", 0x0300}, + {"GSM_TCHF_FRAME", 0x0300}, + {"GSM_TCHF_FRAME_EFR", 0x0301}, + {"GSM_TCHH_FRAME", 0x0302}, + {"GSM_TCH_FRAME_AMR", 0x0303}, + {"GSM_BAD_FRAME", 0x03ff}, {NULL, 0} }; diff --git a/openbsc/src/libtrau/trau_mux.c b/openbsc/src/libtrau/trau_mux.c index 7b9bac0..fd1895f 100644 --- a/openbsc/src/libtrau/trau_mux.c +++ b/openbsc/src/libtrau/trau_mux.c @@ -314,7 +314,7 @@ struct msgb *trau_decode_efr(uint32_t callref, return msg; bad_frame: - frame->msg_type = GSM_TCHF_BAD_FRAME; + frame->msg_type = GSM_BAD_FRAME; return msg; } diff --git a/openbsc/tests/trau/trau_test.c b/openbsc/tests/trau/trau_test.c index f8a48db..b95f1e8 100644 --- a/openbsc/tests/trau/trau_test.c +++ b/openbsc/tests/trau/trau_test.c @@ -57,7 +57,7 @@ void test_trau_fr_efr(unsigned char *data) msg = trau_decode_efr(1, &tf); OSMO_ASSERT(msg != NULL); frame = (struct gsm_data_frame *)msg->data; - OSMO_ASSERT(frame->msg_type == GSM_TCHF_BAD_FRAME); + OSMO_ASSERT(frame->msg_type == GSM_BAD_FRAME); msgb_free(msg); } -- 1.8.1.5
Rename method mncc_rcv_tchf() to mncc_rcv_data(), because the check applies to all types of data frames, not only TCH/F data. --- openbsc/include/openbsc/mncc.h | 8 ++++++++ openbsc/src/libmsc/mncc_builtin.c | 22 ++++++++-------------- openbsc/src/libmsc/mncc_sock.c | 3 +-- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/openbsc/include/openbsc/mncc.h b/openbsc/include/openbsc/mncc.h index c61f6b8..ffac7fd 100644 --- a/openbsc/include/openbsc/mncc.h +++ b/openbsc/include/openbsc/mncc.h @@ -191,4 +191,12 @@ int mncc_sock_from_cc(struct gsm_network *net, struct msgb *msg); int mncc_sock_init(struct gsm_network *gsmnet); +#define mncc_is_data_frame(msg_type) \ + (msg_type == GSM_TCHF_FRAME \ + || msg_type == GSM_TCHF_FRAME_EFR \ + || msg_type == GSM_TCHH_FRAME \ + || msg_type == GSM_TCH_FRAME_AMR \ + || msg_type == GSM_BAD_FRAME) + + #endif diff --git a/openbsc/src/libmsc/mncc_builtin.c b/openbsc/src/libmsc/mncc_builtin.c index be35454..a5a463b 100644 --- a/openbsc/src/libmsc/mncc_builtin.c +++ b/openbsc/src/libmsc/mncc_builtin.c @@ -273,8 +273,8 @@ static int mncc_rel_cnf(struct gsm_call *call, int msg_type, struct gsm_mncc *re return 0; } -/* receiving a TCH/F frame from the BSC code */ -static int mncc_rcv_tchf(struct gsm_call *call, int msg_type, +/* receiving a (speech) traffic frame from the BSC code */ +static int mncc_rcv_data(struct gsm_call *call, int msg_type, struct gsm_data_frame *dfr) { struct gsm_trans *remote_trans; @@ -339,16 +339,14 @@ int int_mncc_recv(struct gsm_network *net, struct msgb *msg) DEBUGP(DMNCC, "(call %x) Call created.\n", call->callref); } - switch (msg_type) { - case GSM_TCHF_FRAME: - case GSM_TCHF_FRAME_EFR: - break; - default: - DEBUGP(DMNCC, "(call %x) Received message %s\n", call->callref, - get_mncc_name(msg_type)); - break; + if (mncc_is_data_frame(msg_type)) { + rc = mncc_rcv_data(call, msg_type, arg); + goto out_free; } + DEBUGP(DMNCC, "(call %x) Received message %s\n", call->callref, + get_mncc_name(msg_type)); + switch(msg_type) { case MNCC_SETUP_IND: rc = mncc_setup_ind(call, msg_type, arg); @@ -408,10 +406,6 @@ int int_mncc_recv(struct gsm_network *net, struct msgb *msg) call->callref, data->cause.value); rc = mncc_tx_to_cc(net, MNCC_RETRIEVE_REJ, data); break; - case GSM_TCHF_FRAME: - case GSM_TCHF_FRAME_EFR: - rc = mncc_rcv_tchf(call, msg_type, arg); - break; default: LOGP(DMNCC, LOGL_NOTICE, "(call %x) Message unhandled\n", callref); break; diff --git a/openbsc/src/libmsc/mncc_sock.c b/openbsc/src/libmsc/mncc_sock.c index cf4bca8..dd0a44f 100644 --- a/openbsc/src/libmsc/mncc_sock.c +++ b/openbsc/src/libmsc/mncc_sock.c @@ -54,8 +54,7 @@ int mncc_sock_from_cc(struct gsm_network *net, struct msgb *msg) if (net->mncc_state->conn_bfd.fd < 0) { LOGP(DMNCC, LOGL_ERROR, "mncc_sock receives %s for external CC app " "but socket is gone\n", get_mncc_name(msg_type)); - if (msg_type != GSM_TCHF_FRAME && - msg_type != GSM_TCHF_FRAME_EFR) { + if (!mncc_is_data_frame(msg_type)) { /* release the request */ struct gsm_mncc mncc_out; memset(&mncc_out, 0, sizeof(mncc_out)); -- 1.8.1.5
Holger Hans Peter Freyther wrote:
+#define mncc_is_data_frame(msg_type) \ Would calling this mncc_is_audio_frame make sense?
holger i thought about it. the reason i call it "data_frame" is because it would also apply to CSD frames that are not (yet) supported.
--- openbsc/src/libmsc/gsm_04_08.c | 6 ++++-- openbsc/src/libtrau/rtp_proxy.c | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index cd2a0b5..df93433 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -2952,6 +2952,7 @@ int mncc_tx_to_cc(struct gsm_network *net, int msg_type, void *arg) return tch_recv_mncc(net, data->callref, 1); case GSM_TCHF_FRAME: case GSM_TCHF_FRAME_EFR: + case GSM_TCHH_FRAME: /* Find callref */ trans = trans_find_by_callref(net, data->callref); if (!trans) { @@ -2963,11 +2964,12 @@ int mncc_tx_to_cc(struct gsm_network *net, int msg_type, void *arg) LOGP(DMNCC, LOGL_NOTICE, "TCH frame for trans without conn\n"); return 0; } - if (trans->conn->lchan->type != GSM_LCHAN_TCH_F) { + if (trans->conn->lchan->type != GSM_LCHAN_TCH_F + && trans->conn->lchan->type != GSM_LCHAN_TCH_H) { /* This should be LOGL_ERROR or NOTICE, but * unfortuantely it happens for a couple of frames at * the beginning of every RTP connection */ - LOGP(DMNCC, LOGL_DEBUG, "TCH frame for lchan != TCH_F\n"); + LOGP(DMNCC, LOGL_DEBUG, "TCH frame for lchan != TCH_F/TCH_H\n"); return 0; } bts = trans->conn->lchan->ts->trx->bts; diff --git a/openbsc/src/libtrau/rtp_proxy.c b/openbsc/src/libtrau/rtp_proxy.c index 94a5b2f..143bfa0 100644 --- a/openbsc/src/libtrau/rtp_proxy.c +++ b/openbsc/src/libtrau/rtp_proxy.c @@ -183,6 +183,15 @@ static int rtp_decode(struct msgb *msg, uint32_t callref, struct msgb **data) return -EINVAL; } break; + case RTP_PT_GSM_HALF: + msg_type = GSM_TCHH_FRAME; + if (payload_len != RTP_LEN_GSM_HALF) { + DEBUGPC(DLMUX, "received RTP half rate frame with " + "payload length != %d (len = %d)\n", + RTP_LEN_GSM_HALF, payload_len); + return -EINVAL; + } + break; default: DEBUGPC(DLMUX, "received RTP frame with unknown payload " "type %d\n", rtph->payload_type); @@ -250,6 +259,11 @@ int rtp_send_frame(struct rtp_socket *rs, struct gsm_data_frame *frame) payload_len = RTP_LEN_GSM_EFR; duration = RTP_GSM_DURATION; break; + case GSM_TCHH_FRAME: + payload_type = RTP_PT_GSM_HALF; + payload_len = RTP_LEN_GSM_HALF; + duration = RTP_GSM_DURATION; + break; default: DEBUGPC(DLMUX, "unsupported message type %d\n", frame->msg_type); -- 1.8.1.5
AMR rate is currently fixed to 5.9k. --- openbsc/src/libmsc/gsm_04_08.c | 1 + openbsc/src/libtrau/rtp_proxy.c | 24 +++++++++++++++++++----- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index df93433..a0650ab 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -2953,6 +2953,7 @@ int mncc_tx_to_cc(struct gsm_network *net, int msg_type, void *arg) case GSM_TCHF_FRAME: case GSM_TCHF_FRAME_EFR: case GSM_TCHH_FRAME: + case GSM_TCH_FRAME_AMR: /* Find callref */ trans = trans_find_by_callref(net, data->callref); if (!trans) { diff --git a/openbsc/src/libtrau/rtp_proxy.c b/openbsc/src/libtrau/rtp_proxy.c index 143bfa0..9c16a95 100644 --- a/openbsc/src/libtrau/rtp_proxy.c +++ b/openbsc/src/libtrau/rtp_proxy.c @@ -116,6 +116,7 @@ static int rtp_decode(struct msgb *msg, uint32_t callref, struct msgb **data) int payload_len; int msg_type; int x_len; + int is_amr = 0; if (msg->len < 12) { DEBUGPC(DLMUX, "received RTP frame too short (len = %d)\n", @@ -192,21 +193,26 @@ static int rtp_decode(struct msgb *msg, uint32_t callref, struct msgb **data) return -EINVAL; } break; + case RTP_PT_AMR: + is_amr = 1; + break; default: DEBUGPC(DLMUX, "received RTP frame with unknown payload " "type %d\n", rtph->payload_type); return -EINVAL; } - new_msg = msgb_alloc(sizeof(struct gsm_data_frame) + payload_len, - "GSM-DATA"); + new_msg = msgb_alloc(sizeof(struct gsm_data_frame) + payload_len + + is_amr, "GSM-DATA"); if (!new_msg) return -ENOMEM; frame = (struct gsm_data_frame *)(new_msg->data); frame->msg_type = msg_type; frame->callref = callref; - memcpy(frame->data, payload, payload_len); - msgb_put(new_msg, sizeof(struct gsm_data_frame) + payload_len); + if (is_amr) + frame->data[0] = payload_len; + memcpy(frame->data + is_amr, payload, payload_len); + msgb_put(new_msg, sizeof(struct gsm_data_frame) + is_amr + payload_len); *data = new_msg; return 0; @@ -239,6 +245,7 @@ int rtp_send_frame(struct rtp_socket *rs, struct gsm_data_frame *frame) int payload_type; int payload_len; int duration; /* in samples */ + int is_amr = 0; if (rs->tx_action != RTP_SEND_DOWNSTREAM) { /* initialize sequences */ @@ -264,6 +271,12 @@ int rtp_send_frame(struct rtp_socket *rs, struct gsm_data_frame *frame) payload_len = RTP_LEN_GSM_HALF; duration = RTP_GSM_DURATION; break; + case GSM_TCH_FRAME_AMR: + payload_type = RTP_PT_AMR; + payload_len = frame->data[0]; + duration = RTP_GSM_DURATION; + is_amr = 1; + break; default: DEBUGPC(DLMUX, "unsupported message type %d\n", frame->msg_type); @@ -305,7 +318,8 @@ int rtp_send_frame(struct rtp_socket *rs, struct gsm_data_frame *frame) rtph->timestamp = htonl(rs->transmit.timestamp); rs->transmit.timestamp += duration; rtph->ssrc = htonl(rs->transmit.ssrc); - memcpy(msg->data + sizeof(struct rtp_hdr), frame->data, payload_len); + memcpy(msg->data + sizeof(struct rtp_hdr), frame->data + is_amr, + payload_len); msgb_put(msg, sizeof(struct rtp_hdr) + payload_len); msgb_enqueue(&rss->tx_queue, msg); rss->bfd.when |= BSC_FD_WRITE; -- 1.8.1.5
On Wed, Feb 19, 2014 at 07:57:53AM +0100, Andreas Eversberg wrote: The patch does not do what the subject is saying. This implements the AMR frame handling. It is a bit unfortunate that NITB is now announcing version4 and not all new features are implemented yet. This somehow defeats the purpose of versioning the protocol. I don't know if you remember but I added the version number as I was trying to figure out why NITB/LCR didn't work together...
+ int is_amr = 0;
by your choice of name you indicate this is a boolean.
+ if (is_amr) + frame->data[0] = payload_len; + memcpy(frame->data + is_amr, payload, payload_len); + msgb_put(new_msg, sizeof(struct gsm_data_frame) + is_amr + payload_len);
here you use it like an int/offset. What about using the msgg routines from the tlv.h code to append the value and data?
+ int is_amr = 0;
same
+ memcpy(msg->data + sizeof(struct rtp_hdr), frame->data + is_amr, + payload_len);
same issue here with boolean.
On Thu, Mar 20, 2014 at 10:46:03PM +0100, Holger Hans Peter Freyther wrote: dear andreas, do you intend to finish your patches? happy easter holger
Holger Hans Peter Freyther wrote:
On Thu, Mar 20, 2014 at 10:46:03PM +0100, Holger Hans Peter Freyther wrote:
dear andreas,
do you intend to finish your patches?
happy easter holger dear holger,
i had that patch done already. (see attachment) happy easter andreas
On Fri, Apr 18, 2014 at 12:07:51PM +0200, Andreas Eversberg wrote: hi,
i had that patch done already. (see attachment)
what was the message id? I didn't see it.
happy easter
happy easter to you too.
+ if (rtph->payload_type == RTP_PT_AMR) { + new_msg = msgb_alloc(sizeof(struct gsm_data_frame) + 1 + + payload_len, "GSM-DATA"); + } else { + new_msg = msgb_alloc(sizeof(struct gsm_data_frame) + + payload_len, "GSM-DATA"); + }
I think the coding style asks us to ommit the {} here. Maybe use different strings too in case we search for a memory leak?
@@ -305,7 +324,13 @@ int rtp_send_frame(struct rtp_socket *rs, struct gsm_data_frame *frame) rtph->timestamp = htonl(rs->transmit.timestamp); rs->transmit.timestamp += duration; rtph->ssrc = htonl(rs->transmit.ssrc); - memcpy(msg->data + sizeof(struct rtp_hdr), frame->data, payload_len); + if (frame->msg_type == GSM_TCH_FRAME_AMR) { + memcpy(msg->data + sizeof(struct rtp_hdr), frame->data + 1, + payload_len); + } else { + memcpy(msg->data + sizeof(struct rtp_hdr), frame->data, + payload_len); + }
This lacks input validation. The code needs to check that the data we read is within the bounds of the msgb and the data we write is within the bounds too.
On Sun, Apr 20, 2014 at 04:30:20PM +0200, Holger Hans Peter Freyther wrote: ping?
i had that patch done already. (see attachment)
what was the message id? I didn't see it.
Could you please answer this one?
This lacks input validation. The code needs to check that the data we read is within the bounds of the msgb and the data we write is within the bounds too.
Do you understand the severity? It is this kind of issue that OpenSSL had with hearbleed. In this case our length is only a uint8_t and our msgb is most likely over-allocated so we might be lucky that nothing else will be leaked from the application. holger
i had that patch done already. (see attachment) what was the message id? I didn't see it. Could you please answer this one? hi holger,
sorry, i did not express myself well enough. i meant that i already wrote the patch, but did not send it to the mailing list.
This lacks input validation. The code needs to check that the data we read is within the bounds of the msgb and the data we write is within the bounds too. i added a check that limits the GSM frames to 33 octets (full speech). (AMR requires only 31 octets + 1 octet length indicator.) an MNCC message has much larger msgb allocation when received. see attachment.
best regards andreas
If a bad TRAU frame is received, it is forwarded to MNCC application as GSM_BAD_FRAME. The application can now handle the GAP of missing audio. (e.g. by extrapolation) If TRAU frames are forwarded via RTP, bad frames are dropped, but frame counter and timestamp of RTP sender state is incremented. --- openbsc/src/libtrau/rtp_proxy.c | 15 ++++++++++++++- openbsc/src/libtrau/trau_mux.c | 5 ++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/openbsc/src/libtrau/rtp_proxy.c b/openbsc/src/libtrau/rtp_proxy.c index 9c16a95..22d135c 100644 --- a/openbsc/src/libtrau/rtp_proxy.c +++ b/openbsc/src/libtrau/rtp_proxy.c @@ -245,7 +245,7 @@ int rtp_send_frame(struct rtp_socket *rs, struct gsm_data_frame *frame) int payload_type; int payload_len; int duration; /* in samples */ - int is_amr = 0; + int is_amr = 0, is_bfi = 0; if (rs->tx_action != RTP_SEND_DOWNSTREAM) { /* initialize sequences */ @@ -277,6 +277,12 @@ int rtp_send_frame(struct rtp_socket *rs, struct gsm_data_frame *frame) duration = RTP_GSM_DURATION; is_amr = 1; break; + case GSM_BAD_FRAME: + payload_type = 0; + payload_len = 0; + duration = RTP_GSM_DURATION; + is_bfi = 1; + break; default: DEBUGPC(DLMUX, "unsupported message type %d\n", frame->msg_type); @@ -304,6 +310,13 @@ int rtp_send_frame(struct rtp_socket *rs, struct gsm_data_frame *frame) } } + if (is_bfi) { + /* In case of a bad frame, just count and drop packt. */ + rs->transmit.timestamp += duration; + rs->transmit.sequence++; + return 0; + } + msg = msgb_alloc(sizeof(struct rtp_hdr) + payload_len, "RTP-GSM-FULL"); if (!msg) return -ENOMEM; diff --git a/openbsc/src/libtrau/trau_mux.c b/openbsc/src/libtrau/trau_mux.c index fd1895f..bb513cc 100644 --- a/openbsc/src/libtrau/trau_mux.c +++ b/openbsc/src/libtrau/trau_mux.c @@ -242,7 +242,10 @@ struct msgb *trau_decode_fr(uint32_t callref, i++; j++; } - frame->msg_type = GSM_TCHF_FRAME; + if (tf->c_bits[11]) /* BFI */ + frame->msg_type = GSM_BAD_FRAME; + else + frame->msg_type = GSM_TCHF_FRAME; frame->callref = callref; msgb_put(msg, sizeof(struct gsm_data_frame) + 33); -- 1.8.1.5
Instead of forwarding traffic through MNCC interface, traffic can be forwarded to a given RTP peer directly. A special MNCC message is used to control the peer's destination. The traffic can still be forwarded through MNCC interface when this special MNCC message is not used. It also works with E1 based BTSs. In conjunction with LCR's "rtp-bridge" feature, the RTP traffic can be directly exchanged with a remote SIP endpoint, so that the traffic is not forwarded by LCR itself. This way the performance of handling traffic only depends on OpenBSC and the remote SIP endpoint. Also the traffic is exchanged with the SIP endpoint without transcoding, to have maximum performance. Increment MNCC version to 5. --- openbsc/include/openbsc/gsm_04_08.h | 3 + openbsc/include/openbsc/mncc.h | 12 +- openbsc/include/openbsc/rtp_proxy.h | 5 +- openbsc/include/openbsc/transaction.h | 2 + openbsc/src/ipaccess/ipaccess-config.c | 6 + openbsc/src/libbsc/bsc_api.c | 1 + openbsc/src/libbsc/handover_logic.c | 1 + openbsc/src/libmsc/gsm_04_08.c | 199 ++++++++++++++++++++++++++------- openbsc/src/libmsc/mncc_sock.c | 14 +++ openbsc/src/libmsc/transaction.c | 1 + openbsc/src/libtrau/rtp_proxy.c | 20 +++- openbsc/src/libtrau/trau_mux.c | 14 ++- openbsc/src/utils/bs11_config.c | 6 + openbsc/tests/abis/abis_test.c | 6 + openbsc/tests/gbproxy/gbproxy_test.c | 6 + 15 files changed, 253 insertions(+), 43 deletions(-) diff --git a/openbsc/include/openbsc/gsm_04_08.h b/openbsc/include/openbsc/gsm_04_08.h index 8df7b73..93348d1 100644 --- a/openbsc/include/openbsc/gsm_04_08.h +++ b/openbsc/include/openbsc/gsm_04_08.h @@ -6,6 +6,7 @@ #include <osmocom/gsm/protocol/gsm_04_08.h> #include <openbsc/meas_rep.h> +#include <openbsc/mncc.h> struct msgb; struct gsm_bts; @@ -75,4 +76,6 @@ void gsm48_lchan2chan_desc(struct gsm48_chan_desc *cd, void release_security_operation(struct gsm_subscriber_connection *conn); void allocate_security_operation(struct gsm_subscriber_connection *conn); +int tch_frame_down(struct gsm_network *net, uint32_t callref, struct gsm_data_frame *data); + #endif diff --git a/openbsc/include/openbsc/mncc.h b/openbsc/include/openbsc/mncc.h index ffac7fd..32a60e9 100644 --- a/openbsc/include/openbsc/mncc.h +++ b/openbsc/include/openbsc/mncc.h @@ -92,6 +92,9 @@ struct gsm_call { #define MNCC_FRAME_RECV 0x0201 #define MNCC_FRAME_DROP 0x0202 #define MNCC_LCHAN_MODIFY 0x0203 +#define MNCC_RTP_CREATE 0x0204 +#define MNCC_RTP_CONNECT 0x0205 +#define MNCC_RTP_FREE 0x0206 #define GSM_TCHF_FRAME 0x0300 #define GSM_TCHF_FRAME_EFR 0x0301 @@ -163,7 +166,7 @@ struct gsm_data_frame { unsigned char data[0]; }; -#define MNCC_SOCK_VERSION 4 +#define MNCC_SOCK_VERSION 5 struct gsm_mncc_hello { uint32_t msg_type; uint32_t version; @@ -179,6 +182,13 @@ struct gsm_mncc_hello { uint32_t lchan_type_offset; }; +struct gsm_mncc_rtp { + uint32_t msg_type; + uint32_t callref; + uint32_t ip; + uint16_t port; +}; + char *get_mncc_name(int value); void mncc_set_cause(struct gsm_mncc *data, int loc, int val); void cc_tx_to_mncc(struct gsm_network *net, struct msgb *msg); diff --git a/openbsc/include/openbsc/rtp_proxy.h b/openbsc/include/openbsc/rtp_proxy.h index 52ffefd..a5f6a2b 100644 --- a/openbsc/include/openbsc/rtp_proxy.h +++ b/openbsc/include/openbsc/rtp_proxy.h @@ -40,8 +40,9 @@ enum rtp_rx_action { RTP_NONE, - RTP_PROXY, - RTP_RECV_UPSTREAM, + RTP_PROXY, /* forward from BTS to BTS */ + RTP_RECV_UPSTREAM, /* forward to application */ + RTP_RECV_APP, /* receive RTP frames from application */ }; enum rtp_tx_action { diff --git a/openbsc/include/openbsc/transaction.h b/openbsc/include/openbsc/transaction.h index b6c859c..db3a5cf 100644 --- a/openbsc/include/openbsc/transaction.h +++ b/openbsc/include/openbsc/transaction.h @@ -28,6 +28,7 @@ struct gsm_trans { /* reference from MNCC or other application */ uint32_t callref; + uint32_t callref_keep; /* to remember callref, even if it is removed */ /* if traffic channel receive was requested */ int tch_recv; @@ -46,6 +47,7 @@ struct gsm_trans { int T308_second; /* used to send release again */ struct osmo_timer_list timer; struct gsm_mncc msg; /* stores setup/disconnect/release message */ + struct rtp_socket *rs; /* application traffic via RTP */ } cc; struct { struct gsm411_smc_inst smc_inst; diff --git a/openbsc/src/ipaccess/ipaccess-config.c b/openbsc/src/ipaccess/ipaccess-config.c index c42c7bb..68b9131 100644 --- a/openbsc/src/ipaccess/ipaccess-config.c +++ b/openbsc/src/ipaccess/ipaccess-config.c @@ -87,6 +87,12 @@ static uint8_t prim_oml_attr[] = { 0x95, 0x00, 7, 0x88, 192, 168, 100, 11, 0x00, static uint8_t unit_id_attr[] = { 0x91, 0x00, 9, '2', '3', '4', '2', '/' , '0', '/', '0', 0x00 }; */ +/* dummy function to keep rtp_proxy.c happy */ +int tch_frame_down(struct gsm_network *net, uint32_t callref, struct gsm_data_frame *data) +{ + return 0; +} + extern int ipaccess_fd_cb(struct osmo_fd *bfd, unsigned int what); extern struct e1inp_line_ops ipaccess_e1inp_line_ops; diff --git a/openbsc/src/libbsc/bsc_api.c b/openbsc/src/libbsc/bsc_api.c index e567038..e6629ff 100644 --- a/openbsc/src/libbsc/bsc_api.c +++ b/openbsc/src/libbsc/bsc_api.c @@ -31,6 +31,7 @@ #include <openbsc/handover.h> #include <openbsc/debug.h> #include <openbsc/gsm_04_08.h> +#include <osmocom/abis/trau_frame.h> #include <openbsc/trau_mux.h> #include <osmocom/gsm/protocol/gsm_08_08.h> diff --git a/openbsc/src/libbsc/handover_logic.c b/openbsc/src/libbsc/handover_logic.c index 36a758b..e20d8f7 100644 --- a/openbsc/src/libbsc/handover_logic.c +++ b/openbsc/src/libbsc/handover_logic.c @@ -39,6 +39,7 @@ #include <openbsc/signal.h> #include <osmocom/core/talloc.h> #include <openbsc/transaction.h> +#include <osmocom/abis/trau_frame.h> #include <openbsc/trau_mux.h> struct bsc_handover { diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index a0650ab..ed3785d 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -1359,8 +1359,15 @@ void _gsm48_cc_trans_free(struct gsm_trans *trans) } if (trans->cc.state != GSM_CSTATE_NULL) new_cc_state(trans, GSM_CSTATE_NULL); + /* Be sure to unmap upstream traffic for our callref only. */ if (trans->conn) - trau_mux_unmap(&trans->conn->lchan->ts->e1_link, trans->callref); + trau_mux_unmap(&trans->conn->lchan->ts->e1_link, trans->callref_keep); + + /* free application RTP socket */ + if (trans->cc.rs) { + rtp_socket_free(trans->cc.rs); + trans->cc.rs = NULL; + } } static int gsm48_cc_tx_setup(struct gsm_trans *trans, void *arg); @@ -1478,6 +1485,7 @@ static int switch_for_handover(struct gsm_lchan *old_lchan, new_rs->receive = old_rs->receive; break; case RTP_NONE: + case RTP_RECV_APP: break; } @@ -1702,6 +1710,153 @@ static int tch_recv_mncc(struct gsm_network *net, uint32_t callref, int enable) return 0; } +static int mncc_rtp_create(struct gsm_network *net, uint32_t callref, + struct gsm_trans *trans, struct rtp_socket *rs, + struct gsm_mncc_rtp *mncc) +{ + /* use RTP instead of MNCC socket, for traffic + * open application RTP socket */ + if (rs) { + LOGP(DCC, LOGL_ERROR, "RTP already created.\n"); + return -EIO; + } + rs = trans->cc.rs = rtp_socket_create(); + if (!rs) { + LOGP(DCC, LOGL_ERROR, "RTP socket creation failed.\n"); + /* reply with IP/port = 0 */ + mncc->ip = 0; + mncc->port = 0; + mncc_recvmsg(net, trans, MNCC_RTP_CREATE, + (struct gsm_mncc *)mncc); + return -EIO; + } + rs->rx_action = RTP_RECV_APP; + rs->receive.net = net; + rs->receive.callref = callref; + /* reply with bound IP/port */ + mncc->ip = ntohl(rs->rtp.sin_local.sin_addr.s_addr); + mncc->port = ntohs(rs->rtp.sin_local.sin_port); + mncc_recvmsg(net, trans, MNCC_RTP_CREATE, (struct gsm_mncc *)mncc); + + return 0; +} + +static int mncc_rtp_connect(struct gsm_network *net, struct gsm_trans *trans, + struct rtp_socket *rs, struct gsm_mncc_rtp *mncc) +{ + int rc; + + if (!rs) { + LOGP(DCC, LOGL_ERROR, "RTP not created.\n"); + return -EIO; + } + rc = rtp_socket_connect(trans->cc.rs, mncc->ip, mncc->port); + if (rc < 0) { + LOGP(DCC, LOGL_ERROR, "RTP socket connect failed.\n"); + /* reply with IP/port = 0 */ + mncc->ip = 0; + mncc->port = 0; + mncc_recvmsg(net, trans, MNCC_RTP_CONNECT, + (struct gsm_mncc *)mncc); + return -EIO; + } + /* reply with local IP/port */ + mncc->ip = ntohl(rs->rtp.sin_local.sin_addr.s_addr); + mncc->port = ntohs(rs->rtp.sin_local.sin_port); + mncc_recvmsg(net, trans, MNCC_RTP_CONNECT, (struct gsm_mncc *)mncc); + + return 0; +} + +static int mncc_rtp_free(struct gsm_network *net, struct gsm_trans *trans, + struct rtp_socket *rs, struct gsm_mncc_rtp *mncc) +{ + if (!rs) { + LOGP(DCC, LOGL_ERROR, "RTP not created.\n"); + return -EIO; + } + rtp_socket_free(trans->cc.rs); + trans->cc.rs = NULL; + /* reply */ + mncc_recvmsg(net, trans, MNCC_RTP_FREE, (struct gsm_mncc *)mncc); + + return 0; +} + +/* handle RTP requests of application */ +static int mncc_rtp(struct gsm_network *net, uint32_t callref, + struct gsm_mncc_rtp *mncc) +{ + struct rtp_socket *rs; + struct gsm_trans *trans; + int rc = -EINVAL; + + /* Find callref */ + trans = trans_find_by_callref(net, callref); + if (!trans) { + LOGP(DCC, LOGL_ERROR, "Unknown transaction for callref=%d\n", + callref); + return -EINVAL; + } + + rs = trans->cc.rs; + + switch (mncc->msg_type) { + case MNCC_RTP_CREATE: + rc = mncc_rtp_create(net, callref, trans, rs, mncc); + break; + case MNCC_RTP_CONNECT: + rc = mncc_rtp_connect(net, trans, rs, mncc); + break; + case MNCC_RTP_FREE: + rc = mncc_rtp_free(net, trans, rs, mncc); + break; + } + + return rc; +} + +/* handle tch frame from application */ +int tch_frame_down(struct gsm_network *net, uint32_t callref, struct gsm_data_frame *data) +{ + struct gsm_trans *trans; + struct gsm_bts *bts; + + /* Find callref */ + trans = trans_find_by_callref(net, data->callref); + if (!trans) { + LOGP(DMNCC, LOGL_ERROR, "TCH frame for non-existing trans\n"); + return -EIO; + } + if (!trans->conn) { + LOGP(DMNCC, LOGL_NOTICE, "TCH frame for trans without conn\n"); + return 0; + } + if (trans->conn->lchan->type != GSM_LCHAN_TCH_F + && trans->conn->lchan->type != GSM_LCHAN_TCH_H) { + /* This should be LOGL_ERROR or NOTICE, but + * unfortuantely it happens for a couple of frames at + * the beginning of every RTP connection */ + LOGP(DMNCC, LOGL_DEBUG, "TCH frame for lchan != TCH_F/TCH_H\n"); + return 0; + } + bts = trans->conn->lchan->ts->trx->bts; + if (!is_e1_bts(bts)) { + if (!trans->conn->lchan->abis_ip.rtp_socket) { + DEBUGP(DMNCC, "TCH frame to lchan without RTP connection\n"); + return 0; + } + if (trans->conn->lchan->abis_ip.rtp_socket->receive.callref != callref) { + /* Drop frame, if not our callref. This happens, if + * the call is on hold or retrieved by another + * transaction. */ + return 0; + } + return rtp_send_frame(trans->conn->lchan->abis_ip.rtp_socket, data); + } else + return trau_send_frame(trans->conn->lchan, data); +} + static int gsm48_cc_rx_status_enq(struct gsm_trans *trans, struct msgb *msg) { DEBUGP(DCC, "-> STATUS ENQ\n"); @@ -2937,7 +3092,6 @@ int mncc_tx_to_cc(struct gsm_network *net, int msg_type, void *arg) int i, rc = 0; struct gsm_trans *trans = NULL, *transt; struct gsm_subscriber_connection *conn = NULL; - struct gsm_bts *bts = NULL; struct gsm_mncc *data = arg, rel; DEBUGP(DMNCC, "receive message %s\n", get_mncc_name(msg_type)); @@ -2950,46 +3104,15 @@ int mncc_tx_to_cc(struct gsm_network *net, int msg_type, void *arg) return tch_recv_mncc(net, data->callref, 0); case MNCC_FRAME_RECV: return tch_recv_mncc(net, data->callref, 1); + case MNCC_RTP_CREATE: + case MNCC_RTP_CONNECT: + case MNCC_RTP_FREE: + return mncc_rtp(net, data->callref, (struct gsm_mncc_rtp *) arg); case GSM_TCHF_FRAME: case GSM_TCHF_FRAME_EFR: case GSM_TCHH_FRAME: case GSM_TCH_FRAME_AMR: - /* Find callref */ - trans = trans_find_by_callref(net, data->callref); - if (!trans) { - LOGP(DMNCC, LOGL_ERROR, "TCH frame for non-existing trans\n"); - return -EIO; - } - log_set_context(BSC_CTX_SUBSCR, trans->subscr); - if (!trans->conn) { - LOGP(DMNCC, LOGL_NOTICE, "TCH frame for trans without conn\n"); - return 0; - } - if (trans->conn->lchan->type != GSM_LCHAN_TCH_F - && trans->conn->lchan->type != GSM_LCHAN_TCH_H) { - /* This should be LOGL_ERROR or NOTICE, but - * unfortuantely it happens for a couple of frames at - * the beginning of every RTP connection */ - LOGP(DMNCC, LOGL_DEBUG, "TCH frame for lchan != TCH_F/TCH_H\n"); - return 0; - } - bts = trans->conn->lchan->ts->trx->bts; - switch (bts->type) { - case GSM_BTS_TYPE_NANOBTS: - case GSM_BTS_TYPE_OSMO_SYSMO: - if (!trans->conn->lchan->abis_ip.rtp_socket) { - DEBUGP(DMNCC, "TCH frame to lchan without RTP connection\n"); - return 0; - } - return rtp_send_frame(trans->conn->lchan->abis_ip.rtp_socket, arg); - case GSM_BTS_TYPE_BS11: - case GSM_BTS_TYPE_RBS2000: - case GSM_BTS_TYPE_NOKIA_SITE: - return trau_send_frame(trans->conn->lchan, arg); - default: - LOGP(DCC, LOGL_ERROR, "Unknown BTS type %u\n", bts->type); - } - return -EINVAL; + return tch_frame_down(net, data->callref, (struct gsm_data_frame *) arg); } memset(&rel, 0, sizeof(struct gsm_mncc)); diff --git a/openbsc/src/libmsc/mncc_sock.c b/openbsc/src/libmsc/mncc_sock.c index dd0a44f..adb56fc 100644 --- a/openbsc/src/libmsc/mncc_sock.c +++ b/openbsc/src/libmsc/mncc_sock.c @@ -37,6 +37,8 @@ #include <openbsc/debug.h> #include <openbsc/mncc.h> #include <openbsc/gsm_data.h> +#include <openbsc/transaction.h> +#include <openbsc/rtp_proxy.h> struct mncc_sock_state { struct gsm_network *net; @@ -50,6 +52,18 @@ int mncc_sock_from_cc(struct gsm_network *net, struct msgb *msg) struct gsm_mncc *mncc_in = (struct gsm_mncc *) msgb_data(msg); int msg_type = mncc_in->msg_type; + /* application uses RTP for this transaction, we send our data via RTP, + * otherwise we send it through MNCC interface */ + if (mncc_is_data_frame(msg_type)) { + struct gsm_trans *trans = trans_find_by_callref(net, mncc_in->callref); + + if (trans && trans->cc.rs) { + rtp_send_frame(trans->cc.rs, (struct gsm_data_frame *) mncc_in); + msgb_free(msg); + return 0; + } + } + /* Check if we currently have a MNCC handler connected */ if (net->mncc_state->conn_bfd.fd < 0) { LOGP(DMNCC, LOGL_ERROR, "mncc_sock receives %s for external CC app " diff --git a/openbsc/src/libmsc/transaction.c b/openbsc/src/libmsc/transaction.c index e425e67..bdd3d8e 100644 --- a/openbsc/src/libmsc/transaction.c +++ b/openbsc/src/libmsc/transaction.c @@ -78,6 +78,7 @@ struct gsm_trans *trans_alloc(struct gsm_subscriber *subscr, trans->protocol = protocol; trans->transaction_id = trans_id; trans->callref = callref; + trans->callref_keep = callref; llist_add_tail(&trans->entry, &subscr->net->trans_list); diff --git a/openbsc/src/libtrau/rtp_proxy.c b/openbsc/src/libtrau/rtp_proxy.c index 22d135c..d2bf88d 100644 --- a/openbsc/src/libtrau/rtp_proxy.c +++ b/openbsc/src/libtrau/rtp_proxy.c @@ -473,7 +473,7 @@ static int rtp_socket_read(struct rtp_socket *rs, struct rtp_sub_socket *rss) other_rss->bfd.when |= BSC_FD_WRITE; break; - case RTP_RECV_UPSTREAM: + case RTP_RECV_UPSTREAM: /* from BTS to application */ if (!rs->receive.callref || !rs->receive.net) { rc = -EIO; goto out_free; @@ -502,6 +502,24 @@ static int rtp_socket_read(struct rtp_socket *rs, struct rtp_sub_socket *rss) trau_tx_to_mncc(rs->receive.net, new_msg); break; + case RTP_RECV_APP: /* from remote application */ + if (!rs->receive.callref || !rs->receive.net) { + rc = -EIO; + goto out_free; + } + if (rss->bfd.priv_nr != RTP_PRIV_RTP) { + rc = ENOTSUP; + goto out_free; + } + rc = rtp_decode(msg, rs->receive.callref, &new_msg); + if (rc < 0) + goto out_free; + msgb_free(msg); + tch_frame_down(rs->receive.net, rs->receive.callref, + (struct gsm_data_frame *) new_msg->data); + msgb_free(new_msg); + break; + case RTP_NONE: /* if socket exists, but disabled by app */ msgb_free(msg); break; diff --git a/openbsc/src/libtrau/trau_mux.c b/openbsc/src/libtrau/trau_mux.c index bb513cc..15a84b0 100644 --- a/openbsc/src/libtrau/trau_mux.c +++ b/openbsc/src/libtrau/trau_mux.c @@ -171,7 +171,9 @@ int trau_mux_unmap(const struct gsm_e1_subslot *ss, uint32_t callref) llist_del(&ue->list); return 0; } - if (ss && !memcmp(&ue->src, ss, sizeof(*ss))) { + /* Only release, if no callref is given. We must ensure that + * only the transaction's upstream is removed, if exists. */ + if (ss && !callref && !memcmp(&ue->src, ss, sizeof(*ss))) { llist_del(&ue->list); return 0; } @@ -497,11 +499,21 @@ int trau_send_frame(struct gsm_lchan *lchan, struct gsm_data_frame *frame) uint8_t trau_bits_out[TRAU_FRAME_BITS]; struct gsm_e1_subslot *dst_e1_ss = &lchan->ts->e1_link; struct subch_mux *mx; + struct upqueue_entry *ue; struct decoded_trau_frame tf; mx = e1inp_get_mux(dst_e1_ss->e1_nr, dst_e1_ss->e1_ts); if (!mx) return -EINVAL; + if (!(ue = lookup_trau_upqueue(dst_e1_ss))) { + /* Call might be on hold, so we drop frames. */ + return 0; + } + if (ue->callref != frame->callref) { + /* Slot has different transaction, due to + * another call. (Ours is on hold.) */ + return 0; + } switch (frame->msg_type) { case GSM_TCHF_FRAME: diff --git a/openbsc/src/utils/bs11_config.c b/openbsc/src/utils/bs11_config.c index e8acb46..f459744 100644 --- a/openbsc/src/utils/bs11_config.c +++ b/openbsc/src/utils/bs11_config.c @@ -83,6 +83,12 @@ struct osmo_counter *osmo_counter_alloc(const char *name) return NULL; } +/* dummy function to keep rtp_proxy.c happy */ +int tch_frame_down(struct gsm_network *net, uint32_t callref, struct gsm_data_frame *data) +{ + return 0; +} + int handle_serial_msg(struct msgb *rx_msg); /* create all objects for an initial configuration */ diff --git a/openbsc/tests/abis/abis_test.c b/openbsc/tests/abis/abis_test.c index e7e78d2..6bb84cc 100644 --- a/openbsc/tests/abis/abis_test.c +++ b/openbsc/tests/abis/abis_test.c @@ -27,6 +27,12 @@ #include <openbsc/abis_nm.h> #include <openbsc/debug.h> +/* dummy function to keep rtp_proxy.c happy */ +int tch_frame_down(struct gsm_network *net, uint32_t callref, struct gsm_data_frame *data) +{ + return 0; +} + static const uint8_t simple_config[] = { /*0, 13, */ 66, 18, 0, 3, 1, 2, 3, 19, 0, 3, 3, 4, 5, diff --git a/openbsc/tests/gbproxy/gbproxy_test.c b/openbsc/tests/gbproxy/gbproxy_test.c index d32ac83..69ce58a 100644 --- a/openbsc/tests/gbproxy/gbproxy_test.c +++ b/openbsc/tests/gbproxy/gbproxy_test.c @@ -34,6 +34,12 @@ #define SGSN_NSEI 0x0100 +/* dummy function to keep rtp_proxy.c happy */ +int tch_frame_down() +{ + return 0; +} + struct gbproxy_config gbcfg; static int gprs_process_message(struct gprs_ns_inst *nsi, const char *text, -- 1.8.1.5
On Wed, Feb 19, 2014 at 07:57:55AM +0100, Andreas Eversberg wrote: Dear Andreas, Harald, the osmo-bsc_nat would require a dummy tch_frame_down implementation. The NAT is using libbsc for bsc_msc.c and gsm48_create_mm_serv_rej, gsm48_create_loc_upd_rej, gsm48_extract_mi, gsm48_paging_extract_mi and gsm_net_update_ctype. Currently libbsc depends on libtrau. Can we move the rtp_socket to another library? Or move out the code used by nat to another libbsc? What do you think? holger
+int tch_frame_down(struct gsm_network *net, uint32_t callref, struct gsm_data_frame *data);
+/* dummy function to keep rtp_proxy.c happy */ +int tch_frame_down(struct gsm_network *net, uint32_t callref, struct gsm_data_frame *data) +{ + return 0; +}
+/* dummy function to keep rtp_proxy.c happy */ +int tch_frame_down(struct gsm_network *net, uint32_t callref, struct gsm_data_frame *data) +{ + return 0; +}
+/* dummy function to keep rtp_proxy.c happy */ +int tch_frame_down(struct gsm_network *net, uint32_t callref, struct gsm_data_frame *data) +{ + return 0; +} +
--- a/openbsc/tests/gbproxy/gbproxy_test.c +++ b/openbsc/tests/gbproxy/gbproxy_test.c @@ -34,6 +34,12 @@
#define SGSN_NSEI 0x0100
+/* dummy function to keep rtp_proxy.c happy */ +int tch_frame_down() +{ + return 0; +} +
Holger Hans Peter Freyther wrote:
Currently libbsc depends on libtrau. Can we move the rtp_socket to another library? Or move out the code used by nat to another libbsc?
What do you think? holger
+int tch_frame_down(struct gsm_network *net, uint32_t callref, struct gsm_data_frame *data); dear holger, dear harald,
i think it would make sense to move tch_frame_down() to rtp_proxy.c. at least we could get rid of all the dummy implementations for this function. best regards, andreas
On Tue, Mar 11, 2014 at 07:29:01AM +0100, Andreas Eversberg wrote:
i think it would make sense to move tch_frame_down() to rtp_proxy.c. at least we could get rid of all the dummy implementations for this function.
Good Morning, could you attempt to make a patch like this? The goal is to have the NAT code link without modifications. But we can solve this by spliting out GSM 04.08 message generating/parsing from libbsc as well. holger
Holger Hans Peter Freyther wrote:
On Tue, Mar 11, 2014 at 07:29:01AM +0100, Andreas Eversberg wrote:
i think it would make sense to move tch_frame_down() to rtp_proxy.c. at least we could get rid of all the dummy implementations for this function. Good Morning,
could you attempt to make a patch like this? The goal is to have the NAT code link without modifications. But we can solve this by spliting out GSM 04.08 message generating/parsing from libbsc as well.
holger dear holger,
i tried to move the tch_frame_down() to trau_mux.c. this function requires trans_find_by_callref(), which is part of libmsc. now all programs that link libtrau will also need to be linked to libmsc. best regards, andreas
On Sat, Mar 15, 2014 at 10:41:38AM +0100, Andreas Eversberg wrote: hi,
i tried to move the tch_frame_down() to trau_mux.c. this function requires trans_find_by_callref(), which is part of libmsc. now all programs that link libtrau will also need to be linked to libmsc.
libbsc requires libtrau, libtrau requires libmsc. So the OsmoBSC and the OsmoNAT would now require libmsc. That is an even bigger step backward. :( Can you move the rtp_socket from the libbsc to the libmsc and use the SS_ABISIP signals just like src/osmo-bsc/osmo_bsc_audio.c is doing it? thanks holger
Holger Hans Peter Freyther wrote:
Can you move the rtp_socket from the libbsc to the libmsc and use the SS_ABISIP signals just like src/osmo-bsc/osmo_bsc_audio.c is doing it? sorry, but i don't understand what you mean by moving the rtp_socket. also, what can i use the SS_ABISIP signal for?
On Mon, Mar 17, 2014 at 07:42:08AM +0100, Andreas Eversberg wrote:
Holger Hans Peter Freyther wrote:
Can you move the rtp_socket from the libbsc to the libmsc and use the SS_ABISIP signals just like src/osmo-bsc/osmo_bsc_audio.c is doing it? sorry, but i don't understand what you mean by moving the rtp_socket. also, what can i use the SS_ABISIP signal for?
Do you understand why the bsc can't use/link libmsc and that circular lib depedencies are an issue? I proposed/asked if you could move the rtp_socket into libmsc? The code in src/osmo-bsc/osmo_bsc_audio.c is solving a similar problem that NITB is dealing with rtp_socket.c. So you could introduce a loose coupling here.. through the signal framework. is that more clear? I currently can't merge this patch as it breaks the NAT compilation. holger
On Thu, Mar 20, 2014 at 10:49:38PM +0100, Holger Hans Peter Freyther wrote:
is that more clear? I currently can't merge this patch as it breaks the NAT compilation.
ping? Do you intend to finish your patches?
Holger Hans Peter Freyther wrote:
On Thu, Mar 20, 2014 at 10:49:38PM +0100, Holger Hans Peter Freyther wrote:
is that more clear? I currently can't merge this patch as it breaks the NAT compilation. ping? Do you intend to finish your patches? this patch is not yet tested. it uses signal to remove dependency between libmsc and libbsc. any comments?
On Fri, Apr 18, 2014 at 12:22:31PM +0200, Andreas Eversberg wrote:
S_LCHAN_HANDOVER_FAIL, /* 04.08 Handover Failed */ S_LCHAN_HANDOVER_DETECT, /* 08.58 Handover Detect */ S_LCHAN_MEAS_REP, /* 08.58 Measurement Report */ + S_LCHAN_RTP_SOCKET_FREE,
Use the opportunity to write a bit of information here? Or maybe just use S_CHALLOC_FREED to free rtp_socket and introduce a S_CHALLOC_RESET to do it in the later case?
+ struct lchan_signal_data sig;
please either use memset on sign or the 0 initializer of C99.
diff --git a/openbsc/src/libmsc/rtp_proxy.c b/openbsc/src/libmsc/rtp_proxy.c
Please use the -M option on git format-patch/send-email to properly detect renames. thanks holger
S_LCHAN_HANDOVER_FAIL, /* 04.08 Handover Failed */ S_LCHAN_HANDOVER_DETECT, /* 08.58 Handover Detect */ S_LCHAN_MEAS_REP, /* 08.58 Measurement Report */ + S_LCHAN_RTP_SOCKET_FREE, Use the opportunity to write a bit of information here? Or maybe just use S_CHALLOC_FREED to free rtp_socket and introduce a S_CHALLOC_RESET to do it in the later case?
+ struct lchan_signal_data sig;
dear holger, i chose to use S_CHALLOC_* to trigger RTP socket freeing. see attached patch. best regards andreas
Since EFR/AMR/HR codecs use dynamic RTP payload, the payload type can be set. If it is set, the frame type must be set also, so OpenBSC knows what frame types are received via RTP. This modification only affects traffic beween application and MNCC interface, not the RTP traffic between OpenBSC and BTS. --- openbsc/include/openbsc/mncc.h | 2 ++ openbsc/include/openbsc/rtp_proxy.h | 2 ++ openbsc/src/libmsc/gsm_04_08.c | 2 ++ openbsc/src/libtrau/rtp_proxy.c | 57 ++++++++++++++++++++++++++----------- 4 files changed, 46 insertions(+), 17 deletions(-) diff --git a/openbsc/include/openbsc/mncc.h b/openbsc/include/openbsc/mncc.h index 32a60e9..46ba237 100644 --- a/openbsc/include/openbsc/mncc.h +++ b/openbsc/include/openbsc/mncc.h @@ -187,6 +187,8 @@ struct gsm_mncc_rtp { uint32_t callref; uint32_t ip; uint16_t port; + uint32_t payload_type; + uint32_t payload_msg_type; }; char *get_mncc_name(int value); diff --git a/openbsc/include/openbsc/rtp_proxy.h b/openbsc/include/openbsc/rtp_proxy.h index a5f6a2b..61ad325 100644 --- a/openbsc/include/openbsc/rtp_proxy.h +++ b/openbsc/include/openbsc/rtp_proxy.h @@ -74,6 +74,8 @@ struct rtp_socket { struct { struct gsm_network *net; uint32_t callref; + uint8_t payload_type; /* dynamic PT */ + int msg_type; /* message type for dynamic PT */ } receive; }; enum rtp_tx_action tx_action; diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index ed3785d..663d855 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -1760,6 +1760,8 @@ static int mncc_rtp_connect(struct gsm_network *net, struct gsm_trans *trans, (struct gsm_mncc *)mncc); return -EIO; } + rs->receive.msg_type = mncc->payload_msg_type; + rs->receive.payload_type = mncc->payload_type; /* reply with local IP/port */ mncc->ip = ntohl(rs->rtp.sin_local.sin_addr.s_addr); mncc->port = ntohs(rs->rtp.sin_local.sin_port); diff --git a/openbsc/src/libtrau/rtp_proxy.c b/openbsc/src/libtrau/rtp_proxy.c index d2bf88d..d4dbe02 100644 --- a/openbsc/src/libtrau/rtp_proxy.c +++ b/openbsc/src/libtrau/rtp_proxy.c @@ -106,7 +106,7 @@ struct rtp_x_hdr { #define RTP_VERSION 2 /* decode an rtp frame and create a new buffer with payload */ -static int rtp_decode(struct msgb *msg, uint32_t callref, struct msgb **data) +static int rtp_decode(struct msgb *msg, uint32_t callref, struct msgb **data, int msg_type) { struct msgb *new_msg; struct gsm_data_frame *frame; @@ -114,7 +114,6 @@ static int rtp_decode(struct msgb *msg, uint32_t callref, struct msgb **data) struct rtp_x_hdr *rtpxh; uint8_t *payload; int payload_len; - int msg_type; int x_len; int is_amr = 0; @@ -165,9 +164,31 @@ static int rtp_decode(struct msgb *msg, uint32_t callref, struct msgb **data) } } - switch (rtph->payload_type) { - case RTP_PT_GSM_FULL: - msg_type = GSM_TCHF_FRAME; + /* If no explicit frame type is given, select frame type from + * payload type. */ + if (!msg_type) { + switch (rtph->payload_type) { + case RTP_PT_GSM_FULL: + msg_type = GSM_TCHF_FRAME; + break; + case RTP_PT_GSM_EFR: + msg_type = GSM_TCHF_FRAME_EFR; + break; + case RTP_PT_GSM_HALF: + msg_type = GSM_TCHH_FRAME; + break; + case RTP_PT_AMR: + msg_type = GSM_TCH_FRAME_AMR; + break; + default: + DEBUGPC(DLMUX, "received RTP frame with unknown " + "payload type %d\n", rtph->payload_type); + return -EINVAL; + } + } + + switch (msg_type) { + case GSM_TCHF_FRAME: if (payload_len != RTP_LEN_GSM_FULL) { DEBUGPC(DLMUX, "received RTP full rate frame with " "payload length != %d (len = %d)\n", @@ -175,8 +196,7 @@ static int rtp_decode(struct msgb *msg, uint32_t callref, struct msgb **data) return -EINVAL; } break; - case RTP_PT_GSM_EFR: - msg_type = GSM_TCHF_FRAME_EFR; + case GSM_TCHF_FRAME_EFR: if (payload_len != RTP_LEN_GSM_EFR) { DEBUGPC(DLMUX, "received RTP extended full rate frame " "with payload length != %d (len = %d)\n", @@ -184,8 +204,7 @@ static int rtp_decode(struct msgb *msg, uint32_t callref, struct msgb **data) return -EINVAL; } break; - case RTP_PT_GSM_HALF: - msg_type = GSM_TCHH_FRAME; + case GSM_TCHH_FRAME: if (payload_len != RTP_LEN_GSM_HALF) { DEBUGPC(DLMUX, "received RTP half rate frame with " "payload length != %d (len = %d)\n", @@ -193,12 +212,11 @@ static int rtp_decode(struct msgb *msg, uint32_t callref, struct msgb **data) return -EINVAL; } break; - case RTP_PT_AMR: + case GSM_TCH_FRAME_AMR: is_amr = 1; break; default: - DEBUGPC(DLMUX, "received RTP frame with unknown payload " - "type %d\n", rtph->payload_type); + DEBUGPC(DLMUX, "Forced message type %x unknown\n", msg_type); return -EINVAL; } @@ -246,6 +264,10 @@ int rtp_send_frame(struct rtp_socket *rs, struct gsm_data_frame *frame) int payload_len; int duration; /* in samples */ int is_amr = 0, is_bfi = 0; + uint8_t dynamic_pt = 0; + + if (rs->rx_action == RTP_RECV_APP) + dynamic_pt = rs->receive.payload_type; if (rs->tx_action != RTP_SEND_DOWNSTREAM) { /* initialize sequences */ @@ -262,17 +284,17 @@ int rtp_send_frame(struct rtp_socket *rs, struct gsm_data_frame *frame) duration = RTP_GSM_DURATION; break; case GSM_TCHF_FRAME_EFR: - payload_type = RTP_PT_GSM_EFR; + payload_type = (dynamic_pt) ? : RTP_PT_GSM_EFR; payload_len = RTP_LEN_GSM_EFR; duration = RTP_GSM_DURATION; break; case GSM_TCHH_FRAME: - payload_type = RTP_PT_GSM_HALF; + payload_type = (dynamic_pt) ? : RTP_PT_GSM_HALF; payload_len = RTP_LEN_GSM_HALF; duration = RTP_GSM_DURATION; break; case GSM_TCH_FRAME_AMR: - payload_type = RTP_PT_AMR; + payload_type = (dynamic_pt) ? : RTP_PT_AMR; payload_len = frame->data[0]; duration = RTP_GSM_DURATION; is_amr = 1; @@ -495,7 +517,7 @@ static int rtp_socket_read(struct rtp_socket *rs, struct rtp_sub_socket *rss) rc = -EINVAL; goto out_free; } - rc = rtp_decode(msg, rs->receive.callref, &new_msg); + rc = rtp_decode(msg, rs->receive.callref, &new_msg, 0); if (rc < 0) goto out_free; msgb_free(msg); @@ -511,7 +533,8 @@ static int rtp_socket_read(struct rtp_socket *rs, struct rtp_sub_socket *rss) rc = ENOTSUP; goto out_free; } - rc = rtp_decode(msg, rs->receive.callref, &new_msg); + rc = rtp_decode(msg, rs->receive.callref, &new_msg, + rs->receive.msg_type); if (rc < 0) goto out_free; msgb_free(msg); -- 1.8.1.5
After OpenBSC stalled for some reason (e.g. CPU overload or database access) or after speech frames have been lost (MNCC application problems / hold/retrieve call), the timestamp and the sequence number of the RTP socket state must be corrected. The amount of incrmentation is calculated from the elapsed time. Not incrementing timestamp and sequence number would cause all frames to be dropped by ipaccess BTS, because the BTS expects frames with more recent timestamps. If speech frames are received too fast, they must be dropped. The timestamp and sequence number of the RTP socket state are not changed in this case. Incmenetating timestamp and sequence number would causes high delay at ipaccess BTS, because the BTS would queue the frames until the timestamp matches the current time. There is a simple test case: Make a call between two phones and check the delay. (When using LCR, make a call to the echo test.) The press CTRL+z to suspend OpenBSC process for a few seconds and enter "fg" to continue OpenBSC process. There shall be no change in the delay, even after repeating this test many times. --- openbsc/src/libtrau/rtp_proxy.c | 89 +++++++++++++++++++++++++++-------------- 1 file changed, 60 insertions(+), 29 deletions(-) diff --git a/openbsc/src/libtrau/rtp_proxy.c b/openbsc/src/libtrau/rtp_proxy.c index d4dbe02..ad3dbb6 100644 --- a/openbsc/src/libtrau/rtp_proxy.c +++ b/openbsc/src/libtrau/rtp_proxy.c @@ -236,19 +236,65 @@ static int rtp_decode(struct msgb *msg, uint32_t callref, struct msgb **data, in return 0; } -/* "to - from" */ -static void tv_difference(struct timeval *diff, const struct timeval *from, - const struct timeval *__to) +#define USEC_1S 1000000 +#define USEC_10MS 10000 +#define USEC_20MS 20000 +#define SAMPLES_1S 8000 +#define USEC_SAMPLE 125 + +/* add usec to tv */ +static void tv_add_usec(struct timeval *tv, long int usec) { - struct timeval _to = *__to, *to = &_to; + struct timeval tv_add; - if (to->tv_usec < from->tv_usec) { - to->tv_sec -= 1; - to->tv_usec += 1000000; + tv_add.tv_sec = usec / USEC_1S; + tv_add.tv_usec = usec % USEC_1S; + timeradd(tv, &tv_add, tv); +} + +static int correct_timestamp(struct rtp_socket *rs, int duration) +{ + struct timeval tv, tv_diff; + long int usec_diff, frame_diff; + int usec_duration = duration * USEC_SAMPLE; + + gettimeofday(&tv, NULL); + timersub(&tv, &rs->transmit.last_tv, &tv_diff); + + usec_diff = tv_diff.tv_sec * USEC_1S + tv_diff.tv_usec; + frame_diff = (usec_diff + (usec_duration >> 1)) / usec_duration; /* round */ + + /* Drop frame, if current time to too much in advance of the last_tv. + * < 0 means that the time difference in frames must be at lease 2 + * frames below the expected difference of 1. + */ + if (frame_diff < 0) + return -1; + + /* Increment last_tv by the duration of one frame. */ + tv_add_usec(&rs->transmit.last_tv, usec_duration); + + /* Increment last_tv, if the current time is too much afterwards. + * Also increment timestamp and sequence number of RTP socket state. + * > 2 means that the time difference in frames must be at least 2 + * frames above the expected difference of 1. + */ + if (frame_diff > 2) { + long int frame_diff_excess = frame_diff - 1; + long int sample_diff_excess = frame_diff_excess * duration; + + /* correct last_tv */ + tv_add_usec(&rs->transmit.last_tv, + sample_diff_excess * USEC_SAMPLE); + LOGP(DLMUX, LOGL_NOTICE, + "Correcting timestamp difference of %ld frames " + "(to %s)\n", frame_diff_excess, + (rs->rx_action == RTP_RECV_APP) ? "app" : "BTS"); + rs->transmit.sequence += frame_diff_excess; + rs->transmit.timestamp += sample_diff_excess; } - diff->tv_usec = to->tv_usec - from->tv_usec; - diff->tv_sec = to->tv_sec - from->tv_sec; + return 0; } /*! \brief encode and send a rtp frame @@ -265,6 +311,7 @@ int rtp_send_frame(struct rtp_socket *rs, struct gsm_data_frame *frame) int duration; /* in samples */ int is_amr = 0, is_bfi = 0; uint8_t dynamic_pt = 0; + int rc; if (rs->rx_action == RTP_RECV_APP) dynamic_pt = rs->receive.payload_type; @@ -275,6 +322,7 @@ int rtp_send_frame(struct rtp_socket *rs, struct gsm_data_frame *frame) rs->transmit.ssrc = rand(); rs->transmit.sequence = random(); rs->transmit.timestamp = random(); + gettimeofday(&rs->transmit.last_tv, NULL); } switch (frame->msg_type) { @@ -311,26 +359,9 @@ int rtp_send_frame(struct rtp_socket *rs, struct gsm_data_frame *frame) return -EINVAL; } - { - struct timeval tv, tv_diff; - long int usec_diff, frame_diff; - - gettimeofday(&tv, NULL); - tv_difference(&tv_diff, &rs->transmit.last_tv, &tv); - rs->transmit.last_tv = tv; - - usec_diff = tv_diff.tv_sec * 1000000 + tv_diff.tv_usec; - frame_diff = (usec_diff / 20000); - - if (abs(frame_diff) > 1) { - long int frame_diff_excess = frame_diff - 1; - - LOGP(DLMUX, LOGL_NOTICE, - "Correcting frame difference of %ld frames\n", frame_diff_excess); - rs->transmit.sequence += frame_diff_excess; - rs->transmit.timestamp += frame_diff_excess * duration; - } - } + rc = correct_timestamp(rs, duration); + if (rc) + return 0; if (is_bfi) { /* In case of a bad frame, just count and drop packt. */ -- 1.8.1.5
When reading from RTP socket, the first read() may fail right after connecting to remote socket. Subsequent read() will work as it should. If the remote socket does not open fast enough, the transmitted RTP payload can cause an ICMP (connection refused) packet reply. This causes the read to fail with errno=111. In all other error cases, the errno is logged at debug level. In all error cases, reading is not disabled. --- openbsc/src/libtrau/rtp_proxy.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/openbsc/src/libtrau/rtp_proxy.c b/openbsc/src/libtrau/rtp_proxy.c index ad3dbb6..52abad0 100644 --- a/openbsc/src/libtrau/rtp_proxy.c +++ b/openbsc/src/libtrau/rtp_proxy.c @@ -497,9 +497,16 @@ static int rtp_socket_read(struct rtp_socket *rs, struct rtp_sub_socket *rss) return -ENOMEM; rc = read(rss->bfd.fd, msg->data, RTP_ALLOC_SIZE); - if (rc <= 0) { - rss->bfd.when &= ~BSC_FD_READ; - return rc; + if (rc == 0) + goto out_free; + if (rc < 0) { + /* Ignore "connection refused". this happens, If we open the + * socket faster than the remove side. */ + if (errno == ECONNREFUSED) + goto out_free; + DEBUGPC(DLMUX, "Read of RTP socket (%p) failed (errno %d, " + "%s)\n", rs, errno, strerror(errno)); + goto out_free; } msgb_put(msg, rc); -- 1.8.1.5
Hi Andreas, 19.02.2014, в 10:57, Andreas Eversberg <jolly@eversberg.eu> написал(а):
When reading from RTP socket, the first read() may fail right after connecting to remote socket. Subsequent read() will work as it should.
If the remote socket does not open fast enough, the transmitted RTP payload can cause an ICMP (connection refused) packet reply. This causes the read to fail with errno=111. In all other error cases, the errno is logged at debug level. In all error cases, reading is not disabled. --- openbsc/src/libtrau/rtp_proxy.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/openbsc/src/libtrau/rtp_proxy.c b/openbsc/src/libtrau/rtp_proxy.c index ad3dbb6..52abad0 100644 --- a/openbsc/src/libtrau/rtp_proxy.c +++ b/openbsc/src/libtrau/rtp_proxy.c @@ -497,9 +497,16 @@ static int rtp_socket_read(struct rtp_socket *rs, struct rtp_sub_socket *rss) return -ENOMEM;
rc = read(rss->bfd.fd, msg->data, RTP_ALLOC_SIZE); - if (rc <= 0) { - rss->bfd.when &= ~BSC_FD_READ; - return rc; + if (rc == 0) + goto out_free; + if (rc < 0) { + /* Ignore "connection refused". this happens, If we open the + * socket faster than the remove side. */
Typo? s/remove/remote/
+ if (errno == ECONNREFUSED) + goto out_free; + DEBUGPC(DLMUX, "Read of RTP socket (%p) failed (errno %d, " + "%s)\n", rs, errno, strerror(errno)); + goto out_free; }
msgb_put(msg, rc); -- 1.8.1.5
On Wed, Feb 19, 2014 at 07:57:50AM +0100, Andreas Eversberg wrote:
The new definitions are: half rate and AMR
Change of definition name for bad frame, because it applies to all types of traffic, not only TCH/F.
Increase MNCC interface version to 4.
-#define MNCC_SOCK_VERSION 2 +#define MNCC_SOCK_VERSION 4
Has version 3 three been used somewhere else?
Holger Hans Peter Freyther wrote:
-#define MNCC_SOCK_VERSION 2 +#define MNCC_SOCK_VERSION 4 Has version 3 three been used somewhere else? dear holger,
yes, it was used before i rebased my testing branch. the former version 3 had more features and was incompatible with this version 4. best regards, andreas
On Mon, Mar 10, 2014 at 09:13:30AM +0100, Andreas Eversberg wrote:
yes, it was used before i rebased my testing branch. the former version 3 had more features and was incompatible with this version 4.
This is important information that should be in the commit message. Is the LCR branch that implemented version 3 of the protocol available to the public? How likely is it that somebody will end up using it? holger
participants (4)
-
Alexey Shamrin -
Andreas Eversberg -
Andreas Eversberg -
Holger Hans Peter Freyther