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

[M] Change in pysim[master]: tests: Add new, data-driven OTA tests
by fixeria 19 Jun '23

19 Jun '23

Attention is currently required from: laforge. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/33348 ) Change subject: tests: Add new, data-driven OTA tests ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33348 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I8789a21fa5a4793bdabd468adc9fee3b6e633c25 Gerrit-Change-Number: 33348 Gerrit-PatchSet: 3 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Mon, 19 Jun 2023 07:40:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in libosmo-netif[master]: stream: Set state to closed before calling disconnect_cb()
by daniel 19 Jun '23

19 Jun '23

daniel has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33344 ) Change subject: stream: Set state to closed before calling disconnect_cb() ...................................................................... stream: Set state to closed before calling disconnect_cb() Fixes recent crashes in ttcn-cbc-test Change-Id: I40ceb17c32d1f58f8d0eeda8d1d794cf3f478e83 Related: OS#6063 --- M src/stream.c 1 file changed, 17 insertions(+), 3 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, approved; Verified diff --git a/src/stream.c b/src/stream.c index 31aa058..c710e1b 100644 --- a/src/stream.c +++ b/src/stream.c @@ -337,6 +337,8 @@ * abstraction and close the socket */ void osmo_stream_cli_close(struct osmo_stream_cli *cli) { + int old_state = cli->state; + if (cli->state == STREAM_CLI_STATE_CLOSED) return; if (cli->state == STREAM_CLI_STATE_WAIT_RECONNECT) { @@ -357,13 +359,13 @@ OSMO_ASSERT(false); } - if (cli->state == STREAM_CLI_STATE_CONNECTED) { + cli->state = STREAM_CLI_STATE_CLOSED; + + if (old_state == STREAM_CLI_STATE_CONNECTED) { LOGSCLI(cli, LOGL_DEBUG, "connection closed\n"); if (cli->disconnect_cb) cli->disconnect_cb(cli); } - - cli->state = STREAM_CLI_STATE_CLOSED; } static inline int osmo_stream_cli_fd(const struct osmo_stream_cli *cli) -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33344 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I40ceb17c32d1f58f8d0eeda8d1d794cf3f478e83 Gerrit-Change-Number: 33344 Gerrit-PatchSet: 2 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-MessageType: merged

1 0

[L] Change in pysim[master]: Add support for encoding/decoding SMS in TPDU and SMPP format
by fixeria 19 Jun '23

19 Jun '23

Attention is currently required from: laforge. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/33347 ) Change subject: Add support for encoding/decoding SMS in TPDU and SMPP format ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33347 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I0d95e62c1e7183a7851d1fe38df0f5133830cb1f Gerrit-Change-Number: 33347 Gerrit-PatchSet: 3 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Mon, 19 Jun 2023 07:39:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: gsm/ipa: Add segmentation callback
by fixeria 19 Jun '23

19 Jun '23

Attention is currently required from: arehbein, pespin, daniel. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33083 ) Change subject: gsm/ipa: Add segmentation callback ...................................................................... Patch Set 11: Code-Review+1 (2 comments) File src/gsm/ipa.c: https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/016e9ecc_e8f8bf49 PS10, Line 735: msgb_length(msg) + msgb_tailroom(msg); > I think you got confused here about `msg->data_len`, `msg->len` and `msgb_length()`. […] Ah, right. Sorry for the noise. https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/c8b28856_cc3853a1 PS10, Line 744: EIO > I took a look at `errno -l` and found `ENOBUFS 105 No buffer space available`. Done -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33083 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3a639e6896cc3b3fc8e9b2e1a58254710efa0d3f Gerrit-Change-Number: 33083 Gerrit-PatchSet: 11 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> 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-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Mon, 19 Jun 2023 07:37:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: core/osmo_io: Fix reception of partial packets
by fixeria 19 Jun '23

19 Jun '23

Attention is currently required from: arehbein. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33346 ) Change subject: core/osmo_io: Fix reception of partial packets ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33346 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0ab8028c807b4181fddd3c00ea2e037c40cf0813 Gerrit-Change-Number: 33346 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Comment-Date: Mon, 19 Jun 2023 07:35:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: gsm/ipa: Add segmentation callback
by arehbein 18 Jun '23

18 Jun '23

