July 2023 - gerrit-log - lists.osmocom.org

[M] Change in libosmo-netif[master]: stream: Add IPA send function, pull IPA headers in segm. cb
by arehbein 09 Jul '23

09 Jul '23

Attention is currently required from: arehbein, laforge. Hello Jenkins Builder, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/33201 to look at the new patch set (#9). Change subject: stream: Add IPA send function, pull IPA headers in segm. cb ...................................................................... stream: Add IPA send function, pull IPA headers in segm. cb Also: Adapt the example to work with the new changes. Related OS#5753, OS#5751 Depends: libosmocore.git Ifc7cf0629e93864bfb3e36c64143d4ccc777e4b6 Change-Id: I61e1fe59166c46595efe8c1f32b8f2607cb6c529 --- M examples/ipa-stream-client.c M examples/ipa-stream-server.c M include/osmocom/netif/ipa.h M include/osmocom/netif/stream.h M src/ipa.c M src/stream.c M tests/stream/stream_test.c M tests/stream/stream_test.ok 8 files changed, 154 insertions(+), 80 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/01/33201/9 -- 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: 9 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newpatchset

1 0

[L] Change in libosmo-netif[master]: stream: Add server-side (segmentation) support for IPA
by arehbein 09 Jul '23

09 Jul '23

Attention is currently required from: laforge, pespin. Hello Jenkins Builder, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/33197 to look at the new patch set (#8). Change subject: stream: Add server-side (segmentation) support for IPA ...................................................................... stream: Add server-side (segmentation) support for IPA With this commit, IPA segmentation can be taken care of by setting the segmentation callback osmo_ipa_segmentation_cb(). Depends: libosmocore.git I3a639e6896cc3b3fc8e9b2e1a58254710efa0d3f Related: OS#5753, OS#5751 Change-Id: I6c91ff385cb5f36ab6b6c96d0e44997995d0d24c --- M include/osmocom/netif/ipa.h M include/osmocom/netif/stream.h M src/stream.c M tests/stream/stream_test.c M tests/stream/stream_test.ok 5 files changed, 390 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/97/33197/8 -- 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: 8 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-MessageType: newpatchset

1 0

[S] Change in libosmo-netif[master]: examples: Add osmo_io based segmentation
by arehbein 09 Jul '23

09 Jul '23

Attention is currently required from: arehbein. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/33200 to look at the new patch set (#8). Change subject: examples: Add osmo_io based segmentation ...................................................................... examples: Add osmo_io based segmentation Related: OS#5751, OS#5753 Change-Id: Ibf5fd3a606da7888f23e6f75cc30fe433897a97b --- M examples/ipa-stream-client.c M examples/ipa-stream-server.c 2 files changed, 12 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/00/33200/8 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33200 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Ibf5fd3a606da7888f23e6f75cc30fe433897a97b Gerrit-Change-Number: 33200 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in libosmo-netif[master]: examples: Add extension header octet to example
by arehbein 09 Jul '23

09 Jul '23

Attention is currently required from: arehbein. Hello Jenkins Builder, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/33202 to look at the new patch set (#8). Change subject: examples: Add extension header octet to example ...................................................................... examples: Add extension header octet to example Mainly to make data/packets look (pseudo-)correct in Wireshark Change-Id: I962b9edcba65cdc98da00d2f8753dc5acd481502 --- M examples/ipa-stream-client.c 1 file changed, 14 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/02/33202/8 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33202 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I962b9edcba65cdc98da00d2f8753dc5acd481502 Gerrit-Change-Number: 33202 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[L] Change in libosmo-netif[master]: stream: Add client-side (segmentation) support for IPA
by arehbein 09 Jul '23

09 Jul '23

Attention is currently required from: arehbein, laforge. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/33198 to look at the new patch set (#10). Change subject: stream: Add client-side (segmentation) support for IPA ...................................................................... stream: Add client-side (segmentation) support for IPA With this commit, IPA segmentation is taken care of by setting the segmentation callback provided by libosmo-netif. Depends: libosmocore.git I3a639e6896cc3b3fc8e9b2e1a58254710efa0d3f Depends: libosmocore.git I5ff2f1416f294ac84b4240fd96da4aa3255fb27b Related: OS#5753, OS#5751 Change-Id: I822abf52c6ae396c90b5c50228a0a39c848d3de6 --- M include/osmocom/netif/stream.h M src/stream.c M tests/stream/stream_test.c M tests/stream/stream_test.ok 4 files changed, 330 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/98/33198/10 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33198 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I822abf52c6ae396c90b5c50228a0a39c848d3de6 Gerrit-Change-Number: 33198 Gerrit-PatchSet: 10 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in libosmo-netif[master]: example: Remove calls to osmo_ipa_process_msg()
by arehbein 09 Jul '23

09 Jul '23

Attention is currently required from: arehbein, pespin. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/33205 to look at the new patch set (#8). Change subject: example: Remove calls to osmo_ipa_process_msg() ...................................................................... example: Remove calls to osmo_ipa_process_msg() That call now happens up the call stack in IPA mode Change-Id: I53283ec7bd7f07dfa612681ae84af93d5cd098b9 --- M examples/ipa-stream-client.c M examples/ipa-stream-server.c 2 files changed, 11 insertions(+), 11 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/05/33205/8 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33205 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I53283ec7bd7f07dfa612681ae84af93d5cd098b9 Gerrit-Change-Number: 33205 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in libosmo-netif[master]: stream: Add IPA send function, pull IPA headers in segm. cb
by arehbein 09 Jul '23

09 Jul '23

Attention is currently required from: arehbein, laforge. Hello Jenkins Builder, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/33201 to look at the new patch set (#8). Change subject: stream: Add IPA send function, pull IPA headers in segm. cb ...................................................................... stream: Add IPA send function, pull IPA headers in segm. cb Also: Adapt the example to work with the new changes. Related OS#5753, OS#5751 Depends: libosmocore.git Ifc7cf0629e93864bfb3e36c64143d4ccc777e4b6 Change-Id: I61e1fe59166c46595efe8c1f32b8f2607cb6c529 --- M examples/ipa-stream-client.c M examples/ipa-stream-server.c M include/osmocom/netif/ipa.h M include/osmocom/netif/stream.h M src/ipa.c M src/stream.c M tests/stream/stream_test.c M tests/stream/stream_test.ok 8 files changed, 155 insertions(+), 81 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/01/33201/8 -- 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: 8 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in libosmocore[master]: msgb: Add helper macro to decide if a segment is incomplete
by arehbein 09 Jul '23

09 Jul '23

arehbein has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/33655 ) Change subject: msgb: Add helper macro to decide if a segment is incomplete ...................................................................... msgb: Add helper macro to decide if a segment is incomplete This helps avoid common logical fallacies that that happen quickly during coding (i.e. giving a false logical condition) for a condition any segmentation callback will need. Related: OS#5753 Change-Id: Ifc7cf0629e93864bfb3e36c64143d4ccc777e4b6 --- M include/osmocom/core/msgb.h 1 file changed, 21 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/55/33655/1 diff --git a/include/osmocom/core/msgb.h b/include/osmocom/core/msgb.h index 5c58c84..372b85b 100644 --- a/include/osmocom/core/msgb.h +++ b/include/osmocom/core/msgb.h @@ -773,4 +773,11 @@ return osmo_hexdump((const unsigned char*) msgb_l4(msg), msgb_l4len(msg)); } +/*! Macro for checking a simple condition (to avoid accidental bugs) + * \param[in] msg Target 'struct msg *' + * \param[in] len_from_msg_metadata Length of data as expected from metadata of msg + * \returns boolean indicating if the data segment starting at msg->data is incomplete + */ +#define msgb_segment_is_incomplete(len_from_msg_metadata, msg)\ + (len_from_msg_metadata > (msg)->len) /*! @} */ -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33655 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ifc7cf0629e93864bfb3e36c64143d4ccc777e4b6 Gerrit-Change-Number: 33655 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in libosmocore[master]: Revert "gsm/ipa: Add segmentation callback"
by arehbein 09 Jul '23

09 Jul '23

arehbein has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/33653 ) Change subject: Revert "gsm/ipa: Add segmentation callback" ...................................................................... Revert "gsm/ipa: Add segmentation callback" It has been decided that the segmentation callback be changed and moved to libosmo-netif, so we remove it here. This reverts commit 2c59d1285e8f4c47122e1749ff1ac60e99a1874e. Related: OS#5753 Change-Id: I9b380326c63587fc79d6a5d8cd458188074fc55d --- M include/osmocom/gsm/ipa.h M src/gsm/ipa.c M src/gsm/libosmogsm.map 3 files changed, 15 insertions(+), 33 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/53/33653/1 diff --git a/include/osmocom/gsm/ipa.h b/include/osmocom/gsm/ipa.h index 8a4dd3f..851b58e 100644 --- a/include/osmocom/gsm/ipa.h +++ b/include/osmocom/gsm/ipa.h @@ -79,6 +79,3 @@ int ipa_msg_recv(int fd, struct msgb **rmsg); int ipa_msg_recv_buffered(int fd, struct msgb **rmsg, struct msgb **tmp_msg); - -/* Callback for segmenting TCP stream data into IPA packets */ -int ipa_segmentation_cb(struct msgb *msg); diff --git a/src/gsm/ipa.c b/src/gsm/ipa.c index 4092614..cd95c6b 100644 --- a/src/gsm/ipa.c +++ b/src/gsm/ipa.c @@ -30,7 +30,6 @@ #include <stdint.h> #include <errno.h> #include <stdlib.h> -#include <inttypes.h> #include <sys/types.h> @@ -719,32 +718,4 @@ return nmsg; } -/*! Segmentation callback used by libosmo-netif streaming backend - * See definition of `struct osmo_io_ops` for callback semantics - * \param[in] msg Original `struct msgb` received via osmo_io - * \returns The total packet length indicated by the first header, - * otherwise - * -EAGAIN, if the header has not been read yet, - * -ENOBUFS, if the header declares a payload too large - */ -int ipa_segmentation_cb(struct msgb *msg) -{ - const struct ipaccess_head *hh = (const struct ipaccess_head *) msg->data; - size_t payload_len, total_len; - size_t available = msgb_length(msg) + msgb_tailroom(msg); - if (msgb_length(msg) < sizeof(*hh)) { - /* Haven't even read the entire header */ - return -EAGAIN; - } - payload_len = osmo_ntohs(hh->len); - total_len = sizeof(*hh) + payload_len; - if (OSMO_UNLIKELY(available < total_len)) { - LOGP(DLINP, LOGL_ERROR, "Not enough space left in message buffer. " - "Have %zu octets, but need %zu\n", - available, total_len); - return -ENOBUFS; - } - return total_len; -} - /*! @} */ diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map index 0dfffd3..951cbcb 100644 --- a/src/gsm/libosmogsm.map +++ b/src/gsm/libosmogsm.map @@ -691,7 +691,6 @@ ipa_ccm_id_resp_parse; ipa_ccm_make_id_resp; ipa_ccm_make_id_resp_from_req; -ipa_segmentation_cb; ipa_msg_alloc; ipa_msg_recv; ipa_msg_recv_buffered; -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33653 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I9b380326c63587fc79d6a5d8cd458188074fc55d Gerrit-Change-Number: 33653 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in libosmocore[master]: osmo_io: Adapt osmo_io msg processing
by arehbein 09 Jul '23

09 Jul '23

arehbein has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/33654 ) Change subject: osmo_io: Adapt osmo_io msg processing ...................................................................... osmo_io: Adapt osmo_io msg processing - segm. callback semantics have been changed, this reflects those changes (e.g., the segm. callback will now change each message once read) - since the segm. callback can now pull headers, the length compared to the length expected from header data must be read after calling the segmentation callback Related: OS#5753 Change-Id: I5ff2f1416f294ac84b4240fd96da4aa3255fb27b --- M src/core/osmo_io.c 1 file changed, 20 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/54/33654/1 diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index fdb9e32..c561583 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -223,29 +223,28 @@ */ static enum iofd_seg_act iofd_handle_segmentation(struct osmo_io_fd *iofd, struct msgb *msg, struct msgb **pending_out) { - int extra_len, received_len; + int extra_len, received_len_post_segm_cb; struct msgb *msg_pending; - received_len = msgb_length(msg); - if (!iofd->io_ops.segmentation_cb) { *pending_out = NULL; return IOFD_SEG_ACT_HANDLE_ONE; } int expected_len = iofd->io_ops.segmentation_cb(msg); + received_len_post_segm_cb = msgb_length(msg); if (expected_len == -EAGAIN) { goto defer; } else if (expected_len < 0) { /* Something is wrong, skip this msgb */ LOGPIO(iofd, LOGL_ERROR, "segmentation_cb returned error (%d), skipping msg of size %d\n", - expected_len, received_len); + expected_len, received_len_post_segm_cb); *pending_out = NULL; msgb_free(msg); return IOFD_SEG_ACT_DEFER; } - extra_len = received_len - expected_len; + extra_len = received_len_post_segm_cb - expected_len; /* No segmentation needed, return the whole msgb */ if (extra_len == 0) { *pending_out = NULL; -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33654 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I5ff2f1416f294ac84b4240fd96da4aa3255fb27b Gerrit-Change-Number: 33654 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-MessageType: newchange

1 0