- gerrit-log - lists.osmocom.org

[M] Change in libosmocore[master]: osmo_io: Remove union in struct osmo_io_ops
by neels 01 Dec '23

01 Dec '23

Attention is currently required from: arehbein, daniel, laforge, pespin. neels 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: (3 comments) Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/35079/comment/4642b068_2e53601d PS4, Line 11: from distinguishing between write_cb() and sendto_cb() etc. AFAIU, iofd->mode should distinguish which union members are in use, so it should be possible to handle things properly .. where exactly is the bug this is fixing? is the mode switched without NULLing the cb pointers or something in that way? File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/35079/comment/504ea742_ce7737fb PS3, Line 546: if (iofd->io_ops.read_cb) > The fact that this happened to work is even more reason to remove the union. i fail to see the problem though, can you explain? File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/35079/comment/2d9f5f19_5048d303 PS4, Line 391: || (iofd->mode == OSMO_IO_FD_MODE_RECVFROM_SENDTO && iofd->io_ops.recvfrom_cb)); this code should also work with the union present? -- 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: neels <nhofmeyr(a)sysmocom.de> 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-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Fri, 01 Dec 2023 02:04:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[L] Change in libosmocore[master]: soft_uart: implement modem status lines and flow control
by neels 01 Dec '23

01 Dec '23

Attention is currently required from: fixeria, jolly, laforge, osmith. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35172?usp=email ) Change subject: soft_uart: implement modem status lines and flow control ...................................................................... Patch Set 3: (4 comments) File include/osmocom/core/soft_uart.h: https://gerrit.osmocom.org/c/libosmocore/+/35172/comment/1868fc74_0520d7cb PS3, Line 54: OSMO_SUART_STATUS_F_CTS = (1 << 5), /*!< Clear To Send */ (personally i prefer an enum storing just 0, 1, 2, 3, 4, 5, because it is easy to get from 5 to 1<<5, but it is hard to get from 0b0100000 to 5.) https://gerrit.osmocom.org/c/libosmocore/+/35172/comment/4d979bcb_8280797d PS3, Line 61: OSMO_SUART_FLOW_CTRL_M_NONE, (apparently it's good practice to set the first item = 0 explicitly) https://gerrit.osmocom.org/c/libosmocore/+/35172/comment/5624218e_614fba12 PS3, Line 100: gets wdym, "gets". it returns void? File src/core/soft_uart.c: https://gerrit.osmocom.org/c/libosmocore/+/35172/comment/c8ecb0f8_d64c2318 PS2, Line 398: if (active) /* assert the given line */ > Logic 1 is negative voltage on RS232 level and is the de-asserted (disabled) level on control lines. re: if assert and de-assert is terminology from the matching spec, then of course keep using those terms here. (could have one comment in the first occurence to explain the term usage maybe) -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35172?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: I26b93ce76f2f6b6fbf017f2684312007db3c6d48 Gerrit-Change-Number: 35172 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-CC: jolly <andreas(a)eversberg.eu> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Fri, 01 Dec 2023 01:57:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Comment-In-Reply-To: jolly <andreas(a)eversberg.eu> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: soft_uart: check Rx/Tx state once in osmo_soft_uart_{rx,tx}_ubits()
by neels 01 Dec '23

01 Dec '23

Attention is currently required from: fixeria, laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35171?usp=email ) Change subject: soft_uart: check Rx/Tx state once in osmo_soft_uart_{rx,tx}_ubits() ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Patchset: PS3: looks sane, though i don't understand in depth -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35171?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: I70f93b3655eb21c2323e451052c40cd305c016c8 Gerrit-Change-Number: 35171 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 01 Dec 2023 01:46:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes 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 neels 01 Dec '23

01 Dec '23

