Attention is currently required from: arehbein, laforge, pespin.
daniel has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmo-netif/+/33201 )
Change subject: stream: Add IPA send function, pull IPA headers in segm. cb
......................................................................
Patch Set 11:
(7 comments)
Patchset:
PS11:
I really still fail to see why do we want to have
& use a new API which puts together prepending the […]
I think it looks nicer
with the helper, but I'd split up this commit into a part that changes the
segmentation_cb and one that adds the send helpers.
File examples/ipa-stream-client.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/225882f8_e022b85f
PS11, Line 109: if (osmo_ipa_process_msg(msg) < 0) {
Remove this, it's now handled in the segmentation_cb
File examples/ipa-stream-server.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/2f74e189_050dada7
PS11, Line 54: if (osmo_ipa_process_msg(msg) < 0) {
Remove this, it's now handled in the segmentation_cb
https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/24775613_32fcd78f
PS11, Line 60: osmo_ipa_stream_srv_send(conn, IPAC_PROTO_UNSPECIFIED,
IPAC_PROTO_UNSPECIFIED, msg);
Using the proto/proto_ext getters here is more clear imho than using
IPAC_PROTO_UNSPECIFIED and dealing with it in osmo_ipa_stream_srv_send()
File src/ipa.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/c5dbc8f9_4483f5a5
PS11, Line 481: if (ipaccess_proto == IPAC_PROTO_UNSPECIFIED) {
: ipaccess_proto = msg_get_ipa_proto(msg);
: pe = msg_get_ipa_proto_ext(msg);
: }
I wouldn't special-case this here. Also I'm not sure where IPAC_PROTO_UNSPECIFIED
is defined and what its value is.
File src/stream.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/018a01ca_84206334
PS11, Line 47: #include <osmocom/netif/ipa.h>
Unneeded
https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/d8eafb25_54a1cc43
PS11, Line 612: if (res == 0) {
Unrelated/Unneeded?
--
To view, visit
https://gerrit.osmocom.org/c/libosmo-netif/+/33201
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I61e1fe59166c46595efe8c1f32b8f2607cb6c529
Gerrit-Change-Number: 33201
Gerrit-PatchSet: 11
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: daniel <dwillmann(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 10 Jul 2023 16:52:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment