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 <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: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Sat, 29 Jul 2023 18:57:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de>
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment