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