January 2024 - gerrit-log - lists.osmocom.org

[S] Change in pysim[master]: euicc: Fix decoding of SubjectKeyIdentifier.
by fixeria 10 Jan '24

10 Jan '24

Attention is currently required from: laforge. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/35523?usp=email ) Change subject: euicc: Fix decoding of SubjectKeyIdentifier. ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/35523?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: Ic922355308747a888083c5b26765d272b6b20bd0 Gerrit-Change-Number: 35523 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Wed, 10 Jan 2024 14:40:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-ttcn3-hacks[master]: ASCI: Correctly confirm talker, so originator can be identified
by jolly 10 Jan '24

10 Jan '24

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/35544?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: ASCI: Correctly confirm talker, so originator can be identified ...................................................................... ASCI: Correctly confirm talker, so originator can be identified Fix the issue that MSC rejects call termination, because talker can't be identified as originator of the call. Fixes: OS#6325 Change-Id: I0381e25e15624e6b7577910c95700a355ed3f811 --- M msc/MSC_Tests_ASCI.ttcn 1 file changed, 19 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/44/35544/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/35544?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I0381e25e15624e6b7577910c95700a355ed3f811 Gerrit-Change-Number: 35544 Gerrit-PatchSet: 2 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-ttcn3-hacks[master]: ASCI: Correctly confirm talker, so originator can be identified
by jolly 10 Jan '24

10 Jan '24

jolly has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/35544?usp=email ) Change subject: ASCI: Correctly confirm talker, so originator can be identified ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Ok, I forgot to split the patches. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/35544?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I0381e25e15624e6b7577910c95700a355ed3f811 Gerrit-Change-Number: 35544 Gerrit-PatchSet: 1 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Wed, 10 Jan 2024 12:19:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in osmo-msc[master]: ASCI: Remove debug line, used during development process
by jolly 10 Jan '24

10 Jan '24

jolly has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/35545?usp=email ) Change subject: ASCI: Remove debug line, used during development process ...................................................................... ASCI: Remove debug line, used during development process Change-Id: Ie297353146cf30360a7030a2d5ee03e3391cce64 --- M src/libmsc/msc_vgcs.c 1 file changed, 9 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/45/35545/1 diff --git a/src/libmsc/msc_vgcs.c b/src/libmsc/msc_vgcs.c index d3f2e0e..264fa49 100644 --- a/src/libmsc/msc_vgcs.c +++ b/src/libmsc/msc_vgcs.c @@ -1776,7 +1776,6 @@ struct osmo_mobile_identity mi; int rc; - puts(osmo_hexdump(l3, l3_len)); rc = osmo_mobile_identity_decode_from_l3_buf(&mi, l3, l3_len, false); if (rc < 0) { LOG_BSS(bss, LOGL_DEBUG, "Talker's Identity cannot be decoded.\n"); -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/35545?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ie297353146cf30360a7030a2d5ee03e3391cce64 Gerrit-Change-Number: 35545 Gerrit-PatchSet: 1 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-MessageType: newchange

1 0

[S] Change in osmo-ttcn3-hacks[master]: ASCI: Correctly confirm talker, so originator can be identified
by jolly 10 Jan '24

10 Jan '24

jolly has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/35544?usp=email ) Change subject: ASCI: Correctly confirm talker, so originator can be identified ...................................................................... ASCI: Correctly confirm talker, so originator can be identified Fix the issue that MSC rejects call termination, because talker can't be identified as originator of the call. Fixes: OS#6325 Change-Id: I0381e25e15624e6b7577910c95700a355ed3f811 --- M library/BSSMAP_Templates.ttcn M msc/MSC_Tests_ASCI.ttcn M msc/osmo-msc.cfg 3 files changed, 39 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/44/35544/1 diff --git a/library/BSSMAP_Templates.ttcn b/library/BSSMAP_Templates.ttcn index a93fb02..a85eb5a 100644 --- a/library/BSSMAP_Templates.ttcn +++ b/library/BSSMAP_Templates.ttcn @@ -1368,6 +1368,22 @@ digits := imsi_digits } +template BSSMAP_IE_MobileIdentity ts_BSSMAP_MobileIdentity_Imsi(hexstring imsi_digits) := { + elementIdentifier := '29'O, + lengthIndicator := 0, /* overwritten */ + typeOfIdentity := '001'B, /* IMSI */ + oddEvenIndicator := f_hex_is_odd_length(imsi_digits), + digits := imsi_digits +} + +template BSSMAP_IE_MobileIdentity tr_BSSMAP_MobileIdentity_Imsi(template hexstring imsi_digits) := { + elementIdentifier := '29'O, + lengthIndicator := ?, /* overwritten */ + typeOfIdentity := '001'B, /* IMSI */ + oddEvenIndicator := ?, + digits := imsi_digits +} + template (value) BSSAMAP_IE_LastUsedEUTRANPLMNId ts_BSSMAP_LastUsedEUTRANPLMNId(OCT3 mcc_mnc) := { elementIdentifier := '95'O, mcc_mnc := mcc_mnc diff --git a/msc/MSC_Tests_ASCI.ttcn b/msc/MSC_Tests_ASCI.ttcn index 76d8d3c..c8bf2da 100644 --- a/msc/MSC_Tests_ASCI.ttcn +++ b/msc/MSC_Tests_ASCI.ttcn @@ -244,9 +244,12 @@ var octetstring gcc_term_req := '3500001900'O; var octetstring bcc_term_req := '3500001a00'O; var octetstring xcc_termination := '340110'O; + /* L3 info carries the mobile identity of the talker requesting the uplink. */ + var octetstring uplink_req_conf := '061103505902082926240000000033'O; var myBSSMAP_Cause cause_success := GSM0808_CAUSE_CALL_CONTROL; var template (value) PDU_BSSAP ts_uplink_rel_ind := ts_BSSMAP_UplinkRelInd(enum2int(cause_success), omit); var template (value) PDU_BSSAP ts_uplink_req := ts_BSSMAP_UplinkReq; + var template (value) PDU_BSSAP ts_uplink_req_conf := ts_BSSMAP_UplinkReqConf(ts_CellId_CI(42), omit, uplink_req_conf); var PDU_BSSAP rx_bssap; f_init_handler(pars); @@ -316,6 +319,9 @@ log("Got Uplink Request Acknowledge: ", rx_bssap); COORD.send(COORD_UPLINK_REQ_ACK); f_sleep(0.2); + /* The BSC confirms the uplink and provides mobile identity to identify originator. */ + log("Sending Uplink Req Confirm: ", ts_uplink_req_conf); + BSSAP.send(ts_uplink_req_conf); /* The MS requests termination of the call. */ log("Sending GCC Termination Request: ", gcc_term_req); BSSAP.send(ts_PDU_DTAP_MO(ts_ML3_MO_GCC(cpars.transaction_id, gcc_term_req))); diff --git a/msc/osmo-msc.cfg b/msc/osmo-msc.cfg index d05e400..d71ab6b 100644 --- a/msc/osmo-msc.cfg +++ b/msc/osmo-msc.cfg @@ -29,6 +29,10 @@ logging level ranap debug logging level vlr debug logging level iucs debug + logging level gcc debug + logging level bcc debug + logging level asci debug + ! stats interval 5 ! -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/35544?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I0381e25e15624e6b7577910c95700a355ed3f811 Gerrit-Change-Number: 35544 Gerrit-PatchSet: 1 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-MessageType: newchange

