February 2022 - gerrit-log - lists.osmocom.org

Change in osmo-mgw[2021q4]: Make function amr_is_octet_aligned publicly available

by pespin

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/27229 ) Change subject: Make function amr_is_octet_aligned publicly available ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27229 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: 2021q4 Gerrit-Change-Id: Iffaf90c1f713feef0c609a7581a346f5f28141d9 Gerrit-Change-Number: 27229 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: Jenkins Builder Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 16 Feb 2022 17:01:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

3 years, 4 months

1
0
0 0

Change in osmo-mgw[2021q4]: mgcp_codec: do not differentiate between oa and bwe when comparing codec

by dexter

Attention is currently required from: dexter. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/27227 to look at the new patch set (#2). Change subject: mgcp_codec: do not differentiate between oa and bwe when comparing codec ...................................................................... mgcp_codec: do not differentiate between oa and bwe when comparing codec AMR that has the payload format bandwith-efficient is the same codec as AMR that has the payload format octet-aligned. Its the same codec, and a comparison of the codec info with the function codecs_same() should return true (=equal). The affected function codecs_same() is used by mgcp_codec_pt_translate(). When the egress payload type number is looked up, the ingress and egress codec information is compared. When one end is using AMR in bandwith-efficient format and the other end is using it in octet-alingned format. Then the codec still must be recognized as the same codec. Othersiwse the payload type number translation would not work, even though the codec is the same on both sides. Change-Id: I64731570c287a75d39c79c10e1bc09a37bdd54d6 Related: SYS#5834 --- M src/libosmo-mgcp/mgcp_codec.c M tests/mgcp/mgcp_test.c M tests/mgcp/mgcp_test.ok 3 files changed, 13 insertions(+), 28 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/27/27227/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27227 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: 2021q4 Gerrit-Change-Id: I64731570c287a75d39c79c10e1bc09a37bdd54d6 Gerrit-Change-Number: 27227 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newpatchset

3 years, 4 months

1
0
0 0

Change in osmo-mgw[2021q4]: Make function amr_is_octet_aligned publicly available

by dexter

dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/27229 ) Change subject: Make function amr_is_octet_aligned publicly available ...................................................................... Make function amr_is_octet_aligned publicly available it will be used by mgcp_iuup.c in follow-up patch. Change-Id: Iffaf90c1f713feef0c609a7581a346f5f28141d9 --- M include/osmocom/mgcp/mgcp_codec.h M src/libosmo-mgcp/mgcp_codec.c 2 files changed, 3 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/29/27229/1 diff --git a/include/osmocom/mgcp/mgcp_codec.h b/include/osmocom/mgcp/mgcp_codec.h index 97650e4..97e6b8d 100644 --- a/include/osmocom/mgcp/mgcp_codec.h +++ b/include/osmocom/mgcp/mgcp_codec.h @@ -17,3 +17,4 @@ int mgcp_codec_pt_translate(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp *conn_dst, int payload_type); const struct mgcp_rtp_codec *mgcp_codec_pt_find_by_subtype_name(struct mgcp_conn_rtp *conn, const char *subtype_name, unsigned int match_nr); +bool mgcp_codec_amr_is_octet_aligned(const struct mgcp_rtp_codec *codec); diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c index 7ab2a17..1b835c9 100644 --- a/src/libosmo-mgcp/mgcp_codec.c +++ b/src/libosmo-mgcp/mgcp_codec.c @@ -355,7 +355,7 @@ * * https://tools.ietf.org/html/rfc4867 */ -static bool amr_is_octet_aligned(const struct mgcp_rtp_codec *codec) +bool mgcp_codec_amr_is_octet_aligned(const struct mgcp_rtp_codec *codec) { if (!codec->param_present) return false; @@ -379,7 +379,7 @@ if (strcmp(codec_a->subtype_name, codec_b->subtype_name)) return false; if (!strcmp(codec_a->subtype_name, "AMR")) { - if (amr_is_octet_aligned(codec_a) != amr_is_octet_aligned(codec_b)) + if (mgcp_codec_amr_is_octet_aligned(codec_a) != mgcp_codec_amr_is_octet_aligned(codec_b)) return false; } -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27229 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: 2021q4 Gerrit-Change-Id: Iffaf90c1f713feef0c609a7581a346f5f28141d9 Gerrit-Change-Number: 27229 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

