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

[L] Change in libosmo-netif[master]: Add osmo_io support to osmo_stream_cli and osmo_stream_srv
by daniel 09 Jun '23

09 Jun '23

Attention is currently required from: arehbein, laforge, pespin. daniel has uploaded a new patch set (#9) to the change originally created by arehbein. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33193 ) Change subject: Add osmo_io support to osmo_stream_cli and osmo_stream_srv ...................................................................... Add osmo_io support to osmo_stream_cli and osmo_stream_srv Change-Id: I2f52c7107c392b6f4b0bf2a84f8c873c084a200c --- M include/osmocom/netif/stream.h M src/stream.c 2 files changed, 304 insertions(+), 28 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/93/33193/9 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33193 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I2f52c7107c392b6f4b0bf2a84f8c873c084a200c Gerrit-Change-Number: 33193 Gerrit-PatchSet: 9 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: daniel <dwillmann(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-MessageType: newpatchset

1 0

[L] Change in libosmo-netif[master]: Add osmo_io support to osmo_stream_cli and osmo_stream_srv
by pespin 09 Jun '23

09 Jun '23

Attention is currently required from: arehbein, laforge, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33193 ) Change subject: Add osmo_io support to osmo_stream_cli and osmo_stream_srv ...................................................................... Patch Set 7: (8 comments) Patchset: PS5: > > > Since we already have osmo_stream_*_get_ofd() we should add osmo_stream_*_get_iofd(). […] I see no problem with having separate APIs regarding what you mention. The socket may be already enabled, but it may not trigger any callback until you return to the mainloop, so you can perfectly call the APIs to set the callbacks before they are called. File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/a95e90f8_70ea30f0 PS7, Line 554: static void handle_connecting(struct osmo_stream_cli *cli, int res) this is for osmo_stream_cli, so adding a "stream_cli" prefix would help easily gastp that. https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/a446295c_14b4e708 PS7, Line 598: handle_connecting(cli, res); This split into a separate function you could do it in a previous separate preparation patch. https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/078ba248_8761d154 PS7, Line 620: if (msg && res <= 0) { EAGAIN would trigger closing the socket? https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/591fa510_4e5313db PS7, Line 881: cli->ofd.priv_nr = 0; /* XXX */ what about this XXX? https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/95a2a986_31e7cdd9 PS7, Line 1046: cli->iofd = osmo_iofd_setup(cli, -1, cli->name, OSMO_IO_FD_MODE_READ_WRITE, &osmo_stream_cli_ioops, cli); shouldn't this -1 be "fd"? https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/242de708_26f4a629 PS7, Line 1540: osmo_stream_srv_destroy(conn); just wondering whether you may need to return -EBADF here? https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/fa73f28d_48f27e8e PS7, Line 1714: conn->iofd_read_cb = read_cb; See, these can perfectly be separate setter APIs like we do for osmo_stream_cli. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33193 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I2f52c7107c392b6f4b0bf2a84f8c873c084a200c Gerrit-Change-Number: 33193 Gerrit-PatchSet: 7 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: daniel <dwillmann(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: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Fri, 09 Jun 2023 12:41:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> 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 libosmo-netif[master]: examples: Use new stream API in {ipa-,}stream-{client,server}
by Jenkins Builder 09 Jun '23

09 Jun '23

Attention is currently required from: arehbein, pespin, daniel. Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33194 ) Change subject: examples: Use new stream API in {ipa-,}stream-{client,server} ...................................................................... Patch Set 6: (2 comments) File examples/stream-client.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-8120): https://gerrit.osmocom.org/c/libosmo-netif/+/33194/comment/a5c617e1_aff611b1 PS6, Line 53: static int read_cb(struct osmo_stream_cli *conn, struct msgb* msg) "foo* bar" should be "foo *bar" File examples/stream-server.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-8120): https://gerrit.osmocom.org/c/libosmo-netif/+/33194/comment/a5ff3e2b_8c7e5880 PS6, Line 45: int read_cb(struct osmo_stream_srv *conn, struct msgb* msg) "foo* bar" should be "foo *bar" -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33194 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I97a9979199c816686b32080534627f6f033e009e Gerrit-Change-Number: 33194 Gerrit-PatchSet: 6 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(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: Fri, 09 Jun 2023 12:29:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in libosmo-netif[master]: examples: Use new stream API in {ipa-,}stream-{client,server}
by daniel 09 Jun '23

09 Jun '23

