Osmocom Libraries » libosmo-netif

(04:27:29 PM) pespin: arehbein, asdfuser why is all that IPA specific stuff being added inside osmo_stream APIs? doesn't look good to me tbh, at least without knowing more context on why this is envisioned this way

(04:28:16 PM) pespin: if really some support needs to be added inside osmo_stream, then at least I would make it so that osmo_stream handles/uses that in a generic way. IMHO it should know about IPA at all

(04:28:53 PM) LaF0rge: pespin: We've always used osmonetif/stream for IPA.  Its just that in pre-osmoio code the caller/user/application did the read/recv themselves and had to take care of de-segmentation

(04:29:27 PM) asdfuser: pespin: The idea was to not deal with segmentation/reassembly in IPA-based protocols (which are a lot)

(04:29:29 PM) LaF0rge: pespin: with osmo_io, the level of abstraction is slightly higher: the read/recv/recvmsg/... is done within osmo_io (possibly io_uring backend) and the application gets a msgb back

(04:30:20 PM) LaF0rge: pespin: so if you want to always return a msgb with a full signalling message to a libosmo-netif user, whether it uses a SCTP protocol (like M3UA) or IPA based protocol, then the level of abstraction to the user is always the  same, no matter of the underlying protocol

(04:30:27 PM) pespin: asdfuser, then add some sort of generic API in osmo_stream to hook in generic segmentation stuff

(04:30:36 PM) pespin: and provide separate APIs/objects to support creating an osmo-stream with IPA on top

(04:31:17 PM) LaF0rge: pespin: but in reality to the user (think of SCCPlite vs. AoIP) looks at this as the same boundary / level of abstraction

(04:32:20 PM) LaF0rge: I'm not arguing the current detailed API implementation must be exactly this way.  I'm just saying the upper boundary of libosmo-netif is what should provide the same interface no matter if the underlying technology is SCTP or some IPA based protocol

(04:33:36 PM) pespin: well to me SCTP vs UDP vs TCP is clearly the same level, while IPA is just a (half) layer on top

(04:33:59 PM) LaF0rge: pespin: that's your point of view as a protocol stack implementor

(04:34:17 PM) LaF0rge: pespin: the *user application* point of view is "something that transmits a message with message boundary in-order from A to B"

(04:34:28 PM) LaF0rge: s/with/while preserving/

(04:35:19 PM) pespin: yeah well, that's then a osmo_msg_boundary_in_order, not a osmo_stream :D

(04:35:22 PM) LaF0rge: pespin: [very] recent kernels even have support for this kind of functionality inside the kernel (on top of TCP) so the sockets (and hence users) don't have to deal with it

(04:35:35 PM) LaF0rge: pespin: and it's still a SOCK_STREAM :P

(04:36:16 PM) pespin: yeah but hence my point, you are mixing 2 things (one on top of the other) in the same object, and I think it should be split. Specially IPA-specific logic

(04:36:40 PM) LaF0rge: I agree the "Stream" is not an ideal name anymore. but do you really want to copy+paste dozens of functions with a new name and change all programs to use that new name?

(04:36:44 PM) pespin: if one wants to add some sort of "message-boundaring-addition" API on top of osmo_stream fine, but I wouldn't start adding tons of IPA specific stuff there

(04:37:24 PM) pespin: LaF0rge, no, I want to add APIs which are generic to osmo-stream and leave the "I want to use IPA" part to the user of osmo_stream

(04:37:39 PM) pespin: I don't want to read the "IPA" string anywhere in osmo_stream :D

(04:38:08 PM) LaF0rge: pespin: this means every current user will have to repeat the same stuff all over again, or have to be ported to a completely new set of APIs

(04:38:17 PM) pespin: that's my point of view. That being said, I'm happy to discuss or provide ideas on how to sort out problems

(04:38:33 PM) LaF0rge: pespin: right now we have probably 10 different implementations of doing IPA stuff all over our code base

(04:38:55 PM) pespin: completely new set of APIs: no, they still use osmo_stream, and anyway they are changing the ipa APIs there now

(04:38:58 PM) LaF0rge: exactly due to the fact that every process used to read/write to its sockets directly, ...

(04:39:27 PM) pespin: LaF0rge, I'm in favour of unifying that, but I don't think implementing IPA inside osmo_stream is the way to go

(04:39:55 PM) LaF0rge: pespin: either every user has to implement the IPA handling internally and use the "pure netif", or they have to switch to a new set of API on top of  netif/stream, which means tons of new symbols doing 99% of the same stuff and lots of code changes everywhere

(04:41:13 PM) pespin: tons of new symbols: 1-2-3 symbols from generic callbacks? which anyway are being added now?

(04:41:35 PM) pespin: lots of code changes everywhere: afaiu the patches presenting the osmo_netif API is already requiring changes everywhere

