Attention is currently required from: laforge, daniel.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32759 )
Change subject: [WIP] osmo_io: Support detecting connect
......................................................................
Patch Set 4:
(1 comment)
File src/core/osmo_io_poll.c:
https://gerrit.osmocom.org/c/libosmocore/+/32759/comment/8f6c863f_b3161fd2
PS2, Line 118: if (osmo_iofd_txqueue_len(iofd) == 0)
> in general, with osmo_io, the question is "why would you want to bother the user with XYZ". […]
I'm fine with that approach. I just want to make sure we don't end up calling write/read_enable/disable from both ends (internal code and user) since that would be a mess. Hence why I'm asking.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32759
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I893cbc3becd5e125f2f06b3654578aed0aacadf3
Gerrit-Change-Number: 32759
Gerrit-PatchSet: 4
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 18 May 2023 10:25:45 +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: laforge, daniel.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32758 )
Change subject: osmo_io: Improve handling of segmentation_cb
......................................................................
Patch Set 3:
(1 comment)
File include/osmocom/core/osmo_io.h:
https://gerrit.osmocom.org/c/libosmocore/+/32758/comment/db6e1aee_7036377f
PS3, Line 46: * Needs to return the size of the next segment. If it returns
> The user should be concerned with sending or receiving protocol-level messages, and not be botered with TCP stream segmentation.
Agree with this. However, I wonder whether doing all this in osmo_io is the right place to do it, or rather keep this simpler and move segmentation one layer up (creating a new layer if needed).
I'm not saying this is wrong, just sharing my concerns. In any case, I think the segmentation_cb documentation should be improved, since at leas to me it's not clearer how this works without having to look deeply at the code.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32758
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I6a0eebb8d4490f09a3cc6eb97d4ff47b4a8fd377
Gerrit-Change-Number: 32758
Gerrit-PatchSet: 3
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 18 May 2023 10:23: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>
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin, daniel.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32759 )
Change subject: [WIP] osmo_io: Support detecting connect
......................................................................
Patch Set 4: Code-Review+1
(2 comments)
Commit Message:
https://gerrit.osmocom.org/c/libosmocore/+/32759/comment/61c2bd05_179847cb
PS4, Line 9: non blocking connect() which is signalled by
: marking the fd writable.
"non-blocking connect(), which as per definition of the socket API is signaled from OS to user by marking the file descriptor writable"?
File src/core/osmo_io_poll.c:
https://gerrit.osmocom.org/c/libosmocore/+/32759/comment/0accc9cd_65aa84ae
PS2, Line 118: if (osmo_iofd_txqueue_len(iofd) == 0)
> AFAIU it's the job of the user of this API to control this kind of enabled/disabled? […]
in general, with osmo_io, the question is "why would you want to bother the user with XYZ". If the answer to that question is "no", then it should be handled by the osmo_io provider (library) and not the user.
The user shouldn't know about low-level details like this. the user shouldn't know low-level implementation details of specific backend implementations (like if there's a libosmocore write-queue at all).
The user wants to write data, and get notified once it is written, or once it has failed to write. Anything else is an implementation detail.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32759
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I893cbc3becd5e125f2f06b3654578aed0aacadf3
Gerrit-Change-Number: 32759
Gerrit-PatchSet: 4
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 18 May 2023 10:19:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
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: pespin, daniel.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32758 )
Change subject: osmo_io: Improve handling of segmentation_cb
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32758
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I6a0eebb8d4490f09a3cc6eb97d4ff47b4a8fd377
Gerrit-Change-Number: 32758
Gerrit-PatchSet: 3
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 18 May 2023 10:13:59 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: pespin, daniel.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32758 )
Change subject: osmo_io: Improve handling of segmentation_cb
......................................................................
Patch Set 3:
(2 comments)
Commit Message:
https://gerrit.osmocom.org/c/libosmocore/+/32758/comment/672ec2f1_eb5f3948
PS3, Line 9: int
in
File include/osmocom/core/osmo_io.h:
https://gerrit.osmocom.org/c/libosmocore/+/32758/comment/95c9980d_082c90b2
PS3, Line 46: * Needs to return the size of the next segment. If it returns
> Maybe the initial point here is that I'm not sure why the segmentation callback is needed in osmo_io […]
pespin: The point of osmo_io is to be a one-stop shop for all users of the kind of telecom protocols that we see - irrespective of whether they're on SEQPACKET bearers like SCTP or on the cumbersome "preserve message boundaries over TCP stream" approahc that IPA does. This significantly unifies code that has to deal with both variants, such as e.g. a BSC speaking BSSAP over either M3UA/SCTP or over IPA. The user should be concerned with sending or receiving protocol-level messages, and not be botered with TCP stream segmentation.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32758
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I6a0eebb8d4490f09a3cc6eb97d4ff47b4a8fd377
Gerrit-Change-Number: 32758
Gerrit-PatchSet: 3
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 18 May 2023 10:13:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment