Attention is currently required from: neels.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/37278?usp=email )
Change subject: mgcp_test.c: fix various missing '\r' and '\n'
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/37278?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: I6d530535a3a5f1d1a0716ab9e4a8079ba1de242e
Gerrit-Change-Number: 37278
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 24 Jun 2024 17:32:46 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/37282?usp=email )
Change subject: do not FAIL on CRCX in sendrecv mode
......................................................................
do not FAIL on CRCX in sendrecv mode
Currently, a CRCX in sendrecv mode results in:
DLMGCP ERROR endpoint:rtpbridge/2@mgw CI:7F4C8EDD CRCX: selected connection mode type requires an opposite end! (mgcp_protocol.c:1090)
But it is not actually practical, nor logical to require another conn
for the 'sendrecv' ConnectionMode.
Impractical: If I want to create two conns on an endpoint that both are
in 'sendrecv' mode, I have to send two CRCX, one for each conn. At the
time of the first CRCX, there cannot be any other conn, so I am
currently forced to send a different ConnectionMode in the CRCX,
followed by another MDCX later, just to change the first conn to
sendrecv.
Illogical: In a situation where two conns are currently in sendrecv
mode, if I now DLCX one of them, I can legally reach a state with a
single conn in sendrecv mode, just as currently forbidden for CRCX.
In general, MGCP is not forcing any particular number of connections.
Simply start forwarding RTP as soon as there is a remote conn, and not
require another explicit MDCX.
Related: SYS#6974 SYS#6907
Related: osmo-ttcn3-hacks I00fd854f058f7f53e2f579e8481ca2b9253f08e3
Change-Id: Ic089485543c5c97a35c7ae24fe0f622bf57d1976
---
M src/libosmo-mgcp/mgcp_protocol.c
M tests/mgcp/mgcp_test.c
M tests/mgcp/mgcp_test.ok
3 files changed, 46 insertions(+), 25 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/82/37282/1
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index ac5c682..16dee03 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -1083,17 +1083,6 @@
mgcp_rtp_end_config(endp, 0, &conn->end);
- /* check connection mode setting */
- if (conn->conn->mode != MGCP_CONN_LOOPBACK
- && conn->conn->mode != MGCP_CONN_RECV_ONLY
- && osmo_sockaddr_port(&conn->end.addr.u.sa) == 0) {
- LOGPCONN(_conn, DLMGCP, LOGL_ERROR,
- "CRCX: selected connection mode type requires an opposite end!\n");
- error_code = 527;
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_NO_REMOTE_CONN_DESC));
- goto error2;
- }
-
/* Find a local address for conn based on policy and initial SDP remote
information, then find a free port for it */
if (mgcp_get_local_addr(conn->end.local_addr, conn) < 0) {
@@ -1295,17 +1284,6 @@
if (conn->type == MGCP_RTP_DEFAULT && strcmp(conn->end.codec->subtype_name, "VND.3GPP.IUFP") == 0)
rc = mgcp_conn_iuup_init(conn);
- /* check connection mode setting */
- if (conn->conn->mode != MGCP_CONN_LOOPBACK
- && conn->conn->mode != MGCP_CONN_RECV_ONLY
- && !mgcp_rtp_end_remote_addr_available(&conn->end)) {
- LOGPCONN(conn->conn, DLMGCP, LOGL_ERROR,
- "MDCX: selected connection mode type requires an opposite end!\n");
- error_code = 527;
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_NO_REMOTE_CONN_DESC));
- goto error3;
- }
-
if (mgcp_conn_rtp_is_osmux(conn)) {
OSMO_ASSERT(conn->osmux.local_cid_allocated);
if (remote_osmux_cid < -1) {
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index 2e06639..21be037 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -145,7 +145,16 @@
"a=ptime:20\r\n"
#define MDCX4_ADDR0000_RET \
- "527 18983216 FAIL\r\n"
+ "200 18983216 OK\r\n" \
+ "\r\n" \
+ "v=0\r\n" \
+ "o=- %s 23 IN IP4 0.0.0.0\r\n" \
+ "s=-\r\n" \
+ "c=IN IP4 0.0.0.0\r\n" \
+ "t=0 0\r\n" \
+ "m=audio 16002 RTP/AVP 99\r\n" \
+ "a=rtpmap:99 AMR/8000\r\n" \
+ "a=ptime:20\r\n"
#define MDCX4 \
"MDCX 18983217 1@mgw MGCP 1.0\r\n" \
diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok
index 5dc00cd..4e72145 100644
--- a/tests/mgcp/mgcp_test.ok
+++ b/tests/mgcp/mgcp_test.ok
@@ -116,9 +116,9 @@
---------8<---------
checking response:
-using message as statically defined for comparison
+using message with patched conn_id for comparison
Response matches our expectations.
-(response does not contain a connection id)
+(response contains a connection id)
================================================
Testing MDCX4
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/37282?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: Ic089485543c5c97a35c7ae24fe0f622bf57d1976
Gerrit-Change-Number: 37282
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newchange
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/37279?usp=email )
Change subject: fix MGCP compat with osmo-mgw <= 1.12.2: CRCX in recvonly
......................................................................
fix MGCP compat with osmo-mgw <= 1.12.2: CRCX in recvonly
Fix a recently introduced problem with MGCP to osmo-mgw:
Send the first CRCX in recvonly mode, not sendrecv. osmo-hnbgw always
sends an additional MDCX including sendrecv mode anyway.
osmo-mgw currently forbids sending an initial CRCX in connection mode
'sendrecv', with this error message:
DLMGCP ERROR endpoint:rtpbridge/2@mgw CI:7F4C8EDD CRCX: selected connection mode type requires an opposite end! (mgcp_protocol.c:1090)
I am submitting an osmo-mgw patch to not fail there, but we want to and
can easily be compatible with current and earlier osmo-mgw:
Sending the initial CRCX in sendrecv was introduced in commit:
"drop legacy hack: do not start MGW endp in loopback mode"
da7d33e2841a2be94fd3364dc44abf8068669998
I0eca75d7abf66f8b9fde9c68ec10d4265f64a189
This patch has not been part of a release yet.
The intention of that commit was to get away from loopback mode. The
logical mode to pick instead indeed is sendrecv, but by that osmo-hnbgw
triggers above osmo-mgw error.
Related: SYS#6974 SYS#6907
Related: osmo-mgw Ic089485543c5c97a35c7ae24fe0f622bf57d1976
Change-Id: I004f96ae36774ceb33f177c9f58f820fefa3ca14
---
M src/osmo-hnbgw/mgw_fsm.c
1 file changed, 35 insertions(+), 1 deletion(-)
Approvals:
Jenkins Builder: Verified
neels: Looks good to me, approved
diff --git a/src/osmo-hnbgw/mgw_fsm.c b/src/osmo-hnbgw/mgw_fsm.c
index 48b4899..d44a6e3 100644
--- a/src/osmo-hnbgw/mgw_fsm.c
+++ b/src/osmo-hnbgw/mgw_fsm.c
@@ -175,7 +175,7 @@
mgw_info = (struct mgcp_conn_peer) {
.call_id = (map->rua_ctx_id << 8) | mgw_fsm_priv->rab_id,
.ptime = 20,
- .conn_mode = MGCP_CONN_RECV_SEND,
+ .conn_mode = MGCP_CONN_RECV_ONLY,
};
mgw_info.codecs[0] = CODEC_IUFP;
mgw_info.codecs_len = 1;
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/37279?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I004f96ae36774ceb33f177c9f58f820fefa3ca14
Gerrit-Change-Number: 37279
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: merged
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/37279?usp=email )
Change subject: fix MGCP compat with osmo-mgw <= 1.12.2: CRCX in recvonly
......................................................................
fix MGCP compat with osmo-mgw <= 1.12.2: CRCX in recvonly
Fix a recently introduced problem with MGCP to osmo-mgw:
Send the first CRCX in recvonly mode, not sendrecv. osmo-hnbgw always
sends an additional MDCX including sendrecv mode anyway.
osmo-mgw currently forbids sending an initial CRCX in connection mode
'sendrecv', with this error message:
DLMGCP ERROR endpoint:rtpbridge/2@mgw CI:7F4C8EDD CRCX: selected connection mode type requires an opposite end! (mgcp_protocol.c:1090)
I am submitting an osmo-mgw patch to not fail there, but we want to and
can easily be compatible with current and earlier osmo-mgw:
Sending the initial CRCX in sendrecv was introduced in commit:
"drop legacy hack: do not start MGW endp in loopback mode"
da7d33e2841a2be94fd3364dc44abf8068669998
I0eca75d7abf66f8b9fde9c68ec10d4265f64a189
This patch has not been part of a release yet.
The intention of that commit was to get away from loopback mode. The
logical mode to pick instead indeed is sendrecv, but by that osmo-hnbgw
triggers above osmo-mgw error.
Related: SYS#6974 SYS#6907
Related: osmo-mgw Ic089485543c5c97a35c7ae24fe0f622bf57d1976
Change-Id: I004f96ae36774ceb33f177c9f58f820fefa3ca14
---
M src/osmo-hnbgw/mgw_fsm.c
1 file changed, 35 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/79/37279/1
diff --git a/src/osmo-hnbgw/mgw_fsm.c b/src/osmo-hnbgw/mgw_fsm.c
index 48b4899..d44a6e3 100644
--- a/src/osmo-hnbgw/mgw_fsm.c
+++ b/src/osmo-hnbgw/mgw_fsm.c
@@ -175,7 +175,7 @@
mgw_info = (struct mgcp_conn_peer) {
.call_id = (map->rua_ctx_id << 8) | mgw_fsm_priv->rab_id,
.ptime = 20,
- .conn_mode = MGCP_CONN_RECV_SEND,
+ .conn_mode = MGCP_CONN_RECV_ONLY,
};
mgw_info.codecs[0] = CODEC_IUFP;
mgw_info.codecs_len = 1;
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/37279?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I004f96ae36774ceb33f177c9f58f820fefa3ca14
Gerrit-Change-Number: 37279
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newchange
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-mgw/+/37278?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: mgcp_test.c: fix various missing '\r' and '\n'
......................................................................
mgcp_test.c: fix various missing '\r' and '\n'
The only valid line endings are '\n' and '\r\n'.
We usually use '\r\n' like we were in MSDOS.
MGCP, RFC3435 3.1:
Headers and session descriptions are encoded as a set of text lines,
separated by a carriage return and line feed character (or,
optionally, a single line-feed character).
SDP, RFC8866 5:
The sequence CRLF (0x0d0a) is used to end a line,
although parsers SHOULD be tolerant and also accept lines terminated
with a single newline character.
There should probably be tests for '\n' line endings, but mixing them in
the same MGCP message is ridiculous.
Change-Id: I6d530535a3a5f1d1a0716ab9e4a8079ba1de242e
---
M tests/mgcp/mgcp_test.c
M tests/mgcp/mgcp_test.ok
2 files changed, 49 insertions(+), 22 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/78/37278/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/37278?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: I6d530535a3a5f1d1a0716ab9e4a8079ba1de242e
Gerrit-Change-Number: 37278
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-MessageType: newpatchset
