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 <arehbein@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin@sysmocom.de> Gerrit-CC: daniel <dwillmann@sysmocom.de> Gerrit-CC: laforge <laforge@osmocom.org> Gerrit-Attention: laforge <laforge@osmocom.org> Gerrit-Attention: pespin <pespin@sysmocom.de> Gerrit-Attention: daniel <dwillmann@sysmocom.de> Gerrit-Comment-Date: Sat, 29 Jul 2023 18:58:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <arehbein@sysmocom.de> Comment-In-Reply-To: laforge <laforge@osmocom.org> Comment-In-Reply-To: pespin <pespin@sysmocom.de> Comment-In-Reply-To: daniel <dwillmann@sysmocom.de> Gerrit-MessageType: comment