3 years, 4 months

1
0
0 0

Change in libosmocore[master]: gsm: [ABI BREAK] Support CellId SAI, change CellId CGI-PS id number

by pespin

Attention is currently required from: pespin. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/27224 to look at the new patch set (#3). Change subject: gsm: [ABI BREAK] Support CellId SAI, change CellId CGI-PS id number ...................................................................... gsm: [ABI BREAK] Support CellId SAI, change CellId CGI-PS id number Those are available in 3GPP TS 48.008 version 16.0.0 Release 16, section 3.2.2.17 Cell Identifier. It can be seen that we have a collision between the osmocom non-standard format and the SAI standard one. This is because CGI-PS is not really a TS 48.008 Cell Identifier, but only specified in TS 48.018 and has no ID number assigned. The CGI-PS was added there because the whole osmo-bsc neighbour configuration works with CellIds to manage neighbours, so it felt natural to extend the APIs to also provide means to use CGI-PS format (TS 48.018 even refers 48.008 existance and mentions there's no explicit ID). At the time this Cell Identifier was added, the firstly available number (11) was taken, which was of course a really bad idea since newer versions of the spec can at some point use it, which is the case if one checks for instance TS 48.008 Release 16 SAI Cell Id. There no perfect way to fix this bad decision at the time, but the CGI-PS is only used in osmo-bsc and only for RIM related purposes, so by changing the ID of CELL_IDENT_WHOLE_GLOBAL_PS, we only break RIM under some specific CIs being used, and when an osmo-bsc is built against older libosmocore and then used at runtime against a newer libosmocore (which should be rare). Hence, the downside is acceptable, and by moving the new ID number to be ouside of the spec proto TS 48.008 range (4 bits), we make sure we don't have the same problem again in the future. Related: SYS#5838 Fixes: ca33a71ca8eeaee98b1b53d5394b147a4ff0b429 Change-Id: Id25e563febdb7640174540136225f399515a0089 --- M TODO-RELEASE M include/osmocom/gsm/gsm0808_utils.h M include/osmocom/gsm/gsm23003.h M include/osmocom/gsm/protocol/gsm_08_08.h M src/gsm/gsm0808_utils.c M src/gsm/gsm23003.c M tests/gsm0808/gsm0808_test.c 7 files changed, 127 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/24/27224/3 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/27224 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id25e563febdb7640174540136225f399515a0089 Gerrit-Change-Number: 27224 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

3 years, 4 months

1
0
0 0

Change in osmo-bsc[master]: bts: Add explicit swithch case for Cell Id SAI

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/27228 ) Change subject: bts: Add explicit swithch case for Cell Id SAI ...................................................................... bts: Add explicit swithch case for Cell Id SAI This patch imposes no logical change in the code on itself, but makes sure people compiling osmo-bsc uses an old enough libosmocore implementing Cell Identifier SAI. This is important since adding the SAI ID (CELL_IDENT_SAI) displaced CELL_IDENT_WHOLE_GLOBAL_PS to a new number outside of the 3GPP range for cell IDS (4 bits, this way we garantee we won't have the same problem again). This means there was an ABI breakage (see Depends below). As a result, using an osmo-bsc compiled against an older libosmocore , and then using at runtime against a newer version of libosmocore, will most probably provoke some RIM features to not work properly, since libosmocore will handle CGI-PS cell ids sent by osmo-bsc as SAI ones, and most probably do wrong comparisons when matching (they only match up to LAI included). ABI break analysis: osmo-bsc uses CELL_IDENT_WHOLE_GLOBAL_PS in: * gsm0808_dec_cell_id_list2() -> this is called on stuff received from the network, so it's actually fine handling it correctly as CELL_IDENT_UTRAN_SAI instead of CGI_PS. * gsm0808_cell_id_list_add same_cell_id_list_entries gsm0808_enc_cell_id_list2 cell_id_to_cgi-> On old osmo-bsc, When CELL_IDENT_WHOLE_GLOBAL_PS is passed to be encoded as CGI, RAC byte is taken for encoding instead of 2nd CI byte. * gsm0808_cell_ids_match gsm0808_cell_id_u_match cell_id_to_cgi -> If CELL_IDENT_WHOLE_GLOBAL_PS as 0x11 (CELL_IDENT_UTRAN_SAI), 1 byte offset when comparing (1 byte of RAC is taken converting to CGI instead of the 2nd byte of CI). That means match would be wrong if 2nd byte of CI differs. Related: SYS#5838 Depends: libosmocore.git Change-Id Id25e563febdb7640174540136225f399515a0089 Change-Id: I70972efffefd57fd36332fab539683696c32f4a5 --- M TODO-RELEASE M src/osmo-bsc/bts.c 2 files changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/28/27228/1 diff --git a/TODO-RELEASE b/TODO-RELEASE index d0852fc..e0c43d9 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -7,3 +7,4 @@ # If any interfaces have been added since the last public release: c:r:a + 1. # If any interfaces have been removed or changed since the last public release: c:r:0. #library what description / commit summary line +libosmogsm >1.6.0 required due to CELL_IDENT_WHOLE_GLOBAL_PS/SAI ABI breakage diff --git a/src/osmo-bsc/bts.c b/src/osmo-bsc/bts.c index 1461585..c3e87b3 100644 --- a/src/osmo-bsc/bts.c +++ b/src/osmo-bsc/bts.c @@ -524,6 +524,7 @@ case CELL_IDENT_UTRAN_PLMN_LAC_RNC: case CELL_IDENT_UTRAN_RNC: case CELL_IDENT_UTRAN_LAC_RNC: + case CELL_IDENT_SAI: return false; default: OSMO_ASSERT(false); -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27228 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I70972efffefd57fd36332fab539683696c32f4a5 Gerrit-Change-Number: 27228 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

3 years, 4 months

1
0
0 0

Change in libosmocore[master]: gsm: [ABI BREAK] Support CellId SAI, change CellId CGI-PS id number

by pespin

Attention is currently required from: pespin. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/27224 to look at the new patch set (#2). Change subject: gsm: [ABI BREAK] Support CellId SAI, change CellId CGI-PS id number ...................................................................... gsm: [ABI BREAK] Support CellId SAI, change CellId CGI-PS id number Those are available in 3GPP TS 48.008 version 16.0.0 Release 16, section 3.2.2.17 Cell Identifier. It can be seen that we have a collision between the osmocom non-standard format and the SAI standard one. This is because CGI-PS is not really a TS 48.008 Cell Identifier, but only specified in TS 48.018 and has no ID number assigned. The CGI-PS was added there because the whole osmo-bsc neighbour configuration works with CellIds to manage neighbours, so it felt natural to extend the APIs to also provide means to use CGI-PS format (TS 48.018 even refers 48.008 existance and mentions there's no explicit ID). At the time this Cell Identifier was added, the firstly available number (11) was taken, which was of course a really bad idea since newer versions of the spec can at some point use it, which is the case if one checks for instance TS 48.008 Release 16 SAI Cell Id. There no perfect way to fix this bad decision at the time, but the CGI-PS is only used in osmo-bsc and only for RIM related purposes, so by changing the ID of CELL_IDENT_WHOLE_GLOBAL_PS, we only break RIM under some specific CIs being used, and when an osmo-bsc is built against older libosmocore and then used at runtime against a newer libosmocore (which should be rare). Hence, the downside is acceptable, and by moving the new ID number to be ouside of the spec proto TS 48.008 range (4 bits), we make sure we don't have the same problem again in the future. Related: SYS#5838 Fixes: ca33a71ca8eeaee98b1b53d5394b147a4ff0b429 Change-Id: Id25e563febdb7640174540136225f399515a0089 --- M include/osmocom/gsm/gsm0808_utils.h M include/osmocom/gsm/gsm23003.h M include/osmocom/gsm/protocol/gsm_08_08.h M src/gsm/gsm0808_utils.c M src/gsm/gsm23003.c M tests/gsm0808/gsm0808_test.c 6 files changed, 126 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/24/27224/2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/27224 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id25e563febdb7640174540136225f399515a0089 Gerrit-Change-Number: 27224 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

3 years, 4 months

1
0
0 0

Change in osmo-mgw[2021q4]: mgcp_codec: do not differentiate between oa and bwe when comparing codec

by dexter

dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/27227 ) Change subject: mgcp_codec: do not differentiate between oa and bwe when comparing codec ...................................................................... mgcp_codec: do not differentiate between oa and bwe when comparing codec AMR that has the payload format bandwith-efficient is the same codec as AMR that has the payload format octet-aligned. Its the same codec, and a comparison of the codec info with the function codecs_same() should return true (=equal). The affected function codecs_same() is used by mgcp_codec_pt_translate(). When the egress payload type number is looked up, the ingress and egress codec information is compared. When one end is using AMR in bandwith-efficient format and the other end is using it in octet-alingned format. Then the codec still must be recognized as the same codec. Othersiwse the payload type number translation would not work, even though the codec is the same on both sides. Change-Id: I64731570c287a75d39c79c10e1bc09a37bdd54d6 Related: SYS#5834 --- M src/libosmo-mgcp/mgcp_codec.c M tests/mgcp/mgcp_test.c M tests/mgcp/mgcp_test.ok 3 files changed, 13 insertions(+), 28 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/27/27227/1 diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c index 7ab2a17..57dcff4 100644 --- a/src/libosmo-mgcp/mgcp_codec.c +++ b/src/libosmo-mgcp/mgcp_codec.c @@ -378,10 +378,10 @@ return false; if (strcmp(codec_a->subtype_name, codec_b->subtype_name)) return false; - if (!strcmp(codec_a->subtype_name, "AMR")) { - if (amr_is_octet_aligned(codec_a) != amr_is_octet_aligned(codec_b)) - return false; - } + + /* Note: AMR allows to set the RTP payload format to octet-aligned or bandwith-efficient (octet-aligned=0) + * via SDP. This difference concerns payload format only, but not the actual codec. It is not a difference + * within the meaning of this function. */ return true; } diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index a8aad14..e6ff0a5 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -1896,16 +1896,13 @@ .codecs = { { { 111, "AMR/8000", &amr_param_octet_aligned_true, }, - { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { { 122, "AMR/8000", &amr_param_octet_aligned_false, }, - { 121, "AMR/8000", &amr_param_octet_aligned_true, }, }, }, .expect = { - { .payload_type_map = {111, 121}, }, - { .payload_type_map = {112, 122} }, + { .payload_type_map = {111, 122}, }, { .end = true }, }, }, @@ -1914,15 +1911,13 @@ .codecs = { { { 111, "AMR/8000", &amr_param_octet_aligned_true, }, - { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { { 122, "AMR/8000", &amr_param_octet_aligned_unset, }, }, }, .expect = { - { .payload_type_map = {111, -EINVAL}, }, - { .payload_type_map = {112, 122} }, + { .payload_type_map = {111, 122}, }, { .end = true }, }, }, @@ -1931,15 +1926,13 @@ .codecs = { { { 111, "AMR/8000", &amr_param_octet_aligned_true, }, - { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { { 122, "AMR/8000", NULL, }, }, }, .expect = { - { .payload_type_map = {111, -EINVAL}, }, - { .payload_type_map = {112, 122} }, + { .payload_type_map = {111, 122}, }, { .end = true }, }, }, diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok index 94fada3..5b8d558 100644 --- a/tests/mgcp/mgcp_test.ok +++ b/tests/mgcp/mgcp_test.ok @@ -1347,32 +1347,24 @@ #5: test AMR with differing octet-aligned settings - add codecs on conn0: 0: 111 AMR/8000 octet-aligned=1 -> rc=0 - 1: 112 AMR/8000 octet-aligned=0 -> rc=0 - add codecs on conn1: 0: 122 AMR/8000 octet-aligned=0 -> rc=0 - 1: 121 AMR/8000 octet-aligned=1 -> rc=0 - - mgcp_codec_pt_translate(conn0, conn1, 111) -> 121 - - mgcp_codec_pt_translate(conn1, conn0, 121) -> 111 - - mgcp_codec_pt_translate(conn0, conn1, 112) -> 122 - - mgcp_codec_pt_translate(conn1, conn0, 122) -> 112 + - mgcp_codec_pt_translate(conn0, conn1, 111) -> 122 + - mgcp_codec_pt_translate(conn1, conn0, 122) -> 111 #6: test AMR with missing octet-aligned settings (defaults to 0) - add codecs on conn0: 0: 111 AMR/8000 octet-aligned=1 -> rc=0 - 1: 112 AMR/8000 octet-aligned=0 -> rc=0 - add codecs on conn1: 0: 122 AMR/8000 octet-aligned=unset -> rc=0 - - mgcp_codec_pt_translate(conn0, conn1, 111) -> -22 - - mgcp_codec_pt_translate(conn0, conn1, 112) -> 122 - - mgcp_codec_pt_translate(conn1, conn0, 122) -> 112 + - mgcp_codec_pt_translate(conn0, conn1, 111) -> 122 + - mgcp_codec_pt_translate(conn1, conn0, 122) -> 111 #7: test AMR with NULL param (defaults to 0) - add codecs on conn0: 0: 111 AMR/8000 octet-aligned=1 -> rc=0 - 1: 112 AMR/8000 octet-aligned=0 -> rc=0 - add codecs on conn1: 0: 122 AMR/8000 -> rc=0 - - mgcp_codec_pt_translate(conn0, conn1, 111) -> -22 - - mgcp_codec_pt_translate(conn0, conn1, 112) -> 122 - - mgcp_codec_pt_translate(conn1, conn0, 122) -> 112 + - mgcp_codec_pt_translate(conn0, conn1, 111) -> 122 + - mgcp_codec_pt_translate(conn1, conn0, 122) -> 111 #8: match FOO/8000/1 and FOO/8000 as identical, single channel is implicit - add codecs on conn0: 0: 0 PCMU/8000/1 -> rc=0 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27227 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: 2021q4 Gerrit-Change-Id: I64731570c287a75d39c79c10e1bc09a37bdd54d6 Gerrit-Change-Number: 27227 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newchange

3 years, 4 months

1
0
0 0

Change in osmo-mgw[master]: mgcp_codec: do not differentiate between oa and bwe when comparing codec

by dexter

dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/27223 ) Change subject: mgcp_codec: do not differentiate between oa and bwe when comparing codec ...................................................................... mgcp_codec: do not differentiate between oa and bwe when comparing codec AMR that has the payload format bandwith-efficient is the same codec as AMR that has the payload format octet-aligned. Its the same codec, and a comparison of the codec info with the function codecs_same() should return true (=equal). The affected function codecs_same() is used by mgcp_codec_pt_translate(). When the egress payload type number is looked up, the ingress and egress codec information is compared. When one end is using AMR in bandwith-efficient format and the other end is using it in octet-alingned format. Then the codec still must be recognized as the same codec. Othersiwse the payload type number translation would not work, even though the codec is the same on both sides. Change-Id: I64731570c287a75d39c79c10e1bc09a37bdd54d6 Related: SYS#5834 --- M src/libosmo-mgcp/mgcp_codec.c M tests/mgcp/mgcp_test.c M tests/mgcp/mgcp_test.ok 3 files changed, 13 insertions(+), 28 deletions(-) Approvals: daniel: Looks good to me, but someone else must approve dexter: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c index 6b8d14a..38aa0a7 100644 --- a/src/libosmo-mgcp/mgcp_codec.c +++ b/src/libosmo-mgcp/mgcp_codec.c @@ -378,10 +378,10 @@ return false; if (strcmp(codec_a->subtype_name, codec_b->subtype_name)) return false; - if (!strcmp(codec_a->subtype_name, "AMR")) { - if (mgcp_codec_amr_is_octet_aligned(codec_a) != mgcp_codec_amr_is_octet_aligned(codec_b)) - return false; - } + + /* Note: AMR allows to set the RTP payload format to octet-aligned or bandwith-efficient (octet-aligned=0) + * via SDP. This difference concerns payload format only, but not the actual codec. It is not a difference + * within the meaning of this function. */ return true; } diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index 9b4933c..4ed18c2 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -1896,16 +1896,13 @@ .codecs = { { { 111, "AMR/8000", &amr_param_octet_aligned_true, }, - { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { { 122, "AMR/8000", &amr_param_octet_aligned_false, }, - { 121, "AMR/8000", &amr_param_octet_aligned_true, }, }, }, .expect = { - { .payload_type_map = {111, 121}, }, - { .payload_type_map = {112, 122} }, + { .payload_type_map = {111, 122}, }, { .end = true }, }, }, @@ -1914,15 +1911,13 @@ .codecs = { { { 111, "AMR/8000", &amr_param_octet_aligned_true, }, - { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { { 122, "AMR/8000", &amr_param_octet_aligned_unset, }, }, }, .expect = { - { .payload_type_map = {111, -EINVAL}, }, - { .payload_type_map = {112, 122} }, + { .payload_type_map = {111, 122}, }, { .end = true }, }, }, @@ -1931,15 +1926,13 @@ .codecs = { { { 111, "AMR/8000", &amr_param_octet_aligned_true, }, - { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { { 122, "AMR/8000", NULL, }, }, }, .expect = { - { .payload_type_map = {111, -EINVAL}, }, - { .payload_type_map = {112, 122} }, + { .payload_type_map = {111, 122}, }, { .end = true }, }, }, diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok index 94fada3..5b8d558 100644 --- a/tests/mgcp/mgcp_test.ok +++ b/tests/mgcp/mgcp_test.ok @@ -1347,32 +1347,24 @@ #5: test AMR with differing octet-aligned settings - add codecs on conn0: 0: 111 AMR/8000 octet-aligned=1 -> rc=0 - 1: 112 AMR/8000 octet-aligned=0 -> rc=0 - add codecs on conn1: 0: 122 AMR/8000 octet-aligned=0 -> rc=0 - 1: 121 AMR/8000 octet-aligned=1 -> rc=0 - - mgcp_codec_pt_translate(conn0, conn1, 111) -> 121 - - mgcp_codec_pt_translate(conn1, conn0, 121) -> 111 - - mgcp_codec_pt_translate(conn0, conn1, 112) -> 122 - - mgcp_codec_pt_translate(conn1, conn0, 122) -> 112 + - mgcp_codec_pt_translate(conn0, conn1, 111) -> 122 + - mgcp_codec_pt_translate(conn1, conn0, 122) -> 111 #6: test AMR with missing octet-aligned settings (defaults to 0) - add codecs on conn0: 0: 111 AMR/8000 octet-aligned=1 -> rc=0 - 1: 112 AMR/8000 octet-aligned=0 -> rc=0 - add codecs on conn1: 0: 122 AMR/8000 octet-aligned=unset -> rc=0 - - mgcp_codec_pt_translate(conn0, conn1, 111) -> -22 - - mgcp_codec_pt_translate(conn0, conn1, 112) -> 122 - - mgcp_codec_pt_translate(conn1, conn0, 122) -> 112 + - mgcp_codec_pt_translate(conn0, conn1, 111) -> 122 + - mgcp_codec_pt_translate(conn1, conn0, 122) -> 111 #7: test AMR with NULL param (defaults to 0) - add codecs on conn0: 0: 111 AMR/8000 octet-aligned=1 -> rc=0 - 1: 112 AMR/8000 octet-aligned=0 -> rc=0 - add codecs on conn1: 0: 122 AMR/8000 -> rc=0 - - mgcp_codec_pt_translate(conn0, conn1, 111) -> -22 - - mgcp_codec_pt_translate(conn0, conn1, 112) -> 122 - - mgcp_codec_pt_translate(conn1, conn0, 122) -> 112 + - mgcp_codec_pt_translate(conn0, conn1, 111) -> 122 + - mgcp_codec_pt_translate(conn1, conn0, 122) -> 111 #8: match FOO/8000/1 and FOO/8000 as identical, single channel is implicit - add codecs on conn0: 0: 0 PCMU/8000/1 -> rc=0 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27223 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I64731570c287a75d39c79c10e1bc09a37bdd54d6 Gerrit-Change-Number: 27223 Gerrit-PatchSet: 4 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