Attention is currently required from: fixeria, pespin, daniel. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33083 ) Change subject: gsm/ipa: Add segmentation callback ...................................................................... Patch Set 11: (8 comments) File src/gsm/ipa.c: https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/334d140f_25a985ca PS8, Line 728: * -EIO, if the header declares a payload too large */ > */ on the next line hmm weird, I remember reading somewhere that Osmocom comments do *not* put the comment end marker on the same line. Admittedly, I don't know where I read it, but I'm seeing the same style in lots of other places. https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/f3c4e7d7_562b1daa PS8, Line 729: int ipa_segmentation_cb(struct msgb *msg) > as per struct osmo_io_ops documentation: […] So what's wrong with the Doxygen style comment? It doesn't contradict the description you quote. https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/1442daa4_d178cad3 PS8, Line 735: const struct ipaccess_head *hh = (const struct ipaccess_head *) msg->data; > You could declare this at the start of the function (we usually declare variables at the start of th […] Done https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/ff847965_719776da PS8, Line 737: size_t total_len = payload_len + sizeof(*hh); > "sizeof(*hh) + payload_len;" it's logically easier to understand, as in lefto-to-right order filling […] Done https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/e0a2a000_2d5aa618 PS8, Line 738: if (msgb_tailroom(msg) + msgb_length(msg) < total_len) { > > iiuc the problem here is that the allocated msgb space is not going to be enough to fit in what IP […] Done File src/gsm/ipa.c: https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/e3da8242_e9830cbf PS10, Line 733: osmo_ntohs(hh->len); > Now the problem is that you're accessing the buffer before checking if there is enough data in it. ah yeah thanks, had that in mind, but forgot it again in between all the other CR comments/changes and rebasing. https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/675c57e2_dea17eb5 PS10, Line 735: msgb_length(msg) + msgb_tailroom(msg); > This is incorrect. […] I think you got confused here about `msg->data_len`, `msg->len` and `msgb_length()`. `msg->data_len` includes all the bytes allocated (including tailroom and headroom); `msg->len` includes all the bytes reserved for the message to be stored (i.e. not the tailroom and not the headroom). At least that's what I'm getting from the code. https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/148de202_bc80783f PS10, Line 744: EIO > `ENOMEM` or `ENOSPC` is a better fit here, IMO. I took a look at `errno -l` and found `ENOBUFS 105 No buffer space available`. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33083 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3a639e6896cc3b3fc8e9b2e1a58254710efa0d3f Gerrit-Change-Number: 33083 Gerrit-PatchSet: 11 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> 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-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Sun, 18 Jun 2023 09:40:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: gsm/ipa: Add segmentation callback
by arehbein 18 Jun '23

18 Jun '23

Attention is currently required from: arehbein, pespin, daniel. Hello Jenkins Builder, laforge, fixeria, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/33083 to look at the new patch set (#11). Change subject: gsm/ipa: Add segmentation callback ...................................................................... gsm/ipa: Add segmentation callback Add segmentation callback to be used by the streaming backend of libosmo-netif Related: OS#5753, OS#5751 Change-Id: I3a639e6896cc3b3fc8e9b2e1a58254710efa0d3f --- M include/osmocom/gsm/ipa.h M src/gsm/ipa.c M src/gsm/libosmogsm.map 3 files changed, 45 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/83/33083/11 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33083 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3a639e6896cc3b3fc8e9b2e1a58254710efa0d3f Gerrit-Change-Number: 33083 Gerrit-PatchSet: 11 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> 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-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[L] Change in pysim[master]: Add support for encoding/decoding SMS in TPDU and SMPP format
by laforge 18 Jun '23

18 Jun '23

Attention is currently required from: fixeria. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/33347 ) Change subject: Add support for encoding/decoding SMS in TPDU and SMPP format ...................................................................... Patch Set 3: (2 comments) File contrib/jenkins.sh: https://gerrit.osmocom.org/c/pysim/+/33347/comment/834ea57f_4a020166 PS1, Line 25: pip install git+https://github.com/hologram-io/smpp.pdu > Is this really needed given that you adding this dep to `requirements. […] thanks, it is indeedredundant. File ota_test.py: https://gerrit.osmocom.org/c/pysim/+/33347/comment/061cdf96_fd492d08 PS1, Line 1: #!/usr > `/usr/bin/env python3` please that entire script should go, it's not for this commit, and has meanwhile been converted to proper python unittest. sorry. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33347 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I0d95e62c1e7183a7851d1fe38df0f5133830cb1f Gerrit-Change-Number: 33347 Gerrit-PatchSet: 3 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Sun, 18 Jun 2023 08:51:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[L] Change in pysim[master]: Add support for encoding/decoding SMS in TPDU and SMPP format
by laforge 18 Jun '23

18 Jun '23

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/pysim/+/33347 to look at the new patch set (#3). Change subject: Add support for encoding/decoding SMS in TPDU and SMPP format ...................................................................... Add support for encoding/decoding SMS in TPDU and SMPP format This is important when talking OTA with a SIM. Change-Id: I0d95e62c1e7183a7851d1fe38df0f5133830cb1f --- M pySim/sms.py M requirements.txt M setup.py A tests/test_sms.py 4 files changed, 472 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/47/33347/3 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33347 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I0d95e62c1e7183a7851d1fe38df0f5133830cb1f Gerrit-Change-Number: 33347 Gerrit-PatchSet: 3 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in pysim[master]: tests: Add new, data-driven OTA tests
by laforge 18 Jun '23

18 Jun '23

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/+/33348 to look at the new patch set (#3). Change subject: tests: Add new, data-driven OTA tests ...................................................................... tests: Add new, data-driven OTA tests Rather than writing one test class with associated method for each OTA algorithm / test, let's do this in a data-driven way, where new test cases just have to provide test data, while the code iterates over it. Change-Id: I8789a21fa5a4793bdabd468adc9fee3b6e633c25 --- M tests/test_ota.py 1 file changed, 151 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/48/33348/3 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33348 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I8789a21fa5a4793bdabd468adc9fee3b6e633c25 Gerrit-Change-Number: 33348 Gerrit-PatchSet: 3 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newpatchset

1 0