March 2023 - gerrit-log - lists.osmocom.org

[S] Change in osmo-upf[master]: unique_ids_test.c: fix coverity ASSERT_SIDE_EFFECT
by neels 22 Mar '23

22 Mar '23

Attention is currently required from: neels. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-upf/+/32019 to look at the new patch set (#2). Change subject: unique_ids_test.c: fix coverity ASSERT_SIDE_EFFECT ...................................................................... unique_ids_test.c: fix coverity ASSERT_SIDE_EFFECT Do the assignment separately, outside of the assert(). Related: CID#311450 Change-Id: I4490a62f444d5048779c9b184b5f580cecd4c149 --- M tests/unique_ids/unique_ids_test.c M tests/unique_ids/unique_ids_test.err 2 files changed, 24 insertions(+), 8 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-upf refs/changes/19/32019/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/32019 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: I4490a62f444d5048779c9b184b5f580cecd4c149 Gerrit-Change-Number: 32019 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-upf[master]: unique_ids_test.c: fix coverity ASSERT_SIDE_EFFECT
by neels 22 Mar '23

22 Mar '23

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-upf/+/32019 ) Change subject: unique_ids_test.c: fix coverity ASSERT_SIDE_EFFECT ...................................................................... unique_ids_test.c: fix coverity ASSERT_SIDE_EFFECT Do the assignment separately, outside of the assert(). Related: CID#311450 Change-Id: I4490a62f444d5048779c9b184b5f580cecd4c149 --- M tests/unique_ids/unique_ids_test.c 1 file changed, 20 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-upf refs/changes/19/32019/1 diff --git a/tests/unique_ids/unique_ids_test.c b/tests/unique_ids/unique_ids_test.c index 2120ab4..1a9b2b3 100644 --- a/tests/unique_ids/unique_ids_test.c +++ b/tests/unique_ids/unique_ids_test.c @@ -327,7 +327,8 @@ dump_state(); log_assert(s1->up_seid == 1); - log_assert(a = llist_first_entry_or_null(&s1->active_gtp_actions, struct up_gtp_action, entry)); + a = llist_first_entry_or_null(&s1->active_gtp_actions, struct up_gtp_action, entry); + log_assert(a); log_assert(a->kind == UP_GTP_U_TUNMAP); log_assert(a->tunmap.core.tun.local.teid == 1); log_assert(a->tunmap.access.tun.local.teid == 2); @@ -345,7 +346,8 @@ dump_state(); log_assert(s2->up_seid == 2); - log_assert(a = llist_first_entry_or_null(&s2->active_gtp_actions, struct up_gtp_action, entry)); + a = llist_first_entry_or_null(&s2->active_gtp_actions, struct up_gtp_action, entry); + log_assert(a); log_assert(a->kind == UP_GTP_U_TUNMAP); log_assert(a->tunmap.core.tun.local.teid == 3); log_assert(a->tunmap.access.tun.local.teid == 4); @@ -370,7 +372,8 @@ dump_state(); log_assert(s3->up_seid == 1); - log_assert(a = llist_first_entry_or_null(&s3->active_gtp_actions, struct up_gtp_action, entry)); + a = llist_first_entry_or_null(&s3->active_gtp_actions, struct up_gtp_action, entry); + log_assert(a); log_assert(a->kind == UP_GTP_U_TUNMAP); log_assert(a->tunmap.core.tun.local.teid == 1); log_assert(a->tunmap.access.tun.local.teid == 2); @@ -383,7 +386,8 @@ dump_state(); log_assert(s4->up_seid == 3); - log_assert(a = llist_first_entry_or_null(&s4->active_gtp_actions, struct up_gtp_action, entry)); + a = llist_first_entry_or_null(&s4->active_gtp_actions, struct up_gtp_action, entry); + log_assert(a); log_assert(a->kind == UP_GTP_U_TUNMAP); log_assert(a->tunmap.core.tun.local.teid == 5); log_assert(a->tunmap.access.tun.local.teid == 6); -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/32019 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: I4490a62f444d5048779c9b184b5f580cecd4c149 Gerrit-Change-Number: 32019 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in osmo-pcu[master]: pdch_ul_controller: log reserved frame numbers
by pespin 22 Mar '23

