Attention is currently required from: fixeria, laforge, pespin.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email
to look at the new patch set (#12).
The following approvals got outdated and were removed:
Code-Review+1 by laforge, Code-Review+1 by pespin, Verified+1 by Jenkins Builder
Change subject: client: collapse codecs[] and ptmap[]; allow codec variants
......................................................................
client: collapse codecs[] and ptmap[]; allow codec variants
codecs[] is an array of enum osmo_mgcp_codecs.
ptmap[] is an array of { enum osmo_mgcp_codecs, unsigned int ptmap }.
MGCP lists first a bunch of payload type numbers and then specifies them
again for details, like the numbers 112, 96, 3 in this example:
m=audio <port> RTP/AVP 112 96 3
a=rtpmap:112 AMR/8000
a=rtpmap:96 VND.3GPP.IUFP/16000
a=rtpmap:3 GSM-FR/8000
So far we keep these lists in two separate arrays:
- codecs[], codecs_len stores the 'm=audio' list
- ptmap[], ptmap_len stores the 'a=rtpmap' list (and may omit some
elements present in codecs[])
This applies to both struct mgcp_response and struct mgcp_msg.
These are semantically identical, and the separation leads to checks,
conversions and dear hopes of correct ordering.
So let's keep only one list with both codec and payload type number in
it. The 'm=audio' list establishes the order of the pt numbers, and the
'a=rtpmap' list adds codec information to the established entries.
In the internal API structs mgcp_msg and mgcp_response, just drop the
codecs[] entirely.
In public API struct mgcp_conn_peer, keep the codecs[] array, mark it
deprecated, and provide a backwards compat conversion: if any caller
invokes mgcp_conn_create() or mgcp_conn_modify() with codecs[] still
present in the verb_info arg, then move codecs[] entries over to the
ptmap[] array in a sensible way.
(BTW, even mgcp_conn_create() and mgcp_conn_modify() are never called
from outside of libosmo-mgcp-client in any of our osmo-cni programs;
users call osmo_mgcpc_ep_ci_add() and osmo_mgcpc_ep_ci_request(), which
in turn may pass user-provided codecs[] lists on to mgcp_conn_create() or
mgcp_conn_modify().)
Tests for parsing the MGCP response are mostly missing. They will be
added in upcoming patch I842ce65a9a70f313570857b7df53727cc572b9e6,
because they will be using only the new ptmap API.
Related: OS#6171
Change-Id: I798e02c6663376d3d52f4a74fc4b32411ce95bed
---
M TODO-RELEASE
M include/osmocom/mgcp_client/mgcp_client.h
M include/osmocom/mgcp_client/mgcp_client_fsm.h
M include/osmocom/mgcp_client/mgcp_client_internal.h
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp-client/mgcp_client_fsm.c
M tests/mgcp_client/mgcp_client_test.c
7 files changed, 227 insertions(+), 74 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/99/34899/12
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed
Gerrit-Change-Number: 34899
Gerrit-PatchSet: 12
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-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
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 1:
(2 comments)
File src/libosmo-mgcp-client/mgcp_client.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35421/comment/4b9f502b_43a4d7bf
PS1, Line 376: LOGP(DLMGCP, LOGL_ERROR,
> it would be the other one below, with the "_pt:" label...
Done
https://gerrit.osmocom.org/c/osmo-mgw/+/35421/comment/1fe4df57_b62da484
PS1, Line 382: "Failed to parse SDP parameter payload types (%s)\n", line);
> ...this one. […]
to recap, I don't agree with dropping log of the SDP string that failed.
i'd just leave it as it is, but we could reduce to INFO or DEBUG now?
let me know or i'll resolve this soon.
--
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: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 04 Jan 2024 23:55:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: fixeria, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email )
Change subject: client: collapse codecs[] and ptmap[]; allow codec variants
......................................................................
Patch Set 11:
(1 comment)
Patchset:
PS11:
> You may want to add info about deprecated fields/symbols in TODO-RELEASE.
hm i thought i did
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?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: I798e02c6663376d3d52f4a74fc4b32411ce95bed
Gerrit-Change-Number: 34899
Gerrit-PatchSet: 11
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-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 04 Jan 2024 23:38:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35302?usp=email )
Change subject: mgw: do not fail MGCP on codec mismatch
......................................................................
Patch Set 5:
(1 comment)
File src/libosmo-mgcp/mgcp_codec.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35302/comment/a7f1fb18_7f893f77
PS1, Line 478: LOGP(DLMGCP, LOGL_ERROR, "no matching codec found\n");
> This one has not been addressed.
or i can drop this log again, it was from debugging.
btw, it seems osmo-mgw does a lot of context-less logging in MGCP and SDP.
re unadressed: I probably just pushed a later patch, causing a new patch set here too -- no intention to skip CR still marked unresolved
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35302?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: I3d1163fe622bdd7dc42a485f796072524ab39db9
Gerrit-Change-Number: 35302
Gerrit-PatchSet: 5
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 04 Jan 2024 23:33:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: laforge.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/pysim/+/35488?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: Add pySim.esim.bsp module implementing BSP (BPP Protection Protocol)
......................................................................
Add pySim.esim.bsp module implementing BSP (BPP Protection Protocol)
This is the protocol used for the ES8+ interface between SM-DP+ and the
eUICC in the GSMA eSIM system.
Change-Id: Ic461936f2e68e1e6f7faab33d06acf3063e261e7
---
A pySim/esim/bsp.py
M requirements.txt
A tests/test_esim_bsp.py
3 files changed, 389 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/88/35488/2
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/35488?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: Ic461936f2e68e1e6f7faab33d06acf3063e261e7
Gerrit-Change-Number: 35488
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset