Attention is currently required from: laforge, pespin, daniel. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33197 )
Change subject: stream: Add server-side (segmentation) support for IPA ...................................................................... Patch Set 12: (8 comments) File include/osmocom/netif/ipa.h: https://gerrit.osmocom.org/c/libosmo-netif/+/33197/comment/adea8e8a_d0c95623 PS10, Line 6: #include <osmocom/netif/stream.h> > Not sure why ipa. […] Done https://gerrit.osmocom.org/c/libosmo-netif/+/33197/comment/c73a9db4_aa6ca599 PS10, Line 46: int osmo_ipa_segmentation_cb(struct msgb *msg); > This belongs in the previous commit Done File include/osmocom/netif/stream.h: https://gerrit.osmocom.org/c/libosmo-netif/+/33197/comment/94d3c33b_b4c08386 PS1, Line 28: /* TODO: Add protocols for > Done. […] Done File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33197/comment/0f040a9f_fca3c3e7 PS10, Line 39: #include <osmocom/gsm/ipa.h> > Not needed. Done File tests/stream/stream_test.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33197/comment/51aa40c5_f016f6c5 PS10, Line 540: osmo_stream_cli_set_addr(osc, "127.0.0.11"); > better use 127.0.0.1 here, all IP addresses being available in linux specific > iirc. I don't understand the second part of the sentence... I copied this from the existing test code for the other tests. Should that be changed as well then? Or can we just leave it at `127.0.0.11`? https://gerrit.osmocom.org/c/libosmo-netif/+/33197/comment/74c8fd4e_21215fc2 PS10, Line 621: rc = clock_gettime(CLOCK_MONOTONIC, &start); > usually alarm() is used for this kind of stuff (exit process on timeout). What is the advantage of using `alarm()` instead, and how wouldn't I need to register some sort of (additional, if it's already handled in lower layers) alarm handling for SIGALRM if I wanted a diagnostic message? @daniel I prefer setting an amount of time to setting an amount of iterations, but maybe that's just me I'd like to leave it as is if possible. Did try it out and it worked... https://gerrit.osmocom.org/c/libosmo-netif/+/33197/comment/b200fbbb_020e5415 PS10, Line 629: rc = clock_gettime(CLOCK_MONOTONIC, &now); > osmo_clock_gettime. […] (see Daniel's reply above) https://gerrit.osmocom.org/c/libosmo-netif/+/33197/comment/21a51fea_fe787c15 PS10, Line 684: rc = test_segm_ipa_stream_srv_run(tall_test, host, port, srv); > we don't usually do this kind of return in tests. […] Done -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33197 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I6c91ff385cb5f36ab6b6c96d0e44997995d0d24c Gerrit-Change-Number: 33197 Gerrit-PatchSet: 12 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:57:43 +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