Attention is currently required from: pespin. neels has posted comments on this change. ( 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 ...................................................................... Patch Set 2: Code-Review-1 (2 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/41b926f6_5c5c1446 PS2, Line 7: mgcp_network: Allow rx of IuUP in loopback mode with set rem IP address and unset rem port (you're getting into a habit of too long summary lines) File src/libosmo-mgcp/mgcp_network.c: https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/215343ec_a396b346 PS2, Line 866: HACK: for IuUP, we want to reply with an IuUP Initialization ACK upon the first RTP : * messag hmm! why is this stuff still here, we have proper IuUP support and everything about this legacy hack should be purged. let's rebase this onto https://gerrit.osmocom.org/c/osmo-mgw/+/35181 and only copy the hack-less part. -- 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-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 01 Dec 2023 00:58:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-mgw[master]: check_rtp_origin: drop special case for legacy IuUP hack
by neels 01 Dec '23

01 Dec '23

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35181?usp=email ) Change subject: check_rtp_origin: drop special case for legacy IuUP hack ...................................................................... check_rtp_origin: drop special case for legacy IuUP hack We have proper IuUP support and everything about this legacy hack should be purged. The purpose of this function is to validate that RTP is coming from the expected address and port. To allow that legacy IuUP hack, which is no longer needed, we punched a hole into this validation, by adding this special case for loopback mode (suddenly we don't care who or what sends RTP and bounce it back to anyone). So let's get rid of this hole that was only needed for very early 3G voice hacking. Related: Idd833997abce46886e9664505b2776fa5dadc8db Change-Id: I158dd046fdfcb10392cde3de8cc88dd095a05b40 --- M src/libosmo-mgcp/mgcp_network.c 1 file changed, 28 insertions(+), 25 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/81/35181/1 diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index 46d0cb4..23879b4 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -835,31 +835,14 @@ char ipbuf[INET6_ADDRSTRLEN]; if (osmo_sockaddr_is_any(&conn->end.addr) != 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 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)); - 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_ntop(&addr->u.sa, ipbuf)); - return -1; - } + /* 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_ntop(&addr->u.sa, ipbuf)); + return -1; } /* Note: Check if the inbound RTP data comes from the same host to -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35181?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: I158dd046fdfcb10392cde3de8cc88dd095a05b40 Gerrit-Change-Number: 35181 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

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

01 Dec '23

Attention is currently required from: pespin. neels has posted comments on this change. ( 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 ...................................................................... Patch Set 1: Code-Review+1 (3 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-mgw/+/35175/comment/ed499b16_29e4fe50 PS1, Line 8: could you include some before and after log output? File src/libosmo-mgcp/mgcp_network.c: https://gerrit.osmocom.org/c/osmo-mgw/+/35175/comment/3265754a_9577d428 PS1, Line 873: osmo_sockaddr_to_str(addr), osmo_sockaddr_ntop(&conn->end.addr.u.sa, ipbuf)); +1 for slashing LOGPC 👍 (we could use osmo_sockaddr_to_str_c(OTC_SELECT,..) or _buf(..) with static buffer for the second addr) https://gerrit.osmocom.org/c/osmo-mgw/+/35175/comment/5115db42_559c8b3d PS1, Line 885: osmo_sockaddr_to_str(addr), osmo_sockaddr_port(&conn->end.addr.u.sa), doesn't osmo_sockaddr_to_str() already include the port? (completely different question, should we even care where RTP is coming from) -- 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-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 01 Dec 2023 00:39:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-mgw[master]: mgcp-cli: Transmit remote IP addr in CRCX if known and port=0
by neels 01 Dec '23

01 Dec '23

Attention is currently required from: dexter, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35152?usp=email ) Change subject: mgcp-cli: Transmit remote IP addr in CRCX if known and port=0 ...................................................................... Patch Set 3: Code-Review-1 (8 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-mgw/+/35152/comment/8b8940cf_ddbe4eb8 PS3, Line 7: cli (i'd prefer unabbreviated "client", vs. "cmdline interface") Patchset: PS3: marking -1 to clarify conflict with https://gerrit.osmocom.org/c/osmo-mgw/+/34899 File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/35152/comment/1a8a1131_f8a0af89 PS3, Line 1317: ip missing msgb_free()? also like below, maybe just omit addr instead https://gerrit.osmocom.org/c/osmo-mgw/+/35152/comment/56e842d6_7a4a48db PS3, Line 1323: urn - maybe we should just omit the address, same as unset flag _PRESENCE_AUDIO_IP? https://gerrit.osmocom.org/c/osmo-mgw/+/35152/comment/db76e38c_fb086adb PS3, Line 1339: et same as above, maybe just skip this when port == 0 https://gerrit.osmocom.org/c/osmo-mgw/+/35152/comment/32bbb2d4_f01cb972 PS3, Line 1353: codecs this conflicts with branch neels/fmtp see https://gerrit.osmocom.org/c/osmo-mgw/+/34899 so after that patch we only use ptmap[], not codecs[]. In that patch, it's not yet clear how to re-add backwards compat; the patch removes codecs[], but instead codecs[] has to stay for compat, even though only ptmap[] is in use; maybe we can dynamically adopt codecs[] into ptmap[] ... so how do we resolve this conflict, rebase this onto neels/fmtp or vice versa? which patch has higher priority for users? either way, ultimately this code should s/codecs[i]/ptmap[i].codec/ https://gerrit.osmocom.org/c/osmo-mgw/+/35152/comment/918b6913_2280908e PS3, Line 1367: * indent https://gerrit.osmocom.org/c/osmo-mgw/+/35152/comment/62c28d66_3cc658f7 PS3, Line 1373: * . -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35152?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: I30165dbac5e484011d0acf46af36f105954a501d Gerrit-Change-Number: 35152 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Fri, 01 Dec 2023 00:30:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-mgw[master]: mgcp-cli: Transmit remote IP addr in CRCX if known and port=0
by neels 01 Dec '23

01 Dec '23

Attention is currently required from: dexter, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35152?usp=email ) Change subject: mgcp-cli: Transmit remote IP addr in CRCX if known and port=0 ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: do you have a test for it? -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35152?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: I30165dbac5e484011d0acf46af36f105954a501d Gerrit-Change-Number: 35152 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 30 Nov 2023 23:58:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: socket: Introduce API osmo_sock_multiaddr_get_name_buf()
by neels 01 Dec '23

01 Dec '23

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35180?usp=email ) Change subject: socket: Introduce API osmo_sock_multiaddr_get_name_buf() ...................................................................... Patch Set 1: (5 comments) File src/core/socket.c: https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/2ad85b55_eccf6410 PS1, Line 1968: return -EBADF; i understand, but for the snprintf() like function signature, if at all possible you should always return sb.chars_needed. Otherwise, if anyone calls OSMO_STRBUF_APPEND(sb, osmo_sock_multiaddr_get_name_buf, ...), they don't get "<error-bad-fd>" returned, in the expected place; instead, it looks like OSMO_STRBUF_APPEND() might even crash? [the place handling < 0 there looks a bit weird; same as insufficient buffer, which should be a different case.] the function's purpose is to print a string, not to validate an fd, so better just do this here: OSMO_STRBUF_PRINTF(sb, "<error-bad-fd>"); return sb.chars_needed; because then the caller will get the error output in the right place, like, contrived example: Stats: reads: 0 writes: 0 address: <error-bad-fd> When you return an error, the string composition may abort and omit the "Stats:..." part too in the resulting output. (as i read on, i see that osmo_sock_get_name_buf() makes the same mistake, i guess i need to fix that) https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/64fdb01a_d46bb32b PS1, Line 1993: v6 = !!s assigning a value to is_v6 in an 'else' clause and using is_v6 below. It looks like the implication is that if num_hostbuf <= 1, then it cannot be a v6 address, which sounds odd to me https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/08a76724_bd72f529 PS1, Line 2005: OSMO_STRBUF_PRINTF(sb, "<need-more-bufs!>"); (could print this above instead of adding the need_more_bufs local var, just my opinion) (also i find it weird to print <need-more-bufs!> in user output, it makes no sense to the reader, because it is reporting an error in internal buffer sizes, and not about the user's config or current status of resources; a user may think that a RAM upgrade is necessary to run osmocom, wildly wrong) https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/accd4083_8c902ac9 PS1, Line 2019: bool is_v6 = false; (can this code dup be collapsed with the above, maybe with a static func?) https://gerrit.osmocom.org/c/libosmocore/+/35180/comment/0dcd0bb5_dbc45421 PS1, Line 2052: int osmo_sock_get_name_buf(char *str, size_t str_len, int fd) > Something which we could do is internally integrating the multiaddr support into the existing genera […] my first thought is "yes, sounds good". Do you think though there may be situations where we don't want all of the addresses in, say, all of the logging or vty output all the time, where instead we want to just continue printing the first address? I guess that is hard to answer, so unfortunately the best backwards compatible way is to only have the separate new API returning multiple addrs. Callers can switch to the new function at their own time. But, hey, how about we call the new function osmo_sock_get_name_buf2(), as in, v2 supports multiple addresses, and everyone should just use that everywhere from now on? -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35180?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: I48950754ed6f61ee5ffa04a447fab8903f10acc0 Gerrit-Change-Number: 35180 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 30 Nov 2023 23:39:27 +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 libosmocore[master]: soft_uart: check Rx/Tx state once in osmo_soft_uart_{rx,tx}_ubits()
by fixeria 30 Nov '23

30 Nov '23

Attention is currently required from: laforge, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35171?usp=email ) Change subject: soft_uart: check Rx/Tx state once in osmo_soft_uart_{rx,tx}_ubits() ...................................................................... Patch Set 3: (2 comments) Patchset: PS3: > Btw, -EAGAIN doesn't sound like the best error code to return in that situation. […] The idea here is to indicate the caller that for whatever reason (here it's "the Rx/Tx is disabled", in the next patch it would also be "Rx/TX is suspended by flow control") the receiver/transmitter did not consume/emit the requested amount of bits. And `EAGAIN` should be interpreted as "try next time, things may change". The caller of `osmo_soft_uart_{rx,tx}_ubits()` is expected not to be in charge of the soft-UART management (enabling or disabling Rx/Tx lines). In a typical scenario it's a logic driving some synchronous medium (like a TCH channel, on which you always Rx and always Tx something). * If the `_rx_ubits()` API returns `-EAGAIN` (or any other `N != 0`), the caller is likely to ignore this and keep feeding the UART with bits received over the synchronous medium. * If the `_tx_ubits()` API returns `-EAGAIN` (or any other `N != 0`), it's responsibility of the caller to initialize the unpopulated bit buffer with stop bits (binary '1') and transmit them over the synchronous medium. I am open for suggestions if you have a better errno value in mind ;) File src/core/soft_uart.c: https://gerrit.osmocom.org/c/libosmocore/+/35171/comment/47f53bd2_09d4bf26 PS3, Line 291: if (!suart->tx.running) > Move this above the other if condition, this one one can use tx_ubits(... […] I would better go for adding proper API, if there would be a need for checking Rx/Tx running state. `-EAGAIN` would also be returned if the Rx/Tx is suspended due to flow control (see the next patch), so... no. Let's keep it as-is. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35171?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: I70f93b3655eb21c2323e451052c40cd305c016c8 Gerrit-Change-Number: 35171 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 30 Nov 2023 20:39:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

2026

2025

2024

2023

2022

gerrit-log