Merge branch 'jerlbeck/mgcp-cleanups'
diff --git a/openbsc/include/openbsc/bsc_nat.h b/openbsc/include/openbsc/bsc_nat.h
index 635b125..150979b 100644
--- a/openbsc/include/openbsc/bsc_nat.h
+++ b/openbsc/include/openbsc/bsc_nat.h
@@ -408,7 +408,8 @@
int bsc_mgcp_nat_init(struct bsc_nat *nat);
struct nat_sccp_connection *bsc_mgcp_find_con(struct bsc_nat *, int endpoint_number);
-struct msgb *bsc_mgcp_rewrite(char *input, int length, int endp, const char *ip, int port);
+struct msgb *bsc_mgcp_rewrite(char *input, int length, int endp, const char *ip,
+ int port, int *payload_type);
void bsc_mgcp_forward(struct bsc_connection *bsc, struct msgb *msg);
void bsc_mgcp_clear_endpoints_for(struct bsc_connection *bsc);
diff --git a/openbsc/src/libmgcp/mgcp_protocol.c b/openbsc/src/libmgcp/mgcp_protocol.c
index 7096495..a0b905d 100644
--- a/openbsc/src/libmgcp/mgcp_protocol.c
+++ b/openbsc/src/libmgcp/mgcp_protocol.c
@@ -36,10 +36,42 @@
#include <openbsc/mgcp.h>
#include <openbsc/mgcp_internal.h>
-#define for_each_line(line, save) \
+#define for_each_non_empty_line(line, save) \
for (line = strtok_r(NULL, "\r\n", &save); line;\
line = strtok_r(NULL, "\r\n", &save))
+#define for_each_line(line, save) \
+ for (line = strline_r(NULL, &save); line;\
+ line = strline_r(NULL, &save))
+
+char *strline_r(char *str, char **saveptr)
+{
+ char *result;
+
+ if (str)
+ *saveptr = str;
+
+ result = *saveptr;
+
+ if (*saveptr != NULL) {
+ *saveptr = strpbrk(*saveptr, "\r\n");
+
+ if (*saveptr != NULL) {
+ char *eos = *saveptr;
+
+ if ((*saveptr)[0] == '\r' && (*saveptr)[1] == '\n')
+ (*saveptr)++;
+ (*saveptr)++;
+ if ((*saveptr)[0] == '\0')
+ *saveptr = NULL;
+
+ *eos = '\0';
+ }
+ }
+
+ return result;
+}
+
/* Assume audio frame length of 20ms */
#define DEFAULT_RTP_AUDIO_FRAME_DUR_NUM 20
#define DEFAULT_RTP_AUDIO_FRAME_DUR_DEN 1000
@@ -229,12 +261,27 @@
int i, code, handled = 0;
struct msgb *resp = NULL;
char *data;
+ unsigned char *tail = msg->l2h + msgb_l2len(msg); /* char after l2 data */
if (msgb_l2len(msg) < 4) {
LOGP(DMGCP, LOGL_ERROR, "msg too short: %d\n", msg->len);
return NULL;
}
+ /* Ensure that the msg->l2h is NUL terminated. */
+ if (tail[-1] == '\0')
+ /* nothing to do */;
+ else if (msgb_tailroom(msg) > 0)
+ tail[0] = '\0';
+ else if (tail[-1] == '\r' || tail[-1] == '\n')
+ tail[-1] = '\0';
+ else {
+ LOGP(DMGCP, LOGL_ERROR, "Cannot NUL terminate MGCP message: "
+ "Length: %d, Buffer size: %d\n",
+ msgb_l2len(msg), msg->data_len);
+ return NULL;
+ }
+
/* attempt to treat it as a response */
if (sscanf((const char *)&msg->l2h[0], "%3d %*s", &code) == 1) {
LOGP(DMGCP, LOGL_DEBUG, "Response: Code: %d\n", code);
@@ -246,11 +293,10 @@
/*
* Check for a duplicate message and respond.
- * FIXME: Verify that the msg->l3h is NULL terminated.
*/
memset(&pdata, 0, sizeof(pdata));
pdata.cfg = cfg;
- data = strtok_r((char *) msg->l3h, "\r\n", &pdata.save);
+ data = strline_r((char *) msg->l3h, &pdata.save);
pdata.found = mgcp_analyze_header(&pdata, data);
if (pdata.endp && pdata.trans
&& pdata.endp->last_trans
@@ -512,6 +558,62 @@
return 0;
}
+static int parse_sdp_data(struct mgcp_rtp_end *rtp, struct mgcp_parse_data *p)
+{
+ char *line;
+ int found_media = 0;
+
+ for_each_line(line, p->save) {
+ switch (line[0]) {
+ case 'a':
+ case 'o':
+ case 's':
+ case 't':
+ case 'v':
+ /* skip these SDP attributes */
+ break;
+ case 'm': {
+ int port;
+ int payload;
+
+ if (sscanf(line, "m=audio %d RTP/AVP %d",
+ &port, &payload) == 2) {
+ rtp->rtp_port = htons(port);
+ rtp->rtcp_port = htons(port + 1);
+ rtp->payload_type = payload;
+ found_media = 1;
+ }
+ break;
+ }
+ case 'c': {
+ char ipv4[16];
+
+ if (sscanf(line, "c=IN IP4 %15s", ipv4) == 1) {
+ inet_aton(ipv4, &rtp->addr);
+ }
+ break;
+ }
+ default:
+ if (p->endp)
+ LOGP(DMGCP, LOGL_NOTICE,
+ "Unhandled SDP option: '%c'/%d on 0x%x\n",
+ line[0], line[0], ENDPOINT_NUMBER(p->endp));
+ else
+ LOGP(DMGCP, LOGL_NOTICE,
+ "Unhandled SDP option: '%c'/%d\n",
+ line[0], line[0]);
+ break;
+ }
+ }
+
+ if (found_media)
+ LOGP(DMGCP, LOGL_NOTICE,
+ "Got media info via SDP: port %d, payload %d, addr %s\n",
+ ntohs(rtp->rtp_port), rtp->payload_type, inet_ntoa(rtp->addr));
+
+ return found_media;
+}
+
static struct msgb *handle_create_con(struct mgcp_parse_data *p)
{
struct mgcp_trunk_config *tcfg;
@@ -522,6 +624,7 @@
const char *callid = NULL;
const char *mode = NULL;
char *line;
+ int have_sdp = 0;
if (p->found != 0)
return create_err_response(NULL, 510, "CRCX", p->trans);
@@ -538,6 +641,9 @@
case 'M':
mode = (const char *) line + 3;
break;
+ case '\0':
+ have_sdp = 1;
+ goto mgcp_header_done;
default:
LOGP(DMGCP, LOGL_NOTICE, "Unhandled option: '%c'/%d on 0x%x\n",
*line, *line, ENDPOINT_NUMBER(endp));
@@ -545,6 +651,7 @@
}
}
+mgcp_header_done:
tcfg = p->endp->tcfg;
/* Check required data */
@@ -595,9 +702,13 @@
goto error2;
endp->allocated = 1;
+
+ /* set up RTP media parameters */
endp->bts_end.payload_type = tcfg->audio_payload;
endp->bts_end.fmtp_extra = talloc_strdup(tcfg->endpoints,
tcfg->audio_fmtp_extra);
+ if (have_sdp)
+ parse_sdp_data(&endp->net_end, p);
/* policy CB */
if (p->cfg->policy_cb) {
@@ -679,35 +790,13 @@
break;
case '\0':
/* SDP file begins */
+ parse_sdp_data(&endp->net_end, p);
+ /* This will exhaust p->save, so the loop will
+ * terminate next time.
+ */
break;
- case 'a':
- case 'o':
- case 's':
- case 't':
- case 'v':
- /* skip these SDP attributes */
- break;
- case 'm': {
- int port;
- int payload;
-
- if (sscanf(line, "m=audio %d RTP/AVP %d", &port, &payload) == 2) {
- endp->net_end.rtp_port = htons(port);
- endp->net_end.rtcp_port = htons(port + 1);
- endp->net_end.payload_type = payload;
- }
- break;
- }
- case 'c': {
- char ipv4[16];
-
- if (sscanf(line, "c=IN IP4 %15s", ipv4) == 1) {
- inet_aton(ipv4, &endp->net_end.addr);
- }
- break;
- }
default:
- LOGP(DMGCP, LOGL_NOTICE, "Unhandled option: '%c'/%d on 0x%x\n",
+ LOGP(DMGCP, LOGL_NOTICE, "Unhandled MGCP option: '%c'/%d on 0x%x\n",
line[0], line[0], ENDPOINT_NUMBER(endp));
break;
}
@@ -1218,7 +1307,7 @@
return -1;
/* this can only parse the message that is created above... */
- for_each_line(line, save) {
+ for_each_non_empty_line(line, save) {
switch (line[0]) {
case 'P':
rc = sscanf(line, "P: PS=%u, OS=%u, PR=%u, OR=%u, PL=%d, JI=%u",
diff --git a/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c b/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c
index 8bb6075..3dad396 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c
@@ -542,8 +542,10 @@
}
/* we need to generate a new and patched message */
- bsc_msg = bsc_mgcp_rewrite((char *) nat->mgcp_msg, nat->mgcp_length, sccp->bsc_endp,
- nat->mgcp_cfg->source_addr, mgcp_endp->bts_end.local_port);
+ bsc_msg = bsc_mgcp_rewrite((char *) nat->mgcp_msg, nat->mgcp_length,
+ sccp->bsc_endp, nat->mgcp_cfg->source_addr,
+ mgcp_endp->bts_end.local_port,
+ &mgcp_endp->net_end.payload_type);
if (!bsc_msg) {
LOGP(DMGCP, LOGL_ERROR, "Failed to patch the msg.\n");
return MGCP_POLICY_CONT;
@@ -683,7 +685,9 @@
* with the value of 0 should be no problem.
*/
output = bsc_mgcp_rewrite((char * ) msg->l2h, msgb_l2len(msg), -1,
- bsc->nat->mgcp_cfg->source_addr, endp->net_end.local_port);
+ bsc->nat->mgcp_cfg->source_addr,
+ endp->net_end.local_port,
+ &endp->bts_end.payload_type);
if (!output) {
LOGP(DMGCP, LOGL_ERROR, "Failed to rewrite MGCP msg.\n");
@@ -743,7 +747,9 @@
}
/* we need to replace some strings... */
-struct msgb *bsc_mgcp_rewrite(char *input, int length, int endpoint, const char *ip, int port)
+struct msgb *bsc_mgcp_rewrite(char *input, int length, int endpoint,
+ const char *ip, int port,
+ int *payload_type)
{
static const char crcx_str[] = "CRCX ";
static const char dlcx_str[] = "DLCX ";
@@ -836,6 +842,9 @@
memcpy(output->l3h, buf, strlen(buf));
}
+ if (payload != -1 && payload_type)
+ *payload_type = payload;
+
return output;
}
diff --git a/openbsc/tests/bsc-nat/bsc_data.c b/openbsc/tests/bsc-nat/bsc_data.c
index 5a76689..d1f8ebc 100644
--- a/openbsc/tests/bsc-nat/bsc_data.c
+++ b/openbsc/tests/bsc-nat/bsc_data.c
@@ -177,6 +177,7 @@
const char *patch;
const char *ip;
const int port;
+ const int payload_type;
};
static const struct mgcp_patch_test mgcp_messages[] = {
@@ -191,24 +192,28 @@
.patch = crcx_resp_patched,
.ip = "10.0.0.1",
.port = 999,
+ .payload_type = 98,
},
{
.orig = mdcx,
.patch = mdcx_patched,
.ip = "10.0.0.23",
.port = 6666,
+ .payload_type = 126,
},
{
.orig = mdcx_resp,
.patch = mdcx_resp_patched,
.ip = "10.0.0.23",
.port = 5555,
+ .payload_type = 98,
},
{
.orig = mdcx_resp2,
.patch = mdcx_resp_patched2,
.ip = "10.0.0.23",
.port = 5555,
+ .payload_type = 98,
},
};
diff --git a/openbsc/tests/bsc-nat/bsc_nat_test.c b/openbsc/tests/bsc-nat/bsc_nat_test.c
index 5158f46..a121c8a 100644
--- a/openbsc/tests/bsc-nat/bsc_nat_test.c
+++ b/openbsc/tests/bsc-nat/bsc_nat_test.c
@@ -624,10 +624,24 @@
const char *patc = mgcp_messages[i].patch;
const char *ip = mgcp_messages[i].ip;
const int port = mgcp_messages[i].port;
+ const int expected_payload_type = mgcp_messages[i].payload_type;
+ int payload_type = -1;
char *input = strdup(orig);
- output = bsc_mgcp_rewrite(input, strlen(input), 0x1e, ip, port);
+ output = bsc_mgcp_rewrite(input, strlen(input), 0x1e,
+ ip, port, &payload_type);
+
+ if (payload_type != -1) {
+ fprintf(stderr, "Found media payload type %d in SDP data\n",
+ payload_type);
+ if (payload_type != expected_payload_type) {
+ printf("Wrong payload type %d (expected %d)\n",
+ payload_type, expected_payload_type);
+ abort();
+ }
+ }
+
if (msgb_l2len(output) != strlen(patc)) {
printf("Wrong sizes for test: %d %d != %d != %d\n", i, msgb_l2len(output), strlen(patc), strlen(orig));
printf("String '%s' vs '%s'\n", (const char *) output->l2h, patc);
diff --git a/openbsc/tests/mgcp/mgcp_test.c b/openbsc/tests/mgcp/mgcp_test.c
index d36aaa8..c58f52d 100644
--- a/openbsc/tests/mgcp/mgcp_test.c
+++ b/openbsc/tests/mgcp/mgcp_test.c
@@ -25,6 +25,40 @@
#include <string.h>
#include <limits.h>
+char *strline_r(char *str, char **saveptr);
+
+const char *strline_test_data =
+ "one CR\r"
+ "two CR\r"
+ "\r"
+ "one CRLF\r\n"
+ "two CRLF\r\n"
+ "\r\n"
+ "one LF\n"
+ "two LF\n"
+ "\n"
+ "mixed (4 lines)\r\r\n\n\r\n";
+
+#define EXPECTED_NUMBER_OF_LINES 13
+
+static void test_strline(void)
+{
+ char *save = NULL;
+ char *line;
+ char buf[2048];
+ int counter = 0;
+
+ strncpy(buf, strline_test_data, sizeof(buf));
+
+ for (line = strline_r(buf, &save); line;
+ line = strline_r(NULL, &save)) {
+ printf("line: '%s'\n", line);
+ counter++;
+ }
+
+ OSMO_ASSERT(counter == EXPECTED_NUMBER_OF_LINES);
+}
+
#define AUEP1 "AUEP 158663169 ds/e1-1/2@172.16.6.66 MGCP 1.0\r\n"
#define AUEP1_RET "200 158663169 OK\r\n"
#define AUEP2 "AUEP 18983213 ds/e1-2/1@172.16.6.66 MGCP 1.0\r\n"
@@ -48,6 +82,26 @@
"t=0 0\r\n" \
"m=audio 0 RTP/AVP 126\r\n" \
"a=rtpmap:126 AMR/8000\r\n"
+#define MDCX4 "MDCX 18983216 1@mgw MGCP 1.0\r\n" \
+ "C: 2\r\n" \
+ "I: 1\r\n" \
+ "L: p:20, a:AMR, nt:IN\r\n" \
+ "\n" \
+ "v=0\r\n" \
+ "o=- 1 23 IN IP4 0.0.0.0\r\n" \
+ "c=IN IP4 0.0.0.0\r\n" \
+ "t=0 0\r\n" \
+ "m=audio 4441 RTP/AVP 99\r\n" \
+ "a=rtpmap:99 AMR/8000\r\n"
+#define MDCX4_RET "200 18983216 OK\r\n" \
+ "I: 1\n" \
+ "\n" \
+ "v=0\r\n" \
+ "o=- 1 23 IN IP4 0.0.0.0\r\n" \
+ "c=IN IP4 0.0.0.0\r\n" \
+ "t=0 0\r\n" \
+ "m=audio 0 RTP/AVP 126\r\n" \
+ "a=rtpmap:126 AMR/8000\r\n"
#define SHORT2 "CRCX 1"
#define SHORT2_RET "510 000000 FAIL\r\n"
@@ -111,10 +165,16 @@
#define RQNT1_RET "200 186908780 OK\r\n"
#define RQNT2_RET "200 186908781 OK\r\n"
+#define PTYPE_IGNORE 0 /* == default initializer */
+#define PTYPE_NONE 128
+#define PTYPE_NYI PTYPE_NONE
+
struct mgcp_test {
const char *name;
const char *req;
const char *exp_resp;
+ int exp_net_ptype;
+ int exp_bts_ptype;
};
static const struct mgcp_test tests[] = {
@@ -122,10 +182,11 @@
{ "AUEP2", AUEP2, AUEP2_RET },
{ "MDCX1", MDCX_WRONG_EP, MDCX_ERR_RET },
{ "MDCX2", MDCX_UNALLOCATED, MDCX_RET },
- { "CRCX", CRCX, CRCX_RET },
- { "MDCX3", MDCX3, MDCX3_RET },
- { "DLCX", DLCX, DLCX_RET },
- { "CRCX_ZYN", CRCX_ZYN, CRCX_ZYN_RET },
+ { "CRCX", CRCX, CRCX_RET, 97, 126 },
+ { "MDCX3", MDCX3, MDCX3_RET, PTYPE_NONE, 126 },
+ { "MDCX4", MDCX4, MDCX4_RET, 99, 126 },
+ { "DLCX", DLCX, DLCX_RET, -1, -1 },
+ { "CRCX_ZYN", CRCX_ZYN, CRCX_ZYN_RET, 97, 126 },
{ "EMPTY", EMPTY, EMPTY_RET },
{ "SHORT1", SHORT, SHORT_RET },
{ "SHORT2", SHORT2, SHORT2_RET },
@@ -133,7 +194,7 @@
{ "SHORT4", SHORT4, SHORT2_RET },
{ "RQNT1", RQNT, RQNT1_RET },
{ "RQNT2", RQNT2, RQNT2_RET },
- { "DLCX", DLCX, DLCX_RET },
+ { "DLCX", DLCX, DLCX_RET, -1, -1 },
};
static const struct mgcp_test retransmit[] = {
@@ -154,9 +215,21 @@
return msg;
}
+static int last_endpoint = -1;
+
+static int mgcp_test_policy_cb(struct mgcp_trunk_config *cfg, int endpoint,
+ int state, const char *transactio_id)
+{
+ fprintf(stderr, "Policy CB got state %d on endpoint %d\n",
+ state, endpoint);
+ last_endpoint = endpoint;
+ return MGCP_POLICY_CONT;
+}
+
static void test_messages(void)
{
struct mgcp_config *cfg;
+ struct mgcp_endpoint *endp;
int i;
cfg = mgcp_config_alloc();
@@ -164,8 +237,16 @@
cfg->trunk.number_endpoints = 64;
mgcp_endpoints_allocate(&cfg->trunk);
+ cfg->policy_cb = mgcp_test_policy_cb;
+
mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));
+ /* reset endpoints */
+ for (i = 0; i < cfg->trunk.number_endpoints; i++) {
+ endp = &cfg->trunk.endpoints[i];
+ endp->net_end.payload_type = PTYPE_NONE;
+ }
+
for (i = 0; i < ARRAY_SIZE(tests); i++) {
const struct mgcp_test *t = &tests[i];
struct msgb *inp;
@@ -173,6 +254,8 @@
printf("Testing %s\n", t->name);
+ last_endpoint = -1;
+
inp = create_msg(t->req);
msg = mgcp_handle_message(cfg, inp);
msgb_free(inp);
@@ -182,6 +265,29 @@
} else if (strcmp((char *) msg->data, t->exp_resp) != 0)
printf("%s failed '%s'\n", t->name, (char *) msg->data);
msgb_free(msg);
+
+ /* Check detected payload type */
+ if (t->exp_net_ptype != PTYPE_IGNORE ||
+ t->exp_bts_ptype != PTYPE_IGNORE) {
+ OSMO_ASSERT(last_endpoint != -1);
+ endp = &cfg->trunk.endpoints[last_endpoint];
+
+ fprintf(stderr, "endpoint %d: "
+ "payload type BTS %d (exp %d), NET %d (exp %d)\n",
+ last_endpoint,
+ endp->bts_end.payload_type, t->exp_bts_ptype,
+ endp->net_end.payload_type, t->exp_net_ptype);
+
+ if (t->exp_bts_ptype != PTYPE_IGNORE)
+ OSMO_ASSERT(endp->bts_end.payload_type ==
+ t->exp_bts_ptype);
+ if (t->exp_net_ptype != PTYPE_IGNORE)
+ OSMO_ASSERT(endp->net_end.payload_type ==
+ t->exp_net_ptype);
+
+ /* Reset them again for next test */
+ endp->net_end.payload_type = PTYPE_NONE;
+ }
}
talloc_free(cfg);
@@ -463,6 +569,7 @@
{
osmo_init_logging(&log_info);
+ test_strline();
test_messages();
test_retransmission();
test_packet_loss_calc();
diff --git a/openbsc/tests/mgcp/mgcp_test.ok b/openbsc/tests/mgcp/mgcp_test.ok
index 5666424..3bfd78b 100644
--- a/openbsc/tests/mgcp/mgcp_test.ok
+++ b/openbsc/tests/mgcp/mgcp_test.ok
@@ -1,9 +1,23 @@
+line: 'one CR'
+line: 'two CR'
+line: ''
+line: 'one CRLF'
+line: 'two CRLF'
+line: ''
+line: 'one LF'
+line: 'two LF'
+line: ''
+line: 'mixed (4 lines)'
+line: ''
+line: ''
+line: ''
Testing AUEP1
Testing AUEP2
Testing MDCX1
Testing MDCX2
Testing CRCX
Testing MDCX3
+Testing MDCX4
Testing DLCX
Testing CRCX_ZYN
Testing EMPTY