Attention is currently required from: neels, pespin, dexter.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31145 )
Change subject: bts: add IMMEDIATE ASSIGNMENT via PCH transmission
......................................................................
Patch Set 19:
(3 comments)
File include/osmocom/pcu/pcuif_proto.h:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/3f5ea786_31b7394e
PS19, Line 279: uint8_t pgroup;
> And AFAIR, yes, it can be bigger than 255.
Here I meant the `IMSI % 1000`, not the actual paging group.
File include/osmocom/pcu/pcuif_proto.h:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/eb3a5f50_14b833c6
PS18, Line 43: PCU_IF_SAPI_PCH_DT
> Yes, its a bit ugly, but we need to stay backwards compatible at least until we updated OsmoBTS as w […]
The patch for `osmo-bts.git` is already in Gerrit (currently WIP to avoid merging before this one). So I see no real need to stay backwards compatible anymore. Where we really need to stay backwards compatible is in `osmo-ttcn3-hacks.git`.
File include/osmocom/pcu/pcuif_proto.h:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/3b015c98_f1c85263
PS17, Line 280: uint8_t pgroup[3];
> Its indeed much simpler. Actually the paging group is only one byte in reality. […]
As per 3GPP TS 45.002 section 6.5.2 it's defined as follows:
```
CCCH_GROUP (0 .. BS_CC_CHANS-1) = ((IMSI mod 1000) mod (BS_CC_CHANS x N)) div N
PAGING_GROUP (0 .. N-1) = ((IMSI mod 1000) mod (BS_CC_CHANS x N)) mod N
```
so it can be in range `0 .. N-1`, where `N` is defined as:
```
N = number of paging blocks "available" on one CCCH = (number of paging blocks "available" in a 51-multiframe on one CCCH) x BS_PA_MFRMS.
```
We have defines for this, so `N(max) = MAX_PAGING_BLOCKS_CCCH * MAX_BS_PA_MFRMS = 81`, therefore the paging group can be in range `0..80`.
But again, what we have in osmo-pcu **is not really a paging group**.
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31145
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I2a78651593323e8b9627c39918d949a33497b70f
Gerrit-Change-Number: 31145
Gerrit-PatchSet: 19
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-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 01 Mar 2023 21:23:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: neels, pespin, dexter.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31145 )
Change subject: bts: add IMMEDIATE ASSIGNMENT via PCH transmission
......................................................................
Patch Set 19: Code-Review-1
(3 comments)
File include/osmocom/pcu/pcuif_proto.h:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/f8017333_0221b78a
PS19, Line 279: uint8_t pgroup;
> isn't pgroup going 0-999? that doesn't fit in a 8bit one (256)
This is wrong. You cannot calculate a paging group in the PCU because you need to know more than just an IMSI to calculate it. And AFAIR, yes, it can be bigger than 255.
File src/pcu_l1_if.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/c8466ab7_9609466b
PS19, Line 298: pgroup
I think this is where the confusion comes from. It's not a *paging group*, it's just three last digits of the IMSI. The argument needs to be renamed [in a separate patch]. See the related discussion: https://gerrit.osmocom.org/c/osmo-bts/+/31600/comments/edd450d1_1ff57336.
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/94eac1ed_12c6281a
PS19, Line 771: } else {
> you cannot reach here according to if block in line 752.
He's adding `info_ind->version != 0x0a` there, so it can be reached.
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31145
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I2a78651593323e8b9627c39918d949a33497b70f
Gerrit-Change-Number: 31145
Gerrit-PatchSet: 19
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-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 01 Mar 2023 20:53:46 +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: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/docker-playground/+/31623 )
Change subject: Introduce debian-bullseye-titan-master
......................................................................
Patch Set 1:
(7 comments)
File debian-bullseye-titan-master/Dockerfile:
https://gerrit.osmocom.org/c/docker-playground/+/31623/comment/bcc6f848_ca8…
PS1, Line 32: Install titan.core dependencies
How about using `apt-get build-dep` for that? This way we [hopefully] always install all dependencies, which may be added by the upstream any time. This is what we do for `open5gs-master`.
https://gerrit.osmocom.org/c/docker-playground/+/31623/comment/645aa82f_3f8…
PS1, Line 74: j8
`$nproc`?
https://gerrit.osmocom.org/c/docker-playground/+/31623/comment/6571a813_6bb…
PS1, Line 84: Debian folks updated the gcc version but not titan
Is this still relevant when building titan from source?
https://gerrit.osmocom.org/c/docker-playground/+/31623/comment/5c151fc5_ad8…
PS1, Line 94: wget
Why not using Docker's `ADD` here to fetch a file? The benefit of using it is that the cache will be invalidated automatically when a new version of `libfftranscode` is available on the server. Also, I suggest making the lib version configurable via an environment variable, so that it's easier to upgrade if needed.
File debian-bullseye-titan-master/Makefile:
https://gerrit.osmocom.org/c/docker-playground/+/31623/comment/290cb100_ad7…
PS1, Line 1: ../debian-bullseye-titan/Makefile
"No newline at end of right file."
File debian-bullseye-titan-master/ttcn3-docker-prepare.sh:
https://gerrit.osmocom.org/c/docker-playground/+/31623/comment/a41f91df_8fb…
PS1, Line 1: #!/bin/sh -e
Might be a good idea to move this file to `common/` and `ADD` it from there, to avoid code duplication. Stuff like `respawn.sh` and `Release.key` is already there.
File debian-bullseye-titan-master/ttcn3-docker-run.sh:
https://gerrit.osmocom.org/c/docker-playground/+/31623/comment/c5358b3c_f13…
PS1, Line 1: #!/bin/bash
Same here, move this file to `common/`.
--
To view, visit https://gerrit.osmocom.org/c/docker-playground/+/31623
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-Change-Id: I19ee98a319ccad167d06c4f183fe80ecac909483
Gerrit-Change-Number: 31623
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 01 Mar 2023 20:41:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: neels, fixeria, dexter.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31145 )
Change subject: bts: add IMMEDIATE ASSIGNMENT via PCH transmission
......................................................................
Patch Set 19:
(5 comments)
File include/osmocom/pcu/pcuif_proto.h:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/8cf4630a_56bc9d04
PS19, Line 279: uint8_t pgroup;
isn't pgroup going 0-999? that doesn't fit in a 8bit one (256)
File src/bts.h:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/9dbd2666_6b5bb5ab
PS19, Line 284: bool use_dt_confirm;
I think it would be best to simply drop this bool, and explicitly check against received info_ind PCUIF version.
File src/bts.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/f3e67c16_20a00afd
PS17, Line 1134: pcu_l1if_tx_pch(bts, immediate_assignment, plen, ms_paging_group(tbf_ms(tbf)));
> Here is the related ticket: https://osmocom. […]
I like the idea of supporting both 0x0a and 0x0b in osmo-pcu, that's great. I edited this comment because I was proposing exactly that and found out it's implemented in the next file in gerrit. Great!
File src/pcu_l1_if.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/578fe4e7_3a8347c0
PS19, Line 769: LOGP(DL1IF, LOGL_NOTICE, "selected IMMEDIATE assignment confirmation method (MAC block as reference) is deprecated. OS#5927\n");
I think in here simply state that "PCUIF version 10 is deprecated" would be more interesting.
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/41a2c6e1_3bf49425
PS19, Line 771: } else {
you cannot reach here according to if block in line 752.
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31145
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I2a78651593323e8b9627c39918d949a33497b70f
Gerrit-Change-Number: 31145
Gerrit-PatchSet: 19
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-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 01 Mar 2023 18:14:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: neels, fixeria, pespin.
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31145 )
Change subject: bts: add IMMEDIATE ASSIGNMENT via PCH transmission
......................................................................
Patch Set 19:
(5 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/ca552eee_6f3870ed
PS18, Line 7: bts: add IMMEDIATE ASSIGNMENT via PCH transmission
> The commit message contains nothing about bumping the PCUIF version. […]
Done
File include/osmocom/pcu/pcuif_proto.h:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/7293ca45_e074be67
PS18, Line 43: PCU_IF_SAPI_PCH_DT
> Didn't we agree that in the new PCUIF version there will be only one way to send DL blocks over PCH […]
Yes, its a bit ugly, but we need to stay backwards compatible at least until we updated OsmoBTS as well. I will do this as soon as possible.
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/db32aad3_21158176
PS18, Line 273: is send
> typo: is sent
Done
File include/osmocom/pcu/pcuif_proto.h:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/c0413f83_80134689
PS17, Line 280: uint8_t pgroup[3];
> Wait a minute, I am confused by this field. […]
Its indeed much simpler. Actually the paging group is only one byte in reality. I have checked this back with the spec. I don't know why we use uint16_t or even unsigned int in other places. This probably has historical reasons.
File src/bts.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/2b7bfb49_4487656e
PS17, Line 1134: pcu_l1if_tx_pch(bts, immediate_assignment, plen, ms_paging_group(tbf_ms(tbf)));
> Here is the BTS side patch: https://gerrit.osmocom.org/c/osmo-bts/+/31600 [WIP]. […]
Here is the related ticket: https://osmocom.org/issues/5927
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31145
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I2a78651593323e8b9627c39918d949a33497b70f
Gerrit-Change-Number: 31145
Gerrit-PatchSet: 19
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-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 01 Mar 2023 16:51:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: neels, fixeria, pespin.
Hello Jenkins Builder, laforge, fixeria, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-pcu/+/31145
to look at the new patch set (#19).
Change subject: bts: add IMMEDIATE ASSIGNMENT via PCH transmission
......................................................................
bts: add IMMEDIATE ASSIGNMENT via PCH transmission
In situations where the PCU is co-located to the BSC, the IMMEDIATE ASSIGNMENT
for downlink TBFs must be sent via RSL and the BSC also must instruct the BTS
to transmit the IMMEDIATE ASSIGNMENT via PCH instead of AGCH. Eventually the
BSC must sent a confirmation message (follow-up patch) where the TLLI is used
as an identifer.
This new method will eventually replace the previous method that uses
the MAC block as an identifier. To remain compatible with older versions
of osmo-bsc, we will keep the old method until osmo-bts is migrated as
well.
This patch also requires new features to be added to the PCU socket
interface the version number of the protocol is incremented from 0x0a to
0x0b. Version 0x0a will remain compatible and use the old method, while
version 0x0b will use the new method introduced with this patch.
Change-Id: I2a78651593323e8b9627c39918d949a33497b70f
Related: OS#5198
---
M include/osmocom/pcu/pcuif_proto.h
M src/bts.cpp
M src/bts.h
M src/pcu_l1_if.cpp
M src/pcu_l1_if.h
5 files changed, 83 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/45/31145/19
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31145
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I2a78651593323e8b9627c39918d949a33497b70f
Gerrit-Change-Number: 31145
Gerrit-PatchSet: 19
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-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: osmith.
Hello Jenkins Builder, laforge, fixeria,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/docker-playground/+/31598
to look at the new patch set (#3).
Change subject: docker kill: wait until containers are stopped
......................................................................
docker kill: wait until containers are stopped
As "docker kill" / "docker container kill" (alias) doesn't block until
the given container stops, make sure to always run "docker wait"
afterwards.
Closes: OS#5928
Change-Id: I0242ece96541d8036ebbf8b0f498ebf231db26b5
---
M jenkins-common.sh
M osmo-cn-latest/run.sh
M osmo-ran/split/jenkins-split.sh
M scripts/regen_doc.sh
M ttcn3-bts-test/jenkins.sh
M ttcn3-fr-test/jenkins.sh
M ttcn3-hnbgw-test/jenkins.sh
M ttcn3-hnodeb-test/jenkins.sh
M ttcn3-remsim-test/jenkins.sh
9 files changed, 41 insertions(+), 19 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/docker-playground refs/changes/98/31598/3
--
To view, visit https://gerrit.osmocom.org/c/docker-playground/+/31598
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: docker-playground
Gerrit-Branch: master
Gerrit-Change-Id: I0242ece96541d8036ebbf8b0f498ebf231db26b5
Gerrit-Change-Number: 31598
Gerrit-PatchSet: 3
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-MessageType: newpatchset