22 Mar '23

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31022 ) Change subject: pdch_ul_controller: log reserved frame numbers ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File src/pdch_ul_controller.c: https://gerrit.osmocom.org/c/osmo-pcu/+/31022/comment/06a07c2b_592e44c0 PS3, Line 196: LOGPDCH(ulc->pdch, DRLCMAC, LOGL_DEBUG, "Reserving FN %u for type %s\n", item->fn, get_value_string(pdch_ul_node_names, item->type)); looks like you want to put this in 2 lines? -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31022 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I4277c572a4cc6cbbf3ac4e67442c9036be687627 Gerrit-Change-Number: 31022 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 22 Mar 2023 16:42:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-pcu[master]: pdch_ul_controller: log reserved frame numbers
by dexter 22 Mar '23

22 Mar '23

Attention is currently required from: pespin. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31022 ) Change subject: pdch_ul_controller: log reserved frame numbers ...................................................................... Patch Set 3: (1 comment) File src/pdch_ul_controller.c: https://gerrit.osmocom.org/c/osmo-pcu/+/31022/comment/11c1bb24_3f7bce59 PS1, Line 196: LOGPDCH(ulc->pdch, DRLCMAC, LOGL_DEBUG, "reserving FN %u\n", item->fn); > I meant item->type, not it->type, same as you are already using to log the FN ;) Done -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31022 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I4277c572a4cc6cbbf3ac4e67442c9036be687627 Gerrit-Change-Number: 31022 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 22 Mar 2023 16:40:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-pcu[master]: pdch_ul_controller: log reserved frame numbers
by dexter 22 Mar '23

22 Mar '23

Attention is currently required from: pespin. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-pcu/+/31022 to look at the new patch set (#3). Change subject: pdch_ul_controller: log reserved frame numbers ...................................................................... pdch_ul_controller: log reserved frame numbers When debugging frame number offset problems betwen l1 and below and the upper layers of the PCU, it may be helpful do know which blocks/frame numbers got reserverd for uplink transmissions. Change-Id: I4277c572a4cc6cbbf3ac4e67442c9036be687627 Related: OS#5198 --- M src/pdch_ul_controller.c 1 file changed, 16 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/22/31022/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31022 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I4277c572a4cc6cbbf3ac4e67442c9036be687627 Gerrit-Change-Number: 31022 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in osmo-mgw[master]: mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate()
by pespin 22 Mar '23

22 Mar '23

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/27392 ) Change subject: mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate() ...................................................................... Patch Set 4: (2 comments) File src/libosmo-mgcp/mgcp_codec.c: https://gerrit.osmocom.org/c/osmo-mgw/+/27392/comment/e2cb63a7_759fc0df PS1, Line 369: static bool codecs_same(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *codec_b, bool match_pload_fmt) > I don't think that this is good. I would find this parameter naming very confusing. […] Format conversion *is* transcoding :D https://gerrit.osmocom.org/c/osmo-mgw/+/27392/comment/2a205f76_d0784c93 PS1, Line 431: /* In case we weren't able to find an exact match, we will try to find a match that is the same codec, but the > I do not see where this is not generic. […] I think that was precisely my point at the time (some weeks ago when I wrote my comment). I think you are mixing 2 things in the same function here: * Matching for the exact same codec (aka forwarding) * Matching if 2 codecs can be transcoded with current transcoding support. IMHO those are 2 different functions and they will differ more and more over time as we hopefully start supporting more transcoding at some point. Hence why it is really a good idea to already have 2 functions, not 1. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27392 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ifbd201a2749009a4644a29bd77e1d0fc0c124a9d Gerrit-Change-Number: 27392 Gerrit-PatchSet: 4 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 22 Mar 2023 16:36:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-mgw[master]: mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate()
by dexter 22 Mar '23

22 Mar '23

