Attention is currently required from: neels, laforge, pespin, fixeria, dexter.
iedemam has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28276 )
Change subject: Expand VTY option which controls use of TCH for signalling
......................................................................
Patch Set 9:
(2 comments)
File src/osmo-bsc/abis_rsl.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/28276/comment/2260fcd7_6ba924cf
PS8, Line 2182: (bts->chan_alloc_tch_signalling_policy == BTS_TCH_SIGNALLING_VOICE &&
> Alignment should be done using a few spaces to end up under "b" character here from previous line he […]
Picky is good. Picky pays off in the long run.
File src/osmo-bsc/bts_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/28276/comment/d80973ef_5de409c4
PS8, Line 593: DEFUN_DEPRECATED(cfg_bts_chan_alloc_allow_tch_for_signalling,
> Better use DEFUN_ATTR(... […]
Ah, thanks. I wasn't really sure of the differences in that set of macros and defines.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28276
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I4459941ddad4e4a3bec8409b180d9a23a735e640
Gerrit-Change-Number: 28276
Gerrit-PatchSet: 9
Gerrit-Owner: iedemam <michael(a)kapsulate.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 15 Jun 2022 13:20:51 +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: iedemam, neels, laforge, fixeria, dexter.
Hello Jenkins Builder, neels, laforge, fixeria, dexter,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bsc/+/28276
to look at the new patch set (#9).
Change subject: Expand VTY option which controls use of TCH for signalling
......................................................................
Expand VTY option which controls use of TCH for signalling
For statistical clarity and site tuning, it is sometimes
desirable to completely disable the use of TCH for signaling.
In the existing version of this VTY command, there is no way to
accomplish this. We can only restrict TCH for signaling non-voice
related actions.
This patch deprecates 'allow-tch-for-signalling (0|1)' and
adds 'tch-signalling-policy (never|emergency|voice|always)' to
provide more options.
Change-Id: I4459941ddad4e4a3bec8409b180d9a23a735e640
---
M include/osmocom/bsc/bts.h
M src/osmo-bsc/abis_rsl.c
M src/osmo-bsc/bts.c
M src/osmo-bsc/bts_vty.c
M tests/osmo-bsc.vty
5 files changed, 82 insertions(+), 22 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/76/28276/9
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28276
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I4459941ddad4e4a3bec8409b180d9a23a735e640
Gerrit-Change-Number: 28276
Gerrit-PatchSet: 9
Gerrit-Owner: iedemam <michael(a)kapsulate.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: iedemam <michael(a)kapsulate.com>
Gerrit-Attention: neels <nhofmeyr(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-MessageType: newpatchset
Attention is currently required from: iedemam, neels, laforge, fixeria, dexter.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28276 )
Change subject: Expand VTY option which controls use of TCH for signalling
......................................................................
Patch Set 8:
(3 comments)
Patchset:
PS8:
From my side fix the 2 small things and I'm happy with it.
File src/osmo-bsc/abis_rsl.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/28276/comment/12550a5a_acc5aa69
PS8, Line 2182: (bts->chan_alloc_tch_signalling_policy == BTS_TCH_SIGNALLING_VOICE &&
Alignment should be done using a few spaces to end up under "b" character here from previous line here.
I'm sorry to be picky here but this code path is quite an important one in channel allocation so I want to have it as clear as possible.
File src/osmo-bsc/bts_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/28276/comment/7f1cfd3b_bd863d1d
PS8, Line 593: DEFUN_DEPRECATED(cfg_bts_chan_alloc_allow_tch_for_signalling,
Better use DEFUN_ATTR(..., CMD_ATTR_IMMEDIATE|CMD_ATTR_DEPRECATED) here instead of "DEFUN_DEPRECTATED()", to avoid lossing the "immediate" flag.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28276
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I4459941ddad4e4a3bec8409b180d9a23a735e640
Gerrit-Change-Number: 28276
Gerrit-PatchSet: 8
Gerrit-Owner: iedemam <michael(a)kapsulate.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: iedemam <michael(a)kapsulate.com>
Gerrit-Attention: neels <nhofmeyr(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: Wed, 15 Jun 2022 12:19:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: laforge, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/28286 )
Change subject: add libosmo-gtlv, moved from osmo-upf.git
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
> I agree with @laforge, let's keep same structure for all libs: […]
- I thought it would be quick, but i've already spent hours on that, it is frustrating. various tasks are stalled because of this, and more and more trivial nonsense seems to show up around every corner. I'd rather do useful work.
- Changing naming just because the repository changes is completely against my opinion. Removing the dash needs changes in libosmo-pfcp, osmo-upf as well as osmo-hnbgw. I don't want that to happen, moving to another repos should not require changes to the users. Ideally we move the Makefile.am stuff 1:1.
Having a separate libosmo-gtlv.git
- does not affect libosmocore
- does not require build style and naming changes
Least grunt work is having gtlv in libosmo-pfcp.git,
we postpone this discussion and don't proliferate overhead needed for
build jobs, git mirrors etc.
I am not convinced that any of this is worth any time spent in the first place, i feel this is spinning off on a tangent.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28286
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I25ab400f0c5707fdc0d8e480aca19871c2e26e71
Gerrit-Change-Number: 28286
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 15 Jun 2022 11:27:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: neels, laforge, fixeria, pespin, dexter.
iedemam has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28276 )
Change subject: Expand VTY option which controls use of TCH for signalling
......................................................................
Patch Set 8:
(6 comments)
Patchset:
PS8:
Hi Pau,
Good points, thanks! Hopefully I've addressed them now.
-Michael
File include/osmocom/bsc/bts.h:
https://gerrit.osmocom.org/c/osmo-bsc/+/28276/comment/7f3e4057_31dbf5ce
PS5, Line 263: BTS_TCH_SIGNALLING_NEVER,
> Maybe NEVER->NONE and ALWAYS->ALL would be more meanginful, but not really sure about it, so letting […]
The description in VTY phrases it as "when can we use TCH" and so I named these accordingly.
File src/osmo-bsc/abis_rsl.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/28276/comment/f2276593_65d89920
PS5, Line 2177: /* else: Emergency calls will be put on a free TCH/H or TCH/F directly
> We should probably discuss this. […]
Agreed. I have now added this.
https://gerrit.osmocom.org/c/osmo-bsc/+/28276/comment/4d7ac89b_f144235d
PS5, Line 2181: if (bts->tch_for_signalling_policy == BTS_TCH_SIGNALLING_ALWAYS
> If you don't mind, this would be probably more readable, since the same check is done first: […]
Done
File src/osmo-bsc/bts_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/28276/comment/ea7c36d4_f688aacd
PS5, Line 568: chan_alloc_allow
> Why are you removing this part? […]
This have been renamed back to contain "chan_alloc", That was a mistake on my part not realizing the name referred to VTY text as well.
https://gerrit.osmocom.org/c/osmo-bsc/+/28276/comment/eba27a84_61d1d918
PS5, Line 582: !strcmp(argv[0], "0")
> argv[0] can not be "0" or "1" anymore (because you've changed the command vector from '0|1' to 'neve […]
I have added a DEPRECATED version of the command. Maybe it is still not correct?
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28276
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I4459941ddad4e4a3bec8409b180d9a23a735e640
Gerrit-Change-Number: 28276
Gerrit-PatchSet: 8
Gerrit-Owner: iedemam <michael(a)kapsulate.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 15 Jun 2022 11:12:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: iedemam, neels, laforge, dexter.
Hello Jenkins Builder, neels, laforge, fixeria, dexter,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bsc/+/28276
to look at the new patch set (#8).
Change subject: Expand VTY option which controls use of TCH for signalling
......................................................................
Expand VTY option which controls use of TCH for signalling
For statistical clarity and site tuning, it is sometimes
desirable to completely disable the use of TCH for signaling.
In the existing version of this VTY command, there is no way to
accomplish this. We can only restrict TCH for signaling non-voice
related actions.
This patch deprecates 'allow-tch-for-signalling (0|1)' and
adds 'tch-signalling-policy (never|emergency|voice|always)' to
provide more options.
Change-Id: I4459941ddad4e4a3bec8409b180d9a23a735e640
---
M include/osmocom/bsc/bts.h
M src/osmo-bsc/abis_rsl.c
M src/osmo-bsc/bts.c
M src/osmo-bsc/bts_vty.c
M tests/osmo-bsc.vty
5 files changed, 83 insertions(+), 24 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/76/28276/8
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28276
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I4459941ddad4e4a3bec8409b180d9a23a735e640
Gerrit-Change-Number: 28276
Gerrit-PatchSet: 8
Gerrit-Owner: iedemam <michael(a)kapsulate.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: iedemam <michael(a)kapsulate.com>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: iedemam, neels, laforge, dexter.
Hello Jenkins Builder, neels, laforge, fixeria, dexter,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bsc/+/28276
to look at the new patch set (#7).
Change subject: Expand VTY option which controls use of TCH for signalling
......................................................................
Expand VTY option which controls use of TCH for signalling
For statistical clarity and site tuning, it is sometimes
desirable to completely disable the use of TCH for signaling.
In the existing version of this VTY command, there is no way to
accomplish this. We can only restrict TCH for signaling non-voice
related actions.
This patch deprecates 'allow-tch-for-signalling (0|1)' and
adds 'tch-signalling-policy (never|voice|always)' to provide
more options.
Change-Id: I4459941ddad4e4a3bec8409b180d9a23a735e640
---
M include/osmocom/bsc/bts.h
M src/osmo-bsc/abis_rsl.c
M src/osmo-bsc/bts.c
M src/osmo-bsc/bts_vty.c
M tests/osmo-bsc.vty
5 files changed, 85 insertions(+), 24 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/76/28276/7
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28276
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I4459941ddad4e4a3bec8409b180d9a23a735e640
Gerrit-Change-Number: 28276
Gerrit-PatchSet: 7
Gerrit-Owner: iedemam <michael(a)kapsulate.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: iedemam <michael(a)kapsulate.com>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: newpatchset