Attention is currently required from: arehbein, pespin, daniel. daniel has uploaded a new patch set (#6) to the change originally created by arehbein. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33194 ) Change subject: examples: Use new stream API in {ipa-,}stream-{client,server} ...................................................................... examples: Use new stream API in {ipa-,}stream-{client,server} Change-Id: I97a9979199c816686b32080534627f6f033e009e --- M examples/ipa-stream-client.c M examples/ipa-stream-server.c M examples/stream-client.c M examples/stream-server.c 4 files changed, 28 insertions(+), 76 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/94/33194/6 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33194 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I97a9979199c816686b32080534627f6f033e009e Gerrit-Change-Number: 33194 Gerrit-PatchSet: 6 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(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 libosmo-netif[master]: Add osmo_io support to osmo_stream_cli and osmo_stream_srv
by daniel 09 Jun '23

09 Jun '23

Attention is currently required from: arehbein, laforge, pespin. daniel has uploaded a new patch set (#8) to the change originally created by arehbein. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33193 ) Change subject: Add osmo_io support to osmo_stream_cli and osmo_stream_srv ...................................................................... Add osmo_io support to osmo_stream_cli and osmo_stream_srv Change-Id: I2f52c7107c392b6f4b0bf2a84f8c873c084a200c --- M include/osmocom/netif/stream.h M src/stream.c 2 files changed, 344 insertions(+), 30 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/93/33193/8 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33193 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I2f52c7107c392b6f4b0bf2a84f8c873c084a200c Gerrit-Change-Number: 33193 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: daniel <dwillmann(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-MessageType: newpatchset

1 0

[L] Change in libosmo-netif[master]: Add osmo_io support to osmo_stream_cli and osmo_stream_srv
by daniel 09 Jun '23

09 Jun '23

Attention is currently required from: arehbein, laforge, pespin. daniel has uploaded a new patch set (#7) to the change originally created by arehbein. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33193 ) Change subject: Add osmo_io support to osmo_stream_cli and osmo_stream_srv ...................................................................... Add osmo_io support to osmo_stream_cli and osmo_stream_srv Change-Id: I2f52c7107c392b6f4b0bf2a84f8c873c084a200c --- M include/osmocom/netif/stream.h M src/stream.c 2 files changed, 342 insertions(+), 30 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/93/33193/7 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33193 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I2f52c7107c392b6f4b0bf2a84f8c873c084a200c Gerrit-Change-Number: 33193 Gerrit-PatchSet: 7 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: daniel <dwillmann(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-MessageType: newpatchset

1 0

[L] Change in libosmo-netif[master]: Add osmo_io support to osmo_stream_cli and osmo_stream_srv
by daniel 09 Jun '23

09 Jun '23

Attention is currently required from: arehbein, laforge, pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33193 ) Change subject: Add osmo_io support to osmo_stream_cli and osmo_stream_srv ...................................................................... Patch Set 7: (1 comment) Patchset: PS5: > > Since we already have osmo_stream_*_get_ofd() we should add osmo_stream_*_get_iofd(). Since they will return pointer we could simply return NULL if it's called with the wrong mode, but maybe an assert is better. > > The question here would be: do we really need that API for iofd? or iofd can be handled totally internally? Ok, it seems this is mostly used in the actual read_cb to get the fd to perform the read. Other uses also use the file descriptor with osmo_sock_get_name2(). So I guess we don't need it. > > osmo_stream_srv_create2 is called with the newly accept()ed fd so I'd still pass the read/close callbacks in create2(). > > I'd keep setting the callbacks as separate APIs. Yes, it does 2 function calls more, occupies 2 more lines, but if we had done this already in the past then now we wouldn't need to be changing this osmo_stream_srv_create() API in first place... so learning for errors = adding separate setter APIs. Well, it's 3 more - setting the callbacks and then calling a function that actually enables reading from the fd. You usually call osmo_stream_srv_create() inside of the connect_cb of osmo_strem_srv_link. At that point the fd is already connected since it has been accept()ed and might also have pending data. Every osmo_stream_* where the fd is initially "inactive" has separate setter functions for the callbacks so this choice feels deliberate to me (and I agree with it). -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33193 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I2f52c7107c392b6f4b0bf2a84f8c873c084a200c Gerrit-Change-Number: 33193 Gerrit-PatchSet: 7 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: daniel <dwillmann(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-Comment-Date: Fri, 09 Jun 2023 12:14:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[L] Change in libosmocore[master]: ASCI: Add message definition and encoding according to 3GPP TS 48.008
by jolly 09 Jun '23

09 Jun '23

Attention is currently required from: jolly, dexter. Hello Jenkins Builder, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/32932 to look at the new patch set (#15). Change subject: ASCI: Add message definition and encoding according to 3GPP TS 48.008 ...................................................................... ASCI: Add message definition and encoding according to 3GPP TS 48.008 Change-Id: Ib94c64136c31ce4af67c314a8550d93946cc844f --- M include/osmocom/gsm/gsm0808.h M include/osmocom/gsm/protocol/gsm_08_08.h M src/gsm/gsm0808.c M src/gsm/libosmogsm.map 4 files changed, 933 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/32/32932/15 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32932 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib94c64136c31ce4af67c314a8550d93946cc844f Gerrit-Change-Number: 32932 Gerrit-PatchSet: 15 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: dexter <pmaier(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[L] Change in libosmocore[master]: ASCI: Add message definition and encoding according to 3GPP TS 48.008
by jolly 09 Jun '23

09 Jun '23

Attention is currently required from: jolly, dexter. Hello Jenkins Builder, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/32932 to look at the new patch set (#14). Change subject: ASCI: Add message definition and encoding according to 3GPP TS 48.008 ...................................................................... ASCI: Add message definition and encoding according to 3GPP TS 48.008 Change-Id: Ib94c64136c31ce4af67c314a8550d93946cc844f --- M include/osmocom/gsm/gsm0808.h M include/osmocom/gsm/protocol/gsm_08_08.h M src/gsm/gsm0808.c M src/gsm/libosmogsm.map 4 files changed, 931 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/32/32932/14 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32932 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib94c64136c31ce4af67c314a8550d93946cc844f Gerrit-Change-Number: 32932 Gerrit-PatchSet: 14 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: dexter <pmaier(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in pysim[master]: test_ota: Add one first OTA SMS AES128 unit test
by dexter 09 Jun '23

09 Jun '23

Attention is currently required from: laforge. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/33235 ) Change subject: test_ota: Add one first OTA SMS AES128 unit test ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33235 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: Id4a66bbfaec2d8610e8a7a2c72c0dfd08332edcd Gerrit-Change-Number: 33235 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Fri, 09 Jun 2023 11:00:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0