Attention is currently required from: daniel, fixeria.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34967?usp=email )
Change subject: tests: Test gsmtap logging if write queue fills up
......................................................................
Patch Set 2:
(1 comment)
File tests/logging/logging_test_gsmtap.err:
https://gerrit.osmocom.org/c/libosmocore/+/34967/comment/6617b38b_dc91df15
PS2, Line 108: 0x611000000328
> Yeah, do you see any other way of removing such information without changing the log message? […]
I think we do patch stuff in output in some places already.
See for instance:
osmo-msc/tests/testsuite.at
24:AT_CHECK([$abs_top_builddir/tests/db_sms/db_sms_test 3>&1 1>&2 2>&3 | grep -v "Failed to load driver" | grep -v "cannot open shared object file"], [], [expout], [experr])
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34967?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id5ae0c4c3820a9ed59eaf4003d2c57b6bdfe3468
Gerrit-Change-Number: 34967
Gerrit-PatchSet: 2
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 30 Nov 2023 14:56:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: daniel, laforge, lynxis lazus.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35078?usp=email )
Change subject: osmo_io: Factor out and use common send function from backend
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/35078/comment/276664ed_b08b253a
PS3, Line 191: msgb_free(msghdr->msg);
is this msgb_free required by the way? or iofd_msghdr-Free already frees it?
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35078?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I6da2653d32aedd0e7872be0cf90a841b56462e59
Gerrit-Change-Number: 35078
Gerrit-PatchSet: 3
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Thu, 30 Nov 2023 14:53:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: arehbein, daniel, laforge.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35079?usp=email )
Change subject: osmo_io: Remove union in struct osmo_io_ops
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
It can know based on the OSMO_IO_FD_MODE configured during osmo_iofd_setup(). So before using one callback, the implementation needs to check ofc also the io mode is the one used. It doesn't make sense for a user of the API to set one io_mode and then set callbacks for the other mode, this is a bug in the user, wrong use of the API.
> it's possible to create an osmo_io_fd using osmo_iofd_setup in one mode (e.g. OSMO_IO_FD_MODE_READ_WRITE) but the register an osmo_io_ops with wrong call-backs (e.g. recvfrom/sendto).
IMHO that's a bug / wrong use of the API at the user and nothing to be fixed in the osmo_io implementation.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35079?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I138d57843edc29000530bb7896bcb239002ecbec
Gerrit-Change-Number: 35079
Gerrit-PatchSet: 4
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 30 Nov 2023 14:51:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35175?usp=email )
Change subject: mgcp_network: Improve err logging when rtp pkt from unexpected origin comes in
......................................................................
mgcp_network: Improve err logging when rtp pkt from unexpected origin comes in
Change-Id: Id9b60395df667ae9898c23cbc2afe56ac7e8b0e5
---
M src/libosmo-mgcp/mgcp_network.c
1 file changed, 18 insertions(+), 17 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/75/35175/1
diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index 46d0cb4..b1bce97 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -845,9 +845,8 @@
* sends the RAB Assignment Response. Hence, if the remote address is 0.0.0.0 and the
* MGCP port is in loopback mode, allow looping back the packet to any source. */
LOGPCONN(conn->conn, DRTP, LOGL_ERROR,
- "In loopback mode and remote address not set:"
- " allowing data from address: %s\n",
- osmo_sockaddr_ntop(&addr->u.sa, ipbuf));
+ "In loopback mode and remote address not set: allowing data from address: %s\n",
+ osmo_sockaddr_to_str(addr));
return 0;
default:
@@ -855,9 +854,8 @@
* this as an error that occurs on every call, keep it more low profile to not
* confuse humans with expected errors. */
LOGPCONN(conn->conn, DRTP, LOGL_INFO,
- "Rx RTP from %s, but remote address not set:"
- " dropping early media\n",
- osmo_sockaddr_ntop(&addr->u.sa, ipbuf));
+ "Rx RTP from %s, but remote address not set: dropping early media\n",
+ osmo_sockaddr_to_str(addr));
return -1;
}
}
@@ -871,11 +869,8 @@
memcmp(&conn->end.addr.u.sin6.sin6_addr, &addr->u.sin6.sin6_addr,
sizeof(struct in6_addr)))) {
LOGPCONN(conn->conn, DRTP, LOGL_ERROR,
- "data from wrong address: %s, ",
- osmo_sockaddr_ntop(&addr->u.sa, ipbuf));
- LOGPC(DRTP, LOGL_ERROR, "expected: %s\n",
- osmo_sockaddr_ntop(&conn->end.addr.u.sa, ipbuf));
- LOGPCONN(conn->conn, DRTP, LOGL_ERROR, "packet tossed\n");
+ "data from wrong src %s, expected IP Address %s. Packet tossed.\n",
+ osmo_sockaddr_to_str(addr), osmo_sockaddr_ntop(&conn->end.addr.u.sa, ipbuf));
return -1;
}
@@ -886,12 +881,9 @@
if (osmo_sockaddr_port(&conn->end.addr.u.sa) != osmo_sockaddr_port(&addr->u.sa) &&
ntohs(conn->end.rtcp_port) != osmo_sockaddr_port(&addr->u.sa)) {
LOGPCONN(conn->conn, DRTP, LOGL_ERROR,
- "data from wrong source port: %d, ",
- osmo_sockaddr_port(&addr->u.sa));
- LOGPC(DRTP, LOGL_ERROR,
- "expected: %d for RTP or %d for RTCP\n",
- osmo_sockaddr_port(&conn->end.addr.u.sa), ntohs(conn->end.rtcp_port));
- LOGPCONN(conn->conn, DRTP, LOGL_ERROR, "packet tossed\n");
+ "data from wrong src %s, expected port: %u for RTP or %u for RTCP. Packet tossed.\n",
+ osmo_sockaddr_to_str(addr), osmo_sockaddr_port(&conn->end.addr.u.sa),
+ ntohs(conn->end.rtcp_port));
return -1;
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35175?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: Id9b60395df667ae9898c23cbc2afe56ac7e8b0e5
Gerrit-Change-Number: 35175
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35176?usp=email )
Change subject: mgcp_network: Allow rx of IuUP in loopback mode with set rem IP address and unset rem port
......................................................................
mgcp_network: Allow rx of IuUP in loopback mode with set rem IP address and unset rem port
An osmo-mgw client (eg. osmo-hnbgw) may wish to initially set a remote
IP address as a hint during CRCX, hence the IP address may already be
set while the port may be unset. We had special cases to handle IuUP
Initialization phase while in loopback mode and IP address being unset,
but we didn't handle the special case where IP address may be set but
port isn't.
Related: SYS#6657
Change-Id: Idd833997abce46886e9664505b2776fa5dadc8db
---
M src/libosmo-mgcp/mgcp_network.c
1 file changed, 43 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/76/35176/1
diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index b1bce97..b423aee 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -860,6 +860,32 @@
}
}
+ if (osmo_sockaddr_port(&conn->end.addr.u.sa) == 0) {
+ switch (conn->conn->mode) {
+ case MGCP_CONN_LOOPBACK:
+ /* HACK: for IuUP, we want to reply with an IuUP Initialization ACK upon the first RTP
+ * message received. We currently hackishly accomplish that by putting the endpoint in
+ * loopback mode and patching over the looped back RTP message to make it look like an
+ * ack. We don't know the femto cell's IP address and port until the RAB Assignment
+ * Response is received, but the nano3G expects an IuUP Initialization Ack before it even
+ * sends the RAB Assignment Response. Hence, if the port number is 0 and the
+ * MGCP conn is in loopback mode, allow looping back the packet to any port. */
+ LOGPCONN(conn->conn, DRTP, LOGL_ERROR,
+ "In loopback mode and remote port not set: allowing data from address: %s\n",
+ osmo_sockaddr_to_str(addr));
+ return 0;
+
+ default:
+ /* Receiving early media before the endpoint is configured. Instead of logging
+ * this as an error that occurs on every call, keep it more low profile to not
+ * confuse humans with expected errors. */
+ LOGPCONN(conn->conn, DRTP, LOGL_INFO,
+ "Rx RTP from %s, but remote address not set: dropping early media\n",
+ osmo_sockaddr_to_str(addr));
+ return -1;
+ }
+ }
+
/* Note: Check if the inbound RTP data comes from the same host to
* which we send our outgoing RTP traffic. */
if (conn->end.addr.u.sa.sa_family != addr->u.sa.sa_family ||
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35176?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: Idd833997abce46886e9664505b2776fa5dadc8db
Gerrit-Change-Number: 35176
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
Attention is currently required from: fixeria.
daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34967?usp=email )
Change subject: tests: Test gsmtap logging if write queue fills up
......................................................................
Patch Set 2:
(1 comment)
File tests/logging/logging_test_gsmtap.err:
https://gerrit.osmocom.org/c/libosmocore/+/34967/comment/d158b54a_45ea80e2
PS2, Line 108: 0x611000000328
> this is non-deterministic output, we should not match it
Yeah, do you see any other way of removing such information without changing the log message?
Do we need a name in osmo_wqueue (and default to %p) so we can print that?
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34967?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id5ae0c4c3820a9ed59eaf4003d2c57b6bdfe3468
Gerrit-Change-Number: 34967
Gerrit-PatchSet: 2
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 30 Nov 2023 13:39:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: laforge, lynxis lazus, pespin.
daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35078?usp=email )
Change subject: osmo_io: Factor out and use common send function from backend
......................................................................
Patch Set 3:
(2 comments)
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/35078/comment/3fc2633a_13cfb999
PS1, Line 360:
> ack, makes sense, the special case for rc=0 is in the recv path (socket closed). […]
send(len=0) actually succeeds (for TCP sockets) and is currently used in libosmo-netif to check whether a connect() succeeded. The poll-based backend can simply check whether the fd is writable or not. But as Harald mentioned in the SCTP ticket this is not the case with SCTP where write(len=0) returns an error.
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/35078/comment/4d203533_bcb50e25
PS2, Line 196: iofd_handle_send_completion(iofd, rc, msghdr);
> The goto can easily be dropped: […]
Ack
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35078?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I6da2653d32aedd0e7872be0cf90a841b56462e59
Gerrit-Change-Number: 35078
Gerrit-PatchSet: 3
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(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: Thu, 30 Nov 2023 13:35:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: laforge, lynxis lazus, pespin.
Hello Jenkins Builder, lynxis lazus,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/35078?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: osmo_io: Factor out and use common send function from backend
......................................................................
osmo_io: Factor out and use common send function from backend
This handles reenqueuing a message on EAGAIN and incomplete write
Change-Id: I6da2653d32aedd0e7872be0cf90a841b56462e59
---
M src/core/osmo_io.c
M src/core/osmo_io_internal.h
M src/core/osmo_io_poll.c
M src/core/osmo_io_uring.c
4 files changed, 58 insertions(+), 60 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/78/35078/3
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35078?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I6da2653d32aedd0e7872be0cf90a841b56462e59
Gerrit-Change-Number: 35078
Gerrit-PatchSet: 3
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(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-MessageType: newpatchset