Attention is currently required from: neels, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34573?usp=email )
Change subject: bsc: Use different CIC in concurrent calls from same MSC
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34573?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: Iac09590693bf39ccc90bf2b0645ca672a4787c95
Gerrit-Change-Number: 34573
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 02 Oct 2023 09:28:07 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: arehbein, fixeria.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33201?usp=email )
Change subject: stream: Add and use IPA send function
......................................................................
Patch Set 20: Code-Review+1
(1 comment)
File src/ipa.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/98abd5de_d91c8df1
PS20, Line 456: osmo_ipa_stream_srv_send
> TBH, I don't really see the benefits of having this API. […]
I agree with fixeria here, I already shared my concerns about this kind of API ;)
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33201?usp=email
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: 20
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 02 Oct 2023 09:21:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: arehbein, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33201?usp=email )
The change is no longer submittable: Code-Review is unsatisfied now.
Change subject: stream: Add and use IPA send function
......................................................................
Patch Set 20: Code-Review-2
(4 comments)
File include/osmocom/netif/ipa.h:
https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/d05bd983_5908bc94
PS20, Line 6: #include <osmocom/netif/stream.h>
This header file contains a protocol definition, and making it contain the whole stream API does not look right to me. AFAIU, this header needed for `struct osmo_stream_srv` pointers? If so, you could just add a forward declaration instead of including the whole header.
File src/ipa.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/2090a5d5_b859887f
PS20, Line 443: struct msgb *msg
If we go for exposing this API (see my other comment), the `msg` pointer should become the first argument for the sake of consistency with other API.
https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/929bbed7_f908aea5
PS20, Line 456: osmo_ipa_stream_srv_send
TBH, I don't really see the benefits of having this API. It's just three lines of code (or even two, if we exclude the assert) in both cases. Where exactly and how much this API is going to be used?
My concern is that neither it's purely a `stream_{srv,cli}` API, nor purely an IPA specific API - it's a mix of both. And it's not like you're saving a lot of effort by using it.
As a compromise, I would suggest adding only the `[osmo_]ipa_push_headers()` function, but not `osmo_ipa_stream_{srv,cli}_send()`. Below is how a typical use of this API would look like:
```
osmo_ipa_push_headers(msg, IPAC_PROTO_OSMO, IPAC_PROTO_EXT_CTRL);
osmo_stream_{srv, cli}_send({conn, cli}, msg);
```
File tests/stream/stream_test.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/b35d9c86_6db1628c
PS14, Line 582: osmo_ipa_stream_srv_send(conn, IPAC_PROTO_IPACCESS, -1, m);
> I still think mixing filling the msgb (just 1-2 API calls) + sending is a bad idea, but since others […]
Agreeing with @pespin@sysmocom.de here.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33201?usp=email
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: 20
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 02 Oct 2023 09:17:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: dexter, fixeria, laforge.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34583?usp=email )
Change subject: pcuif_proto: rename PCU_IF_FLAG_SYSMO to PCU_IF_FLAG_DIRECT_PHY
......................................................................
Patch Set 1:
(1 comment)
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34583/comment/da057570_e101c92f
PS1, Line 203: info_ind->flags |= PCU_IF_FLAG_DIRECT_PHY
> (unrelated to this patch) I guess this needs to be configurable somehow? We cannot blindly assume he […]
For now it must be for sure direct phy access in osmo-bsc, because we don't support any way to forward data to the bts/cell from osmo-bsc.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34583?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I23df067df99b76048667131905c4448d32d80640
Gerrit-Change-Number: 34583
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 02 Oct 2023 09:06:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: dexter, laforge.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34583?usp=email )
Change subject: pcuif_proto: rename PCU_IF_FLAG_SYSMO to PCU_IF_FLAG_DIRECT_PHY
......................................................................
Patch Set 1:
(1 comment)
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34583/comment/896176c8_7a4d70ad
PS1, Line 203: info_ind->flags |= PCU_IF_FLAG_DIRECT_PHY
(unrelated to this patch) I guess this needs to be configurable somehow? We cannot blindly assume here that the PCU process is having direct PHY access, right?
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34583?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I23df067df99b76048667131905c4448d32d80640
Gerrit-Change-Number: 34583
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 02 Oct 2023 08:34:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment