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

[S] Change in libosmo-netif[master]: stream: Update log messages
by daniel 08 Jun '23

08 Jun '23

Attention is currently required from: daniel. Hello Jenkins Builder, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/33221 to look at the new patch set (#6). Change subject: stream: Update log messages ...................................................................... stream: Update log messages Change-Id: Ife20b9d18e6ca86a06991d68165694e31052c58a --- M src/stream.c M tests/stream/stream_test.err 2 files changed, 17 insertions(+), 10 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/21/33221/6 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33221 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Ife20b9d18e6ca86a06991d68165694e31052c58a Gerrit-Change-Number: 33221 Gerrit-PatchSet: 6 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in libosmo-netif[master]: cosmetic: Change name of osmo_stream_src_fd_cb()
by daniel 08 Jun '23

08 Jun '23

daniel has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33233 ) Change subject: cosmetic: Change name of osmo_stream_src_fd_cb() ...................................................................... cosmetic: Change name of osmo_stream_src_fd_cb() Change-Id: I8a9d27856ebdd5405b1977d12e91f8dcbdcd1d6b --- M src/stream.c 1 file changed, 11 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/33/33233/1 diff --git a/src/stream.c b/src/stream.c index c62d875..90b793f 100644 --- a/src/stream.c +++ b/src/stream.c @@ -953,7 +953,7 @@ int flags; }; -static int osmo_stream_srv_fd_cb(struct osmo_fd *ofd, unsigned int what) +static int osmo_stream_srv_ofd_cb(struct osmo_fd *ofd, unsigned int what) { int ret; int sock_fd; @@ -1038,7 +1038,7 @@ link->sk_domain = AF_UNSPEC; link->sk_type = SOCK_STREAM; link->proto = IPPROTO_TCP; - osmo_fd_setup(&link->ofd, -1, OSMO_FD_READ | OSMO_FD_WRITE, osmo_stream_srv_fd_cb, link, 0); + osmo_fd_setup(&link->ofd, -1, OSMO_FD_READ | OSMO_FD_WRITE, osmo_stream_srv_ofd_cb, link, 0); return link; } -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33233 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I8a9d27856ebdd5405b1977d12e91f8dcbdcd1d6b Gerrit-Change-Number: 33233 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

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

08 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 6: (2 comments) File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/1bbd6fe2_ef095c7b PS1, Line 1758: OSMO_ASSERT > I don't know the code like you do, but I generally agree with your point here. […] Done File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/fcde1c9d_0e7f0934 PS5, Line 1197: static int osmo_stream_srv_ofd_cb(struct osmo_fd *ofd, unsigned int what) > I'd welcome if you did this kind of changes in separate previous patches to try to trim this one a b […] Done -- 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: 6 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: Thu, 08 Jun 2023 15:10:41 +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 dexter 08 Jun '23

08 Jun '23

Attention is currently required from: jolly, pespin. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32932 ) Change subject: ASCI: Add message definition and encoding according to 3GPP TS 48.008 ...................................................................... Patch Set 12: (3 comments) File include/osmocom/gsm/protocol/gsm_08_08.h: https://gerrit.osmocom.org/c/libosmocore/+/32932/comment/39e5ed28_426525f1 PS12, Line 175: BSS_MAP_MSG_UPLINK_REJECT_CMD = 75, (not critical: you only change the formatting here. Usually we put those changes in a follow up patch) File src/gsm/gsm0808.c: https://gerrit.osmocom.org/c/libosmocore/+/32932/comment/b7180209_33b546bc PS12, Line 22: #ifdef HAVE_SYS_SOCKET_H As far as I understand from the discussion around patch-set 5, this is to prevent building this code on embedded targets. I would split this part into a patch preceding this one. File tests/tlv/tlv_test.c: https://gerrit.osmocom.org/c/libosmocore/+/32932/comment/2ffe244f_1a500d76 PS12, Line 4: TLV parsing should work on embedded targets. I can see that this has ties to gsm0808 (osmo_bssap_tlv_parse). If we could remove this dependency then we could include the tlv_test in embedded builds again. I also see an additional problem: This is a unit test. We probably run into problems when we just remove this for some builds. The output will change and no longer match the .ok and .err files. (But the build passes in jenkins, maybe it isn't a problem though...) -- 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: 12 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: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 15:06:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in libosmo-netif[master]: examples: Use osmo_io in {ipa-,}stream-{client,server}
by laforge 08 Jun '23