1 0

[S] Change in osmo-mgw[master]: client SDP: more verbose error logging
by neels 10 Jan '24

10 Jan '24

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email ) Change subject: client SDP: more verbose error logging ...................................................................... client SDP: more verbose error logging So far it was pure guess work to find out why a message fails. Change-Id: Ibc6343db82281789004c140ba98d99e5f6f73d83 --- M src/libosmo-mgcp-client/mgcp_client.c 1 file changed, 27 insertions(+), 5 deletions(-) Approvals: dexter: Looks good to me, but someone else must approve Jenkins Builder: Verified neels: Looks good to me, approved laforge: Looks good to me, but someone else must approve diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c index 8df65cd..489ce69 100644 --- a/src/libosmo-mgcp-client/mgcp_client.c +++ b/src/libosmo-mgcp-client/mgcp_client.c @@ -329,20 +329,31 @@ errno = 0; pt = strtoul(pt_str, &pt_end, 0); if ((errno == ERANGE && pt == ULONG_MAX) || (errno && !pt) || - pt_str == pt_end) + pt_str == pt_end) { + LOGP(DLMGCP, LOGL_ERROR, "SDP: cannot parse payload type number from '%s'\n", pt_str); goto response_parse_failure_pt; + } - if (pt >> 7) /* PT is 7 bit field, higher values not allowed */ + /* PT is 7 bit field, higher values not allowed */ + if (pt >> 7) { + LOGP(DLMGCP, LOGL_ERROR, "SDP: payload type number out of range: %lu > 127\n", pt); goto response_parse_failure_pt; + } /* Do not allow duplicate payload types */ - for (i = 0; i < ptmap_len; i++) - if (r->ptmap[i].pt == pt) + for (i = 0; i < ptmap_len; i++) { + if (r->ptmap[i].pt == pt) { + LOGP(DLMGCP, LOGL_ERROR, "SDP: payload type number %lu listed twice\n", pt); goto response_parse_failure_pt; + } + } /* Do not allow excessive payload types */ - if (ptmap_len >= ARRAY_SIZE(r->ptmap)) + if (ptmap_len >= ARRAY_SIZE(r->ptmap)) { + LOGP(DLMGCP, LOGL_ERROR, + "SDP: can parse only up to %zu payload type numbers\n", ARRAY_SIZE(r->ptmap)); goto response_parse_failure_pt; + } /* Some payload type numbers imply a specific codec. For those, using the PT number as enum mgcp_codecs * yields the correct result. If no more specific information on the codec follows in "a=rtpmap:N" -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ibc6343db82281789004c140ba98d99e5f6f73d83 Gerrit-Change-Number: 35421 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[L] Change in osmo-mgw[master]: mgcp_client_test: add test_parse_response()
by neels 10 Jan '24

10 Jan '24

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35422?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: mgcp_client_test: add test_parse_response() ...................................................................... mgcp_client_test: add test_parse_response() Change-Id: I842ce65a9a70f313570857b7df53727cc572b9e6 --- M tests/mgcp_client/mgcp_client_test.c M tests/mgcp_client/mgcp_client_test.err 2 files changed, 266 insertions(+), 0 deletions(-) Approvals: pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, approved diff --git a/tests/mgcp_client/mgcp_client_test.c b/tests/mgcp_client/mgcp_client_test.c index 3883c3a..9b356c3 100644 --- a/tests/mgcp_client/mgcp_client_test.c +++ b/tests/mgcp_client/mgcp_client_test.c @@ -673,6 +673,214 @@ OSMO_ASSERT(epname == NULL); } +struct parse_response_test { + const char *body; + int expect_rc; + struct mgcp_response expect_params; +}; + +static struct parse_response_test parse_response_tests[] = { + { + .body = "200 2 OK\r\n" + "I: foo\r\n" + "\r\n" + "v=0\r\n" + "o=- name 23 IN IP4 0.0.0.0\r\n" + "s=-\r\n" + "c=IN IP4 1.2.3.4\r\n" + "t=0 0\r\n" + "m=audio 23 RTP/AVP 112 3\r\n" /* <-- implicit: 3 = GSM-FR */ + "a=rtpmap:112 AMR/8000\r\n" + "a=ptime:20\r\n", + .expect_rc = 0, + .expect_params = { + .audio_port = 23, + .audio_ip = "1.2.3.4", + .ptmap = { + { .codec = CODEC_AMR_8000_1, .pt = 112 }, + { .codec = CODEC_GSM_8000_1, .pt = 3 }, + }, + .ptmap_len = 2, + }, + }, + { + .body = "200 2 OK\r\n" + "I: foo\r\n" + "\r\n" + "v=0\r\n" + "o=- name 23 IN IP4 0.0.0.0\r\n" + "s=-\r\n" + "c=IN IP4 1.2.3.4\r\n" + "t=0 0\r\n" + "m=audio 23 RTP/AVP 112 3\r\n" + "a=rtpmap:112 AMR/8000\r\n" + "a=rtpmap:3 GSM/8000\r\n" /* 3 == GSM-FR implicitly, is an explicit entry a problem? */ + "a=ptime:20\r\n", + .expect_rc = 0, + .expect_params = { + .audio_port = 23, + .audio_ip = "1.2.3.4", + .ptmap = { + { .codec = CODEC_AMR_8000_1, .pt = 112 }, + { .codec = CODEC_GSM_8000_1, .pt = 3 }, /* no, not a problem */ + }, + .ptmap_len = 2, + }, + }, + { + .body = "200 2 OK\r\n" + "I: foo\r\n" + "\r\n" + "v=0\r\n" + "o=- name 23 IN IP4 0.0.0.0\r\n" + "s=-\r\n" + "c=IN IP4 1.2.3.4\r\n" + "t=0 0\r\n" + "m=audio 23 RTP/AVP 3\r\n" /* <-- 112 is missing here. Will it still appear? */ + "a=rtpmap:112 AMR/8000\r\n" + "a=ptime:20\r\n", + .expect_rc = 0, + .expect_params = { + .audio_port = 23, + .audio_ip = "1.2.3.4", + .ptmap = { + { .codec = CODEC_GSM_8000_1, .pt = 3 }, + { .codec = CODEC_AMR_8000_1, .pt = 112 }, /* <-- yes, it was added to the end. */ + }, + .ptmap_len = 2, + }, + }, + { + /* test MGCP_MAX_CODECS */ + .body = "200 2 OK\r\n" + "I: foo\r\n" + "\r\n" + "v=0\r\n" + "o=- name 23 IN IP4 0.0.0.0\r\n" + "s=-\r\n" + "c=IN IP4 1.2.3.4\r\n" + "t=0 0\r\n" + "m=audio 23 RTP/AVP 101 102 103 104 105 106 107 108 109 110\r\n" /* <-- 10 codecs max */ + "a=rtpmap:101 AMR/8000\r\n" + "a=rtpmap:102 AMR/8000\r\n" + "a=rtpmap:103 AMR/8000\r\n" + "a=rtpmap:104 AMR/8000\r\n" + "a=rtpmap:105 AMR/8000\r\n" + "a=rtpmap:106 AMR/8000\r\n" + "a=rtpmap:107 AMR/8000\r\n" + "a=rtpmap:108 AMR/8000\r\n" + "a=rtpmap:109 AMR/8000\r\n" + "a=rtpmap:110 AMR/8000\r\n" + "a=ptime:20\r\n", + .expect_rc = 0, + .expect_params = { + .audio_port = 23, + .audio_ip = "1.2.3.4", + .ptmap = { + { .codec = CODEC_AMR_8000_1, .pt = 101 }, + { .codec = CODEC_AMR_8000_1, .pt = 102 }, + { .codec = CODEC_AMR_8000_1, .pt = 103 }, + { .codec = CODEC_AMR_8000_1, .pt = 104 }, + { .codec = CODEC_AMR_8000_1, .pt = 105 }, + { .codec = CODEC_AMR_8000_1, .pt = 106 }, + { .codec = CODEC_AMR_8000_1, .pt = 107 }, + { .codec = CODEC_AMR_8000_1, .pt = 108 }, + { .codec = CODEC_AMR_8000_1, .pt = 109 }, + { .codec = CODEC_AMR_8000_1, .pt = 110 }, + }, + .ptmap_len = 10, + }, + }, + { + /* test MGCP_MAX_CODECS */ + .body = "200 2 OK\r\n" + "I: foo\r\n" + "\r\n" + "v=0\r\n" + "o=- name 23 IN IP4 0.0.0.0\r\n" + "s=-\r\n" + "c=IN IP4 1.2.3.4\r\n" + "t=0 0\r\n" + "m=audio 23 RTP/AVP 101 102 103 104 105 106 107 108 109 110 3\r\n" /* <-- 11 > MGCP_MAX_CODECS */ + "a=rtpmap:101 AMR/8000\r\n" + "a=rtpmap:102 AMR/8000\r\n" + "a=rtpmap:103 AMR/8000\r\n" + "a=rtpmap:104 AMR/8000\r\n" + "a=rtpmap:105 AMR/8000\r\n" + "a=rtpmap:106 AMR/8000\r\n" + "a=rtpmap:107 AMR/8000\r\n" + "a=rtpmap:108 AMR/8000\r\n" + "a=rtpmap:109 AMR/8000\r\n" + "a=rtpmap:110 AMR/8000\r\n" + "a=ptime:20\r\n", + .expect_rc = -EINVAL, + }, +}; + +static void test_parse_response(void) +{ + int i; + int failures = 0; + + for (i = 0; i < ARRAY_SIZE(parse_response_tests); i++) { + int rc; + struct parse_response_test *t = &parse_response_tests[i]; + struct mgcp_response *r = talloc_zero(ctx, struct mgcp_response); + int p; + + r->body = talloc_strdup(r, t->body); + + //printf("\n%s() test [%d]:\n", __func__, i); + fprintf(stderr, "\n%s() test [%d]:\n", __func__, i); + fprintf(stderr, "body: \"%s\"\n", osmo_escape_str(r->body, -1)); + + rc = mgcp_response_parse_params(r); + + fprintf(stderr, "got rc=%d\n", rc); + if (rc != t->expect_rc) { + fprintf(stderr, "FAIL: Expected rc=%d\n", t->expect_rc); + failures++; + } + if (rc) { + talloc_free(r); + continue; + } + + fprintf(stderr, "got audio_ip=\"%s\"\n", r->audio_ip); + if (strcmp(r->audio_ip, t->expect_params.audio_ip)) { + fprintf(stderr, "FAIL: Expected audio_ip=\"%s\"\n", t->expect_params.audio_ip); + failures++; + } + fprintf(stderr, "got audio_port=%u\n", r->audio_port); + if (r->audio_port != t->expect_params.audio_port) { + fprintf(stderr, "FAIL: Expected audio_port=%u\n", t->expect_params.audio_port); + failures++; + } + + for (p = 0; p < r->ptmap_len; p++) { + struct ptmap *got = &r->ptmap[p]; + struct ptmap *expect = NULL; + fprintf(stderr, " %d %s\n", got->pt, osmo_mgcpc_codec_name(got->codec)); + if (p >= t->expect_params.ptmap_len) { + fprintf(stderr, " - ERROR: too many codec entries\n"); + failures++; + continue; + } + expect = &t->expect_params.ptmap[p]; + if (ptmap_cmp(got, expect)) { + fprintf(stderr, " - ERROR: expected: %d %s\n", + expect->pt, osmo_mgcpc_codec_name(expect->codec)); + failures++; + } + } + + talloc_free(r); + } + + OSMO_ASSERT(!failures); +} + + static const struct log_info_cat log_categories[] = { }; @@ -704,6 +912,8 @@ test_map_str_to_codec(); test_mgcp_client_e1_epname(); + test_parse_response(); + printf("Done\n"); fprintf(stderr, "Done\n"); return EXIT_SUCCESS; diff --git a/tests/mgcp_client/mgcp_client_test.err b/tests/mgcp_client/mgcp_client_test.err index 22ad3cc..94fe351 100644 --- a/tests/mgcp_client/mgcp_client_test.err +++ b/tests/mgcp_client/mgcp_client_test.err @@ -137,4 +137,51 @@ DLMGCP MGW(mgw) Cannot compose MGCP e1-endpoint name (ds/e1-15/s-1/su8-16@mgw), rate(8)/offset(16) combination is invalid! DLMGCP MGW(mgw) Cannot compose MGCP e1-endpoint name (ds/e1-15/s-0/su8-2@mgw), E1-timeslot number (0) is invalid! DLMGCP MGW(mgw) Cannot compose MGCP e1-endpoint name (ds/e1-15/s-64/su8-2@mgw), E1-timeslot number (64) is invalid! + +test_parse_response() test [0]: +body: "200 2 OK\r\nI: foo\r\n\r\nv=0\r\no=- name 23 IN IP4 0.0.0.0\r\ns=-\r\nc=IN IP4 1.2.3.4\r\nt=0 0\r\nm=audio 23 RTP/AVP 112 3\r\na=rtpmap:112 AMR/8000\r\na=ptime:20\r\n" +got rc=0 +got audio_ip="1.2.3.4" +got audio_port=23 + 112 AMR/8000/1 + 3 GSM/8000/1 + +test_parse_response() test [1]: +body: "200 2 OK\r\nI: foo\r\n\r\nv=0\r\no=- name 23 IN IP4 0.0.0.0\r\ns=-\r\nc=IN IP4 1.2.3.4\r\nt=0 0\r\nm=audio 23 RTP/AVP 112 3\r\na=rtpmap:112 AMR/8000\r\na=rtpmap:3 GSM/8000\r\na=ptime:20\r\n" +got rc=0 +got audio_ip="1.2.3.4" +got audio_port=23 + 112 AMR/8000/1 + 3 GSM/8000/1 + +test_parse_response() test [2]: +body: "200 2 OK\r\nI: foo\r\n\r\nv=0\r\no=- name 23 IN IP4 0.0.0.0\r\ns=-\r\nc=IN IP4 1.2.3.4\r\nt=0 0\r\nm=audio 23 RTP/AVP 3\r\na=rtpmap:112 AMR/8000\r\na=ptime:20\r\n" +DLMGCP error in MGCP message: 'a=rtpmap:112' has no matching entry in 'm=audio ... 112' +got rc=0 +got audio_ip="1.2.3.4" +got audio_port=23 + 3 GSM/8000/1 + 112 AMR/8000/1 + +test_parse_response() test [3]: +body: "200 2 OK\r\nI: foo\r\n\r\nv=0\r\no=- name 23 IN IP4 0.0.0.0\r\ns=-\r\nc=IN IP4 1.2.3.4\r\nt=0 0\r\nm=audio 23 RTP/AVP 101 102 103 104 105 106 107 108 109 110\r\na=rtpmap:101 AMR/8000\r\na=rtpmap:102 AMR/8000\r\na=rtpmap:103 AMR/8000\r\na=rtpmap:104 AMR/8" +got rc=0 +got audio_ip="1.2.3.4" +got audio_port=23 + 101 AMR/8000/1 + 102 AMR/8000/1 + 103 AMR/8000/1 + 104 AMR/8000/1 + 105 AMR/8000/1 + 106 AMR/8000/1 + 107 AMR/8000/1 + 108 AMR/8000/1 + 109 AMR/8000/1 + 110 AMR/8000/1 + +test_parse_response() test [4]: +body: "200 2 OK\r\nI: foo\r\n\r\nv=0\r\no=- name 23 IN IP4 0.0.0.0\r\ns=-\r\nc=IN IP4 1.2.3.4\r\nt=0 0\r\nm=audio 23 RTP/AVP 101 102 103 104 105 106 107 108 109 110 3\r\na=rtpmap:101 AMR/8000\r\na=rtpmap:102 AMR/8000\r\na=rtpmap:103 AMR/8000\r\na=rtpmap:104 AMR" +DLMGCP SDP: can parse only up to 10 payload type numbers +DLMGCP Failed to parse SDP parameter payload types (RTP/AVP) +got rc=-22 Done -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35422?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I842ce65a9a70f313570857b7df53727cc572b9e6 Gerrit-Change-Number: 35422 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmo-mgw[master]: drop (now) unused code
by neels 10 Jan '24

10 Jan '24

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35247?usp=email ) Change subject: drop (now) unused code ...................................................................... drop (now) unused code Removing the duality of codecs[] and ptmap[] in structs mgcp_msg, mgcp_response and mgcp_conn_peer has removed the need to "map" from codec type enum to payload type number. They are stored together now. Remove functions that are no longer used. None of our osmocom users of libosmo-mgcp-client call these functions. Change-Id: I84e5285831397c992af59deee12dea8458d16cc6 --- M TODO-RELEASE M include/osmocom/mgcp_client/mgcp_client.h M src/libosmo-mgcp-client/mgcp_client.c M tests/mgcp_client/mgcp_client_test.c M tests/mgcp_client/mgcp_client_test.err M tests/mgcp_client/mgcp_client_test.ok 6 files changed, 21 insertions(+), 177 deletions(-) Approvals: Jenkins Builder: Verified neels: Looks good to me, approved diff --git a/TODO-RELEASE b/TODO-RELEASE index 964ffe1..f1a702f 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -38,3 +38,8 @@ libosmo-mgcp-client deprecate public API New code should no longer use codecs[], instead use ptmap[].codec. There is backwards compat code that moves codecs[] entries, if any, over to ptmap[], so callers may migrate at own leisure. +libosmo-mgcp-client remove public API Since codecs[] has been deprecated in favor of ptmap[], there is no use + for the following functions; all known callers have been within + osmo-mgw.git: + map_codec_to_pt() + map_pt_to_codec() diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h index 1d33690..e1748a6 100644 --- a/include/osmocom/mgcp_client/mgcp_client.h +++ b/include/osmocom/mgcp_client/mgcp_client.h @@ -119,9 +119,5 @@ } enum mgcp_codecs map_str_to_codec(const char *str); -unsigned int map_codec_to_pt(const struct ptmap *ptmap, unsigned int ptmap_len, - enum mgcp_codecs codec); -enum mgcp_codecs map_pt_to_codec(struct ptmap *ptmap, unsigned int ptmap_len, - unsigned int pt); const char *mgcp_client_name(const struct mgcp_client *mgcp); diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c index 489ce69..bfe69e6 100644 --- a/src/libosmo-mgcp-client/mgcp_client.c +++ b/src/libosmo-mgcp-client/mgcp_client.c @@ -105,94 +105,6 @@ return -1; } -/* Check the ptmap for illegal mappings */ -static int check_ptmap(const struct ptmap *ptmap) -{ - /* Check if there are mappings that leave the IANA assigned dynamic - * payload type range. Under normal conditions such mappings should - * not occur */ - - /* Its ok to have a 1:1 mapping in the statically defined - * range, this won't hurt */ - if (ptmap->codec == ptmap->pt) - return 0; - - if (ptmap->codec < 96 || ptmap->codec > 127) - goto error; - if (ptmap->pt < 96 || ptmap->pt > 127) - goto error; - - return 0; -error: - LOGP(DLMGCP, LOGL_ERROR, - "ptmap contains illegal mapping: codec=%u maps to pt=%u\n", - ptmap->codec, ptmap->pt); - return -1; -} - -/*! Map a codec to a payload type. - * \ptmap[in] payload pointer to payload type map with specified payload types. - * \ptmap[in] ptmap_len length of the payload type map. - * \ptmap[in] codec the codec for which the payload type should be looked up. - * \returns assigned payload type */ -unsigned int map_codec_to_pt(const struct ptmap *ptmap, unsigned int ptmap_len, - enum mgcp_codecs codec) -{ - unsigned int i; - - /*! Note: If the payload type map is empty or the codec is not found - * in the map, then a 1:1 mapping is performed. If the codec falls - * into the statically defined range or if the mapping table isself - * tries to map to the statically defined range, then the mapping - * is also ignored and a 1:1 mapping is performed instead. */ - - /* we may return the codec directly since enum mgcp_codecs directly - * corresponds to the statically assigned payload types */ - if (codec < 96 || codec > 127) - return codec; - - for (i = 0; i < ptmap_len; i++) { - /* Skip illegal map entries */ - if (check_ptmap(ptmap) == 0 && ptmap->codec == codec) - return ptmap->pt; - ptmap++; - } - - /* If nothing is found, do not perform any mapping */ - return codec; -} - -/*! Map a payload type to a codec. - * \ptmap[in] payload pointer to payload type map with specified payload types. - * \ptmap[in] ptmap_len length of the payload type map. - * \ptmap[in] payload type for which the codec should be looked up. - * \returns codec that corresponds to the specified payload type */ -enum mgcp_codecs map_pt_to_codec(struct ptmap *ptmap, unsigned int ptmap_len, - unsigned int pt) -{ - unsigned int i; - - /*! Note: If the payload type map is empty or the payload type is not - * found in the map, then a 1:1 mapping is performed. If the payload - * type falls into the statically defined range or if the mapping - * table isself tries to map to the statically defined range, then - * the mapping is also ignored and a 1:1 mapping is performed - * instead. */ - - /* See also note in map_codec_to_pt() */ - if (pt < 96 || pt > 127) - return pt; - - for (i = 0; i < ptmap_len; i++) { - if (check_ptmap(ptmap) == 0 && ptmap->pt == pt) - return ptmap->codec; - ptmap++; - } - - /* If nothing is found, do not perform any mapping */ - return pt; -} - static void _mgcp_client_conf_init(struct mgcp_client_conf *conf) { /* NULL and -1 default to MGCP_CLIENT_*_DEFAULT values */ diff --git a/tests/mgcp_client/mgcp_client_test.c b/tests/mgcp_client/mgcp_client_test.c index 9b356c3..95a742f 100644 --- a/tests/mgcp_client/mgcp_client_test.c +++ b/tests/mgcp_client/mgcp_client_test.c @@ -571,57 +571,6 @@ OSMO_ASSERT(map_str_to_codec("AMR-WB####################################################################################################################") == -1); } -static void test_map_codec_to_pt_and_map_pt_to_codec(void) -{ - struct ptmap ptmap[10]; - unsigned int ptmap_len; - unsigned int i; - - ptmap[0].codec = CODEC_GSMEFR_8000_1; - ptmap[0].pt = 96; - ptmap[1].codec = CODEC_GSMHR_8000_1; - ptmap[1].pt = 97; - ptmap[2].codec = CODEC_AMR_8000_1; - ptmap[2].pt = 98; - ptmap[3].codec = CODEC_AMRWB_16000_1; - ptmap[3].pt = 99; - ptmap_len = 4; - - /* Mappings that are covered by the table */ - for (i = 0; i < ptmap_len; i++) - printf(" %u => %u\n", ptmap[i].codec, map_codec_to_pt(ptmap, ptmap_len, ptmap[i].codec)); - for (i = 0; i < ptmap_len; i++) - printf(" %u <= %u\n", ptmap[i].pt, map_pt_to_codec(ptmap, ptmap_len, ptmap[i].pt)); - printf("\n"); - - /* Map some codecs/payload types from the static range, result must - * always be a 1:1 mapping */ - printf(" %u => %u\n", CODEC_PCMU_8000_1, map_codec_to_pt(ptmap, ptmap_len, CODEC_PCMU_8000_1)); - printf(" %u => %u\n", CODEC_GSM_8000_1, map_codec_to_pt(ptmap, ptmap_len, CODEC_GSM_8000_1)); - printf(" %u => %u\n", CODEC_PCMA_8000_1, map_codec_to_pt(ptmap, ptmap_len, CODEC_PCMA_8000_1)); - printf(" %u => %u\n", CODEC_G729_8000_1, map_codec_to_pt(ptmap, ptmap_len, CODEC_G729_8000_1)); - printf(" %u <= %u\n", CODEC_PCMU_8000_1, map_pt_to_codec(ptmap, ptmap_len, CODEC_PCMU_8000_1)); - printf(" %u <= %u\n", CODEC_GSM_8000_1, map_pt_to_codec(ptmap, ptmap_len, CODEC_GSM_8000_1)); - printf(" %u <= %u\n", CODEC_PCMA_8000_1, map_pt_to_codec(ptmap, ptmap_len, CODEC_PCMA_8000_1)); - printf(" %u <= %u\n", CODEC_G729_8000_1, map_pt_to_codec(ptmap, ptmap_len, CODEC_G729_8000_1)); - printf("\n"); - - /* Try to do mappings from statically defined range to danymic range and vice versa. This - * is illegal and should result into a 1:1 mapping */ - ptmap[3].codec = CODEC_AMRWB_16000_1; - ptmap[3].pt = 2; - ptmap[4].codec = CODEC_PCMU_8000_1; - ptmap[4].pt = 100; - ptmap_len = 5; - - /* Apply all mappings again, the illegal ones we defined should result into 1:1 mappings */ - for (i = 0; i < ptmap_len; i++) - printf(" %u => %u\n", ptmap[i].codec, map_codec_to_pt(ptmap, ptmap_len, ptmap[i].codec)); - for (i = 0; i < ptmap_len; i++) - printf(" %u <= %u\n", ptmap[i].pt, map_pt_to_codec(ptmap, ptmap_len, ptmap[i].pt)); - printf("\n"); -} - void test_mgcp_client_e1_epname(void) { char *epname; @@ -908,7 +857,6 @@ test_mgcp_msg(); test_mgcp_client_cancel(); test_sdp_section_start(); - test_map_codec_to_pt_and_map_pt_to_codec(); test_map_str_to_codec(); test_mgcp_client_e1_epname(); diff --git a/tests/mgcp_client/mgcp_client_test.err b/tests/mgcp_client/mgcp_client_test.err index 94fe351..9e98792 100644 --- a/tests/mgcp_client/mgcp_client_test.err +++ b/tests/mgcp_client/mgcp_client_test.err @@ -128,10 +128,6 @@ body: "some mgcp header data\r\nand header params\r\n\r\nc=IN IP4 \r\n" DLMGCP Failed to parse MGCP response header (audio ip) got rc=-22 -DLMGCP ptmap contains illegal mapping: codec=113 maps to pt=2 -DLMGCP ptmap contains illegal mapping: codec=0 maps to pt=100 -DLMGCP ptmap contains illegal mapping: codec=113 maps to pt=2 -DLMGCP ptmap contains illegal mapping: codec=0 maps to pt=100 DLMGCP MGW(mgw) MGCP client: using endpoint domain '@mgw' DLMGCP MGW(mgw) Cannot compose MGCP e1-endpoint name (ds/e1-15/s-1/su128-0@mgw), rate(128)/offset(0) combination is invalid! DLMGCP MGW(mgw) Cannot compose MGCP e1-endpoint name (ds/e1-15/s-1/su8-16@mgw), rate(8)/offset(16) combination is invalid! diff --git a/tests/mgcp_client/mgcp_client_test.ok b/tests/mgcp_client/mgcp_client_test.ok index 039fbd9..b16d1bc 100644 --- a/tests/mgcp_client/mgcp_client_test.ok +++ b/tests/mgcp_client/mgcp_client_test.ok @@ -181,35 +181,6 @@ test_sdp_section_start() test [17]: test_sdp_section_start() test [18]: - 110 => 96 - 111 => 97 - 112 => 98 - 113 => 99 - 96 <= 110 - 97 <= 111 - 98 <= 112 - 99 <= 113 - - 0 => 0 - 3 => 3 - 8 => 8 - 18 => 18 - 0 <= 0 - 3 <= 3 - 8 <= 8 - 18 <= 18 - - 110 => 96 - 111 => 97 - 112 => 98 - 113 => 113 - 0 => 0 - 96 <= 110 - 97 <= 111 - 98 <= 112 - 2 <= 2 - 100 <= 100 - ds/e1-1/s-15/su64-0@mgw ds/e1-2/s-14/su32-0@mgw ds/e1-3/s-13/su32-4@mgw -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35247?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I84e5285831397c992af59deee12dea8458d16cc6 Gerrit-Change-Number: 35247 Gerrit-PatchSet: 6 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in osmo-mgw[master]: client SDP: more verbose error logging
by neels 10 Jan '24

10 Jan '24

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email ) Change subject: client SDP: more verbose error logging ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ibc6343db82281789004c140ba98d99e5f6f73d83 Gerrit-Change-Number: 35421 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 10 Jan 2024 08:00:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-bsc[master]: vty: disallow combination of early-IA and frequency hopping
by neels 10 Jan '24

