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 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 10: (6 comments) File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/5368ac86_5373c2e5 PS7, Line 598: handle_connecting(cli, res); > This split into a separate function you could do it in a previous separate preparation patch. Ack https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/6c84d18b_b1b934e3 PS7, Line 620: if (msg && res <= 0) { > EAGAIN would trigger closing the socket? This shouldn't happen since osmo_io only writes to the fd if it's writable. https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/bc4afef1_4581ada5 PS7, Line 881: cli->ofd.priv_nr = 0; /* XXX */ > what about this XXX? Done https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/b6c5350a_6c95c2c9 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"? The relevant fd is in osmo_iofd_register(). In the reconnect case when osmo_stream_cli_open is called cli->iofd will already be set and osmo_iofd_register() registers a new fd. https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/7a08bcd5_9d49e29b PS7, Line 1540: osmo_stream_srv_destroy(conn); > just wondering whether you may need to return -EBADF here? Not in a void function :-) osmo_stream_srv_destroy() calls osmo_iofd_free() which does the right thing™ It'll close the fd, defer free() until after the callback has returned and not call the other callback. https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/b6e69d6f_6b476914 PS7, Line 1714: conn->iofd_read_cb = read_cb; > See, these can perfectly be separate setter APIs like we do for osmo_stream_cli. I never argued that they couldn't be -- 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: 10 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 14:07:20 +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-ttcn3-hacks[master]: BSSMAP_Templates: Add template to receive ChannelType IE
by fixeria 09 Jun '23

09 Jun '23

Attention is currently required from: jolly, laforge. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33145 ) Change subject: BSSMAP_Templates: Add template to receive ChannelType IE ...................................................................... Patch Set 4: Code-Review+2 (2 comments) File library/BSSMAP_Templates.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33145/comment/888c4402_40a0… PS3, Line 570: template BIT4 speech_data_ind > All fields in `record BSSMAP_IE_ChannelType` are mandatory (not optional), so all template params sh […] Done https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33145/comment/b62ef337_78c3… PS3, Line 570: template BSSMAP_IE_ChannelType tr_BSSMAP_IE_ChannelType(template BIT4 speech_data_ind, > template (present) as it always returns something specific? Done -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33145 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Ia6f283c17a1687ee65f8997bbeba0b09d1a462cf Gerrit-Change-Number: 33145 Gerrit-PatchSet: 4 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Fri, 09 Jun 2023 14:06:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-ttcn3-hacks[master]: BSSMAP_Templates: Add templates to receive CellID IE
by fixeria 09 Jun '23

09 Jun '23

Attention is currently required from: jolly, laforge, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33144 ) Change subject: BSSMAP_Templates: Add templates to receive CellID IE ...................................................................... Patch Set 4: (2 comments) File library/BSSMAP_Templates.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33144/comment/6608da07_978c… PS1, Line 371: template BSSMAP_IE_CellIdentifier tr_CellId_CGI(hexstring mcc, hexstring mnc, BssmapLAC lac, BssmapCI ci) > this looks really strange: a tr_* template having only value params, modifying another tr template, […] Done File library/BSSMAP_Templates.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33144/comment/aa636db1_8596… PS3, Line 441: template BSSMAP_IE_CellIdentifier tr_CellID_SAI(template (present) OCT3 mcc_mnc, > template (present) here, as I guess there's no way that it could return anything else Done -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33144 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I42d52d871c8011db7e0897dfe752afeefa6d9662 Gerrit-Change-Number: 33144 Gerrit-PatchSet: 4 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-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 09 Jun 2023 14:05:27 +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> Gerrit-MessageType: comment

1 0

[M] Change in osmo-ttcn3-hacks[master]: BSSMAP_Templates: Add templates to receive CellID IE
by fixeria 09 Jun '23

09 Jun '23

Attention is currently required from: jolly. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33144 ) Change subject: BSSMAP_Templates: Add templates to receive CellID IE ...................................................................... Patch Set 4: Code-Review+2 (2 comments) File library/BSSMAP_Templates.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33144/comment/b86717f5_fca3… PS3, Line 371: present > There's still something wrong with this template. […] We had a call with @jolly and agreed that it's better to accept MCC/MNC pair in form of already encoded blob. This obligates the caller to call `f_enc_mcc_mnc()` when using this template, but on the other hand allows passing `?`. Marking as resolved. https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33144/comment/ca30e74c_d6ad… PS3, Line 1467: GsmMcc mcc, GsmMnc mnc > Here I suggest keeping both `mcc` and `mnc` as they are, i.e. value parameters. […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33144 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I42d52d871c8011db7e0897dfe752afeefa6d9662 Gerrit-Change-Number: 33144 Gerrit-PatchSet: 4 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-Comment-Date: Fri, 09 Jun 2023 14:04:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: Added generation of include/osmocom/core/socket_compat.h
by pespin 09 Jun '23

09 Jun '23

Attention is currently required from: jolly. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33256 ) Change subject: Added generation of include/osmocom/core/socket_compat.h ...................................................................... Patch Set 2: (1 comment) File include/osmocom/core/Makefile.am: https://gerrit.osmocom.org/c/libosmocore/+/33256/comment/f48746f8_fab97aa5 PS2, Line 102: EXTRA_DIST = socket_compat.h.tpl we usually put this above, around line 83 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33256 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia3eafc992221900bbbf1760f669725bf9da92105 Gerrit-Change-Number: 33256 Gerrit-PatchSet: 2 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Fri, 09 Jun 2023 14:03:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: Added generation of include/osmocom/core/socket_compat.h
by jolly 09 Jun '23