Attention is currently required from: pespin. Hello Jenkins Builder, msuraev, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/27392 to look at the new patch set (#4). Change subject: mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate() ...................................................................... mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate() The function mgcp_codec_pt_translate is very strict when comparing the codecs to each other to find a matching payload type number to be used on the egress side. This poses a problem when one side uses AMR in bandwith-efficient, while the other side uses AMR in octet-aligned payload type format. To the pt translate function the difference in the payload format will appear as if the codec were different and eventually the payload type number cannot be translated. since osmo-mgw offers conversion between the payload type format it would be no problem to ignore the payload type format when making the translation decision. The only exception here would be if one side would announce AMR two times, the first time with octet-aligned and the second time with bandwith-efficient format. Then we would have to use the payload type number from the exact match. (and skip any formatconversion) To archive such an optimized decision we will first go through the codec lists and perform an exact match. If we don't get a match we go through the codec lists a second time, but this time we ignore the payload format. Change-Id: Ifbd201a2749009a4644a29bd77e1d0fc0c124a9d Related: OS#5461 --- M src/libosmo-mgcp/mgcp_codec.c M tests/mgcp/mgcp_test.c M tests/mgcp/mgcp_test.ok 3 files changed, 92 insertions(+), 14 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/92/27392/4 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27392 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ifbd201a2749009a4644a29bd77e1d0fc0c124a9d Gerrit-Change-Number: 27392 Gerrit-PatchSet: 4 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-mgw[master]: Revert "mgcp_codec: do not differentiate between oa and bwe when comp...
by dexter 22 Mar '23

22 Mar '23

Hello Jenkins Builder, pespin, msuraev, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/27391 to look at the new patch set (#3). Change subject: Revert "mgcp_codec: do not differentiate between oa and bwe when comparing codec" ...................................................................... Revert "mgcp_codec: do not differentiate between oa and bwe when comparing codec" This reverts commit e0058b7207e022b698aea10f96cc7c0b1209058a. The reason for this revert is that the solution in the reverted patch does not cover a situation where the other side announces both payload formats at the same time. It could be that the end facing to a transit network announces both formats under two different payload types. In this case no conversion would be necessary. Depending on the input format the output would be send to the transit network under the payload type that matches and no conversion would happen at all. This revert re-intruduces the problem that was fixed in the patch before. Therefore it must be merged together with the follow up patch (Ifbd201a2749009a4644a29bd77e1d0fc0c124a9d) that contains the proper fix. Change-Id: I0b2854ef2397f38606fab3425be586a3d0ca27d1 Related: OS#5461 --- M src/libosmo-mgcp/mgcp_codec.c M tests/mgcp/mgcp_test.c M tests/mgcp/mgcp_test.ok 3 files changed, 39 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/91/27391/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27391 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I0b2854ef2397f38606fab3425be586a3d0ca27d1 Gerrit-Change-Number: 27391 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-mgw[master]: Revert "mgcp_codec: do not differentiate between oa and bwe when comp...
by dexter 22 Mar '23

22 Mar '23

dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/27391 ) Change subject: Revert "mgcp_codec: do not differentiate between oa and bwe when comparing codec" ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: CAUTION: This is just a revert patch, it has to be merged together with the follow up patch mentioned in the commit message. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27391 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I0b2854ef2397f38606fab3425be586a3d0ca27d1 Gerrit-Change-Number: 27391 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Wed, 22 Mar 2023 16:11:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in osmo-mgw[master]: mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate()
by dexter 22 Mar '23

22 Mar '23

Attention is currently required from: pespin. Hello Jenkins Builder, msuraev, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/27392 to look at the new patch set (#3). Change subject: mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate() ...................................................................... mgcp_codec: fix oa/bwe comparison in mgcp_codec_pt_translate() The function mgcp_codec_pt_translate is very strict when comparing the codecs to each other to find a matching payload type number to be used on the egress side. This poses a problem when one side uses AMR in bandwith-efficient, while the other side uses AMR in octet-aligned payload type format. To the pt translate function the difference in the payload format will appear as if the codec were different and eventually the payload type number cannot be translated. since osmo-mgw offers conversion between the payload type format it would be no problem to ignore the payload type format when making the translation decision. The only exception here would be if one side would announce AMR two times, the first time with octet-aligned and the second time with bandwith-efficient format. Then we would have to use the payload type number from the exact match. (and skip any formatconversion) To archive such an optimized decision we will first go through the codec lists and perform an exact match. If we don't get a match we go through the codec lists a second time, but this time we ignore the payload format. Change-Id: Ifbd201a2749009a4644a29bd77e1d0fc0c124a9d Related: OS#5461 --- M src/libosmo-mgcp/mgcp_codec.c M tests/mgcp/mgcp_test.c M tests/mgcp/mgcp_test.ok 3 files changed, 92 insertions(+), 14 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/92/27392/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27392 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ifbd201a2749009a4644a29bd77e1d0fc0c124a9d Gerrit-Change-Number: 27392 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0