10 Jan '24

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35097?usp=email ) Change subject: vty: disallow combination of early-IA and frequency hopping ...................................................................... vty: disallow combination of early-IA and frequency hopping Related: osmo-bts Id9a930e5c67122812b229dc27ea2bfe246b67611 Related: SYS#6655 Change-Id: I8d375e5155be7b53034d5c0be5566d2f33af5db0 --- M src/osmo-bsc/bts_trx_vty.c M src/osmo-bsc/bts_vty.c A tests/early_ia.vty 3 files changed, 67 insertions(+), 3 deletions(-) Approvals: neels: Looks good to me, approved Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve osmith: Looks good to me, but someone else must approve diff --git a/src/osmo-bsc/bts_trx_vty.c b/src/osmo-bsc/bts_trx_vty.c index 385336b..9a58089 100644 --- a/src/osmo-bsc/bts_trx_vty.c +++ b/src/osmo-bsc/bts_trx_vty.c @@ -349,6 +349,13 @@ return CMD_WARNING; } + if (enabled && bts->imm_ass_time != IMM_ASS_TIME_POST_CHAN_ACK) { + vty_out(vty, + "%% ERROR: 'hopping enabled 1' works only with 'immediate-assignment post-chan-ack'%s", + VTY_NEWLINE); + return CMD_WARNING; + } + ts->hopping.enabled = enabled; return CMD_SUCCESS; diff --git a/src/osmo-bsc/bts_vty.c b/src/osmo-bsc/bts_vty.c index 54eb5cf..8b97dce 100644 --- a/src/osmo-bsc/bts_vty.c +++ b/src/osmo-bsc/bts_vty.c @@ -3242,13 +3242,35 @@ CMD_ATTR_IMMEDIATE) { struct gsm_bts *bts = vty->index; + enum imm_ass_time imm_ass_time; if (!strcmp(argv[0], "pre-ts-ack")) - bts->imm_ass_time = IMM_ASS_TIME_PRE_TS_ACK; + imm_ass_time = IMM_ASS_TIME_PRE_TS_ACK; else if (!strcmp(argv[0], "pre-chan-ack")) - bts->imm_ass_time = IMM_ASS_TIME_PRE_CHAN_ACK; + imm_ass_time = IMM_ASS_TIME_PRE_CHAN_ACK; else - bts->imm_ass_time = IMM_ASS_TIME_POST_CHAN_ACK; + imm_ass_time = IMM_ASS_TIME_POST_CHAN_ACK; + + if (imm_ass_time != IMM_ASS_TIME_POST_CHAN_ACK) { + struct gsm_bts_trx *trx; + + /* Early-IA does not work with frequency hopping, because the IMM ASS does not convey an ARFCN when + * frequency hopping is in use. Make sure the user knows that. */ + llist_for_each_entry(trx, &bts->trx_list, list) { + int ts_nr; + for (ts_nr = 0; ts_nr < TRX_NR_TS; ts_nr++) { + struct gsm_bts_trx_ts *ts = &trx->ts[ts_nr]; + if (ts->hopping.enabled) { + vty_out(vty, "%% ERROR: 'hopping enabled 1' works only with" + " 'immediate-assignment post-chan-ack', see timeslot %s%s", + ts->fi->id, VTY_NEWLINE); + return CMD_WARNING; + } + } + } + } + + bts->imm_ass_time = imm_ass_time; return CMD_SUCCESS; } diff --git a/tests/early_ia.vty b/tests/early_ia.vty new file mode 100644 index 0000000..4ba6b3a --- /dev/null +++ b/tests/early_ia.vty @@ -0,0 +1,24 @@ +OsmoBSC> enable +OsmoBSC# configure terminal +OsmoBSC(config)# network +OsmoBSC(config-net)# bts 0 + +OsmoBSC(config-net-bts)# ### Check that first setting early-IA and then enabling frequency hopping throws an error +OsmoBSC(config-net-bts)# immediate-assignment pre-chan-ack +OsmoBSC(config-net-bts)# trx 0 +OsmoBSC(config-net-bts-trx)# timeslot 2 +OsmoBSC(config-net-bts-trx-ts)# hopping enabled 1 +% ERROR: 'hopping enabled 1' works only with 'immediate-assignment post-chan-ack' +OsmoBSC(config-net-bts-trx-ts)# hopping enabled 0 +OsmoBSC(config-net-bts-trx-ts)# exit +OsmoBSC(config-net-bts-trx)# exit +OsmoBSC(config-net-bts)# immediate-assignment post-chan-ack + +OsmoBSC(config-net-bts)# ### Check that first enabling frequency hopping and then setting early-IA throws an error +OsmoBSC(config-net-bts)# trx 0 +OsmoBSC(config-net-bts-trx)# timeslot 2 +OsmoBSC(config-net-bts-trx-ts)# hopping enabled 1 +OsmoBSC(config-net-bts-trx-ts)# exit +OsmoBSC(config-net-bts-trx)# exit +OsmoBSC(config-net-bts)# immediate-assignment pre-chan-ack +% ERROR: 'hopping enabled 1' works only with 'immediate-assignment post-chan-ack', see timeslot 0-0-2-NONE -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35097?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I8d375e5155be7b53034d5c0be5566d2f33af5db0 Gerrit-Change-Number: 35097 Gerrit-PatchSet: 5 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0