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

[S] Change in libosmo-sccp[master]: osmo_sccp_simple_client_on_ss7_id(): Support ASP explicitly configure...
by pespin 08 Jun '23

08 Jun '23

Attention is currently required from: neels, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/33216 ) Change subject: osmo_sccp_simple_client_on_ss7_id(): Support ASP explicitly configured as sctp server ...................................................................... Patch Set 4: (1 comment) File src/sccp_user.c: https://gerrit.osmocom.org/c/libosmo-sccp/+/33216/comment/c3718bb4_e94d4b09 PS1, Line 633: if (default_local_ip) > I think in the end a slightly confusing code with huge additional functionality to the user might be […] Done -- To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/33216 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-sccp Gerrit-Branch: master Gerrit-Change-Id: I20de33edb8751a515d6719c49efadfc387dd85f8 Gerrit-Change-Number: 33216 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Thu, 08 Jun 2023 16:14:00 +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

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

08 Jun '23

Attention is currently required from: arehbein, fixeria, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33083 ) Change subject: gsm/ipa: Add segmentation callback ...................................................................... Patch Set 8: (5 comments) File src/gsm/ipa.c: https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/a870c855_c921fab6 PS8, Line 728: * -EIO, if the header declares a payload too large */ */ on the next line https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/8df9a684_074de13b PS8, Line 729: int ipa_segmentation_cb(struct msgb *msg) as per struct osmo_io_ops documentation: /*! call-back function to segment the data at message boundaries. * Needs to return the size of the next message. If it returns * -EAGAIN or a value larger than msgb_length() (message is incomplete) * osmo_io will wait for more data to be read. Other negative values * cause the msg to be discarded. */ int (*segmentation_cb)(struct msgb *msg); https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/2253840c_fe1fc679 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 the block). Then, you could already use sizeof(*hh) in line 731, instead of using different things for sizeof() as in line 737 which makes it confusing to read. https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/a30a90cf_89d1a9d8 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. https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/eb741a5b_9095490b 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 IPA message expects. Talking about that in the log message would be a lot more usefu for the user? since that probably means the dev has to increase the msgb size when allocating it? -- 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: 8 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 16:13:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in libosmo-sccp[master]: osmo_sccp_simple_client_on_ss7_id(): Support ASP explicitly configure...
by pespin 08 Jun '23

08 Jun '23

Attention is currently required from: neels, pespin. Hello Jenkins Builder, neels, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-sccp/+/33216 to look at the new patch set (#4). Change subject: osmo_sccp_simple_client_on_ss7_id(): Support ASP explicitly configured as sctp server ...................................................................... osmo_sccp_simple_client_on_ss7_id(): Support ASP explicitly configured as sctp server Right now, if a user configures an cs7 instance, as and asp (with sctp-role server) in the VTY, then the function osmo_sccp_simple_client_on_ss7_id() (used by osmo-bsc, osmo-msc, osmo-hnbgw, etc.) will silently change the config to be SCTP client. This is really confusing, since the user configured explicitly the ASP as server but ends up running as client. Instead, if the user explicitly configured the ASP as SCTP server, allow it (after validating the user also properly configured a xUA server through VTY). Change-Id: I20de33edb8751a515d6719c49efadfc387dd85f8 --- M src/sccp_user.c 1 file changed, 40 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/16/33216/4 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/33216 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-sccp Gerrit-Branch: master Gerrit-Change-Id: I20de33edb8751a515d6719c49efadfc387dd85f8 Gerrit-Change-Number: 33216 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

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

08 Jun '23

Attention is currently required from: pespin, fixeria, daniel. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33083 ) Change subject: gsm/ipa: Add segmentation callback ...................................................................... Patch Set 8: (2 comments) File src/gsm/ipa.c: https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/b1dde6eb_d9131fdd PS5, Line 722: int ipa_segmentation_cb(struct msgb *msg) > Please, proper doxygen documentation, it's a public API afaiu. Done https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/9a10f314_0b1158d5 PS5, Line 725: /* Haven't even the read entire header */ > IIUC, it should be "/* Haven't even read the entire header */". […] Had to reread it twice. I blame the heat 8] -- 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: 8 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 16:04:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

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

08 Jun '23

Attention is currently required from: arehbein, fixeria, daniel. Hello Jenkins Builder, laforge, 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 (#8). 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, 42 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/83/33083/8 -- 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: 8 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(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 pespin 08 Jun '23

08 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 6: (1 comment) Patchset: PS5: > Fine with your proposal, but I'd remove "iofd" and use "2" or alike, I'd really avoid naming "iofd" […] >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... Good point, I didn't think about that. Then probably keep the old code and set the OFD vs IOFD mode internally based on the APIs used. -- 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: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 15:54:13 +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 pespin 08 Jun '23

08 Jun '23

Attention is currently required from: jolly. pespin 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: (1 comment) File include/osmocom/gsm/gsm0808.h: https://gerrit.osmocom.org/c/libosmocore/+/32932/comment/979dd1c8_0a6df6e9 PS12, Line 22: #ifdef HAVE_SYS_SOCKET_H > the problem I see here is that this will actually be evaluated when building the app against libosmo […] Something to look into would be having a socket_compat.h (similar to timer_compat.h we have), and include it here, which could define the sockaddr_storage if not found using configure.ac or similar. -- 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-Comment-Date: Thu, 08 Jun 2023 15:52:11 +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]: gsm/ipa: Add segmentation callback
by pespin 08 Jun '23

08 Jun '23

Attention is currently required from: arehbein, fixeria, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33083 ) Change subject: gsm/ipa: Add segmentation callback ...................................................................... Patch Set 7: (2 comments) File src/gsm/ipa.c: https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/94861ce9_73c2b390 PS5, Line 722: int ipa_segmentation_cb(struct msgb *msg) > Ah I see now that my message was cut off. […] Please, proper doxygen documentation, it's a public API afaiu. https://gerrit.osmocom.org/c/libosmocore/+/33083/comment/61d60b8a_782767d0 PS5, Line 725: /* Haven't even the read entire header */ > I don't see anything wrong with it IIUC, it should be "/* Haven't even read the entire header */". So either I don't know English grammar or you are dyslexic :D (or I'm dyslexic :/) -- 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: 7 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 15:49:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> 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, 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 6: (1 comment) Patchset: PS5: > > Internally, the osmo_stream code calls the read_cb() if non NULL, or/and the iofd_read_cb() if not […] Fine with your proposal, but I'd remove "iofd" and use "2" or alike, I'd really avoid naming "iofd" since there's anyway nothing "iofd" specific struct from the interface point of view? -- 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: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 15:46:37 +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

[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 6: Code-Review+2 -- 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-Comment-Date: Thu, 08 Jun 2023 15:44:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0