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
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
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
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
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
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
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
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
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
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
