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

[M] Change in libosmocore[master]: osmo_io: Remove union in struct osmo_io_ops
by pespin 30 Nov '23

30 Nov '23

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: > I disagree that we should put the burden of this onto the user. It's an easy enough mistake to make. […] So you set mode (which is usually a hardcoded value in the code), and then an ops, which is also a hardcoded struct pointing to known functions, which is a union known by the user of the API. To me it's fine. To me what you say it's like making a claim that you open an AF_INET socket and then something is wrong becuase you pass an AF_INET6 address to some API... I agree it would be nice to have more protection there, but current state is good enough for me, specially since changing it has the 2 drawbacks I mentioned early (ABI break, more memory needed per fd). That's my current opinion, let's see if other have other opinions here. -- 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 15:38:33 +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

1 0

[M] Change in libosmocore[master]: osmo_io: Remove union in struct osmo_io_ops
by daniel 30 Nov '23

30 Nov '23

Attention is currently required from: arehbein, laforge, pespin. daniel 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(). […] I disagree that we should put the burden of this onto the user. It's an easy enough mistake to make. This is complicated by the fact that the only way to set (either initially or later) the callback function is by passing in a struct osmo_iofd_ops *. This means that with the union there is *no* way at all to detect this. ``` osmo_iofd_setup(iofd, ..., mode, &ops); // Checking ops callbacks according to mode doesn't work osmo_iofd_set_ioops(iofd, &ops); // Same as above, we have iofd->mode, but ops is ambiguous ``` Maybe you're thinking of osmo_stream_* where we have *_set_read_cb() etc. functions? -- 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: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 30 Nov 2023 15:22:26 +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

1 0

[M] Change in libosmocore[master]: osmo_io: Factor out and use common send function from backend
by daniel 30 Nov '23

30 Nov '23

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: (1 comment) File src/core/osmo_io_uring.c: https://gerrit.osmocom.org/c/libosmocore/+/35078/comment/50d40183_d1fee170 PS3, Line 191: msgb_free(msghdr->msg); > is this msgb_free required by the way? or iofd_msghdr-Free already frees it? msg is not owned by msghdr because during receive msg needs to live longer. -- 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: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Thu, 30 Nov 2023 15:15:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-mgw[master]: mgcp_network: Allow rx of IuUP in loopback mode with set rem IP addre...
by pespin 30 Nov '23

30 Nov '23

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/35176?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_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/2 -- 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: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

1 0

[L] Change in libosmocore[master]: tests: Test gsmtap logging if write queue fills up
by pespin 30 Nov '23

30 Nov '23

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

1 0

[M] Change in libosmocore[master]: osmo_io: Factor out and use common send function from backend
by pespin 30 Nov '23

30 Nov '23

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

1 0

[M] Change in libosmocore[master]: osmo_io: Remove union in struct osmo_io_ops
by pespin 30 Nov '23

30 Nov '23

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

1 0

[M] Change in osmo-bts[master]: Use polling based LAPDm with frame numbers
by fixeria 30 Nov '23

30 Nov '23

Attention is currently required from: jolly, laforge. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35007?usp=email ) Change subject: Use polling based LAPDm with frame numbers ...................................................................... Patch Set 7: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35007?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ic6d7902b13cf491daaa8752db78f9875387aeffd Gerrit-Change-Number: 35007 Gerrit-PatchSet: 7 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Thu, 30 Nov 2023 14:51:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: LAPDm: Reject (release) establishment on DCCH, SAPI 0 without L3 payload
by fixeria 30 Nov '23

30 Nov '23

Attention is currently required from: jolly, laforge. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35082?usp=email ) Change subject: LAPDm: Reject (release) establishment on DCCH, SAPI 0 without L3 payload ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35082?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I6819b51a876b8743c2d4a04165b7900723a1631c Gerrit-Change-Number: 35082 Gerrit-PatchSet: 7 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Thu, 30 Nov 2023 14:50:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-mgw[master]: mgcp_network: Improve err logging when rtp pkt from unexpected origin...
by pespin 30 Nov '23

30 Nov '23

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

1 0