3 years, 4 months

1
0
0 0

Change in osmo-mgw[master]: mgcp_codec: do not differentiate between oa and bwe when comparing codec

by dexter

Attention is currently required from: osmith, neels, laforge, pespin, lynxis lazus. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/27223 ) Change subject: mgcp_codec: do not differentiate between oa and bwe when comparing codec ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27223 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I64731570c287a75d39c79c10e1bc09a37bdd54d6 Gerrit-Change-Number: 27223 Gerrit-PatchSet: 4 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Wed, 16 Feb 2022 16:41:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

3 years, 4 months

1
0
0 0

Change in osmo-ttcn3-hacks[master]: ggsn_tests: Early exit and improve logging on some setverdict fail cases

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/27226 ) Change subject: ggsn_tests: Early exit and improve logging on some setverdict fail cases ...................................................................... ggsn_tests: Early exit and improve logging on some setverdict fail cases Change-Id: I25b84419b2278ce883b0dbc526ab7ffe30b17d0c --- M ggsn_tests/GGSN_Tests.ttcn 1 file changed, 22 insertions(+), 13 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/26/27226/1 diff --git a/ggsn_tests/GGSN_Tests.ttcn b/ggsn_tests/GGSN_Tests.ttcn index 48e87bf..675c435 100644 --- a/ggsn_tests/GGSN_Tests.ttcn +++ b/ggsn_tests/GGSN_Tests.ttcn @@ -337,27 +337,30 @@ /* Check if EUA type corresponds to requested type */ if (match(ctx.eua, t_EuaIPv4(?)) and not match(cpr.endUserAddress, tr_EuaIPv4(?))){ - setverdict(fail); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "EUAv4 expectancies didn't match"); } if (match(ctx.eua, t_EuaIPv6(?)) and not match(cpr.endUserAddress, tr_EuaIPv6(?))) { - setverdict(fail); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "EUAv6 expectancies didn't match"); } if (match(ctx.eua, t_EuaIPv4v6(?, ?)) and not match(cpr.endUserAddress, tr_EuaIPv4v6(?, ?))) { - setverdict(fail); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "EUAv4v6 expectancies didn't match"); } /* Check if PCO response corresponds to request */ if (ispresent(ctx.pco_req)) { if (match(ctx.pco_req, ts_PCO_IPv4_DNS_CONT) and not match(cpr.protConfigOptions, tr_PCO_IPv4_DNS_CONT_resp(?))) { - log("IPv4 DNS Container requested, but missing"); - setverdict(fail); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "IPv4 DNS Container requested, but missing"); } if (match(ctx.pco_req, ts_PCO_IPv6_DNS) and not match(cpr.protConfigOptions, tr_PCO_IPv6_DNS_resp(?))) { - log("IPv6 DNS Container requested, but missing"); - setverdict(fail); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "IPv6 DNS Container requested, but missing"); } } ctx.teid_remote := cpr.teidDataI.teidDataI; @@ -372,7 +375,8 @@ } setverdict(pass); } else { - setverdict(fail); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "CreatePDPContextResp: cause expectancies didn't match"); } } @@ -388,7 +392,8 @@ } else if (exp_cause != '80'O and exp_cause == upr.cause.causevalue) { setverdict(pass); } else { - setverdict(fail); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "UpdatePDPContextResp: cause expectancies didn't match"); } } @@ -409,8 +414,8 @@ req_type, req_num)); } [] DIAMETER_UNIT.receive(PDU_DIAMETER:?) -> value rx_dia { - setverdict(fail, "Received unexpected DIAMETER ", rx_dia); - self.stop; + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + log2str("Received unexpected DIAMETER ", rx_dia)); } } @@ -452,13 +457,16 @@ ud.gtpc.gtpc_pdu.deletePDPContextRequest.cause.causevalue == valueof(expect_cause)) { setverdict(pass); } else { - setverdict(fail); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "DeletePDPContextReq: cause expectancies didn't match"); } if (expect_teardown == ispresent(ud.gtpc.gtpc_pdu.deletePDPContextRequest.tearDownIndicator)) { setverdict(pass); } else { setverdict(fail); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "DeletePDPContextReq: tearDownIndicator expectancies didn't match"); } } } @@ -490,7 +498,8 @@ if (ud.gtpc.gtpc_pdu.deletePDPContextResponse.cause.causevalue == expect_causevalue) { setverdict(pass); } else { - setverdict(fail); + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, + "DeletePDPContextResp: cause expectancies didn't match"); } } } -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/27226 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: I25b84419b2278ce883b0dbc526ab7ffe30b17d0c Gerrit-Change-Number: 27226 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

3 years, 4 months

1
0
0 0

2025

2024

2023

2022

gerrit-log February 2022