(04:42:10 PM) LaF0rge: pespin: I'm talking about wrapping osmo_stream_* APIs in somehing else, which carries the state you don't want in 'struct osmo_stream_srv/clnt'

(04:42:25 PM) pespin: every user has to implement the IPA handling internally: Not really, helper functions can be added as an extension in libosmo-netif, just not inside osmo_stream

(04:42:51 PM) LaF0rge: pespin: the only changes required with the current proposal ar e those that are required anyway (just affecting read/write calls), but not every osmo_stream* call

(04:42:54 PM) pespin: LaF0rge, I was not necessarily thinking on wrapping all calls in other objects

(04:43:20 PM) LaF0rge: pespin: then how do you want to do it? you need additional state for every connection

(04:43:20 PM) LaF0rge: pespin: you don't want that in the osmo_stream_srv etc.

(04:43:46 PM) pespin: there's a priv struct. Or another generic one "segmentation_state" can be added, which callbacks, etc.

(04:43:53 PM) LaF0rge: pespin: so where will it go? in the application? that's what we want to avoid. the application should not have to care about it.  so you have to wrap every osmo_stream* struct in another new struct and add new apis for all calls...

(04:44:20 PM) LaF0rge: https://gerrit.osmocom.org/c/libosmo-netif/+/33197 is just adding a new 'mode' member and the segmentation call-backs are already implemented in a way that IPA is not the only possible user

(04:44:45 PM) pespin: It doesn't need to be a wrapper struct, it can be a sibling one which can be added/configured, etc. to interact with osmo_stream

(04:45:49 PM) pespin: what about https://gerrit.osmocom.org/c/libosmo-netif/+/33201/5/src/stream.c ?

(04:46:26 PM) LaF0rge: what about it?

(04:46:46 PM) pespin: IPA logic internally in there, plus new APIs like osmo_stream_srv_send_ipa()

(04:46:50 PM) LaF0rge: it adds a convenience wrapper on top of stream_srv_send so yo ucan specify the IPA proto + ext_proto

(04:47:07 PM) LaF0rge: exactly what makse sense to me;)

(04:47:27 PM) pespin: and cannot that be done generically in osmo_stream API?

(04:47:31 PM) LaF0rge: though one could pass the IPA protocol via msgb->cb[] to avoid a new symbol.

(04:48:09 PM) pespin: if we later on we want to add more message boundary systems on top of TCP then we add another API, one for each type?

(04:48:10 PM) LaF0rge: what's the problem with a function doing some extra stuff (osmo_stream_srv_send_ipa) which then calls osmo_stream_srv_send?

(04:48:30 PM) LaF0rge: yes, what's the problem with more convenience functions?  If you don't have that, every user has to implement this privately

(04:49:28 PM) pespin: you can have convenience functions, which use generic APIs/callbacks from osmo_stream. But I don't liking filling osmo_stream with IPA specific logic.

(04:49:32 PM) asdfuser: The main issue with another layer I think is having a read/write_cb in osmo_io, then one in osmo_stream_* handling the connect logic and then one in the ipa layer handling the decoding (segmentation would be handled in osmo_io already) plus the actual user callback

(04:51:10 PM) pespin: asdfuser, my understanding (I may be wrong) is that to support "message-boundary" protocols on top of stream, you need an extra read_cb, write_cb and segmentation_cb to apply extra logic

(04:51:19 PM) roh [~roh@osmocom/roh] entered the room.

(04:51:57 PM) asdfuser: We could rename to osmo_ipa_stream_cli_send(), but it doesn't make sense to have a struct osmo_ipa_stream_cli

(04:52:46 PM) asdfuser: pespin: No, segmentation is built into osmo_io, so from osmo_stream_* POV you only need to set the segmentation_cb

(04:52:56 PM) pespin: asdfuser, so my proposal would be to have some sort of generic "message_boundary_cfg" fields in osmo_stream where you can configure the extra logic. And then we can have a separate module osmo_stream_ipa (stream_ipa.h for instance) with those functions which can be passed as pointers

(04:53:26 PM) pespin: just to be clear, I'm not saying it should be implemented in one specific way, I'm just sharing my concerns and trying to get something better than what I'm seeing now

(04:53:30 PM) asdfuser: The reason read and write_cb are changed in the patchset is because we also want to parse the ipa header

(04:55:04 PM) pespin: asdfuser, you want to parse the ipa header because you need it for segmentation iiuc?

(04:55:29 PM) pespin: but can't you do that inside the segmentation cb already?

(04:56:05 PM) asdfuser: Maybe LaF0rge 's idea of using msg->cb[] also for sending would improve the API. We then wouldn't need an osmo_stream_*_send_ipa function

(04:56:28 PM) pespin: asdfuser, can you point me to it?

(04:56:35 PM) pespin: or briefly explain

(04:56:48 PM) asdfuser: pespin: Well, the segmentation_cb wasn't designed to change the msgb

(04:57:14 PM) LaF0rge: https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comments/e7542ad7_700a835c