08 Jun '23

Attention is currently required from: arehbein, pespin, daniel. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33194 ) Change subject: examples: Use osmo_io in {ipa-,}stream-{client,server} ...................................................................... Patch Set 4: (1 comment) File examples/ipa-stream-client.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33194/comment/f5ca0579_63a725b4 PS4, Line 176: osmo_stream_cli_set_iofd_read_cb(conn, read_cb); > I don't really like polluting the APIs in osmo_stream with implementation specific "iofd" names. […] I tend to agree with pau. -- 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: 4 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: Thu, 08 Jun 2023 15:01:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

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

08 Jun '23

Attention is currently required from: arehbein, pespin, daniel. laforge 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 5: (1 comment) Patchset: PS5: > implementation of osmo_stream should always use iofd, hence the old implementation is dropped (or #if 0 or ENV disabled for now). I'd love to see that, given that osmo_io has the select/poll backend anyway for backwards compatibility. The problem I see is: How would you handle compatibility with old code that registers a read_cb? That old code expects to do the actual read itself, from the file descriptor. But in osmo_io that already has happened by the infrastructure at the tiem you get to the read_cb... -- 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: 5 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: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 15:01:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in libosmo-netif[master]: examples: Use osmo_io in {ipa-,}stream-{client,server}
by pespin 08 Jun '23

08 Jun '23

Attention is currently required from: arehbein, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33194 ) Change subject: examples: Use osmo_io in {ipa-,}stream-{client,server} ...................................................................... Patch Set 4: (1 comment) File examples/ipa-stream-client.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33194/comment/3cf4fed5_e161ca7f PS4, Line 176: osmo_stream_cli_set_iofd_read_cb(conn, read_cb); I don't really like polluting the APIs in osmo_stream with implementation specific "iofd" names. Rather call them (since they are the preferred way from now on to use the objects): osmo_stream_cli_create2 osmo_stream_cli_set_read_cb2 (or osmo_stream_cli_set_read_cb_msg) -- 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: 4 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: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 14:33:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in libosmo-netif[master]: stream: Update log messages
by pespin 08 Jun '23

08 Jun '23

Attention is currently required from: daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33221 ) Change subject: stream: Update log messages ...................................................................... Patch Set 5: Code-Review+1 (1 comment) Patchset: PS5: Better move this small fixup commits before the main commit so that we already have them merged and the "complex iofd patch" looks easier to gasp. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33221 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Ife20b9d18e6ca86a06991d68165694e31052c58a Gerrit-Change-Number: 33221 Gerrit-PatchSet: 5 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 14:31:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

08 Jun '23

Attention is currently required from: arehbein, 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 5: (1 comment) Patchset: PS5: > Proposal: […] If you want to still keep the old ofd behavior, you could even set the ofd/iofd type internally based on whether the new osmo_stream_*_set_read_cb() was called. -- 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: 5 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: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 14:29:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

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

08 Jun '23

Attention is currently required from: arehbein, 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 5: (2 comments) Patchset: PS5: Proposal: - implementation of osmo_stream should always use iofd, hence the old implementation is dropped (or #if 0 or ENV disabled for now). - the old _create() functions with read_cb and close_cb are kept for backward compat - a new _create() function is added, which has no read_cb nor close_cb, but a const char *name. - Separate APIs are added to set the new iofd_read_cb and iofd_close_cb. - Internally, the osmo_stream code calls the read_cb() if non NULL, or/and the iofd_read_cb() if not NULL (the only difference afaiu is the extra msg ptr being passed). File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/4bf8852b_01fcb000 PS5, Line 1197: static int osmo_stream_srv_ofd_cb(struct osmo_fd *ofd, unsigned int what) I'd welcome if you did this kind of changes in separate previous patches to try to trim this one a bit so we can focus on how to add iofd in here. -- 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: 5 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: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 14:28:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0