09 Jun '23

Attention is currently required from: jolly. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/33256 to look at the new patch set (#2). Change subject: Added generation of include/osmocom/core/socket_compat.h ...................................................................... Added generation of include/osmocom/core/socket_compat.h This file is required to compile header files on machines that do not have sys/socket.h. Change-Id: Ia3eafc992221900bbbf1760f669725bf9da92105 --- M configure.ac M include/osmocom/core/Makefile.am A include/osmocom/core/socket_compat.h.tpl 3 files changed, 30 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/56/33256/2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33256 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia3eafc992221900bbbf1760f669725bf9da92105 Gerrit-Change-Number: 33256 Gerrit-PatchSet: 2 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-MessageType: newpatchset

1 0

[M] Change in osmo-ttcn3-hacks[master]: BSSMAP_Templates: Add templates to receive CellID IE
by pespin 09 Jun '23

09 Jun '23

Attention is currently required from: jolly. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33144 ) Change subject: BSSMAP_Templates: Add templates to receive CellID IE ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33144 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I42d52d871c8011db7e0897dfe752afeefa6d9662 Gerrit-Change-Number: 33144 Gerrit-PatchSet: 4 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-Comment-Date: Fri, 09 Jun 2023 14:00:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in libosmo-netif[master]: stream: Use cli->state to check if cli is already closed
by pespin 09 Jun '23

09 Jun '23

Attention is currently required from: daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33254 ) Change subject: stream: Use cli->state to check if cli is already closed ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33254 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I92231528da08f8891e20c1226b61989a65e00ccd Gerrit-Change-Number: 33254 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Fri, 09 Jun 2023 13:55:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in libosmo-netif[master]: stream: Factor out reconnection handling
by pespin 09 Jun '23

09 Jun '23

Attention is currently required from: daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33253 ) Change subject: stream: Factor out reconnection handling ...................................................................... Patch Set 1: (1 comment) File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33253/comment/493917bb_df80959a PS1, Line 472: osmo_fd_write_disable(&cli->ofd); what if connect_cb() freed the cli object? you'd be accessing it here. This needs to be moved inside stream_cli_handle_connecting at the proper place. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33253 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I418377eabd465ee4ffce9b4440e96287c7734924 Gerrit-Change-Number: 33253 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Fri, 09 Jun 2023 13:55:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: Added generation of include/osmocom/core/socket_compat.h
by laforge 09 Jun '23

09 Jun '23

Attention is currently required from: jolly. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33256 ) Change subject: Added generation of include/osmocom/core/socket_compat.h ...................................................................... Patch Set 1: (1 comment) File include/osmocom/core/Makefile.am: https://gerrit.osmocom.org/c/libosmocore/+/33256/comment/87330990_25616806 PS1, Line 98: socket_compat.h.tpl this likely needs to go into EXTRA_DIST or something along those lines. Otherwise "make dist" will not add it to the tarball. "make distcheck" should tell. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33256 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia3eafc992221900bbbf1760f669725bf9da92105 Gerrit-Change-Number: 33256 Gerrit-PatchSet: 1 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-CC: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Fri, 09 Jun 2023 13:49:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0