Attention is currently required from: laforge, pespin, daniel. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33201 )
Change subject: stream: Add and use IPA send function ...................................................................... Patch Set 13: (10 comments) File examples/ipa-stream-client.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/39ea5790_a03fe733 PS11, Line 109: if (osmo_ipa_process_msg(msg) < 0) { > what about this comment? Done File examples/ipa-stream-server.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/1c6bc327_d66ed1a3 PS11, Line 54: if (osmo_ipa_process_msg(msg) < 0) { > Remove this, it's now handled in the segmentation_cb This had previously been done separately in change I53283ec7bd7f07dfa612681ae84af93d5cd098b9 . I have adapted the patch series now since your uploads incorporated removing this in previous changes of the series. https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/2a224216_0c1d18bc 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 deal […] I've changed it and I couldn't agree more. File include/osmocom/netif/ipa.h: https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/63e37189_6e441808 PS12, Line 57: #define IPAC_PROTO_UNSPECIFIED -1 > it's a bit strange to declare this here while others are probably declared > somewhere else? Had renamed this to `OSMO_IPA_IPAC_PROTO_UNSPECIFIED` (the 'other' values are in `libosmocore.git:src/gsm/protocoll/ipaccess.h`; assumed you wouldn't have wanted a non-protocol value in there), but then removed the functionality using it due to another review comment. File src/ipa.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/aecfd2d5_12e68ed7 PS10, Line 475: void ipa_stream_srv_send > ah yes I wondered about this shortly... […] Done File src/ipa.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/b48a6481_d44cf902 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. […] I assume by that you mean you don't want this functionality anymore, altogether (otherwise I have no idea where else this check should go). I removed it. File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/fb220732_d14da066 PS1, Line 1959: OSMO_ASSERT > Yes, because `conn` isn't used by this function before it is passed to > `osmo_stream_srv_send()`, whi […] Done File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/2f907762_8cb5c868 PS5, Line 1947: /* msgb_l1(msg) is expected to be set */ > The idea was providing the possibility of quickly reusing the headers of a > message received in IPA m […] Done File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/49e009ae_c9754cda PS11, Line 47: #include <osmocom/netif/ipa.h> > Unneeded Done https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/4545f5c3_3d7f354b PS11, Line 612: if (res == 0) { > Unrelated/Unneeded? Done -- 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: 13 Gerrit-Owner: arehbein <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <[email protected]> Gerrit-CC: daniel <[email protected]> Gerrit-CC: laforge <[email protected]> Gerrit-Attention: laforge <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Attention: daniel <[email protected]> Gerrit-Comment-Date: Sat, 29 Jul 2023 18:58:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <[email protected]> Comment-In-Reply-To: laforge <[email protected]> Comment-In-Reply-To: pespin <[email protected]> Comment-In-Reply-To: daniel <[email protected]> Gerrit-MessageType: comment
