Attention is currently required from: laforge, pespin.
arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33197 )
Change subject: stream: Add server-side (segmentation) support for IPAC
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
> IIUC, the only that actually matters here for the user of the osmo_stream is adding a specific segme […]
This also determines that the IPA specific read mode added in later commits of this patch series will be used. I could add something like:
"Will also cause the stream backend to use the IPA-specific read mode added in change <change hash>"
Concerning the design choice: I understood the task given to me as specifically adding this functionality to the stream API. Maybe it's because IPA is widely used along with the stream API by different upper layers (?)
I've CCed Daniel so he can chime in because he has the wide background knowledge to the task.
--
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: 5
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-Comment-Date: Fri, 30 Jun 2023 14:24:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: arehbein.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33206 )
Change subject: stream (cosmetic): Fix osmo_panic log fmts
......................................................................
Patch Set 5: Code-Review+1
(1 comment)
Patchset:
PS5:
You probably want to submit this in a separate thread or move it to the start of the thread so that we can merge it quickly :)
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33206
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: Id082a9473b788f8de20cdc2ba4430b3289f4ce5a
Gerrit-Change-Number: 33206
Gerrit-PatchSet: 5
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Jun 2023 13:50:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: arehbein.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33205 )
Change subject: example: Remove call to osmo_ipa_process_msg()
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
why do it that way? why is it needed? why is it better than having it properly separated in multiple layers?
--
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: 5
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-Comment-Date: Fri, 30 Jun 2023 13:49:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: arehbein.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33203 )
Change subject: stream: Move helper functions
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
Isn't this being added in one of the previous patches in this patchset? squash it into it?
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33203
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I318965538e5329c44d0910694621b5e1f1db0626
Gerrit-Change-Number: 33203
Gerrit-PatchSet: 5
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Jun 2023 13:47:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: osmith.
arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-dev/+/33381 )
Change subject: src/grd: Add option for cherry-picking
......................................................................
Patch Set 2:
(3 comments)
File src/grd:
https://gerrit.osmocom.org/c/osmo-dev/+/33381/comment/2e9f24d7_d2535db4
PS1, Line 73: cmd = ["git", "cherry-pick", "FETCH_HEAD"];
> for consistency: let's also print the command, like in git_checkout_fetch_head()
Done
https://gerrit.osmocom.org/c/osmo-dev/+/33381/comment/06bb8847_541c5473
PS1, Line 95: , disabled by default
> I'd remove the ", disabled by default" part, it is redundant as this is an optional parameter and th […]
Done
https://gerrit.osmocom.org/c/osmo-dev/+/33381/comment/efdc5702_d11bee7a
PS1, Line 112: git_cherry_pick_fetch_head()
> I'd switch the logic, so the non-inverted case comes first: […]
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-dev/+/33381
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-dev
Gerrit-Branch: master
Gerrit-Change-Id: I85b1a2c4915e3da374e4b1201f2e977708fc7c4c
Gerrit-Change-Number: 33381
Gerrit-PatchSet: 2
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Jun 2023 13:46:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: arehbein.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33201 )
Change subject: stream: Add IPA send function/IPA-mode read to srv
......................................................................
Patch Set 5: Code-Review-1
(1 comment)
Patchset:
PS5:
I'm totally lost here. Why are we adding tons of IPA specific stuff to a generic osmo_stream API? All this at least is lacking proper rationale in the commit message to start with.
--
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: 5
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-Comment-Date: Fri, 30 Jun 2023 13:46:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: arehbein.
Hello osmith,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-dev/+/33381
to look at the new patch set (#2).
Change subject: src/grd: Add option for cherry-picking
......................................................................
src/grd: Add option for cherry-picking
Option '-c|--cherry-pick' will now cherry-pick into
the current branch if passed.
Change-Id: I85b1a2c4915e3da374e4b1201f2e977708fc7c4c
---
M src/grd
1 file changed, 27 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-dev refs/changes/81/33381/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-dev/+/33381
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-dev
Gerrit-Branch: master
Gerrit-Change-Id: I85b1a2c4915e3da374e4b1201f2e977708fc7c4c
Gerrit-Change-Number: 33381
Gerrit-PatchSet: 2
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: arehbein.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33202 )
Change subject: examples: Add extension header octet to example
......................................................................
Patch Set 5: Code-Review+1
--
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: 5
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-Comment-Date: Fri, 30 Jun 2023 13:43:24 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: arehbein, laforge.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33197 )
Change subject: stream: Add server-side (segmentation) support for IPAC
......................................................................
Patch Set 5: Code-Review-1
(1 comment)
Patchset:
PS5:
IIUC, the only that actually matters here for the user of the osmo_stream is adding a specific segmentation callback specific to IPAC. So I'd really avoid adding IPAC specific information in osmo_stream, and simply add a set_segmentation_callback() where the user wishing to establish an IPA stream can pass the relevant callback.
And tbh, I'm not even sure why this segmentation callback needs to be done in osmo_stream, to me it belongs to a layer on top of it.
I'm happy to discuss or be convinced otherwise.
--
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: 5
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-Comment-Date: Fri, 30 Jun 2023 13:38:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment